Hi Ferruh,
On Mon, Oct 23, 2017 at 07:08:25PM -0700, Ferruh Yigit wrote:
> 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.
Before this patch:
- PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED: stripped and saved
- PKT_RX_VLAN_STRIPPED: unused, but also means stripped and saved
- PKT_RX_VLAN_PKT: undefined, but present in several drivers. In some of
them, I think it means 'not stripped and saved'.
After this patch:
- PKT_RX_VLAN: not stripped and saved
- PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED: stripped and saved
- PKT_RX_VLAN_STRIPPED: same (stripped and saved), PKT_RX_VLAN is implied
I think we can forbid to have this flag alone.
> 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.
Yes, I can clarify it in the documentation.
> [...]
> > 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"
>From what I see, currently PKT_RX_VLAN_STRIPPED is always used
with PKT_RX_VLAN_PKT in PMDs, so I think there will be not impact.
> > 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.
Old applications using PKT_RX_VLAN_PKT have been notified
since 16.11 that the flag is deprecated.
If the application is relying on the undocumented fact that
PKT_RX_VLAN_PKT means "not stripped and saved" on some PMDs, it will
continue to work as is after renaming.
> > 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
I think one good side effect of having only one patch is that
it doesn't break applications that were using an undocumented
behavior of a PMD (it just requires a rename).
Olivier
