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

[Qemu-devel] [PATCH V2 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: 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

[Qemu-devel] [PATCH V2 1/3] COLO-compare: Rename compare function and remove duplicate codes

[Zhang Chen]

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

[Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp

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

Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp

[Jason Wang]

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)

Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp

[Zhang Chen]

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

Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp

[Jason Wang]

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

Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp

[Zhang Chen]

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

Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp

[Jason Wang]

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

[Qemu-devel] [PATCH V2 3/3] COLO-compare: Fix icmp and udp compare different packet always dump bug

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