Re: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of packet_type in rte_mbuf

["Zhang, Helin" <helin.zhang@intel.com>]

> -----Original Message-----
> From: Chilikin, Andrey
> Sent: Monday, March 2, 2015 7:48 PM
> To: Zhang, Helin; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of packet_type in
> rte_mbuf
> 
> Hi Helin,
> 
> I see that you have removed "uint16_t reserved" member from rte_mbuf:
> 
> > +	uint16_t data_len;        /**< Amount of data in segment buffer. */
> >  	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order)
> */
> > -	uint16_t reserved;
> >  	union {
> >  		uint32_t rss;     /**< RSS hash result if RSS enabled */
> 
> This reserved field was kept next to  vlan_tci as a placeholder for the second
> VLAN label for QinQ support so if need be vlan_tci + reserved could be casted to
> 32 bit QinQ value or one 32bit VNTAG label. Without keeping two label adjusted
> to each other casting to 32 bit will not be possible and will affect QinQ
> performance.
Yes, but packet type is quite important which needs to be extended from 16 bits to 32 bits.
For FVL, the vlan tags are in different fields. We can think of putting them together in mbuf,
Possibly move current vlan tag down, and add one more 16 bits. Let's see what is the best then.
Thanks for the notes!

Regards,
Helin

> 
> Regards,
> Andrey
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Helin Zhang
> > Sent: Friday, February 27, 2015 1:11 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of packet_type
> > in rte_mbuf
> >
> > 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.
> >
> > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > ---
> >  config/common_linuxapp                             |  2 +-
> >  .../linuxapp/eal/include/exec-env/rte_kni_common.h |  4 ++--
> >  lib/librte_mbuf/rte_mbuf.h                         | 23
> +++++++++++++++-------
> >  3 files changed, 19 insertions(+), 10 deletions(-)
> >
> > 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.
> >
> > diff --git a/config/common_linuxapp b/config/common_linuxapp index
> > 97f1c9e..97d7bae 100644
> > --- a/config/common_linuxapp
> > +++ b/config/common_linuxapp
> > @@ -166,7 +166,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..bd1cc09 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,9 @@ struct rte_kni_mbuf {
> >  	uint16_t data_off;      /**< Start address of data in segment buffer. */
> >  	char pad1[4];
> >  	uint64_t ol_flags;      /**< Offload features. */
> > -	char pad2[2];
> > -	uint16_t data_len;      /**< Amount of data in segment buffer. */
> > +	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. */
> >
> >  	/* 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 17ba791..f5b7a8b 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -258,17 +258,26 @@ struct rte_mbuf {
> >  	/* remaining bytes are set on RX when pulling packet from descriptor
> > */
> >  	MARKER rx_descriptor_fields1;
> >
> > -	/**
> > -	 * The packet type, which is used to indicate ordinary packet and also
> > -	 * tunneled packet format, i.e. each number is represented a type of
> > -	 * packet.
> > +	/*
> > +	 * The packet type, which is the combination of outer/inner L2, L3, L4
> > +	 * and tunnel types.
> >  	 */
> > -	uint16_t packet_type;
> > +	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. */
> > +		};
> > +	};
> >
> > -	uint16_t data_len;        /**< Amount of data in segment buffer. */
> >  	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)
> */
> > -	uint16_t reserved;
> >  	union {
> >  		uint32_t rss;     /**< RSS hash result if RSS enabled */
> >  		struct {
> > --
> > 1.9.3