* [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance @ 2017-08-21 8:55 Zhang Chen 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 1/3] net/colo-compare.c: Optimize unpredictable tcp options comparison Zhang Chen ` (4 more replies) 0 siblings, 5 replies; 12+ messages in thread From: Zhang Chen @ 2017-08-21 8:55 UTC (permalink / raw) To: qemu devel, Jason Wang; +Cc: Zhang Chen, Li Zhijian In this serise, we do a lot of job to optimize COLO net performance. Mainly focus on TCP protocol. V4: - Remove the old patch1. V3: - Rebase on upstream. - Remove origin p2. - Move the "checkpoint_time_ms" to CompareState, in order to aviod multi colo-compare instance conflict. - Add "TODO comments" for reset s->checkpoint_time_ms. - Add a new patch fix comments and scheme. V2: - Rename p2's subject. Zhang Chen (3): net/colo-compare.c: Optimize unpredictable tcp options comparison net/colo-compare.c: Adjust net queue pop order for performance net/colo-compare.c: Fix comments and scheme net/colo-compare.c | 62 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 22 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V4 1/3] net/colo-compare.c: Optimize unpredictable tcp options comparison 2017-08-21 8:55 [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance Zhang Chen @ 2017-08-21 8:55 ` Zhang Chen 2017-09-01 9:35 ` Dou Liyang 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 2/3] net/colo-compare.c: Adjust net queue pop order for performance Zhang Chen ` (3 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Zhang Chen @ 2017-08-21 8:55 UTC (permalink / raw) To: qemu devel, Jason Wang; +Cc: Zhang Chen, Li Zhijian 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. Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> --- net/colo-compare.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index ca67c68..f6bda41 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -186,7 +186,10 @@ 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, int offset) +static int colo_packet_compare_common(Packet *ppkt, + Packet *spkt, + int poffset, + int soffset) { if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20]; @@ -201,12 +204,13 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset) sec_ip_src, sec_ip_dst); } - offset = ppkt->vnet_hdr_len + offset; + poffset = ppkt->vnet_hdr_len + poffset; + soffset = ppkt->vnet_hdr_len + soffset; - if (ppkt->size == spkt->size) { - return memcmp(ppkt->data + offset, - spkt->data + offset, - spkt->size - offset); + if (ppkt->size == spkt->size || poffset != 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; @@ -263,13 +267,22 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) * so we just need skip this field. */ if (ptcp->th_off > 5) { - ptrdiff_t tcp_offset; + ptrdiff_t ptcp_offset, stcp_offset; - tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data - + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; - res = colo_packet_compare_common(ppkt, spkt, tcp_offset); + ptcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data + + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; + stcp_offset = spkt->transport_header - (uint8_t *)spkt->data + + (stcp->th_off * 4) - 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); } else if (ptcp->th_sum == stcp->th_sum) { - res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN, ETH_HLEN); } else { res = -1; } @@ -329,6 +342,7 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) * the ip payload here. */ ret = colo_packet_compare_common(ppkt, spkt, + network_header_length + ETH_HLEN, network_header_length + ETH_HLEN); if (ret) { @@ -366,6 +380,7 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) * the ip payload here. */ if (colo_packet_compare_common(ppkt, spkt, + network_header_length + ETH_HLEN, network_header_length + ETH_HLEN)) { trace_colo_compare_icmp_miscompare("primary pkt size", ppkt->size); @@ -403,7 +418,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt) sec_ip_src, sec_ip_dst); } - return colo_packet_compare_common(ppkt, spkt, 0); + return colo_packet_compare_common(ppkt, spkt, 0, 0); } static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time) -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 1/3] net/colo-compare.c: Optimize unpredictable tcp options comparison 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 1/3] net/colo-compare.c: Optimize unpredictable tcp options comparison Zhang Chen @ 2017-09-01 9:35 ` Dou Liyang 2017-09-01 16:02 ` Zhang Chen 0 siblings, 1 reply; 12+ messages in thread From: Dou Liyang @ 2017-09-01 9:35 UTC (permalink / raw) To: qemu-devel, zhangchen.fnst Cc: jasowang, Li, Zhijian/李 智坚 Hi chen, At 08/21/2017 04:55 PM, Zhang Chen wrote: > 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. > > Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> > --- > net/colo-compare.c | 39 +++++++++++++++++++++++++++------------ > 1 file changed, 27 insertions(+), 12 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index ca67c68..f6bda41 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -186,7 +186,10 @@ 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, int offset) > +static int colo_packet_compare_common(Packet *ppkt, > + Packet *spkt, > + int poffset, > + int soffset) > { > if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { > char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20]; > @@ -201,12 +204,13 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset) > sec_ip_src, sec_ip_dst); > } > > - offset = ppkt->vnet_hdr_len + offset; > + poffset = ppkt->vnet_hdr_len + poffset; > + soffset = ppkt->vnet_hdr_len + soffset; > > - if (ppkt->size == spkt->size) { > - return memcmp(ppkt->data + offset, > - spkt->data + offset, > - spkt->size - offset); > + if (ppkt->size == spkt->size || poffset != soffset) { > + return memcmp(ppkt->data + poffset, > + spkt->data + soffset, > + spkt->size - soffset); Here maybe a mistake, I guess you should make sure (ppkt->size - poffset) == (spkt->size - soffset) is true in case of (poffset != soffset) at the same time. Because, if sack make the header not equal, it is also possible that the data is not equal at the same time. > } else { > trace_colo_compare_main("Net packet size are not the same"); > return -1; > @@ -263,13 +267,22 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) > * so we just need skip this field. > */ > if (ptcp->th_off > 5) { > - ptrdiff_t tcp_offset; > + ptrdiff_t ptcp_offset, stcp_offset; > > - tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data > - + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; > - res = colo_packet_compare_common(ppkt, spkt, tcp_offset); > + ptcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data > + + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; > + stcp_offset = spkt->transport_header - (uint8_t *)spkt->data > + + (stcp->th_off * 4) - 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); we add a new parameter in xxx_common() just for xxx_tcp(). In my opinion, it seems not good. Can we split the tcp related code out from xxx_common, and make the xxx_common function just do like what its name says. > } else if (ptcp->th_sum == stcp->th_sum) { > - res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); > + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN, ETH_HLEN); > } else { > res = -1; > } > @@ -329,6 +342,7 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) > * the ip payload here. > */ > ret = colo_packet_compare_common(ppkt, spkt, > + network_header_length + ETH_HLEN, > network_header_length + ETH_HLEN); ditto > > if (ret) { > @@ -366,6 +380,7 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) > * the ip payload here. > */ > if (colo_packet_compare_common(ppkt, spkt, > + network_header_length + ETH_HLEN, > network_header_length + ETH_HLEN)) { ditto > trace_colo_compare_icmp_miscompare("primary pkt size", > ppkt->size); > @@ -403,7 +418,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt) > sec_ip_src, sec_ip_dst); > } > > - return colo_packet_compare_common(ppkt, spkt, 0); > + return colo_packet_compare_common(ppkt, spkt, 0, 0); ditto Thanks, dou. > } > > static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time) > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 1/3] net/colo-compare.c: Optimize unpredictable tcp options comparison 2017-09-01 9:35 ` Dou Liyang @ 2017-09-01 16:02 ` Zhang Chen 2017-09-04 1:52 ` Dou Liyang 0 siblings, 1 reply; 12+ messages in thread From: Zhang Chen @ 2017-09-01 16:02 UTC (permalink / raw) To: Dou Liyang, qemu-devel, zhangchen.fnst Cc: jasowang, Li, Zhijian/李 智坚, zhangckid On 09/01/2017 05:35 PM, Dou Liyang wrote: > Hi chen, > > At 08/21/2017 04:55 PM, Zhang Chen wrote: >> 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. >> >> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >> --- >> net/colo-compare.c | 39 +++++++++++++++++++++++++++------------ >> 1 file changed, 27 insertions(+), 12 deletions(-) >> >> diff --git a/net/colo-compare.c b/net/colo-compare.c >> index ca67c68..f6bda41 100644 >> --- a/net/colo-compare.c >> +++ b/net/colo-compare.c >> @@ -186,7 +186,10 @@ 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, >> int offset) >> +static int colo_packet_compare_common(Packet *ppkt, >> + Packet *spkt, >> + int poffset, >> + int soffset) >> { >> if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { >> char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], >> sec_ip_dst[20]; >> @@ -201,12 +204,13 @@ static int colo_packet_compare_common(Packet >> *ppkt, Packet *spkt, int offset) >> sec_ip_src, sec_ip_dst); >> } >> >> - offset = ppkt->vnet_hdr_len + offset; >> + poffset = ppkt->vnet_hdr_len + poffset; >> + soffset = ppkt->vnet_hdr_len + soffset; >> >> - if (ppkt->size == spkt->size) { >> - return memcmp(ppkt->data + offset, >> - spkt->data + offset, >> - spkt->size - offset); >> + if (ppkt->size == spkt->size || poffset != soffset) { >> + return memcmp(ppkt->data + poffset, >> + spkt->data + soffset, >> + spkt->size - soffset); > > Here maybe a mistake, > > I guess you should make sure > > (ppkt->size - poffset) == (spkt->size - soffset) is true in case of > (poffset != soffset) at the same time. > > Because, if sack make the header not equal, it is also possible that > the data is not equal at the same time. Good catch! In rare situation will miss a different packet, I will fix it in next version. > >> } else { >> trace_colo_compare_main("Net packet size are not the same"); >> return -1; >> @@ -263,13 +267,22 @@ static int colo_packet_compare_tcp(Packet >> *spkt, Packet *ppkt) >> * so we just need skip this field. >> */ >> if (ptcp->th_off > 5) { >> - ptrdiff_t tcp_offset; >> + ptrdiff_t ptcp_offset, stcp_offset; >> >> - tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data >> - + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; >> - res = colo_packet_compare_common(ppkt, spkt, tcp_offset); >> + ptcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data >> + + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; >> + stcp_offset = spkt->transport_header - (uint8_t *)spkt->data >> + + (stcp->th_off * 4) - 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); > > we add a new parameter in xxx_common() just for xxx_tcp(). > In my opinion, it seems not good. > > Can we split the tcp related code out from xxx_common, and make the > xxx_common function just do like what its name says. We introduce this parameter for all protocol, just tcp is the first user. We should have the ability of compare all kinds of packet. Maybe we must use different offset for other protocol except tcp in the future. Thanks Zhang Chen > > >> } else if (ptcp->th_sum == stcp->th_sum) { >> - res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); >> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN, >> ETH_HLEN); >> } else { >> res = -1; >> } >> @@ -329,6 +342,7 @@ static int colo_packet_compare_udp(Packet *spkt, >> Packet *ppkt) >> * the ip payload here. >> */ >> ret = colo_packet_compare_common(ppkt, spkt, >> + network_header_length + ETH_HLEN, >> network_header_length + ETH_HLEN); > > ditto > >> >> if (ret) { >> @@ -366,6 +380,7 @@ static int colo_packet_compare_icmp(Packet *spkt, >> Packet *ppkt) >> * the ip payload here. >> */ >> if (colo_packet_compare_common(ppkt, spkt, >> + network_header_length + ETH_HLEN, >> network_header_length + ETH_HLEN)) { > > ditto > >> trace_colo_compare_icmp_miscompare("primary pkt size", >> ppkt->size); >> @@ -403,7 +418,7 @@ static int colo_packet_compare_other(Packet >> *spkt, Packet *ppkt) >> sec_ip_src, sec_ip_dst); >> } >> >> - return colo_packet_compare_common(ppkt, spkt, 0); >> + return colo_packet_compare_common(ppkt, spkt, 0, 0); > > ditto > > Thanks, > dou. > >> } >> >> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time) >> > > > -- Thanks Zhang Chen ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 1/3] net/colo-compare.c: Optimize unpredictable tcp options comparison 2017-09-01 16:02 ` Zhang Chen @ 2017-09-04 1:52 ` Dou Liyang 0 siblings, 0 replies; 12+ messages in thread From: Dou Liyang @ 2017-09-04 1:52 UTC (permalink / raw) To: Zhang Chen, qemu-devel, zhangchen.fnst Cc: jasowang, Li, Zhijian/李 智坚 Hi Chen, At 09/02/2017 12:02 AM, Zhang Chen wrote: > > > On 09/01/2017 05:35 PM, Dou Liyang wrote: >> Hi chen, >> >> At 08/21/2017 04:55 PM, Zhang Chen wrote: >>> 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. >>> >>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >>> --- >>> net/colo-compare.c | 39 +++++++++++++++++++++++++++------------ >>> 1 file changed, 27 insertions(+), 12 deletions(-) >>> >>> diff --git a/net/colo-compare.c b/net/colo-compare.c >>> index ca67c68..f6bda41 100644 >>> --- a/net/colo-compare.c >>> +++ b/net/colo-compare.c >>> @@ -186,7 +186,10 @@ 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, >>> int offset) >>> +static int colo_packet_compare_common(Packet *ppkt, >>> + Packet *spkt, >>> + int poffset, >>> + int soffset) >>> { >>> if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { >>> char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], >>> sec_ip_dst[20]; >>> @@ -201,12 +204,13 @@ static int colo_packet_compare_common(Packet >>> *ppkt, Packet *spkt, int offset) >>> sec_ip_src, sec_ip_dst); >>> } >>> >>> - offset = ppkt->vnet_hdr_len + offset; >>> + poffset = ppkt->vnet_hdr_len + poffset; >>> + soffset = ppkt->vnet_hdr_len + soffset; >>> >>> - if (ppkt->size == spkt->size) { >>> - return memcmp(ppkt->data + offset, >>> - spkt->data + offset, >>> - spkt->size - offset); >>> + if (ppkt->size == spkt->size || poffset != soffset) { >>> + return memcmp(ppkt->data + poffset, >>> + spkt->data + soffset, >>> + spkt->size - soffset); >> >> Here maybe a mistake, >> >> I guess you should make sure >> >> (ppkt->size - poffset) == (spkt->size - soffset) is true in case of >> (poffset != soffset) at the same time. >> >> Because, if sack make the header not equal, it is also possible that >> the data is not equal at the same time. > > Good catch! In rare situation will miss a different packet, I will fix > it in next version. > Thank you very much! >> >>> } else { >>> trace_colo_compare_main("Net packet size are not the same"); >>> return -1; >>> @@ -263,13 +267,22 @@ static int colo_packet_compare_tcp(Packet >>> *spkt, Packet *ppkt) >>> * so we just need skip this field. >>> */ >>> if (ptcp->th_off > 5) { >>> - ptrdiff_t tcp_offset; >>> + ptrdiff_t ptcp_offset, stcp_offset; >>> >>> - tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data >>> - + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; >>> - res = colo_packet_compare_common(ppkt, spkt, tcp_offset); >>> + ptcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data >>> + + (ptcp->th_off * 4) - ppkt->vnet_hdr_len; >>> + stcp_offset = spkt->transport_header - (uint8_t *)spkt->data >>> + + (stcp->th_off * 4) - 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); >> >> we add a new parameter in xxx_common() just for xxx_tcp(). >> In my opinion, it seems not good. >> >> Can we split the tcp related code out from xxx_common, and make the >> xxx_common function just do like what its name says. > > We introduce this parameter for all protocol, just tcp is the first user. > We should have the ability of compare all kinds of packet. > Maybe we must use different offset for other protocol except tcp in the > future. > Got it, Thanks, dou. > Thanks > Zhang Chen > >> >> >>> } else if (ptcp->th_sum == stcp->th_sum) { >>> - res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); >>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN, >>> ETH_HLEN); >>> } else { >>> res = -1; >>> } >>> @@ -329,6 +342,7 @@ static int colo_packet_compare_udp(Packet *spkt, >>> Packet *ppkt) >>> * the ip payload here. >>> */ >>> ret = colo_packet_compare_common(ppkt, spkt, >>> + network_header_length + ETH_HLEN, >>> network_header_length + ETH_HLEN); >> >> ditto >> >>> >>> if (ret) { >>> @@ -366,6 +380,7 @@ static int colo_packet_compare_icmp(Packet *spkt, >>> Packet *ppkt) >>> * the ip payload here. >>> */ >>> if (colo_packet_compare_common(ppkt, spkt, >>> + network_header_length + ETH_HLEN, >>> network_header_length + ETH_HLEN)) { >> >> ditto >> >>> trace_colo_compare_icmp_miscompare("primary pkt size", >>> ppkt->size); >>> @@ -403,7 +418,7 @@ static int colo_packet_compare_other(Packet >>> *spkt, Packet *ppkt) >>> sec_ip_src, sec_ip_dst); >>> } >>> >>> - return colo_packet_compare_common(ppkt, spkt, 0); >>> + return colo_packet_compare_common(ppkt, spkt, 0, 0); >> >> ditto >> >> Thanks, >> dou. >> >>> } >>> >>> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time) >>> >> >> >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V4 2/3] net/colo-compare.c: Adjust net queue pop order for performance 2017-08-21 8:55 [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance Zhang Chen 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 1/3] net/colo-compare.c: Optimize unpredictable tcp options comparison Zhang Chen @ 2017-08-21 8:55 ` Zhang Chen 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 3/3] net/colo-compare.c: Fix comments and scheme Zhang Chen ` (2 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: Zhang Chen @ 2017-08-21 8:55 UTC (permalink / raw) To: qemu devel, Jason Wang; +Cc: Zhang Chen, Li Zhijian The packet_enqueue() use g_queue_push_tail() to enqueue net packet, so it is more efficent way use g_queue_pop_head() to get packet for compare. That will improve the success rate of comparison. In my test the performance of ftp put 1000M file will increase 10% Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> --- net/colo-compare.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index f6bda41..a8ccac7 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -483,7 +483,7 @@ static void colo_compare_connection(void *opaque, void *user_data) while (!g_queue_is_empty(&conn->primary_list) && !g_queue_is_empty(&conn->secondary_list)) { - pkt = g_queue_pop_tail(&conn->primary_list); + pkt = g_queue_pop_head(&conn->primary_list); switch (conn->ip_proto) { case IPPROTO_TCP: result = g_queue_find_custom(&conn->secondary_list, @@ -521,7 +521,7 @@ static void colo_compare_connection(void *opaque, void *user_data) * until next comparison. */ trace_colo_compare_main("packet different"); - g_queue_push_tail(&conn->primary_list, pkt); + g_queue_push_head(&conn->primary_list, pkt); /* TODO: colo_notify_checkpoint();*/ break; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V4 3/3] net/colo-compare.c: Fix comments and scheme 2017-08-21 8:55 [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance Zhang Chen 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 1/3] net/colo-compare.c: Optimize unpredictable tcp options comparison Zhang Chen 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 2/3] net/colo-compare.c: Adjust net queue pop order for performance Zhang Chen @ 2017-08-21 8:55 ` Zhang Chen 2017-08-22 7:16 ` [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance no-reply 2017-08-29 5:34 ` Zhang Chen 4 siblings, 0 replies; 12+ messages in thread From: Zhang Chen @ 2017-08-21 8:55 UTC (permalink / raw) To: qemu devel, Jason Wang; +Cc: Zhang Chen, Li Zhijian Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> --- net/colo-compare.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index a8ccac7..e8af8bb 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -44,7 +44,7 @@ + CompareState ++ | | +---------------+ +---------------+ +---------------+ - |conn list +--->conn +--------->conn | + | conn list + - > conn + ------- > conn + -- > ....... +---------------+ +---------------+ +---------------+ | | | | | | +---------------+ +---v----+ +---v----+ +---v----+ +---v----+ @@ -75,14 +75,14 @@ typedef struct CompareState { SocketReadState sec_rs; bool vnet_hdr; - /* connection list: the connections belonged to this NIC could be found - * in this list. - * element type: Connection + /* + * Record the connection that through the NIC + * Element type: Connection */ GQueue conn_list; - /* hashtable to save connection */ + /* Record the connection without repetition */ GHashTable *connection_track_table; - /* compare thread, a thread for each NIC */ + /* This thread just do packet compare job */ QemuThread thread; GMainContext *worker_context; @@ -444,8 +444,11 @@ static int colo_old_packet_check_one_conn(Connection *conn, (GCompareFunc)colo_old_packet_check_one); if (result) { - /* do checkpoint will flush old packet */ - /* TODO: colo_notify_checkpoint();*/ + /* Do checkpoint will flush old packet */ + /* + * TODO: Notify colo frame to do checkpoint. + * colo_compare_inconsistent_notify(); + */ return 0; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance 2017-08-21 8:55 [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance Zhang Chen ` (2 preceding siblings ...) 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 3/3] net/colo-compare.c: Fix comments and scheme Zhang Chen @ 2017-08-22 7:16 ` no-reply 2017-08-29 9:01 ` Jason Wang 2017-08-29 5:34 ` Zhang Chen 4 siblings, 1 reply; 12+ messages in thread From: no-reply @ 2017-08-22 7:16 UTC (permalink / raw) To: zhangchen.fnst; +Cc: famz, qemu-devel, jasowang, lizhijian Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 1503305719-2512-1-git-send-email-zhangchen.fnst@cn.fujitsu.com Subject: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 37052358b5 net/colo-compare.c: Fix comments and scheme 46824a6565 net/colo-compare.c: Adjust net queue pop order for performance acea048383 net/colo-compare.c: Optimize unpredictable tcp options comparison === OUTPUT BEGIN === Checking PATCH 1/3: net/colo-compare.c: Optimize unpredictable tcp options comparison... Checking PATCH 2/3: net/colo-compare.c: Adjust net queue pop order for performance... Checking PATCH 3/3: net/colo-compare.c: Fix comments and scheme... ERROR: space prohibited after that '-' (ctx:WxW) #18: FILE: net/colo-compare.c:47: + | conn list + - > conn + ------- > conn + -- > ....... ^ ERROR: space prohibited after that '-' (ctx:OxW) #18: FILE: net/colo-compare.c:47: + | conn list + - > conn + ------- > conn + -- > ....... ^ ERROR: space required one side of that '--' (ctx:WxW) #18: FILE: net/colo-compare.c:47: + | conn list + - > conn + ------- > conn + -- > ....... ^ total: 3 errors, 0 warnings, 40 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@freelists.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance 2017-08-22 7:16 ` [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance no-reply @ 2017-08-29 9:01 ` Jason Wang 2017-08-29 10:45 ` Fam Zheng 0 siblings, 1 reply; 12+ messages in thread From: Jason Wang @ 2017-08-29 9:01 UTC (permalink / raw) To: qemu-devel, zhangchen.fnst, famz; +Cc: lizhijian On 2017年08月22日 15:16, no-reply@patchew.org wrote: > Hi, > > This series seems to have some coding style problems. See output below for > more information: > > Type: series > Message-id: 1503305719-2512-1-git-send-email-zhangchen.fnst@cn.fujitsu.com > Subject: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance > > === TEST SCRIPT BEGIN === > #!/bin/bash > > BASE=base > n=1 > total=$(git log --oneline $BASE.. | wc -l) > failed=0 > > git config --local diff.renamelimit 0 > git config --local diff.renames True > > commits="$(git log --format=%H --reverse $BASE..)" > for c in $commits; do > echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." > if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then > failed=1 > echo > fi > n=$((n+1)) > done > > exit $failed > === TEST SCRIPT END === > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > Switched to a new branch 'test' > 37052358b5 net/colo-compare.c: Fix comments and scheme > 46824a6565 net/colo-compare.c: Adjust net queue pop order for performance > acea048383 net/colo-compare.c: Optimize unpredictable tcp options comparison > > === OUTPUT BEGIN === > Checking PATCH 1/3: net/colo-compare.c: Optimize unpredictable tcp options comparison... > Checking PATCH 2/3: net/colo-compare.c: Adjust net queue pop order for performance... > Checking PATCH 3/3: net/colo-compare.c: Fix comments and scheme... > ERROR: space prohibited after that '-' (ctx:WxW) > #18: FILE: net/colo-compare.c:47: > + | conn list + - > conn + ------- > conn + -- > ....... > ^ > > ERROR: space prohibited after that '-' (ctx:OxW) > #18: FILE: net/colo-compare.c:47: > + | conn list + - > conn + ------- > conn + -- > ....... > ^ > > ERROR: space required one side of that '--' (ctx:WxW) > #18: FILE: net/colo-compare.c:47: > + | conn list + - > conn + ------- > conn + -- > ....... > ^ > > total: 3 errors, 0 warnings, 40 lines checked > > Your patch has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > === OUTPUT END === > > Test command exited with code: 1 > > > --- > Email generated automatically by Patchew [http://patchew.org/]. > Please send your feedback to patchew-devel@freelists.org Fam, this looks like a false positive since it was actually an ascii graph inside a comment? Thanks ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance 2017-08-29 9:01 ` Jason Wang @ 2017-08-29 10:45 ` Fam Zheng 2017-08-30 1:47 ` Jason Wang 0 siblings, 1 reply; 12+ messages in thread From: Fam Zheng @ 2017-08-29 10:45 UTC (permalink / raw) To: Jason Wang; +Cc: qemu-devel, zhangchen.fnst, lizhijian On Tue, 08/29 17:01, Jason Wang wrote: > > > On 2017年08月22日 15:16, no-reply@patchew.org wrote: > > Hi, > > > > This series seems to have some coding style problems. See output below for > > more information: > > > > Type: series > > Message-id: 1503305719-2512-1-git-send-email-zhangchen.fnst@cn.fujitsu.com > > Subject: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance > > > > === TEST SCRIPT BEGIN === > > #!/bin/bash > > > > BASE=base > > n=1 > > total=$(git log --oneline $BASE.. | wc -l) > > failed=0 > > > > git config --local diff.renamelimit 0 > > git config --local diff.renames True > > > > commits="$(git log --format=%H --reverse $BASE..)" > > for c in $commits; do > > echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." > > if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then > > failed=1 > > echo > > fi > > n=$((n+1)) > > done > > > > exit $failed > > === TEST SCRIPT END === > > > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > > Switched to a new branch 'test' > > 37052358b5 net/colo-compare.c: Fix comments and scheme > > 46824a6565 net/colo-compare.c: Adjust net queue pop order for performance > > acea048383 net/colo-compare.c: Optimize unpredictable tcp options comparison > > > > === OUTPUT BEGIN === > > Checking PATCH 1/3: net/colo-compare.c: Optimize unpredictable tcp options comparison... > > Checking PATCH 2/3: net/colo-compare.c: Adjust net queue pop order for performance... > > Checking PATCH 3/3: net/colo-compare.c: Fix comments and scheme... > > ERROR: space prohibited after that '-' (ctx:WxW) > > #18: FILE: net/colo-compare.c:47: > > + | conn list + - > conn + ------- > conn + -- > ....... > > ^ > > > > ERROR: space prohibited after that '-' (ctx:OxW) > > #18: FILE: net/colo-compare.c:47: > > + | conn list + - > conn + ------- > conn + -- > ....... > > ^ > > > > ERROR: space required one side of that '--' (ctx:WxW) > > #18: FILE: net/colo-compare.c:47: > > + | conn list + - > conn + ------- > conn + -- > ....... > > ^ > > > > total: 3 errors, 0 warnings, 40 lines checked > > > > Your patch has style problems, please review. If any of these errors > > are false positives report them to the maintainer, see > > CHECKPATCH in MAINTAINERS. > > > > === OUTPUT END === > > > > Test command exited with code: 1 > > > > > > --- > > Email generated automatically by Patchew [http://patchew.org/]. > > Please send your feedback to patchew-devel@freelists.org > > Fam, this looks like a false positive since it was actually an ascii graph > inside a comment? > Yes, let's just ignore this report. Fam ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance 2017-08-29 10:45 ` Fam Zheng @ 2017-08-30 1:47 ` Jason Wang 0 siblings, 0 replies; 12+ messages in thread From: Jason Wang @ 2017-08-30 1:47 UTC (permalink / raw) To: Fam Zheng; +Cc: qemu-devel, zhangchen.fnst, lizhijian On 2017年08月29日 18:45, Fam Zheng wrote: > On Tue, 08/29 17:01, Jason Wang wrote: >> >> On 2017年08月22日 15:16, no-reply@patchew.org wrote: >>> Hi, >>> >>> This series seems to have some coding style problems. See output below for >>> more information: >>> >>> Type: series >>> Message-id: 1503305719-2512-1-git-send-email-zhangchen.fnst@cn.fujitsu.com >>> Subject: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance >>> >>> === TEST SCRIPT BEGIN === >>> #!/bin/bash >>> >>> BASE=base >>> n=1 >>> total=$(git log --oneline $BASE.. | wc -l) >>> failed=0 >>> >>> git config --local diff.renamelimit 0 >>> git config --local diff.renames True >>> >>> commits="$(git log --format=%H --reverse $BASE..)" >>> for c in $commits; do >>> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." >>> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then >>> failed=1 >>> echo >>> fi >>> n=$((n+1)) >>> done >>> >>> exit $failed >>> === TEST SCRIPT END === >>> >>> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 >>> Switched to a new branch 'test' >>> 37052358b5 net/colo-compare.c: Fix comments and scheme >>> 46824a6565 net/colo-compare.c: Adjust net queue pop order for performance >>> acea048383 net/colo-compare.c: Optimize unpredictable tcp options comparison >>> >>> === OUTPUT BEGIN === >>> Checking PATCH 1/3: net/colo-compare.c: Optimize unpredictable tcp options comparison... >>> Checking PATCH 2/3: net/colo-compare.c: Adjust net queue pop order for performance... >>> Checking PATCH 3/3: net/colo-compare.c: Fix comments and scheme... >>> ERROR: space prohibited after that '-' (ctx:WxW) >>> #18: FILE: net/colo-compare.c:47: >>> + | conn list + - > conn + ------- > conn + -- > ....... >>> ^ >>> >>> ERROR: space prohibited after that '-' (ctx:OxW) >>> #18: FILE: net/colo-compare.c:47: >>> + | conn list + - > conn + ------- > conn + -- > ....... >>> ^ >>> >>> ERROR: space required one side of that '--' (ctx:WxW) >>> #18: FILE: net/colo-compare.c:47: >>> + | conn list + - > conn + ------- > conn + -- > ....... >>> ^ >>> >>> total: 3 errors, 0 warnings, 40 lines checked >>> >>> Your patch has style problems, please review. If any of these errors >>> are false positives report them to the maintainer, see >>> CHECKPATCH in MAINTAINERS. >>> >>> === OUTPUT END === >>> >>> Test command exited with code: 1 >>> >>> >>> --- >>> Email generated automatically by Patchew [http://patchew.org/]. >>> Please send your feedback to patchew-devel@freelists.org >> Fam, this looks like a false positive since it was actually an ascii graph >> inside a comment? >> > Yes, let's just ignore this report. > > Fam Thanks. I've queued this series. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance 2017-08-21 8:55 [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance Zhang Chen ` (3 preceding siblings ...) 2017-08-22 7:16 ` [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance no-reply @ 2017-08-29 5:34 ` Zhang Chen 4 siblings, 0 replies; 12+ messages in thread From: Zhang Chen @ 2017-08-29 5:34 UTC (permalink / raw) To: qemu devel, Jason Wang; +Cc: zhangchen.fnst, Li Zhijian Hi~ Jason. Have any comments for this series? Thanks Zhang Chen On 08/21/2017 04:55 PM, Zhang Chen wrote: > In this serise, we do a lot of job to optimize COLO net performance. > Mainly focus on TCP protocol. > > V4: > - Remove the old patch1. > > V3: > - Rebase on upstream. > - Remove origin p2. > - Move the "checkpoint_time_ms" to CompareState, > in order to aviod multi colo-compare instance conflict. > - Add "TODO comments" for reset s->checkpoint_time_ms. > - Add a new patch fix comments and scheme. > > V2: > - Rename p2's subject. > > > Zhang Chen (3): > net/colo-compare.c: Optimize unpredictable tcp options comparison > net/colo-compare.c: Adjust net queue pop order for performance > net/colo-compare.c: Fix comments and scheme > > net/colo-compare.c | 62 +++++++++++++++++++++++++++++++++++------------------- > 1 file changed, 40 insertions(+), 22 deletions(-) > -- Thanks Zhang Chen ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-09-04 1:55 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-21 8:55 [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance Zhang Chen 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 1/3] net/colo-compare.c: Optimize unpredictable tcp options comparison Zhang Chen 2017-09-01 9:35 ` Dou Liyang 2017-09-01 16:02 ` Zhang Chen 2017-09-04 1:52 ` Dou Liyang 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 2/3] net/colo-compare.c: Adjust net queue pop order for performance Zhang Chen 2017-08-21 8:55 ` [Qemu-devel] [PATCH V4 3/3] net/colo-compare.c: Fix comments and scheme Zhang Chen 2017-08-22 7:16 ` [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance no-reply 2017-08-29 9:01 ` Jason Wang 2017-08-29 10:45 ` Fam Zheng 2017-08-30 1:47 ` Jason Wang 2017-08-29 5:34 ` Zhang Chen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.