All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V2 0/3] COLO-compare: Optimize the code and fix some bug
@ 2017-02-28  5:18 Zhang Chen
  2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 1/3] COLO-compare: Rename compare function and remove duplicate codes Zhang Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Zhang Chen @ 2017-02-28  5:18 UTC (permalink / raw)
  To: qemu devel, Jason Wang
  Cc: Zhang Chen, zhanghailiang, eddie . dong, bian naimeng, Li Zhijian

This series we will Optimize the code and fix some bug.
Patch1: Rename colo_packet_compare() and remove duplicate check in compare icmp.
Patch2: Optimize compare_common and increase compare performance.
Patch3: Fix debug info always print bug.


Zhang Chen (3):
  COLO-compare: Rename compare function and remove duplicate codes
  COLO-compare: Optimize compare_common and compare_tcp
  COLO-compare: Fix icmp and udp compare different packet always dump
    bug

 net/colo-compare.c | 66 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 25 deletions(-)

-- 
2.7.4

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

* [Qemu-devel] [PATCH V2 1/3] COLO-compare: Rename compare function and remove duplicate codes
  2017-02-28  5:18 [Qemu-devel] [PATCH V2 0/3] COLO-compare: Optimize the code and fix some bug Zhang Chen
@ 2017-02-28  5:18 ` Zhang Chen
  2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp Zhang Chen
  2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 3/3] COLO-compare: Fix icmp and udp compare different packet always dump bug Zhang Chen
  2 siblings, 0 replies; 9+ messages in thread
From: Zhang Chen @ 2017-02-28  5:18 UTC (permalink / raw)
  To: qemu devel, Jason Wang
  Cc: Zhang Chen, zhanghailiang, eddie . dong, bian naimeng, Li Zhijian

Rename colo_packet_compare() to colo_packet_compare_common() that
make tcp_compare udp_compare icmp_compare reuse this function.
Remove minimum packet size check in icmp_compare, because we have
check this in parse_packet_early().

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

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 300f017..602a758 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;
     }
 }
@@ -205,6 +206,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
     int res;
 
     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");
@@ -263,7 +265,8 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
     int ret;
 
     trace_colo_compare_main("compare udp");
-    ret = colo_packet_compare(ppkt, spkt);
+
+    ret = colo_packet_compare_common(ppkt, spkt);
 
     if (ret) {
         trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
@@ -281,16 +284,9 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
  */
 static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
 {
-    int network_length;
-
     trace_colo_compare_main("compare icmp");
-    network_length = ppkt->ip->ip_hl * 4;
-    if (ppkt->size != spkt->size ||
-        ppkt->size < network_length + ETH_HLEN) {
-        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 +312,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

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

* [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp
  2017-02-28  5:18 [Qemu-devel] [PATCH V2 0/3] COLO-compare: Optimize the code and fix some bug Zhang Chen
  2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 1/3] COLO-compare: Rename compare function and remove duplicate codes Zhang Chen
@ 2017-02-28  5:18 ` Zhang Chen
  2017-02-28  5:47   ` Jason Wang
  2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 3/3] COLO-compare: Fix icmp and udp compare different packet always dump bug Zhang Chen
  2 siblings, 1 reply; 9+ messages in thread
From: Zhang Chen @ 2017-02-28  5:18 UTC (permalink / raw)
  To: qemu devel, Jason Wang
  Cc: Zhang Chen, zhanghailiang, eddie . dong, bian naimeng, Li Zhijian

Add offset args for colo_packet_compare_common, optimize
colo_packet_compare_icmp() and colo_packet_compare_udp()
just compare the IP payload. Before compare all tcp packet,
we compare tcp checksum firstly, this function can get
better performance.

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

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 602a758..61fcdf2 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;
@@ -207,13 +208,6 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
 
     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");
-        }
-        return -1;
-    }
-
     ptcp = (struct tcphdr *)ppkt->transport_header;
     stcp = (struct tcphdr *)spkt->transport_header;
 
@@ -231,8 +225,11 @@ 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));
+    if (ptcp->th_sum == stcp->th_sum) {
+        res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
+    } else {
+        res = -1;
+    }
 
     if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
         trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
@@ -263,10 +260,18 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
 static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
 {
     int ret;
+    int network_length = ppkt->ip->ip_hl * 4;
 
     trace_colo_compare_main("compare udp");
 
-    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);
@@ -284,9 +289,18 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
  */
 static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
 {
+    int network_length = ppkt->ip->ip_hl * 4;
+
     trace_colo_compare_main("compare icmp");
 
-    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",
@@ -312,7 +326,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

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

* [Qemu-devel] [PATCH V2 3/3] COLO-compare: Fix icmp and udp compare different packet always dump bug
  2017-02-28  5:18 [Qemu-devel] [PATCH V2 0/3] COLO-compare: Optimize the code and fix some bug Zhang Chen
  2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 1/3] COLO-compare: Rename compare function and remove duplicate codes Zhang Chen
  2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp Zhang Chen
@ 2017-02-28  5:18 ` Zhang Chen
  2 siblings, 0 replies; 9+ messages in thread
