Re: PKT_RX_VLAN_PKT when VLAN stripping is disabled

["John Daley (johndale)" <johndale@cisco.com>]

Hi Olivier and Ananyev,

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?

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*.

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?

*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.

-john

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, April 25, 2016 6:51 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; John Daley
> (johndale) <johndale@cisco.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] PKT_RX_VLAN_PKT when VLAN stripping is disabled
> 
> Hi,
> 
> On 04/25/2016 02:02 PM, Ananyev, Konstantin wrote:
> > Hi John,
> > From rte_mbuf.h:
> > #define PKT_RX_VLAN_PKT      (1ULL << 0)  /**< RX packet is a 802.1q VLAN
> packet. */
> > So yes, in theory it should be set up for vlan packet with both stripping
> on/off.
> > The problem is that (as far as I know) when VLAN stripping is disabled
> > FVL RXD doesn't contain information does that packet contain a VLAN or
> not.
> > Don't really know what is the best option in that case: keep things as
> > it is or change the meaning of the VLAN_PKT flag to indicate is
> mbuf.vlan_tci field is valid or not.
> > Konstantin
> 
> 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?
> 
> 
> 
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of John Daley
> >> (johndale)
> >> Sent: Friday, April 22, 2016 12:37 AM
> >> To: dev@dpdk.org
> >> Subject: [dpdk-dev] PKT_RX_VLAN_PKT when VLAN stripping is disabled
> >>
> >> Hi,
> >>
> >> When VLAN stripping is disabled, X710 and 82599ES act differently for
> >> me in this case when receiving VLAN tagged packets. On 82599ES the flag
> is set but on X710 the flag not set.
> >>
> >> Do I maybe have old X710 firmware? Or is it not set for X710 on
> >> purpose in this case and instead the flag is used to indicate if vlan_tci is
> valid? Right now the enic pmd does what my X710 does, which I think is
> incorrect and I want to fix it.
> >>
> >> Thanks,
> >> John
> >