[PATCH 1/2] net/colo-compare.c: Optimize compare order for performance

[PATCH 1/2] net/colo-compare.c: Optimize compare order for performance

[Zhang Chen]

COLO-compare use the glib function g_queue_find_custom to dump
another VM's networking packet to compare. But this function always
start find from the queue->head(here is the newest packet), It will
reduce the success rate of comparison. So this patch reversed
the order of the queues for performance.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reported-by: leirao <lei.rao@intel.com>
---
 net/colo-compare.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index b966e7e514..216de5a12b 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -197,7 +197,7 @@ static void colo_compare_inconsistency_notify(CompareState *s)
 /* Use restricted to colo_insert_packet() */
 static gint seq_sorter(Packet *a, Packet *b, gpointer data)
 {
-    return a->tcp_seq - b->tcp_seq;
+    return b->tcp_seq - a->tcp_seq;
 }
 
 static void fill_pkt_tcp_info(void *data, uint32_t *max_ack)
@@ -421,13 +421,13 @@ pri:
     if (g_queue_is_empty(&conn->primary_list)) {
         return;
     }
-    ppkt = g_queue_pop_head(&conn->primary_list);
+    ppkt = g_queue_pop_tail(&conn->primary_list);
 sec:
     if (g_queue_is_empty(&conn->secondary_list)) {
-        g_queue_push_head(&conn->primary_list, ppkt);
+        g_queue_push_tail(&conn->primary_list, ppkt);
         return;
     }
-    spkt = g_queue_pop_head(&conn->secondary_list);
+    spkt = g_queue_pop_tail(&conn->secondary_list);
 
     if (ppkt->tcp_seq == ppkt->seq_end) {
         colo_release_primary_pkt(s, ppkt);
@@ -458,7 +458,7 @@ sec:
             }
         }
         if (!ppkt) {
-            g_queue_push_head(&conn->secondary_list, spkt);
+            g_queue_push_tail(&conn->secondary_list, spkt);
             goto pri;
         }
     }
