[dpdk-dev] [PATCH] mbuf: rename deprecated VLAN flags

Ferruh Yigit ferruh.yigit at intel.com
Tue Oct 24 04:08:25 CEST 2017


On 10/23/2017 5:16 AM, Olivier Matz wrote:
> PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated for a while.
> As explained in [1], these flags were kept to let the applications and
> PMDs move to the new flag. There is also a need to support Rx vlan
> offload without vlan strip (at least for the ixgbe driver).
> 
> This patch renames the old flags for this feature, knowing that some
> PMDs were using PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT to indicate that
> the vlan tci has been saved in the mbuf structure.

So this patch merges two steps,
- remove old flags
- Add new flag to separate stripped and saved vlan info.

Overall looks like a good idea.

Just to confirm, in old usage, "PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED",
both were meaning same thing, right. And "PKT_RX_VLAN_PKT" kept for backward
compatibility.
So previous "PKT_RX_VLAN_PKT" was way to say "stripped and saved", converting it
to "PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED" as done in this patch should be safe.
Only may be missing cases that only "PKT_RX_VLAN" is valid.

And is "PKT_RX_VLAN_STRIPPED" implies the "PKT_RX_VLAN"? This was mentioned is
the initial proposal, but it looks like that is not the case in this patch.

> It is likely that some PMDs do not set the proper flags when doing vlan
> offload, and it would be worth making a pass on all of them.

This is the worrying statement :)

> 
> Link: [1] http://dpdk.org/ml/archives/dev/2017-June/067712.html
> Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
> ---
> 
> An alternative would have been to keep PKT_RX_VLAN_PKT and
> PKT_RX_QINQ_PKT and recycle them for this new feature.

I think it will be confusing to use same names.

> 
> I choosed this way (rename the flags) because:
> - changing the name is a good way to highlight that something
>   changed

+1

> - the old name was not that good (PKT_blabla_PKT)
> - there was no reaction to the proposition in [1]
> 
> I'm also open to recycle the old name if we find good reasons
> for it. It would make the patch smaller since there would be no
> modification in drivers.
> 
> In both cases, it is important that PMD maintainers check
> that their use of VLAN flags is correct. Example:
> - vlan is not stripped and tci is saved: PKT_RX_VLAN
> - vlan is stripped and tci is saved: PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED
> This can be completed by the packet type which describes the
> packet data. I hope the API documentation is clear enough.
> 
> Ferruh, do you think you could check with PMD maintainers for next
> version?

Sure I can try, but according current experience there is no efficient way to do
this right now.

> This patch has no impact on applications that do not
> use the deprecated flags.

I guess this is only true if "PKT_RX_VLAN_STRIPPED" implies the "PKT_RX_VLAN"

> For other applications, renaming
> the flag would restore the same behavior. 

Not sure, for an old application, using "PKT_RX_VLAN_PKT", can't just rename to
"PKT_RX_VLAN" and use as it is because now it doesn't say if vlan stripped or
not. So old applications seems may be effected, if so this may require to be
notified in advance.

> But, since we are
> close to the release, applying it early after the release could
> also be considered.

Is there any benefit to be in this release, one think I can think of is 17.11
being LTS, is there any other?

And what do you think doing in two steps,
- in this release remove deprecated flags
- in the beginning of the next release introduce the new flags

Thanks,
ferruh

<...>



More information about the dev mailing list