[dpdk-dev] [PATCH 1/4] mbuf: add accessor function for private data area

Andrew Rybchenko arybchenko at solarflare.com
Sat Jun 9 11:23:21 CEST 2018


Hi Dan,

On 06/09/2018 03:24 AM, Dan Gora wrote:
>>>> +{
>>>> +    if (md->priv_size == 0)
>>>> +            return NULL;
>>>> +
>>>> +    return RTE_PTR_ADD(md, sizeof(struct rte_mbuf));
>>> Also a nit...
>>> I'd use sizeof(*md) (or sizeof(*m) in fact as described above) here.
>>> At least previous functions do it in such way.
>> I believe the sizeof(struct rte_mbuf) is much more readable then sizeof(*m) it makes the reader have to look up what ‘m’ is defined to. I know this is a small function, but readability is still a good reason to not use sizeof(*m) IMO.
> On one hand, using sizeof(*m) is useful in case the type of 'm' ever
> changes, you don't have to remember to change the arguments to sizeof.
> On the other hand, it does make it slightly harder to read, but not a
> lot really.
>
> For me, it's six one way, half a dozen the other.   I just cut-pasted
> this from the ipsec-secgw code.  I'm kind of inclined to leave it
> sizeof(struct rte_mbuf) just to leave it clear.

OK, I agree.

> Any opinion on my question from the cover letter?

Sorry, I was going to reply as I understand it, but forgot.

> Specifically when should rte_mbuf_XXX be used vs rte_pktmbuf_XXX for
> mbuf API functions?  Why is there this inconsistency there?  Are they
> just historical names which couldn't get changed?

I think that Olivier is best placed to answer it.
As I understand it is mainly historical right now, since ctrlmbuf API was
removed recently. For me, there is still a flavour of packet head in 
pktmbuf,
but boundaries are so vague.

> One more quick question:
>
> When sending a v2 of a patch series, should I resend the whole bundle,
> even if there are no changes in the other patches or just send a v2 of
> the patch which actually contains changes from the v1 version?

All patches should be resent in v2.

BTW, thinking about function I found out there is a trap in private area
size related to the function. I think that the function description should
highlight that rte_pktmbuf_priv_size(m->pool) should be used to
find out the size of private area since indirect mbuf has size of the
direct private are in its priv_size (but we return pointer to the indirect
mbuf (the mbuf itself) private area here).

Andrew.


More information about the dev mailing list