qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] colo-compare: minor fixes
@ 2020-09-22  9:55 Li Zhijian
  2020-09-22  9:55 ` [PATCH 1/3] colo-compare: return -1 if no packet is queued Li Zhijian
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Li Zhijian @ 2020-09-22  9:55 UTC (permalink / raw)
  To: chen.zhang, jasowang; +Cc: qemu-devel, Li Zhijian


Li Zhijian (3):
  colo-compare: return -1 if no packet is enqueued
  colo-compare: fix missing compare_seq init
  colo-compare: check mark in mutual exclusion

 net/colo-compare.c | 7 +++----
 net/colo.c         | 5 +----
 2 files changed, 4 insertions(+), 8 deletions(-)

-- 
2.28.0





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] colo-compare: return -1 if no packet is queued
  2020-09-22  9:55 [PATCH 0/3] colo-compare: minor fixes Li Zhijian
@ 2020-09-22  9:55 ` Li Zhijian
  2020-09-23  1:41   ` Zhang, Chen
  2020-09-22  9:55 ` [PATCH 2/3] colo-compare: fix missing compare_seq initialization Li Zhijian
  2020-09-22  9:55 ` [PATCH 3/3] colo-compare: check mark in mutual exclusion Li Zhijian
  2 siblings, 1 reply; 9+ messages in thread
From: Li Zhijian @ 2020-09-22  9:55 UTC (permalink / raw)
  To: chen.zhang, jasowang; +Cc: qemu-devel, Li Zhijian

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(+)

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





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] colo-compare: fix missing compare_seq initialization
  2020-09-22  9:55 [PATCH 0/3] colo-compare: minor fixes Li Zhijian
  2020-09-22  9:55 ` [PATCH 1/3] colo-compare: return -1 if no packet is queued Li Zhijian
@ 2020-09-22  9:55 ` Li Zhijian
  2020-09-23  1:42   ` Zhang, Chen
  2020-09-22  9:55 ` [PATCH 3/3] colo-compare: check mark in mutual exclusion Li Zhijian
  2 siblings, 1 reply; 9+ messages in thread
From: Li Zhijian @ 2020-09-22  9:55 UTC (permalink / raw)
  To: chen.zhang, jasowang; +Cc: qemu-devel, Li Zhijian

Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
---
 net/colo.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/colo.c b/net/colo.c
index a6c66d829a..ef00609848 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -133,14 +133,11 @@ void reverse_connection_key(ConnectionKey *key)
 
 Connection *connection_new(ConnectionKey *key)
 {
-    Connection *conn = g_slice_new(Connection);
+    Connection *conn = g_slice_new0(Connection);
 
     conn->ip_proto = key->ip_proto;
     conn->processing = false;
-    conn->offset = 0;
     conn->tcp_state = TCPS_CLOSED;
-    conn->pack = 0;
-    conn->sack = 0;
     g_queue_init(&conn->primary_list);
     g_queue_init(&conn->secondary_list);
 
-- 
2.28.0





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] colo-compare: check mark in mutual exclusion
  2020-09-22  9:55 [PATCH 0/3] colo-compare: minor fixes Li Zhijian
  2020-09-22  9:55 ` [PATCH 1/3] colo-compare: return -1 if no packet is queued Li Zhijian
  2020-09-22  9:55 ` [PATCH 2/3] colo-compare: fix missing compare_seq initialization Li Zhijian
@ 2020-09-22  9:55 ` Li Zhijian
  2020-09-23  1:46   ` Zhang, Chen
  2 siblings, 1 reply; 9+ messages in thread
From: Li Zhijian @ 2020-09-22  9:55 UTC (permalink / raw)
  To: chen.zhang, jasowang; +Cc: qemu-devel, Li Zhijian

Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
---
 net/colo-compare.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 039b515611..19633fc684 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -481,13 +481,11 @@ sec:
             colo_release_primary_pkt(s, ppkt);
             g_queue_push_head(&conn->secondary_list, spkt);
             goto pri;
-        }
-        if (mark == COLO_COMPARE_FREE_SECONDARY) {
+        } else if (mark == COLO_COMPARE_FREE_SECONDARY) {
             conn->compare_seq = spkt->seq_end;
             packet_destroy(spkt, NULL);
             goto sec;
-        }
-        if (mark == (COLO_COMPARE_FREE_PRIMARY | COLO_COMPARE_FREE_SECONDARY)) {
+        } else if (mark == (COLO_COMPARE_FREE_PRIMARY | COLO_COMPARE_FREE_SECONDARY)) {
             conn->compare_seq = ppkt->seq_end;
             colo_release_primary_pkt(s, ppkt);
             packet_destroy(spkt, NULL);
-- 
2.28.0





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/3] colo-compare: return -1 if no packet is queued
  2020-09-22  9:55 ` [PATCH 1/3] colo-compare: return -1 if no packet is queued Li Zhijian
@ 2020-09-23  1:41   ` Zhang, Chen
  2020-09-23  6:17     ` Li Zhijian
  0 siblings, 1 reply; 9+ messages in thread
From: Zhang, Chen @ 2020-09-23  1:41 UTC (permalink / raw)
  To: Li Zhijian, jasowang@redhat.com; +Cc: qemu-devel@nongnu.org



