From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 3A9C72BF5 for ; Thu, 26 Jan 2017 17:33:58 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP; 26 Jan 2017 08:33:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,290,1477983600"; d="scan'208";a="1118198865" Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by fmsmga002.fm.intel.com with ESMTP; 26 Jan 2017 08:33:56 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.38]) by IRSMSX154.ger.corp.intel.com ([169.254.12.40]) with mapi id 14.03.0248.002; Thu, 26 Jan 2017 16:33:56 +0000 From: "Ananyev, Konstantin" To: Olivier MATZ CC: "Wu, Jingjing" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload flags Thread-Index: AQHSdjf/GdSv8Vm7LEyQbSCKSFxHsqFK25YggAADLQCAAAe9wIAABvCAgAAJJ0A= Date: Thu, 26 Jan 2017 16:33:55 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583F10D25D@irsmsx105.ger.corp.intel.com> References: <1485258454-86107-1-git-send-email-jingjing.wu@intel.com> <2601191342CEEE43887BDE71AB9772583F10D0CB@irsmsx105.ger.corp.intel.com> <20170126160516.398ac002@glumotte.dev.6wind.com> <2601191342CEEE43887BDE71AB9772583F10D10F@irsmsx105.ger.corp.intel.com> <20170126165747.09ac480c@glumotte.dev.6wind.com> In-Reply-To: <20170126165747.09ac480c@glumotte.dev.6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: 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 1/2] mbuf: fix bitmask of Tx 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: , X-List-Received-Date: Thu, 26 Jan 2017 16:33:59 -0000 > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Thursday, January 26, 2017 3:58 PM > To: Ananyev, Konstantin > Cc: Olivier MATZ ; Wu, Jingjing ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload flags >=20 > On Thu, 26 Jan 2017 15:35:29 +0000, "Ananyev, Konstantin" > wrote: > > Hi Olivier, > > > > > -----Original Message----- > > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > > Sent: Thursday, January 26, 2017 3:05 PM > > > To: Ananyev, Konstantin > > > Cc: Wu, Jingjing ; dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload > > > flags > > > > > > On Thu, 26 Jan 2017 14:58:08 +0000, "Ananyev, Konstantin" > > > wrote: > > > > Hi Jingjng, > > > > > > > > > -----Original Message----- > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu > > > > > Sent: Tuesday, January 24, 2017 11:48 AM > > > > > To: dev@dpdk.org > > > > > Cc: Wu, Jingjing > > > > > Subject: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload > > > > > flags > > > > > > > > > > Some Tx offload flags are missed in Bitmask of all supported > > > > > packet Tx offload features flags. > > > > > This patch fixes it. > > > > > > > > Not sure what it exactly fixes? > > > > As I remember these flags don't specify any TX offload for HW to > > > > perform, But just provide information to the TX function. > > > > Again, why only i40e code is modified? > > > > As I remember we have the same code in other PMDs too. > > > > Konstantin > > > > > > > > > > > > > > Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation") > > > > > Signed-off-by: Jingjing Wu > > > > > --- > > > > > lib/librte_mbuf/rte_mbuf.h | 4 ++++ > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > diff --git a/lib/librte_mbuf/rte_mbuf.h > > > > > b/lib/librte_mbuf/rte_mbuf.h index bfce9f4..e57a4d2 100644 > > > > > --- a/lib/librte_mbuf/rte_mbuf.h > > > > > +++ b/lib/librte_mbuf/rte_mbuf.h > > > > > @@ -295,8 +295,12 @@ extern "C" { > > > > > */ > > > > > #define PKT_TX_OFFLOAD_MASK ( \ > > > > > PKT_TX_IP_CKSUM | \ > > > > > + PKT_TX_IPV4 | \ > > > > > + PKT_TX_IPV6 | \ > > > > > PKT_TX_L4_MASK | \ > > > > > PKT_TX_OUTER_IP_CKSUM | \ > > > > > + PKT_TX_OUTER_IPV4 | \ > > > > > + PKT_TX_OUTER_IPV6 | \ > > > > > PKT_TX_TCP_SEG | \ > > > > > PKT_TX_QINQ_PKT | \ > > > > > PKT_TX_VLAN_PKT | \ > > > > > -- > > > > > 2.4.11 > > > > > > > > > > Also, it looks like MACSEC is missing. To avoid forgetting flags in > > > the future, what do you think about doing the following (not > > > tested)? > > > > > > > > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > > > index b3cccfc..aa1dc76 100644 > > > --- a/lib/librte_mbuf/rte_mbuf.h > > > +++ b/lib/librte_mbuf/rte_mbuf.h > > > @@ -182,9 +182,11 @@ extern "C" { > > > */ > > > #define PKT_RX_TIMESTAMP (1ULL << 17) > > > > > > -/* add new RX flags here */ > > > +/* add new RX flags here, and update __PKT_RX_NEXT */ > > > +#define __PKT_RX_NEXT (1ULL << 18) > > > > > > -/* add new TX flags here */ > > > +/* add new TX flags here, and update __PKT_TX_NEXT */ > > > +#define __PKT_TX_NEXT (1ULL << 43) > > > > > > /** > > > * Offload the MACsec. This flag must be set by the application to > > > enable @@ -295,17 +297,16 @@ extern "C" { > > > #define PKT_TX_OUTER_IPV6 (1ULL << 60) > > > > > > /** > > > + * Bitmask of all supported packet Rx offload features flags, > > > + * which can be set for packet. > > > + */ > > > +#define PKT_RX_OFFLOAD_MASK (__PKT_RX_NEXT - 1) > > > + > > > +/** > > > * Bitmask of all supported packet Tx offload features flags, > > > * which can be set for packet. > > > */ > > > -#define PKT_TX_OFFLOAD_MASK ( \ > > > - PKT_TX_IP_CKSUM | \ > > > - PKT_TX_L4_MASK | \ > > > - PKT_TX_OUTER_IP_CKSUM | \ > > > - PKT_TX_TCP_SEG | \ > > > - PKT_TX_QINQ_PKT | \ > > > - PKT_TX_VLAN_PKT | \ > > > - PKT_TX_TUNNEL_MASK) > > > +#define PKT_TX_OFFLOAD_MASK ((~(__PKT_TX_NEXT - 1)) & > > > 0x1fffffffffffffff) > > > > I see your point but should, let say, PKT_TX_IPV4 be part of > > PKT_TX_OFFLOAD_MASK at all? It doesn't really define any offload for > > PMD/HW to perform. It just provide extra information for PMD so it > > can successfully process other offload requests. Konstantin >=20 > Yes, that's right. On the other hand, differentiating them may confuse > the PMD developpers (that's probably the case for this patch). >=20 > Having PKT_TX_MASK that includes all TX flags automatically would also > do the job (knowing PMDs must be updated),=20 Yes, all other PMDs that do use PKT_TX_OFFLOAD_MASK have to be updated in t= hat case... But ok, I agree what do you propose might help to avoid confusion.=20 >and would avoid to forget > flags in the future... if we decide to do it, it would be better before > 17.02, because PKT_TX_OFFLOAD_MASK was added after 16.11, so it is not > yet part of the API. Agree. >=20 > In any case, we need a patch to fix the missing PKT_TX_MACSEC in > PKT_TX_OFFLOAD_MASK. >=20 >=20 Yes, we do. Konstantin