Re: [dpdk-dev] [PATCH RFC 06/11] mbuf: replace data pointer by an offset

[Thomas Monjalon <thomas.monjalon@6wind.com>]

Hi Olivier,

2014-05-09 16:50, Olivier Matz:
> The mbuf structure already contains a pointer to the beginning of the
> buffer (m->buf_addr). It is not needed to use 8 bytes again to store
> another pointer to the beginning of the data.
> 
> Using a 16 bits unsigned integer is enough as we know that a mbuf is
> never longer than 64KB. We gain 6 bytes in the structure thanks to
> this modification.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
[...]
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -132,6 +132,13 @@ struct rte_mbuf {
>  	void *buf_addr;           /**< Virtual address of segment buffer. */
>  	uint64_t buf_physaddr:48; /**< Physical address of segment buffer. */
>  	uint64_t buf_len:16;      /**< Length of segment buffer. */
> +
> +	/* valid for any segment */
> +	struct rte_mbuf *next;    /**< Next segment of scattered packet. */
> +	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
>  	/**
>  	 * 16-bit Reference counter.
> @@ -142,36 +149,30 @@ struct rte_mbuf {
>  	 * config option.
>  	 */
>  	union {
> -		rte_atomic16_t refcnt_atomic;   /**< Atomically accessed refcnt */
> -		uint16_t refcnt;                /**< Non-atomically accessed refcnt 
*/
> +		rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
> +		uint16_t refcnt;  /**< Non-atomically accessed refcnt */
>  	};
>  #else
> -	uint16_t refcnt_reserved;     /**< Do not use this field */
> +	uint16_t refcnt_reserved; /**< Do not use this field */
>  #endif
> 
> -	uint16_t ol_flags;            /**< Offload features. */
> -	uint32_t reserved;             /**< Unused field. Required for padding. 
*/
> -
> -	/* valid for any segment */
> -	struct rte_mbuf *next;  /**< Next segment of scattered packet. */
> -	void* data;             /**< Start address of data in segment buffer. */
> -	uint16_t data_len;      /**< Amount of data in segment buffer. */
> -
>  	/* these fields are valid for first segment only */
> -	uint8_t nb_segs;        /**< Number of segments. */
> -	uint8_t in_port;        /**< Input port. */
> -	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len.
> */ +	uint8_t nb_segs;          /**< Number of segments. */
> +	uint8_t in_port;          /**< Input port. */
> +	uint16_t ol_flags;        /**< Offload features. */
> +	uint16_t reserved;        /**< Unused field. Required for padding. */
> 
>  	/* offload features, valid for first segment only */
>  	union rte_vlan_macip vlan_macip;
>  	union {
> -		uint32_t rss;       /**< RSS hash result if RSS enabled */
> +		uint32_t rss;     /**< RSS hash result if RSS enabled */
>  		struct {
>  			uint16_t hash;
>  			uint16_t id;
> -		} fdir;             /**< Filter identifier if FDIR enabled */
> -		uint32_t sched;     /**< Hierarchical scheduler */
> -	} hash;                 /**< hash information */
> +		} fdir;           /**< Filter identifier if FDIR enabled */
> +		uint32_t sched;   /**< Hierarchical scheduler */
> +	} hash;                   /**< hash information */
> +	uint64_t reserved2;       /**< Unused field. Required for padding. */
>  } __rte_cache_aligned;

There are some cosmetic changes mixed with real changes.
It make hard to read them.
Please split this patch.

-- 
Thomas