| Message ID | 6991b2261016c48122aaf7d71e33cc1d74b620fd.1512552817.git.maozy.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
| Series | Rewrite TCP packet comparison in colo | expand |
On Wed, Dec 6, 2017 at 5:57 PM, Mao Zhongyi <maozy.fnst@cn.fujitsu.com> wrote: > Modified the function colo_packet_compare_common to prepare for the > tcp packet comparison in the next patch. > > Cc: Zhang Chen <zhangckid@gmail.com> > Cc: Li Zhijian <lizhijian@cn.fujitsu.com> > Cc: Jason Wang <jasowang@redhat.com> > > Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> > --- > net/colo-compare.c | 71 ++++++++++++++++++++++++++++++ > ------------------------ > 1 file changed, 39 insertions(+), 32 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index 0afb5f0..f833eba 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -191,10 +191,11 @@ static int packet_enqueue(CompareState *s, int mode, > Connection **con) > * return: 0 means packet same > * > 0 || < 0 means packet different > */ > -static int colo_packet_compare_common(Packet *ppkt, > - Packet *spkt, > - int poffset, > - int soffset) > +static int colo_compare_packet_payload(Packet *ppkt, > + Packet *spkt, > + uint16_t poffset, > + uint16_t soffset, > + uint16_t len) > { > if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { > char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], > sec_ip_dst[20]; > @@ -209,17 +210,7 @@ static int colo_packet_compare_common(Packet *ppkt, > sec_ip_src, sec_ip_dst); > } > > - poffset = ppkt->vnet_hdr_len + poffset; > - soffset = ppkt->vnet_hdr_len + soffset; > - > - if (ppkt->size - poffset == spkt->size - soffset) { > - return memcmp(ppkt->data + poffset, > - spkt->data + soffset, > - spkt->size - soffset); > - } else { > - trace_colo_compare_main("Net packet size are not the same"); > - return -1; > - } > + return memcmp(ppkt->data + poffset, spkt->data + soffset, len); > } > > /* > @@ -274,16 +265,23 @@ static int colo_packet_compare_tcp(Packet *spkt, > Packet *ppkt) > ptrdiff_t ptcp_offset, stcp_offset; > > ptcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data > - + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; > + + (ptcp->th_off << 2) - ppkt->vnet_hdr_len; > stcp_offset = spkt->transport_header - (uint8_t *)spkt->data > - + (stcp->th_off * 4) - spkt->vnet_hdr_len; > + + (stcp->th_off << 2) - spkt->vnet_hdr_len; > /* > * When network is busy, some tcp options(like sack) will > unpredictable > * occur in primary side or secondary side. it will make packet size > * not same, but the two packet's payload is identical. colo just > * care about packet payload, so we skip the option field. > */ > In the patch 1，you should remove this comments, it's out of date. > - res = colo_packet_compare_common(ppkt, spkt, ptcp_offset, > stcp_offset); > + if (ppkt->size - ptcp_offset == spkt->size - stcp_offset) { > + res = colo_compare_packet_payload(ppkt, spkt, > + ptcp_offset, stcp_offset, > + ppkt->size - ptcp_offset); > + } else { > + trace_colo_compare_main("TCP: the size of packets are different"); > Should fix this comments to "TCP: payload size of packets are diffenrent!". Thanks Zhang Chen > + res = -1; > + } > > if (res != 0 && > trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { > @@ -325,8 +323,8 @@ static int colo_packet_compare_tcp(Packet *spkt, > Packet *ppkt) > */ > static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) > { > - int ret; > - int network_header_length = ppkt->ip->ip_hl * 4; > + uint16_t network_header_length = ppkt->ip->ip_hl << 2; > + uint16_t offset = network_header_length + ETH_HLEN + > ppkt->vnet_hdr_len; > > trace_colo_compare_main("compare udp"); > > @@ -340,11 +338,12 @@ static int colo_packet_compare_udp(Packet *spkt, > Packet *ppkt) > * other field like TOS,TTL,IP Checksum. we only need to compare > * the ip payload here. > */ > - ret = colo_packet_compare_common(ppkt, spkt, > - network_header_length + ETH_HLEN, > - network_header_length + ETH_HLEN); > - > - if (ret) { > + if (ppkt->size != spkt->size) { > + trace_colo_compare_main("UDP: the size of packets are different"); > + return -1; > + } > + if (colo_compare_packet_payload(ppkt, spkt, offset, offset, > + ppkt->size - offset)) { > trace_colo_compare_udp_miscompare("primary pkt size", > ppkt->size); > trace_colo_compare_udp_miscompare("Secondary pkt size", > spkt->size); > if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) > { > @@ -353,9 +352,10 @@ static int colo_packet_compare_udp(Packet *spkt, > Packet *ppkt) > qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec > pkt", > spkt->size); > } > + return -1; > + } else { > + return 0; > } > - > - return ret; > } > > /* > @@ -364,7 +364,8 @@ static int colo_packet_compare_udp(Packet *spkt, > Packet *ppkt) > */ > static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) > { > - int network_header_length = ppkt->ip->ip_hl * 4; > + uint16_t network_header_length = ppkt->ip->ip_hl << 2; > + uint16_t offset = network_header_length + ETH_HLEN + > ppkt->vnet_hdr_len; > > trace_colo_compare_main("compare icmp"); > > @@ -378,9 +379,12 @@ static int colo_packet_compare_icmp(Packet *spkt, > Packet *ppkt) > * other field like TOS,TTL,IP Checksum. we only need to compare > * the ip payload here. > */ > - if (colo_packet_compare_common(ppkt, spkt, > - network_header_length + ETH_HLEN, > - network_header_length + ETH_HLEN)) { > + if (ppkt->size != spkt->size) { > + trace_colo_compare_main("ICMP: the size of packets are > different"); > + return -1; > + } > + if (colo_compare_packet_payload(ppkt, spkt, offset, offset, > + ppkt->size - offset)) { > trace_colo_compare_icmp_miscompare("primary pkt size", > ppkt->size); > trace_colo_compare_icmp_miscompare("Secondary pkt size", > @@ -403,6 +407,8 @@ static int colo_packet_compare_icmp(Packet *spkt, > Packet *ppkt) > */ > static int colo_packet_compare_other(Packet *spkt, Packet *ppkt) > { > + uint16_t offset = ppkt->vnet_hdr_len; > + > trace_colo_compare_main("compare other"); > if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { > char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], > sec_ip_dst[20]; > @@ -417,7 +423,8 @@ static int colo_packet_compare_other(Packet *spkt, > Packet *ppkt) > sec_ip_src, sec_ip_dst); > } > > - return colo_packet_compare_common(ppkt, spkt, 0, 0); > + return colo_compare_packet_payload(ppkt, spkt, offset, offset, > + ppkt->size - offset); > } > > static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time) > -- > 2.9.4 > > > >
On 12/12/2017 11:19 PM, Zhang Chen wrote: > > > On Wed, Dec 6, 2017 at 5:57 PM, Mao Zhongyi <maozy.fnst@cn.fujitsu.com <mailto:maozy.fnst@cn.fujitsu.com>> wrote: > > Modified the function colo_packet_compare_common to prepare for the > tcp packet comparison in the next patch. > > Cc: Zhang Chen <zhangckid@gmail.com <mailto:zhangckid@gmail.com>> > Cc: Li Zhijian <lizhijian@cn.fujitsu.com <mailto:lizhijian@cn.fujitsu.com>> > Cc: Jason Wang <jasowang@redhat.com <mailto:jasowang@redhat.com>> > > Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com <mailto:maozy.fnst@cn.fujitsu.com>> > --- > net/colo-compare.c | 71 ++++++++++++++++++++++++++++++------------------------ > 1 file changed, 39 insertions(+), 32 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index 0afb5f0..f833eba 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -191,10 +191,11 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) > * return: 0 means packet same > * > 0 || < 0 means packet different > */ > -static int colo_packet_compare_common(Packet *ppkt, > - Packet *spkt, > - int poffset, > - int soffset) > +static int colo_compare_packet_payload(Packet *ppkt, > + Packet *spkt, > + uint16_t poffset, > + uint16_t soffset, > + uint16_t len) > { > if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { > char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20]; > @@ -209,17 +210,7 @@ static int colo_packet_compare_common(Packet *ppkt, > sec_ip_src, sec_ip_dst); > } > > - poffset = ppkt->vnet_hdr_len + poffset; > - soffset = ppkt->vnet_hdr_len + soffset; > - > - if (ppkt->size - poffset == spkt->size - soffset) { > - return memcmp(ppkt->data + poffset, > - spkt->data + soffset, > - spkt->size - soffset); > - } else { > - trace_colo_compare_main("Net packet size are not the same"); > - return -1; > - } > + return memcmp(ppkt->data + poffset, spkt->data + soffset, len); > } > > /* > @@ -274,16 +265,23 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) > ptrdiff_t ptcp_offset, stcp_offset; > > ptcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data > - + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; > + + (ptcp->th_off << 2) - ppkt->vnet_hdr_len; > stcp_offset = spkt->transport_header - (uint8_t *)spkt->data > - + (stcp->th_off * 4) - spkt->vnet_hdr_len; > + + (stcp->th_off << 2) - spkt->vnet_hdr_len; > /* > * When network is busy, some tcp options(like sack) will unpredictable > * occur in primary side or secondary side. it will make packet size > * not same, but the two packet's payload is identical. colo just > * care about packet payload, so we skip the option field. > */ > > > In the patch 1，you should remove this comments, it's out of date. Ah, I got it. > > > > - res = colo_packet_compare_common(ppkt, spkt, ptcp_offset, stcp_offset); > + if (ppkt->size - ptcp_offset == spkt->size - stcp_offset) { > + res = colo_compare_packet_payload(ppkt, spkt, > + ptcp_offset, stcp_offset, > + ppkt->size - ptcp_offset); > + } else { > + trace_colo_compare_main("TCP: the size of packets are different"); > > > > Should fix this comments to "TCP: payload size of packets are diffenrent!". OK, I will. Thanks, Mao > > Thanks > Zhang Chen > > > > + res = -1; > + } > > if (res != 0 && > trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { > @@ -325,8 +323,8 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) > */ > static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) > { > - int ret; > - int network_header_length = ppkt->ip->ip_hl * 4; > + uint16_t network_header_length = ppkt->ip->ip_hl << 2; > + uint16_t offset = network_header_length + ETH_HLEN + ppkt->vnet_hdr_len; > > trace_colo_compare_main("compare udp"); > > @@ -340,11 +338,12 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) > * other field like TOS,TTL,IP Checksum. we only need to compare > * the ip payload here. > */ > - ret = colo_packet_compare_common(ppkt, spkt, > - network_header_length + ETH_HLEN, > - network_header_length + ETH_HLEN); > - > - if (ret) { > + if (ppkt->size != spkt->size) { > + trace_colo_compare_main("UDP: the size of packets are different"); > + return -1; > + } > + if (colo_compare_packet_payload(ppkt, spkt, offset, offset, > + ppkt->size - offset)) { > trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size); > trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size); > if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { > @@ -353,9 +352,10 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) > qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt", > spkt->size); > } > + return -1; > + } else { > + return 0; > } > - > - return ret; > } > > /* > @@ -364,7 +364,8 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) > */ > static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) > { > - int network_header_length = ppkt->ip->ip_hl * 4; > + uint16_t network_header_length = ppkt->ip->ip_hl << 2; > + uint16_t offset = network_header_length + ETH_HLEN + ppkt->vnet_hdr_len; > > trace_colo_compare_main("compare icmp"); > > @@ -378,9 +379,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) > * other field like TOS,TTL,IP Checksum. we only need to compare > * the ip payload here. > */ > - if (colo_packet_compare_common(ppkt, spkt, > - network_header_length + ETH_HLEN, > - network_header_length + ETH_HLEN)) { > + if (ppkt->size != spkt->size) { > + trace_colo_compare_main("ICMP: the size of packets are different"); > + return -1; > + } > + if (colo_compare_packet_payload(ppkt, spkt, offset, offset, > + ppkt->size - offset)) { > trace_colo_compare_icmp_miscompare("primary pkt size", > ppkt->size); > trace_colo_compare_icmp_miscompare("Secondary pkt size", > @@ -403,6 +407,8 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) > */ > static int colo_packet_compare_other(Packet *spkt, Packet *ppkt) > { > + uint16_t offset = ppkt->vnet_hdr_len; > + > trace_colo_compare_main("compare other"); > if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { > char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20]; > @@ -417,7 +423,8 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt) > sec_ip_src, sec_ip_dst); > } > > - return colo_packet_compare_common(ppkt, spkt, 0, 0); > + return colo_compare_packet_payload(ppkt, spkt, offset, offset, > + ppkt->size - offset); > } > > static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time) > -- > 2.9.4 > > > >
diff --git a/net/colo-compare.c b/net/colo-compare.c index 0afb5f0..f833eba 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -191,10 +191,11 @@ static int packet_enqueue(CompareState *s, int mode, Connection **con) * return: 0 means packet same * > 0 || < 0 means packet different */ -static int colo_packet_compare_common(Packet *ppkt, - Packet *spkt, - int poffset, - int soffset) +static int colo_compare_packet_payload(Packet *ppkt, + Packet *spkt, + uint16_t poffset, + uint16_t soffset, + uint16_t len) { if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20]; @@ -209,17 +210,7 @@ static int colo_packet_compare_common(Packet *ppkt, sec_ip_src, sec_ip_dst); } - poffset = ppkt->vnet_hdr_len + poffset; - soffset = ppkt->vnet_hdr_len + soffset; - - if (ppkt->size - poffset == spkt->size - soffset) { - return memcmp(ppkt->data + poffset, - spkt->data + soffset, - spkt->size - soffset); - } else { - trace_colo_compare_main("Net packet size are not the same"); - return -1; - } + return memcmp(ppkt->data + poffset, spkt->data + soffset, len); } /* @@ -274,16 +265,23 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) ptrdiff_t ptcp_offset, stcp_offset; ptcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data - + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; + + (ptcp->th_off << 2) - ppkt->vnet_hdr_len; stcp_offset = spkt->transport_header - (uint8_t *)spkt->data - + (stcp->th_off * 4) - spkt->vnet_hdr_len; + + (stcp->th_off << 2) - spkt->vnet_hdr_len; /* * When network is busy, some tcp options(like sack) will unpredictable * occur in primary side or secondary side. it will make packet size * not same, but the two packet's payload is identical. colo just * care about packet payload, so we skip the option field. */ - res = colo_packet_compare_common(ppkt, spkt, ptcp_offset, stcp_offset); + if (ppkt->size - ptcp_offset == spkt->size - stcp_offset) { + res = colo_compare_packet_payload(ppkt, spkt, + ptcp_offset, stcp_offset, + ppkt->size - ptcp_offset); + } else { + trace_colo_compare_main("TCP: the size of packets are different"); + res = -1; + } if (res != 0 && trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { @@ -325,8 +323,8 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) */ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) { - int ret; - int network_header_length = ppkt->ip->ip_hl * 4; + uint16_t network_header_length = ppkt->ip->ip_hl << 2; + uint16_t offset = network_header_length + ETH_HLEN + ppkt->vnet_hdr_len; trace_colo_compare_main("compare udp"); @@ -340,11 +338,12 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) * other field like TOS,TTL,IP Checksum. we only need to compare * the ip payload here. */ - ret = colo_packet_compare_common(ppkt, spkt, - network_header_length + ETH_HLEN, - network_header_length + ETH_HLEN); - - if (ret) { + if (ppkt->size != spkt->size) { + trace_colo_compare_main("UDP: the size of packets are different"); + return -1; + } + if (colo_compare_packet_payload(ppkt, spkt, offset, offset, + ppkt->size - offset)) { trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size); trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size); if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { @@ -353,9 +352,10 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt", spkt->size); } + return -1; + } else { + return 0; } - - return ret; } /* @@ -364,7 +364,8 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) */ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) { - int network_header_length = ppkt->ip->ip_hl * 4; + uint16_t network_header_length = ppkt->ip->ip_hl << 2; + uint16_t offset = network_header_length + ETH_HLEN + ppkt->vnet_hdr_len; trace_colo_compare_main("compare icmp"); @@ -378,9 +379,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) * other field like TOS,TTL,IP Checksum. we only need to compare * the ip payload here. */ - if (colo_packet_compare_common(ppkt, spkt, - network_header_length + ETH_HLEN, - network_header_length + ETH_HLEN)) { + if (ppkt->size != spkt->size) { + trace_colo_compare_main("ICMP: the size of packets are different"); + return -1; + } + if (colo_compare_packet_payload(ppkt, spkt, offset, offset, + ppkt->size - offset)) { trace_colo_compare_icmp_miscompare("primary pkt size", ppkt->size); trace_colo_compare_icmp_miscompare("Secondary pkt size", @@ -403,6 +407,8 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) */ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt) { + uint16_t offset = ppkt->vnet_hdr_len; + trace_colo_compare_main("compare other"); if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) { char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20]; @@ -417,7 +423,8 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt) sec_ip_src, sec_ip_dst); } - return colo_packet_compare_common(ppkt, spkt, 0, 0); + return colo_compare_packet_payload(ppkt, spkt, offset, offset, + ppkt->size - offset); } static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
Modified the function colo_packet_compare_common to prepare for the tcp packet comparison in the next patch. Cc: Zhang Chen <zhangckid@gmail.com> Cc: Li Zhijian <lizhijian@cn.fujitsu.com> Cc: Jason Wang <jasowang@redhat.com> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> --- net/colo-compare.c | 71 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 32 deletions(-)