Re: [dpdk-dev] [PATCH] mbuf: make rearm_data address naturally aligned

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

2016-05-19 19:05, Jerin Jacob:
> On Thu, May 19, 2016 at 12:18:57PM +0000, Ananyev, Konstantin wrote:
> > > On Thu, May 19, 2016 at 12:20:16AM +0530, Jerin Jacob wrote:
> > > > On Wed, May 18, 2016 at 05:43:00PM +0100, Bruce Richardson wrote:
> > > > > On Wed, May 18, 2016 at 07:27:43PM +0530, Jerin Jacob wrote:
> > I wonder does anyone really use mbuf port field?
> > My though was - could we to drop it completely?
> > Actually, after discussing it with Bruce offline, an interesting idea came out:
> > if we'll drop port and make mbuf_prefree() to reset nb_segs=1, then
> > we can reduce RX rearm_data to 4B. So with that layout:
> > 
> > struct rte_mbuf {
> > 
> >          MARKER cacheline0;
> > 
> >         void *buf_addr;           
> >         phys_addr_t buf_physaddr; 
> >         uint16_t buf_len;
> >         uint8_t nb_segs;
> >         uint8_t reserved_1byte;   /* former port */
> >         
> >         MARKER32 rearm_data;
> >         uint16_t data_off;
> >        uint16_t refcnt;
> >        
> >         uint64_t ol_flags;
> >         ...
> > 
> > We can keep buf_len at its place and avoid 2B gap, while making rearm_data
> > 4B long and 4B aligned.
> 
> Couple of comments,
> - IMO, It is good if nb_segs can move under rearm_data, as some
> drivers(not in ixgbe may be) can write nb_segs in one shot also
> in segmented rx handler case
> - I think, it makes sense to keep port in mbuf so that application
> can make use of it(Not sure what real application developers think of
> this)

I agree we could try to remove the port id from mbuf.
These mbuf data are a common base to pass infos between drivers and apps.
If you need to store some data which are read and write from the app only,
you can have use the private zone (see rte_pktmbuf_priv_size).

> - if Writing 4B and 8B consume same cycles(at least in arm64) then I think it
> makes sense to make it as 8B wide with maximum pre-built constants are possible.
> 
> > 
> > Another similar alternative, is to make mbuf_prefree() to set refcnt=1
> > (as it update it anyway). Then we can remove refcnt from the RX rearm_data,
> > and again make rearm_data 4B long and 4B aligned:
> > 
> > struct rte_mbuf {
> > 
> >          MARKER cacheline0;
> > 
> >         void *buf_addr;           
> >         phys_addr_t buf_physaddr; 
> >         uint16_t buf_len;
> >         uint16_t refcnt;
> > 
> >         MARKER32 rearm_data;
> >         uint16_t data_off;
> >         uint8_t nb_segs;
> >         uint8_t port;
> 
> The only problem I think with this approach is that, port data type cannot be
> extended to uint16_t in future.

It is a major problem. The port id should be extended to uint16_t.