@@ -477,7 +477,7 @@ sec:
         if (mark == COLO_COMPARE_FREE_PRIMARY) {
             conn->compare_seq = ppkt->seq_end;
             colo_release_primary_pkt(s, ppkt);
-            g_queue_push_head(&conn->secondary_list, spkt);
+            g_queue_push_tail(&conn->secondary_list, spkt);
             goto pri;
         } else if (mark == COLO_COMPARE_FREE_SECONDARY) {
             conn->compare_seq = spkt->seq_end;
@@ -490,8 +490,8 @@ sec:
             goto pri;
         }
     } else {
-        g_queue_push_head(&conn->primary_list, ppkt);
-        g_queue_push_head(&conn->secondary_list, spkt);
+        g_queue_push_tail(&conn->primary_list, ppkt);
+        g_queue_push_tail(&conn->secondary_list, spkt);
 
 #ifdef DEBUG_COLO_PACKETS
         qemu_hexdump(stderr, "colo-compare ppkt", ppkt->data, ppkt->size);
@@ -673,7 +673,7 @@ static void colo_compare_packet(CompareState *s, Connection *conn,
 
     while (!g_queue_is_empty(&conn->primary_list) &&
            !g_queue_is_empty(&conn->secondary_list)) {
-        pkt = g_queue_pop_head(&conn->primary_list);
+        pkt = g_queue_pop_tail(&conn->primary_list);
         result = g_queue_find_custom(&conn->secondary_list,
                  pkt, (GCompareFunc)HandlePacket);
 
@@ -689,7 +689,7 @@ static void colo_compare_packet(CompareState *s, Connection *conn,
              * timeout, it will trigger a checkpoint request.
              */
             trace_colo_compare_main("packet different");
-            g_queue_push_head(&conn->primary_list, pkt);
+            g_queue_push_tail(&conn->primary_list, pkt);
 
             colo_compare_inconsistency_notify(s);
             break;
@@ -819,7 +819,7 @@ static int compare_chr_send(CompareState *s,
         entry->buf = g_malloc(size);
         memcpy(entry->buf, buf, size);
     }
-    g_queue_push_head(&sendco->send_list, entry);
+    g_queue_push_tail(&sendco->send_list, entry);
 
     if (sendco->done) {
         sendco->co = qemu_coroutine_create(_compare_chr_send, sendco);
@@ -1347,7 +1347,7 @@ static void colo_flush_packets(void *opaque, void *user_data)
     Packet *pkt = NULL;
 
     while (!g_queue_is_empty(&conn->primary_list)) {
-        pkt = g_queue_pop_head(&conn->primary_list);
+        pkt = g_queue_pop_tail(&conn->primary_list);
         compare_chr_send(s,
                          pkt->data,
                          pkt->size,
@@ -1357,7 +1357,7 @@ static void colo_flush_packets(void *opaque, void *user_data)
         packet_destroy_partial(pkt, NULL);
     }
     while (!g_queue_is_empty(&conn->secondary_list)) {
-        pkt = g_queue_pop_head(&conn->secondary_list);
+        pkt = g_queue_pop_tail(&conn->secondary_list);
         packet_destroy(pkt, NULL);
     }
 }
-- 
2.25.1

[PATCH 2/2] net/colo-compare.c: Update the default value comments

[Zhang Chen]

Make the comments consistent with the REGULAR_PACKET_CHECK_MS.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 net/colo-compare.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 216de5a12b..62554b5b3c 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1267,7 +1267,7 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
     }
 
     if (!s->expired_scan_cycle) {
-        /* Set default value to 3000 MS */
+        /* Set default value to 1000 MS */
         s->expired_scan_cycle = REGULAR_PACKET_CHECK_MS;
     }
 
-- 
2.25.1

Re: [PATCH 1/2] net/colo-compare.c: Optimize compare order for performance

[Jason Wang]

On Mon, Dec 20, 2021 at 9:16 AM Zhang Chen <chen.zhang@intel.com> wrote:
>
> COLO-compare use the glib function g_queue_find_custom to dump
> another VM's networking packet to compare. But this function always
> start find from the queue->head(here is the newest packet), It will
> reduce the success rate of comparison. So this patch reversed
> the order of the queues for performance.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> Reported-by: leirao <lei.rao@intel.com>
> ---
>  net/colo-compare.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)

Applied.

Thanks

>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index b966e7e514..216de5a12b 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -197,7 +197,7 @@ static void colo_compare_inconsistency_notify(CompareState *s)
>  /* Use restricted to colo_insert_packet() */
>  static gint seq_sorter(Packet *a, Packet *b, gpointer data)
>  {
> -    return a->tcp_seq - b->tcp_seq;
> +    return b->tcp_seq - a->tcp_seq;
>  }
>
>  static void fill_pkt_tcp_info(void *data, uint32_t *max_ack)
> @@ -421,13 +421,13 @@ pri:
>      if (g_queue_is_empty(&conn->primary_list)) {
>          return;
>      }
> -    ppkt = g_queue_pop_head(&conn->primary_list);
> +    ppkt = g_queue_pop_tail(&conn->primary_list);
>  sec:
>      if (g_queue_is_empty(&conn->secondary_list)) {
> -        g_queue_push_head(&conn->primary_list, ppkt);
> +        g_queue_push_tail(&conn->primary_list, ppkt);
>          return;
>      }
> -    spkt = g_queue_pop_head(&conn->secondary_list);
> +    spkt = g_queue_pop_tail(&conn->secondary_list);
>
>      if (ppkt->tcp_seq == ppkt->seq_end) {
>          colo_release_primary_pkt(s, ppkt);
> @@ -458,7 +458,7 @@ sec:
>              }
>          }
>          if (!ppkt) {
> -            g_queue_push_head(&conn->secondary_list, spkt);
> +            g_queue_push_tail(&conn->secondary_list, spkt);
>              goto pri;
>          }
>      }
> @@ -477,7 +477,7 @@ sec:
>          if (mark == COLO_COMPARE_FREE_PRIMARY) {
>              conn->compare_seq = ppkt->seq_end;
>              colo_release_primary_pkt(s, ppkt);
> -            g_queue_push_head(&conn->secondary_list, spkt);
> +            g_queue_push_tail(&conn->secondary_list, spkt);
>              goto pri;
>          } else if (mark == COLO_COMPARE_FREE_SECONDARY) {
>              conn->compare_seq = spkt->seq_end;
> @@ -490,8 +490,8 @@ sec:
>              goto pri;
>          }
>      } else {
> -        g_queue_push_head(&conn->primary_list, ppkt);
> -        g_queue_push_head(&conn->secondary_list, spkt);
> +        g_queue_push_tail(&conn->primary_list, ppkt);
> +        g_queue_push_tail(&conn->secondary_list, spkt);
>
>  #ifdef DEBUG_COLO_PACKETS
>          qemu_hexdump(stderr, "colo-compare ppkt", ppkt->data, ppkt->size);
> @@ -673,7 +673,7 @@ static void colo_compare_packet(CompareState *s, Connection *conn,
>
>      while (!g_queue_is_empty(&conn->primary_list) &&
>             !g_queue_is_empty(&conn->secondary_list)) {
> -        pkt = g_queue_pop_head(&conn->primary_list);
> +        pkt = g_queue_pop_tail(&conn->primary_list);
>          result = g_queue_find_custom(&conn->secondary_list,
>                   pkt, (GCompareFunc)HandlePacket);
>
> @@ -689,7 +689,7 @@ static void colo_compare_packet(CompareState *s, Connection *conn,
>               * timeout, it will trigger a checkpoint request.
>               */
>              trace_colo_compare_main("packet different");
> -            g_queue_push_head(&conn->primary_list, pkt);
> +            g_queue_push_tail(&conn->primary_list, pkt);
>
>              colo_compare_inconsistency_notify(s);
>              break;
> @@ -819,7 +819,7 @@ static int compare_chr_send(CompareState *s,
>          entry->buf = g_malloc(size);
>          memcpy(entry->buf, buf, size);
>      }
> -    g_queue_push_head(&sendco->send_list, entry);
> +    g_queue_push_tail(&sendco->send_list, entry);
>
>      if (sendco->done) {
>          sendco->co = qemu_coroutine_create(_compare_chr_send, sendco);
> @@ -1347,7 +1347,7 @@ static void colo_flush_packets(void *opaque, void *user_data)
>      Packet *pkt = NULL;
>
>      while (!g_queue_is_empty(&conn->primary_list)) {
> -        pkt = g_queue_pop_head(&conn->primary_list);
> +        pkt = g_queue_pop_tail(&conn->primary_list);
>          compare_chr_send(s,
>                           pkt->data,
>                           pkt->size,
> @@ -1357,7 +1357,7 @@ static void colo_flush_packets(void *opaque, void *user_data)
>          packet_destroy_partial(pkt, NULL);
>      }
>      while (!g_queue_is_empty(&conn->secondary_list)) {
> -        pkt = g_queue_pop_head(&conn->secondary_list);
> +        pkt = g_queue_pop_tail(&conn->secondary_list);
>          packet_destroy(pkt, NULL);
>      }
>  }
> --
> 2.25.1
>

Re: [PATCH 2/2] net/colo-compare.c: Update the default value comments

[Jason Wang]

On Mon, Dec 20, 2021 at 9:17 AM Zhang Chen <chen.zhang@intel.com> wrote:
>
> Make the comments consistent with the REGULAR_PACKET_CHECK_MS.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>

Applied.

Thanks

> ---
>  net/colo-compare.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 216de5a12b..62554b5b3c 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -1267,7 +1267,7 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
>      }
>
>      if (!s->expired_scan_cycle) {
> -        /* Set default value to 3000 MS */
> +        /* Set default value to 1000 MS */
>          s->expired_scan_cycle = REGULAR_PACKET_CHECK_MS;
>      }
>
> --
> 2.25.1
>