* [PATCH v5 0/1] COLO: handling of the full primary or secondary queue @ 2020-04-10 2:00 Derek Su 2020-04-10 2:00 ` [PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue() Derek Su 0 siblings, 1 reply; 5+ messages in thread From: Derek Su @ 2020-04-10 2:00 UTC (permalink / raw) To: qemu-devel Cc: lizhijian, chyang, Derek Su, jasowang, ctcheng, chen.zhang, jwsu1986 The series is to handle the full primary or secondary queue in colo-compare. Fix the "pkt" memory leak in packet_enqueue(). Reproduce steps: 1. Setup PVM and SVM both with NIC e1000 by the steps descripted in the wiki qemu/COLO 2. Run "iperf3 -s" in PVM 3. Run "iperf3 -c <PVM-IP> -t 7200" in client The memory usage of qemu-system-x86_64 increases as the PVM's QMP shows "qemu-system-x86_64: colo compare secondary queue size too big, drop packet". Please help to review, thanks. V5: - Replace the error_report of full queue with a trace event - Remove handling of the full primary or secondary queue which hurt network throughput too much V4: - Remove redundant flush of packets V3: - handling of the full primary or secondary queue according to the suggestion from Zhang Chen V2: - Fix incorrect patch format Derek Su (1): colo-compare: Fix memory leak in packet_enqueue() net/colo-compare.c | 23 +++++++++++++++-------- net/trace-events | 1 + 2 files changed, 16 insertions(+), 8 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue() 2020-04-10 2:00 [PATCH v5 0/1] COLO: handling of the full primary or secondary queue Derek Su @ 2020-04-10 2:00 ` Derek Su 2020-04-11 3:51 ` Zhang, Chen 2020-04-26 11:25 ` Lukas Straub 0 siblings, 2 replies; 5+ messages in thread From: Derek Su @ 2020-04-10 2:00 UTC (permalink / raw) To: qemu-devel Cc: lizhijian, chyang, Derek Su, jasowang, ctcheng, chen.zhang, jwsu1986 The patch is to fix the "pkt" memory leak in packet_enqueue(). The allocated "pkt" needs to be freed if the colo compare primary or secondary queue is too big. Replace the error_report of full queue with a trace event. Signed-off-by: Derek Su <dereksu@qnap.com> --- net/colo-compare.c | 23 +++++++++++++++-------- net/trace-events | 1 + 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 10c0239f9d..035e11d4d3 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -122,6 +122,10 @@ enum { SECONDARY_IN, }; +static const char *colo_mode[] = { + [PRIMARY_IN] = "primary", + [SECONDARY_IN] = "secondary", +}; static int compare_chr_send(CompareState *s, const uint8_t *buf, @@ -217,6 +221,7 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) ConnectionKey key; Packet *pkt = NULL; Connection *conn; + int ret; if (mode == PRIMARY_IN) { pkt = packet_new(s->pri_rs.buf, @@ -245,16 +250,18 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) } if (mode == PRIMARY_IN) { - if (!colo_insert_packet(&conn->primary_list, pkt, &conn->pack)) { - error_report("colo compare primary queue size too big," - "drop packet"); - } + ret = colo_insert_packet(&conn->primary_list, pkt, &conn->pack); } else { - if (!colo_insert_packet(&conn->secondary_list, pkt, &conn->sack)) { - error_report("colo compare secondary queue size too big," - "drop packet"); - } + ret = colo_insert_packet(&conn->secondary_list, pkt, &conn->sack); } + + if (!ret) { + trace_colo_compare_drop_packet(colo_mode[mode], + "queue size too big, drop packet"); + packet_destroy(pkt, NULL); + pkt = NULL; + } + *con = conn; return 0; diff --git a/net/trace-events b/net/trace-events index 02c13fd0ba..fa49c71533 100644 --- a/net/trace-events +++ b/net/trace-events @@ -12,6 +12,7 @@ colo_proxy_main(const char *chr) ": %s" # colo-compare.c colo_compare_main(const char *chr) ": %s" +colo_compare_drop_packet(const char *queue, const char *chr) ": %s: %s" colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d" colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d" colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s" -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue() 2020-04-10 2:00 ` [PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue() Derek Su @ 2020-04-11 3:51 ` Zhang, Chen 2020-04-26 11:25 ` Lukas Straub 1 sibling, 0 replies; 5+ messages in thread From: Zhang, Chen @ 2020-04-11 3:51 UTC (permalink / raw) To: Derek Su, qemu-devel@nongnu.org Cc: jasowang@redhat.com, jwsu1986@gmail.com, chyang@qnap.com, lizhijian@cn.fujitsu.com, ctcheng@qnap.com > -----Original Message----- > From: Derek Su <dereksu@qnap.com> > Sent: Friday, April 10, 2020 10:01 AM > To: qemu-devel@nongnu.org > Cc: Zhang, Chen <chen.zhang@intel.com>; lizhijian@cn.fujitsu.com; > jasowang@redhat.com; ctcheng@qnap.com; chyang@qnap.com; > jwsu1986@gmail.com; Derek Su <dereksu@qnap.com> > Subject: [PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue() > > The patch is to fix the "pkt" memory leak in packet_enqueue(). > The allocated "pkt" needs to be freed if the colo compare primary or > secondary queue is too big. > > Replace the error_report of full queue with a trace event. > Looks good for me. Thanks your contribution. Reviewed-by: Zhang Chen <chen.zhang@intel.com> Thanks Zhang Chen > Signed-off-by: Derek Su <dereksu@qnap.com> > --- > net/colo-compare.c | 23 +++++++++++++++-------- > net/trace-events | 1 + > 2 files changed, 16 insertions(+), 8 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c index > 10c0239f9d..035e11d4d3 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -122,6 +122,10 @@ enum { > SECONDARY_IN, > }; > > +static const char *colo_mode[] = { > + [PRIMARY_IN] = "primary", > + [SECONDARY_IN] = "secondary", > +}; > > static int compare_chr_send(CompareState *s, > const uint8_t *buf, @@ -217,6 +221,7 @@ static int > packet_enqueue(CompareState *s, int mode, Connection **con) > ConnectionKey key; > Packet *pkt = NULL; > Connection *conn; > + int ret; > > if (mode == PRIMARY_IN) { > pkt = packet_new(s->pri_rs.buf, @@ -245,16 +250,18 @@ static int > packet_enqueue(CompareState *s, int mode, Connection **con) > } > > if (mode == PRIMARY_IN) { > - if (!colo_insert_packet(&conn->primary_list, pkt, &conn->pack)) { > - error_report("colo compare primary queue size too big," > - "drop packet"); > - } > + ret = colo_insert_packet(&conn->primary_list, pkt, > + &conn->pack); > } else { > - if (!colo_insert_packet(&conn->secondary_list, pkt, &conn->sack)) { > - error_report("colo compare secondary queue size too big," > - "drop packet"); > - } > + ret = colo_insert_packet(&conn->secondary_list, pkt, > + &conn->sack); > } > + > + if (!ret) { > + trace_colo_compare_drop_packet(colo_mode[mode], > + "queue size too big, drop packet"); > + packet_destroy(pkt, NULL); > + pkt = NULL; > + } > + > *con = conn; > > return 0; > diff --git a/net/trace-events b/net/trace-events index > 02c13fd0ba..fa49c71533 100644 > --- a/net/trace-events > +++ b/net/trace-events > @@ -12,6 +12,7 @@ colo_proxy_main(const char *chr) ": %s" > > # colo-compare.c > colo_compare_main(const char *chr) ": %s" > +colo_compare_drop_packet(const char *queue, const char *chr) ": %s: %s" > colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d" > colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d" > colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, > const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, > spkt size = %d, ip_src = %s, ip_dst = %s" > -- > 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue() 2020-04-10 2:00 ` [PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue() Derek Su 2020-04-11 3:51 ` Zhang, Chen @ 2020-04-26 11:25 ` Lukas Straub 2020-04-27 3:11 ` Zhang, Chen 1 sibling, 1 reply; 5+ messages in thread From: Lukas Straub @ 2020-04-26 11:25 UTC (permalink / raw) To: Derek Su Cc: lizhijian, chyang, jasowang, qemu-devel, ctcheng, chen.zhang, jwsu1986 [-- Attachment #1: Type: text/plain, Size: 3097 bytes --] On Fri, 10 Apr 2020 10:00:56 +0800 Derek Su <dereksu@qnap.com> wrote: > The patch is to fix the "pkt" memory leak in packet_enqueue(). > The allocated "pkt" needs to be freed if the colo compare > primary or secondary queue is too big. > > Replace the error_report of full queue with a trace event. > > Signed-off-by: Derek Su <dereksu@qnap.com> Looks good now and works well in my tests. Reviewed-by: Lukas Straub <lukasstraub2@web.de> Tested-by: Lukas Straub <lukasstraub2@web.de> Regards, Lukas Straub > --- > net/colo-compare.c | 23 +++++++++++++++-------- > net/trace-events | 1 + > 2 files changed, 16 insertions(+), 8 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index 10c0239f9d..035e11d4d3 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -122,6 +122,10 @@ enum { > SECONDARY_IN, > }; > > +static const char *colo_mode[] = { > + [PRIMARY_IN] = "primary", > + [SECONDARY_IN] = "secondary", > +}; > > static int compare_chr_send(CompareState *s, > const uint8_t *buf, > @@ -217,6 +221,7 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) > ConnectionKey key; > Packet *pkt = NULL; > Connection *conn; > + int ret; > > if (mode == PRIMARY_IN) { > pkt = packet_new(s->pri_rs.buf, > @@ -245,16 +250,18 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) > } > > if (mode == PRIMARY_IN) { > - if (!colo_insert_packet(&conn->primary_list, pkt, &conn->pack)) { > - error_report("colo compare primary queue size too big," > - "drop packet"); > - } > + ret = colo_insert_packet(&conn->primary_list, pkt, &conn->pack); > } else { > - if (!colo_insert_packet(&conn->secondary_list, pkt, &conn->sack)) { > - error_report("colo compare secondary queue size too big," > - "drop packet"); > - } > + ret = colo_insert_packet(&conn->secondary_list, pkt, &conn->sack); > } > + > + if (!ret) { > + trace_colo_compare_drop_packet(colo_mode[mode], > + "queue size too big, drop packet"); > + packet_destroy(pkt, NULL); > + pkt = NULL; > + } > + > *con = conn; > > return 0; > diff --git a/net/trace-events b/net/trace-events > index 02c13fd0ba..fa49c71533 100644 > --- a/net/trace-events > +++ b/net/trace-events > @@ -12,6 +12,7 @@ colo_proxy_main(const char *chr) ": %s" > > # colo-compare.c > colo_compare_main(const char *chr) ": %s" > +colo_compare_drop_packet(const char *queue, const char *chr) ": %s: %s" > colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d" > colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d" > colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s" [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue() 2020-04-26 11:25 ` Lukas Straub @ 2020-04-27 3:11 ` Zhang, Chen 0 siblings, 0 replies; 5+ messages in thread From: Zhang, Chen @ 2020-04-27 3:11 UTC (permalink / raw) To: Lukas Straub, Derek Su Cc: lizhijian@cn.fujitsu.com, chyang@qnap.com, jasowang@redhat.com, qemu-devel@nongnu.org, ctcheng@qnap.com, jwsu1986@gmail.com > -----Original Message----- > From: Lukas Straub <lukasstraub2@web.de> > Sent: Sunday, April 26, 2020 7:25 PM > To: Derek Su <dereksu@qnap.com> > Cc: qemu-devel@nongnu.org; lizhijian@cn.fujitsu.com; chyang@qnap.com; > jasowang@redhat.com; ctcheng@qnap.com; Zhang, Chen > <chen.zhang@intel.com>; jwsu1986@gmail.com > Subject: Re: [PATCH v5 1/1] colo-compare: Fix memory leak in > packet_enqueue() > > On Fri, 10 Apr 2020 10:00:56 +0800 > Derek Su <dereksu@qnap.com> wrote: > > > The patch is to fix the "pkt" memory leak in packet_enqueue(). > > The allocated "pkt" needs to be freed if the colo compare primary or > > secondary queue is too big. > > > > Replace the error_report of full queue with a trace event. > > > > Signed-off-by: Derek Su <dereksu@qnap.com> > > Looks good now and works well in my tests. > Reviewed-by: Lukas Straub <lukasstraub2@web.de> > Tested-by: Lukas Straub <lukasstraub2@web.de> > I will queue this patch to COLO branch, then submit to Jason. Thanks Zhang Chen > Regards, > Lukas Straub > > > --- > > net/colo-compare.c | 23 +++++++++++++++-------- > > net/trace-events | 1 + > > 2 files changed, 16 insertions(+), 8 deletions(-) > > > > diff --git a/net/colo-compare.c b/net/colo-compare.c index > > 10c0239f9d..035e11d4d3 100644 > > --- a/net/colo-compare.c > > +++ b/net/colo-compare.c > > @@ -122,6 +122,10 @@ enum { > > SECONDARY_IN, > > }; > > > > +static const char *colo_mode[] = { > > + [PRIMARY_IN] = "primary", > > + [SECONDARY_IN] = "secondary", > > +}; > > > > static int compare_chr_send(CompareState *s, > > const uint8_t *buf, @@ -217,6 +221,7 @@ > > static int packet_enqueue(CompareState *s, int mode, Connection **con) > > ConnectionKey key; > > Packet *pkt = NULL; > > Connection *conn; > > + int ret; > > > > if (mode == PRIMARY_IN) { > > pkt = packet_new(s->pri_rs.buf, @@ -245,16 +250,18 @@ static > > int packet_enqueue(CompareState *s, int mode, Connection **con) > > } > > > > if (mode == PRIMARY_IN) { > > - if (!colo_insert_packet(&conn->primary_list, pkt, &conn->pack)) { > > - error_report("colo compare primary queue size too big," > > - "drop packet"); > > - } > > + ret = colo_insert_packet(&conn->primary_list, pkt, > > + &conn->pack); > > } else { > > - if (!colo_insert_packet(&conn->secondary_list, pkt, &conn->sack)) { > > - error_report("colo compare secondary queue size too big," > > - "drop packet"); > > - } > > + ret = colo_insert_packet(&conn->secondary_list, pkt, > > + &conn->sack); > > } > > + > > + if (!ret) { > > + trace_colo_compare_drop_packet(colo_mode[mode], > > + "queue size too big, drop packet"); > > + packet_destroy(pkt, NULL); > > + pkt = NULL; > > + } > > + > > *con = conn; > > > > return 0; > > diff --git a/net/trace-events b/net/trace-events index > > 02c13fd0ba..fa49c71533 100644 > > --- a/net/trace-events > > +++ b/net/trace-events > > @@ -12,6 +12,7 @@ colo_proxy_main(const char *chr) ": %s" > > > > # colo-compare.c > > colo_compare_main(const char *chr) ": %s" > > +colo_compare_drop_packet(const char *queue, const char *chr) > ": %s: %s" > > colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d" > > colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d" > > colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, > const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, > spkt size = %d, ip_src = %s, ip_dst = %s" ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-27 3:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-10 2:00 [PATCH v5 0/1] COLO: handling of the full primary or secondary queue Derek Su 2020-04-10 2:00 ` [PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue() Derek Su 2020-04-11 3:51 ` Zhang, Chen 2020-04-26 11:25 ` Lukas Straub 2020-04-27 3:11 ` Zhang, Chen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).