[dpdk-dev] PKT_RX_VLAN_PKT when VLAN stripping is disabled

[Olivier MATZ]

Hi John,

(adding some PMD maintainers in the loop)

> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
>> It seems the meaning of the PKT_RX_VLAN_PKT bit depends on the port
>> configuration:
>> - if vlan stripping is configured, it means VLAN is present in vlan_tci
>>    mbuf field
>> - if not configured, it means a VLAN is present in the packet
>>
>> I don't think this is a good behavior since the application has to know the port
>> configuration to properly interpret the meaning of the flag.
>>
>> I suggest to change the meaning of this flag to: "vlan was stripped by
>> hardware, and vlan tag is now located in m->vlan_tci".
>>
>> The same could apply to PKT_RX_QINQ_PKT and m->outer_vlan_tci.
>>
>> We could add a new packet_type to tell if the mbuf is a VLAN/QinQ is
>> detected in the packet but not stripped.
>>
>> Example:
>>
>> - vlan stripping is disabled
>>
>>    - vlan packet recvd: flags=0, ptype=RTE_PTYPE_L2_ETHER_VLAN
>>    - qinq packet recvd: flags=0, ptype=RTE_PTYPE_L2_ETHER_QINQ
>>
>> - vlan stripping is enabled
>>
>>    - vlan packet recvd: flags=PKT_RX_VLAN_PKT,
>> ptype=RTE_PTYPE_L2_ETHER,
>>          m->vlan_tci=id
>>    - qinq packet recvd: flags=PKT_RX_VLAN_PKT|PKT_RX_QINQ_PKT,
>>          ptype=RTE_PTYPE_L2_ETHER, m->vlan_tci=id, m->vlan_tci_outer=id
>>
>>
>> Thoughts?

On 04/26/2016 02:16 AM, John Daley (johndale) wrote:
> I like the new packet types and how they work the same for VLAN and QINQ. Just
> so I understand your suggestion, X710 (as it seems to work today) would not set
> RTE_PTYPE_L2_ETHER_VLAN in dev_supported_ptypes_get() because it does not know
> how to determine that packet type in the receive path if stripping is disabled?
> But if stripping was enabled, the application could still trust m->vlan_tci if
> the flag was set?

Well, this depends on the exact meaning of the supported_ptype flags:
does it mean that the hw/driver *must* recognize and set the
ptype or does it mean that the hw/driver *can* recognize and set the
ptype?

In case a packet with 2 vlans is received, if the hw/driver is able
to strip the first one and recognize the second one, I would say
that RTE_PTYPE_L2_ETHER_VLAN should be set in the mbuf flags.


> Re changing the meaning of the PKT_RX_VLAN_PKT flag- I think it could cause hard
> to find errors and confusion. I would rather see the flag deprecated and a new
> one defined. Perhaps the flag could be called PKT_RX_VLAN_STRIPPED*.

Yes, good idea.


> Maybe another less elegant but more compatible solution would be just keep the
> Niantic behavior and fix other pmd's to match its behavior. For X710, with vlan
> stripping disabled this might mean looking at each packet's Ethernet type and
> set the flag accordingly.  It might not be too expensive since Ethernet type is
> in the 1st cacheline and hopefully prefetched. Thoughts?

I don't think the ixgbe behavior is correct. Today, depending on the
configuration (vlan stripped or not), the meaning of the flag is
different. This is not useable in an application that may use several
ports with different configurations.


> *In the future perhaps another flag could be added called
> PKT_RX_VLAN_TCI_VALID. This may not be the same as PKT_RX_VLAN_STRIPPED- enic
> and maybe some other nics are able to set vlan_tci even when not stripping vlan
> tags and this feature could be exposed with this separate flag.

At first read, I would say it's a bit overkill because it will
complexify the combinatorial dimension of the offload flags. But
if you agree, let's say it's another topic and focus on the fix
of PKT_RX_VLAN_PKT first.


Thanks,
Olivier


PS: please avoid top-posting, it's harder to read in a thread