Re: [dpdk-dev] [PATCH 09/15] net/vmxnet3: switch MSS hint to dynamic mbuf field

[Thomas Monjalon <thomas@monjalon.net>]

26/10/2020 17:50, Thomas Monjalon:
> 26/10/2020 16:21, Andrew Rybchenko:
> > On 10/26/20 6:14 PM, Andrew Rybchenko wrote:
> > > On 10/26/20 8:20 AM, Thomas Monjalon wrote:
> > >> -			rxm->udata64 = rcde->segCnt;
> > >> +			*RTE_MBUF_DYNFIELD(rxm, vmxnet3_segs_dynfield_offset,
> > >> +					uint8_t *) = rcde->segCnt;
> > > 
> > > I think it should be a rule of thumb to introduce helper
> > > macro to access a dynamic field (as you do in few of
> > > previous patches).
> > > 
> > > It should be just nearby declaration of the the offset
> > > variable.
> > 
> > May be inline function is even better since, IMHO, if you
> > both ways are possible, inline function is the right choice.
> > In this particular case inline function will not add value
> > from type safety point of view, but it is still better as
> > an example to follow. In some case inline function could be
> > used as a place to put build assertion to check size.
> 
> OK I will review the patches to provide a static inline getter function
> for each dynamic field.

After more thoughts, I think it is good to have static inline
returning a pointer to the field in general case:

static inline my_type *
rte_foo_dynfield_bar(struct rte_mbuf * mbuf)
{
	return RTE_MBUF_DYNFIELD(mbuf, my_dynfield_offset, my_type *);
}

But when the scope is limited, it is easier to read a macro
which contains dereferencing of the field pointer with the right type:

#define MY_DYNFIELD(mbuf) \
	(*RTE_MBUF_DYNFIELD(mbuf, my_dynfield_offset, my_type *))

If type cast is never needed, this form allows simple assignment:
	MY_DYNFIELD(mbuf) = field_value;

I would tend to use this latter form for tests or small apps.