[Qemu-devel] [PATCH 0/3] COLO-compare: Optimize the code and fix some bug

[Qemu-devel] [PATCH 0/3] COLO-compare: Optimize the code and fix some bug

[Zhang Chen]

This series we will Optimize the code and fix some bug.
Patch1: Add packet minimum size check in compare tcp/udp like compare icmp.
Patch2: Optimize compare_common and increase compare performance.
Patch3: Fix debug info always print bug.

V1:
  - init patch.

Zhang Chen (3):
  COLO-compare: Add minimum packet size check and some fix
  COLO-compare: Optimize colo_packet_compare_common
  COLO-compare: Make qemu_hexdump() print data when trace-event ON

 net/colo-compare.c | 68 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 51 insertions(+), 17 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and some fix

[Zhang Chen]

Add packet minimum size check in colo_packet_compare_udp()
and colo_packet_compare_udp() like colo_packet_compare_icmp(),
rename function colo_packet_compare() to colo_packet_compare_common()
that we will reuse it later.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo-compare.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 300f017..e75f0ae 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
  * return:    0  means packet same
  *            > 0 || < 0 means packet different
  */
-static int colo_packet_compare(Packet *ppkt, Packet *spkt)
+static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
 {
     trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
                                inet_ntoa(ppkt->ip->ip_dst), spkt->size,
@@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
     if (ppkt->size == spkt->size) {
         return memcmp(ppkt->data, spkt->data, spkt->size);
     } else {
+        trace_colo_compare_main("Net packet size are not the same");
         return -1;
     }
 }
@@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
 static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
 {
     struct tcphdr *ptcp, *stcp;
-    int res;
+    int res, network_length;
 
     trace_colo_compare_main("compare tcp");
+
     if (ppkt->size != spkt->size) {
         if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
             trace_colo_compare_main("pkt size not same");
@@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
         return -1;
     }
 
+    network_length = ppkt->ip->ip_hl * 4;
+    if (ppkt->size < network_length + ETH_HLEN) {
+        trace_colo_compare_main("tcp packet size error");
+        return -1;
+    }
+
     ptcp = (struct tcphdr *)ppkt->transport_header;
     stcp = (struct tcphdr *)spkt->transport_header;
 
@@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
  */
 static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
 {
-    int ret;
+    int ret, network_length;
 
     trace_colo_compare_main("compare udp");
-    ret = colo_packet_compare(ppkt, spkt);
+    network_length = ppkt->ip->ip_hl * 4;
+    if (ppkt->size < network_length + ETH_HLEN) {
+        trace_colo_compare_main("udp packet size error");
+        return -1;
+    }
+
+    ret = colo_packet_compare_common(ppkt, spkt);
 
     if (ret) {
         trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
@@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
 
     trace_colo_compare_main("compare icmp");
     network_length = ppkt->ip->ip_hl * 4;
-    if (ppkt->size != spkt->size ||
-        ppkt->size < network_length + ETH_HLEN) {
+    if (ppkt->size < network_length + ETH_HLEN) {
+        trace_colo_compare_main("icmp packet size error");
         return -1;
     }
 
-    if (colo_packet_compare(ppkt, spkt)) {
+    if (colo_packet_compare_common(ppkt, spkt)) {
         trace_colo_compare_icmp_miscompare("primary pkt size",
                                            ppkt->size);
         qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
@@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
                                inet_ntoa(ppkt->ip->ip_dst), spkt->size,
                                inet_ntoa(spkt->ip->ip_src),
                                inet_ntoa(spkt->ip->ip_dst));
-    return colo_packet_compare(ppkt, spkt);
+    return colo_packet_compare_common(ppkt, spkt);
 }
 
 static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
-- 
2.7.4

[Qemu-devel] [PATCH 1/3] COLO-compare: Add packet size check and some fix

[Zhang Chen]

Add packet size check in colo_packet_compare_udp()
and colo_packet_compare_udp(), rename function colo_packet_compare()
to colo_packet_compare_common() that we will reuse it later.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo-compare.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 300f017..e75f0ae 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
  * return:    0  means packet same
  *            > 0 || < 0 means packet different
  */
-static int colo_packet_compare(Packet *ppkt, Packet *spkt)
+static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
 {
     trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
                                inet_ntoa(ppkt->ip->ip_dst), spkt->size,
@@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
     if (ppkt->size == spkt->size) {
         return memcmp(ppkt->data, spkt->data, spkt->size);
     } else {
+        trace_colo_compare_main("Net packet size are not the same");
         return -1;
     }
 }
@@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
 static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
 {
     struct tcphdr *ptcp, *stcp;
-    int res;
+    int res, network_length;
 
     trace_colo_compare_main("compare tcp");
+
     if (ppkt->size != spkt->size) {
         if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
             trace_colo_compare_main("pkt size not same");
@@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
         return -1;
     }
 
+    network_length = ppkt->ip->ip_hl * 4;
+    if (ppkt->size < network_length + ETH_HLEN) {
+        trace_colo_compare_main("tcp packet size error");
+        return -1;
+    }
+
     ptcp = (struct tcphdr *)ppkt->transport_header;
     stcp = (struct tcphdr *)spkt->transport_header;
 
@@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
  */
 static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
 {
-    int ret;
+    int ret, network_length;
 
     trace_colo_compare_main("compare udp");
-    ret = colo_packet_compare(ppkt, spkt);
+    network_length = ppkt->ip->ip_hl * 4;
+    if (ppkt->size < network_length + ETH_HLEN) {
+        trace_colo_compare_main("udp packet size error");
+        return -1;
+    }
+
+    ret = colo_packet_compare_common(ppkt, spkt);
 
     if (ret) {
         trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
@@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
 
     trace_colo_compare_main("compare icmp");
     network_length = ppkt->ip->ip_hl * 4;
-    if (ppkt->size != spkt->size ||
-        ppkt->size < network_length + ETH_HLEN) {
+    if (ppkt->size < network_length + ETH_HLEN) {
+        trace_colo_compare_main("icmp packet size error");
         return -1;
     }
 
-    if (colo_packet_compare(ppkt, spkt)) {
+    if (colo_packet_compare_common(ppkt, spkt)) {
         trace_colo_compare_icmp_miscompare("primary pkt size",
                                            ppkt->size);
         qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
@@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
                                inet_ntoa(ppkt->ip->ip_dst), spkt->size,
                                inet_ntoa(spkt->ip->ip_src),
                                inet_ntoa(spkt->ip->ip_dst));
-    return colo_packet_compare(ppkt, spkt);
+    return colo_packet_compare_common(ppkt, spkt);
 }
 
 static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
-- 
2.7.4

[Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Zhang Chen]

Add offset args for colo_packet_compare_common, optimize
colo_packet_compare_icmp() and colo_packet_compare_udp()
just compare the IP payload.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo-compare.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index e75f0ae..9853232 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
  * return:    0  means packet same
  *            > 0 || < 0 means packet different
  */
-static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
+static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
 {
     trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
                                inet_ntoa(ppkt->ip->ip_dst), spkt->size,
@@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
                                inet_ntoa(spkt->ip->ip_dst));
 
     if (ppkt->size == spkt->size) {
-        return memcmp(ppkt->data, spkt->data, spkt->size);
+        return memcmp(ppkt->data + offset, spkt->data + offset,
+                      spkt->size - offset);
     } else {
         trace_colo_compare_main("Net packet size are not the same");
         return -1;
@@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
         spkt->ip->ip_sum = ppkt->ip->ip_sum;
     }
 
-    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
-                (spkt->size - ETH_HLEN));
+    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
 
     if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
         trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
@@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
         return -1;
     }
 
-    ret = colo_packet_compare_common(ppkt, spkt);
+    /*
+     * Because of ppkt and spkt are both in the same connection,
+     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
+     * same with spkt. In addition, IP header's Identification is a random
+     * field, we can handle it in IP fragmentation function later.
+     * So we just compare the ip payload here.
+     */
+    ret = colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN);
 
     if (ret) {
         trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
@@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
         return -1;
     }
 
-    if (colo_packet_compare_common(ppkt, spkt)) {
+    /*
+     * Because of ppkt and spkt are both in the same connection,
+     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
+     * same with spkt. In addition, IP header's Identification is a random
+     * field, we can handle it in IP fragmentation function later.
+     * So we just compare the ip payload here.
+     */
+    if (colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN)) {
         trace_colo_compare_icmp_miscompare("primary pkt size",
                                            ppkt->size);
         qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
@@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
                                inet_ntoa(ppkt->ip->ip_dst), spkt->size,
                                inet_ntoa(spkt->ip->ip_src),
                                inet_ntoa(spkt->ip->ip_dst));
-    return colo_packet_compare_common(ppkt, spkt);
+    return colo_packet_compare_common(ppkt, spkt, 0);
 }
 
 static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
-- 
2.7.4

[Qemu-devel] [PATCH 3/3] COLO-compare: Make qemu_hexdump() print data when trace-event ON

[Zhang Chen]

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo-compare.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 9853232..5ffc4a3 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -288,9 +288,13 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
 
     if (ret) {
         trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
-        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
         trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
-        qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
+        if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+            qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
+                         ppkt->size);
+            qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
+                         spkt->size);
+        }
     }
 
     return ret;
@@ -321,12 +325,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
     if (colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN)) {
         trace_colo_compare_icmp_miscompare("primary pkt size",
                                            ppkt->size);
-        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
-                     ppkt->size);
         trace_colo_compare_icmp_miscompare("Secondary pkt size",
                                            spkt->size);
