| Message ID | 1492334677-14170-2-git-send-email-zhangchen.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Zhang, On 04/16/2017 06:24 AM, Zhang Chen wrote: > In this patch we support packet that have tcp options field. > Add tcp options field check, If the packet have options > field we just skip it and compare tcp payload, > Avoid unnecessary checkpoint, optimize performance. > > Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> > --- > net/colo-compare.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index aada04e..881d6b2 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -228,6 +228,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) > { > struct tcphdr *ptcp, *stcp; > int res; > + int tcp_offset = 0; No need to declare here, it can be restricted to the if {} body. > > trace_colo_compare_main("compare tcp"); > > @@ -248,7 +249,31 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) > spkt->ip->ip_sum = ppkt->ip->ip_sum; > } > > - if (ptcp->th_sum == stcp->th_sum) { > + /* > + * Check tcp header length for tcp option field. > + * th_off > 5 means this tcp packet have options field. > + * The tcp options maybe always different. > + * for example: > + * From RFC 7323. > + * TCP Timestamps option (TSopt): > + * Kind: 8 > + * > + * Length: 10 bytes > + * > + * +-------+-------+---------------------+---------------------+ > + * |Kind=8 | 10 | TS Value (TSval) |TS Echo Reply (TSecr)| > + * +-------+-------+---------------------+---------------------+ > + * 1 1 4 4 > + * > + * In this case the primary guest's timestamp always different with > + * the secondary guest's timestamp. COLO just focus on payload, > + * so we just need skip this field. > + */ > + if (ptcp->th_off > 5) { You can declare here. I'd rather declare tcp_offset as a ptrdiff_t. > + tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data > + + (ptcp->th_off * 4); > + res = colo_packet_compare_common(ppkt, spkt, tcp_offset); > + } else if (ptcp->th_sum == stcp->th_sum) { > res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); > } else { > res = -1; >
On 04/17/2017 09:43 PM, Philippe Mathieu-Daudé wrote: > Hi Zhang, > > On 04/16/2017 06:24 AM, Zhang Chen wrote: >> In this patch we support packet that have tcp options field. >> Add tcp options field check, If the packet have options >> field we just skip it and compare tcp payload, >> Avoid unnecessary checkpoint, optimize performance. >> >> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >> --- >> net/colo-compare.c | 27 ++++++++++++++++++++++++++- >> 1 file changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/net/colo-compare.c b/net/colo-compare.c >> index aada04e..881d6b2 100644 >> --- a/net/colo-compare.c >> +++ b/net/colo-compare.c >> @@ -228,6 +228,7 @@ static int colo_packet_compare_tcp(Packet *spkt, >> Packet *ppkt) >> { >> struct tcphdr *ptcp, *stcp; >> int res; >> + int tcp_offset = 0; > > No need to declare here, it can be restricted to the if {} body. OK~ > >> >> trace_colo_compare_main("compare tcp"); >> >> @@ -248,7 +249,31 @@ static int colo_packet_compare_tcp(Packet *spkt, >> Packet *ppkt) >> spkt->ip->ip_sum = ppkt->ip->ip_sum; >> } >> >> - if (ptcp->th_sum == stcp->th_sum) { >> + /* >> + * Check tcp header length for tcp option field. >> + * th_off > 5 means this tcp packet have options field. >> + * The tcp options maybe always different. >> + * for example: >> + * From RFC 7323. >> + * TCP Timestamps option (TSopt): >> + * Kind: 8 >> + * >> + * Length: 10 bytes >> + * >> + * +-------+-------+---------------------+---------------------+ >> + * |Kind=8 | 10 | TS Value (TSval) |TS Echo Reply (TSecr)| >> + * +-------+-------+---------------------+---------------------+ >> + * 1 1 4 4 >> + * >> + * In this case the primary guest's timestamp always different with >> + * the secondary guest's timestamp. COLO just focus on payload, >> + * so we just need skip this field. >> + */ >> + if (ptcp->th_off > 5) { > > You can declare here. > > I'd rather declare tcp_offset as a ptrdiff_t. I got your point, will fix it in next version. Thanks Zhang Chen > >> + tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data >> + + (ptcp->th_off * 4); >> + res = colo_packet_compare_common(ppkt, spkt, tcp_offset); >> + } else if (ptcp->th_sum == stcp->th_sum) { >> res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); >> } else { >> res = -1; >> > > > . >
diff --git a/net/colo-compare.c b/net/colo-compare.c index aada04e..881d6b2 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -228,6 +228,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) { struct tcphdr *ptcp, *stcp; int res; + int tcp_offset = 0; trace_colo_compare_main("compare tcp"); @@ -248,7 +249,31 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) spkt->ip->ip_sum = ppkt->ip->ip_sum; } - if (ptcp->th_sum == stcp->th_sum) { + /* + * Check tcp header length for tcp option field. + * th_off > 5 means this tcp packet have options field. + * The tcp options maybe always different. + * for example: + * From RFC 7323. + * TCP Timestamps option (TSopt): + * Kind: 8 + * + * Length: 10 bytes + * + * +-------+-------+---------------------+---------------------+ + * |Kind=8 | 10 | TS Value (TSval) |TS Echo Reply (TSecr)| + * +-------+-------+---------------------+---------------------+ + * 1 1 4 4 + * + * In this case the primary guest's timestamp always different with + * the secondary guest's timestamp. COLO just focus on payload, + * so we just need skip this field. + */ + if (ptcp->th_off > 5) { + tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data + + (ptcp->th_off * 4); + res = colo_packet_compare_common(ppkt, spkt, tcp_offset); + } else if (ptcp->th_sum == stcp->th_sum) { res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); } else { res = -1;
In this patch we support packet that have tcp options field. Add tcp options field check, If the packet have options field we just skip it and compare tcp payload, Avoid unnecessary checkpoint, optimize performance. Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> --- net/colo-compare.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)