| Message ID | 1492482020-29745-2-git-send-email-zhangchen.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2017年04月18日 10:20, 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..049f6f8 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -248,7 +248,32 @@ 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.h Probably a good explanation why we can skip this kind of header. But it does not explain why we can skip all the rest? Thanks > + */ > + if (ptcp->th_off > 5) { > + ptrdiff_t tcp_offset; > + 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/20/2017 02:43 PM, Jason Wang wrote: > > > On 2017年04月18日 10:20, 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..049f6f8 100644 >> --- a/net/colo-compare.c >> +++ b/net/colo-compare.c >> @@ -248,7 +248,32 @@ 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.h > > Probably a good explanation why we can skip this kind of header. But > it does not explain why we can skip all the rest? I found tcp options have many kind number to express different meaning, Here I just give an example for the different options situation, and this field not the COLO-proxy focus on, COLO just concern the payload. Maybe we will optimize in the feature. Currently we want to make COLO full-function running in qemu upstream. Thanks Zhang Chen > > Thanks > >> + */ >> + if (ptcp->th_off > 5) { >> + ptrdiff_t tcp_offset; >> + 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 2017年04月21日 11:48, Zhang Chen wrote: > > > On 04/20/2017 02:43 PM, Jason Wang wrote: >> >> >> On 2017年04月18日 10:20, 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..049f6f8 100644 >>> --- a/net/colo-compare.c >>> +++ b/net/colo-compare.c >>> @@ -248,7 +248,32 @@ 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.h >> >> Probably a good explanation why we can skip this kind of header. But >> it does not explain why we can skip all the rest? > > I found tcp options have many kind number to express different meaning, > Here I just give an example for the different options situation, > and this field not the COLO-proxy focus on, COLO just concern the > payload. > Maybe we will optimize in the feature. Currently we want to make COLO > full-function > running in qemu upstream. I see, but the questions are: - If we see packets with different options (except for the case you explain above), does it mean primary and secondary run out of sync? - What will happen if we compare and ask for synchronization if we find options are not exactly the same? Thanks > > Thanks > Zhang Chen > >> >> Thanks >> >>> + */ >>> + if (ptcp->th_off > 5) { >>> + ptrdiff_t tcp_offset; >>> + 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/21/2017 12:10 PM, Jason Wang wrote: > > > On 2017年04月21日 11:48, Zhang Chen wrote: >> >> >> On 04/20/2017 02:43 PM, Jason Wang wrote: >>> >>> >>> On 2017年04月18日 10:20, 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..049f6f8 100644 >>>> --- a/net/colo-compare.c >>>> +++ b/net/colo-compare.c >>>> @@ -248,7 +248,32 @@ 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.h >>> >>> Probably a good explanation why we can skip this kind of header. But >>> it does not explain why we can skip all the rest? >> >> I found tcp options have many kind number to express different meaning, >> Here I just give an example for the different options situation, >> and this field not the COLO-proxy focus on, COLO just concern the >> payload. >> Maybe we will optimize in the feature. Currently we want to make COLO >> full-function >> running in qemu upstream. > > I see, but the questions are: > > - If we see packets with different options (except for the case you > explain above), does it mean primary and secondary run out of sync? > - What will happen if we compare and ask for synchronization if we > find options are not exactly the same? > The tcp options begin from the first SYN packet, if client connect to guest, the primary guest and secondary guest just give a responses to the client's options, so I think the different options part mostly from guest's running statues(like the timestamp), and if kind number are not same, the payload more likely not same too. finally, after next checkpoint, all things will be same. If guest connect to client, primary guest send a packet and secondary not, It will trigger old packet scan to do checkpoint. Thanks Zhang Chen > Thanks > >> >> Thanks >> Zhang Chen >> >>> >>> Thanks >>> >>>> + */ >>>> + if (ptcp->th_off > 5) { >>>> + ptrdiff_t tcp_offset; >>>> + 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 2017年04月21日 16:22, Zhang Chen wrote: > > > On 04/21/2017 12:10 PM, Jason Wang wrote: >> >> >> On 2017年04月21日 11:48, Zhang Chen wrote: >>> >>> >>> On 04/20/2017 02:43 PM, Jason Wang wrote: >>>> >>>> >>>> On 2017年04月18日 10:20, 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..049f6f8 100644 >>>>> --- a/net/colo-compare.c >>>>> +++ b/net/colo-compare.c >>>>> @@ -248,7 +248,32 @@ 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.h >>>> >>>> Probably a good explanation why we can skip this kind of header. >>>> But it does not explain why we can skip all the rest? >>> >>> I found tcp options have many kind number to express different meaning, >>> Here I just give an example for the different options situation, >>> and this field not the COLO-proxy focus on, COLO just concern the >>> payload. >>> Maybe we will optimize in the feature. Currently we want to make >>> COLO full-function >>> running in qemu upstream. >> >> I see, but the questions are: >> >> - If we see packets with different options (except for the case you >> explain above), does it mean primary and secondary run out of sync? >> - What will happen if we compare and ask for synchronization if we >> find options are not exactly the same? >> > > > The tcp options begin from the first SYN packet, if client connect to > guest, the primary guest and secondary guest > just give a responses to the client's options, so I think the > different options part mostly from guest's running > statues(like the timestamp), and if kind number are not same, the > payload more likely not same too. finally, after > next checkpoint, all things will be same. If guest connect to client, > primary guest send a packet and secondary not, > It will trigger old packet scan to do checkpoint. > > Thanks > Zhang Chen Ok this sounds more or less a question of whether or not we want to be more relaxed. Thanks > > >> Thanks >> >>> >>> Thanks >>> Zhang Chen >>> >>>> >>>> Thanks >>>> >>>>> + */ >>>>> + if (ptcp->th_off > 5) { >>>>> + ptrdiff_t tcp_offset; >>>>> + 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..049f6f8 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -248,7 +248,32 @@ 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) { + ptrdiff_t tcp_offset; + 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(-)