-        qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
-                     spkt->size);
+        if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+            qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
+                         ppkt->size);
+            qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
+                         spkt->size);
+        }
         return -1;
     } else {
         return 0;
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add packet size check and some fix

[Zhang Chen]

Sorry, This patch has been renamed.

please ignore this patch.


Thanks

Zhang Chen


On 02/25/2017 11:32 AM, Zhang Chen wrote:
> Add packet size check in colo_packet_compare_udp()
> and colo_packet_compare_udp(), rename function colo_packet_compare()
> to colo_packet_compare_common() that we will reuse it later.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 30 ++++++++++++++++++++++--------
>   1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 300f017..e75f0ae 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>    * return:    0  means packet same
>    *            > 0 || < 0 means packet different
>    */
> -static int colo_packet_compare(Packet *ppkt, Packet *spkt)
> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>   {
>       trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
>                                  inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> @@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
>       if (ppkt->size == spkt->size) {
>           return memcmp(ppkt->data, spkt->data, spkt->size);
>       } else {
> +        trace_colo_compare_main("Net packet size are not the same");
>           return -1;
>       }
>   }
> @@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
>   static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>   {
>       struct tcphdr *ptcp, *stcp;
> -    int res;
> +    int res, network_length;
>   
>       trace_colo_compare_main("compare tcp");
> +
>       if (ppkt->size != spkt->size) {
>           if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>               trace_colo_compare_main("pkt size not same");
> @@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>           return -1;
>       }
>   
> +    network_length = ppkt->ip->ip_hl * 4;
> +    if (ppkt->size < network_length + ETH_HLEN) {
> +        trace_colo_compare_main("tcp packet size error");
> +        return -1;
> +    }
> +
>       ptcp = (struct tcphdr *)ppkt->transport_header;
>       stcp = (struct tcphdr *)spkt->transport_header;
>   
> @@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>    */
>   static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>   {
> -    int ret;
> +    int ret, network_length;
>   
>       trace_colo_compare_main("compare udp");
> -    ret = colo_packet_compare(ppkt, spkt);
> +    network_length = ppkt->ip->ip_hl * 4;
> +    if (ppkt->size < network_length + ETH_HLEN) {
> +        trace_colo_compare_main("udp packet size error");
> +        return -1;
> +    }
> +
> +    ret = colo_packet_compare_common(ppkt, spkt);
>   
>       if (ret) {
>           trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>   
>       trace_colo_compare_main("compare icmp");
>       network_length = ppkt->ip->ip_hl * 4;
> -    if (ppkt->size != spkt->size ||
> -        ppkt->size < network_length + ETH_HLEN) {
> +    if (ppkt->size < network_length + ETH_HLEN) {
> +        trace_colo_compare_main("icmp packet size error");
>           return -1;
>       }
>   
> -    if (colo_packet_compare(ppkt, spkt)) {
> +    if (colo_packet_compare_common(ppkt, spkt)) {
>           trace_colo_compare_icmp_miscompare("primary pkt size",
>                                              ppkt->size);
>           qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
> @@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
>                                  inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>                                  inet_ntoa(spkt->ip->ip_src),
>                                  inet_ntoa(spkt->ip->ip_dst));
> -    return colo_packet_compare(ppkt, spkt);
> +    return colo_packet_compare_common(ppkt, spkt);
>   }
>   
>   static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and some fix

[Hailiang Zhang]

Hi,

On 2017/2/25 11:32, Zhang Chen wrote:
> Add packet minimum size check in colo_packet_compare_udp()
> and colo_packet_compare_udp() like colo_packet_compare_icmp(),
> rename function colo_packet_compare() to colo_packet_compare_common()
> that we will reuse it later.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 30 ++++++++++++++++++++++--------
>   1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 300f017..e75f0ae 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>    * return:    0  means packet same
>    *            > 0 || < 0 means packet different
>    */
> -static int colo_packet_compare(Packet *ppkt, Packet *spkt)
> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>   {
>       trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
>                                  inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> @@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
>       if (ppkt->size == spkt->size) {
>           return memcmp(ppkt->data, spkt->data, spkt->size);
>       } else {
> +        trace_colo_compare_main("Net packet size are not the same");
>           return -1;
>       }
>   }
> @@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
>   static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>   {
>       struct tcphdr *ptcp, *stcp;
> -    int res;
> +    int res, network_length;
>
>       trace_colo_compare_main("compare tcp");
> +
>       if (ppkt->size != spkt->size) {
>           if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>               trace_colo_compare_main("pkt size not same");
> @@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>           return -1;
>       }
>
> +    network_length = ppkt->ip->ip_hl * 4;
> +    if (ppkt->size < network_length + ETH_HLEN) {

I think the check here is useless, because you have such check in
parse_packet_early() which is been called before these helpers.
And what check you need to add is, to check if the packet's size
>= packet's length been record in ip header.

Like:
+++ b/net/colo.c
@@ -78,6 +78,12 @@ int parse_packet_early(Packet *pkt)
          trace_colo_proxy_main("pkt->size < network_header + network_length");
          return 1;
      }
+
+    if (pkt->size < ETH_HLEN + ntohs(pkt->ip->ip_len)) {
+        fprintf(stderr, "pkt->size %d < pkt expect total len %ld\n", pkt->size,
+            pkt_MAChdr_len + ntohs(pkt->ip->ip_len));
+        return -1;
+    }


> +        trace_colo_compare_main("tcp packet size error");
> +        return -1;
> +    }
> +
>       ptcp = (struct tcphdr *)ppkt->transport_header;
>       stcp = (struct tcphdr *)spkt->transport_header;
>
> @@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>    */
>   static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>   {
> -    int ret;
> +    int ret, network_length;
>
>       trace_colo_compare_main("compare udp");
> -    ret = colo_packet_compare(ppkt, spkt);
> +    network_length = ppkt->ip->ip_hl * 4;
> +    if (ppkt->size < network_length + ETH_HLEN) {
> +        trace_colo_compare_main("udp packet size error");
> +        return -1;
> +    }
> +
> +    ret = colo_packet_compare_common(ppkt, spkt);
>
>       if (ret) {
>           trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>
>       trace_colo_compare_main("compare icmp");
>       network_length = ppkt->ip->ip_hl * 4;
> -    if (ppkt->size != spkt->size ||
> -        ppkt->size < network_length + ETH_HLEN) {
> +    if (ppkt->size < network_length + ETH_HLEN) {
> +        trace_colo_compare_main("icmp packet size error");
>           return -1;
>       }
>
> -    if (colo_packet_compare(ppkt, spkt)) {
> +    if (colo_packet_compare_common(ppkt, spkt)) {
>           trace_colo_compare_icmp_miscompare("primary pkt size",
>                                              ppkt->size);
>           qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
> @@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
>                                  inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>                                  inet_ntoa(spkt->ip->ip_src),
>                                  inet_ntoa(spkt->ip->ip_dst));
> -    return colo_packet_compare(ppkt, spkt);
> +    return colo_packet_compare_common(ppkt, spkt);
>   }
>
>   static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>

Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Hailiang Zhang]

On 2017/2/25 11:32, Zhang Chen wrote:
> Add offset args for colo_packet_compare_common, optimize
> colo_packet_compare_icmp() and colo_packet_compare_udp()
> just compare the IP payload.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 28 +++++++++++++++++++++-------
>   1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index e75f0ae..9853232 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>    * return:    0  means packet same
>    *            > 0 || < 0 means packet different
>    */
> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
>   {
>       trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
>                                  inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>                                  inet_ntoa(spkt->ip->ip_dst));
>
>       if (ppkt->size == spkt->size) {
> -        return memcmp(ppkt->data, spkt->data, spkt->size);
> +        return memcmp(ppkt->data + offset, spkt->data + offset,
> +                      spkt->size - offset);
>       } else {
>           trace_colo_compare_main("Net packet size are not the same");
>           return -1;
> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>           spkt->ip->ip_sum = ppkt->ip->ip_sum;
>       }
>
> -    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
> -                (spkt->size - ETH_HLEN));
> +    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>

For tcp packets check, why not ignore the ip headers, just like udp packets check ?
Besides, here, can we compare the checksum stored in headers of tcp and udp
before call colo_packet_compare_common(), which i think will improve the comparing
performance.

Thanks.
Hailiang

>       if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>           trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>           return -1;
>       }
>
> -    ret = colo_packet_compare_common(ppkt, spkt);
> +    /*
> +     * Because of ppkt and spkt are both in the same connection,
> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
> +     * same with spkt. In addition, IP header's Identification is a random
> +     * field, we can handle it in IP fragmentation function later.
> +     * So we just compare the ip payload here.
> +     */
> +    ret = colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN);
>
>       if (ret) {
>           trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>           return -1;
>       }
>
> -    if (colo_packet_compare_common(ppkt, spkt)) {
> +    /*
> +     * Because of ppkt and spkt are both in the same connection,
> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
> +     * same with spkt. In addition, IP header's Identification is a random
> +     * field, we can handle it in IP fragmentation function later.
> +     * So we just compare the ip payload here.
> +     */
> +    if (colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN)) {
>           trace_colo_compare_icmp_miscompare("primary pkt size",
>                                              ppkt->size);
>           qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
>                                  inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>                                  inet_ntoa(spkt->ip->ip_src),
>                                  inet_ntoa(spkt->ip->ip_dst));
> -    return colo_packet_compare_common(ppkt, spkt);
> +    return colo_packet_compare_common(ppkt, spkt, 0);
>   }
>
>   static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>

Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Hailiang Zhang]

On 2017/2/25 11:32, Zhang Chen wrote:
> Add offset args for colo_packet_compare_common, optimize
> colo_packet_compare_icmp() and colo_packet_compare_udp()
> just compare the IP payload.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 28 +++++++++++++++++++++-------
>   1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index e75f0ae..9853232 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>    * return:    0  means packet same
>    *            > 0 || < 0 means packet different
>    */
> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
>   {
>       trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
>                                  inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>                                  inet_ntoa(spkt->ip->ip_dst));
>
>       if (ppkt->size == spkt->size) {

This check is useless, because we have done such checks in every caller.

> -        return memcmp(ppkt->data, spkt->data, spkt->size);
> +        return memcmp(ppkt->data + offset, spkt->data + offset,
> +                      spkt->size - offset);
>       } else {
>           trace_colo_compare_main("Net packet size are not the same");
>           return -1;
> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>           spkt->ip->ip_sum = ppkt->ip->ip_sum;
>       }
>
> -    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
> -                (spkt->size - ETH_HLEN));
> +    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>
>       if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>           trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>           return -1;
>       }
>
> -    ret = colo_packet_compare_common(ppkt, spkt);
> +    /*
> +     * Because of ppkt and spkt are both in the same connection,
> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
> +     * same with spkt. In addition, IP header's Identification is a random
> +     * field, we can handle it in IP fragmentation function later.
> +     * So we just compare the ip payload here.
> +     */
> +    ret = colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN);
>
>       if (ret) {
>           trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>           return -1;
>       }
>
> -    if (colo_packet_compare_common(ppkt, spkt)) {
> +    /*
> +     * Because of ppkt and spkt are both in the same connection,
> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
> +     * same with spkt. In addition, IP header's Identification is a random
> +     * field, we can handle it in IP fragmentation function later.
> +     * So we just compare the ip payload here.
> +     */
> +    if (colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN)) {
>           trace_colo_compare_icmp_miscompare("primary pkt size",
>                                              ppkt->size);
>           qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
>                                  inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>                                  inet_ntoa(spkt->ip->ip_src),
>                                  inet_ntoa(spkt->ip->ip_dst));
> -    return colo_packet_compare_common(ppkt, spkt);
> +    return colo_packet_compare_common(ppkt, spkt, 0);
>   }
>
>   static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>

Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and some fix

