Re: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags

["Wang, Haiyue" <haiyue.wang@intel.com>]

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Thursday, July 11, 2019 16:21
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags
> 
> On Thu, Jul 11, 2019 at 08:04:00AM +0000, Wang, Haiyue wrote:
> > > -----Original Message-----
> > > From: Olivier Matz [mailto:olivier.matz@6wind.com]
> > > Sent: Thursday, July 11, 2019 15:26
> > > To: Wang, Haiyue <haiyue.wang@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags
> > >
> > > Hi,
> > >
> > > On Wed, Jul 10, 2019 at 05:14:33PM +0000, Wang, Haiyue wrote:
> > > > Hi,
> > > >
> > > > Sounds cool, just have some questions inline.
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > > > > Sent: Wednesday, July 10, 2019 17:29
> > > > > To: dev@dpdk.org
> > > > > Subject: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags
> > > > >
> > > > > Many features require to store data inside the mbuf. As the room in mbuf
> > > > > structure is limited, it is not possible to have a field for each
> > > > > feature. Also, changing fields in the mbuf structure can break the API
> > > > > or ABI.
> > > > >
> > > > > This commit addresses these issues, by enabling the dynamic registration
> > > > > of fields or flags:
> > > > >
> > > > > - a dynamic field is a named area in the rte_mbuf structure, with a
> > > > >   given size (>= 1 byte) and alignment constraint.
> > > > > - a dynamic flag is a named bit in the rte_mbuf structure.
> > > > >
> > > > > The typical use case is a PMD that registers space for an offload
> > > > > feature, when the application requests to enable this feature.  As
> > > > > the space in mbuf is limited, the space should only be reserved if it
> > > > > is going to be used (i.e when the application explicitly asks for it).
> > > > >
> > > > > The registration can be done at any moment, but it is not possible
> > > > > to unregister fields or flags for now.
> > > > >
> > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > >
> > > (...)
> > >
> > > > > +/**
> > > > > + * @file
> > > > > + * RTE Mbuf dynamic fields and flags
> > > > > + *
> > > > > + * Many features require to store data inside the mbuf. As the room in
> > > > > + * mbuf structure is limited, it is not possible to have a field for
> > > > > + * each feature. Also, changing fields in the mbuf structure can break
> > > > > + * the API or ABI.
> > > > > + *
> > > > > + * This module addresses this issue, by enabling the dynamic
> > > > > + * registration of fields or flags:
> > > > > + *
> > > > > + * - a dynamic field is a named area in the rte_mbuf structure, with a
> > > > > + *   given size (>= 1 byte) and alignment constraint.
> > > > > + * - a dynamic flag is a named bit in the rte_mbuf structure.
> > > > > + *
> > > > > + * The typical use case is a PMD that registers space for an offload
> > > > > + * feature, when the application requests to enable this feature.  As
> > > > > + * the space in mbuf is limited, the space should only be reserved if it
> > > > > + * is going to be used (i.e when the application explicitly asks for it).
> > > > > + *
> > > > > + * The registration can be done at any moment, but it is not possible
> > > > > + * to unregister fields or flags for now.
> > > > > + *
> > > > > + * Example of use:
> > > > > + *
> > > > > + * - RTE_MBUF_DYN_<feature>_(ID|SIZE|ALIGN) are defined in this file
> > > >
> > > > Does it means that all PMDs define their own 'RTE_MBUF_DYN_<feature>_(ID|SIZE|ALIGN)'
> > > > here ? In other words, each PMD can expose its private DYN_<feature> here for public
> > > > using ?
> > >
> > > For generic fields, I think they should be declared in this file. For
> > > instance, if we decide to replace the current m->timestamp field by a
> > > dynamic field, we should add like this:
> > >
> > > #define RTE_MBUF_DYN_TIMESTAMP_ID "rte_timestamp"
> > > #define RTE_MBUF_DYN_TIMESTAMP_SIZE sizeof(uint64_t)
> > > #define RTE_MBUF_DYN_TIMESTAMP_ALIGN __alignof__(uint64_t)
> > >
> > > If the feature is PMD-specific, the defines could be exposed in a
> > > PMD header.
> > >
> >
> > Now, understand the comments a little : ... must not define identifers prefixed with "rte_",
> > which are reserved for standard features. Seems have big plan ?
> 
> The dynamic field can also be used by an external application or by an
> external library. For instance, a field to tag a packet, like skb->mark
> in linux. In this case, id, size and alignment would be defined outside
> dpdk subtree.
> 
> To avoid name conflicts, I think we should define a convention for
> identifiers, so they are in different namespaces:
> 
> - "rte_*" for identifiers declared inside dpdk subtree
> - any other name for identifiers declared in an external application or
>   library

Very clearer now, thanks, this convention can be in programming guide document. :)