[dpdk-dev,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.
  

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.

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
  

> -----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
  

> -----Original Message-----
> From: Zhang, Helin
> Sent: Wednesday, March 4, 2015 8:34 AM
> To: Chilikin, Andrey; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of packet_type
> in rte_mbuf
> 
> 
> 
> > -----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. 
I do not see how FVL internal descriptor can affect DPDK mbuf structure.

> 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.
But if we know that we would need this change for QinQ anyway should we move
vlan_tci +reserved 16bits now in this patch instead of testing performance twice -
for this and for future patch when we move vlan_tci+reserved?

Regards,
Andrey

> 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
  

> -----Original Message-----
> From: Chilikin, Andrey
> Sent: Wednesday, March 4, 2015 6:59 PM
> To: Zhang, Helin; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of packet_type in
> rte_mbuf
> 
> > -----Original Message-----
> > From: Zhang, Helin
> > Sent: Wednesday, March 4, 2015 8:34 AM
> > To: Chilikin, Andrey; dev@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of
> > packet_type in rte_mbuf
> >
> >
> >
> > > -----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.
> I do not see how FVL internal descriptor can affect DPDK mbuf structure.
FVL rx descriptor plays key role of the mbuf structure definition, as we have
Vector PMD.

> 
> > 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.
> But if we know that we would need this change for QinQ anyway should we
> move vlan_tci +reserved 16bits now in this patch instead of testing
> performance twice - for this and for future patch when we move
> vlan_tci+reserved?
Good idea, and I will discuss it with team members to see if there is any objection.
Generally we modify things as needed, but not modify things by prediction. This
was indicated by Thomas and other reviewers several times.

Regards,
Helin

> 
> Regards,
> Andrey
> 
> > 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