[Zhang Chen]

On 02/25/2017 02:43 PM, Hailiang Zhang wrote:
> Hi,
>
> On 2017/2/25 11:32, Zhang Chen wrote:
>> Add packet minimum size check in colo_packet_compare_udp()
>> and colo_packet_compare_udp() like colo_packet_compare_icmp(),
>> rename function colo_packet_compare() to colo_packet_compare_common()
>> that we will reuse it later.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/colo-compare.c | 30 ++++++++++++++++++++++--------
>>   1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 300f017..e75f0ae 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>>    * return:    0  means packet same
>>    *            > 0 || < 0 means packet different
>>    */
>> -static int colo_packet_compare(Packet *ppkt, Packet *spkt)
>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>>   {
>>       trace_colo_compare_ip_info(ppkt->size, 
>> inet_ntoa(ppkt->ip->ip_src),
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> @@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, 
>> Packet *spkt)
>>       if (ppkt->size == spkt->size) {
>>           return memcmp(ppkt->data, spkt->data, spkt->size);
>>       } else {
>> +        trace_colo_compare_main("Net packet size are not the same");
>>           return -1;
>>       }
>>   }
>> @@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt, 
>> Packet *spkt)
>>   static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>>   {
>>       struct tcphdr *ptcp, *stcp;
>> -    int res;
>> +    int res, network_length;
>>
>>       trace_colo_compare_main("compare tcp");
>> +
>>       if (ppkt->size != spkt->size) {
>>           if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>               trace_colo_compare_main("pkt size not same");
>> @@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt, 
>> Packet *ppkt)
>>           return -1;
>>       }
>>
>> +    network_length = ppkt->ip->ip_hl * 4;
>> +    if (ppkt->size < network_length + ETH_HLEN) {
>
> I think the check here is useless, because you have such check in
> parse_packet_early() which is been called before these helpers.
> And what check you need to add is, to check if the packet's size
>> = packet's length been record in ip header.
>
> Like:
> +++ b/net/colo.c
> @@ -78,6 +78,12 @@ int parse_packet_early(Packet *pkt)
>          trace_colo_proxy_main("pkt->size < network_header + 
> network_length");
>          return 1;
>      }
> +
> +    if (pkt->size < ETH_HLEN + ntohs(pkt->ip->ip_len)) {
> +        fprintf(stderr, "pkt->size %d < pkt expect total len %ld\n", 
> pkt->size,
> +            pkt_MAChdr_len + ntohs(pkt->ip->ip_len));
> +        return -1;
> +    }

This check we also have done in parse_packet_early()

     network_length = pkt->ip->ip_hl * 4;
     if (pkt->size < l2hdr_len + network_length) {
         trace_colo_proxy_main("pkt->size < network_header + 
network_length");
         return 1;
     }

So, maybe I need remove my before change and the compare_icmp() check.

Thanks
Zhang Chen



>
>
>> +        trace_colo_compare_main("tcp packet size error");
>> +        return -1;
>> +    }
>> +
>>       ptcp = (struct tcphdr *)ppkt->transport_header;
>>       stcp = (struct tcphdr *)spkt->transport_header;
>>
>> @@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet 
>> *spkt, Packet *ppkt)
>>    */
>>   static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>>   {
>> -    int ret;
>> +    int ret, network_length;
>>
>>       trace_colo_compare_main("compare udp");
>> -    ret = colo_packet_compare(ppkt, spkt);
>> +    network_length = ppkt->ip->ip_hl * 4;
>> +    if (ppkt->size < network_length + ETH_HLEN) {
>> +        trace_colo_compare_main("udp packet size error");
>> +        return -1;
>> +    }
>> +
>> +    ret = colo_packet_compare_common(ppkt, spkt);
>>
>>       if (ret) {
>>           trace_colo_compare_udp_miscompare("primary pkt size", 
>> ppkt->size);
>> @@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet 
>> *spkt, Packet *ppkt)
>>
>>       trace_colo_compare_main("compare icmp");
>>       network_length = ppkt->ip->ip_hl * 4;
>> -    if (ppkt->size != spkt->size ||
>> -        ppkt->size < network_length + ETH_HLEN) {
>> +    if (ppkt->size < network_length + ETH_HLEN) {
>> +        trace_colo_compare_main("icmp packet size error");
>>           return -1;
>>       }
>>
>> -    if (colo_packet_compare(ppkt, spkt)) {
>> +    if (colo_packet_compare_common(ppkt, spkt)) {
>>           trace_colo_compare_icmp_miscompare("primary pkt size",
>>                                              ppkt->size);
>>           qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>> @@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet 
>> *spkt, Packet *ppkt)
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> inet_ntoa(spkt->ip->ip_src),
>> inet_ntoa(spkt->ip->ip_dst));
>> -    return colo_packet_compare(ppkt, spkt);
>> +    return colo_packet_compare_common(ppkt, spkt);
>>   }
>>
>>   static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>>
>
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Zhang Chen]