> -----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
> 
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 2/3] colo-compare: fix missing compare_seq initialization
  2020-09-22  9:55 ` [PATCH 2/3] colo-compare: fix missing compare_seq initialization Li Zhijian
@ 2020-09-23  1:42   ` Zhang, Chen
  0 siblings, 0 replies; 9+ messages in thread
From: Zhang, Chen @ 2020-09-23  1:42 UTC (permalink / raw)
  To: Li Zhijian, jasowang@redhat.com; +Cc: qemu-devel@nongnu.org



> -----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 2/3] colo-compare: fix missing compare_seq initialization
> 
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>

Reviewed-by: Zhang Chen <chen.zhang@intel.com>

Thanks
Zhang Chen

> ---
>  net/colo.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/net/colo.c b/net/colo.c
> index a6c66d829a..ef00609848 100644
> --- a/net/colo.c
> +++ b/net/colo.c
> @@ -133,14 +133,11 @@ void reverse_connection_key(ConnectionKey *key)
> 
>  Connection *connection_new(ConnectionKey *key)  {
> -    Connection *conn = g_slice_new(Connection);
> +    Connection *conn = g_slice_new0(Connection);
> 
>      conn->ip_proto = key->ip_proto;
>      conn->processing = false;
> -    conn->offset = 0;
>      conn->tcp_state = TCPS_CLOSED;
> -    conn->pack = 0;
> -    conn->sack = 0;
>      g_queue_init(&conn->primary_list);
>      g_queue_init(&conn->secondary_list);
> 
> --
> 2.28.0
> 
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 3/3] colo-compare: check mark in mutual exclusion
  2020-09-22  9:55 ` [PATCH 3/3] colo-compare: check mark in mutual exclusion Li Zhijian
@ 2020-09-23  1:46   ` Zhang, Chen
  0 siblings, 0 replies; 9+ messages in thread
From: Zhang, Chen @ 2020-09-23  1:46 UTC (permalink / raw)
  To: Li Zhijian, jasowang@redhat.com; +Cc: qemu-devel@nongnu.org



> -----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 3/3] colo-compare: check mark in mutual exclusion
> 
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>

Reviewed-by: Zhang Chen <chen.zhang@intel.com>
By the way, we have other optimization here, will be sent in the future.

Thanks
Zhang Chen

> ---
>  net/colo-compare.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/colo-compare.c b/net/colo-compare.c index
> 039b515611..19633fc684 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -481,13 +481,11 @@ sec:
>              colo_release_primary_pkt(s, ppkt);
>              g_queue_push_head(&conn->secondary_list, spkt);
>              goto pri;
> -        }
> -        if (mark == COLO_COMPARE_FREE_SECONDARY) {
> +        } else if (mark == COLO_COMPARE_FREE_SECONDARY) {
>              conn->compare_seq = spkt->seq_end;
>              packet_destroy(spkt, NULL);
>              goto sec;
> -        }
> -        if (mark == (COLO_COMPARE_FREE_PRIMARY |
> COLO_COMPARE_FREE_SECONDARY)) {
> +        } else if (mark == (COLO_COMPARE_FREE_PRIMARY |
> + COLO_COMPARE_FREE_SECONDARY)) {
>              conn->compare_seq = ppkt->seq_end;
>              colo_release_primary_pkt(s, ppkt);
>              packet_destroy(spkt, NULL);
> --
> 2.28.0
> 
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] colo-compare: return -1 if no packet is queued
  2020-09-23  1:41   ` Zhang, Chen
@ 2020-09-23  6:17     ` Li Zhijian
  2020-09-23  6:51       ` Zhang, Chen
  0 siblings, 1 reply; 9+ messages in thread
From: Li Zhijian @ 2020-09-23  6:17 UTC (permalink / raw)
  To: Zhang, Chen, jasowang@redhat.com; +Cc: qemu-devel@nongnu.org



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
>>
>>
>
>





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/3] colo-compare: return -1 if no packet is queued
  2020-09-23  6:17     ` Li Zhijian
@ 2020-09-23  6:51       ` Zhang, Chen
  0 siblings, 0 replies; 9+ messages in thread
From: Zhang, Chen @ 2020-09-23  6:51 UTC (permalink / raw)
  To: Li Zhijian, jasowang@redhat.com; +Cc: qemu-devel@nongnu.org



> -----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
> >>
> >>
> >
> >
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-09-23  6:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-22  9:55 [PATCH 0/3] colo-compare: minor fixes Li Zhijian
2020-09-22  9:55 ` [PATCH 1/3] colo-compare: return -1 if no packet is queued Li Zhijian
2020-09-23  1:41   ` Zhang, Chen
2020-09-23  6:17     ` Li Zhijian
2020-09-23  6:51       ` Zhang, Chen
2020-09-22  9:55 ` [PATCH 2/3] colo-compare: fix missing compare_seq initialization Li Zhijian
2020-09-23  1:42   ` Zhang, Chen
2020-09-22  9:55 ` [PATCH 3/3] colo-compare: check mark in mutual exclusion Li Zhijian
2020-09-23  1:46   ` 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).