RE: [PATCH v4 2/2] net/colo-compare.c: handling of the full primary or secondary queue

["Zhang, Chen" <chen.zhang@intel.com>]

> -----Original Message-----
> From: Derek Su <jwsu1986@gmail.com>
> Sent: Thursday, April 9, 2020 3:11 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Lukas Straub <lukasstraub2@web.de>; Derek Su <dereksu@qnap.com>;
> qemu-devel@nongnu.org; lizhijian@cn.fujitsu.com; chyang@qnap.com;
> jasowang@redhat.com; ctcheng@qnap.com
> Subject: Re: [PATCH v4 2/2] net/colo-compare.c: handling of the full primary
> or secondary queue
> 
> Hello, Zhang and Lukas
> 
> Sure, after my re-test, the performance is hurt. Will update it later.
> 
> By the way, could I also move the "error_report("colo compare
> primary/secondary queue size too big, drop packet");" to trace?

Yes.

> The use of error_report is a little strange and make a flood in log.
> 
> May I  also make "MAX_QUEUE_SIZE"  be user-configurable in this series?

I have this patch internal and will send it later.

Thanks
Zhang Chen

> 
> Thanks,
> Derek Su
> 
> 
> 
> 
> Zhang, Chen <chen.zhang@intel.com> 於 2020年4月9日 週四 下午2:59
> 寫道：
> >
> >
> >
> > > -----Original Message-----
> > > From: Lukas Straub <lukasstraub2@web.de>
> > > Sent: Thursday, April 9, 2020 3:19 AM
> > > 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 v4 2/2] net/colo-compare.c: handling of the full
> > > primary or secondary queue
> > >
> > > On Sat, 28 Mar 2020 20:46:46 +0800
> > > Derek Su <dereksu@qnap.com> wrote:
> > >
> > > > The pervious handling of the full primary or queue is only
> > > > dropping the packet. If there are lots of clients to the guest VM, the
> "drop"
> > > > will lead to the lost of the networking connection until next
> > > > checkpoint.
> > > >
> > > > To address the issue, this patch drops the packet firstly.
> > > > Then, do checkpoint and flush packets.
> > > >
> > > > Signed-off-by: Derek Su <dereksu@qnap.com>
> > >
> > > Hello,
> > > I had a look at this again and did some benchmarking.
> > > First just qemu 5.0-rc1 with my bugfixes (
> > > https://lists.nongnu.org/archive/html/qemu-devel/2020-
> > > 04/msg01432.html ) Then qemu 5.0-rc1 with my bugfixes and this patch
> > > series.
> > >
> > > This commit hurts performance too much:
> > > Client-to-server bandwidth falls from ~45.9 Mbit/s to 22.9 Mbit/s.
> > > Server-to-client bandwidth falls from ~6.3 Mbit/s to just ~674 Kbit/s.
> > > Average latency rises from ~197ms to ~397ms.
> > >
> > > Meanwhile the packet loss without this commit is negligible, only
> > > 1-2 ping packets got lost during each test run.
> > >
> > > Instead I think we should just turn the error message into a trace
> > > so it doesn't flood the logs.
> >
> > We re-test this patch, Lukas is right.
> > Sorry for the original idea, looks like it did not show better performance in
> the test.
> > Agree with Lukas's comments. Derek, can you please change it?
> >
> > Thanks
> > Zhang Chen
> >
> >
> > >
> > > Regards,
> > > Lukas Straub