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

[Zhang, Chen]

> -----Original Message-----
> From: Lukas Straub <address@hidden>
> Sent: Thursday, April 9, 2020 3:19 AM
> To: Derek Su <address@hidden>
> Cc: address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; Zhang, Chen
> <address@hidden>; address@hidden
> 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 <address@hidden> 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 <address@hidden>
> 
> 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