From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 614BD2C71 for ; Wed, 4 Jan 2017 18:05:38 +0100 (CET) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga104.fm.intel.com with ESMTP; 04 Jan 2017 09:05:33 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,459,1477983600"; d="scan'208";a="45699659" Received: from dpdk19.sh.intel.com (HELO dpdk19) ([10.239.129.113]) by orsmga004.jf.intel.com with ESMTP; 04 Jan 2017 09:05:31 -0800 Date: Thu, 5 Jan 2017 01:00:11 +0800 From: Tiwei Bie To: "Ananyev, Konstantin" Cc: "dev@dpdk.org" , "adrien.mazarguil@6wind.com" , "Lu, Wenzhuo" , "Mcnamara, John" , "olivier.matz@6wind.com" , "thomas.monjalon@6wind.com" , "Zhang, Helin" , "Dai, Wei" , "Wang, Xiao W" Message-ID: <20170104170011.GB56511@dpdk19> References: <1482939691-34855-1-git-send-email-tiwei.bie@intel.com> <1483514502-32841-1-git-send-email-tiwei.bie@intel.com> <1483514502-32841-4-git-send-email-tiwei.bie@intel.com> <2601191342CEEE43887BDE71AB9772583F0FEE0C@irsmsx105.ger.corp.intel.com> <20170104143923.GA57552@dpdk19> <2601191342CEEE43887BDE71AB9772583F0FEE6D@irsmsx105.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0FEE6D@irsmsx105.ger.corp.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [dpdk-dev] [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API 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: Wed, 04 Jan 2017 17:05:39 -0000 On Wed, Jan 04, 2017 at 11:14:25PM +0800, Ananyev, Konstantin wrote: > > > > -----Original Message----- > > From: Bie, Tiwei > > Sent: Wednesday, January 4, 2017 2:39 PM > > To: Ananyev, Konstantin > > Cc: dev@dpdk.org; adrien.mazarguil@6wind.com; Lu, Wenzhuo ; Mcnamara, John ; > > olivier.matz@6wind.com; thomas.monjalon@6wind.com; Zhang, Helin ; Dai, Wei ; Wang, > > Xiao W > > Subject: Re: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API > > > > On Wed, Jan 04, 2017 at 10:21:08PM +0800, Ananyev, Konstantin wrote: > > > Hi Twei, > > > > > > > -----Original Message----- > > > > From: Bie, Tiwei > > > > Sent: Wednesday, January 4, 2017 7:22 AM > > > > To: dev@dpdk.org > > > > Cc: adrien.mazarguil@6wind.com; Lu, Wenzhuo ; Mcnamara, John ; > > > > olivier.matz@6wind.com; thomas.monjalon@6wind.com; Ananyev, Konstantin ; Zhang, Helin > > > > ; Dai, Wei ; Wang, Xiao W > > > > Subject: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API > > > > > > > > Reserve a Tx capability flag and a Rx capability flag, that can be > > > > used by PMD to define its own capability flags when implementing the > > > > PMD-specific API. > > > > > > > > Suggested-by: Adrien Mazarguil > > > > Signed-off-by: Tiwei Bie > > > > Acked-by: Wenzhuo Lu > > > > --- > > > > lib/librte_ether/rte_ethdev.h | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h > > > > index d465825..8800b39 100644 > > > > --- a/lib/librte_ether/rte_ethdev.h > > > > +++ b/lib/librte_ether/rte_ethdev.h > > > > @@ -857,6 +857,7 @@ struct rte_eth_conf { > > > > #define DEV_RX_OFFLOAD_TCP_LRO 0x00000010 > > > > #define DEV_RX_OFFLOAD_QINQ_STRIP 0x00000020 > > > > #define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040 > > > > +#define DEV_RX_OFFLOAD_RESERVED_0 0x00000080 /**< Used for PMD-specific API. */ > > > > > > > > /** > > > > * TX offload capabilities of a device. > > > > @@ -874,6 +875,7 @@ struct rte_eth_conf { > > > > #define DEV_TX_OFFLOAD_GRE_TNL_TSO 0x00000400 /**< Used for tunneling packet. */ > > > > #define DEV_TX_OFFLOAD_IPIP_TNL_TSO 0x00000800 /**< Used for tunneling packet. */ > > > > #define DEV_TX_OFFLOAD_GENEVE_TNL_TSO 0x00001000 /**< Used for tunneling packet. */ > > > > +#define DEV_TX_OFFLOAD_RESERVED_0 0x00002000 /**< Used for PMD-specific API. */ > > > > > > > > /** > > > > * Ethernet device information > > > > -- > > > > 2.7.4 > > > > > > I am not sure how that supposed to work and how user should know that DEV_RX_OFFLOAD_RESERVED_0 > > > is actually a MACSEC for ixgbe? > > > > Users are not supposed to use DEV_RX_OFFLOAD_RESERVED_0, instead, they > > should use the capabilities and the likes defined in rte_pmd_ixgbe.h > > where the PMD-specifics APIs are declared: > > > > /** > > * If these flags are advertised by the PMD, the NIC supports the MACsec > > * offload. The incoming MACsec traffics can be offloaded transparently > > * after the MACsec offload is configured correctly by the application. > > * And the application can set the PKT_TX_IXGBE_MACSEC flag in mbufs to > > * enable the MACsec offload for the packets to be transmitted. > > */ > > #define DEV_RX_OFFLOAD_IXGBE_MACSEC_STRIP DEV_RX_OFFLOAD_RESERVED_0 > > #define DEV_TX_OFFLOAD_IXGBE_MACSEC_INSERT DEV_TX_OFFLOAD_RESERVED_0 > > > > /** > > * This event will occur when the PN counter in a MACsec connection > > * reach the exhaustion threshold. > > */ > > #define RTE_ETH_EVENT_IXGBE_MACSEC RTE_ETH_EVENT_RESERVED_0 > > > > /** > > * Offload the MACsec. This flag must be set by the application in mbuf > > * to enable this offload feature for a packet to be transmitted. > > */ > > #define PKT_TX_IXGBE_MACSEC PKT_TX_RESERVED_0 > > > > PMD-specific APIs can only be used on the corresponding driver/device, > > so different PMD can share the same reserved bit to represent different > > things when implementing their own PMD-specific APIs. > > Ok, and why do we need it? > Why we can't just have PKT_TX_MACSEC straightway? > What are you trying to gain here? > Is it just for future opportunity to save an extra bit in mbuf.ol_flags? > I don't think we are short of bits here right now, and we don't consume them exra-fast > to start to worry about it. > Again, if it *really* would be for ixgbe only forever, and y don't want to waste a bit in tx_olflags, > why not to introduce device specific ol_flags in mbuf second cache line? > Probably uint16_t would be enough for that. > > > > > > Another question what to do if you would like to create a bonded device over two devices with different NIC types? > > > As I understand you can end up in situation when DEV_RX_OFFLOAD_RESERVED_0 would mean different capabilities. > > > Why not to have this MACSEC capability and ol_flag value as generic ones, as you have them in previous versions of your patch? > > > > Those flags are only used in PMD-specific APIs. I don't think we could > > use the PMD-specific APIs provided by a certain PMD on a bonded device. > > I understand that. > My question was: suppose user would like to create a bonded device over 2 NICs. > One of them is ixgbe, while other would be some other type. > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set. > But it would mean completely different thing. > How bonded device would know that to deal properly? > > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously. > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt. > > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set. > What does it really mean if there are different NIC types in the system? > The only way to answer that question, as I can see, is to keep track from what NIC that packet was received. > Which I suppose, is not always convenient. > The main purpose is to put the PMD-specific APIs in a separate namespace instead of mixing the PMD-specific APIs and global APIs up, and also save the bits in mbuf.ol_flags. There are other ways to achieve this goal, such as introducing the PMD specific ol_flags in mbuf second cache line as you said. I just thought defining some reserved bits seems to be the most simple way which won't introduce many changes. What's your suggestions? Should I just revert the changes and define the generic flags directly? Thanks & regards, Tiwei Bie