From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 448E45693 for ; Mon, 1 Jun 2015 10:14:27 +0200 (CEST) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1YzKvv-0005Sg-7z; Mon, 01 Jun 2015 10:18:59 +0200 Message-ID: <556C1478.9040005@6wind.com> Date: Mon, 01 Jun 2015 10:14:48 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0 MIME-Version: 1.0 To: Helin Zhang , dev@dpdk.org 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> In-Reply-To: <1433144045-30847-2-git-send-email-helin.zhang@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Mon, 01 Jun 2015 08:14:27 -0000 Hi Helin, +CC Neil 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. What are the plans for this compile-time option in the future? 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. 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 (starting from "Some ABI changes may be too significant to reasonably maintain multiple versions of"). Regards, Olivier > > 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=n > CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n > CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n > CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC=y > -CONFIG_RTE_IXGBE_INC_VECTOR=y > +CONFIG_RTE_IXGBE_INC_VECTOR=n > CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=y > > # > 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 { >