From: Zhang Chen @ 2017-02-28  5:18 UTC (permalink / raw)
  To: qemu devel, Jason Wang
  Cc: Zhang Chen, zhanghailiang, eddie . dong, bian naimeng, Li Zhijian

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 61fcdf2..6b38723 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -275,9 +275,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 pri pkt",
+                         ppkt->size);
+            qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
+                         spkt->size);
+        }
     }
 
     return ret;
@@ -303,12 +307,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 pri pkt",
+                         ppkt->size);
+            qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
+                         spkt->size);
+        }
         return -1;
     } else {
         return 0;
-- 
2.7.4

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

* Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp
  2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp Zhang Chen
@ 2017-02-28  5:47   ` Jason Wang
  2017-02-28  6:11     ` Zhang Chen
  0 siblings, 1 reply; 9+ messages in thread
From: Jason Wang @ 2017-02-28  5:47 UTC (permalink / raw)
  To: Zhang Chen, qemu devel
  Cc: zhanghailiang, eddie . dong, bian naimeng, Li Zhijian



On 2017年02月28日 13:18, 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. Before compare all tcp packet,
> we compare tcp checksum firstly, this function can get
> better performance.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 42 ++++++++++++++++++++++++++++--------------
>   1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 602a758..61fcdf2 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;
> @@ -207,13 +208,6 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>   
>       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");
> -        }
> -        return -1;
> -    }
> -
>       ptcp = (struct tcphdr *)ppkt->transport_header;
>       stcp = (struct tcphdr *)spkt->transport_header;
>   
> @@ -231,8 +225,11 @@ 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));
> +    if (ptcp->th_sum == stcp->th_sum) {
> +        res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
> +    } else {
> +        res = -1;
> +    }
>   
>       if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>           trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
> @@ -263,10 +260,18 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>   static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>   {
>       int ret;
> +    int network_length = ppkt->ip->ip_hl * 4;

network_header_length looks better.

>   
>       trace_colo_compare_main("compare udp");
>   
> -    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.
> +     */

I'm afraid there're some other fields were ignored e.g TOS,TTL, and 
options. We'd better explain this in the comment too.

> +    ret = colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN);
>   
>       if (ret) {
>           trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -284,9 +289,18 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>    */
>   static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>   {
> +    int network_length = ppkt->ip->ip_hl * 4;
> +
>       trace_colo_compare_main("compare icmp");
>   
> -    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",
> @@ -312,7 +326,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)

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

* Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp
  2017-02-28  5:47   ` Jason Wang
@ 2017-02-28  6:11     ` Zhang Chen
  2017-03-02  7:51       ` Jason Wang
  0 siblings, 1 reply; 9+ messages in thread
From: Zhang Chen @ 2017-02-28  6:11 UTC (permalink / raw)
  To: Jason Wang, qemu devel
  Cc: zhangchen.fnst, zhanghailiang, eddie . dong, bian naimeng, Li Zhijian



On 02/28/2017 01:47 PM, Jason Wang wrote:
>
>
> On 2017年02月28日 13:18, 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. Before compare all tcp packet,
>> we compare tcp checksum firstly, this function can get
>> better performance.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/colo-compare.c | 42 ++++++++++++++++++++++++++++--------------
>>   1 file changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 602a758..61fcdf2 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;
>> @@ -207,13 +208,6 @@ static int colo_packet_compare_tcp(Packet *spkt, 
>> Packet *ppkt)
>>         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");
>> -        }
>> -        return -1;
>> -    }
>> -
>>       ptcp = (struct tcphdr *)ppkt->transport_header;
>>       stcp = (struct tcphdr *)spkt->transport_header;
>>   @@ -231,8 +225,11 @@ 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));
>> +    if (ptcp->th_sum == stcp->th_sum) {
>> +        res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>> +    } else {
>> +        res = -1;
>> +    }
>>         if (res != 0 && 
>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>> @@ -263,10 +260,18 @@ static int colo_packet_compare_tcp(Packet 
>> *spkt, Packet *ppkt)
>>   static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>>   {
>>       int ret;
>> +    int network_length = ppkt->ip->ip_hl * 4;
>
> network_header_length looks better.

OK, I will rename it.

>
>>         trace_colo_compare_main("compare udp");
>>   -    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.
>> +     */
>
> I'm afraid there're some other fields were ignored e.g TOS,TTL, and 
> options. We'd better explain this in the comment too.

OK, How about this?

+    /*
+     * 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. we ignored all IP header
+     * include other field like TOS,TTL,IP Checksum.
+     */

Thanks
Zhang Chen


>
>> +    ret = colo_packet_compare_common(ppkt, spkt, network_length + 
>> ETH_HLEN);
>>         if (ret) {
>>           trace_colo_compare_udp_miscompare("primary pkt size", 
>> ppkt->size);
>> @@ -284,9 +289,18 @@ static int colo_packet_compare_udp(Packet *spkt, 
>> Packet *ppkt)
>>    */
>>   static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>>   {
>> +    int network_length = ppkt->ip->ip_hl * 4;
>> +
>>       trace_colo_compare_main("compare icmp");
>>   -    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",
>> @@ -312,7 +326,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

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

* Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp
  2017-02-28  6:11     ` Zhang Chen
@ 2017-03-02  7:51       ` Jason Wang
  2017-03-02  8:15         ` Zhang Chen
  0 siblings, 1 reply; 9+ messages in thread
From: Jason Wang @ 2017-03-02  7:51 UTC (permalink / raw)
  To: Zhang Chen, qemu devel
  Cc: Li Zhijian, bian naimeng, eddie . dong, zhanghailiang



On 2017年02月28日 14:11, Zhang Chen wrote:
>>
>
> OK, How about this?
>
> +    /*
> +     * 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. we ignored all IP header
> +     * include other field like TOS,TTL,IP Checksum.
> +     */
>
> Thanks
> Zhang Chen

But this does not explain why we can ignore those fields.

Thanks

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

* Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp
  2017-03-02  7:51       ` Jason Wang
@ 2017-03-02  8:15         ` Zhang Chen
  2017-03-02  9:35           ` Jason Wang
  0 siblings, 1 reply; 9+ messages in thread
From: Zhang Chen @ 2017-03-02  8:15 UTC (permalink / raw)
  To: Jason Wang, qemu devel
  Cc: zhangchen.fnst, Li Zhijian, bian naimeng, eddie . dong, zhanghailiang



On 03/02/2017 03:51 PM, Jason Wang wrote:
>
>
> On 2017年02月28日 14:11, Zhang Chen wrote:
>>>
>>
>> OK, How about this?
>>
>> +    /*
>> +     * 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. we ignored all IP header
>> +     * include other field like TOS,TTL,IP Checksum.
>> +     */
>>
>> Thanks
>> Zhang Chen
>
> But this does not explain why we can ignore those fields.

+    /*
+     * 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.
+     * COLO just concern the response net packet payload from primary guest
+     * and secondary guest are same or not, So we ignored all IP header 
include
+     * other field like TOS,TTL,IP Checksum. we only need to compare
+     * the ip payload here.
+     */

How about this fixed comments?

Thanks
Zhang Chen


>
> Thanks
>
>
>

-- 
Thanks
Zhang Chen

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

* Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp
  2017-03-02  8:15         ` Zhang Chen
@ 2017-03-02  9:35           ` Jason Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Jason Wang @ 2017-03-02  9:35 UTC (permalink / raw)
  To: Zhang Chen, qemu devel
  Cc: eddie . dong, bian naimeng, Li Zhijian, zhanghailiang



On 2017年03月02日 16:15, Zhang Chen wrote:
>
>
> On 03/02/2017 03:51 PM, Jason Wang wrote:
>>
>>
>> On 2017年02月28日 14:11, Zhang Chen wrote:
>>>>
>>>
>>> OK, How about this?
>>>
>>> +    /*
>>> +     * 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. we ignored all IP 
>>> header
>>> +     * include other field like TOS,TTL,IP Checksum.
>>> +     */
>>>
>>> Thanks
>>> Zhang Chen
>>
>> But this does not explain why we can ignore those fields.
>
> +    /*
> +     * 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.
> +     * COLO just concern the response net packet payload from primary 
> guest
> +     * and secondary guest are same or not, So we ignored all IP 
> header include
> +     * other field like TOS,TTL,IP Checksum. we only need to compare
> +     * the ip payload here.
> +     */
>
> How about this fixed comments?
>
> Thanks
> Zhang Chen
>
>

Looks ok.

Thanks

>>
>> Thanks
>>
>>
>>
>

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

end of thread, other threads:[~2017-03-02  9:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-28  5:18 [Qemu-devel] [PATCH V2 0/3] COLO-compare: Optimize the code and fix some bug Zhang Chen
2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 1/3] COLO-compare: Rename compare function and remove duplicate codes Zhang Chen
2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp Zhang Chen
2017-02-28  5:47   ` Jason Wang
2017-02-28  6:11     ` Zhang Chen
2017-03-02  7:51       ` Jason Wang
2017-03-02  8:15         ` Zhang Chen
2017-03-02  9:35           ` Jason Wang
2017-02-28  5:18 ` [Qemu-devel] [PATCH V2 3/3] COLO-compare: Fix icmp and udp compare different packet always dump bug Zhang Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.