From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5619AA04C8; Fri, 18 Sep 2020 16:21:57 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9EDAD1DAB0; Fri, 18 Sep 2020 16:21:56 +0200 (CEST) Received: from nat-hk.nvidia.com (nat-hk.nvidia.com [203.18.50.4]) by dpdk.org (Postfix) with ESMTP id E503A1DAAF for ; Fri, 18 Sep 2020 16:21:53 +0200 (CEST) Received: from HKMAIL102.nvidia.com (Not Verified[10.18.92.9]) by nat-hk.nvidia.com (using TLS: TLSv1.2, AES256-SHA) id ; Fri, 18 Sep 2020 22:21:52 +0800 Received: from HKMAIL102.nvidia.com (10.18.16.11) by HKMAIL102.nvidia.com (10.18.16.11) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 18 Sep 2020 14:21:49 +0000 Received: from NAM04-BN3-obe.outbound.protection.outlook.com (104.47.46.56) by HKMAIL102.nvidia.com (10.18.16.11) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Fri, 18 Sep 2020 14:21:49 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VP/OM5vwx6iXq6b+2WlSCYoQ2QgVmOpI2p24vRPxaXAsNG3ZohpPFc90UivDB97VKgwF7SNTF4ei5e7aWhNg/3PNUsVvzc76jJKiYZqvya/QH+rPkb9eqQFSy43jIqQDuyLnhB/qJgxwjAYoEsTh1nzwpfZPjapwblgaYG90iUA7+118H6A10PxIVk52XncTE1V2aIqb+Q4DH+V0/0C9F4sBjd0ihPPFddBOHSSl7Vw4TH90chfJQ2CYxzt48pMwAT+SqNp5xx2i/rqWSY3k/tnKBFkseRl3b+ZSwTXFbSnzfe2aCq+Buj5Mfg/7/FDx1SSyhWITijXvUE/07KOeiA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Z0dIwgtmEG1k3w0Up1shK0FXgqL1r5zP77J1hlbqWcw=; b=ZIa8Ib7YJMm3HSyPZ7dCpzRZikd36njlpbL4bY7GuU9lTADmbg63188gble6D+qZJzjZtYEcwPgiqWLPwuDkhGZGql5M8SUmhqzPUwEYgSJ71KGmTR4vuU1F0Vur2TE3L+Hg7NJy6hiWrZ94TSUVaR1cizO4UBiewatF3OyiIJAYj4hU30kNB5N7Dt/NArxJ3uK5KdnT9uK5Rvdz/YHmVaY+zpdcICICH8kqH3/13i5bl5w+upZABUGkXB2px993f1xitvQ+6Pg5RA3JxxU55+1vEJYDT4AittWQoQDxjCuSuIlvVi+u1A593R+c416MZhcrO6DE8rjtsHRELJgR7w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none Received: from DM5PR12MB1161.namprd12.prod.outlook.com (2603:10b6:3:73::16) by DM6PR12MB4337.namprd12.prod.outlook.com (2603:10b6:5:2a9::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3370.16; Fri, 18 Sep 2020 14:21:46 +0000 Received: from DM5PR12MB1161.namprd12.prod.outlook.com ([fe80::b0b9:2a96:7ad7:9a93]) by DM5PR12MB1161.namprd12.prod.outlook.com ([fe80::b0b9:2a96:7ad7:9a93%8]) with mapi id 15.20.3391.017; Fri, 18 Sep 2020 14:21:46 +0000 From: Ophir Munk To: Olivier Matz CC: "dev@dpdk.org" , Wenzhuo Lu , "Beilei Xing" , Bernard Iremonger , Ferruh Yigit , "Ophir Munk" Thread-Topic: [dpdk-dev] [PATCH v4 1/3] app/testpmd: add GENEVE parsing Thread-Index: AQHWi2KR8ZV7zBuaNUCLn137I85f7qlsw9sAgAGj8fA= Date: Fri, 18 Sep 2020 14:21:46 +0000 Message-ID: References: <20200915125319.16568-2-ophirmu@nvidia.com> <20200915131717.18252-1-ophirmu@nvidia.com> <20200915131717.18252-2-ophirmu@nvidia.com> <20200917122314.GQ21395@platinum> In-Reply-To: <20200917122314.GQ21395@platinum> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: 6wind.com; dkim=none (message not signed) header.d=none;6wind.com; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [84.229.96.120] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 1854adc9-225a-4534-ddd2-08d85bde2cd8 x-ms-traffictypediagnostic: DM6PR12MB4337: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: WMmNMSPccTi9+v0tAUVKTMl6htRO1+ns/6wikNFYwrk8+ofFtq/aumREqMJJuaoP9RFVYfAlLXg6DtjOigb9USGbU3w7xk4VHowrL4dUGX9zZ0RIhSZmZvZLgMS3OH+QUn8h/PYt5RizPaStsNkOQRKzo27zKZBpBNeTjQ82lOm9xiGexCDOxMHSZ1bu+gRaExDI1zCb6k+NgDLEUODbZyuPryUpVyOd5xDTz1e7KOvvtMdxObJjq9LVVDI55sYYTJZGj2QG1d5ce/DgtDRZ6mi4Vq9c0zKg50nCe4m+2IV2ZNqc8IyTKWRgsZi2BlLxMRjKaiARSFtgG1Rcl7A0qyVENlohHTs77iSxytUa6xk63skHsOYlUfiy2AToOQ+ios/9sXcaTz9VmK3zXd2Mew== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM5PR12MB1161.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(346002)(376002)(366004)(396003)(136003)(39860400002)(6916009)(55016002)(7696005)(8676002)(9686003)(52536014)(53546011)(83080400001)(54906003)(8936002)(966005)(478600001)(107886003)(6506007)(4326008)(186003)(26005)(86362001)(71200400001)(33656002)(66446008)(2906002)(5660300002)(45080400002)(66574015)(316002)(66946007)(66556008)(83380400001)(66476007)(64756008)(76116006); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: S2p+eTZ0gjOVnjawrgwmajDY6+VUHzn3eQE+OIlwoU7Gvj6x2j7dQUMhe5wDYK+N9DMMJaOFbJGwMfhDW/1Kp4l/yxeVV8B3g6e7NlTE2DA2TVdieDvmcs74LUTDJczP7r1wqyzzQ9hsX1PGnH3QSWBfsTI4z2QjfWC0ME9UrPQD2mk/uBAROnUxdxT05dYzfW0wt+1umUArdL3TGAbdBNdaVkjr8eNCiKqEWtYJz1h7dBvBTUp0i7GH7WrHuaH6sw4QOH5oNKwuqsepiaNRVoOu//xA8SzFrYE4bz9/hWt3pP4NfqJHPW4J8p947TKvcgNNCjC0hnAbkeX5EhI1xBgBqqNAqh37q/dg+vNqzLiCAbpofqPu7lmMDgA1lLRNGzhEWdDCZaMa3cyR68gupaOd3w6KTW4kiXnmeikiaEuqIsJPCClOdFnu57RdJsubS5Jdue8PVdrOHX+FzVOnk+9VKQYiix6OPduuib/UanJKLduz5PPd9F4ca5iuc5z7wD47vTWpOnP1tw4mwsIBonc1h2ujuXtxe924rozugaMzRKPEXFKcwgQ3h6XE7z7OwK1jx4tWYabx80hnolFnACN/9Hsty8L0CgFB7WpecDq1/97cYn4YEtOcKYkUV5SnvcAm8+pjr2FPaEAA/hniUg== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1161.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1854adc9-225a-4534-ddd2-08d85bde2cd8 X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Sep 2020 14:21:46.4033 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: demj6hcHnEHk5PpE3DfC8nElzE2phNnqwhLEyQk4RL9h/IvaI4m0E7dEyaQJrYblvucpAvsoTcZqfzPNSLi0og== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4337 X-OriginatorOrg: Nvidia.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1600438912; bh=Z0dIwgtmEG1k3w0Up1shK0FXgqL1r5zP77J1hlbqWcw=; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:From:To: CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References: In-Reply-To:Accept-Language:Content-Language:X-MS-Has-Attach: X-MS-TNEF-Correlator:authentication-results:x-originating-ip: x-ms-publictraffictype:x-ms-office365-filtering-correlation-id: x-ms-traffictypediagnostic:x-ms-exchange-transport-forked: x-microsoft-antispam-prvs:x-ms-oob-tlc-oobclassifiers: x-ms-exchange-senderadcheck:x-microsoft-antispam: x-microsoft-antispam-message-info:x-forefront-antispam-report: x-ms-exchange-antispam-messagedata:Content-Type: Content-Transfer-Encoding:MIME-Version: X-MS-Exchange-CrossTenant-AuthAs: X-MS-Exchange-CrossTenant-AuthSource: X-MS-Exchange-CrossTenant-Network-Message-Id: X-MS-Exchange-CrossTenant-originalarrivaltime: X-MS-Exchange-CrossTenant-fromentityheader: X-MS-Exchange-CrossTenant-id:X-MS-Exchange-CrossTenant-mailboxtype: X-MS-Exchange-CrossTenant-userprincipalname: X-MS-Exchange-Transport-CrossTenantHeadersStamped:X-OriginatorOrg; b=DbcwhceZfFFir+ZPfzYrJ0jqYqA2CByuciTvpgo96hsHUtvqB5XvjknyFDTdOIreW prkY8NwwWu5kiChmQAjEMJ0BJukzkkvjiwJT98qMMP38KWDEASfLvl+X+zX83ADVyv P4Bcng11vE+w53pPiKqzn0qyrWgVfI7mJMOrHQzZyaLG0OOlsza80+A9m8nPHWvJH3 FJ/oen6yl4F0IDSRXxBbwOVZFA+xBFytcopstDEqC70KwZ89Y3qF+W7Nms6pVeLtHA QR4Sw4/FRVoFIILZqdZBYquXuc5rm9SIxHOadmc9AkxaO94Po2RaIMqzFJohFeCu6x dgvhuX4ivqzhw== Subject: Re: [dpdk-dev] [PATCH v4 1/3] app/testpmd: add GENEVE parsing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Olivier, Please find comments inline. I sent v5 based on your review. http://patches.dpdk.org/project/dpdk/list/?series=3D12354 > -----Original Message----- > From: Olivier Matz > Sent: Thursday, September 17, 2020 3:23 PM > To: Ophir Munk > Cc: dev@dpdk.org; Wenzhuo Lu ; Beilei Xing > ; Bernard Iremonger > ; Ferruh Yigit ; > Ophir Munk > Subject: Re: [dpdk-dev] [PATCH v4 1/3] app/testpmd: add GENEVE parsing >=20 > Hi Ophir, >=20 > Please find some comments below. >=20 > On Tue, Sep 15, 2020 at 01:17:15PM +0000, Ophir Munk wrote: > > From: Ophir Munk > > > > GENEVE is a widely used tunneling protocol in modern Virtualized > > Networks. testpmd already supports parsing of several tunneling > > protocols including VXLAN, VXLAN-GPE, GRE. This commit adds GENEVE > > parsing of inner protocols (IPv4-0x0800, IPv6-0x86dd, Ethernet-0x6558) > > based on IETF draft-ietf-nvo3-geneve-09. GENEVE is considered more > > flexible than the other protocols. In terms of protocol format GENEVE > > header has a variable length options as opposed to other tunneling > > protocols which have a fixed header size. > > > > Signed-off-by: Ophir Munk >=20 >=20 > > app/test-pmd/csumonly.c | 70 > ++++++++++++++++++++++++++++++++++++++++++- > > app/test-pmd/testpmd.h | 1 + > > lib/librte_net/meson.build | 3 +- > > lib/librte_net/rte_geneve.h | 72 > > +++++++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 144 insertions(+), 2 deletions(-) create mode > > 100644 lib/librte_net/rte_geneve.h >=20 > An entry could be added in doc/api/doxy-api-index.md. Some more protocols > are missing, I'll send a patch to add them. >=20 Thanks for sending a patch. I would appreciate it if it includes Geneve as = well. > > --- /dev/null > > +++ b/lib/librte_net/rte_geneve.h > > @@ -0,0 +1,72 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright 2020 Mellanox Technologies, Ltd */ > > + > > +#ifndef _RTE_GENEVE_H_ > > +#define _RTE_GENEVE_H_ > > + > > +/** > > + * @file > > + * > > + * GENEVE-related definitions > > + */ > > + > > +#include > > + > > +#include >=20 > Is this include needed? Maybe it comes from a copy/paste of VXLAN? >=20 The include was dropped in v5 (not needed). > > + > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +/** GENEVE default port. */ > > +#define RTE_GENEVE_DEFAULT_PORT 6081 > > + > > +/** > > + * GENEVE protocol header. (draft-ietf-nvo3-geneve-09) > > + * Contains: > > + * 2-bits version (must be 0). > > + * 6-bits option length in four byte multiples, not including the eigh= t > > + * bytes of the fixed tunnel header. > > + * 1-bit control packet. > > + * 1-bit critical options in packet. > > + * 6-bits reserved > > + * 16-bits Protocol Type. The protocol data unit after the Geneve head= er > > + * following the EtherType convention. Ethernet itself is represented = by > > + * the value 0x6558. > > + * 24-bits Virtual Network Identifier (VNI). Virtual network unique > identified. > > + * 8-bits reserved bits (must be 0 on transmission and ignored on rece= ipt). > > + * More-bits (optional) variable length options. > > + */ > > +__extension__ > > +struct rte_geneve_hdr { > > +#if RTE_BYTE_ORDER =3D=3D RTE_BIG_ENDIAN > > + uint8_t ver:2; /**< Version (2). */ >=20 > Isn't the (2) in the comment redundant with the :2 in the type? > Here is how bitfield look like in doxygen documentation: > https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fdoc. > dpdk.org%2Fapi%2Fstructrte__flow__attr.html%23ae4d19341d5298a2bc61f > 9eb941b1179c&data=3D02%7C01%7Cophirmu%40nvidia.com%7C95918f4 > 2491c4832d8a308d85b047a22%7C43083d15727340c1b7db39efd9ccc17a%7 > C0%7C0%7C637359422086641153&sdata=3DLJjYkeHy9eHGc9xu42E%2F8 > h54dzxrmiO7V0NCeLXndpU%3D&reserved=3D0 >=20 > It's true that the field documentation miss the number of bits. > So if you feel it's needed, I prefer something more explicit like "(2 bit= s)" > instead of just "(2)". >=20 The number of bits is removed from the comments (v5). > By the way, there are 2 spaces at the end of the comment. >=20 Extra spaces removed. > > + uint8_t opt_len:6; /**< Options length (6). */ > > + uint8_t oam:1; /**< Control packet (1). */ > > + uint8_t critical:1; /**< Critical packet (1). */ > > + uint8_t rsvd1:6; /**< Reserved (6). */ >=20 > "reserved" instead of "rsvd"? >=20 In v5 all "rsvd" fields are renamed as "reserved". > The Internet-Draft says "Rsvd" for this one, but "Reserved" for the other= . >=20 > > +#else > > + uint8_t opt_len:6; /**< Options length (6). */ > > + uint8_t ver:2; /**< Version (2). */ > > + uint8_t rsvd1:6; /**< Reserved (6). */ > > + uint8_t critical:1; /**< Critical packet (1). */ > > + uint8_t oam:1; /**< Control packet (1). */ > > +#endif > > + rte_be16_t proto; /**< Protocol type (16). */ > > + uint8_t vni[3]; /**< Virtual network identifier (24). */ > > + uint8_t rsvd2; /**< Reserved (8). */ >=20 > vni is an identifier, so I wonder if it would make sense to have it as an= integer > instead of an array of uint8. Something like this: >=20 > #if RTE_BYTE_ORDER =3D=3D RTE_BIG_ENDIAN > uint32_t vni:24; > uint32_t reserved2:8; > #else > uint32_t vni:24; > uint32_t reserved2:8; > #endif >=20 I suggest leaving the vni as is (array of unsigned chars) since I do not se= e a gain by changing it to an integer of 24 bits. If we did change - it will not turn VNI from network order to host order. S= o in case you wanted to print the VNI (host order) you will have to manipul= ate the bytes order anyway based on endianness.=20 What do you think? > > + uint8_t opts[]; /**=20 > Since the option length is a multiple of four-bytes, would uint32_t[] mak= e > more sense here? >=20 opts[] type changed to uint32_t in v5. > Missing a space after "<". >=20 Space added. > > +} __rte_packed; > > + > > +/* GENEVE next protocol types */ > > +#define RTE_GENEVE_TYPE_IPV4 0x0800 /**< IPv4 Protocol. > */ > > +#define RTE_GENEVE_TYPE_IPV6 0x86dd /**< IPv6 Protocol. > */ > > +#define RTE_GENEVE_TYPE_ETH 0x6558 /**< Ethernet > Protocol. */ >=20 > From what I understand in the draft, I think only RTE_GENEVE_TYPE_ETH is > needed. >=20 > Protocol Type (16 bits): The type of the protocol data unit > appearing after the Geneve header. This follows the EtherType > [ETYPES] convention; with Ethernet itself being represented by the > value 0x6558. >=20 > Shouldn't we use RTE_ETHER_TYPE_* instead of redefining them here? >=20 > 0x6558 is RTE_ETHER_TYPE_TEB (Transparent Ethernet Bridging) and is also > used in case of NVGRE. >=20 >=20 Definitions RTE_GENEVE_TYPE_IPV4 and RTE_GENEVE_TYPE_IPV6 dropped (remainin= g with RTE_GENEVE_TYPE_ETH only). > Regards, > Olivier Regards, Ophir