From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH 07/13] mbuf: use macros only to access the mbuf metadata Date: Wed, 17 Sep 2014 16:01:37 +0200 Message-ID: <3536812.LpadffOgB2@xps13> References: <1409759378-10113-1-git-send-email-bruce.richardson@intel.com> <682698A055A0F44AA47192B20141149711B1FFE6@BGSMSX102.gar.corp.intel.com> <59AF69C657FD0841A61C55336867B5B0343F2BD2@IRSMSX103.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev-VfR2kkLFssw@public.gmane.org To: "Richardson, Bruce" Return-path: In-Reply-To: <59AF69C657FD0841A61C55336867B5B0343F2BD2-kPTMFJFq+rELt2AQoY/u9bfspsVTdybXVpNB7YpNyf8@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" 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