[{"id":1762913,"web_url":"http://patchwork.ozlabs.org/comment/1762913/","msgid":"<6e950754-7be8-1e9e-1fb8-3fffbdff44e2@cn.fujitsu.com>","list_archive_url":null,"date":"2017-09-05T02:11:07","subject":"Re: [Qemu-devel] [PATCH V5 1/3] net/colo-compare.c: Optimize\n\tunpredictable tcp options comparison","submitter":{"id":69629,"url":"http://patchwork.ozlabs.org/api/people/69629/","name":"Dou Liyang","email":"douly.fnst@cn.fujitsu.com"},"content":"Hi Chen,\n\nAt 09/04/2017 02:14 PM, Zhang Chen wrote:\n> When network is busy, some tcp options(like sack) will unpredictable\n> occur in primary side or secondary side. it will make packet size\n> not same, but the two packet's payload is identical. colo just\n> care about packet payload, so we skip the option field.\n>\n> Signed-off-by: Zhang Chen \n> ---\n> net/colo-compare.c | 40 ++++++++++++++++++++++++++++------------\n> 1 file changed, 28 insertions(+), 12 deletions(-)\n>\n> diff --git a/net/colo-compare.c b/net/colo-compare.c\n> index ca67c68..18a9ebf 100644\n> --- a/net/colo-compare.c\n> +++ b/net/colo-compare.c\n> @@ -186,7 +186,10 @@ static int packet_enqueue(CompareState *s, int mode)\n> * return: 0 means packet same\n> * > 0 || < 0 means packet different\n> */\n> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)\n> +static int colo_packet_compare_common(Packet *ppkt,\n> + Packet *spkt,\n> + int poffset,\n> + int soffset)\n> {\n> if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {\n> char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];\n> @@ -201,12 +204,14 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)\n> sec_ip_src, sec_ip_dst);\n> }\n>\n> - offset = ppkt->vnet_hdr_len + offset;\n> + poffset = ppkt->vnet_hdr_len + poffset;\n> + soffset = ppkt->vnet_hdr_len + soffset;\n>\n> - if (ppkt->size == spkt->size) {\n> - return memcmp(ppkt->data + offset,\n> - spkt->data + offset,\n> - spkt->size - offset);\n> + if (ppkt->size == spkt->size ||\n> + ppkt->size - poffset == spkt->size - soffset) {\n\nThis logic has a problem:\n\nppkt->data\n |-----------|---------rangeP---------|\n | \\ |\n poffset ppkt->size\n \\ equal range \\\nspkt->data \\ \\\n |---------------|------rangeS--------|\n | |\n soffset spkt->size\n\nSee the above picture, (ppkt->size == spkt->size) is true,\nif [soffset, spkt->size] == [poffset, poffset+(spkt->size- soffset) is\nalso ture, the code will return 0, but actually, they are not equal.\n\nPlease use following code instead,\n\nif (ppkt->size - poffset == spkt->size - soffset)\n\nI am a new boy in COLO, let's see what we actually want to compare,\nIf I am wrong, please correct me. :-)\n\nppkt->data\n |-----------|---------rangeP---------|\n | |\n poffset ppkt->size\n\nspkt->data\n |----|----------rangeS---------|\n | |\n soffset spkt->size\n\nThe data in rangeP and rangeS is what we want to compare.\nSo, we just need care about the rangeX's size and head pointer,\nnot the whole size.\n\n\nThanks,\n\tdou.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmVbG67q3z9s0Z\n\tfor ;\n\tTue, 5 Sep 2017 12:12:09 +1000 (AEST)","from localhost ([::1]:56395 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp3LT-0002iz-55\n\tfor incoming@patchwork.ozlabs.org; Mon, 04 Sep 2017 22:12:07 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59907)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp3L1-0002iY-Tg\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 22:11:44 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp3Kx-0007aT-0y\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 22:11:39 -0400","from mail.cn.fujitsu.com ([183.91.158.132]:60105\n\thelo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp3Kw-0007VH-Dj\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 22:11:34 -0400","from localhost (HELO cn.fujitsu.com) ([10.167.33.5])\n\tby heian.cn.fujitsu.com with ESMTP; 05 Sep 2017 10:11:13 +0800","from G08CNEXCHPEKD03.g08.fujitsu.local (unknown [10.167.33.85])\n\tby cn.fujitsu.com (Postfix) with ESMTP id F17B94725543;\n\tTue, 5 Sep 2017 10:11:08 +0800 (CST)","from localhost.localdomain (10.167.226.106) by\n\tG08CNEXCHPEKD03.g08.fujitsu.local (10.167.33.89) with Microsoft SMTP\n\tServer (TLS) id 14.3.361.1; Tue, 5 Sep 2017 10:11:08 +0800"],"X-IronPort-AV":"E=Sophos;i=\"5.41,477,1498492800\"; d=\"scan'208\";a=\"25216262\"","To":"Zhang Chen , qemu devel\n\t, Jason Wang ","References":"<1504505688-32455-1-git-send-email-zhangchen.fnst@cn.fujitsu.com>\n\t<1504505688-32455-2-git-send-email-zhangchen.fnst@cn.fujitsu.com>","From":"Dou Liyang ","Message-ID":"<6e950754-7be8-1e9e-1fb8-3fffbdff44e2@cn.fujitsu.com>","Date":"Tue, 5 Sep 2017 10:11:07 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.4.0","MIME-Version":"1.0","In-Reply-To":"<1504505688-32455-2-git-send-email-zhangchen.fnst@cn.fujitsu.com>","Content-Type":"text/plain; charset=\"gbk\"; format=flowed","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.167.226.106]","X-yoursite-MailScanner-ID":"F17B94725543.A95A6","X-yoursite-MailScanner":"Found to be clean","X-yoursite-MailScanner-From":"douly.fnst@cn.fujitsu.com","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"183.91.158.132","Subject":"Re: [Qemu-devel] [PATCH V5 1/3] net/colo-compare.c: Optimize\n\tunpredictable tcp options comparison","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Li Zhijian , Zhang Chen ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1762988,"web_url":"http://patchwork.ozlabs.org/comment/1762988/","msgid":"<0045d465-8e1e-448c-a634-9b85372c14e3@cn.fujitsu.com>","list_archive_url":null,"date":"2017-09-05T06:22:19","subject":"Re: [Qemu-devel] [PATCH V5 1/3] net/colo-compare.c: Optimize\n\tunpredictable tcp options comparison","submitter":{"id":67759,"url":"http://patchwork.ozlabs.org/api/people/67759/","name":"Zhang Chen","email":"zhangchen.fnst@cn.fujitsu.com"},"content":"On 09/05/2017 10:11 AM, Dou Liyang wrote:\n> Hi Chen,\n>\n> At 09/04/2017 02:14 PM, Zhang Chen wrote:\n>> When network is busy, some tcp options(like sack) will unpredictable\n>> occur in primary side or secondary side. it will make packet size\n>> not same, but the two packet's payload is identical. colo just\n>> care about packet payload, so we skip the option field.\n>>\n>> Signed-off-by: Zhang Chen \n>> ---\n>> net/colo-compare.c | 40 ++++++++++++++++++++++++++++------------\n>> 1 file changed, 28 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/net/colo-compare.c b/net/colo-compare.c\n>> index ca67c68..18a9ebf 100644\n>> --- a/net/colo-compare.c\n>> +++ b/net/colo-compare.c\n>> @@ -186,7 +186,10 @@ static int packet_enqueue(CompareState *s, int \n>> mode)\n>> * return: 0 means packet same\n>> * > 0 || < 0 means packet different\n>> */\n>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, \n>> int offset)\n>> +static int colo_packet_compare_common(Packet *ppkt,\n>> + Packet *spkt,\n>> + int poffset,\n>> + int soffset)\n>> {\n>> if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {\n>> char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], \n>> sec_ip_dst[20];\n>> @@ -201,12 +204,14 @@ static int colo_packet_compare_common(Packet \n>> *ppkt, Packet *spkt, int offset)\n>> sec_ip_src, sec_ip_dst);\n>> }\n>>\n>> - offset = ppkt->vnet_hdr_len + offset;\n>> + poffset = ppkt->vnet_hdr_len + poffset;\n>> + soffset = ppkt->vnet_hdr_len + soffset;\n>>\n>> - if (ppkt->size == spkt->size) {\n>> - return memcmp(ppkt->data + offset,\n>> - spkt->data + offset,\n>> - spkt->size - offset);\n>> + if (ppkt->size == spkt->size ||\n>> + ppkt->size - poffset == spkt->size - soffset) {\n>\n> This logic has a problem:\n>\n> ppkt->data\n> |-----------|---------rangeP---------|\n> | \\ |\n> poffset ppkt->size\n> \\ equal range \\\n> spkt->data \\ \\\n> |---------------|------rangeS--------|\n> | |\n> soffset spkt->size\n>\n> See the above picture, (ppkt->size == spkt->size) is true,\n> if [soffset, spkt->size] == [poffset, poffset+(spkt->size- soffset) is\n> also ture, the code will return 0, but actually, they are not equal.\n>\n> Please use following code instead,\n>\n> if (ppkt->size - poffset == spkt->size - soffset)\n>\n> I am a new boy in COLO, let's see what we actually want to compare,\n> If I am wrong, please correct me. :-)\n>\n> ppkt->data\n> |-----------|---------rangeP---------|\n> | |\n> poffset ppkt->size\n>\n> spkt->data\n> |----|----------rangeS---------|\n> | |\n> soffset spkt->size\n>\n> The data in rangeP and rangeS is what we want to compare.\n> So, we just need care about the rangeX's size and head pointer,\n> not the whole size.\n\nYes, I have already considered this rare situation, but for packet \ncomparing efficiency\nI ignored it. As you know, every packet will be compared in COLO FT, \nmost of packet\nhave the same offset, in this time use pkt->size is ok. the other packet \nwith different\noffset always have a different pkt->size. But we need cover all \nsituation firstly,\nI will update the V6 later.\n\nThanks\nZhang Chen\n\n>\n>\n> Thanks,\n> dou.\n> .\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmc741Djgz9s83\n\tfor ;\n\tTue, 5 Sep 2017 16:21:33 +1000 (AEST)","from localhost ([::1]:57049 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp7Em-0000zh-Dv\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 02:21:28 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:54832)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp7EK-0000z9-P2\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 02:21:05 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp7EF-0007Ep-SG\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 02:21:00 -0400","from mail.cn.fujitsu.com ([183.91.158.132]:38772\n\thelo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp7EF-00079A-0m\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 02:20:55 -0400","from localhost (HELO cn.fujitsu.com) ([10.167.33.5])\n\tby heian.cn.fujitsu.com with ESMTP; 05 Sep 2017 14:20:50 +0800","from G08CNEXCHPEKD02.g08.fujitsu.local (unknown [10.167.33.83])\n\tby cn.fujitsu.com (Postfix) with ESMTP id 21889472554B;\n\tTue, 5 Sep 2017 14:20:51 +0800 (CST)","from [10.167.226.56] (10.167.226.56) by\n\tG08CNEXCHPEKD02.g08.fujitsu.local (10.167.33.89) with Microsoft SMTP\n\tServer (TLS) id 14.3.361.1; Tue, 5 Sep 2017 14:20:50 +0800"],"X-IronPort-AV":"E=Sophos;i=\"5.41,478,1498492800\"; d=\"scan'208\";a=\"25224372\"","From":"Zhang Chen ","To":"Dou Liyang , qemu devel\n\t, Jason Wang ","References":"<1504505688-32455-1-git-send-email-zhangchen.fnst@cn.fujitsu.com>\n\t<1504505688-32455-2-git-send-email-zhangchen.fnst@cn.fujitsu.com>\n\t<6e950754-7be8-1e9e-1fb8-3fffbdff44e2@cn.fujitsu.com>","Message-ID":"<0045d465-8e1e-448c-a634-9b85372c14e3@cn.fujitsu.com>","Date":"Tue, 5 Sep 2017 14:22:19 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<6e950754-7be8-1e9e-1fb8-3fffbdff44e2@cn.fujitsu.com>","Content-Type":"text/plain; charset=\"utf-8\"; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.167.226.56]","X-yoursite-MailScanner-ID":"21889472554B.A5A61","X-yoursite-MailScanner":"Found to be clean","X-yoursite-MailScanner-From":"zhangchen.fnst@cn.fujitsu.com","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"183.91.158.132","Subject":"Re: [Qemu-devel] [PATCH V5 1/3] net/colo-compare.c: Optimize\n\tunpredictable tcp options comparison","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Li Zhijian , zhangchen.fnst@cn.fujitsu.com,\n\tZhang Chen ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]