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 0A913A00E6 for ; Thu, 8 Aug 2019 18:53:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4CE1F1BE92; Thu, 8 Aug 2019 18:53:55 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 16E601BE8B for ; Thu, 8 Aug 2019 18:53:52 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Aug 2019 09:53:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,362,1559545200"; d="scan'208";a="169045991" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by orsmga008.jf.intel.com with ESMTP; 08 Aug 2019 09:53:49 -0700 Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by irsmsx110.ger.corp.intel.com (163.33.3.25) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 8 Aug 2019 17:53:48 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.164]) by irsmsx155.ger.corp.intel.com ([169.254.14.201]) with mapi id 14.03.0439.000; Thu, 8 Aug 2019 17:53:48 +0100 From: "Ananyev, Konstantin" To: Jerin Jacob Kollanukkaran , "Pavan Nikhilesh Bhagavatula" , "stephen@networkplumber.org" , "arybchenko@solarflare.com" , "hemant.agrawal@nxp.com" , "thomas@monjalon.net" , "Yigit, Ferruh" , "Richardson, Bruce" , Neil Horman , "Mcnamara, John" , "Kovacevic, Marko" CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags Thread-Index: AQHVTceV2pPltlQueEKmYkOKhhENaqbw9/aQ///8DYCAABJmkP//89kAgAARl1D///iRgAANnyBQ Date: Thu, 8 Aug 2019 16:53:48 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580168A63C89@irsmsx105.ger.corp.intel.com> References: <20190808081752.516-1-pbhagavatula@marvell.com> <20190808085859.796-1-pbhagavatula@marvell.com> <2601191342CEEE43887BDE71AB9772580168A63989@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772580168A639E4@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772580168A63A1B@irsmsx105.ger.corp.intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZWIyNDA5MDgtNzFkMS00Njg4LWI2NDItZTg1OTNkMTNhM2Y5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiQzF0TGZieHRkeE1kWmNpRmNCRlNwSHZ6M1g1N2NaUzRJZUpTZ3dnZWRZZmZ3U244dmlkdEMzYW8rOHlZR0hWZCJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags 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" > > > > > > > Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``, > > > > > > ``DEV_RX_OFFLOAD_RSS`` > > > > > > > and ``DEV_RX_OFFLOAD_FLOW_MARK``. > > > > > > > > > > > > > > Signed-off-by: Pavan Nikhilesh > > > > > > > Acked-by: Andrew Rybchenko > > > > > > > Acked-by: Jerin Jacob > > > > > > > --- > > > > > > > v3 Changes: > > > > > > > - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH > > (anndrew). > > > > > > > > > > > > > > v2 Changes: > > > > > > > - Reword for clarity. > > > > > > > > > > > > > > doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++ > > > > > > > 1 file changed, 13 insertions(+) > > > > > > > > > > > > > > diff --git a/doc/guides/rel_notes/deprecation.rst > > > > > > > b/doc/guides/rel_notes/deprecation.rst > > > > > > > index 37b8592b6..056c5709f 100644 > > > > > > > --- a/doc/guides/rel_notes/deprecation.rst > > > > > > > +++ b/doc/guides/rel_notes/deprecation.rst > > > > > > > @@ -78,3 +78,16 @@ Deprecation Notices > > > > > > > to set new power environment if power environment was > > > > > > > already > > > > > > initialized. > > > > > > > In this case the function will return -1 unless the > > > > > > > environment is unset > > > > > > first > > > > > > > (using ``rte_power_unset_env``). Other function usage > > > > > > > scenarios will not > > > > > > change. > > > > > > > + > > > > > > > +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``, > > > > > > > +``DEV_RX_OFFLOAD_RSS_HASH`` > > > > > > > + and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11. > > > > > > > > > > > > One question about DEV_RX_OFFLOAD_PTYPE: > > > > > > Does it mean that new ol_flags value (PKT_RX_PTYPE) will be > > > > > > introduced to indicate that mbuf.packet_type value is set? > > > > > > Or PMD will have to set mbuf.packet_type to zero, when > > > > > > DEV_RX_OFFLOAD_PTYPE was not enabled by user? > > > > > > > > > > I was thinking when DEV_RX_OFFLOAD_PTYPE is set > > > > > - mbuf.packet_type will be valid and mbuf.packet_type will have > > > > > parsed > > > > packet type. > > > > > If not set > > > > > - mbuf.packet_type can be anything application should not use > > > > mbuf.packet_type field. > > > > > > > > But in that case, we do need a new value for ol_flags, PKT_RX_PTYPE > > > > or so, right? > > > > > > Since application has two knobs rte_eth_dev_get_supported_ptypes() an= d > > > DEV_RX_OFFLOAD_PTYPE. We may not need to new ol_flags for this > > change. Right? > > > i.e if application sets the DEV_RX_OFFLOAD_PTYPE, The application wil= l > > > get the parsed ptypes by the driver(=3D > > rte_eth_dev_get_supported_ptypes()). > > > So there is no scope ambiguity. Right? > > > > I still think there is: > > Imagine user has 2 eth devices, one does support DEV_RX_OFFLOAD_PTYPE, > > second doesn't. Now he has a mix of packets from both devices, that yo= u > > want t process. > > How would he figure out for which of them ptype values are valid, and f= or > > each are not? > > Trace back from what port he has received them? > > Not very convenient, and not always possible. >=20 > I thought so. But in that case, application can always set DEV_RX_OFFLOAD= _PTYPE > Flags for all the ethdev ports. Right? Rather having any complicated ol_f= lags > or port based parsing. If limit the _contract_ to following, we are good.= Right? > # when DEV_RX_OFFLOAD_PTYPE is set, mbuf.packet_type will be valid > and mbuf.packet_type will have parsed packet type Yes sure in principle user can calculate smallest common subset of RX offlo= ads supported by all devs in the system and use only them. Then he can use some global value for ol_flags that will be setup at initia= lization time, instead of checking ol_flags for every mbuf.=20 Though inside DPDK we don't use that method for other offloads (cksum, vlan= , rss). Why we should do different here? Again how to deal with hot-plugged devices with such approach? >=20 > or the negative offload(This contract is pretty clear, I don't think any = ambiguity at all) > # when DEV_RX_OFFLOAD_NO_PTYPE(something similar) is set, > mbuf.packet_type will be invalid. >=20 > > I think we need either to introduce new ol_flag value (as we usually do= for > > other RX offloads), > > or force PMD to always set ptype value. >=20 > Setting new ol_flag value may effect performance for existing drivers > which don't planning to use this offload If the driver doesn't support DEV_RX_OFFLOAD_PTYPE, it wouldn't need to set= anything (neither ol_flags, neither packet_type). > and it complicates the > application to have additional check based on ol_flag. If you see any cor= ner case with above section, >=20 > How about just setting as ptype as 0 incase it is not parsed by driver. As I said above - ok by me. AFAIK, this is current behavior, so no changes in PMD will be required. > Actual lookup is the costly one, writing 0 to pytpe is not costly > as there are plenty of writes in Rx and it will be write merged(No CPU st= all) Yes packet_type is at first 64B, so shouldn't cause any extra overhead. >=20 > I did not get the complete picture of "rte_eth_dev_set_supported_ptypes(u= int16_t port_id, uint32_t > ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE? scheme", Does it help? I thought about it as just a different way to disable(/limit) requested by = user PTYPE support. If let say user is not interested in ptype information at all, he can ask P= MD to just=20 always set ptype value to 0: rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_UNKNOWN); if he is interested just in L2/L3 layer info, he can ask PMD to provide ptype information only for L2/L3: rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MAS= K); Or to enable all supported by PMD ptypes:=20 rte_eth_dev_set_supported_ptypes(port, UINT32_MAX)