On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
> On 2017/2/25 11:32, Zhang Chen wrote:
>> Add offset args for colo_packet_compare_common, optimize
>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>> just compare the IP payload.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/colo-compare.c | 28 +++++++++++++++++++++-------
>>   1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index e75f0ae..9853232 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>>    * return:    0  means packet same
>>    *            > 0 || < 0 means packet different
>>    */
>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, 
>> int offset)
>>   {
>>       trace_colo_compare_ip_info(ppkt->size, 
>> inet_ntoa(ppkt->ip->ip_src),
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet 
>> *ppkt, Packet *spkt)
>> inet_ntoa(spkt->ip->ip_dst));
>>
>>       if (ppkt->size == spkt->size) {
>> -        return memcmp(ppkt->data, spkt->data, spkt->size);
>> +        return memcmp(ppkt->data + offset, spkt->data + offset,
>> +                      spkt->size - offset);
>>       } else {
>>           trace_colo_compare_main("Net packet size are not the same");
>>           return -1;
>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt, 
>> Packet *ppkt)
>>           spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>       }
>>
>> -    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>> -                (spkt->size - ETH_HLEN));
>> +    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>
>
> For tcp packets check, why not ignore the ip headers, just like udp 
> packets check ?
> Besides, here, can we compare the checksum stored in headers of tcp 
> and udp
> before call colo_packet_compare_common(), which i think will improve 
> the comparing
> performance.

