net/colo-compare.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
'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
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)) { >
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)) { >>
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)) { >>>
© 2016 - 2024 Red Hat, Inc.