Re: [PATCH v4 1/2] mbuf: support attaching external buffer to mbuf

[Thomas Monjalon <thomas@monjalon.net>]

24/04/2018 21:15, Olivier Matz:
> On Tue, Apr 24, 2018 at 09:21:00PM +0300, Andrew Rybchenko wrote:
> > On 04/24/2018 07:02 PM, Olivier Matz wrote:
> > > On Tue, Apr 24, 2018 at 03:28:33PM +0300, Andrew Rybchenko wrote:
> > > > On 04/24/2018 04:38 AM, Yongseok Koh wrote:
> > > > > + * Returns TRUE if given mbuf is cloned by mbuf indirection, or FALSE
> > > > > + * otherwise.
> > > > > + *
> > > > > + * If a mbuf has its data in another mbuf and references it by mbuf
> > > > > + * indirection, this mbuf can be defined as a cloned mbuf.
> > > > > + */
> > > > > +#define RTE_MBUF_CLONED(mb)     ((mb)->ol_flags & IND_ATTACHED_MBUF)
> > > > > +
> > > > > +/**
> > > > >     * Returns TRUE if given mbuf is indirect, or FALSE otherwise.
> > > > >     */
> > > > > -#define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
> > > > > +#define RTE_MBUF_INDIRECT(mb)   RTE_MBUF_CLONED(mb)
> > > > It is still confusing that INDIRECT != !DIRECT.
> > > > May be we have no good options right now, but I'd suggest to at least
> > > > deprecate
> > > > RTE_MBUF_INDIRECT() and completely remove it in the next release.
> > > Agree. I may have missed something, but is my previous suggestion
> > > not doable?
> > > 
> > > - direct = embeds its own data      (and indirect = !direct)
> > > - clone (or another name) = data is another mbuf
> > > - extbuf = data is in an external buffer
> > 
> > I guess the problem that it changes INDIRECT semantics since EXTBUF
> > is added as well. I think strictly speaking it is an API change.
> > Is it OK to make it without announcement?
> 
> In any case, there will be an ABI change, because an application
> compiled for 18.02 will not be able to handle these new kind of
> mbuf.
> 
> So unfortunatly yes, I think this kind of changes should first be
> announced.
> 
> Thomas, what do you think?

What is the impact for the application developer?
Is there something to change in the application after this patch?