That's another way to compare the packet suggested by Dr. David Alan 
Gilbert,
It makes two packets IP header be same firstly, then compare all IP packet.
This way can tell people why we ignore IP header explicitly in other 
packets check like udp.
For performance, If we ignore the IP header that will reduce at least 20 
byte not to be compared.
So, ignore ip header have better comparing performance.

Thanks
Zhang Chen


>
> Thanks.
> Hailiang
>
>>       if (res != 0 && 
>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, 
>> Packet *ppkt)
>>           return -1;
>>       }
>>
>> -    ret = colo_packet_compare_common(ppkt, spkt);
>> +    /*
>> +     * Because of ppkt and spkt are both in the same connection,
>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>> +     * same with spkt. In addition, IP header's Identification is a 
>> random
>> +     * field, we can handle it in IP fragmentation function later.
>> +     * So we just compare the ip payload here.
>> +     */
>> +    ret = colo_packet_compare_common(ppkt, spkt, network_length + 
>> ETH_HLEN);
>>
>>       if (ret) {
>>           trace_colo_compare_udp_miscompare("primary pkt size", 
>> ppkt->size);
>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet 
>> *spkt, Packet *ppkt)
>>           return -1;
>>       }
>>
>> -    if (colo_packet_compare_common(ppkt, spkt)) {
>> +    /*
>> +     * Because of ppkt and spkt are both in the same connection,
>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>> +     * same with spkt. In addition, IP header's Identification is a 
>> random
>> +     * field, we can handle it in IP fragmentation function later.
>> +     * So we just compare the ip payload here.
>> +     */
>> +    if (colo_packet_compare_common(ppkt, spkt, network_length + 
>> ETH_HLEN)) {
>>           trace_colo_compare_icmp_miscompare("primary pkt size",
>>                                              ppkt->size);
>>           qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet 
>> *spkt, Packet *ppkt)
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> inet_ntoa(spkt->ip->ip_src),
>> inet_ntoa(spkt->ip->ip_dst));
>> -    return colo_packet_compare_common(ppkt, spkt);
>> +    return colo_packet_compare_common(ppkt, spkt, 0);
>>   }
>>
>>   static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>>
>
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Zhang Chen]

On 02/25/2017 03:26 PM, Hailiang Zhang wrote:
> On 2017/2/25 11:32, Zhang Chen wrote:
>> Add offset args for colo_packet_compare_common, optimize
>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>> just compare the IP payload.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/colo-compare.c | 28 +++++++++++++++++++++-------
>>   1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index e75f0ae..9853232 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>>    * return:    0  means packet same
>>    *            > 0 || < 0 means packet different
>>    */
>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, 
>> int offset)
>>   {
>>       trace_colo_compare_ip_info(ppkt->size, 
>> inet_ntoa(ppkt->ip->ip_src),
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet 
>> *ppkt, Packet *spkt)
>> inet_ntoa(spkt->ip->ip_dst));
>>
>>       if (ppkt->size == spkt->size) {
>
> This check is useless, because we have done such checks in every caller.

Oh, I forgot it, not in good condition.
I will remove it in next version.

Thanks
Zhang Chen


>
>> -        return memcmp(ppkt->data, spkt->data, spkt->size);
>> +        return memcmp(ppkt->data + offset, spkt->data + offset,
>> +                      spkt->size - offset);
>>       } else {
>>           trace_colo_compare_main("Net packet size are not the same");
>>           return -1;
>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt, 
>> Packet *ppkt)
>>           spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>       }
>>
>> -    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>> -                (spkt->size - ETH_HLEN));
>> +    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>
>>       if (res != 0 && 
>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, 
>> Packet *ppkt)
>>           return -1;
>>       }
>>
>> -    ret = colo_packet_compare_common(ppkt, spkt);
>> +    /*
>> +     * Because of ppkt and spkt are both in the same connection,
>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>> +     * same with spkt. In addition, IP header's Identification is a 
>> random
>> +     * field, we can handle it in IP fragmentation function later.
>> +     * So we just compare the ip payload here.
>> +     */
>> +    ret = colo_packet_compare_common(ppkt, spkt, network_length + 
>> ETH_HLEN);
>>
>>       if (ret) {
>>           trace_colo_compare_udp_miscompare("primary pkt size", 
>> ppkt->size);
>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet 
>> *spkt, Packet *ppkt)
>>           return -1;
>>       }
>>
>> -    if (colo_packet_compare_common(ppkt, spkt)) {
>> +    /*
>> +     * Because of ppkt and spkt are both in the same connection,
>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>> +     * same with spkt. In addition, IP header's Identification is a 
>> random
>> +     * field, we can handle it in IP fragmentation function later.
>> +     * So we just compare the ip payload here.
>> +     */
>> +    if (colo_packet_compare_common(ppkt, spkt, network_length + 
>> ETH_HLEN)) {
>>           trace_colo_compare_icmp_miscompare("primary pkt size",
>>                                              ppkt->size);
>>           qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet 
>> *spkt, Packet *ppkt)
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> inet_ntoa(spkt->ip->ip_src),
>> inet_ntoa(spkt->ip->ip_dst));
>> -    return colo_packet_compare_common(ppkt, spkt);
>> +    return colo_packet_compare_common(ppkt, spkt, 0);
>>   }
>>
>>   static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>>
>
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Hailiang Zhang]

