qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] COLO-compare: Fix incorrect `if` logic
@ 2019-09-24 14:08 Fan Yang
  2019-09-24 15:35 ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 4+ messages in thread
From: Fan Yang @ 2019-09-24 14:08 UTC (permalink / raw)
  To: qemu-devel

'colo_mark_tcp_pkt' should return 'true' when packets are the same, and
'false' otherwise.  However, it returns 'true' when
'colo_compare_packet_payload' returns non-zero while
'colo_compare_packet_payload' is just a 'memcmp'.  The result is that
COLO-compare reports inconsistent TCP packets when they are actually
the same.

Signed-off-by: Fan Yang <Fan_Yang@sjtu.edu.cn>
---
 net/colo-compare.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 7489840bde..7ee17f2cf8 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -319,7 +319,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
     *mark = 0;
 
     if (ppkt->tcp_seq == spkt->tcp_seq && ppkt->seq_end == spkt->seq_end) {
-        if (colo_compare_packet_payload(ppkt, spkt,
+        if (!colo_compare_packet_payload(ppkt, spkt,
                                         ppkt->header_size, spkt->header_size,
                                         ppkt->payload_size)) {
             *mark = COLO_COMPARE_FREE_SECONDARY | COLO_COMPARE_FREE_PRIMARY;
@@ -329,7 +329,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
 
     /* one part of secondary packet payload still need to be compared */
     if (!after(ppkt->seq_end, spkt->seq_end)) {
-        if (colo_compare_packet_payload(ppkt, spkt,
+        if (!colo_compare_packet_payload(ppkt, spkt,
                                         ppkt->header_size + ppkt->offset,
                                         spkt->header_size + spkt->offset,
                                         ppkt->payload_size - ppkt->offset)) {
@@ -348,7 +348,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
         /* primary packet is longer than secondary packet, compare
          * the same part and mark the primary packet offset
          */
-        if (colo_compare_packet_payload(ppkt, spkt,
+        if (!colo_compare_packet_payload(ppkt, spkt,
                                         ppkt->header_size + ppkt->offset,
                                         spkt->header_size + spkt->offset,
                                         spkt->payload_size - spkt->offset)) {
-- 
2.17.1



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

* Re: [PATCH] COLO-compare: Fix incorrect `if` logic
  2019-09-24 14:08 [PATCH] COLO-compare: Fix incorrect `if` logic Fan Yang
@ 2019-09-24 15:35 ` Philippe Mathieu-Daudé
  2019-09-25  4:20   ` Jason Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-24 15:35 UTC (permalink / raw)
  To: Fan Yang, qemu-devel, Zhang Chen, Li Zhijian, Jason Wang

Hi Fan,

you forgot to Cc the maintainers (doing that for you):

./scripts/get_maintainer.pl -f net/colo-compare.c
Zhang Chen <chen.zhang@intel.com> (supporter:COLO Proxy)
Li Zhijian <lizhijian@cn.fujitsu.com> (supporter:COLO Proxy)
Jason Wang <jasowang@redhat.com> (maintainer:Network device ba...)
qemu-devel@nongnu.org (open list:All patches CC here)

On 9/24/19 4:08 PM, Fan Yang wrote:
> 'colo_mark_tcp_pkt' should return 'true' when packets are the same, and
> 'false' otherwise.  However, it returns 'true' when
> 'colo_compare_packet_payload' returns non-zero while
> 'colo_compare_packet_payload' is just a 'memcmp'.  The result is that
> COLO-compare reports inconsistent TCP packets when they are actually
> the same.
> 

Fixes: f449c9e549c
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> Signed-off-by: Fan Yang <Fan_Yang@sjtu.edu.cn>
> ---
>  net/colo-compare.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 7489840bde..7ee17f2cf8 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -319,7 +319,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
>      *mark = 0;
>  
>      if (ppkt->tcp_seq == spkt->tcp_seq && ppkt->seq_end == spkt->seq_end) {
> -        if (colo_compare_packet_payload(ppkt, spkt,
> +        if (!colo_compare_packet_payload(ppkt, spkt,
>                                          ppkt->header_size, spkt->header_size,
>                                          ppkt->payload_size)) {
>              *mark = COLO_COMPARE_FREE_SECONDARY | COLO_COMPARE_FREE_PRIMARY;
> @@ -329,7 +329,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
>  
>      /* one part of secondary packet payload still need to be compared */
>      if (!after(ppkt->seq_end, spkt->seq_end)) {
> -        if (colo_compare_packet_payload(ppkt, spkt,
> +        if (!colo_compare_packet_payload(ppkt, spkt,
>                                          ppkt->header_size + ppkt->offset,
>                                          spkt->header_size + spkt->offset,
>                                          ppkt->payload_size - ppkt->offset)) {
> @@ -348,7 +348,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
>          /* primary packet is longer than secondary packet, compare
>           * the same part and mark the primary packet offset
>           */
> -        if (colo_compare_packet_payload(ppkt, spkt,
> +        if (!colo_compare_packet_payload(ppkt, spkt,
>                                          ppkt->header_size + ppkt->offset,
>                                          spkt->header_size + spkt->offset,
>                                          spkt->payload_size - spkt->offset)) {
> 


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

* Re: [PATCH] COLO-compare: Fix incorrect `if` logic
  2019-09-24 15:35 ` Philippe Mathieu-Daudé
@ 2019-09-25  4:20   ` Jason Wang
  2019-09-25  5:53     ` Fan Yang
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Wang @ 2019-09-25  4:20 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Fan Yang, qemu-devel, Zhang Chen,
	Li Zhijian


On 2019/9/24 下午11:35, Philippe Mathieu-Daudé wrote:
> Hi Fan,
>
> you forgot to Cc the maintainers (doing that for you):
>
> ./scripts/get_maintainer.pl -f net/colo-compare.c
> Zhang Chen <chen.zhang@intel.com> (supporter:COLO Proxy)
> Li Zhijian <lizhijian@cn.fujitsu.com> (supporter:COLO Proxy)
> Jason Wang <jasowang@redhat.com> (maintainer:Network device ba...)
> qemu-devel@nongnu.org (open list:All patches CC here)
>
> On 9/24/19 4:08 PM, Fan Yang wrote:
>> 'colo_mark_tcp_pkt' should return 'true' when packets are the same, and
>> 'false' otherwise.  However, it returns 'true' when
>> 'colo_compare_packet_payload' returns non-zero while
>> 'colo_compare_packet_payload' is just a 'memcmp'.  The result is that
>> COLO-compare reports inconsistent TCP packets when they are actually
>> the same.
>>
> Fixes: f449c9e549c
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


Applied.

Thanks


>
>> Signed-off-by: Fan Yang <Fan_Yang@sjtu.edu.cn>
>> ---
>>  net/colo-compare.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 7489840bde..7ee17f2cf8 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -319,7 +319,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
>>      *mark = 0;
>>  
>>      if (ppkt->tcp_seq == spkt->tcp_seq && ppkt->seq_end == spkt->seq_end) {
>> -        if (colo_compare_packet_payload(ppkt, spkt,
>> +        if (!colo_compare_packet_payload(ppkt, spkt,
>>                                          ppkt->header_size, spkt->header_size,
>>                                          ppkt->payload_size)) {
>>              *mark = COLO_COMPARE_FREE_SECONDARY | COLO_COMPARE_FREE_PRIMARY;
>> @@ -329,7 +329,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
>>  
>>      /* one part of secondary packet payload still need to be compared */
>>      if (!after(ppkt->seq_end, spkt->seq_end)) {
>> -        if (colo_compare_packet_payload(ppkt, spkt,
>> +        if (!colo_compare_packet_payload(ppkt, spkt,
>>                                          ppkt->header_size + ppkt->offset,
>>                                          spkt->header_size + spkt->offset,
>>                                          ppkt->payload_size - ppkt->offset)) {
>> @@ -348,7 +348,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
>>          /* primary packet is longer than secondary packet, compare
>>           * the same part and mark the primary packet offset
>>           */
>> -        if (colo_compare_packet_payload(ppkt, spkt,
>> +        if (!colo_compare_packet_payload(ppkt, spkt,
>>                                          ppkt->header_size + ppkt->offset,
>>                                          spkt->header_size + spkt->offset,
>>                                          spkt->payload_size - spkt->offset)) {
>>


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

* Re: [PATCH] COLO-compare: Fix incorrect `if` logic
  2019-09-25  4:20   ` Jason Wang
@ 2019-09-25  5:53     ` Fan Yang
  0 siblings, 0 replies; 4+ messages in thread
From: Fan Yang @ 2019-09-25  5:53 UTC (permalink / raw)
  To: Jason Wang, Philippe Mathieu-Daudé, qemu-devel, Zhang Chen,
	Li Zhijian

OK, thank you all :)

Jason Wang <jasowang@redhat.com> writes:

> On 2019/9/24 下午11:35, Philippe Mathieu-Daudé wrote:
>> Hi Fan,
>>
>> you forgot to Cc the maintainers (doing that for you):
>>
>> ./scripts/get_maintainer.pl -f net/colo-compare.c
>> Zhang Chen <chen.zhang@intel.com> (supporter:COLO Proxy)
>> Li Zhijian <lizhijian@cn.fujitsu.com> (supporter:COLO Proxy)
>> Jason Wang <jasowang@redhat.com> (maintainer:Network device ba...)
>> qemu-devel@nongnu.org (open list:All patches CC here)
>>
>> On 9/24/19 4:08 PM, Fan Yang wrote:
>>> 'colo_mark_tcp_pkt' should return 'true' when packets are the same, and
>>> 'false' otherwise.  However, it returns 'true' when
>>> 'colo_compare_packet_payload' returns non-zero while
>>> 'colo_compare_packet_payload' is just a 'memcmp'.  The result is that
>>> COLO-compare reports inconsistent TCP packets when they are actually
>>> the same.
>>>
>> Fixes: f449c9e549c
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>
> Applied.
>
> Thanks
>
>
>>
>>> Signed-off-by: Fan Yang <Fan_Yang@sjtu.edu.cn>
>>> ---
>>>  net/colo-compare.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>> index 7489840bde..7ee17f2cf8 100644
>>> --- a/net/colo-compare.c
>>> +++ b/net/colo-compare.c
>>> @@ -319,7 +319,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
>>>      *mark = 0;
>>>  
>>>      if (ppkt->tcp_seq == spkt->tcp_seq && ppkt->seq_end == spkt->seq_end) {
>>> -        if (colo_compare_packet_payload(ppkt, spkt,
>>> +        if (!colo_compare_packet_payload(ppkt, spkt,
>>>                                          ppkt->header_size, spkt->header_size,
>>>                                          ppkt->payload_size)) {
>>>              *mark = COLO_COMPARE_FREE_SECONDARY | COLO_COMPARE_FREE_PRIMARY;
>>> @@ -329,7 +329,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
>>>  
>>>      /* one part of secondary packet payload still need to be compared */
>>>      if (!after(ppkt->seq_end, spkt->seq_end)) {
>>> -        if (colo_compare_packet_payload(ppkt, spkt,
>>> +        if (!colo_compare_packet_payload(ppkt, spkt,
>>>                                          ppkt->header_size + ppkt->offset,
>>>                                          spkt->header_size + spkt->offset,
>>>                                          ppkt->payload_size - ppkt->offset)) {
>>> @@ -348,7 +348,7 @@ static bool colo_mark_tcp_pkt(Packet *ppkt, Packet *spkt,
>>>          /* primary packet is longer than secondary packet, compare
>>>           * the same part and mark the primary packet offset
>>>           */
>>> -        if (colo_compare_packet_payload(ppkt, spkt,
>>> +        if (!colo_compare_packet_payload(ppkt, spkt,
>>>                                          ppkt->header_size + ppkt->offset,
>>>                                          spkt->header_size + spkt->offset,
>>>                                          spkt->payload_size - spkt->offset)) {
>>>


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

end of thread, other threads:[~2019-09-25  5:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-24 14:08 [PATCH] COLO-compare: Fix incorrect `if` logic Fan Yang
2019-09-24 15:35 ` Philippe Mathieu-Daudé
2019-09-25  4:20   ` Jason Wang
2019-09-25  5:53     ` Fan Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).