| Message ID | 1487993530-30875-4-git-send-email-zhangchen.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 2017/2/25 11:32, 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. > > Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> > --- > net/colo-compare.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index e75f0ae..9853232 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; > @@ -237,8 +238,7 @@ 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)); > + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); > For tcp packets check, why not ignore the ip headers, just like udp packets check ? Besides, here, can we compare the checksum stored in headers of tcp and udp before call colo_packet_compare_common(), which i think will improve the comparing performance. Thanks. Hailiang > if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { > trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), > @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) > return -1; > } > > - 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); > @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) > return -1; > } > > - 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", > @@ -330,7 +344,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) >
On 2017/2/25 11:32, 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. > > Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> > --- > net/colo-compare.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index e75f0ae..9853232 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) { This check is useless, because we have done such checks in every caller. > - 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; > @@ -237,8 +238,7 @@ 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)); > + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); > > if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { > trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), > @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) > return -1; > } > > - 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); > @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) > return -1; > } > > - 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", > @@ -330,7 +344,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) >
On 02/25/2017 02:58 PM, Hailiang Zhang wrote: > On 2017/2/25 11:32, 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. >> >> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >> --- >> net/colo-compare.c | 28 +++++++++++++++++++++------- >> 1 file changed, 21 insertions(+), 7 deletions(-) >> >> diff --git a/net/colo-compare.c b/net/colo-compare.c >> index e75f0ae..9853232 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; >> @@ -237,8 +238,7 @@ 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)); >> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); >> > > For tcp packets check, why not ignore the ip headers, just like udp > packets check ? > Besides, here, can we compare the checksum stored in headers of tcp > and udp > before call colo_packet_compare_common(), which i think will improve > the comparing > performance. That's another way to compare the packet suggested by Dr. David Alan Gilbert, It makes two packets IP header be same firstly, then compare all IP packet. This way can tell people why we ignore IP header explicitly in other packets check like udp. For performance, If we ignore the IP header that will reduce at least 20 byte not to be compared. So, ignore ip header have better comparing performance. Thanks Zhang Chen > > Thanks. > Hailiang > >> if (res != 0 && >> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { >> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), >> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, >> Packet *ppkt) >> return -1; >> } >> >> - 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); >> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet >> *spkt, Packet *ppkt) >> return -1; >> } >> >> - 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", >> @@ -330,7 +344,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) >> > > > > > . >
On 02/25/2017 03:26 PM, Hailiang Zhang wrote: > On 2017/2/25 11:32, 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. >> >> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >> --- >> net/colo-compare.c | 28 +++++++++++++++++++++------- >> 1 file changed, 21 insertions(+), 7 deletions(-) >> >> diff --git a/net/colo-compare.c b/net/colo-compare.c >> index e75f0ae..9853232 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) { > > This check is useless, because we have done such checks in every caller. Oh, I forgot it, not in good condition. I will remove it in next version. Thanks Zhang Chen > >> - 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; >> @@ -237,8 +238,7 @@ 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)); >> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); >> >> if (res != 0 && >> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { >> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), >> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, >> Packet *ppkt) >> return -1; >> } >> >> - 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); >> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet >> *spkt, Packet *ppkt) >> return -1; >> } >> >> - 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", >> @@ -330,7 +344,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) >> > > > > > . >
On 2017/2/27 15:03, Zhang Chen wrote: > > > On 02/25/2017 02:58 PM, Hailiang Zhang wrote: >> On 2017/2/25 11:32, 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. >>> >>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >>> --- >>> net/colo-compare.c | 28 +++++++++++++++++++++------- >>> 1 file changed, 21 insertions(+), 7 deletions(-) >>> >>> diff --git a/net/colo-compare.c b/net/colo-compare.c >>> index e75f0ae..9853232 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; >>> @@ -237,8 +238,7 @@ 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)); >>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); >>> >> >> For tcp packets check, why not ignore the ip headers, just like udp >> packets check ? >> Besides, here, can we compare the checksum stored in headers of tcp >> and udp >> before call colo_packet_compare_common(), which i think will improve >> the comparing >> performance. > > That's another way to compare the packet suggested by Dr. David Alan > Gilbert, > It makes two packets IP header be same firstly, then compare all IP packet. > This way can tell people why we ignore IP header explicitly in other > packets check like udp. > For performance, If we ignore the IP header that will reduce at least 20 > byte not to be compared. > So, ignore ip header have better comparing performance. > OK, here, i think we can re-use the checksum value stored in headers of tcp or udp, comparing it first before comparing the complete payload of tcp or udp packets is another way to improve the performance, it is only 2 bytes. Thanks. > Thanks > Zhang Chen > > >> >> Thanks. >> Hailiang >> >>> if (res != 0 && >>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { >>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), >>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, >>> Packet *ppkt) >>> return -1; >>> } >>> >>> - 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); >>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet >>> *spkt, Packet *ppkt) >>> return -1; >>> } >>> >>> - 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", >>> @@ -330,7 +344,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) >>> >> >> >> >> >> . >> >
On 02/27/2017 03:28 PM, Hailiang Zhang wrote: > On 2017/2/27 15:03, Zhang Chen wrote: >> >> >> On 02/25/2017 02:58 PM, Hailiang Zhang wrote: >>> On 2017/2/25 11:32, 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. >>>> >>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >>>> --- >>>> net/colo-compare.c | 28 +++++++++++++++++++++------- >>>> 1 file changed, 21 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/net/colo-compare.c b/net/colo-compare.c >>>> index e75f0ae..9853232 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; >>>> @@ -237,8 +238,7 @@ 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)); >>>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); >>>> >>> >>> For tcp packets check, why not ignore the ip headers, just like udp >>> packets check ? >>> Besides, here, can we compare the checksum stored in headers of tcp >>> and udp >>> before call colo_packet_compare_common(), which i think will improve >>> the comparing >>> performance. >> >> That's another way to compare the packet suggested by Dr. David Alan >> Gilbert, >> It makes two packets IP header be same firstly, then compare all IP >> packet. >> This way can tell people why we ignore IP header explicitly in other >> packets check like udp. >> For performance, If we ignore the IP header that will reduce at least 20 >> byte not to be compared. >> So, ignore ip header have better comparing performance. >> > > OK, here, i think we can re-use the checksum value stored in headers > of tcp > or udp, comparing it first before comparing the complete payload of > tcp or udp > packets is another way to improve the performance, it is only 2 bytes. No, The IP header's checksum are always different, Because the IP header's ID field are always different. Thanks Zhang Chen > > Thanks. > >> Thanks >> Zhang Chen >> >> >>> >>> Thanks. >>> Hailiang >>> >>>> if (res != 0 && >>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { >>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), >>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, >>>> Packet *ppkt) >>>> return -1; >>>> } >>>> >>>> - 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); >>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet >>>> *spkt, Packet *ppkt) >>>> return -1; >>>> } >>>> >>>> - 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", >>>> @@ -330,7 +344,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) >>>> >>> >>> >>> >>> >>> . >>> >> > > > > . >
On 2017/2/27 15:34, Zhang Chen wrote: > > > On 02/27/2017 03:28 PM, Hailiang Zhang wrote: >> On 2017/2/27 15:03, Zhang Chen wrote: >>> >>> >>> On 02/25/2017 02:58 PM, Hailiang Zhang wrote: >>>> On 2017/2/25 11:32, 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. >>>>> >>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >>>>> --- >>>>> net/colo-compare.c | 28 +++++++++++++++++++++------- >>>>> 1 file changed, 21 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/net/colo-compare.c b/net/colo-compare.c >>>>> index e75f0ae..9853232 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; >>>>> @@ -237,8 +238,7 @@ 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)); >>>>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); >>>>> >>>> >>>> For tcp packets check, why not ignore the ip headers, just like udp >>>> packets check ? >>>> Besides, here, can we compare the checksum stored in headers of tcp >>>> and udp >>>> before call colo_packet_compare_common(), which i think will improve >>>> the comparing >>>> performance. >>> >>> That's another way to compare the packet suggested by Dr. David Alan >>> Gilbert, >>> It makes two packets IP header be same firstly, then compare all IP >>> packet. >>> This way can tell people why we ignore IP header explicitly in other >>> packets check like udp. >>> For performance, If we ignore the IP header that will reduce at least 20 >>> byte not to be compared. >>> So, ignore ip header have better comparing performance. >>> >> >> OK, here, i think we can re-use the checksum value stored in headers >> of tcp >> or udp, comparing it first before comparing the complete payload of >> tcp or udp >> packets is another way to improve the performance, it is only 2 bytes. > > No, The IP header's checksum are always different, Because the IP > header's ID field > are always different. > Not checksum in ip header， i mean checksum in tcp header or udp header. > Thanks > Zhang Chen > >> >> Thanks. >> >>> Thanks >>> Zhang Chen >>> >>> >>>> >>>> Thanks. >>>> Hailiang >>>> >>>>> if (res != 0 && >>>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { >>>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), >>>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, >>>>> Packet *ppkt) >>>>> return -1; >>>>> } >>>>> >>>>> - 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); >>>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet >>>>> *spkt, Packet *ppkt) >>>>> return -1; >>>>> } >>>>> >>>>> - 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", >>>>> @@ -330,7 +344,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) >>>>> >>>> >>>> >>>> >>>> >>>> . >>>> >>> >> >> >> >> . >> >
On 02/27/2017 04:43 PM, Hailiang Zhang wrote: > On 2017/2/27 15:34, Zhang Chen wrote: >> >> >> On 02/27/2017 03:28 PM, Hailiang Zhang wrote: >>> On 2017/2/27 15:03, Zhang Chen wrote: >>>> >>>> >>>> On 02/25/2017 02:58 PM, Hailiang Zhang wrote: >>>>> On 2017/2/25 11:32, 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. >>>>>> >>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >>>>>> --- >>>>>> net/colo-compare.c | 28 +++++++++++++++++++++------- >>>>>> 1 file changed, 21 insertions(+), 7 deletions(-) >>>>>> >>>>>> diff --git a/net/colo-compare.c b/net/colo-compare.c >>>>>> index e75f0ae..9853232 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; >>>>>> @@ -237,8 +238,7 @@ 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)); >>>>>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); >>>>>> >>>>> >>>>> For tcp packets check, why not ignore the ip headers, just like udp >>>>> packets check ? >>>>> Besides, here, can we compare the checksum stored in headers of tcp >>>>> and udp >>>>> before call colo_packet_compare_common(), which i think will improve >>>>> the comparing >>>>> performance. >>>> >>>> That's another way to compare the packet suggested by Dr. David Alan >>>> Gilbert, >>>> It makes two packets IP header be same firstly, then compare all IP >>>> packet. >>>> This way can tell people why we ignore IP header explicitly in other >>>> packets check like udp. >>>> For performance, If we ignore the IP header that will reduce at >>>> least 20 >>>> byte not to be compared. >>>> So, ignore ip header have better comparing performance. >>>> >>> >>> OK, here, i think we can re-use the checksum value stored in headers >>> of tcp >>> or udp, comparing it first before comparing the complete payload of >>> tcp or udp >>> packets is another way to improve the performance, it is only 2 bytes. >> >> No, The IP header's checksum are always different, Because the IP >> header's ID field >> are always different. >> > > Not checksum in ip header， i mean checksum in tcp header or udp header. TCP header can do it, but UDP header's checksum is optional. Thanks Zhang Chen > > >> Thanks >> Zhang Chen >> >>> >>> Thanks. >>> >>>> Thanks >>>> Zhang Chen >>>> >>>> >>>>> >>>>> Thanks. >>>>> Hailiang >>>>> >>>>>> if (res != 0 && >>>>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { >>>>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), >>>>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet >>>>>> *spkt, >>>>>> Packet *ppkt) >>>>>> return -1; >>>>>> } >>>>>> >>>>>> - 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); >>>>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet >>>>>> *spkt, Packet *ppkt) >>>>>> return -1; >>>>>> } >>>>>> >>>>>> - 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", >>>>>> @@ -330,7 +344,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) >>>>>> >>>>> >>>>> >>>>> >>>>> >>>>> . >>>>> >>>> >>> >>> >>> >>> . >>> >> > > > > . >
diff --git a/net/colo-compare.c b/net/colo-compare.c index e75f0ae..9853232 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; @@ -237,8 +238,7 @@ 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)); + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) return -1; } - 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); @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) return -1; } - 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", @@ -330,7 +344,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)
Add offset args for colo_packet_compare_common, optimize colo_packet_compare_icmp() and colo_packet_compare_udp() just compare the IP payload. Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> --- net/colo-compare.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-)