| Message ID | 20200922095525.4081603-2-lizhijian@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | colo-compare: minor fixes | expand |
> -----Original Message----- > From: Li Zhijian <lizhijian@cn.fujitsu.com> > Sent: Tuesday, September 22, 2020 5:55 PM > To: Zhang, Chen <chen.zhang@intel.com>; jasowang@redhat.com > Cc: qemu-devel@nongnu.org; Li Zhijian <lizhijian@cn.fujitsu.com> > Subject: [PATCH 1/3] colo-compare: return -1 if no packet is queued > > Return 0 will trigger a packet comparison > Yes, we need active trigger a checkpoint to flush all the queued packets here. Otherwise, we should drop all the packet after this time still next checkpoint. So, I think original logic is a better choice. Thanks Zhang Chen > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > --- > net/colo-compare.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/colo-compare.c b/net/colo-compare.c index > 3a45d64175..039b515611 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -289,6 +289,7 @@ static int packet_enqueue(CompareState *s, int > mode, Connection **con) > "queue size too big, drop packet"); > packet_destroy(pkt, NULL); > pkt = NULL; > + return -1; > } > > *con = conn; > -- > 2.28.0 > >
On 9/23/20 9:41 AM, Zhang, Chen wrote: > >> -----Original Message----- >> From: Li Zhijian <lizhijian@cn.fujitsu.com> >> Sent: Tuesday, September 22, 2020 5:55 PM >> To: Zhang, Chen <chen.zhang@intel.com>; jasowang@redhat.com >> Cc: qemu-devel@nongnu.org; Li Zhijian <lizhijian@cn.fujitsu.com> >> Subject: [PATCH 1/3] colo-compare: return -1 if no packet is queued >> >> Return 0 will trigger a packet comparison >> > Yes, we need active trigger a checkpoint to flush all the queued packets here. Previously, no new checkpoint will be triggered since no new packet is queued though colo_compare_connection() is called. actually we should send a notify to colo frame immediately, no need to compare them any more in order to less latency. diff --git a/net/colo-compare.c b/net/colo-compare.c index 3a45d64175..23092e4496 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -285,10 +285,13 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) } if (!ret) { + /* queue is too long, do a checkpoint to release all queued packets */ + colo_compare_inconsistency_notify(s); trace_colo_compare_drop_packet(colo_mode[mode], "queue size too big, drop packet"); packet_destroy(pkt, NULL); pkt = NULL; + return -1; } *con = conn; > Otherwise, we should drop all the packet after this time still next checkpoint. > So, I think original logic is a better choice. > > Thanks > Zhang Chen > >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> --- >> net/colo-compare.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/net/colo-compare.c b/net/colo-compare.c index >> 3a45d64175..039b515611 100644 >> --- a/net/colo-compare.c >> +++ b/net/colo-compare.c >> @@ -289,6 +289,7 @@ static int packet_enqueue(CompareState *s, int >> mode, Connection **con) >> "queue size too big, drop packet"); >> packet_destroy(pkt, NULL); >> pkt = NULL; >> + return -1; >> } >> >> *con = conn; >> -- >> 2.28.0 >> >> > >
> -----Original Message----- > From: Li Zhijian <lizhijian@cn.fujitsu.com> > Sent: Wednesday, September 23, 2020 2:18 PM > To: Zhang, Chen <chen.zhang@intel.com>; jasowang@redhat.com > Cc: qemu-devel@nongnu.org > Subject: Re: [PATCH 1/3] colo-compare: return -1 if no packet is queued > > > > On 9/23/20 9:41 AM, Zhang, Chen wrote: > > > >> -----Original Message----- > >> From: Li Zhijian <lizhijian@cn.fujitsu.com> > >> Sent: Tuesday, September 22, 2020 5:55 PM > >> To: Zhang, Chen <chen.zhang@intel.com>; jasowang@redhat.com > >> Cc: qemu-devel@nongnu.org; Li Zhijian <lizhijian@cn.fujitsu.com> > >> Subject: [PATCH 1/3] colo-compare: return -1 if no packet is queued > >> > >> Return 0 will trigger a packet comparison > >> > > Yes, we need active trigger a checkpoint to flush all the queued packets > here. > Previously, no new checkpoint will be triggered since no new packet is > queued though colo_compare_connection() is called. > actually we should send a notify to colo frame immediately, no need to > compare them any more in order to less latency. Yes, you are right. We can change this patch to directly send notify here. Thanks Zhang Chen > > diff --git a/net/colo-compare.c b/net/colo-compare.c index > 3a45d64175..23092e4496 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -285,10 +285,13 @@ static int packet_enqueue(CompareState *s, int > mode, Connection **con) > } > > if (!ret) { > + /* queue is too long, do a checkpoint to release all queued > +packets */ > + colo_compare_inconsistency_notify(s); > trace_colo_compare_drop_packet(colo_mode[mode], > "queue size too big, drop packet"); > packet_destroy(pkt, NULL); > pkt = NULL; > + return -1; > } > > *con = conn; > > > > Otherwise, we should drop all the packet after this time still next > checkpoint. > > So, I think original logic is a better choice. > > > > Thanks > > Zhang Chen > > > >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > >> --- > >> net/colo-compare.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/net/colo-compare.c b/net/colo-compare.c index > >> 3a45d64175..039b515611 100644 > >> --- a/net/colo-compare.c > >> +++ b/net/colo-compare.c > >> @@ -289,6 +289,7 @@ static int packet_enqueue(CompareState *s, int > >> mode, Connection **con) > >> "queue size too big, drop packet"); > >> packet_destroy(pkt, NULL); > >> pkt = NULL; > >> + return -1; > >> } > >> > >> *con = conn; > >> -- > >> 2.28.0 > >> > >> > > > > > >
diff --git a/net/colo-compare.c b/net/colo-compare.c index 3a45d64175..039b515611 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -289,6 +289,7 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) "queue size too big, drop packet"); packet_destroy(pkt, NULL); pkt = NULL; + return -1; } *con = conn;
Return 0 will trigger a packet comparison Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> --- net/colo-compare.c | 1 + 1 file changed, 1 insertion(+)