> -----Original Message-----
> From: Lukas Straub <lukasstra...@web.de>
> Sent: Sunday, April 26, 2020 7:25 PM
> To: Derek Su <dere...@qnap.com>
> Cc: qemu-devel@nongnu.org; lizhij...@cn.fujitsu.com; chy...@qnap.com;
> jasow...@redhat.com; ctch...@qnap.com; Zhang, Chen
> <chen.zh...@intel.com>; jwsu1...@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 <dere...@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 <dere...@qnap.com>
>
> Looks good now and works well in my tests.
> Reviewed-by: Lukas Straub <lukasstra...@web.de>
> Tested-by: Lukas Straub <lukasstra...@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"