On 2017/2/27 15:03, Zhang Chen wrote:
>
>
> On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
>> On 2017/2/25 11:32, Zhang Chen wrote:
>>> Add offset args for colo_packet_compare_common, optimize
>>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>>> just compare the IP payload.
>>>
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>> ---
>>>    net/colo-compare.c | 28 +++++++++++++++++++++-------
>>>    1 file changed, 21 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>> index e75f0ae..9853232 100644
>>> --- a/net/colo-compare.c
>>> +++ b/net/colo-compare.c
>>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>>>     * return:    0  means packet same
>>>     *            > 0 || < 0 means packet different
>>>     */
>>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>>> int offset)
>>>    {
>>>        trace_colo_compare_ip_info(ppkt->size,
>>> inet_ntoa(ppkt->ip->ip_src),
>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>>> *ppkt, Packet *spkt)
>>> inet_ntoa(spkt->ip->ip_dst));
>>>
>>>        if (ppkt->size == spkt->size) {
>>> -        return memcmp(ppkt->data, spkt->data, spkt->size);
>>> +        return memcmp(ppkt->data + offset, spkt->data + offset,
>>> +                      spkt->size - offset);
>>>        } else {
>>>            trace_colo_compare_main("Net packet size are not the same");
>>>            return -1;
>>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>>> Packet *ppkt)
>>>            spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>>        }
>>>
>>> -    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>>> -                (spkt->size - ETH_HLEN));
>>> +    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>>
>>
>> For tcp packets check, why not ignore the ip headers, just like udp
>> packets check ?
>> Besides, here, can we compare the checksum stored in headers of tcp
>> and udp
>> before call colo_packet_compare_common(), which i think will improve
>> the comparing
>> performance.
>
> That's another way to compare the packet suggested by Dr. David Alan
> Gilbert,
> It makes two packets IP header be same firstly, then compare all IP packet.
> This way can tell people why we ignore IP header explicitly in other
> packets check like udp.
> For performance, If we ignore the IP header that will reduce at least 20
> byte not to be compared.
> So, ignore ip header have better comparing performance.
>

OK, here, i think we can re-use the checksum value stored in headers of tcp
or udp, comparing it first before comparing the complete payload of tcp or udp
packets is another way to improve the performance, it is only 2 bytes.

Thanks.

> Thanks
> Zhang Chen
>
>
>>
>> Thanks.
>> Hailiang
>>
>>>        if (res != 0 &&
>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt,
>>> Packet *ppkt)
>>>            return -1;
>>>        }
>>>
>>> -    ret = colo_packet_compare_common(ppkt, spkt);
>>> +    /*
>>> +     * Because of ppkt and spkt are both in the same connection,
>>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>>> +     * same with spkt. In addition, IP header's Identification is a
>>> random
>>> +     * field, we can handle it in IP fragmentation function later.
>>> +     * So we just compare the ip payload here.
>>> +     */
>>> +    ret = colo_packet_compare_common(ppkt, spkt, network_length +
>>> ETH_HLEN);
>>>
>>>        if (ret) {
>>>            trace_colo_compare_udp_miscompare("primary pkt size",
>>> ppkt->size);
>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>>> *spkt, Packet *ppkt)
>>>            return -1;
>>>        }
>>>
>>> -    if (colo_packet_compare_common(ppkt, spkt)) {
>>> +    /*
>>> +     * Because of ppkt and spkt are both in the same connection,
>>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>>> +     * same with spkt. In addition, IP header's Identification is a
>>> random
>>> +     * field, we can handle it in IP fragmentation function later.
>>> +     * So we just compare the ip payload here.
>>> +     */
>>> +    if (colo_packet_compare_common(ppkt, spkt, network_length +
>>> ETH_HLEN)) {
>>>            trace_colo_compare_icmp_miscompare("primary pkt size",
>>>                                               ppkt->size);
>>>            qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>>> *spkt, Packet *ppkt)
>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>> inet_ntoa(spkt->ip->ip_src),
>>> inet_ntoa(spkt->ip->ip_dst));
>>> -    return colo_packet_compare_common(ppkt, spkt);
>>> +    return colo_packet_compare_common(ppkt, spkt, 0);
>>>    }
>>>
>>>    static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>>>
>>
>>
>>
>>
>> .
>>
>

Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Zhang Chen]

On 02/27/2017 03:28 PM, Hailiang Zhang wrote:
> On 2017/2/27 15:03, Zhang Chen wrote:
>>
>>
>> On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
>>> On 2017/2/25 11:32, Zhang Chen wrote:
>>>> Add offset args for colo_packet_compare_common, optimize
>>>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>>>> just compare the IP payload.
>>>>
>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>> ---
>>>>    net/colo-compare.c | 28 +++++++++++++++++++++-------
>>>>    1 file changed, 21 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>>> index e75f0ae..9853232 100644
>>>> --- a/net/colo-compare.c
>>>> +++ b/net/colo-compare.c
>>>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int 
>>>> mode)
>>>>     * return:    0  means packet same
>>>>     *            > 0 || < 0 means packet different
>>>>     */
>>>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>>>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>>>> int offset)
>>>>    {
>>>>        trace_colo_compare_ip_info(ppkt->size,
>>>> inet_ntoa(ppkt->ip->ip_src),
>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>>>> *ppkt, Packet *spkt)
>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>
>>>>        if (ppkt->size == spkt->size) {
>>>> -        return memcmp(ppkt->data, spkt->data, spkt->size);
>>>> +        return memcmp(ppkt->data + offset, spkt->data + offset,
>>>> +                      spkt->size - offset);
>>>>        } else {
>>>>            trace_colo_compare_main("Net packet size are not the 
>>>> same");
>>>>            return -1;
>>>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>>>> Packet *ppkt)
>>>>            spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>>>        }
>>>>
>>>> -    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>>>> -                (spkt->size - ETH_HLEN));
>>>> +    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>>>
>>>
>>> For tcp packets check, why not ignore the ip headers, just like udp
>>> packets check ?
>>> Besides, here, can we compare the checksum stored in headers of tcp
>>> and udp
>>> before call colo_packet_compare_common(), which i think will improve
>>> the comparing
>>> performance.
>>
>> That's another way to compare the packet suggested by Dr. David Alan
>> Gilbert,
>> It makes two packets IP header be same firstly, then compare all IP 
>> packet.
>> This way can tell people why we ignore IP header explicitly in other
>> packets check like udp.
>> For performance, If we ignore the IP header that will reduce at least 20
>> byte not to be compared.
>> So, ignore ip header have better comparing performance.
>>
>
> OK, here, i think we can re-use the checksum value stored in headers 
> of tcp
> or udp, comparing it first before comparing the complete payload of 
> tcp or udp
> packets is another way to improve the performance, it is only 2 bytes.

No, The IP header's checksum are always different, Because the IP 
header's ID field
are always different.

Thanks
Zhang Chen

