From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id DC4FDC312 for ; Tue, 2 Jun 2015 15:27:12 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 02 Jun 2015 06:27:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,539,1427785200"; d="scan'208";a="739406566" Received: from irsmsx109.ger.corp.intel.com ([163.33.3.23]) by orsmga002.jf.intel.com with ESMTP; 02 Jun 2015 06:27:11 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.36]) by IRSMSX109.ger.corp.intel.com ([169.254.13.51]) with mapi id 14.03.0224.002; Tue, 2 Jun 2015 14:27:10 +0100 From: "O'Driscoll, Tim" To: Olivier MATZ , "Zhang, Helin" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v6 01/18] mbuf: redefine packet_type in rte_mbuf Thread-Index: AQHQnEMUAoQMe4f3wkyEnSARE5pRJZ2ZNYYg Date: Tue, 2 Jun 2015 13:27:09 +0000 Message-ID: <26FA93C7ED1EAA44AB77D62FBE1D27BA54D536B3@IRSMSX102.ger.corp.intel.com> References: <1432284264-17376-1-git-send-email-helin.zhang@intel.com> <1433144045-30847-1-git-send-email-helin.zhang@intel.com> <1433144045-30847-2-git-send-email-helin.zhang@intel.com> <556C1478.9040005@6wind.com> In-Reply-To: <556C1478.9040005@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 v6 01/18] mbuf: redefine packet_type in rte_mbuf X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Jun 2015 13:27:13 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ > Sent: Monday, June 1, 2015 9:15 AM > To: Zhang, Helin; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 01/18] mbuf: redefine packet_type in > rte_mbuf >=20 > Hi Helin, >=20 > +CC Neil >=20 > On 06/01/2015 09:33 AM, Helin Zhang wrote: > > In order to unify the packet type, the field of 'packet_type' in > > 'struct rte_mbuf' needs to be extended from 16 to 32 bits. > > Accordingly, some fields in 'struct rte_mbuf' are re-organized to > > support this change for Vector PMD. As 'struct rte_kni_mbuf' for > > KNI should be right mapped to 'struct rte_mbuf', it should be > > modified accordingly. In addition, Vector PMD of ixgbe is disabled > > by default, as 'struct rte_mbuf' changed. > > To avoid breaking ABI compatibility, all the changes would be > > enabled by RTE_UNIFIED_PKT_TYPE, which is disabled by default. >=20 > What are the plans for this compile-time option in the future? >=20 > I wonder what are the benefits of having this option in terms > of ABI compatibility: when it is disabled, it is ABI-compatible but > the packet-type feature is not present, and when it is enabled we > have the feature but it breaks the compatibility. >=20 > In my opinion, the v5 is preferable: for this kind of features, I > don't see how the ABI can be preserved, and I think packet-type > won't be the only feature that will modify the mbuf structure. I think > the process described here should be applied: > http://dpdk.org/browse/dpdk/tree/doc/guides/rel_notes/abi.rst >=20 > (starting from "Some ABI changes may be too significant to reasonably > maintain multiple versions of"). >=20 >=20 > Regards, > Olivier >=20 This is just like the change that Steve (Cunming) Liang submitted for Inter= rupt Mode. We have the same problem in both cases: we want to find a way to= get the features included, but need to comply with our ABI policy. So, in = both cases, the proposal is to add a config option to enable the change by = default, so we maintain backward compatibility. Users that want these chang= es, and are willing to accept the associated ABI change, have to specifical= ly enable them. We can note in the Deprecation Notices in the Release Notes for 2.1 that th= ese config options will be removed in 2.2. The features will then be enable= d by default. This seems like a good compromise which allows us to get these changes into= 2.1 but avoids breaking the ABI policy. Tim >=20 >=20 > > > > Signed-off-by: Helin Zhang > > Signed-off-by: Cunming Liang > > --- > > config/common_linuxapp | 2 +- > > .../linuxapp/eal/include/exec-env/rte_kni_common.h | 6 ++++++ > > lib/librte_mbuf/rte_mbuf.h | 23 > ++++++++++++++++++++++ > > 3 files changed, 30 insertions(+), 1 deletion(-) > > > > v2 changes: > > * Enlarged the packet_type field from 16 bits to 32 bits. > > * Redefined the packet type sub-fields. > > * Updated the 'struct rte_kni_mbuf' for KNI according to the mbuf > changes. > > > > v3 changes: > > * Put the mbuf layout changes into a single patch. > > * Disabled vector ixgbe PMD by default, as mbuf layout changed. > > > > v5 changes: > > * Re-worded the commit logs. > > > > v6 changes: > > * Disabled the code changes for unified packet type by default, to > > avoid breaking ABI compatibility. > > > > diff --git a/config/common_linuxapp b/config/common_linuxapp > > index 0078dc9..6b067c7 100644 > > --- a/config/common_linuxapp > > +++ b/config/common_linuxapp > > @@ -167,7 +167,7 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_TX_FREE=3Dn > > CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=3Dn > > CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=3Dn > > CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC=3Dy > > -CONFIG_RTE_IXGBE_INC_VECTOR=3Dy > > +CONFIG_RTE_IXGBE_INC_VECTOR=3Dn > > CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=3Dy > > > > # > > diff --git a/lib/librte_eal/linuxapp/eal/include/exec- > env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec- > env/rte_kni_common.h > > index 1e55c2d..7a2abbb 100644 > > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > @@ -117,9 +117,15 @@ struct rte_kni_mbuf { > > uint16_t data_off; /**< Start address of data in segment > buffer. */ > > char pad1[4]; > > uint64_t ol_flags; /**< Offload features. */ > > +#ifdef RTE_UNIFIED_PKT_TYPE > > + char pad2[4]; > > + uint32_t pkt_len; /**< Total pkt len: sum of all segment > data_len. */ > > + uint16_t data_len; /**< Amount of data in segment buffer. */ > > +#else > > char pad2[2]; > > uint16_t data_len; /**< Amount of data in segment buffer. */ > > uint32_t pkt_len; /**< Total pkt len: sum of all segment > data_len. */ > > +#endif > > > > /* fields on second cache line */ > > char pad3[8] __attribute__((__aligned__(RTE_CACHE_LINE_SIZE))); > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > > index ab6de67..a8662c2 100644 > > --- a/lib/librte_mbuf/rte_mbuf.h > > +++ b/lib/librte_mbuf/rte_mbuf.h > > @@ -269,6 +269,28 @@ struct rte_mbuf { > > /* remaining bytes are set on RX when pulling packet from > descriptor */ > > MARKER rx_descriptor_fields1; > > > > +#ifdef RTE_UNIFIED_PKT_TYPE > > + /* > > + * The packet type, which is the combination of outer/inner L2, L3, > L4 > > + * and tunnel types. > > + */ > > + union { > > + uint32_t packet_type; /**< L2/L3/L4 and tunnel information. > */ > > + struct { > > + uint32_t l2_type:4; /**< (Outer) L2 type. */ > > + uint32_t l3_type:4; /**< (Outer) L3 type. */ > > + uint32_t l4_type:4; /**< (Outer) L4 type. */ > > + uint32_t tun_type:4; /**< Tunnel type. */ > > + uint32_t inner_l2_type:4; /**< Inner L2 type. */ > > + uint32_t inner_l3_type:4; /**< Inner L3 type. */ > > + uint32_t inner_l4_type:4; /**< Inner L4 type. */ > > + }; > > + }; > > + > > + uint32_t pkt_len; /**< Total pkt len: sum of all segments. > */ > > + uint16_t data_len; /**< Amount of data in segment buffer. */ > > + uint16_t vlan_tci; /**< VLAN Tag Control Identifier (CPU > order) */ > > +#else > > /** > > * The packet type, which is used to indicate ordinary packet and > also > > * tunneled packet format, i.e. each number is represented a type > of > > @@ -280,6 +302,7 @@ struct rte_mbuf { > > uint32_t pkt_len; /**< Total pkt len: sum of all segments. > */ > > uint16_t vlan_tci; /**< VLAN Tag Control Identifier (CPU > order) */ > > uint16_t reserved; > > +#endif > > union { > > uint32_t rss; /**< RSS hash result if RSS enabled */ > > struct { > >