Re: [dpdk-dev] [PATCH v6 1/2] mbuf: support attaching external buffer to mbuf

[Olivier Matz <olivier.matz@6wind.com>]

On Thu, Apr 26, 2018 at 10:18:14AM -0700, Yongseok Koh wrote:
>

[...]

> > >   /** Mbuf prefetch */
> > >   #define RTE_MBUF_PREFETCH_TO_FREE(m) do {       \
> > >   	if ((m) != NULL)                        \
> > > @@ -1213,11 +1307,157 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
> > >   }
> > >   /**
> > > + * Initialize shared data at the end of an external buffer before attaching
> > > + * to a mbuf by ``rte_pktmbuf_attach_extbuf()``. This is not a mandatory
> > > + * initialization but a helper function to simply spare a few bytes at the
> > > + * end of the buffer for shared data. If shared data is allocated
> > > + * separately, this should not be called but application has to properly
> > > + * initialize the shared data according to its need.
> > > + *
> > > + * Free callback and its argument is saved and the refcnt is set to 1.
> > > + *
> > > + * @warning
> > > + * buf_len must be adjusted to RTE_PTR_DIFF(shinfo, buf_addr) after this
> > > + * initialization. For example,
> > 
> > May be buf_len should be inout and it should be done by the function?
> > Just a question since current approach looks fragile.
> 
> Yeah, I thought about that but I didn't want to alter user's variable, I thought
> it could be error-prone. Anyway either way is okay to me. Will wait for a day to
> get input because I will send out a new version (hopefully last :-) to fix the
> nit you mentioned below.

+1, I had exactly the same comment than Andrew in mind.
To me, it looks better to have buf_len as in/out.

I don't think it's a problem to have this change for rc2.

So,
Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thank you Yongseok for this nice improvement.