[{"id":1748753,"web_url":"http://patchwork.ozlabs.org/comment/1748753/","msgid":"<60BBDE3D-F44D-4D7B-937D-6E6B1780520E@vmware.com>","list_archive_url":null,"date":"2017-08-16T17:45:48","subject":"Re: [ovs-dev] [PATCH v2] datapath-windows: Update Orig Tuple to use\n\tICMP Type and Code","submitter":{"id":69801,"url":"http://patchwork.ozlabs.org/api/people/69801/","name":"Anand Kumar","email":"kumaranand@vmware.com"},"content":"Hi Shashank,\n\nThanks for reviewing the patch. I will update the commit message and send out a v3 patch.\nFor the other comment, please find my response inline.\n\nThanks,\nAnand Kumar\n\n\nOn 8/16/17, 10:03 AM, \"Shashank Ram\" wrote:\n\n ________________________________________\n From: ovs-dev-bounces@openvswitch.org on behalf of Anand Kumar \n Sent: Tuesday, August 15, 2017 3:23 PM\n To: dev@openvswitch.org\n Subject: [ovs-dev] [PATCH v2] datapath-windows: Update Orig Tuple to use ICMP Type and Code\n \n - Also add in some padding for the ct_endpoint's union.\n >>> Mention in the commit description why this padding is required.\n\n Co-authored-by: Sairam Venugopal \n Signed-off-by: Anand Kumar \n ---\n datapath-windows/ovsext/Conntrack.c | 11 +++++++++--\n datapath-windows/ovsext/Conntrack.h | 6 ++++--\n 2 files changed, 13 insertions(+), 4 deletions(-)\n \n diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c\n index 917ebee..ce8c1c8 100644\n --- a/datapath-windows/ovsext/Conntrack.c\n +++ b/datapath-windows/ovsext/Conntrack.c\n @@ -782,9 +782,16 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,\n \n key->ct.tuple_ipv4.ipv4_src = ctKey->src.addr.ipv4_aligned;\n key->ct.tuple_ipv4.ipv4_dst = ctKey->dst.addr.ipv4_aligned;\n - key->ct.tuple_ipv4.src_port = ctKey->src.port;\n - key->ct.tuple_ipv4.dst_port = ctKey->dst.port;\n key->ct.tuple_ipv4.ipv4_proto = ctKey->nw_proto;\n +\n + /* Orig tuple Port is overloaded to take in ICMP-Type & Code */\n + /* This mimics the behavior in lib/conntrack.c*/\n + key->ct.tuple_ipv4.src_port = ctKey->nw_proto != IPPROTO_ICMP ?\n + ctKey->src.port :\n + htons(ctKey->src.icmp_type);\n + key->ct.tuple_ipv4.dst_port = ctKey->nw_proto != IPPROTO_ICMP ?\n + ctKey->dst.port :\n + htons(ctKey->src.icmp_code);\n }\n \n if (entryCreated && entry) {\n diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath-windows/ovsext/Conntrack.h\n index 04ca299..bca7d90 100644\n --- a/datapath-windows/ovsext/Conntrack.h\n +++ b/datapath-windows/ovsext/Conntrack.h\n @@ -41,14 +41,16 @@ struct ct_addr {\n struct ct_endpoint {\n struct ct_addr addr;\n union {\n - ovs_be16 port;\n + struct {\n + ovs_be16 port;\n + uint16 pad_port;\n >>> It might be worth to add a comment about why the pad is required, so that if the struct changes later the padding is not forgotten about.\n [Anand Kumar] : I believe a comment here is not needed, it evident from the variable name. Can add it if it is really required.\n + };\n struct {\n ovs_be16 icmp_id;\n uint8_t icmp_type;\n uint8_t icmp_code;\n };\n };\n - UINT16 pad;\n };\n \n typedef enum CT_UPDATE_RES {\n --\n 2.9.3.windows.1\n \n _______________________________________________","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"QJk668Me\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=kumaranand@vmware.com; "],"Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xXcFx13PDz9t2v\n\tfor ;\n\tThu, 17 Aug 2017 03:45:55 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id BE032E4E;\n\tWed, 16 Aug 2017 17:45:52 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id E39BACA1\n\tfor ; Wed, 16 Aug 2017 17:45:51 +0000 (UTC)","from NAM02-SN1-obe.outbound.protection.outlook.com\n\t(mail-sn1nam02on0048.outbound.protection.outlook.com [104.47.36.48])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id A9F6141D\n\tfor ; Wed, 16 Aug 2017 17:45:51 +0000 (UTC)","from DM2PR05MB317.namprd05.prod.outlook.com (10.141.103.151) by\n\tDM2PR05MB384.namprd05.prod.outlook.com (10.141.101.153) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.1.1362.12; Wed, 16 Aug 2017 17:45:48 +0000","from DM2PR05MB317.namprd05.prod.outlook.com\n\t([fe80::4469:c218:32a9:6e7e]) by\n\tDM2PR05MB317.namprd05.prod.outlook.com\n\t([fe80::4469:c218:32a9:6e7e%18]) with mapi id 15.01.1320.022;\n\tWed, 16 Aug 2017 17:45:48 +0000"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=onevmw.onmicrosoft.com; s=selector1-vmware-com;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=MJhNOw1BB1nFJ8LvWH64jyYzqLdPMxwdj3CFxgB8M64=;\n\tb=QJk668MeOesgfv248nCkXIeAYsoanBn+k+Dve52EVJaWr5dj3pKZcu1G0zF4bDJbrGged0Xrf7YEwaAnXH4MlvoU8AT3Za0wHmJCC1qe42PnXmGHKiSe+QMHze+/TIFMGmUi/945Ec8rwJrc0B5qKe5WT3CMNJu0krspGWbIy3Q=","From":"Anand Kumar ","To":"Shashank Ram ,\n\t\"dev@openvswitch.org\" ","Thread-Topic":"[ovs-dev] [PATCH v2] datapath-windows: Update Orig Tuple to use\n\tICMP Type and Code","Thread-Index":"AQHTFrd+jQw3TgVfNUmv+jjJjii7XA==","Date":"Wed, 16 Aug 2017 17:45:48 +0000","Message-ID":"<60BBDE3D-F44D-4D7B-937D-6E6B1780520E@vmware.com>","References":"<20170815222337.4444-1-kumaranand@vmware.com>\n\t","In-Reply-To":"","Accept-Language":"en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","authentication-results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=onevmw.onmicrosoft.com\n\theader.i=@onevmw.onmicrosoft.com header.b=\"QJk668Me\"; \n\tdkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=kumaranand@vmware.com; "],"x-originating-ip":"[208.91.1.34]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; DM2PR05MB384;\n\t20:16B+QvZOqcKmMlHqeZB5QaoOKc2zVwUA2X+PDbmjfzpFEYk2glfaIfoq2eWUdCZqKrowRms4eqVmohgFDS/O57lJEhOhF8wdAiRqQeTP5FO7RvErN7gMK1qxoEoH8J7MKNm9Q/xxoq+mRJoWckTfq3n6oB0mUnRcMBJDXrSXr9Y=","x-ms-office365-filtering-correlation-id":"2ec83de8-07dd-4969-0f34-08d4e4cea157","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(300000503095)(300135400095)(2017052603031)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:DM2PR05MB384; ","x-ms-traffictypediagnostic":"DM2PR05MB384:","x-exchange-antispam-report-test":"UriScan:(61668805478150)(216315784871565);","x-microsoft-antispam-prvs":"","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(8121501046)(5005006)(3002001)(93006095)(93001095)(100000703101)(100105400095)(10201501046)(6041248)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123558100)(20161123555025)(20161123564025)(20161123560025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:DM2PR05MB384; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:DM2PR05MB384; ","x-forefront-prvs":"0401647B7F","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(189002)(199003)(377454003)(24454002)(36756003)(83716003)(6436002)(105586002)(2501003)(106356001)(6486002)(2950100002)(2900100001)(229853002)(3846002)(53936002)(102836003)(97736004)(6512007)(6116002)(99286003)(6506006)(5250100002)(86362001)(15650500001)(66066001)(3280700002)(14454004)(8936002)(5660300001)(82746002)(53546010)(478600001)(76176999)(54356999)(68736007)(2906002)(50986999)(6246003)(189998001)(3660700001)(101416001)(25786009)(7736002)(305945005)(33656002)(81156014)(81166006)(8676002);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:DM2PR05MB384;\n\tH:DM2PR05MB317.namprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; MX:1; A:1; LANG:en; ","received-spf":"None (protection.outlook.com: vmware.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-ID":"","MIME-Version":"1.0","X-OriginatorOrg":"vmware.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"16 Aug 2017 17:45:48.4503\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"DM2PR05MB384","Subject":"Re: [ovs-dev] [PATCH v2] datapath-windows: Update Orig Tuple to use\n\tICMP Type and Code","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}}]