>
> Thanks.
>
>> Thanks
>> Zhang Chen
>>
>>
>>>
>>> Thanks.
>>> Hailiang
>>>
>>>>        if (res != 0 &&
>>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt,
>>>> Packet *ppkt)
>>>>            return -1;
>>>>        }
>>>>
>>>> -    ret = colo_packet_compare_common(ppkt, spkt);
>>>> +    /*
>>>> +     * Because of ppkt and spkt are both in the same connection,
>>>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all 
>>>> are
>>>> +     * same with spkt. In addition, IP header's Identification is a
>>>> random
>>>> +     * field, we can handle it in IP fragmentation function later.
>>>> +     * So we just compare the ip payload here.
>>>> +     */
>>>> +    ret = colo_packet_compare_common(ppkt, spkt, network_length +
>>>> ETH_HLEN);
>>>>
>>>>        if (ret) {
>>>>            trace_colo_compare_udp_miscompare("primary pkt size",
>>>> ppkt->size);
>>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>>>> *spkt, Packet *ppkt)
>>>>            return -1;
>>>>        }
>>>>
>>>> -    if (colo_packet_compare_common(ppkt, spkt)) {
>>>> +    /*
>>>> +     * Because of ppkt and spkt are both in the same connection,
>>>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all 
>>>> are
>>>> +     * same with spkt. In addition, IP header's Identification is a
>>>> random
>>>> +     * field, we can handle it in IP fragmentation function later.
>>>> +     * So we just compare the ip payload here.
>>>> +     */
>>>> +    if (colo_packet_compare_common(ppkt, spkt, network_length +
>>>> ETH_HLEN)) {
>>>>            trace_colo_compare_icmp_miscompare("primary pkt size",
>>>> ppkt->size);
>>>>            qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>>>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>>>> *spkt, Packet *ppkt)
>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>> inet_ntoa(spkt->ip->ip_src),
>>>> inet_ntoa(spkt->ip->ip_dst));
>>>> -    return colo_packet_compare_common(ppkt, spkt);
>>>> +    return colo_packet_compare_common(ppkt, spkt, 0);
>>>>    }
>>>>
>>>>    static int colo_old_packet_check_one(Packet *pkt, int64_t 
>>>> *check_time)
>>>>
>>>
>>>
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Hailiang Zhang]

On 2017/2/27 15:34, Zhang Chen wrote:
>
>
> On 02/27/2017 03:28 PM, Hailiang Zhang wrote:
>> On 2017/2/27 15:03, Zhang Chen wrote:
>>>
>>>
>>> On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
>>>> On 2017/2/25 11:32, Zhang Chen wrote:
>>>>> Add offset args for colo_packet_compare_common, optimize
>>>>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>>>>> just compare the IP payload.
>>>>>
>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>> ---
>>>>>     net/colo-compare.c | 28 +++++++++++++++++++++-------
>>>>>     1 file changed, 21 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>>>> index e75f0ae..9853232 100644
>>>>> --- a/net/colo-compare.c
>>>>> +++ b/net/colo-compare.c
>>>>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int
>>>>> mode)
>>>>>      * return:    0  means packet same
>>>>>      *            > 0 || < 0 means packet different
>>>>>      */
>>>>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>>>>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>>>>> int offset)
>>>>>     {
>>>>>         trace_colo_compare_ip_info(ppkt->size,
>>>>> inet_ntoa(ppkt->ip->ip_src),
>>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>>>>> *ppkt, Packet *spkt)
>>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>>
>>>>>         if (ppkt->size == spkt->size) {
>>>>> -        return memcmp(ppkt->data, spkt->data, spkt->size);
>>>>> +        return memcmp(ppkt->data + offset, spkt->data + offset,
>>>>> +                      spkt->size - offset);
>>>>>         } else {
>>>>>             trace_colo_compare_main("Net packet size are not the
>>>>> same");
>>>>>             return -1;
>>>>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>>>>> Packet *ppkt)
>>>>>             spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>>>>         }
>>>>>
>>>>> -    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>>>>> -                (spkt->size - ETH_HLEN));
>>>>> +    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>>>>
>>>>
>>>> For tcp packets check, why not ignore the ip headers, just like udp
>>>> packets check ?
>>>> Besides, here, can we compare the checksum stored in headers of tcp
>>>> and udp
>>>> before call colo_packet_compare_common(), which i think will improve
>>>> the comparing
>>>> performance.
>>>
>>> That's another way to compare the packet suggested by Dr. David Alan
>>> Gilbert,
>>> It makes two packets IP header be same firstly, then compare all IP
>>> packet.
>>> This way can tell people why we ignore IP header explicitly in other
>>> packets check like udp.
>>> For performance, If we ignore the IP header that will reduce at least 20
>>> byte not to be compared.
>>> So, ignore ip header have better comparing performance.
>>>
>>
>> OK, here, i think we can re-use the checksum value stored in headers
>> of tcp
>> or udp, comparing it first before comparing the complete payload of
>> tcp or udp
>> packets is another way to improve the performance, it is only 2 bytes.
>
> No, The IP header's checksum are always different, Because the IP
> header's ID field
> are always different.
>

Not checksum in ip header， i mean checksum in tcp header or udp header.


> Thanks
> Zhang Chen
>
>>
>> Thanks.
>>
>>> Thanks
>>> Zhang Chen
>>>
>>>
>>>>
>>>> Thanks.
>>>> Hailiang
>>>>
>>>>>         if (res != 0 &&
>>>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>>>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt,
>>>>> Packet *ppkt)
>>>>>             return -1;
>>>>>         }
>>>>>
>>>>> -    ret = colo_packet_compare_common(ppkt, spkt);
>>>>> +    /*
>>>>> +     * Because of ppkt and spkt are both in the same connection,
>>>>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>>> are
>>>>> +     * same with spkt. In addition, IP header's Identification is a
>>>>> random
>>>>> +     * field, we can handle it in IP fragmentation function later.
>>>>> +     * So we just compare the ip payload here.
>>>>> +     */
>>>>> +    ret = colo_packet_compare_common(ppkt, spkt, network_length +
>>>>> ETH_HLEN);
>>>>>
>>>>>         if (ret) {
>>>>>             trace_colo_compare_udp_miscompare("primary pkt size",
>>>>> ppkt->size);
>>>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>>>>> *spkt, Packet *ppkt)
>>>>>             return -1;
>>>>>         }
>>>>>
>>>>> -    if (colo_packet_compare_common(ppkt, spkt)) {
>>>>> +    /*
>>>>> +     * Because of ppkt and spkt are both in the same connection,
>>>>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>>> are
>>>>> +     * same with spkt. In addition, IP header's Identification is a
>>>>> random
>>>>> +     * field, we can handle it in IP fragmentation function later.
>>>>> +     * So we just compare the ip payload here.
>>>>> +     */
>>>>> +    if (colo_packet_compare_common(ppkt, spkt, network_length +
>>>>> ETH_HLEN)) {
>>>>>             trace_colo_compare_icmp_miscompare("primary pkt size",
>>>>> ppkt->size);
>>>>>             qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>>>>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>>>>> *spkt, Packet *ppkt)
>>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>>> inet_ntoa(spkt->ip->ip_src),
>>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>> -    return colo_packet_compare_common(ppkt, spkt);
>>>>> +    return colo_packet_compare_common(ppkt, spkt, 0);
>>>>>     }
>>>>>
>>>>>     static int colo_old_packet_check_one(Packet *pkt, int64_t
>>>>> *check_time)
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>
>>
>>
>> .
>>
>

Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Zhang Chen]

