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 2A5F0A0512; Wed, 3 Jun 2020 18:14:17 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6071F1D580; Wed, 3 Jun 2020 18:14:16 +0200 (CEST) Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2052.outbound.protection.outlook.com [40.107.21.52]) by dpdk.org (Postfix) with ESMTP id 4D6AE1D55F; Wed, 3 Jun 2020 18:14:15 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BRhYBwlCy7gyRdDHqLDgILMcSvvQ9aWJPNmK10rX2BNkJrYjM8juIhQ+sTtb4/b3/TT7bGvwlz8M3gACs0ApAyABNbqsNGGZFbioOKgeXGfKVOXtQmW3abPr1NztcFSjI1eLHJh3wCtGTreAQuW8ZCQzyQE86qtilxeHegCag2IaxCxMCpKwkd1sSiJqL6tXSMh2irkM3BOBxH53w1dlrpAPeZ7vpDrWP83X2IZjdqcdDkjz1wWWwYB0cs8/D4uG7PDTdxAfeKKXfWQZShts9W+Syz2lPLsdepcJaakYjLrTTYarGfUqFjGpYZwFmtgkeeosluVqFLZO1EIcijTCFw== 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=C6gnhTa+YR+GVG24fX9+2GYilD+1ITuWWUSngk71798=; b=PYqv8UGcRmzgIRnR3R+FxS3gzt0OAN9lvVKpMHyt1hmjuhekdg+AHjEfc5iQpIkZQvsCxrK3pvWT0dhcYyQ25Dc829S32sIGjdo0naOT3sFri0GKPdUZVeGG6SwFQIFLFLzGTREJ1o7zenkJxUnJtn90/BT6PB+MnroNeSl3URFn3zyMVTR/XYjp4IMaDegWPEqH6k4L+z1QvsulyprgM3JgfRKHHJfojz1OoKdrwaObe5BuBZAWkBJrvAr6Z+REinCADNRr16imQDFFikSTXutjPHYe8p4jozPD17tQMtsz2QwlEcOIDaniWV39bKqx2X8z/svCLt5aPozaFIcdhQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=mellanox.com; dmarc=pass action=none header.from=mellanox.com; dkim=pass header.d=mellanox.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=C6gnhTa+YR+GVG24fX9+2GYilD+1ITuWWUSngk71798=; b=ooy5RRZU8X/wO2mKXf6EZOcxiRz3qFoj7ndDxwn9rABJlNCW77fzJK+7ZIB5dWfwvq7F6WTEUbPZRPi4/oC5GM/5gFwVUSdEydu1RvFe4tQtD0CdGvbXdlDUyzv8VIgfbuWNQ872fkddv0ou/b8nuZHj1KxRhKLMKtTR4UNv0OI= Received: from AM4PR05MB3265.eurprd05.prod.outlook.com (2603:10a6:205:8::26) by AM4PR05MB3268.eurprd05.prod.outlook.com (2603:10a6:205:4::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3045.19; Wed, 3 Jun 2020 16:14:13 +0000 Received: from AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::bdf0:88a3:3a39:4be4]) by AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::bdf0:88a3:3a39:4be4%7]) with mapi id 15.20.3066.018; Wed, 3 Jun 2020 16:14:13 +0000 From: Slava Ovsiienko To: Nithin Dabilpuram , Olivier Matz CC: "Ananyev, Konstantin" , Jerin Jacob , "Dumitrescu, Cristian" , Nithin Dabilpuram , Thomas Monjalon , "Yigit, Ferruh" , Andrew Rybchenko , Ori Kam , "Burakov, Anatoly" , "Mcnamara, John" , "Kovacevic, Marko" , dpdk-dev , Jerin Jacob , Krzysztof Kanas , "techboard@dpdk.org" Thread-Topic: [dpdk-dev] [EXT] Re: [PATCH 1/3] mbuf: add Tx offloads for packet marking Thread-Index: AQHWOMwOOZbZGYaFPk+nh9CQblgXP6jFYcaAgAEungCAACXcAIAADyAAgAAUowCAAC4sgA== Date: Wed, 3 Jun 2020 16:14:13 +0000 Message-ID: References: <20200515100845.GA19989@outlook.office365.com> <20200515135746.GB9696@outlook.office365.com> <20200528154328.GA3029@outlook.office365.com> <20200602142537.GA6274@outlook.office365.com> <20200603082844.GG12564@platinum> <20200603104414.GA28936@outlook.office365.com> <20200603113822.GI12564@platinum> <20200603125214.GA8237@outlook.office365.com> In-Reply-To: <20200603125214.GA8237@outlook.office365.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: marvell.com; dkim=none (message not signed) header.d=none;marvell.com; dmarc=none action=none header.from=mellanox.com; x-originating-ip: [95.164.10.10] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: f10e2007-6141-4a35-9669-08d807d92841 x-ms-traffictypediagnostic: AM4PR05MB3268: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 04238CD941 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 8AShts+zwRQK258bKkrZP7Yu6A/dOmos0gqQljLhDcvb+xMBpiy1PCw5Nvl7RV8tbwfZsxp3yS8155xN/5yTH56wPz+3WYWQNyzgFrDGyko4sTUJMvw9pHFANWp1M/M7Vgd7S52eWSr5ZyqvGgo8YNdG8tkTXF/FOOeYSSg9Wuu609nB9n7yjk2l9OtB4fFEIW0kHZ5mRrfyEGvvCpOb+B3uCl2AXgj+kIKh72CiNC7sfRFG87tFjczatvB8EM/FjGQ4B8PHHlERB2ltxL94ADpDw5s3/KgaQkcqwCKppCdW2C3U7aSZZANW5eFjfM16KfszQ2Eb4eD98AYXTJWfqJmfL7ZUrTdc3WUUyAcvLxrFKL/rbIzyvsNMfI4lJGZpWxVfxhs8NtIFwz3C2JSL4A== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AM4PR05MB3265.eurprd05.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(366004)(39860400002)(136003)(396003)(376002)(346002)(86362001)(45080400002)(478600001)(186003)(5660300002)(7416002)(30864003)(966005)(66556008)(54906003)(53546011)(7696005)(52536014)(4326008)(76116006)(26005)(316002)(66946007)(8936002)(6506007)(64756008)(33656002)(71200400001)(110136005)(66476007)(2906002)(66446008)(55016002)(9686003)(83380400001)(8676002); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: ydM/yc/zX7mWEPfzUpwavxzNxv9MMCwxMji6dUi0AZ30zi9/eoZ6n1JP3NCMTP5Cdyws0HbqeRUWEG2cbth8BZhhJoWo80wlWS9pwIvPtVTFyUD4M1yFQYa88F5lKh5pC8WtdutojX5/bgb30/Q41cUduFzmop42MIddi6hpDhzImcbgpkcrwCe8Q6Qtln68e6eG2ksu9zIFx8A5h/wBZtGvXWhX8rcAbnYWlcVeOnb8WMIfyra9hMLaxFArz6eKgu9f2uZ5qSz9P6vqYEdq87HS2NPTm0UCS/BBMfldo6WSNiC4sTJxN4nAmLJX458T6A6SlEvSbRtMYqQGNcBOEHMivRa60Wa6XhhFph/Mlfj2RmJYcyHm60R0Rjfm7UbzhhO+T5b/84PUPC3XpjnWxnN22g74bAEsefxvFFq6XqRe1RCtnyirRtDyDzI18qWyLjh32mNcZRUUlmKmr8hVpAqRSli8XLteSj5FvEq5uXo= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: f10e2007-6141-4a35-9669-08d807d92841 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jun 2020 16:14:13.6540 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: oLq8bK8+9+o1HZwRBG4FabZvS9GKrF/cvkfmKcDhYGZQSRO6nP9TECvfAW97M0ly/ZiqZRCRzd0//1QJVtZKMnMPPfcpYTpjuC6d/YmJy4I= X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR05MB3268 Subject: Re: [dpdk-dev] [EXT] Re: [PATCH 1/3] mbuf: add Tx offloads for packet marking 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" > -----Original Message----- > From: dev On Behalf Of Nithin Dabilpuram > Sent: Wednesday, June 3, 2020 15:52 > To: Olivier Matz > Cc: Ananyev, Konstantin ; Jerin Jacob > ; Dumitrescu, Cristian > ; Nithin Dabilpuram > ; Thomas Monjalon ; > Yigit, Ferruh ; Andrew Rybchenko > ; Ori Kam ; Burakov, > Anatoly ; Mcnamara, John > ; Kovacevic, Marko > ; dpdk-dev ; Jerin Jacob > ; Krzysztof Kanas ; > techboard@dpdk.org > Subject: Re: [dpdk-dev] [EXT] Re: [PATCH 1/3] mbuf: add Tx offloads for > packet marking >=20 > On Wed, Jun 03, 2020 at 01:38:22PM +0200, Olivier Matz wrote: > > Hi Nithin, > > > > On Wed, Jun 03, 2020 at 04:14:14PM +0530, Nithin Dabilpuram wrote: > > > On Wed, Jun 03, 2020 at 10:28:44AM +0200, Olivier Matz wrote: > > > > Hi, > > > > > > > > On Tue, Jun 02, 2020 at 07:55:37PM +0530, Nithin Dabilpuram wrote: > > > > > On Tue, Jun 02, 2020 at 10:53:08AM +0000, Ananyev, Konstantin > wrote: > > > > > > Hi Jerin, > > > > > > > > > > > > > > > > I also share Olivier's concern about consuming 3 bits i= n > ol_flags for that feature. > > > > > > > > > > Can it probably be squeezed somehow? > > > > > > > > > > Let say we reserve one flag that this information is > > > > > > > > > > present or not, and re-use one of rx-only fields for st= ore > additional information (packet_type, or so). > > > > > > > > > > Or might be some other approach. > > > > > > > > > > > > > > > > > > We are fine with this approach where we define one bit > > > > > > > > > in Tx offloads for pkt marking and and 3 bits reused from= Rx > offload flags area. > > > > > > > > > > > > > > > > > > For example: > > > > > > > > > > > > > > > > > > @@ -186,10 +186,16 @@ extern "C" { > > > > > > > > > > > > > > > > > > /* add new RX flags here, don't forget to update > > > > > > > > > PKT_FIRST_FREE */ > > > > > > > > > > > > > > > > > > +/* Reused Rx offload bits for Tx offloads */ > > > > > > > > > +#define PKT_X_TX_MARK_VLAN_DEI (1ULL << 0) > > > > > > > > > +#define PKT_X_TX_MARK_IP_DSCP (1ULL << 1) > > > > > > > > > +#define PKT_X_TX_MARK_IP_ECN (1ULL << 2) > > > > > > > > > + > > > > > > > > > #define PKT_FIRST_FREE (1ULL << 23) -#define > > > > > > > > > PKT_LAST_FREE (1ULL << 40) > > > > > > > > > +#define PKT_LAST_FREE (1ULL << 39) > > > > > > > > > > > > > > > > > > /* add new TX flags here, don't forget to update > > > > > > > > > PKT_LAST_FREE */ > > > > > > > > > +#define PKT_TX_MARK_EN (1ULL << 40) > > > > > > > > > > > > > > > > > > Is this fine ? > > > > > > > > > > > > > > > > Any thoughts on this approach which uses only 1 bit in Tx > > > > > > > > flags out of 18 and reuse unused Rx flag bits ? > > > > > > > > > > > > My thought was not about re-defining the flags (I think it is > > > > > > better to keep them intact), but adding a union for one of rx-o= nly > fields (packet_type/rss/timestamp). > > > > > > > > > > Ok. Adding a union field at packet_type field is also fine like b= elow. > > > > > > > > > > @@ -187,9 +187,10 @@ extern "C" { > > > > > /* add new RX flags here, don't forget to update PKT_FIRST_FREE > > > > > */ > > > > > > > > > > #define PKT_FIRST_FREE (1ULL << 23) -#define PKT_LAST_FREE > > > > > (1ULL << 40) > > > > > +#define PKT_LAST_FREE (1ULL << 39) > > > > > > > > > > /* add new TX flags here, don't forget to update PKT_LAST_FREE > > > > > */ > > > > > +#define PKT_TX_MARK_EN (1ULL << 40) > > > > > > > > > > /** > > > > > * Outer UDP checksum offload flag. This flag is used for > > > > > enabling @@ -461,6 +462,14 @@ enum { #endif }; > > > > > > > > > > +/* Tx packet marking flags in rte_mbuf::tx_mark. > > > > > + * Valid only when PKT_TX_MARK_EN is set in > > > > > + * rte_mbuf::ol_flags. > > > > > + */ > > > > > +#define TX_MARK_VLAN_DEI (1ULL << 0) > > > > > +#define TX_MARK_IP_DSCP (1ULL << 1) > > > > > +#define TX_MARK_IP_ECN (1ULL << 2) > > > > > + > > > > > /** > > > > > * The generic rte_mbuf, containing a packet mbuf. > > > > > */ > > > > > @@ -543,6 +552,10 @@ struct rte_mbuf { > > > > > }; > > > > > uint32_t inner_l4_type:4; /**< Inner L4 type. */ > > > > > }; > > > > > + struct { > > > > > + uint32_t reserved:29; > > > > > + uint32_t tx_mark:3; > > > > > + }; > > > > > }; > > > > > > > > > > > > > > > > > > > > Please correct me if this is not what you mean. > > > > > > > > I'm not a big fan of reusing Rx fields or flags for Tx. > > > > It's not obvious for an application than adding a tx_mark will > > > > overwrite the packet_type. I understand that the risk is limited > > > > because packet_type is Rx and the marks are Tx, but there is still = one. > > > > > > I'm also not a big fan but just wanted to take this approach so > > > that, it can both conserve space and also help fast path. > > > Reusing Rx area is however not a new thing as is already followed > > > for > > > mbuf->txadapter field. > > > > Yes, and in my opinion this is something we should avoid when > > possible, because it makes some features exclusive (ex: the big union > > with sched/rss/adapter/usr/...). > > > > > Apart from documentation issue, Is there any other issue or future > > > ramification with using Rx field's for Tx ? > > > > No, I don't see any other issue except the ones we already mentioned (d= oc, > code clarity, ). > > > > > If it is only about documentation, then we can add more documentation > to make things clear. > > > > > > > > To summarize the different proposed approaches (please correct me i= f > I'm wrong): > > > > > > > > a- add 3 Tx mbuf flags > > > > (-) consumes limited resource > > > > > > > > b- add 3 dynamic flags > > > > (-) slower > > > > > > - Tx burst Vector implementation can't be done for this tx offload as > > > offset keeps changing. > > > > A vector implementation can be done. But yes, it would be slower than > > with a static flag. >=20 > Very slow atleast in our HW as, we try to translate ol_flags to HW descri= ptor > flags in addition to extra operations to be done like offset calculations= etc. The dynamic flag offset is not subject to be changed after registration. So, flag offset can be converted once into appropriate mask and stored loca= lly by PMD for further using in vector instructions. The only difference - loaded variable mask instead of constant one, should = not affect performance too much. With the cmpeq/shifts the and results can be converted to any desired prede= fined mask (like static one). >=20 > So if we have fixed offsets, then it is easy to have static constant BTW, how this constant is loaded? I suppose on x86 it would be rather mm_load?_si128(*constant array) - no di= fference=20 between dynamic and static masks at all. BTW, there is the hint - to make l= ocal copy of the mask/offset in order to avoid cache-line concurrency in global variable sto= rage. > 128/256 bit words with offsets and use things like shuffle/table lookup t= o > reorganize multiple mbuf flags to descriptor fields in a single instructi= on. Offsets in the HW descriptor remain the fixed ones, so shuffle would still = work OK. Do you already have some vectorized implementation? It would be curious to = have a look at. I strongly share the concern about defining the static mbuf flags, we should consider all ways to avoid doing this. WBR, Slava >=20 > > > > > > > > > > c- add 1 Tx flag and union with Rx field > > > > (-) exclusive with Rx field > > > > (-) still consumes one flag > > > > > > > > My preference is still b-, for these reasons: > > > > > > > > - There are many different DPDK use cases, and resources in mbuf is > tight. > > > > Recent contributions (rte_flow and ice driver) already made use o= f > dynamic > > > > fields/flags. > > > - Since RTE_FLOW metadata is 32-bit field, it is a clear candidate > > > for dynamic flags. > > > > I'm not sure to get why it is a better candidate than packet marking. > > You mean because it requires more room in mbuf? >=20 > Yes, I feel space consumption is one way to decide whether it should be a > dynfield or static field. >=20 > IMO, other parameter to judge could be whether the field definition/usage > itself is well know standard and is a part of RTE spec or its definition = is > vendor specific. >=20 > > > > > - ICE PMD's dynamic field is however a vendor specific field and > > > only for ICE PMD users. > > > > Yes, but ICE PMD users may be as important as packet marking users. >=20 > Agree, I only meant that the flag ICE PMD registered cannot be used for > other PMD's so by using dynamic field, we are avoiding wastage of a stati= c > field that is needed only by one specific PMD irrespective of whether tha= t > PMD is probed or not. >=20 > > > > > In this case, it is just 1 bit out of 18 free bits available in ol_fl= ags. > > > > > > > > > > > - When I implemented the dynamic fields/flags feature, I did a test > which > > > > showed that the cost of having a dynamic offset was few cycles (o= n > my test > > > > platform, it was~3 cycles for reading a field and ~2 cycles for w= riting a > > > > field). > > > > > > I think this cost is of the case where the address where the > > > dyn_offset is stored is already in cache as it needs to be read first= . > > > > This fetch of the value (in case it is not in cache) can be done once > > per bulk, so I'm not sure the impact would be high. >=20 > Agreed, for bulk case offset loading should have less impact. >=20 > > > > > > Regards, > > Olivier > > > > > > > > > > > > > > > > > > Regards, > > > > Olivier > > > > > > > > > > > > > > > > > > > > > > > > > > > > > + Techboard > > > > > > > > > > > > > > There is a related thread going on > > > > > > > https://eur03.safelinks.protection.outlook.com/?url=3Dhttps%3= A > > > > > > > %2F%2Furldefense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttp- > 3A__ma > > > > > > > ils.dpdk.org_archives_dev_2020- > 2DMay_168810.html%26d%3DDwIGa > > > > > > > > Q%26c%3DnKjWec2b6R0mOyPaz7xtfQ%26r%3DFZ_tPCbgFOh18zwRPO9H0y > D > > > > > > > > x8VW38vuapifdDfc8SFQ%26m%3DnyV4Rud03HW6DbWMpyvOCulQNkagmf > o0w > > > > > > > > KtrwQ7zmmg%26s%3DVuktoUb_xoLsHKdB9mV87x67cP9tXk3DqVXptt9nF_s > > > > > > > > %26e%3D&data=3D02%7C01%7Cviacheslavo%40mellanox.com%7C5e7a > > > > > > > > 9c93fd684e09267108d807bd0160%7Ca652971c7d2e4d9ba6a4d149256f4 > > > > > > > > 61b%7C0%7C0%7C637267855650797843&sdata=3Dr%2B0JIDapZocl6DD > > > > > > > A8wbNd4LV0CX6zEdDoQJhBpMoELw%3D&reserved=3D0 > > > > > > > > > > > > > > If there is no consensus on email, then I would like to add > > > > > > > this item to the next TB meeting. > > > > > > > > > > > > Ok, I'll add that to tomorrow meeting agenda. > > > > > > Konstantin > > > > > > > > > > > > > >