From 55513cc99866ae7125ed786451a7dc37ee5a28d3 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 22 Mar 2015 07:44:14 +0200 Subject: [PATCH 1/6] Improved modutil_get_complete_packets to only allocate a single buffer. --- server/core/modutil.c | 42 +++++++++++++++++++++++-- server/modules/protocol/mysql_backend.c | 25 +++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/server/core/modutil.c b/server/core/modutil.c index 6362d70a0..f4a0a4e4e 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -537,12 +537,48 @@ return_packetbuf: */ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) { - GWBUF *buff = NULL, *packet = NULL; + GWBUF *buff = NULL, *packet; + uint8_t *ptr,*end; + unsigned int len,total = 0; + + if(p_readbuf == NULL || (*p_readbuf) == NULL || + gwbuf_length(*p_readbuf) < 3) + return NULL; + + packet = *p_readbuf; + ptr = (uint8_t*)packet->start; + end = (uint8_t*)packet->end; + len = gw_mysql_get_byte3(ptr) + 4; - while((packet = modutil_get_next_MySQL_packet(p_readbuf)) != NULL) + if(ptr + len >= end) + return NULL; + + while(ptr + len < end) { - buff = gwbuf_append(buff,packet); + ptr += len; + total += len; + len = gw_mysql_get_byte3(ptr) + 4; } + + /** Full packets only, return original */ + if(total == gwbuf_length(packet)) + return packet; + + /** The next packet is a partial, split into complete and partial packets */ + total -= len; + + if(buff = gwbuf_alloc(total)) + { + skygw_log_write(LOGFILE_ERROR, + "Error: Failed to allocate new buffer " + " of %d bytes while splitting buffer" + " into complete packets.", + total); + return NULL; + } + + memcpy(buff->start,packet->start,total); + gwbuf_consume(*p_readbuf,total); return buff; } diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index 2096f937d..7fff6871f 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -493,6 +493,31 @@ static int gw_read_backend_event(DCB *dcb) { ss_dassert(read_buffer != NULL || dcb->dcb_readqueue != NULL); } + read_buffer = gwbuf_append(read_buffer,dcb->dcb_readqueue); + nbytes_read = gwbuf_length(read_buffer); + + if (nbytes_read < 3) + { + dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, read_buffer); + rc = 0; + goto return_rc; + } + + { + GWBUF *tmp = modutil_get_complete_packets(&read_buffer); + + if(tmp == NULL) + { + dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, read_buffer); + rc = 0; + goto return_rc; + + } + + dcb->dcb_readqueue = read_buffer; + read_buffer = tmp; + } + /** * If protocol has session command set, concatenate whole * response into one buffer. From e0ce987d72e68f08a47db8d84e23a6d91fc4162a Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 22 Mar 2015 08:56:18 +0200 Subject: [PATCH 2/6] Mysql_backend now only sends complete packets from the backend servers. --- server/core/buffer.c | 2 ++ server/core/modutil.c | 24 +++++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/server/core/buffer.c b/server/core/buffer.c index fa2fa8d13..4c788cdd2 100644 --- a/server/core/buffer.c +++ b/server/core/buffer.c @@ -336,6 +336,8 @@ gwbuf_append(GWBUF *head, GWBUF *tail) { if (!head) return tail; + if(!tail) + return head; CHK_GWBUF(head); head->tail->next = tail; head->tail = tail->tail; diff --git a/server/core/modutil.c b/server/core/modutil.c index f4a0a4e4e..20e57a9e6 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -539,19 +539,29 @@ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) { GWBUF *buff = NULL, *packet; uint8_t *ptr,*end; - unsigned int len,total = 0; + unsigned int len,blen,total = 0; if(p_readbuf == NULL || (*p_readbuf) == NULL || gwbuf_length(*p_readbuf) < 3) return NULL; - packet = *p_readbuf; + packet = gwbuf_make_contiguous(*p_readbuf); ptr = (uint8_t*)packet->start; end = (uint8_t*)packet->end; len = gw_mysql_get_byte3(ptr) + 4; if(ptr + len >= end) - return NULL; + { + if(len == gwbuf_length(*p_readbuf)) + { + *p_readbuf = NULL; + return packet; + } + else + { + return NULL; + } + } while(ptr + len < end) { @@ -561,13 +571,17 @@ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) } /** Full packets only, return original */ - if(total == gwbuf_length(packet)) + + if(total + len == (blen = gwbuf_length(packet))) + { + *p_readbuf = NULL; return packet; + } /** The next packet is a partial, split into complete and partial packets */ total -= len; - if(buff = gwbuf_alloc(total)) + if((buff = gwbuf_alloc(total)) == NULL) { skygw_log_write(LOGFILE_ERROR, "Error: Failed to allocate new buffer " From 6cfc2338c1bddcd7d416295c7e62289d3b985eb9 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 22 Mar 2015 10:09:58 +0200 Subject: [PATCH 3/6] Small fix to modutil_get_complete_packets. --- server/core/modutil.c | 27 +++++++++++-------------- server/modules/protocol/mysql_backend.c | 6 ++++-- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/server/core/modutil.c b/server/core/modutil.c index 20e57a9e6..e170f8fd0 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -539,31 +539,30 @@ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) { GWBUF *buff = NULL, *packet; uint8_t *ptr,*end; - unsigned int len,blen,total = 0; + int len,blen,total = 0; if(p_readbuf == NULL || (*p_readbuf) == NULL || gwbuf_length(*p_readbuf) < 3) return NULL; packet = gwbuf_make_contiguous(*p_readbuf); + *p_readbuf = packet; ptr = (uint8_t*)packet->start; end = (uint8_t*)packet->end; len = gw_mysql_get_byte3(ptr) + 4; + blen = gwbuf_length(packet); - if(ptr + len >= end) + if(ptr + len == end) { - if(len == gwbuf_length(*p_readbuf)) - { *p_readbuf = NULL; return packet; - } - else - { - return NULL; - } + } + else if(ptr + len > end) + { + return NULL; } - while(ptr + len < end) + while(total + len < blen) { ptr += len; total += len; @@ -572,14 +571,13 @@ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) /** Full packets only, return original */ - if(total + len == (blen = gwbuf_length(packet))) + if(total + len == blen) { *p_readbuf = NULL; return packet; } /** The next packet is a partial, split into complete and partial packets */ - total -= len; if((buff = gwbuf_alloc(total)) == NULL) { @@ -592,15 +590,14 @@ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) } memcpy(buff->start,packet->start,total); - gwbuf_consume(*p_readbuf,total); - + gwbuf_consume(packet,total); return buff; } /** * Count the number of EOF, OK or ERR packets in the buffer. Only complete * packets are inspected and the buffer is assumed to only contain whole packets. - * If partial packets are in the buffer, they are ingnored. The caller must handle the + * If partial packets are in the buffer, they are ignored. The caller must handle the * detection of partial packets in buffers. * @param reply Buffer to use * @param use_ok Whether the DEPRECATE_EOF flag is set diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index 7fff6871f..aa6f7fd05 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -492,8 +492,10 @@ static int gw_read_backend_event(DCB *dcb) { { ss_dassert(read_buffer != NULL || dcb->dcb_readqueue != NULL); } - - read_buffer = gwbuf_append(read_buffer,dcb->dcb_readqueue); + if(dcb->dcb_readqueue) + { + read_buffer = gwbuf_append(dcb->dcb_readqueue,read_buffer); + } nbytes_read = gwbuf_length(read_buffer); if (nbytes_read < 3) From b416455f4f44820caf545e7e946ae9a41d82770e Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 22 Mar 2015 16:53:51 +0200 Subject: [PATCH 4/6] Added missing type to GWBUF returned from mysql_backend. --- server/core/modutil.c | 14 +++++++------- server/modules/protocol/mysql_backend.c | 7 +++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/server/core/modutil.c b/server/core/modutil.c index e170f8fd0..a4209a09d 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -532,8 +532,8 @@ return_packetbuf: /** * Parse the buffer and split complete packets into individual buffers. * Any partial packets are left in the old buffer. - * @param p_readbuf Buffer to split - * @return Head of the chain of complete packets + * @param p_readbuf Buffer to split, set to NULL if no partial packets are left + * @return Head of the chain of complete packets, all in a single, contiguous buffer */ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) { @@ -546,18 +546,19 @@ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) return NULL; packet = gwbuf_make_contiguous(*p_readbuf); + packet->next = NULL; *p_readbuf = packet; ptr = (uint8_t*)packet->start; end = (uint8_t*)packet->end; len = gw_mysql_get_byte3(ptr) + 4; blen = gwbuf_length(packet); - if(ptr + len == end) + if(len == blen) { *p_readbuf = NULL; return packet; } - else if(ptr + len > end) + else if(len > blen) { return NULL; } @@ -570,7 +571,6 @@ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) } /** Full packets only, return original */ - if(total + len == blen) { *p_readbuf = NULL; @@ -578,7 +578,6 @@ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) } /** The next packet is a partial, split into complete and partial packets */ - if((buff = gwbuf_alloc(total)) == NULL) { skygw_log_write(LOGFILE_ERROR, @@ -588,7 +587,8 @@ GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) total); return NULL; } - + buff->next = NULL; + gwbuf_set_type(buff,GWBUF_TYPE_MYSQL); memcpy(buff->start,packet->start,total); gwbuf_consume(packet,total); return buff; diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index aa6f7fd05..ba5786851 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -492,15 +492,17 @@ static int gw_read_backend_event(DCB *dcb) { { ss_dassert(read_buffer != NULL || dcb->dcb_readqueue != NULL); } + if(dcb->dcb_readqueue) { read_buffer = gwbuf_append(dcb->dcb_readqueue,read_buffer); } + nbytes_read = gwbuf_length(read_buffer); if (nbytes_read < 3) { - dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, read_buffer); + dcb->dcb_readqueue = read_buffer; rc = 0; goto return_rc; } @@ -510,7 +512,8 @@ static int gw_read_backend_event(DCB *dcb) { if(tmp == NULL) { - dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, read_buffer); + /** No complete packets */ + dcb->dcb_readqueue = read_buffer; rc = 0; goto return_rc; From 9e1dc40bb549fdaf05fb4f8c12dc159c30ad4c41 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 23 Mar 2015 11:17:07 +0200 Subject: [PATCH 5/6] Removed walking of packets from mysql_backend and moved storing of partial packets to session command handling section. --- server/modules/protocol/mysql_backend.c | 41 ++++++++++--------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index ba5786851..3048c3d4a 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -492,35 +492,19 @@ static int gw_read_backend_event(DCB *dcb) { { ss_dassert(read_buffer != NULL || dcb->dcb_readqueue != NULL); } - - if(dcb->dcb_readqueue) - { - read_buffer = gwbuf_append(dcb->dcb_readqueue,read_buffer); - } - - nbytes_read = gwbuf_length(read_buffer); - - if (nbytes_read < 3) - { - dcb->dcb_readqueue = read_buffer; - rc = 0; - goto return_rc; - } + if (dcb->dcb_readqueue) { - GWBUF *tmp = modutil_get_complete_packets(&read_buffer); - - if(tmp == NULL) + if (read_buffer != NULL) { - /** No complete packets */ - dcb->dcb_readqueue = read_buffer; - rc = 0; - goto return_rc; - + read_buffer = gwbuf_append(dcb->dcb_readqueue, read_buffer); } - - dcb->dcb_readqueue = read_buffer; - read_buffer = tmp; + else + { + read_buffer = dcb->dcb_readqueue; + } + + nbytes_read = gwbuf_length(read_buffer); } /** @@ -530,6 +514,13 @@ static int gw_read_backend_event(DCB *dcb) { if (protocol_get_srv_command((MySQLProtocol *)dcb->protocol, false) != MYSQL_COM_UNDEFINED) { + if(nbytes_read < 5) + { + dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, read_buffer); + rc = 0; + goto return_rc; + } + read_buffer = process_response_data(dcb, read_buffer, nbytes_read); /** * Received incomplete response to session command. From 54d242758f3f8d55532dc383a57fd3b2c0077ab6 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 23 Mar 2015 11:55:03 +0200 Subject: [PATCH 6/6] Revert "Removed walking of packets from mysql_backend and moved storing of partial packets to session command handling section." This reverts commit 9e1dc40bb549fdaf05fb4f8c12dc159c30ad4c41. --- server/modules/protocol/mysql_backend.c | 41 +++++++++++++++---------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index 3048c3d4a..ba5786851 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -492,19 +492,35 @@ static int gw_read_backend_event(DCB *dcb) { { ss_dassert(read_buffer != NULL || dcb->dcb_readqueue != NULL); } + + if(dcb->dcb_readqueue) + { + read_buffer = gwbuf_append(dcb->dcb_readqueue,read_buffer); + } + + nbytes_read = gwbuf_length(read_buffer); + + if (nbytes_read < 3) + { + dcb->dcb_readqueue = read_buffer; + rc = 0; + goto return_rc; + } - if (dcb->dcb_readqueue) { - if (read_buffer != NULL) + GWBUF *tmp = modutil_get_complete_packets(&read_buffer); + + if(tmp == NULL) { - read_buffer = gwbuf_append(dcb->dcb_readqueue, read_buffer); + /** No complete packets */ + dcb->dcb_readqueue = read_buffer; + rc = 0; + goto return_rc; + } - else - { - read_buffer = dcb->dcb_readqueue; - } - - nbytes_read = gwbuf_length(read_buffer); + + dcb->dcb_readqueue = read_buffer; + read_buffer = tmp; } /** @@ -514,13 +530,6 @@ static int gw_read_backend_event(DCB *dcb) { if (protocol_get_srv_command((MySQLProtocol *)dcb->protocol, false) != MYSQL_COM_UNDEFINED) { - if(nbytes_read < 5) - { - dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, read_buffer); - rc = 0; - goto return_rc; - } - read_buffer = process_response_data(dcb, read_buffer, nbytes_read); /** * Received incomplete response to session command.