On 02/27/2017 04:43 PM, Hailiang Zhang wrote:
> On 2017/2/27 15:34, Zhang Chen wrote:
>>
>>
>> On 02/27/2017 03:28 PM, Hailiang Zhang wrote:
>>> On 2017/2/27 15:03, Zhang Chen wrote:
>>>>
>>>>
>>>> On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
>>>>> On 2017/2/25 11:32, Zhang Chen wrote:
>>>>>> Add offset args for colo_packet_compare_common, optimize
>>>>>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>>>>>> just compare the IP payload.
>>>>>>
>>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>>     net/colo-compare.c | 28 +++++++++++++++++++++-------
>>>>>>     1 file changed, 21 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>>>>> index e75f0ae..9853232 100644
>>>>>> --- a/net/colo-compare.c
>>>>>> +++ b/net/colo-compare.c
>>>>>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int
>>>>>> mode)
>>>>>>      * return:    0  means packet same
>>>>>>      *            > 0 || < 0 means packet different
>>>>>>      */
>>>>>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>>>>>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>>>>>> int offset)
>>>>>>     {
>>>>>>         trace_colo_compare_ip_info(ppkt->size,
>>>>>> inet_ntoa(ppkt->ip->ip_src),
>>>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>>>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>>>>>> *ppkt, Packet *spkt)
>>>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>>>
>>>>>>         if (ppkt->size == spkt->size) {
>>>>>> -        return memcmp(ppkt->data, spkt->data, spkt->size);
>>>>>> +        return memcmp(ppkt->data + offset, spkt->data + offset,
>>>>>> +                      spkt->size - offset);
>>>>>>         } else {
>>>>>>             trace_colo_compare_main("Net packet size are not the
>>>>>> same");
>>>>>>             return -1;
>>>>>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>>>>>> Packet *ppkt)
>>>>>>             spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>>>>>         }
>>>>>>
>>>>>> -    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>>>>>> -                (spkt->size - ETH_HLEN));
>>>>>> +    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>>>>>
>>>>>
>>>>> For tcp packets check, why not ignore the ip headers, just like udp
>>>>> packets check ?
>>>>> Besides, here, can we compare the checksum stored in headers of tcp
>>>>> and udp
>>>>> before call colo_packet_compare_common(), which i think will improve
>>>>> the comparing
>>>>> performance.
>>>>
>>>> That's another way to compare the packet suggested by Dr. David Alan
>>>> Gilbert,
>>>> It makes two packets IP header be same firstly, then compare all IP
>>>> packet.
>>>> This way can tell people why we ignore IP header explicitly in other
>>>> packets check like udp.
>>>> For performance, If we ignore the IP header that will reduce at 
>>>> least 20
>>>> byte not to be compared.
>>>> So, ignore ip header have better comparing performance.
>>>>
>>>
>>> OK, here, i think we can re-use the checksum value stored in headers
>>> of tcp
>>> or udp, comparing it first before comparing the complete payload of
>>> tcp or udp
>>> packets is another way to improve the performance, it is only 2 bytes.
>>
>> No, The IP header's checksum are always different, Because the IP
>> header's ID field
>> are always different.
>>
>
> Not checksum in ip header， i mean checksum in tcp header or udp header.

TCP header can do it, but UDP header's checksum is optional.

Thanks
Zhang Chen

>
>
>> Thanks
>> Zhang Chen
>>
>>>
>>> Thanks.
>>>
>>>> Thanks
>>>> Zhang Chen
>>>>
>>>>
>>>>>
>>>>> Thanks.
>>>>> Hailiang
>>>>>
>>>>>>         if (res != 0 &&
>>>>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>>>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>>>>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet 
>>>>>> *spkt,
>>>>>> Packet *ppkt)
>>>>>>             return -1;
>>>>>>         }
>>>>>>
>>>>>> -    ret = colo_packet_compare_common(ppkt, spkt);
>>>>>> +    /*
>>>>>> +     * Because of ppkt and spkt are both in the same connection,
>>>>>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>>>> are
>>>>>> +     * same with spkt. In addition, IP header's Identification is a
>>>>>> random
>>>>>> +     * field, we can handle it in IP fragmentation function later.
>>>>>> +     * So we just compare the ip payload here.
>>>>>> +     */
>>>>>> +    ret = colo_packet_compare_common(ppkt, spkt, network_length +
>>>>>> ETH_HLEN);
>>>>>>
>>>>>>         if (ret) {
>>>>>>             trace_colo_compare_udp_miscompare("primary pkt size",
>>>>>> ppkt->size);
>>>>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>>>>>> *spkt, Packet *ppkt)
>>>>>>             return -1;
>>>>>>         }
>>>>>>
>>>>>> -    if (colo_packet_compare_common(ppkt, spkt)) {
>>>>>> +    /*
>>>>>> +     * Because of ppkt and spkt are both in the same connection,
>>>>>> +     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>>>> are
>>>>>> +     * same with spkt. In addition, IP header's Identification is a
>>>>>> random
>>>>>> +     * field, we can handle it in IP fragmentation function later.
>>>>>> +     * So we just compare the ip payload here.
>>>>>> +     */
>>>>>> +    if (colo_packet_compare_common(ppkt, spkt, network_length +
>>>>>> ETH_HLEN)) {
>>>>>>             trace_colo_compare_icmp_miscompare("primary pkt size",
>>>>>> ppkt->size);
>>>>>>             qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>>>>>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>>>>>> *spkt, Packet *ppkt)
>>>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>>>> inet_ntoa(spkt->ip->ip_src),
>>>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>>> -    return colo_packet_compare_common(ppkt, spkt);
>>>>>> +    return colo_packet_compare_common(ppkt, spkt, 0);
>>>>>>     }
>>>>>>
>>>>>>     static int colo_old_packet_check_one(Packet *pkt, int64_t
>>>>>> *check_time)
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add packet size check and some fix

[Jason Wang]

On 2017年02月25日 11:43, Zhang Chen wrote:
> Sorry, This patch has been renamed.
>
> please ignore this patch.
>
>
> Thanks
>
> Zhang Chen 

Want to repost to just withdraw this series (patch 2 does not apply)?

Thanks

Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add packet size check and some fix

[Zhang Chen]

On 02/28/2017 11:22 AM, Jason Wang wrote:
>
>
> On 2017年02月25日 11:43, Zhang Chen wrote:
>> Sorry, This patch has been renamed.
>>
>> please ignore this patch.
>>
>>
>> Thanks
>>
>> Zhang Chen 
>
> Want to repost to just withdraw this series (patch 2 does not apply)?
>

I have repost this patch, and will send V2 about this series later.
[Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and 
some fix

Thanks
Zhang Chen

> Thanks
>
>
>

-- 
Thanks
Zhang Chen