Re: [dpdk-dev] [PATCH v2 02/13] mbuf: reorder fields by time of use

["Richardson, Bruce" <bruce.richardson@intel.com>]

> -----Original Message-----
> From: Liu, Jijiang
> Sent: Monday, September 15, 2014 8:12 AM
> To: Richardson, Bruce; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 02/13] mbuf: reorder fields by time of use
> 
> Hi Bruce,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Thursday, September 11, 2014 9:16 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 02/13] mbuf: reorder fields by time of use
> >
> > *  Reorder the fields in the mbuf so that we have fields that are used together
> > side-by-side in the structure. This means that we have a contiguous block of
> > 8-bytes in the mbuf which are used to reset an mbuf of descriptor rearm, and a
> > block of 16-bytes of data (excluding flags) which are set on RX from the
> > received packet descriptor.
> > * Use dummy fields as appropriate to ensure alignment or to reserve gaps for
> > later field additions.
> > * Place most items which are not used by fast-path RX separately at the end of
> > the structure so they can later be moved to a separate cache line.
> > [The l2/l3 length fields are not moved at this stage as doing so will cause
> > overflow to the next cache line].
> >
> > Updated in V2:
> > * Updated the KNI buffer structure to match that of new mbuf
> > * Cleaned up commit message typos.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  .../linuxapp/eal/include/exec-env/rte_kni_common.h | 12 ++++++-----
> >  lib/librte_mbuf/rte_mbuf.h                         | 25
> > ++++++++++++----------
> >  2 files changed, 21 insertions(+), 16 deletions(-)
> >
> > 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 07908ac..f2b502c 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
> > @@ -108,15 +108,17 @@ struct rte_kni_fifo {
> >   * Padding is necessary to assure the offsets of these fields
> >   */
> >  struct rte_kni_mbuf {
> > -	void *pool;
> >  	void *buf_addr;
> > -	char pad0[16];
> > -	void *next;
> > +	char pad0[10];
> >  	uint16_t data_off;      /**< Start address of data in segment buffer. */
> > +	char pad1[4];
> > +	uint16_t ol_flags;      /**< Offload features. */
> > +	char pad2[8];
> >  	uint16_t data_len;      /**< Amount of data in segment buffer. */
> >  	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len.
> > */
> > -	char pad2[4];
> > -	uint16_t ol_flags;      /**< Offload features. */
> > +	char pad3[8];
> > +	void *pool;
> > +	void *next;
> >  } __attribute__((__aligned__(64)));
> >
> >  /*
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index
> > 71080e5..413a5a1 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -115,16 +115,12 @@ extern "C" {
> >   * The generic rte_mbuf, containing a packet mbuf.
> >   */
> >  struct rte_mbuf {
> > -	struct rte_mempool *pool; /**< Pool from which mbuf was allocated.
> */
> >  	void *buf_addr;           /**< Virtual address of segment buffer. */
> >  	phys_addr_t buf_physaddr; /**< Physical address of segment buffer. */
> > -	uint16_t buf_len;         /**< Length of segment buffer. */
> >
> > -	/* valid for any segment */
> > -	struct rte_mbuf *next;    /**< Next segment of scattered packet. */
> > +	/* next 8 bytes are initialised on RX descriptor rearm */
> > +	uint16_t buf_len;         /**< Length of segment buffer. */
> >  	uint16_t data_off;
> > -	uint16_t data_len;        /**< Amount of data in segment buffer. */
> > -	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
> >
> >  #ifdef RTE_MBUF_REFCNT
> >  	/**
> > @@ -142,14 +138,17 @@ struct rte_mbuf {
> >  #else
> >  	uint16_t refcnt_reserved;     /**< Do not use this field */
> >  #endif
> > -	uint16_t reserved;            /**< Unused field. Required for padding
> > */
> > -	uint16_t ol_flags;            /**< Offload features. */
> > -
> > -	/* these fields are valid for first segment only */
> >  	uint8_t nb_segs;        /**< Number of segments. */
> >  	uint8_t port;           /**< Input port. */
> >
> > -	/* offload features, valid for first segment only */
> > +	uint16_t ol_flags;      /**< Offload features. */
> > +	uint16_t reserved0;     /**< Unused field. Required for padding */
> > +	uint32_t reserved1;     /**< Unused field. Required for padding */
> > +
> > +	/* remaining bytes are set on RX when pulling packet from descriptor */
> > +	uint16_t reserved2;     /**< Unused field. Required for padding */
> > +	uint16_t data_len;      /**< Amount of data in segment buffer. */
> > +	uint32_t pkt_len;       /**< Total pkt len: sum of all segments. */
> >  	union {
> >  		uint16_t l2_l3_len; /**< combined l2/l3 lengths as single var */
> >  		struct {
> > @@ -167,6 +166,10 @@ struct rte_mbuf {
> >  		uint32_t sched;     /**< Hierarchical scheduler */
> >  	} hash;                 /**< hash information */
> >
> > +	/* fields only used in slow path or on TX */
> > +	struct rte_mempool *pool; /**< Pool from which mbuf was allocated.
> */
> > +	struct rte_mbuf *next;    /**< Next segment of scattered packet. */
> > +
> >  	union {
> >  		uint8_t metadata[0];
> >  		uint16_t metadata16[0];
> > --
> > 1.9.3
> 
> It seems that you removed the inner_l3_len and inner_l2_len fields from the
> rte_mbuf.h structure, what is the reason?
> 
> /Jijiang Liu

No, those fields were never added to the mbuf, they were simply proposed as a new field addition in an RFC patch set. The adding of new fields to the mbuf is beyond the scope of the two patch sets already submitted. Those patch sets merely reorder the existing l2 and l3 length fields without changing their size or name.

/Bruce