Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the mbuf metadata

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

2014-09-17 10:31, Richardson, Bruce:
> > From: Ramia, Kannan Babu
> > 
> > I completely agree with Cristian here, instead of leaving to applications
> > where to place their meta data, we can provide a guidance by having this
> > field about placement of application meta while maintaining transparency
> > on the contents of application meta information.
> 
> My opinion on this is that this is better served via documentation or a
> comment in the code. The reason is that this approach is not going to be
> suitable for all applications. The mbuf headroom being used by the metadata
> is actually designed to be used for any additional headers to be added to
> the packet - though other things can obviously be stored in it too.
> Therefore the amount of metadata that can be stored in it will depend from
> application to application, as any apps doing e.g. tunnelling will need the
> headroom for tunnelling headers and may only be able to store a small
> amount of metadata - potentially none. For larger amounts of metadata - I
> would feel that anything over 64-bytes or so - I have proposed adding in a
> separate userdata pointer in the mbuf structure so that apps have the
> option of storing the metadata externally e.g. pointing to a flow table
> entry or similar. [Please see mbuf rework patch set 3 proposal]. Because of
> this, I think it's better to put in a comment in the code indicating that
> metadata can go in the headroom, document this properly - including caveats
> and limitations - in the documentation, and provide an example of doing
> such - something we already have in the packet framework.

I agree that replacing these markers by documentation give more accurate
informations and (bonus) is simpler.
When documentation will be embedded in the git repository, I'd like to see
a patch to document these mbuf usages.

> All that being said, and while I think this is a good patch, I don't feel
> too strongly about it. I'm happy enough if this particular patch does not
> get merged in for 1.8, as it's incidental to the overall mbuf changes.

I think also it's a good patch so I keep it.

-- 
Thomas