PKT_RX_VLAN_PKT when VLAN stripping is disabled

PKT_RX_VLAN_PKT when VLAN stripping is disabled

[John Daley (johndale)]

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

Re: PKT_RX_VLAN_PKT when VLAN stripping is disabled

[Ananyev, Konstantin]

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 

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

Re: PKT_RX_VLAN_PKT when VLAN stripping is disabled

[Olivier Matz]

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
> 

Re: PKT_RX_VLAN_PKT when VLAN stripping is disabled

[Ananyev, Konstantin]

Hi Olivier,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, April 25, 2016 2:51 PM
> To: Ananyev, Konstantin; 'John Daley (johndale)'; 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?

Sounds like a reasonable change to me.
Konstantin


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

Re: PKT_RX_VLAN_PKT when VLAN stripping is disabled

[John Daley (johndale)]

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

Re: 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@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

[RFC] mbuf: new flag when vlan is stripped

[Olivier Matz]

The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
PMDs not advertising the same flags in similar conditions.

Following discussion in http://dpdk.org/ml/archives/dev/2016-April/037837.html,
introduce 2 new flags PKT_RX_VLAN_STRIPPED and PKT_RX_QINQ_STRIPPED
that are better defined:

  PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
  tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
  is enabled in the RX configuration of the PMD.

For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
It should be removed from applications and PMDs in a future revision.

This patch also try to fix the drivers. For PKT_RX_VLAN_PKT:

- e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
  had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
  required.
- fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
  PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
- ixgbe: modification done for standard mode (vector does not support
  vlan stripping)
- the other drivers do not support vlan stripping.

For PKT_RX_QINQ_PKT, it was only supported on i40e, and the meaning was
already correct, so we can reuse the same value for PKT_RX_QINQ_STRIPPED.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

Hi,

This is a draft patch that implements what was previously discussed,
except the packet_type, which does not exist for vlan today (and I
think it is not mandatory for now, let's do it in another patch).

After doing this patch, it appeared that ixgbe was the only driver that
had a different behavior for the PKT_RX_VLAN_PKT flag. An alternative
to this patch would be to only change the behavior of the ixgbe driver,
and just document better document the PKT_RX_VLAN_PKT flags in
rte_mbuf.h without adding new flags. I think this is a better option.

Comments are welcome.


This patch is tested on ixgbe and igb (82575):

  # we use scapy to send packets like this:
  # Ether(src="00:01:02:03:04:05", dst="00:1B:21:AB:8F:10")/Dot1Q(vlan=0x666)/IP()/UDP()/Raw("x"*32)

  cd dpdk.org/
  make config T=x86_64-native-linuxapp-gcc
  make -j32
  mkdir -p /mnt/huge
  mount -t hugetlbfs nodev /mnt/huge
  echo 256 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
  modprobe uio_pci_generic

  # test-pmd is started with vlan stripping (default)
  ./build/app/testpmd -l 2,4 -- --total-num-mbufs=65536 -i --port-topology=chained --enable-rx-cksum --disable-hw-vlan-filter
  # and without:
  ./build/app/testpmd -l 2,4 -- --total-num-mbufs=65536 -i --port-topology=chained --enable-rx-cksum --disable-hw-vlan-filter --disable-hw-vlan-strip

  # we run test-pmd in rxonly mode, displaying the packet information.
  set fwd rxonly
  set verbose 1
  start


  # ixgbe: the behavior of the flag PKT_RX_VLAN_PKT is kept as before,
  # and the new flag PKT_RX_VLAN_STRIPPED is introduced when vlan stripping
  # is enabled and a vlan is stripped.

  # ixgbe, before patch, with vlan stripping
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
  PKT_RX_VLAN_PKT

  # ixgbe, before patch, without vlan stripping
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - VLAN tci=0x0 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
  PKT_RX_VLAN_PKT

  # ixgbe, after patch, with vlan stripping
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED

  # ixgbe, after patch, without vlan stripping
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT


  # igb: the new flag PKT_RX_VLAN_STRIPPED is added in addition to PKT_RX_VLAN_PKT.

  # igb: before patch, with vlan stripping
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

  # igb: before patch, without vlan stripping
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT

  # igb: after patch, with vlan stripping
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

  # igb: after patch, without vlan stripping
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED


Thanks,
Olivier



 app/test-pmd/rxonly.c              |  4 ++--
 drivers/net/e1000/em_rxtx.c        |  3 ++-
 drivers/net/e1000/igb_rxtx.c       |  3 ++-
 drivers/net/enic/enic_rx.c         |  2 +-
 drivers/net/i40e/i40e_rxtx.c       |  2 +-
 drivers/net/ixgbe/ixgbe_ethdev.c   |  7 ++++++
 drivers/net/ixgbe/ixgbe_rxtx.c     | 20 ++++++++++++----
 drivers/net/ixgbe/ixgbe_rxtx.h     |  1 +
 drivers/net/mlx5/mlx5_rxtx.c       |  6 +++--
 drivers/net/nfp/nfp_net.c          |  2 +-
 drivers/net/vmxnet3/vmxnet3_rxtx.c |  2 +-
 lib/librte_mbuf/rte_mbuf.c         |  2 ++
 lib/librte_mbuf/rte_mbuf.h         | 47 ++++++++++++++++++++++++++++++++++----
 13 files changed, 81 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index 14555ab..c69b344 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -156,9 +156,9 @@ pkt_burst_receive(struct fwd_stream *fs)
 				printf("hash=0x%x ID=0x%x ",
 				       mb->hash.fdir.hash, mb->hash.fdir.id);
 		}
-		if (ol_flags & PKT_RX_VLAN_PKT)
+		if (ol_flags & PKT_RX_VLAN_STRIPPED)
 			printf(" - VLAN tci=0x%x", mb->vlan_tci);
-		if (ol_flags & PKT_RX_QINQ_PKT)
+		if (ol_flags & PKT_RX_QINQ_STRIPPED)
 			printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
 					mb->vlan_tci, mb->vlan_tci_outer);
 		if (mb->packet_type) {
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 441ccad..0d1931b 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -639,7 +639,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	uint64_t pkt_flags;
 
 	/* Check if VLAN present */
-	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0);
+	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
+		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
 
 	return pkt_flags;
 }
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 4a987e3..73cfd26 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -739,7 +739,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	uint64_t pkt_flags;
 
 	/* Check if VLAN present */
-	pkt_flags = (rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
+	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
+		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
 
 #if defined(RTE_LIBRTE_IEEE1588)
 	if (rx_status & E1000_RXD_STAT_TMST)
diff --git a/drivers/net/enic/enic_rx.c b/drivers/net/enic/enic_rx.c
index c21ee47..4574fee 100644
--- a/drivers/net/enic/enic_rx.c
+++ b/drivers/net/enic/enic_rx.c
@@ -202,7 +202,7 @@ enic_cq_rx_to_pkt_flags(struct cq_desc *cqd, struct rte_mbuf *mbuf)
 
 	/* VLAN stripping */
 	if (bwflags & CQ_ENET_RQ_DESC_FLAGS_VLAN_STRIPPED) {
-		pkt_flags |= PKT_RX_VLAN_PKT;
+		pkt_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		mbuf->vlan_tci = enic_cq_rx_desc_vlan(cqrd);
 	} else {
 		mbuf->vlan_tci = 0;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 699b264..405b26b 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -99,7 +99,7 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union i40e_rx_desc *rxdp)
 #ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
 	if (rte_le_to_cpu_16(rxdp->wb.qword2.ext_status) &
 		(1 << I40E_RX_DESC_EXT_STATUS_L2TAG2P_SHIFT)) {
-		mb->ol_flags |= PKT_RX_QINQ_PKT;
+		mb->ol_flags |= PKT_RX_QINQ_STRIPPED;
 		mb->vlan_tci_outer = mb->vlan_tci;
 		mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_2);
 		PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u",
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a2b170b..e7717e3 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1636,6 +1636,7 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 {
 	struct ixgbe_hwstrip *hwstrip =
 		IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(dev->data->dev_private);
+	struct ixgbe_rx_queue *rxq;
 
 	if (queue >= IXGBE_MAX_RX_QUEUE_NUM)
 		return;
@@ -1644,6 +1645,12 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 		IXGBE_SET_HWSTRIP(hwstrip, queue);
 	else
 		IXGBE_CLEAR_HWSTRIP(hwstrip, queue);
+
+	if (queue >= dev->data->nb_rx_queues)
+		return;
+
+	rxq = dev->data->rx_queues[queue];
+	rxq->vlan_strip = on;
 }
 
 static void
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index ceaf4ab..6f6327e 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1232,16 +1232,23 @@ ixgbe_rxd_pkt_info_to_pkt_flags(uint16_t pkt_info)
 }
 
 static inline uint64_t
-rx_desc_status_to_pkt_flags(uint32_t rx_status)
+rx_desc_status_to_pkt_flags(uint32_t rx_status, uint8_t vlan_strip)
 {
 	uint64_t pkt_flags;
+	uint64_t vlan_flags;
+
+	/* if vlan is stripped, set the proper flag */
+	if (vlan_strip)
+		vlan_flags = PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
+	else
+		vlan_flags = PKT_RX_VLAN_PKT;
 
 	/*
 	 * Check if VLAN present only.
 	 * Do not check whether L3/L4 rx checksum done by NIC or not,
 	 * That can be found from rte_eth_rxmode.hw_ip_checksum flag
 	 */
-	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
+	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  vlan_flags : 0;
 
 #ifdef RTE_LIBRTE_IEEE1588
 	if (rx_status & IXGBE_RXD_STAT_TMST)
@@ -1298,6 +1305,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 	uint32_t pkt_info[LOOK_AHEAD];
 	int i, j, nb_rx = 0;
 	uint32_t status;
+	uint8_t vlan_strip = rxq->vlan_strip;
 
 	/* get references to current descriptor and S/W ring entry */
 	rxdp = &rxq->rx_ring[rxq->rx_tail];
@@ -1339,7 +1347,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 			mb->vlan_tci = rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
 
 			/* convert descriptor fields to rte mbuf flags */
-			pkt_flags = rx_desc_status_to_pkt_flags(s[j]);
+			pkt_flags = rx_desc_status_to_pkt_flags(s[j], vlan_strip);
 			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
 			pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags
 					((uint16_t)pkt_info[j]);
@@ -1555,6 +1563,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	uint16_t nb_rx;
 	uint16_t nb_hold;
 	uint64_t pkt_flags;
+	uint8_t vlan_strip;
 
 	nb_rx = 0;
 	nb_hold = 0;
@@ -1562,6 +1571,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	rx_id = rxq->rx_tail;
 	rx_ring = rxq->rx_ring;
 	sw_ring = rxq->sw_ring;
+	vlan_strip = rxq->vlan_strip;
 	while (nb_rx < nb_pkts) {
 		/*
 		 * The order of operations here is important as the DD status
@@ -1671,7 +1681,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		/* Only valid if PKT_RX_VLAN_PKT set in pkt_flags */
 		rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan);
 
-		pkt_flags = rx_desc_status_to_pkt_flags(staterr);
+		pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_strip);
 		pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr);
 		pkt_flags = pkt_flags |
 			ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
@@ -1764,7 +1774,7 @@ ixgbe_fill_cluster_head_buf(
 	 */
 	head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
 	pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
-	pkt_flags = rx_desc_status_to_pkt_flags(staterr);
+	pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_strip);
 	pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
 	pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 	head->ol_flags = pkt_flags;
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 3691a19..9ca0e8b 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -146,6 +146,7 @@ struct ixgbe_rx_queue {
 	uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
 	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
 	uint8_t             rx_deferred_start; /**< not in global dev start. */
+	uint8_t             vlan_strip; /**< 1 if vlan stripping enabled. */
 	/** need to alloc dummy mbuf, for wraparound when scanning hw ring */
 	struct rte_mbuf fake_mbuf;
 	/** hold packets to return to application */
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 3e901be..9896a0a 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1003,7 +1003,8 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 			pkt_buf->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
 #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
 			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
-				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT;
+				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT |
+					PKT_RX_VLAN_STRIPPED;
 				pkt_buf->vlan_tci = vlan_tci;
 			}
 #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
@@ -1159,7 +1160,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 			seg->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
 #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
 			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
-				seg->ol_flags |= PKT_RX_VLAN_PKT;
+				seg->ol_flags |= PKT_RX_VLAN_PKT |
+					PKT_RX_VLAN_STRIPPED;
 				seg->vlan_tci = vlan_tci;
 			}
 #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index ea5a2a3..5c9f350 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1800,7 +1800,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		if ((rxds->rxd.flags & PCIE_DESC_RX_VLAN) &&
 		    (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) {
 			mb->vlan_tci = rte_cpu_to_le_32(rxds->rxd.vlan);
-			mb->ol_flags |= PKT_RX_VLAN_PKT;
+			mb->ol_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		}
 
 		/* Adding the mbuff to the mbuff array passed by the app */
diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index f084ccd..63d91da 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -589,7 +589,7 @@ vmxnet3_rx_offload(const Vmxnet3_RxCompDesc *rcd, struct rte_mbuf *rxm)
 {
 	/* Check for hardware stripped VLAN tag */
 	if (rcd->ts) {
-		rxm->ol_flags |= PKT_RX_VLAN_PKT;
+		rxm->ol_flags |= (PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED);
 		rxm->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci);
 	}
 
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 878db89..09c525e 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -254,8 +254,10 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
 	case PKT_RX_L4_CKSUM_BAD: return "PKT_RX_L4_CKSUM_BAD";
 	case PKT_RX_IP_CKSUM_BAD: return "PKT_RX_IP_CKSUM_BAD";
 	case PKT_RX_EIP_CKSUM_BAD: return "PKT_RX_EIP_CKSUM_BAD";
+	case PKT_RX_VLAN_STRIPPED: return "PKT_RX_VLAN_STRIPPED";
 	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
 	case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST";
+	case PKT_RX_QINQ_STRIPPED: return "PKT_RX_QINQ_STRIPPED";
 	default: return NULL;
 	}
 }
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 3663d17..42e5345 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -79,18 +79,50 @@ extern "C" {
  * Keep these flags synchronized with rte_get_rx_ol_flag_name() and
  * rte_get_tx_ol_flag_name().
  */
-#define PKT_RX_VLAN_PKT      (1ULL << 0)  /**< RX packet is a 802.1q VLAN packet. */
+
+/**
+ * Deprecated.
+ * RX packet is a 802.1q VLAN packet. This flag was set by PMDs when
+ * the packet is recognized as a VLAN, but the behavior between PMDs
+ * was not the same. This flag is kept for some time to avoid breaking
+ * applications and should be replaced by PKT_RX_VLAN_STRIPPED.
+ */
+#define PKT_RX_VLAN_PKT      (1ULL << 0)
+
 #define PKT_RX_RSS_HASH      (1ULL << 1)  /**< RX packet with RSS hash result. */
 #define PKT_RX_FDIR          (1ULL << 2)  /**< RX packet with FDIR match indicate. */
 #define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt. is not OK. */
 #define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)  /**< IP cksum of RX pkt. is not OK. */
 #define PKT_RX_EIP_CKSUM_BAD (1ULL << 5)  /**< External IP header checksum error. */
+
+/**
+ * A vlan has been stripped by the hardware and its tci is saved in
+ * mbuf->vlan_tci. This can only happen if vlan stripping is enabled
+ * in the RX configuration of the PMD.
+ */
+#define PKT_RX_VLAN_STRIPPED (1ULL << 6)
+
 /* hole, some bits can be reused here  */
 #define PKT_RX_IEEE1588_PTP  (1ULL << 9)  /**< RX IEEE1588 L2 Ethernet PT Packet. */
 #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588 L2/L4 timestamped packet.*/
 #define PKT_RX_FDIR_ID       (1ULL << 13) /**< FD id reported if FDIR match. */
 #define PKT_RX_FDIR_FLX      (1ULL << 14) /**< Flexible bytes reported if FDIR match. */
-#define PKT_RX_QINQ_PKT      (1ULL << 15)  /**< RX packet with double VLAN stripped. */
+
+/**
+ * The 2 vlans have been stripped by the hardware and their tci are
+ * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
+ * This can only happen if vlan stripping is enabled in the RX
+ * configuration of the PMD. If this flag is set, PKT_RX_VLAN_STRIPPED
+ * must also be set.
+ */
+#define PKT_RX_QINQ_STRIPPED (1ULL << 15)
+
+/**
+ * Deprecated.
+ * This flag is replaced by PKT_RX_QINQ_STRIPPED.
+ */
+#define PKT_RX_QINQ_PKT      PKT_RX_QINQ_STRIPPED  /**< RX packet with double VLAN stripped. */
+
 /* add new RX flags here */
 
 /* add new TX flags here */
@@ -758,7 +790,10 @@ struct rte_mbuf {
 
 	/*
 	 * The packet type, which is the combination of outer/inner L2, L3, L4
-	 * and tunnel types.
+	 * and tunnel types. The packet_type is about data really present in the
+	 * mbuf. Example: if vlan stripping is enabled, a received vlan packet
+	 * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the
+	 * vlan is stripped from the data.
 	 */
 	union {
 		uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */
@@ -775,7 +810,8 @@ struct rte_mbuf {
 
 	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
 	uint16_t data_len;        /**< Amount of data in segment buffer. */
-	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
+	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order),
+				   *   valid if PKT_RX_VLAN_STRIPPED is set. */
 
 	union {
 		uint32_t rss;     /**< RSS hash result if RSS enabled */
@@ -801,7 +837,8 @@ struct rte_mbuf {
 
 	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
 
-	uint16_t vlan_tci_outer;  /**< Outer VLAN Tag Control Identifier (CPU order) */
+	uint16_t vlan_tci_outer;  /**< Outer VLAN Tag Control Identifier (CPU order),
+				   *   valid if PKT_RX_QINQ_STRIPPED is set. */
 
 	/* second cache line - fields only used in slow path or on TX */
 	MARKER cacheline1 __rte_cache_min_aligned;
-- 
2.8.0.rc3

Re: [RFC] mbuf: new flag when vlan is stripped

[John Daley (johndale)]

Hi Olivier,

> ...
> This is a draft patch that implements what was previously discussed, except
> the packet_type, which does not exist for vlan today (and I think it is not
> mandatory for now, let's do it in another patch).
> 
> After doing this patch, it appeared that ixgbe was the only driver that had a
> different behavior for the PKT_RX_VLAN_PKT flag. An alternative to this
> patch would be to only change the behavior of the ixgbe driver, and just
> document better document the PKT_RX_VLAN_PKT flags in rte_mbuf.h
> without adding new flags. I think this is a better option.
> 
> Comments are welcome.
>
There are applications depending on the current behavior of PKT_RX_VLAN_PKT as confusing as it may be.  I know of one that has VLAN stripping disabled and uses the flag to determine if the packet delivered to the app has a VLAN tag. This is actually how the flag behaves for ixgbe, and they patched enic and maybe other drivers to act accordingly. To avoid breaking the app (and any others like it), I think we should keep the flag behavior the same and add the new flag PKT_RX_VLAN_STRIPPED .

We should follow on with the new packet type since it enables a nice performance improvement by not forcing apps to break open the packet just to see if there is a VLAN tag.

Thanks,
john

Re: [RFC] mbuf: new flag when vlan is stripped

[Olivier Matz]

Hi John,

On 05/12/2016 10:36 PM, John Daley (johndale) wrote:
>> ... This is a draft patch that implements what was previously
>> discussed, except the packet_type, which does not exist for vlan
>> today (and I think it is not mandatory for now, let's do it in
>> another patch).
>> 
>> After doing this patch, it appeared that ixgbe was the only driver
>> that had a different behavior for the PKT_RX_VLAN_PKT flag. An
>> alternative to this patch would be to only change the behavior of
>> the ixgbe driver, and just document better document the
>> PKT_RX_VLAN_PKT flags in rte_mbuf.h without adding new flags. I
>> think this is a better option.
>> 
>> Comments are welcome.
>> 
> There are applications depending on the current behavior of
> PKT_RX_VLAN_PKT as confusing as it may be.  I know of one that has
> VLAN stripping disabled and uses the flag to determine if the packet
> delivered to the app has a VLAN tag. This is actually how the flag
> behaves for ixgbe, and they patched enic and maybe other drivers to
> act accordingly. To avoid breaking the app (and any others like it),
> I think we should keep the flag behavior the same and add the new
> flag PKT_RX_VLAN_STRIPPED .

OK, thanks for your comment.
So it means the v1 will be the same than RFC. I'm submitting it.

> We should follow on with the new packet type since it enables a nice
> performance improvement by not forcing apps to break open the packet
> just to see if there is a VLAN tag.

Yep, agree.

Thanks,
Olivier

[PATCH] mbuf: new flag when Vlan is stripped

[Olivier Matz]

The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
PMDs not advertising the same flags in similar conditions.

Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
and PKT_RX_QINQ_STRIPPED that are better defined:

  PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
  tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
  is enabled in the RX configuration of the PMD.

For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
It should be removed from applications and PMDs in a future revision.

This patch also updates the drivers. For PKT_RX_VLAN_PKT:

- e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
  had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
  required.
- fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
  PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
- ixgbe: modification done for standard mode (vector does not support
  vlan stripping)
- the other drivers do not support vlan stripping.

For PKT_RX_QINQ_PKT, it was only supported on i40e, and the meaning was
already correct, so we can reuse the same value for PKT_RX_QINQ_STRIPPED.

[1] http://dpdk.org/ml/archives/dev/2016-April/037837.html,

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

RFC -> v1:
- fix checkpatch and check-git-log.sh issues
- add a deprecation notice for the old vlan flags
- rebase on head


 app/test-pmd/rxonly.c                |  4 +--
 doc/guides/rel_notes/deprecation.rst |  5 ++++
 drivers/net/e1000/em_rxtx.c          |  3 ++-
 drivers/net/e1000/igb_rxtx.c         |  3 ++-
 drivers/net/enic/enic_rx.c           |  2 +-
 drivers/net/i40e/i40e_rxtx.c         |  2 +-
 drivers/net/ixgbe/ixgbe_ethdev.c     |  7 +++++
 drivers/net/ixgbe/ixgbe_rxtx.c       | 21 +++++++++++----
 drivers/net/ixgbe/ixgbe_rxtx.h       |  1 +
 drivers/net/mlx5/mlx5_rxtx.c         |  6 +++--
 drivers/net/nfp/nfp_net.c            |  2 +-
 drivers/net/vmxnet3/vmxnet3_rxtx.c   |  2 +-
 lib/librte_mbuf/rte_mbuf.c           |  2 ++
 lib/librte_mbuf/rte_mbuf.h           | 50 ++++++++++++++++++++++++++++++++----
 14 files changed, 90 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index 14555ab..c69b344 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -156,9 +156,9 @@ pkt_burst_receive(struct fwd_stream *fs)
 				printf("hash=0x%x ID=0x%x ",
 				       mb->hash.fdir.hash, mb->hash.fdir.id);
 		}
-		if (ol_flags & PKT_RX_VLAN_PKT)
+		if (ol_flags & PKT_RX_VLAN_STRIPPED)
 			printf(" - VLAN tci=0x%x", mb->vlan_tci);
-		if (ol_flags & PKT_RX_QINQ_PKT)
+		if (ol_flags & PKT_RX_QINQ_STRIPPED)
 			printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
 					mb->vlan_tci, mb->vlan_tci_outer);
 		if (mb->packet_type) {
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index ad05eba..2233a90 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -57,3 +57,8 @@ Deprecation Notices
   a handle, like the way kernel exposes an fd to user for locating a
   specific file, and to keep all major structures internally, so that
   we are likely to be free from ABI violations in future.
+
+* The mbuf flags PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated and
+  are respectively replaced by PKT_RX_VLAN_STRIPPED and
+  PKT_RX_QINQ_STRIPPED, that are better described. The old flags and
+  their behavior will be kept in 16.07 and will be removed in 16.11.
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 3d36f21..6d8750a 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -629,7 +629,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	uint64_t pkt_flags;
 
 	/* Check if VLAN present */
-	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0);
+	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
+		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
 
 	return pkt_flags;
 }
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 18aeead..9d80a0b 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -729,7 +729,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	uint64_t pkt_flags;
 
 	/* Check if VLAN present */
-	pkt_flags = (rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
+	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
+		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
 
 #if defined(RTE_LIBRTE_IEEE1588)
 	if (rx_status & E1000_RXD_STAT_TMST)
diff --git a/drivers/net/enic/enic_rx.c b/drivers/net/enic/enic_rx.c
index f92f6bc..6459e97 100644
--- a/drivers/net/enic/enic_rx.c
+++ b/drivers/net/enic/enic_rx.c
@@ -197,7 +197,7 @@ enic_cq_rx_to_pkt_flags(struct cq_desc *cqd, struct rte_mbuf *mbuf)
 
 	/* VLAN stripping */
 	if (bwflags & CQ_ENET_RQ_DESC_FLAGS_VLAN_STRIPPED) {
-		pkt_flags |= PKT_RX_VLAN_PKT;
+		pkt_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		mbuf->vlan_tci = enic_cq_rx_desc_vlan(cqrd);
 	} else {
 		mbuf->vlan_tci = 0;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index c833aa3..aa161a9 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -99,7 +99,7 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union i40e_rx_desc *rxdp)
 #ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
 	if (rte_le_to_cpu_16(rxdp->wb.qword2.ext_status) &
 		(1 << I40E_RX_DESC_EXT_STATUS_L2TAG2P_SHIFT)) {
-		mb->ol_flags |= PKT_RX_QINQ_PKT;
+		mb->ol_flags |= PKT_RX_QINQ_STRIPPED;
 		mb->vlan_tci_outer = mb->vlan_tci;
 		mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_2);
 		PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u",
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a2b170b..e7717e3 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1636,6 +1636,7 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 {
 	struct ixgbe_hwstrip *hwstrip =
 		IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(dev->data->dev_private);
+	struct ixgbe_rx_queue *rxq;
 
 	if (queue >= IXGBE_MAX_RX_QUEUE_NUM)
 		return;
@@ -1644,6 +1645,12 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 		IXGBE_SET_HWSTRIP(hwstrip, queue);
 	else
 		IXGBE_CLEAR_HWSTRIP(hwstrip, queue);
+
+	if (queue >= dev->data->nb_rx_queues)
+		return;
+
+	rxq = dev->data->rx_queues[queue];
+	rxq->vlan_strip = on;
 }
 
 static void
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 9c6eaf2..3d740df 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1221,16 +1221,23 @@ ixgbe_rxd_pkt_info_to_pkt_flags(uint16_t pkt_info)
 }
 
 static inline uint64_t
-rx_desc_status_to_pkt_flags(uint32_t rx_status)
+rx_desc_status_to_pkt_flags(uint32_t rx_status, uint8_t vlan_strip)
 {
 	uint64_t pkt_flags;
+	uint64_t vlan_flags;
+
+	/* if vlan is stripped, set the proper flag */
+	if (vlan_strip)
+		vlan_flags = PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
+	else
+		vlan_flags = PKT_RX_VLAN_PKT;
 
 	/*
 	 * Check if VLAN present only.
 	 * Do not check whether L3/L4 rx checksum done by NIC or not,
 	 * That can be found from rte_eth_rxmode.hw_ip_checksum flag
 	 */
-	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
+	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  vlan_flags : 0;
 
 #ifdef RTE_LIBRTE_IEEE1588
 	if (rx_status & IXGBE_RXD_STAT_TMST)
@@ -1287,6 +1294,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 	uint32_t pkt_info[LOOK_AHEAD];
 	int i, j, nb_rx = 0;
 	uint32_t status;
+	uint8_t vlan_strip = rxq->vlan_strip;
 
 	/* get references to current descriptor and S/W ring entry */
 	rxdp = &rxq->rx_ring[rxq->rx_tail];
@@ -1328,7 +1336,8 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 			mb->vlan_tci = rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
 
 			/* convert descriptor fields to rte mbuf flags */
-			pkt_flags = rx_desc_status_to_pkt_flags(s[j]);
+			pkt_flags = rx_desc_status_to_pkt_flags(s[j],
+				vlan_strip);
 			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
 			pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags
 					((uint16_t)pkt_info[j]);
@@ -1544,6 +1553,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	uint16_t nb_rx;
 	uint16_t nb_hold;
 	uint64_t pkt_flags;
+	uint8_t vlan_strip;
 
 	nb_rx = 0;
 	nb_hold = 0;
@@ -1551,6 +1561,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	rx_id = rxq->rx_tail;
 	rx_ring = rxq->rx_ring;
 	sw_ring = rxq->sw_ring;
+	vlan_strip = rxq->vlan_strip;
 	while (nb_rx < nb_pkts) {
 		/*
 		 * The order of operations here is important as the DD status
@@ -1660,7 +1671,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		/* Only valid if PKT_RX_VLAN_PKT set in pkt_flags */
 		rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan);
 
-		pkt_flags = rx_desc_status_to_pkt_flags(staterr);
+		pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_strip);
 		pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr);
 		pkt_flags = pkt_flags |
 			ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
@@ -1753,7 +1764,7 @@ ixgbe_fill_cluster_head_buf(
 	 */
 	head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
 	pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
-	pkt_flags = rx_desc_status_to_pkt_flags(staterr);
+	pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_strip);
 	pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
 	pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 	head->ol_flags = pkt_flags;
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 3691a19..9ca0e8b 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -146,6 +146,7 @@ struct ixgbe_rx_queue {
 	uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
 	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
 	uint8_t             rx_deferred_start; /**< not in global dev start. */
+	uint8_t             vlan_strip; /**< 1 if vlan stripping enabled. */
 	/** need to alloc dummy mbuf, for wraparound when scanning hw ring */
 	struct rte_mbuf fake_mbuf;
 	/** hold packets to return to application */
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 13c8d71..ac96fc9 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1051,7 +1051,8 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 			pkt_buf->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
 #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
 			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
-				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT;
+				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT |
+					PKT_RX_VLAN_STRIPPED;
 				pkt_buf->vlan_tci = vlan_tci;
 			}
 #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
@@ -1207,7 +1208,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 			seg->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
 #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
 			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
-				seg->ol_flags |= PKT_RX_VLAN_PKT;
+				seg->ol_flags |= PKT_RX_VLAN_PKT |
+					PKT_RX_VLAN_STRIPPED;
 				seg->vlan_tci = vlan_tci;
 			}
 #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index ea5a2a3..5c9f350 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1800,7 +1800,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		if ((rxds->rxd.flags & PCIE_DESC_RX_VLAN) &&
 		    (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) {
 			mb->vlan_tci = rte_cpu_to_le_32(rxds->rxd.vlan);
-			mb->ol_flags |= PKT_RX_VLAN_PKT;
+			mb->ol_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		}
 
 		/* Adding the mbuff to the mbuff array passed by the app */
diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index 9fe8752..ccafc0c 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -579,7 +579,7 @@ vmxnet3_rx_offload(const Vmxnet3_RxCompDesc *rcd, struct rte_mbuf *rxm)
 {
 	/* Check for hardware stripped VLAN tag */
 	if (rcd->ts) {
-		rxm->ol_flags |= PKT_RX_VLAN_PKT;
+		rxm->ol_flags |= (PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED);
 		rxm->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci);
 	}
 
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index eec1456..2ece742 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -258,8 +258,10 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
 	/* case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW"; */
 	/* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */
 	/* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */
+	case PKT_RX_VLAN_STRIPPED: return "PKT_RX_VLAN_STRIPPED";
 	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
 	case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST";
+	case PKT_RX_QINQ_STRIPPED: return "PKT_RX_QINQ_STRIPPED";
 	default: return NULL;
 	}
 }
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 48911a6..5b8a11a 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -79,7 +79,16 @@ extern "C" {
  * Keep these flags synchronized with rte_get_rx_ol_flag_name() and
  * rte_get_tx_ol_flag_name().
  */
-#define PKT_RX_VLAN_PKT      (1ULL << 0)  /**< RX packet is a 802.1q VLAN packet. */
+
+/**
+ * Deprecated.
+ * RX packet is a 802.1q VLAN packet. This flag was set by PMDs when
+ * the packet is recognized as a VLAN, but the behavior between PMDs
+ * was not the same. This flag is kept for some time to avoid breaking
+ * applications and should be replaced by PKT_RX_VLAN_STRIPPED.
+ */
+#define PKT_RX_VLAN_PKT      (1ULL << 0)
+
 #define PKT_RX_RSS_HASH      (1ULL << 1)  /**< RX packet with RSS hash result. */
 #define PKT_RX_FDIR          (1ULL << 2)  /**< RX packet with FDIR match indicate. */
 #define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt. is not OK. */
@@ -89,11 +98,37 @@ extern "C" {
 #define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer overflow. */
 #define PKT_RX_RECIP_ERR     (0ULL << 0)  /**< Hardware processing error. */
 #define PKT_RX_MAC_ERR       (0ULL << 0)  /**< MAC error. */
+
+/**
+ * A vlan has been stripped by the hardware and its tci is saved in
+ * mbuf->vlan_tci. This can only happen if vlan stripping is enabled
+ * in the RX configuration of the PMD.
+ */
+#define PKT_RX_VLAN_STRIPPED (1ULL << 6)
+
+/* hole, some bits can be reused here  */
+
 #define PKT_RX_IEEE1588_PTP  (1ULL << 9)  /**< RX IEEE1588 L2 Ethernet PT Packet. */
 #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588 L2/L4 timestamped packet.*/
 #define PKT_RX_FDIR_ID       (1ULL << 13) /**< FD id reported if FDIR match. */
 #define PKT_RX_FDIR_FLX      (1ULL << 14) /**< Flexible bytes reported if FDIR match. */
-#define PKT_RX_QINQ_PKT      (1ULL << 15)  /**< RX packet with double VLAN stripped. */
+
+/**
+ * The 2 vlans have been stripped by the hardware and their tci are
+ * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
+ * This can only happen if vlan stripping is enabled in the RX
+ * configuration of the PMD. If this flag is set, PKT_RX_VLAN_STRIPPED
+ * must also be set.
+ */
+#define PKT_RX_QINQ_STRIPPED (1ULL << 15)
+
+/**
+ * Deprecated.
+ * RX packet with double VLAN stripped.
+ * This flag is replaced by PKT_RX_QINQ_STRIPPED.
+ */
+#define PKT_RX_QINQ_PKT      PKT_RX_QINQ_STRIPPED
+
 /* add new RX flags here */
 
 /* add new TX flags here */
@@ -761,7 +796,10 @@ struct rte_mbuf {
 
 	/*
 	 * The packet type, which is the combination of outer/inner L2, L3, L4
-	 * and tunnel types.
+	 * and tunnel types. The packet_type is about data really present in the
+	 * mbuf. Example: if vlan stripping is enabled, a received vlan packet
+	 * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the
+	 * vlan is stripped from the data.
 	 */
 	union {
 		uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */
@@ -778,7 +816,8 @@ struct rte_mbuf {
 
 	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
 	uint16_t data_len;        /**< Amount of data in segment buffer. */
-	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
+	/** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */
+	uint16_t vlan_tci;
 
 	union {
 		uint32_t rss;     /**< RSS hash result if RSS enabled */
@@ -804,7 +843,8 @@ struct rte_mbuf {
 
 	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
 
-	uint16_t vlan_tci_outer;  /**< Outer VLAN Tag Control Identifier (CPU order) */
+	/** Outer VLAN TCI (CPU order), valid if PKT_RX_QINQ_STRIPPED is set. */
+	uint16_t vlan_tci_outer;
 
 	/* second cache line - fields only used in slow path or on TX */
 	MARKER cacheline1 __rte_cache_min_aligned;
-- 
2.8.0.rc3

Re: [PATCH] mbuf: new flag when Vlan is stripped

[Ananyev, Konstantin]

Hi Olivier,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, May 23, 2016 9:47 AM
> To: dev@dpdk.org
> Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; adrien.mazarguil@6wind.com; rahul.lakkireddy@chelsio.com;
> alejandro.lucero@netronome.com; sony.chacko@qlogic.com
> Subject: [PATCH] mbuf: new flag when Vlan is stripped
> 
> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
> PMDs not advertising the same flags in similar conditions.
> 
> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
> and PKT_RX_QINQ_STRIPPED that are better defined:
> 
>   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
>   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
>   is enabled in the RX configuration of the PMD.
> 
> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
> It should be removed from applications and PMDs in a future revision.
> 
> This patch also updates the drivers. For PKT_RX_VLAN_PKT:
> 
> - e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
>   had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
>   required.
> - fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
>   PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
> - ixgbe: modification done for standard mode (vector does not support
>   vlan stripping)
> - the other drivers do not support vlan stripping.
> 
> For PKT_RX_QINQ_PKT, it was only supported on i40e, and the meaning was
> already correct, so we can reuse the same value for PKT_RX_QINQ_STRIPPED.
> 
> [1] http://dpdk.org/ml/archives/dev/2016-April/037837.html,
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> 
> RFC -> v1:
> - fix checkpatch and check-git-log.sh issues
> - add a deprecation notice for the old vlan flags
> - rebase on head
> 
> 
>  app/test-pmd/rxonly.c                |  4 +--
>  doc/guides/rel_notes/deprecation.rst |  5 ++++
>  drivers/net/e1000/em_rxtx.c          |  3 ++-
>  drivers/net/e1000/igb_rxtx.c         |  3 ++-
>  drivers/net/enic/enic_rx.c           |  2 +-
>  drivers/net/i40e/i40e_rxtx.c         |  2 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c     |  7 +++++
>  drivers/net/ixgbe/ixgbe_rxtx.c       | 21 +++++++++++----
>  drivers/net/ixgbe/ixgbe_rxtx.h       |  1 +
>  drivers/net/mlx5/mlx5_rxtx.c         |  6 +++--
>  drivers/net/nfp/nfp_net.c            |  2 +-
>  drivers/net/vmxnet3/vmxnet3_rxtx.c   |  2 +-
>  lib/librte_mbuf/rte_mbuf.c           |  2 ++
>  lib/librte_mbuf/rte_mbuf.h           | 50 ++++++++++++++++++++++++++++++++----
>  14 files changed, 90 insertions(+), 20 deletions(-)


I don't see ixgbe/i4oe_rxtx_vec.c updated.
Would it be another patch for them?
Thanks
Konstantin

> 
> diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
> index 14555ab..c69b344 100644
> --- a/app/test-pmd/rxonly.c
> +++ b/app/test-pmd/rxonly.c
> @@ -156,9 +156,9 @@ pkt_burst_receive(struct fwd_stream *fs)
>  				printf("hash=0x%x ID=0x%x ",
>  				       mb->hash.fdir.hash, mb->hash.fdir.id);
>  		}
> -		if (ol_flags & PKT_RX_VLAN_PKT)
> +		if (ol_flags & PKT_RX_VLAN_STRIPPED)
>  			printf(" - VLAN tci=0x%x", mb->vlan_tci);
> -		if (ol_flags & PKT_RX_QINQ_PKT)
> +		if (ol_flags & PKT_RX_QINQ_STRIPPED)
>  			printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
>  					mb->vlan_tci, mb->vlan_tci_outer);
>  		if (mb->packet_type) {
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index ad05eba..2233a90 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -57,3 +57,8 @@ Deprecation Notices
>    a handle, like the way kernel exposes an fd to user for locating a
>    specific file, and to keep all major structures internally, so that
>    we are likely to be free from ABI violations in future.
> +
> +* The mbuf flags PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated and
> +  are respectively replaced by PKT_RX_VLAN_STRIPPED and
> +  PKT_RX_QINQ_STRIPPED, that are better described. The old flags and
> +  their behavior will be kept in 16.07 and will be removed in 16.11.
> diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
> index 3d36f21..6d8750a 100644
> --- a/drivers/net/e1000/em_rxtx.c
> +++ b/drivers/net/e1000/em_rxtx.c
> @@ -629,7 +629,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
>  	uint64_t pkt_flags;
> 
>  	/* Check if VLAN present */
> -	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0);
> +	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
> +		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
> 
>  	return pkt_flags;
>  }
> diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
> index 18aeead..9d80a0b 100644
> --- a/drivers/net/e1000/igb_rxtx.c
> +++ b/drivers/net/e1000/igb_rxtx.c
> @@ -729,7 +729,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
>  	uint64_t pkt_flags;
> 
>  	/* Check if VLAN present */
> -	pkt_flags = (rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
> +	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
> +		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
> 
>  #if defined(RTE_LIBRTE_IEEE1588)
>  	if (rx_status & E1000_RXD_STAT_TMST)
> diff --git a/drivers/net/enic/enic_rx.c b/drivers/net/enic/enic_rx.c
> index f92f6bc..6459e97 100644
> --- a/drivers/net/enic/enic_rx.c
> +++ b/drivers/net/enic/enic_rx.c
> @@ -197,7 +197,7 @@ enic_cq_rx_to_pkt_flags(struct cq_desc *cqd, struct rte_mbuf *mbuf)
> 
>  	/* VLAN stripping */
>  	if (bwflags & CQ_ENET_RQ_DESC_FLAGS_VLAN_STRIPPED) {
> -		pkt_flags |= PKT_RX_VLAN_PKT;
> +		pkt_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
>  		mbuf->vlan_tci = enic_cq_rx_desc_vlan(cqrd);
>  	} else {
>  		mbuf->vlan_tci = 0;
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index c833aa3..aa161a9 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -99,7 +99,7 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union i40e_rx_desc *rxdp)
>  #ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
>  	if (rte_le_to_cpu_16(rxdp->wb.qword2.ext_status) &
>  		(1 << I40E_RX_DESC_EXT_STATUS_L2TAG2P_SHIFT)) {
> -		mb->ol_flags |= PKT_RX_QINQ_PKT;
> +		mb->ol_flags |= PKT_RX_QINQ_STRIPPED;
>  		mb->vlan_tci_outer = mb->vlan_tci;
>  		mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_2);
>  		PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u",
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index a2b170b..e7717e3 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1636,6 +1636,7 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
>  {
>  	struct ixgbe_hwstrip *hwstrip =
>  		IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(dev->data->dev_private);
> +	struct ixgbe_rx_queue *rxq;
> 
>  	if (queue >= IXGBE_MAX_RX_QUEUE_NUM)
>  		return;
> @@ -1644,6 +1645,12 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
>  		IXGBE_SET_HWSTRIP(hwstrip, queue);
>  	else
>  		IXGBE_CLEAR_HWSTRIP(hwstrip, queue);
> +
> +	if (queue >= dev->data->nb_rx_queues)
> +		return;
> +
> +	rxq = dev->data->rx_queues[queue];
> +	rxq->vlan_strip = on;
>  }
> 
>  static void
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 9c6eaf2..3d740df 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -1221,16 +1221,23 @@ ixgbe_rxd_pkt_info_to_pkt_flags(uint16_t pkt_info)
>  }
> 
>  static inline uint64_t
> -rx_desc_status_to_pkt_flags(uint32_t rx_status)
> +rx_desc_status_to_pkt_flags(uint32_t rx_status, uint8_t vlan_strip)
>  {
>  	uint64_t pkt_flags;
> +	uint64_t vlan_flags;
> +
> +	/* if vlan is stripped, set the proper flag */
> +	if (vlan_strip)
> +		vlan_flags = PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
> +	else
> +		vlan_flags = PKT_RX_VLAN_PKT;
> 
>  	/*
>  	 * Check if VLAN present only.
>  	 * Do not check whether L3/L4 rx checksum done by NIC or not,
>  	 * That can be found from rte_eth_rxmode.hw_ip_checksum flag
>  	 */
> -	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
> +	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  vlan_flags : 0;
> 
>  #ifdef RTE_LIBRTE_IEEE1588
>  	if (rx_status & IXGBE_RXD_STAT_TMST)
> @@ -1287,6 +1294,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
>  	uint32_t pkt_info[LOOK_AHEAD];
>  	int i, j, nb_rx = 0;
>  	uint32_t status;
> +	uint8_t vlan_strip = rxq->vlan_strip;
> 
>  	/* get references to current descriptor and S/W ring entry */
>  	rxdp = &rxq->rx_ring[rxq->rx_tail];
> @@ -1328,7 +1336,8 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
>  			mb->vlan_tci = rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
> 
>  			/* convert descriptor fields to rte mbuf flags */
> -			pkt_flags = rx_desc_status_to_pkt_flags(s[j]);
> +			pkt_flags = rx_desc_status_to_pkt_flags(s[j],
> +				vlan_strip);
>  			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
>  			pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags
>  					((uint16_t)pkt_info[j]);
> @@ -1544,6 +1553,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  	uint16_t nb_rx;
>  	uint16_t nb_hold;
>  	uint64_t pkt_flags;
> +	uint8_t vlan_strip;
> 
>  	nb_rx = 0;
>  	nb_hold = 0;
> @@ -1551,6 +1561,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  	rx_id = rxq->rx_tail;
>  	rx_ring = rxq->rx_ring;
>  	sw_ring = rxq->sw_ring;
> +	vlan_strip = rxq->vlan_strip;
>  	while (nb_rx < nb_pkts) {
>  		/*
>  		 * The order of operations here is important as the DD status
> @@ -1660,7 +1671,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		/* Only valid if PKT_RX_VLAN_PKT set in pkt_flags */
>  		rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan);
> 
> -		pkt_flags = rx_desc_status_to_pkt_flags(staterr);
> +		pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_strip);
>  		pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr);
>  		pkt_flags = pkt_flags |
>  			ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
> @@ -1753,7 +1764,7 @@ ixgbe_fill_cluster_head_buf(
>  	 */
>  	head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
>  	pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
> -	pkt_flags = rx_desc_status_to_pkt_flags(staterr);
> +	pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_strip);
>  	pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
>  	pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
>  	head->ol_flags = pkt_flags;
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
> index 3691a19..9ca0e8b 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> @@ -146,6 +146,7 @@ struct ixgbe_rx_queue {
>  	uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
>  	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
>  	uint8_t             rx_deferred_start; /**< not in global dev start. */
> +	uint8_t             vlan_strip; /**< 1 if vlan stripping enabled. */
>  	/** need to alloc dummy mbuf, for wraparound when scanning hw ring */
>  	struct rte_mbuf fake_mbuf;
>  	/** hold packets to return to application */
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index 13c8d71..ac96fc9 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -1051,7 +1051,8 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  			pkt_buf->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
>  #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
>  			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
> -				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT;
> +				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT |
> +					PKT_RX_VLAN_STRIPPED;
>  				pkt_buf->vlan_tci = vlan_tci;
>  			}
>  #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
> @@ -1207,7 +1208,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  			seg->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
>  #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
>  			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
> -				seg->ol_flags |= PKT_RX_VLAN_PKT;
> +				seg->ol_flags |= PKT_RX_VLAN_PKT |
> +					PKT_RX_VLAN_STRIPPED;
>  				seg->vlan_tci = vlan_tci;
>  			}
>  #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> index ea5a2a3..5c9f350 100644
> --- a/drivers/net/nfp/nfp_net.c
> +++ b/drivers/net/nfp/nfp_net.c
> @@ -1800,7 +1800,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  		if ((rxds->rxd.flags & PCIE_DESC_RX_VLAN) &&
>  		    (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) {
>  			mb->vlan_tci = rte_cpu_to_le_32(rxds->rxd.vlan);
> -			mb->ol_flags |= PKT_RX_VLAN_PKT;
> +			mb->ol_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
>  		}
> 
>  		/* Adding the mbuff to the mbuff array passed by the app */
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> index 9fe8752..ccafc0c 100644
> --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> @@ -579,7 +579,7 @@ vmxnet3_rx_offload(const Vmxnet3_RxCompDesc *rcd, struct rte_mbuf *rxm)
>  {
>  	/* Check for hardware stripped VLAN tag */
>  	if (rcd->ts) {
> -		rxm->ol_flags |= PKT_RX_VLAN_PKT;
> +		rxm->ol_flags |= (PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED);
>  		rxm->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci);
>  	}
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index eec1456..2ece742 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -258,8 +258,10 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
>  	/* case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW"; */
>  	/* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */
>  	/* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */
> +	case PKT_RX_VLAN_STRIPPED: return "PKT_RX_VLAN_STRIPPED";
>  	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
>  	case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST";
> +	case PKT_RX_QINQ_STRIPPED: return "PKT_RX_QINQ_STRIPPED";
>  	default: return NULL;
>  	}
>  }
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 48911a6..5b8a11a 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -79,7 +79,16 @@ extern "C" {
>   * Keep these flags synchronized with rte_get_rx_ol_flag_name() and
>   * rte_get_tx_ol_flag_name().
>   */
> -#define PKT_RX_VLAN_PKT      (1ULL << 0)  /**< RX packet is a 802.1q VLAN packet. */
> +
> +/**
> + * Deprecated.
> + * RX packet is a 802.1q VLAN packet. This flag was set by PMDs when
> + * the packet is recognized as a VLAN, but the behavior between PMDs
> + * was not the same. This flag is kept for some time to avoid breaking
> + * applications and should be replaced by PKT_RX_VLAN_STRIPPED.
> + */
> +#define PKT_RX_VLAN_PKT      (1ULL << 0)
> +
>  #define PKT_RX_RSS_HASH      (1ULL << 1)  /**< RX packet with RSS hash result. */
>  #define PKT_RX_FDIR          (1ULL << 2)  /**< RX packet with FDIR match indicate. */
>  #define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt. is not OK. */
> @@ -89,11 +98,37 @@ extern "C" {
>  #define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer overflow. */
>  #define PKT_RX_RECIP_ERR     (0ULL << 0)  /**< Hardware processing error. */
>  #define PKT_RX_MAC_ERR       (0ULL << 0)  /**< MAC error. */
> +
> +/**
> + * A vlan has been stripped by the hardware and its tci is saved in
> + * mbuf->vlan_tci. This can only happen if vlan stripping is enabled
> + * in the RX configuration of the PMD.
> + */
> +#define PKT_RX_VLAN_STRIPPED (1ULL << 6)
> +
> +/* hole, some bits can be reused here  */
> +
>  #define PKT_RX_IEEE1588_PTP  (1ULL << 9)  /**< RX IEEE1588 L2 Ethernet PT Packet. */
>  #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588 L2/L4 timestamped packet.*/
>  #define PKT_RX_FDIR_ID       (1ULL << 13) /**< FD id reported if FDIR match. */
>  #define PKT_RX_FDIR_FLX      (1ULL << 14) /**< Flexible bytes reported if FDIR match. */
> -#define PKT_RX_QINQ_PKT      (1ULL << 15)  /**< RX packet with double VLAN stripped. */
> +
> +/**
> + * The 2 vlans have been stripped by the hardware and their tci are
> + * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
> + * This can only happen if vlan stripping is enabled in the RX
> + * configuration of the PMD. If this flag is set, PKT_RX_VLAN_STRIPPED
> + * must also be set.
> + */
> +#define PKT_RX_QINQ_STRIPPED (1ULL << 15)
> +
> +/**
> + * Deprecated.
> + * RX packet with double VLAN stripped.
> + * This flag is replaced by PKT_RX_QINQ_STRIPPED.
> + */
> +#define PKT_RX_QINQ_PKT      PKT_RX_QINQ_STRIPPED
> +
>  /* add new RX flags here */
> 
>  /* add new TX flags here */
> @@ -761,7 +796,10 @@ struct rte_mbuf {
> 
>  	/*
>  	 * The packet type, which is the combination of outer/inner L2, L3, L4
> -	 * and tunnel types.
> +	 * and tunnel types. The packet_type is about data really present in the
> +	 * mbuf. Example: if vlan stripping is enabled, a received vlan packet
> +	 * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the
> +	 * vlan is stripped from the data.
>  	 */
>  	union {
>  		uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */
> @@ -778,7 +816,8 @@ struct rte_mbuf {
> 
>  	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
>  	uint16_t data_len;        /**< Amount of data in segment buffer. */
> -	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
> +	/** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */
> +	uint16_t vlan_tci;
> 
>  	union {
>  		uint32_t rss;     /**< RSS hash result if RSS enabled */
> @@ -804,7 +843,8 @@ struct rte_mbuf {
> 
>  	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
> 
> -	uint16_t vlan_tci_outer;  /**< Outer VLAN Tag Control Identifier (CPU order) */
> +	/** Outer VLAN TCI (CPU order), valid if PKT_RX_QINQ_STRIPPED is set. */
> +	uint16_t vlan_tci_outer;
> 
>  	/* second cache line - fields only used in slow path or on TX */
>  	MARKER cacheline1 __rte_cache_min_aligned;
> --
> 2.8.0.rc3

Re: [PATCH] mbuf: new flag when Vlan is stripped

[Olivier Matz]

Hi Konstantin,

On 05/23/2016 10:59 AM, Ananyev, Konstantin wrote:
> Hi Olivier,
> 
>> -----Original Message-----
>> From: Olivier Matz [mailto:olivier.matz@6wind.com]
>> Sent: Monday, May 23, 2016 9:47 AM
>> To: dev@dpdk.org
>> Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; adrien.mazarguil@6wind.com; rahul.lakkireddy@chelsio.com;
>> alejandro.lucero@netronome.com; sony.chacko@qlogic.com
>> Subject: [PATCH] mbuf: new flag when Vlan is stripped
>>
>> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
>> PMDs not advertising the same flags in similar conditions.
>>
>> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
>> and PKT_RX_QINQ_STRIPPED that are better defined:
>>
>>   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
>>   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
>>   is enabled in the RX configuration of the PMD.
>>
>> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
>> It should be removed from applications and PMDs in a future revision.
>>
>> This patch also updates the drivers. For PKT_RX_VLAN_PKT:
>>
>> - e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
>>   had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
>>   required.
>> - fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
>>   PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
>> - ixgbe: modification done for standard mode (vector does not support
>>   vlan stripping)
>> - the other drivers do not support vlan stripping.
>>
>> For PKT_RX_QINQ_PKT, it was only supported on i40e, and the meaning was
>> already correct, so we can reuse the same value for PKT_RX_QINQ_STRIPPED.
>>
> 
> I don't see ixgbe/i4oe_rxtx_vec.c updated.
> Would it be another patch for them?

The ixgbe vector and i40e vector do not support vlan stripping, so
from what I see there is nothing to do:
- The new flag PKT_RX_VLAN_STRIPPED is never returned
- We keep the old behavior for PKT_RX_VLAN_PKT.

Thanks,
Olivier

Re: [PATCH] mbuf: new flag when Vlan is stripped

[Ananyev, Konstantin]

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, May 23, 2016 10:13 AM
> To: Ananyev, Konstantin; dev@dpdk.org
> Cc: johndale@cisco.com; Zhang, Helin; adrien.mazarguil@6wind.com; rahul.lakkireddy@chelsio.com;
> alejandro.lucero@netronome.com; sony.chacko@qlogic.com
> Subject: Re: [PATCH] mbuf: new flag when Vlan is stripped
> 
> Hi Konstantin,
> 
> On 05/23/2016 10:59 AM, Ananyev, Konstantin wrote:
> > Hi Olivier,
> >
> >> -----Original Message-----
> >> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> >> Sent: Monday, May 23, 2016 9:47 AM
> >> To: dev@dpdk.org
> >> Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; adrien.mazarguil@6wind.com; rahul.lakkireddy@chelsio.com;
> >> alejandro.lucero@netronome.com; sony.chacko@qlogic.com
> >> Subject: [PATCH] mbuf: new flag when Vlan is stripped
> >>
> >> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
> >> PMDs not advertising the same flags in similar conditions.
> >>
> >> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
> >> and PKT_RX_QINQ_STRIPPED that are better defined:
> >>
> >>   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
> >>   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
> >>   is enabled in the RX configuration of the PMD.
> >>
> >> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
> >> It should be removed from applications and PMDs in a future revision.
> >>
> >> This patch also updates the drivers. For PKT_RX_VLAN_PKT:
> >>
> >> - e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
> >>   had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
> >>   required.
> >> - fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
> >>   PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
> >> - ixgbe: modification done for standard mode (vector does not support
> >>   vlan stripping)
> >> - the other drivers do not support vlan stripping.
> >>
> >> For PKT_RX_QINQ_PKT, it was only supported on i40e, and the meaning was
> >> already correct, so we can reuse the same value for PKT_RX_QINQ_STRIPPED.
> >>
> >
> > I don't see ixgbe/i4oe_rxtx_vec.c updated.
> > Would it be another patch for them?
> 
> The ixgbe vector and i40e vector do not support vlan stripping, 

As I remember, they do.
Konstantin

>so  from what I see there is nothing to do:
> - The new flag PKT_RX_VLAN_STRIPPED is never returned
> - We keep the old behavior for PKT_RX_VLAN_PKT.
> 
> Thanks,
> Olivier

Re: [PATCH] mbuf: new flag when Vlan is stripped

[Olivier Matz]

Hi,

>>> I don't see ixgbe/i4oe_rxtx_vec.c updated.
>>> Would it be another patch for them?
>>
>> The ixgbe vector and i40e vector do not support vlan stripping, 
> 
> As I remember, they do.

You are right, I misinterpreted this code in condition_check():

#ifndef RTE_IXGBE_RX_OLFLAGS_ENABLE
	/* whithout rx ol_flags, no VP flag report */
	if (rxmode->hw_vlan_strip != 0 ||
	    rxmode->hw_vlan_extend != 0)
		return -1;
#endif

I'll update the patch accordingly, thanks for reviewing.

Olivier

Re: [PATCH] mbuf: new flag when Vlan is stripped

[Ananyev, Konstantin]

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, May 23, 2016 9:47 AM
> To: dev@dpdk.org
> Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; adrien.mazarguil@6wind.com; rahul.lakkireddy@chelsio.com;
> alejandro.lucero@netronome.com; sony.chacko@qlogic.com
> Subject: [PATCH] mbuf: new flag when Vlan is stripped
> 
> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
> PMDs not advertising the same flags in similar conditions.
> 
> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
> and PKT_RX_QINQ_STRIPPED that are better defined:
> 
>   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
>   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
>   is enabled in the RX configuration of the PMD.
> 
> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
> It should be removed from applications and PMDs in a future revision.
> 
> This patch also updates the drivers. For PKT_RX_VLAN_PKT:
> 
> - e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
>   had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
>   required.
> - fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
>   PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
> - ixgbe: modification done for standard mode (vector does not support
>   vlan stripping)
> - the other drivers do not support vlan stripping.
> 
> For PKT_RX_QINQ_PKT, it was only supported on i40e, and the meaning was
> already correct, so we can reuse the same value for PKT_RX_QINQ_STRIPPED.
> 
> [1] http://dpdk.org/ml/archives/dev/2016-April/037837.html,
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> 
> RFC -> v1:
> - fix checkpatch and check-git-log.sh issues
> - add a deprecation notice for the old vlan flags
> - rebase on head
> 
> 
>  app/test-pmd/rxonly.c                |  4 +--
>  doc/guides/rel_notes/deprecation.rst |  5 ++++
>  drivers/net/e1000/em_rxtx.c          |  3 ++-
>  drivers/net/e1000/igb_rxtx.c         |  3 ++-
>  drivers/net/enic/enic_rx.c           |  2 +-
>  drivers/net/i40e/i40e_rxtx.c         |  2 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c     |  7 +++++
>  drivers/net/ixgbe/ixgbe_rxtx.c       | 21 +++++++++++----
>  drivers/net/ixgbe/ixgbe_rxtx.h       |  1 +
>  drivers/net/mlx5/mlx5_rxtx.c         |  6 +++--
>  drivers/net/nfp/nfp_net.c            |  2 +-
>  drivers/net/vmxnet3/vmxnet3_rxtx.c   |  2 +-
>  lib/librte_mbuf/rte_mbuf.c           |  2 ++
>  lib/librte_mbuf/rte_mbuf.h           | 50 ++++++++++++++++++++++++++++++++----
>  14 files changed, 90 insertions(+), 20 deletions(-)
> 
> diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
> index 14555ab..c69b344 100644
> --- a/app/test-pmd/rxonly.c
> +++ b/app/test-pmd/rxonly.c
> @@ -156,9 +156,9 @@ pkt_burst_receive(struct fwd_stream *fs)
>  				printf("hash=0x%x ID=0x%x ",
>  				       mb->hash.fdir.hash, mb->hash.fdir.id);
>  		}
> -		if (ol_flags & PKT_RX_VLAN_PKT)
> +		if (ol_flags & PKT_RX_VLAN_STRIPPED)
>  			printf(" - VLAN tci=0x%x", mb->vlan_tci);
> -		if (ol_flags & PKT_RX_QINQ_PKT)
> +		if (ol_flags & PKT_RX_QINQ_STRIPPED)
>  			printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
>  					mb->vlan_tci, mb->vlan_tci_outer);
>  		if (mb->packet_type) {
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index ad05eba..2233a90 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -57,3 +57,8 @@ Deprecation Notices
>    a handle, like the way kernel exposes an fd to user for locating a
>    specific file, and to keep all major structures internally, so that
>    we are likely to be free from ABI violations in future.
> +
> +* The mbuf flags PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated and
> +  are respectively replaced by PKT_RX_VLAN_STRIPPED and
> +  PKT_RX_QINQ_STRIPPED, that are better described. The old flags and
> +  their behavior will be kept in 16.07 and will be removed in 16.11.
> diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
> index 3d36f21..6d8750a 100644
> --- a/drivers/net/e1000/em_rxtx.c
> +++ b/drivers/net/e1000/em_rxtx.c
> @@ -629,7 +629,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
>  	uint64_t pkt_flags;
> 
>  	/* Check if VLAN present */
> -	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0);
> +	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
> +		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
> 
>  	return pkt_flags;
>  }
> diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
> index 18aeead..9d80a0b 100644
> --- a/drivers/net/e1000/igb_rxtx.c
> +++ b/drivers/net/e1000/igb_rxtx.c
> @@ -729,7 +729,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
>  	uint64_t pkt_flags;
> 
>  	/* Check if VLAN present */
> -	pkt_flags = (rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
> +	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
> +		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
> 
>  #if defined(RTE_LIBRTE_IEEE1588)
>  	if (rx_status & E1000_RXD_STAT_TMST)
> diff --git a/drivers/net/enic/enic_rx.c b/drivers/net/enic/enic_rx.c
> index f92f6bc..6459e97 100644
> --- a/drivers/net/enic/enic_rx.c
> +++ b/drivers/net/enic/enic_rx.c
> @@ -197,7 +197,7 @@ enic_cq_rx_to_pkt_flags(struct cq_desc *cqd, struct rte_mbuf *mbuf)
> 
>  	/* VLAN stripping */
>  	if (bwflags & CQ_ENET_RQ_DESC_FLAGS_VLAN_STRIPPED) {
> -		pkt_flags |= PKT_RX_VLAN_PKT;
> +		pkt_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
>  		mbuf->vlan_tci = enic_cq_rx_desc_vlan(cqrd);
>  	} else {
>  		mbuf->vlan_tci = 0;
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index c833aa3..aa161a9 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -99,7 +99,7 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union i40e_rx_desc *rxdp)
>  #ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
>  	if (rte_le_to_cpu_16(rxdp->wb.qword2.ext_status) &
>  		(1 << I40E_RX_DESC_EXT_STATUS_L2TAG2P_SHIFT)) {
> -		mb->ol_flags |= PKT_RX_QINQ_PKT;
> +		mb->ol_flags |= PKT_RX_QINQ_STRIPPED;
>  		mb->vlan_tci_outer = mb->vlan_tci;
>  		mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_2);
>  		PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u",
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index a2b170b..e7717e3 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1636,6 +1636,7 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
>  {
>  	struct ixgbe_hwstrip *hwstrip =
>  		IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(dev->data->dev_private);
> +	struct ixgbe_rx_queue *rxq;
> 
>  	if (queue >= IXGBE_MAX_RX_QUEUE_NUM)
>  		return;
> @@ -1644,6 +1645,12 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
>  		IXGBE_SET_HWSTRIP(hwstrip, queue);
>  	else
>  		IXGBE_CLEAR_HWSTRIP(hwstrip, queue);
> +
> +	if (queue >= dev->data->nb_rx_queues)
> +		return;
> +
> +	rxq = dev->data->rx_queues[queue];
> +	rxq->vlan_strip = on;
>  }
> 
>  static void
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 9c6eaf2..3d740df 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -1221,16 +1221,23 @@ ixgbe_rxd_pkt_info_to_pkt_flags(uint16_t pkt_info)
>  }
> 
>  static inline uint64_t
> -rx_desc_status_to_pkt_flags(uint32_t rx_status)
> +rx_desc_status_to_pkt_flags(uint32_t rx_status, uint8_t vlan_strip)
>  {
>  	uint64_t pkt_flags;
> +	uint64_t vlan_flags;
> +
> +	/* if vlan is stripped, set the proper flag */
> +	if (vlan_strip)
> +		vlan_flags = PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
> +	else
> +		vlan_flags = PKT_RX_VLAN_PKT;
> 
>  	/*
>  	 * Check if VLAN present only.
>  	 * Do not check whether L3/L4 rx checksum done by NIC or not,
>  	 * That can be found from rte_eth_rxmode.hw_ip_checksum flag
>  	 */
> -	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
> +	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  vlan_flags : 0;


Instead of storing in rxq (and passing as a parameter) a bool value for vlan_strip (=on/off),
you probably can store in rxq and pass as a parameter here uint64_t vlan_flags;
Then it will be:
 
rx_desc_status_to_pkt_flags(uint32_t rx_status, uint64_t vlan_flags)
{
   ...
   pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  vlan_flags : 0;
   ...
}

...
pkt_flags = rx_desc_status_to_pkt_flags(s[j], rxq->vlan_flags);

Might help to save few cycles here.
Konstantin

> 
>  #ifdef RTE_LIBRTE_IEEE1588
>  	if (rx_status & IXGBE_RXD_STAT_TMST)
> @@ -1287,6 +1294,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
>  	uint32_t pkt_info[LOOK_AHEAD];
>  	int i, j, nb_rx = 0;
>  	uint32_t status;
> +	uint8_t vlan_strip = rxq->vlan_strip;
> 
>  	/* get references to current descriptor and S/W ring entry */
>  	rxdp = &rxq->rx_ring[rxq->rx_tail];
> @@ -1328,7 +1336,8 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
>  			mb->vlan_tci = rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
> 
>  			/* convert descriptor fields to rte mbuf flags */
> -			pkt_flags = rx_desc_status_to_pkt_flags(s[j]);
> +			pkt_flags = rx_desc_status_to_pkt_flags(s[j],
> +				vlan_strip);
>  			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
>  			pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags
>  					((uint16_t)pkt_info[j]);
> @@ -1544,6 +1553,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  	uint16_t nb_rx;
>  	uint16_t nb_hold;
>  	uint64_t pkt_flags;
> +	uint8_t vlan_strip;
> 
>  	nb_rx = 0;
>  	nb_hold = 0;
> @@ -1551,6 +1561,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  	rx_id = rxq->rx_tail;
>  	rx_ring = rxq->rx_ring;
>  	sw_ring = rxq->sw_ring;
> +	vlan_strip = rxq->vlan_strip;
>  	while (nb_rx < nb_pkts) {
>  		/*
>  		 * The order of operations here is important as the DD status
> @@ -1660,7 +1671,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		/* Only valid if PKT_RX_VLAN_PKT set in pkt_flags */
>  		rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan);
> 
> -		pkt_flags = rx_desc_status_to_pkt_flags(staterr);
> +		pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_strip);
>  		pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr);
>  		pkt_flags = pkt_flags |
>  			ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
> @@ -1753,7 +1764,7 @@ ixgbe_fill_cluster_head_buf(
>  	 */
>  	head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
>  	pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
> -	pkt_flags = rx_desc_status_to_pkt_flags(staterr);
> +	pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_strip);
>  	pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
>  	pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
>  	head->ol_flags = pkt_flags;
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
> index 3691a19..9ca0e8b 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> @@ -146,6 +146,7 @@ struct ixgbe_rx_queue {
>  	uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
>  	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
>  	uint8_t             rx_deferred_start; /**< not in global dev start. */
> +	uint8_t             vlan_strip; /**< 1 if vlan stripping enabled. */
>  	/** need to alloc dummy mbuf, for wraparound when scanning hw ring */
>  	struct rte_mbuf fake_mbuf;
>  	/** hold packets to return to application */

Re: [PATCH] mbuf: new flag when Vlan is stripped

[Olivier Matz]

>>  static inline uint64_t
>> -rx_desc_status_to_pkt_flags(uint32_t rx_status)
>> +rx_desc_status_to_pkt_flags(uint32_t rx_status, uint8_t vlan_strip)
>>  {
>>  	uint64_t pkt_flags;
>> +	uint64_t vlan_flags;
>> +
>> +	/* if vlan is stripped, set the proper flag */
>> +	if (vlan_strip)
>> +		vlan_flags = PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
>> +	else
>> +		vlan_flags = PKT_RX_VLAN_PKT;
>>
>>  	/*
>>  	 * Check if VLAN present only.
>>  	 * Do not check whether L3/L4 rx checksum done by NIC or not,
>>  	 * That can be found from rte_eth_rxmode.hw_ip_checksum flag
>>  	 */
>> -	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
>> +	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  vlan_flags : 0;
> 
> 
> Instead of storing in rxq (and passing as a parameter) a bool value for vlan_strip (=on/off),
> you probably can store in rxq and pass as a parameter here uint64_t vlan_flags;
> Then it will be:
>  
> rx_desc_status_to_pkt_flags(uint32_t rx_status, uint64_t vlan_flags)
> {
>    ...
>    pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  vlan_flags : 0;
>    ...
> }
> 
> ...
> pkt_flags = rx_desc_status_to_pkt_flags(s[j], rxq->vlan_flags);
> 
> Might help to save few cycles here.

Yep, will do in v2.


Olivier

[PATCH v2] mbuf: new flag when Vlan is stripped

[Olivier Matz]

The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
PMDs not advertising the same flags in similar conditions.

Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
and PKT_RX_QINQ_STRIPPED that are better defined:

  PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
  tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
  is enabled in the RX configuration of the PMD.

For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
It should be removed from applications and PMDs in a future revision.

This patch also updates the drivers. For PKT_RX_VLAN_PKT:

- e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
  had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
  required.
- fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
  PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
- ixgbe: modification done (vector and normal), the old flag was set
  when a vlan was recognized, even if vlan stripping was disabled.
- the other drivers do not support vlan stripping.

For PKT_RX_QINQ_PKT, it was only supported on i40e, and the behavior was
already correct, so we can reuse the same bit value for
PKT_RX_QINQ_STRIPPED.

[1] http://dpdk.org/ml/archives/dev/2016-April/037837.html,

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

v1 -> v2:
- fix ixgbe (vector mode) and i40e (normal and vector mode)
- store vlan flags instead of a boolean value in ixgbe rxq, as
  suggested by Konstantin
- replay tests on ixgbe (normal + vector) and i40e (normal +
  vector). See below.

RFC -> v1:
- fix checkpatch and check-git-log.sh issues
- add a deprecation notice for the old vlan flags
- rebase on head


This patch is tested on ixgbe (normal + vector), i40e (normal +
vector) and igb (hardware is a 82575):

  # we use scapy to send packets like this:
  # Ether(src="00:01:02:03:04:05", dst="00:1B:21:AB:8F:10")/Dot1Q(vlan=0x666)/IP()/UDP()/Raw("x"*32)

  cd dpdk.org/
  make config T=x86_64-native-linuxapp-gcc
  make -j32
  mkdir -p /mnt/huge
  mount -t hugetlbfs nodev /mnt/huge
  echo 256 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
  modprobe uio_pci_generic

  # test-pmd is started with vlan stripping, using the rx-vector
  # function if available (i40e and ixgbe)
  ./build/app/testpmd -l 2,4 -- --total-num-mbufs=65536 -i --port-topology=chained \
    --disable-hw-vlan-filter
  # to disable vlan stripping, add:
  --disable-hw-vlan-strip
  # to disable the vector mode (it can be checked in debug logs), add:
  --enable-rx-cksum

  # we run test-pmd in rxonly mode, displaying the packet information.
  set fwd rxonly
  set verbose 1
  start

==== IXGBE normal rx function

  # ixgbe: the behavior of the flag PKT_RX_VLAN_PKT is kept as before,
  # and the new flag PKT_RX_VLAN_STRIPPED is introduced when vlan stripping
  # is enabled and a vlan is stripped.

--- vlan stripping enabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED

--- vlan stripping disabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT

==== IXGBE vector rx function

--- vlan stripping enabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1Unknown packet type
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666Unknown packet type
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED

--- vlan stripping disabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1Unknown packet type
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1Unknown packet type
 - Receive queue=0x0
  PKT_RX_VLAN_PKT

==== I40E normal rx function

--- vlan stripping enabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED

--- vlan stripping disabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown            
 - Receive queue=0x0     

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 

==== I40E vector rx function

--- vlan stripping enabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1Unknown packet type
 - Receive queue=0x0     

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666Unknown packet type
 - Receive queue=0x0     
  PKT_RX_VLAN_PKT        
  PKT_RX_VLAN_STRIPPED   

--- vlan stripping disabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1Unknown packet type
 - Receive queue=0x0
port 0/queue 0: received 1 packets

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1Unknown packet type
 - Receive queue=0x0

==== IGB

(not retested since RFC patch, but there was no code modification)

--- vlan stripping enabled

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

--- vlan stripping disabled

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED


 app/test-pmd/rxonly.c                |  4 +--
 doc/guides/rel_notes/deprecation.rst |  5 ++++
 drivers/net/e1000/em_rxtx.c          |  3 ++-
 drivers/net/e1000/igb_rxtx.c         |  3 ++-
 drivers/net/enic/enic_rx.c           |  2 +-
 drivers/net/i40e/i40e_rxtx.c         |  4 +--
 drivers/net/i40e/i40e_rxtx_vec.c     |  2 +-
 drivers/net/ixgbe/ixgbe_ethdev.c     | 11 ++++++++
 drivers/net/ixgbe/ixgbe_rxtx.c       | 14 ++++++----
 drivers/net/ixgbe/ixgbe_rxtx.h       |  2 ++
 drivers/net/ixgbe/ixgbe_rxtx_vec.c   | 36 +++++++++++++++++---------
 drivers/net/mlx5/mlx5_rxtx.c         |  6 +++--
 drivers/net/nfp/nfp_net.c            |  2 +-
 drivers/net/vmxnet3/vmxnet3_rxtx.c   |  2 +-
 lib/librte_mbuf/rte_mbuf.c           |  2 ++
 lib/librte_mbuf/rte_mbuf.h           | 50 ++++++++++++++++++++++++++++++++----
 16 files changed, 114 insertions(+), 34 deletions(-)

diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index 14555ab..c69b344 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -156,9 +156,9 @@ pkt_burst_receive(struct fwd_stream *fs)
 				printf("hash=0x%x ID=0x%x ",
 				       mb->hash.fdir.hash, mb->hash.fdir.id);
 		}
-		if (ol_flags & PKT_RX_VLAN_PKT)
+		if (ol_flags & PKT_RX_VLAN_STRIPPED)
 			printf(" - VLAN tci=0x%x", mb->vlan_tci);
-		if (ol_flags & PKT_RX_QINQ_PKT)
+		if (ol_flags & PKT_RX_QINQ_STRIPPED)
 			printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
 					mb->vlan_tci, mb->vlan_tci_outer);
 		if (mb->packet_type) {
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index ad05eba..2233a90 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -57,3 +57,8 @@ Deprecation Notices
   a handle, like the way kernel exposes an fd to user for locating a
   specific file, and to keep all major structures internally, so that
   we are likely to be free from ABI violations in future.
+
+* The mbuf flags PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated and
+  are respectively replaced by PKT_RX_VLAN_STRIPPED and
+  PKT_RX_QINQ_STRIPPED, that are better described. The old flags and
+  their behavior will be kept in 16.07 and will be removed in 16.11.
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 3d36f21..6d8750a 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -629,7 +629,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	uint64_t pkt_flags;
 
 	/* Check if VLAN present */
-	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0);
+	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
+		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
 
 	return pkt_flags;
 }
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 18aeead..9d80a0b 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -729,7 +729,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	uint64_t pkt_flags;
 
 	/* Check if VLAN present */
-	pkt_flags = (rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
+	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
+		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
 
 #if defined(RTE_LIBRTE_IEEE1588)
 	if (rx_status & E1000_RXD_STAT_TMST)
diff --git a/drivers/net/enic/enic_rx.c b/drivers/net/enic/enic_rx.c
index f92f6bc..6459e97 100644
--- a/drivers/net/enic/enic_rx.c
+++ b/drivers/net/enic/enic_rx.c
@@ -197,7 +197,7 @@ enic_cq_rx_to_pkt_flags(struct cq_desc *cqd, struct rte_mbuf *mbuf)
 
 	/* VLAN stripping */
 	if (bwflags & CQ_ENET_RQ_DESC_FLAGS_VLAN_STRIPPED) {
-		pkt_flags |= PKT_RX_VLAN_PKT;
+		pkt_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		mbuf->vlan_tci = enic_cq_rx_desc_vlan(cqrd);
 	} else {
 		mbuf->vlan_tci = 0;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index c833aa3..eea246b 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -88,7 +88,7 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union i40e_rx_desc *rxdp)
 {
 	if (rte_le_to_cpu_64(rxdp->wb.qword1.status_error_len) &
 		(1 << I40E_RX_DESC_STATUS_L2TAG1P_SHIFT)) {
-		mb->ol_flags |= PKT_RX_VLAN_PKT;
+		mb->ol_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		mb->vlan_tci =
 			rte_le_to_cpu_16(rxdp->wb.qword0.lo_dword.l2tag1);
 		PMD_RX_LOG(DEBUG, "Descriptor l2tag1: %u",
@@ -99,7 +99,7 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union i40e_rx_desc *rxdp)
 #ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
 	if (rte_le_to_cpu_16(rxdp->wb.qword2.ext_status) &
 		(1 << I40E_RX_DESC_EXT_STATUS_L2TAG2P_SHIFT)) {
-		mb->ol_flags |= PKT_RX_QINQ_PKT;
+		mb->ol_flags |= PKT_RX_QINQ_STRIPPED;
 		mb->vlan_tci_outer = mb->vlan_tci;
 		mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_2);
 		PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u",
diff --git a/drivers/net/i40e/i40e_rxtx_vec.c b/drivers/net/i40e/i40e_rxtx_vec.c
index eef80d9..634bd39 100644
--- a/drivers/net/i40e/i40e_rxtx_vec.c
+++ b/drivers/net/i40e/i40e_rxtx_vec.c
@@ -154,7 +154,7 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	/* map rss and vlan type to rss hash and vlan flag */
 	const __m128i vlan_flags = _mm_set_epi8(0, 0, 0, 0,
 			0, 0, 0, 0,
-			0, 0, 0, PKT_RX_VLAN_PKT,
+			0, 0, 0, PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED,
 			0, 0, 0, 0);
 
 	const __m128i rss_flags = _mm_set_epi8(0, 0, 0, 0,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a2b170b..5f3e047 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1636,6 +1636,7 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 {
 	struct ixgbe_hwstrip *hwstrip =
 		IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(dev->data->dev_private);
+	struct ixgbe_rx_queue *rxq;
 
 	if (queue >= IXGBE_MAX_RX_QUEUE_NUM)
 		return;
@@ -1644,6 +1645,16 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 		IXGBE_SET_HWSTRIP(hwstrip, queue);
 	else
 		IXGBE_CLEAR_HWSTRIP(hwstrip, queue);
+
+	if (queue >= dev->data->nb_rx_queues)
+		return;
+
+	rxq = dev->data->rx_queues[queue];
+
+	if (on)
+		rxq->vlan_flags = PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
+	else
+		rxq->vlan_flags = PKT_RX_VLAN_PKT;
 }
 
 static void
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 9c6eaf2..5a7064c 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1221,7 +1221,7 @@ ixgbe_rxd_pkt_info_to_pkt_flags(uint16_t pkt_info)
 }
 
 static inline uint64_t
-rx_desc_status_to_pkt_flags(uint32_t rx_status)
+rx_desc_status_to_pkt_flags(uint32_t rx_status, uint64_t vlan_flags)
 {
 	uint64_t pkt_flags;
 
@@ -1230,7 +1230,7 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	 * Do not check whether L3/L4 rx checksum done by NIC or not,
 	 * That can be found from rte_eth_rxmode.hw_ip_checksum flag
 	 */
-	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
+	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  vlan_flags : 0;
 
 #ifdef RTE_LIBRTE_IEEE1588
 	if (rx_status & IXGBE_RXD_STAT_TMST)
@@ -1287,6 +1287,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 	uint32_t pkt_info[LOOK_AHEAD];
 	int i, j, nb_rx = 0;
 	uint32_t status;
+	uint64_t vlan_flags = rxq->vlan_flags;
 
 	/* get references to current descriptor and S/W ring entry */
 	rxdp = &rxq->rx_ring[rxq->rx_tail];
@@ -1328,7 +1329,8 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 			mb->vlan_tci = rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
 
 			/* convert descriptor fields to rte mbuf flags */
-			pkt_flags = rx_desc_status_to_pkt_flags(s[j]);
+			pkt_flags = rx_desc_status_to_pkt_flags(s[j],
+				vlan_flags);
 			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
 			pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags
 					((uint16_t)pkt_info[j]);
@@ -1544,6 +1546,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	uint16_t nb_rx;
 	uint16_t nb_hold;
 	uint64_t pkt_flags;
+	uint64_t vlan_flags;
 
 	nb_rx = 0;
 	nb_hold = 0;
@@ -1551,6 +1554,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	rx_id = rxq->rx_tail;
 	rx_ring = rxq->rx_ring;
 	sw_ring = rxq->sw_ring;
+	vlan_flags = rxq->vlan_flags;
 	while (nb_rx < nb_pkts) {
 		/*
 		 * The order of operations here is important as the DD status
@@ -1660,7 +1664,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		/* Only valid if PKT_RX_VLAN_PKT set in pkt_flags */
 		rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan);
 
-		pkt_flags = rx_desc_status_to_pkt_flags(staterr);
+		pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_flags);
 		pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr);
 		pkt_flags = pkt_flags |
 			ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
@@ -1753,7 +1757,7 @@ ixgbe_fill_cluster_head_buf(
 	 */
 	head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
 	pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
-	pkt_flags = rx_desc_status_to_pkt_flags(staterr);
+	pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_flags);
 	pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
 	pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 	head->ol_flags = pkt_flags;
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 3691a19..2608b36 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -146,6 +146,8 @@ struct ixgbe_rx_queue {
 	uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
 	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
 	uint8_t             rx_deferred_start; /**< not in global dev start. */
+	/** flags to set in mbuf when a vlan is detected. */
+	uint64_t            vlan_flags;
 	/** need to alloc dummy mbuf, for wraparound when scanning hw ring */
 	struct rte_mbuf fake_mbuf;
 	/** hold packets to return to application */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
index e97ea82..d895bf1 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
@@ -140,10 +140,9 @@ ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq)
  */
 #ifdef RTE_IXGBE_RX_OLFLAGS_ENABLE
 
-#define VTAG_SHIFT     (3)
-
 static inline void
-desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
+desc_to_olflags_v(__m128i descs[4], uint8_t vlan_flags,
+	struct rte_mbuf **rx_pkts)
 {
 	__m128i ptype0, ptype1, vtag0, vtag1;
 	union {
@@ -151,12 +150,6 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 		uint64_t dword;
 	} vol;
 
-	/* pkt type + vlan olflags mask */
-	const __m128i pkttype_msk = _mm_set_epi16(
-			0x0000, 0x0000, 0x0000, 0x0000,
-			PKT_RX_VLAN_PKT, PKT_RX_VLAN_PKT,
-			PKT_RX_VLAN_PKT, PKT_RX_VLAN_PKT);
-
 	/* mask everything except rss type */
 	const __m128i rsstype_msk = _mm_set_epi16(
 			0x0000, 0x0000, 0x0000, 0x0000,
@@ -168,6 +161,19 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 			PKT_RX_RSS_HASH, 0, PKT_RX_RSS_HASH, 0,
 			PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, 0);
 
+	/* mask everything except vlan present bit */
+	const __m128i vlan_msk = _mm_set_epi16(
+			0x0000, 0x0000,
+			0x0000, 0x0000,
+			IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
+			IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP);
+	/* map vlan present (0x8) to ol_flags */
+	const __m128i vlan_map = _mm_set_epi8(
+		0, 0, 0, 0,
+		0, 0, 0, vlan_flags,
+		0, 0, 0, 0,
+		0, 0, 0, 0);
+
 	ptype0 = _mm_unpacklo_epi16(descs[0], descs[1]);
 	ptype1 = _mm_unpacklo_epi16(descs[2], descs[3]);
 	vtag0 = _mm_unpackhi_epi16(descs[0], descs[1]);
@@ -178,8 +184,8 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	ptype0 = _mm_shuffle_epi8(rss_flags, ptype0);
 
 	vtag1 = _mm_unpacklo_epi32(vtag0, vtag1);
-	vtag1 = _mm_srli_epi16(vtag1, VTAG_SHIFT);
-	vtag1 = _mm_and_si128(vtag1, pkttype_msk);
+	vtag1 = _mm_and_si128(vtag1, vlan_msk);
+	vtag1 = _mm_shuffle_epi8(vlan_map, vtag1);
 
 	vtag1 = _mm_or_si128(ptype0, vtag1);
 	vol.dword = _mm_cvtsi128_si64(vtag1);
@@ -221,6 +227,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 				0, 0            /* ignore pkt_type field */
 			);
 	__m128i dd_check, eop_check;
+	uint8_t vlan_flags;
 
 	/* nb_pkts shall be less equal than RTE_IXGBE_MAX_RX_BURST */
 	nb_pkts = RTE_MIN(nb_pkts, RTE_IXGBE_MAX_RX_BURST);
@@ -270,6 +277,11 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	 */
 	sw_ring = &rxq->sw_ring[rxq->rx_tail];
 
+	/* ensure these 2 flags are in the lower 8 bits */
+	RTE_BUILD_BUG_ON(((PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED) &
+			0xffffffffffffff00ULL) != 0ULL);
+	vlan_flags = rxq->vlan_flags & 0xff;
+
 	/* A. load 4 packet in one loop
 	 * [A*. mask out 4 unused dirty field in desc]
 	 * B. copy 4 mbuf point from swring to rx_pkts
@@ -330,7 +342,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		sterr_tmp1 = _mm_unpackhi_epi32(descs[1], descs[0]);
 
 		/* set ol_flags with vlan packet type */
-		desc_to_olflags_v(descs, &rx_pkts[pos]);
+		desc_to_olflags_v(descs, vlan_flags, &rx_pkts[pos]);
 
 		/* D.2 pkt 3,4 set in_port/nb_seg and remove crc */
 		pkt_mb4 = _mm_add_epi16(pkt_mb4, crc_adjust);
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 29bfcec..d5b2286 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1051,7 +1051,8 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 			pkt_buf->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
 #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
 			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
-				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT;
+				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT |
+					PKT_RX_VLAN_STRIPPED;
 				pkt_buf->vlan_tci = vlan_tci;
 			}
 #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
@@ -1207,7 +1208,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 			seg->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
 #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
 			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
-				seg->ol_flags |= PKT_RX_VLAN_PKT;
+				seg->ol_flags |= PKT_RX_VLAN_PKT |
+					PKT_RX_VLAN_STRIPPED;
 				seg->vlan_tci = vlan_tci;
 			}
 #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index ea5a2a3..5c9f350 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1800,7 +1800,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		if ((rxds->rxd.flags & PCIE_DESC_RX_VLAN) &&
 		    (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) {
 			mb->vlan_tci = rte_cpu_to_le_32(rxds->rxd.vlan);
-			mb->ol_flags |= PKT_RX_VLAN_PKT;
+			mb->ol_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		}
 
 		/* Adding the mbuff to the mbuff array passed by the app */
diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index 9fe8752..ccafc0c 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -579,7 +579,7 @@ vmxnet3_rx_offload(const Vmxnet3_RxCompDesc *rcd, struct rte_mbuf *rxm)
 {
 	/* Check for hardware stripped VLAN tag */
 	if (rcd->ts) {
-		rxm->ol_flags |= PKT_RX_VLAN_PKT;
+		rxm->ol_flags |= (PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED);
 		rxm->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci);
 	}
 
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index eec1456..2ece742 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -258,8 +258,10 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
 	/* case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW"; */
 	/* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */
 	/* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */
+	case PKT_RX_VLAN_STRIPPED: return "PKT_RX_VLAN_STRIPPED";
 	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
 	case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST";
+	case PKT_RX_QINQ_STRIPPED: return "PKT_RX_QINQ_STRIPPED";
 	default: return NULL;
 	}
 }
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 11fa06d..76b4f55 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -79,7 +79,16 @@ extern "C" {
  * Keep these flags synchronized with rte_get_rx_ol_flag_name() and
  * rte_get_tx_ol_flag_name().
  */
-#define PKT_RX_VLAN_PKT      (1ULL << 0)  /**< RX packet is a 802.1q VLAN packet. */
+
+/**
+ * Deprecated.
+ * RX packet is a 802.1q VLAN packet. This flag was set by PMDs when
+ * the packet is recognized as a VLAN, but the behavior between PMDs
+ * was not the same. This flag is kept for some time to avoid breaking
+ * applications and should be replaced by PKT_RX_VLAN_STRIPPED.
+ */
+#define PKT_RX_VLAN_PKT      (1ULL << 0)
+
 #define PKT_RX_RSS_HASH      (1ULL << 1)  /**< RX packet with RSS hash result. */
 #define PKT_RX_FDIR          (1ULL << 2)  /**< RX packet with FDIR match indicate. */
 #define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt. is not OK. */
@@ -89,11 +98,37 @@ extern "C" {
 #define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer overflow. */
 #define PKT_RX_RECIP_ERR     (0ULL << 0)  /**< Hardware processing error. */
 #define PKT_RX_MAC_ERR       (0ULL << 0)  /**< MAC error. */
+
+/**
+ * A vlan has been stripped by the hardware and its tci is saved in
+ * mbuf->vlan_tci. This can only happen if vlan stripping is enabled
+ * in the RX configuration of the PMD.
+ */
+#define PKT_RX_VLAN_STRIPPED (1ULL << 6)
+
+/* hole, some bits can be reused here  */
+
 #define PKT_RX_IEEE1588_PTP  (1ULL << 9)  /**< RX IEEE1588 L2 Ethernet PT Packet. */
 #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588 L2/L4 timestamped packet.*/
 #define PKT_RX_FDIR_ID       (1ULL << 13) /**< FD id reported if FDIR match. */
 #define PKT_RX_FDIR_FLX      (1ULL << 14) /**< Flexible bytes reported if FDIR match. */
-#define PKT_RX_QINQ_PKT      (1ULL << 15)  /**< RX packet with double VLAN stripped. */
+
+/**
+ * The 2 vlans have been stripped by the hardware and their tci are
+ * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
+ * This can only happen if vlan stripping is enabled in the RX
+ * configuration of the PMD. If this flag is set, PKT_RX_VLAN_STRIPPED
+ * must also be set.
+ */
+#define PKT_RX_QINQ_STRIPPED (1ULL << 15)
+
+/**
+ * Deprecated.
+ * RX packet with double VLAN stripped.
+ * This flag is replaced by PKT_RX_QINQ_STRIPPED.
+ */
+#define PKT_RX_QINQ_PKT      PKT_RX_QINQ_STRIPPED
+
 /* add new RX flags here */
 
 /* add new TX flags here */
@@ -761,7 +796,10 @@ struct rte_mbuf {
 
 	/*
 	 * The packet type, which is the combination of outer/inner L2, L3, L4
-	 * and tunnel types.
+	 * and tunnel types. The packet_type is about data really present in the
+	 * mbuf. Example: if vlan stripping is enabled, a received vlan packet
+	 * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the
+	 * vlan is stripped from the data.
 	 */
 	union {
 		uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */
@@ -778,7 +816,8 @@ struct rte_mbuf {
 
 	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
 	uint16_t data_len;        /**< Amount of data in segment buffer. */
-	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
+	/** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */
+	uint16_t vlan_tci;
 
 	union {
 		uint32_t rss;     /**< RSS hash result if RSS enabled */
@@ -804,7 +843,8 @@ struct rte_mbuf {
 
 	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
 
-	uint16_t vlan_tci_outer;  /**< Outer VLAN Tag Control Identifier (CPU order) */
+	/** Outer VLAN TCI (CPU order), valid if PKT_RX_QINQ_STRIPPED is set. */
+	uint16_t vlan_tci_outer;
 
 	/* second cache line - fields only used in slow path or on TX */
 	MARKER cacheline1 __rte_cache_min_aligned;
-- 
2.8.0.rc3

Re: [PATCH v2] mbuf: new flag when Vlan is stripped

[Olivier Matz]

Hi,

On 05/27/2016 04:33 PM, Olivier Matz wrote:
> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
> PMDs not advertising the same flags in similar conditions.
> 
> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
> and PKT_RX_QINQ_STRIPPED that are better defined:
> 
>   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
>   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
>   is enabled in the RX configuration of the PMD.
> 
> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
> It should be removed from applications and PMDs in a future revision.
> 
> This patch also updates the drivers. For PKT_RX_VLAN_PKT:
> 
> - e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
>   had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
>   required.
> - fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
>   PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
> - ixgbe: modification done (vector and normal), the old flag was set
>   when a vlan was recognized, even if vlan stripping was disabled.
> - the other drivers do not support vlan stripping.
> 
> For PKT_RX_QINQ_PKT, it was only supported on i40e, and the behavior was
> already correct, so we can reuse the same bit value for
> PKT_RX_QINQ_STRIPPED.
> 
> [1] http://dpdk.org/ml/archives/dev/2016-April/037837.html,
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Any comment about this patch?

Re: [PATCH v2] mbuf: new flag when Vlan is stripped

[Ananyev, Konstantin]

Hi Olivier,

> 
> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
> PMDs not advertising the same flags in similar conditions.
> 
> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
> and PKT_RX_QINQ_STRIPPED that are better defined:
> 
>   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
>   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
>   is enabled in the RX configuration of the PMD.
> 
> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
> It should be removed from applications and PMDs in a future revision.

I am not sure it has to be deprecated & removed.
ixgbe (and igb as I can read the specs) devices can provide information is that
a vlan packet or not even when vlan stripping is disabled. 
Right now ixgbe PMD do carry thins information to the user,
and I suppose igb could be improved to carry it too.
So obviously we need a way to pass that information to the upper layer.
I remember it was a discussion about introducing new packet_type
instead of ol_flag value PKT_RX_VLAN_PKT.
But right now it is not there, and again I don't know how easy it would be to replace
one with another without performance considering that packet_type is not supported
now by ixgbe vRX.
If we would be able to replace it, then yes we can deprecate and drop the   PKT_RX_VLAN_PKT.
But till then, I think we'd better keep it.

> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> index e97ea82..d895bf1 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> @@ -140,10 +140,9 @@ ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq)
>   */
>  #ifdef RTE_IXGBE_RX_OLFLAGS_ENABLE
> 
> -#define VTAG_SHIFT     (3)
> -
>  static inline void
> -desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
> +desc_to_olflags_v(__m128i descs[4], uint8_t vlan_flags,
> +	struct rte_mbuf **rx_pkts)
>  {
>  	__m128i ptype0, ptype1, vtag0, vtag1;
>  	union {
> @@ -151,12 +150,6 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
>  		uint64_t dword;
>  	} vol;
> 
> -	/* pkt type + vlan olflags mask */
> -	const __m128i pkttype_msk = _mm_set_epi16(
> -			0x0000, 0x0000, 0x0000, 0x0000,
> -			PKT_RX_VLAN_PKT, PKT_RX_VLAN_PKT,
> -			PKT_RX_VLAN_PKT, PKT_RX_VLAN_PKT);
> -
>  	/* mask everything except rss type */
>  	const __m128i rsstype_msk = _mm_set_epi16(
>  			0x0000, 0x0000, 0x0000, 0x0000,
> @@ -168,6 +161,19 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
>  			PKT_RX_RSS_HASH, 0, PKT_RX_RSS_HASH, 0,
>  			PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, 0);
> 
> +	/* mask everything except vlan present bit */
> +	const __m128i vlan_msk = _mm_set_epi16(
> +			0x0000, 0x0000,
> +			0x0000, 0x0000,
> +			IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
> +			IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP);
> +	/* map vlan present (0x8) to ol_flags */
> +	const __m128i vlan_map = _mm_set_epi8(
> +		0, 0, 0, 0,
> +		0, 0, 0, vlan_flags,
> +		0, 0, 0, 0,
> +		0, 0, 0, 0);
> +
>  	ptype0 = _mm_unpacklo_epi16(descs[0], descs[1]);
>  	ptype1 = _mm_unpacklo_epi16(descs[2], descs[3]);
>  	vtag0 = _mm_unpackhi_epi16(descs[0], descs[1]);
> @@ -178,8 +184,8 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
>  	ptype0 = _mm_shuffle_epi8(rss_flags, ptype0);
> 
>  	vtag1 = _mm_unpacklo_epi32(vtag0, vtag1);
> -	vtag1 = _mm_srli_epi16(vtag1, VTAG_SHIFT);
> -	vtag1 = _mm_and_si128(vtag1, pkttype_msk);
> +	vtag1 = _mm_and_si128(vtag1, vlan_msk);
> +	vtag1 = _mm_shuffle_epi8(vlan_map, vtag1);
> 
>  	vtag1 = _mm_or_si128(ptype0, vtag1);
>  	vol.dword = _mm_cvtsi128_si64(vtag1);
> @@ -221,6 +227,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
>  				0, 0            /* ignore pkt_type field */
>  			);
>  	__m128i dd_check, eop_check;
> +	uint8_t vlan_flags;
> 
>  	/* nb_pkts shall be less equal than RTE_IXGBE_MAX_RX_BURST */
>  	nb_pkts = RTE_MIN(nb_pkts, RTE_IXGBE_MAX_RX_BURST);
> @@ -270,6 +277,11 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
>  	 */
>  	sw_ring = &rxq->sw_ring[rxq->rx_tail];
> 
> +	/* ensure these 2 flags are in the lower 8 bits */
> +	RTE_BUILD_BUG_ON(((PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED) &
> +			0xffffffffffffff00ULL) != 0ULL);


I suppose your need here:
RTE_BUILD_BUG_ON(((PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED) & UINT8_MAX) == 
PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED));

To make sure both flags are inside first 8 bits?

			
> +	vlan_flags = rxq->vlan_flags & 0xff;
> +

Probably better to do that check/ AND at setup  phase, not run-time?
As a nit: s/0xff/UINT8_MAX/.
Konstantin

Re: [PATCH v2] mbuf: new flag when Vlan is stripped

[Olivier Matz]

Hi Konstantin,

On 06/13/2016 04:42 PM, Ananyev, Konstantin wrote:
>> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
>> PMDs not advertising the same flags in similar conditions.
>>
>> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
>> and PKT_RX_QINQ_STRIPPED that are better defined:
>>
>>   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
>>   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
>>   is enabled in the RX configuration of the PMD.
>>
>> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
>> It should be removed from applications and PMDs in a future revision.
> 
> I am not sure it has to be deprecated & removed.
> ixgbe (and igb as I can read the specs) devices can provide information is that
> a vlan packet or not even when vlan stripping is disabled. 
> Right now ixgbe PMD do carry thins information to the user,
> and I suppose igb could be improved to carry it too.
> So obviously we need a way to pass that information to the upper layer.
> I remember it was a discussion about introducing new packet_type
> instead of ol_flag value PKT_RX_VLAN_PKT.
> But right now it is not there, and again I don't know how easy it would be to replace
> one with another without performance considering that packet_type is not supported
> now by ixgbe vRX.
> If we would be able to replace it, then yes we can deprecate and drop the   PKT_RX_VLAN_PKT.
> But till then, I think we'd better keep it.

I think the packet_type feature would be more appropriate than a flag
for carrying this kind of info.

Currently the behavior of PKT_RX_VLAN_PKT is not properly defined,
and it is not the same on all PMDs. So, from an application
perspective, it's not usable except if it knows that the underlying
PMD is an ixgbe. This is not acceptable for a generic API and that's
why I think this flag, as it is today, should be deprecated.

It won't prevent an application from using the flag right after my
commit, but it will warn the user that the flag should not be used
as is. If someone is willing to work on this feature for 16.11, why
not but again, I think using the packet_type is more appropriate.

The problem is that I don't want to have this flag in this state
forever, and I also don't want to add in rte_mbuf.h a comment
saying "this flag does this on ixgbe and that on other drivers".

If we decide to generalize the ixgbe behavior for all PMDs for this
flag, it will break the applications relying on this flag but with
other PMDs. So for the same reason we added a new PKT_RX_VLAN_STRIPPED
we cannot change the behavior of an existing flag.



>> @@ -270,6 +277,11 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
>>  	 */
>>  	sw_ring = &rxq->sw_ring[rxq->rx_tail];
>>
>> +	/* ensure these 2 flags are in the lower 8 bits */
>> +	RTE_BUILD_BUG_ON(((PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED) &
>> +			0xffffffffffffff00ULL) != 0ULL);
> 
> 
> I suppose your need here:
> RTE_BUILD_BUG_ON(((PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED) & UINT8_MAX) == 
> PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED));
> 
> To make sure both flags are inside first 8 bits?

Yes, indeed that's much better than 14 'f' :)

>> +	vlan_flags = rxq->vlan_flags & 0xff;
>> +
> 
> Probably better to do that check/ AND at setup  phase, not run-time?
> As a nit: s/0xff/UINT8_MAX/.

Yep, agree, I'll change that.


Thanks for reviewing
Olivier

Re: [PATCH v2] mbuf: new flag when Vlan is stripped

[Ananyev, Konstantin]

Hi Olivier,

> 	
> Hi Konstantin,
> 
> On 06/13/2016 04:42 PM, Ananyev, Konstantin wrote:
> >> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
> >> PMDs not advertising the same flags in similar conditions.
> >>
> >> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
> >> and PKT_RX_QINQ_STRIPPED that are better defined:
> >>
> >>   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
> >>   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
> >>   is enabled in the RX configuration of the PMD.
> >>
> >> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
> >> It should be removed from applications and PMDs in a future revision.
> >
> > I am not sure it has to be deprecated & removed.
> > ixgbe (and igb as I can read the specs) devices can provide information is that
> > a vlan packet or not even when vlan stripping is disabled.
> > Right now ixgbe PMD do carry thins information to the user,
> > and I suppose igb could be improved to carry it too.
> > So obviously we need a way to pass that information to the upper layer.
> > I remember it was a discussion about introducing new packet_type
> > instead of ol_flag value PKT_RX_VLAN_PKT.
> > But right now it is not there, and again I don't know how easy it would be to replace
> > one with another without performance considering that packet_type is not supported
> > now by ixgbe vRX.
> > If we would be able to replace it, then yes we can deprecate and drop the   PKT_RX_VLAN_PKT.
> > But till then, I think we'd better keep it.
> 
> I think the packet_type feature would be more appropriate than a flag
> for carrying this kind of info.
> 
> Currently the behavior of PKT_RX_VLAN_PKT is not properly defined,
> and it is not the same on all PMDs. So, from an application
> perspective, it's not usable except if it knows that the underlying
> PMD is an ixgbe.

Yes, but it might be apps which do use that ixgbe functionality.

> This is not acceptable for a generic API and that's
> why I think this flag, as it is today, should be deprecated.

I suppose we can't deprecate existing functionality without
providing working alternative.
I agree there is no proper way to know right now which device
supports it, which not, but to me it means we should add such ability,
not deprecate existing and (I believe) useful functionality.

> 
> It won't prevent an application from using the flag right after my
> commit, but it will warn the user that the flag should not be used
> as is. If someone is willing to work on this feature for 16.11, why
> not but again, I think using the packet_type is more appropriate.

I am not against providing that information via packet_type.
What I am saying:
1) right now it is not here.
2) it might not that easy in terms of performance.
 
> The problem is that I don't want to have this flag in this state
> forever, and I also don't want to add in rte_mbuf.h a comment
> saying "this flag does this on ixgbe and that on other drivers".

Then we need either:
- implement it as ptype
- add user ability to query is that flag is supported by the underlying device.

> 
> If we decide to generalize the ixgbe behavior for all PMDs for this
> flag, it will break the applications relying on this flag but with
> other PMDs. So for the same reason we added a new PKT_RX_VLAN_STRIPPED
> we cannot change the behavior of an existing flag.

Ok, then let's make PKT_RX_VLAN_STRIPPED == PKT_RX_VLAN,
and assign new value to the  PKT_RX_VLAN.
Or have PKT_RX_VLAN_STRIPPED == PKT_RX_VLAN and create a new one:
PKT_RX_VLAN_PRESENT or so.
?

Konstantin

Re: [PATCH v2] mbuf: new flag when Vlan is stripped

[Olivier MATZ]

Hi Konstantin,

On 06/13/2016 06:31 PM, Ananyev, Konstantin wrote:
>
> Hi Olivier,
>
>> 	
>> Hi Konstantin,
>>
>> On 06/13/2016 04:42 PM, Ananyev, Konstantin wrote:
>>>> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
>>>> PMDs not advertising the same flags in similar conditions.
>>>>
>>>> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
>>>> and PKT_RX_QINQ_STRIPPED that are better defined:
>>>>
>>>>    PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
>>>>    tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
>>>>    is enabled in the RX configuration of the PMD.
>>>>
>>>> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
>>>> It should be removed from applications and PMDs in a future revision.
>>>
>>> I am not sure it has to be deprecated & removed.
>>> ixgbe (and igb as I can read the specs) devices can provide information is that
>>> a vlan packet or not even when vlan stripping is disabled.
>>> Right now ixgbe PMD do carry thins information to the user,
>>> and I suppose igb could be improved to carry it too.
>>> So obviously we need a way to pass that information to the upper layer.
>>> I remember it was a discussion about introducing new packet_type
>>> instead of ol_flag value PKT_RX_VLAN_PKT.
>>> But right now it is not there, and again I don't know how easy it would be to replace
>>> one with another without performance considering that packet_type is not supported
>>> now by ixgbe vRX.
>>> If we would be able to replace it, then yes we can deprecate and drop the   PKT_RX_VLAN_PKT.
>>> But till then, I think we'd better keep it.
>>
>> I think the packet_type feature would be more appropriate than a flag
>> for carrying this kind of info.
>>
>> Currently the behavior of PKT_RX_VLAN_PKT is not properly defined,
>> and it is not the same on all PMDs. So, from an application
>> perspective, it's not usable except if it knows that the underlying
>> PMD is an ixgbe.
>
> Yes, but it might be apps which do use that ixgbe functionality.
>
>> This is not acceptable for a generic API and that's
>> why I think this flag, as it is today, should be deprecated.
>
> I suppose we can't deprecate existing functionality without
> providing working alternative.
> I agree there is no proper way to know right now which device
> supports it, which not, but to me it means we should add such ability,
> not deprecate existing and (I believe) useful functionality.
>
>>
>> It won't prevent an application from using the flag right after my
>> commit, but it will warn the user that the flag should not be used
>> as is. If someone is willing to work on this feature for 16.11, why
>> not but again, I think using the packet_type is more appropriate.
>
> I am not against providing that information via packet_type.
> What I am saying:
> 1) right now it is not here.
> 2) it might not that easy in terms of performance.
>
>> The problem is that I don't want to have this flag in this state
>> forever, and I also don't want to add in rte_mbuf.h a comment
>> saying "this flag does this on ixgbe and that on other drivers".
>
> Then we need either:
> - implement it as ptype
> - add user ability to query is that flag is supported by the underlying device.
>
>>
>> If we decide to generalize the ixgbe behavior for all PMDs for this
>> flag, it will break the applications relying on this flag but with
>> other PMDs. So for the same reason we added a new PKT_RX_VLAN_STRIPPED
>> we cannot change the behavior of an existing flag.
>
> Ok, then let's make PKT_RX_VLAN_STRIPPED == PKT_RX_VLAN,
> and assign new value to the  PKT_RX_VLAN.
> Or have PKT_RX_VLAN_STRIPPED == PKT_RX_VLAN and create a new one:
> PKT_RX_VLAN_PRESENT or so.
> ?
>

I think adding this new flag/packet_type is a new feature,
because only ixgbe was behaving like this, and this was not
documented. To me, marking the old flag as deprecated is
a good compromise to keep the application relying on this
working. If you feel the term "deprecated" is not adapted,
we could reword it to something weaker.

We should try to not stay in that state too long, and anybody
willing to implement this feature would be welcome. For my
part, this is not something I plan to do yet.

Regards,
Olivier

Re: [PATCH v2] mbuf: new flag when Vlan is stripped

[Ananyev, Konstantin]

Hi Olivier

> 
> Hi Konstantin,
> 
> On 06/13/2016 06:31 PM, Ananyev, Konstantin wrote:
> >
> > Hi Olivier,
> >
> >>
> >> Hi Konstantin,
> >>
> >> On 06/13/2016 04:42 PM, Ananyev, Konstantin wrote:
> >>>> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
> >>>> PMDs not advertising the same flags in similar conditions.
> >>>>
> >>>> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
> >>>> and PKT_RX_QINQ_STRIPPED that are better defined:
> >>>>
> >>>>    PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
> >>>>    tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
> >>>>    is enabled in the RX configuration of the PMD.
> >>>>
> >>>> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
> >>>> It should be removed from applications and PMDs in a future revision.
> >>>
> >>> I am not sure it has to be deprecated & removed.
> >>> ixgbe (and igb as I can read the specs) devices can provide information is that
> >>> a vlan packet or not even when vlan stripping is disabled.
> >>> Right now ixgbe PMD do carry thins information to the user,
> >>> and I suppose igb could be improved to carry it too.
> >>> So obviously we need a way to pass that information to the upper layer.
> >>> I remember it was a discussion about introducing new packet_type
> >>> instead of ol_flag value PKT_RX_VLAN_PKT.
> >>> But right now it is not there, and again I don't know how easy it would be to replace
> >>> one with another without performance considering that packet_type is not supported
> >>> now by ixgbe vRX.
> >>> If we would be able to replace it, then yes we can deprecate and drop the   PKT_RX_VLAN_PKT.
> >>> But till then, I think we'd better keep it.
> >>
> >> I think the packet_type feature would be more appropriate than a flag
> >> for carrying this kind of info.
> >>
> >> Currently the behavior of PKT_RX_VLAN_PKT is not properly defined,
> >> and it is not the same on all PMDs. So, from an application
> >> perspective, it's not usable except if it knows that the underlying
> >> PMD is an ixgbe.
> >
> > Yes, but it might be apps which do use that ixgbe functionality.
> >
> >> This is not acceptable for a generic API and that's
> >> why I think this flag, as it is today, should be deprecated.
> >
> > I suppose we can't deprecate existing functionality without
> > providing working alternative.
> > I agree there is no proper way to know right now which device
> > supports it, which not, but to me it means we should add such ability,
> > not deprecate existing and (I believe) useful functionality.
> >
> >>
> >> It won't prevent an application from using the flag right after my
> >> commit, but it will warn the user that the flag should not be used
> >> as is. If someone is willing to work on this feature for 16.11, why
> >> not but again, I think using the packet_type is more appropriate.
> >
> > I am not against providing that information via packet_type.
> > What I am saying:
> > 1) right now it is not here.
> > 2) it might not that easy in terms of performance.
> >
> >> The problem is that I don't want to have this flag in this state
> >> forever, and I also don't want to add in rte_mbuf.h a comment
> >> saying "this flag does this on ixgbe and that on other drivers".
> >
> > Then we need either:
> > - implement it as ptype
> > - add user ability to query is that flag is supported by the underlying device.
> >
> >>
> >> If we decide to generalize the ixgbe behavior for all PMDs for this
> >> flag, it will break the applications relying on this flag but with
> >> other PMDs. So for the same reason we added a new PKT_RX_VLAN_STRIPPED
> >> we cannot change the behavior of an existing flag.
> >
> > Ok, then let's make PKT_RX_VLAN_STRIPPED == PKT_RX_VLAN,
> > and assign new value to the  PKT_RX_VLAN.
> > Or have PKT_RX_VLAN_STRIPPED == PKT_RX_VLAN and create a new one:
> > PKT_RX_VLAN_PRESENT or so.
> > ?
> >
> 
> I think adding this new flag/packet_type is a new feature,
> because only ixgbe was behaving like this, and this was not
> documented. To me, marking the old flag as deprecated is
> a good compromise to keep the application relying on this
> working. If you feel the term "deprecated" is not adapted,
> we could reword it to something weaker.

Yes, that would do I think.
Basically my only concern that we will mark it as deprecated,
and then will remove it (as it is deprecated), without providing
anything new to replace it. 

> 
> We should try to not stay in that state too long,

Agree.

> and anybody willing to implement this feature would be welcome. For my
> part, this is not something I plan to do yet.
> 

Ok, we'll see what we can do for 16.11.
But no hard promises right now either :)

Thanks
Konstantin

> Regards,
> Olivier

Re: [PATCH v2] mbuf: new flag when Vlan is stripped

[Olivier MATZ]

On 06/14/2016 11:15 AM, Ananyev, Konstantin wrote:
>>>> If we decide to generalize the ixgbe behavior for all PMDs for this
>>>> flag, it will break the applications relying on this flag but with
>>>> other PMDs. So for the same reason we added a new PKT_RX_VLAN_STRIPPED
>>>> we cannot change the behavior of an existing flag.
>>>
>>> Ok, then let's make PKT_RX_VLAN_STRIPPED == PKT_RX_VLAN,
>>> and assign new value to the  PKT_RX_VLAN.
>>> Or have PKT_RX_VLAN_STRIPPED == PKT_RX_VLAN and create a new one:
>>> PKT_RX_VLAN_PRESENT or so.
>>> ?
>>>
>>
>> I think adding this new flag/packet_type is a new feature,
>> because only ixgbe was behaving like this, and this was not
>> documented. To me, marking the old flag as deprecated is
>> a good compromise to keep the application relying on this
>> working. If you feel the term "deprecated" is not adapted,
>> we could reword it to something weaker.
>
> Yes, that would do I think.
> Basically my only concern that we will mark it as deprecated,
> and then will remove it (as it is deprecated), without providing
> anything new to replace it.
>
>>
>> We should try to not stay in that state too long,
>
> Agree.
>
>> and anybody willing to implement this feature would be welcome. For my
>> part, this is not something I plan to do yet.
>>
>
> Ok, we'll see what we can do for 16.11.
> But no hard promises right now either :)

Great, thanks :)
I'll send an update of the patch taking your comments in
account.

Olivier

[PATCH v3] mbuf: new flag when Vlan is stripped

[Olivier Matz]

The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
PMDs not advertising the same flags in similar conditions.

Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
and PKT_RX_QINQ_STRIPPED that are better defined:

  PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
  tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
  is enabled in the RX configuration of the PMD.

For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
It should be removed from applications and PMDs in a future revision.

This patch also updates the drivers. For PKT_RX_VLAN_PKT:

- e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
  had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
  required.
- fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
  PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
- ixgbe: modification done (vector and normal), the old flag was set
  when a vlan was recognized, even if vlan stripping was disabled.
- the other drivers do not support vlan stripping.

For PKT_RX_QINQ_PKT, it was only supported on i40e, and the behavior was
already correct, so we can reuse the same bit value for
PKT_RX_QINQ_STRIPPED.

[1] http://dpdk.org/ml/archives/dev/2016-April/037837.html,

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

v2 -> v3:
- remove the "deprecated" notice from PKT_RX_VLAN_PKT comment.
- use UINT8_MAX when appropriate in ixgbe_vec
- simplify check of vlan flags in RTE_BUILD_BUG_ON() in ixgbe_vec

v1 -> v2:
- fix ixgbe (vector mode) and i40e (normal and vector mode)
- store vlan flags instead of a boolean value in ixgbe rxq, as
  suggested by Konstantin
- replay tests on ixgbe (normal + vector) and i40e (normal +
  vector). See below.

RFC -> v1:
- fix checkpatch and check-git-log.sh issues
- add a deprecation notice for the old vlan flags
- rebase on head

 app/test-pmd/rxonly.c                |  4 +--
 doc/guides/rel_notes/deprecation.rst |  5 ++++
 drivers/net/e1000/em_rxtx.c          |  3 ++-
 drivers/net/e1000/igb_rxtx.c         |  3 ++-
 drivers/net/enic/enic_rx.c           |  2 +-
 drivers/net/i40e/i40e_rxtx.c         |  4 +--
 drivers/net/i40e/i40e_rxtx_vec.c     |  2 +-
 drivers/net/ixgbe/ixgbe_ethdev.c     | 11 ++++++++
 drivers/net/ixgbe/ixgbe_rxtx.c       | 14 +++++++----
 drivers/net/ixgbe/ixgbe_rxtx.h       |  2 ++
 drivers/net/ixgbe/ixgbe_rxtx_vec.c   | 35 +++++++++++++++++---------
 drivers/net/mlx5/mlx5_rxtx.c         |  6 +++--
 drivers/net/nfp/nfp_net.c            |  2 +-
 drivers/net/vmxnet3/vmxnet3_rxtx.c   |  2 +-
 lib/librte_mbuf/rte_mbuf.c           |  2 ++
 lib/librte_mbuf/rte_mbuf.h           | 49 ++++++++++++++++++++++++++++++++----
 16 files changed, 112 insertions(+), 34 deletions(-)


This patch is tested on ixgbe (normal + vector), i40e (normal +
vector) and igb (hardware is a 82575):

  # we use scapy to send packets like this:
  # Ether(src="00:01:02:03:04:05", dst="00:1B:21:AB:8F:10")/Dot1Q(vlan=0x666)/IP()/UDP()/Raw("x"*32)

  cd dpdk.org/
  make config T=x86_64-native-linuxapp-gcc
  make -j32
  mkdir -p /mnt/huge
  mount -t hugetlbfs nodev /mnt/huge
  echo 256 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
  modprobe uio_pci_generic

  # test-pmd is started with vlan stripping, using the rx-vector
  # function if available (i40e and ixgbe)
  ./build/app/testpmd -l 2,4 -- --total-num-mbufs=65536 -i --port-topology=chained \
    --disable-hw-vlan-filter
  # to disable vlan stripping, add:
  --disable-hw-vlan-strip
  # to disable the vector mode (it can be checked in debug logs), add:
  --enable-rx-cksum

  # we run test-pmd in rxonly mode, displaying the packet information.
  set fwd rxonly
  set verbose 1
  start

==== IXGBE normal rx function

  # ixgbe: the behavior of the flag PKT_RX_VLAN_PKT is kept as before,
  # and the new flag PKT_RX_VLAN_STRIPPED is introduced when vlan stripping
  # is enabled and a vlan is stripped.

--- vlan stripping enabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED

--- vlan stripping disabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT

==== IXGBE vector rx function

--- vlan stripping enabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1Unknown packet type
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666Unknown packet type
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED

--- vlan stripping disabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1Unknown packet type
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1Unknown packet type
 - Receive queue=0x0
  PKT_RX_VLAN_PKT

==== I40E normal rx function

--- vlan stripping enabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED

--- vlan stripping disabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown            
 - Receive queue=0x0     

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4_EXT_UNKNOWN - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 

==== I40E vector rx function

--- vlan stripping enabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1Unknown packet type
 - Receive queue=0x0     

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666Unknown packet type
 - Receive queue=0x0     
  PKT_RX_VLAN_PKT        
  PKT_RX_VLAN_STRIPPED   

--- vlan stripping disabled

  # packet without vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1Unknown packet type
 - Receive queue=0x0
port 0/queue 0: received 1 packets

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1Unknown packet type
 - Receive queue=0x0

==== IGB

(not retested since RFC patch, but there was no code modification)

--- vlan stripping enabled

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x8100 - length=78 - nb_segs=1 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0

--- vlan stripping disabled

  # packet with vlan
  src=00:01:02:03:04:05 - dst=00:1B:21:AB:8F:10 - type=0x0800 - length=74 - nb_segs=1 - VLAN tci=0x666 - (outer) L2 type: ETHER - (outer) L3 type: IPV4 - (outer) L4 type: UDP - Tunnel type: Unknown - Inner L2 type: Unknown - Inner L3 type: Unknown - Inner L4 type: Unknown
 - Receive queue=0x0
  PKT_RX_VLAN_PKT
  PKT_RX_VLAN_STRIPPED


diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index 14555ab..c69b344 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -156,9 +156,9 @@ pkt_burst_receive(struct fwd_stream *fs)
 				printf("hash=0x%x ID=0x%x ",
 				       mb->hash.fdir.hash, mb->hash.fdir.id);
 		}
-		if (ol_flags & PKT_RX_VLAN_PKT)
+		if (ol_flags & PKT_RX_VLAN_STRIPPED)
 			printf(" - VLAN tci=0x%x", mb->vlan_tci);
-		if (ol_flags & PKT_RX_QINQ_PKT)
+		if (ol_flags & PKT_RX_QINQ_STRIPPED)
 			printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
 					mb->vlan_tci, mb->vlan_tci_outer);
 		if (mb->packet_type) {
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 7d947ae..702dfce 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -54,3 +54,8 @@ Deprecation Notices
   a handle, like the way kernel exposes an fd to user for locating a
   specific file, and to keep all major structures internally, so that
   we are likely to be free from ABI violations in future.
+
+* The mbuf flags PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated and
+  are respectively replaced by PKT_RX_VLAN_STRIPPED and
+  PKT_RX_QINQ_STRIPPED, that are better described. The old flags and
+  their behavior will be kept in 16.07 and will be removed in 16.11.
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 3d36f21..6d8750a 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -629,7 +629,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	uint64_t pkt_flags;
 
 	/* Check if VLAN present */
-	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0);
+	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
+		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
 
 	return pkt_flags;
 }
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 18aeead..9d80a0b 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -729,7 +729,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	uint64_t pkt_flags;
 
 	/* Check if VLAN present */
-	pkt_flags = (rx_status & E1000_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
+	pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ?
+		PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0);
 
 #if defined(RTE_LIBRTE_IEEE1588)
 	if (rx_status & E1000_RXD_STAT_TMST)
diff --git a/drivers/net/enic/enic_rx.c b/drivers/net/enic/enic_rx.c
index f92f6bc..6459e97 100644
--- a/drivers/net/enic/enic_rx.c
+++ b/drivers/net/enic/enic_rx.c
@@ -197,7 +197,7 @@ enic_cq_rx_to_pkt_flags(struct cq_desc *cqd, struct rte_mbuf *mbuf)
 
 	/* VLAN stripping */
 	if (bwflags & CQ_ENET_RQ_DESC_FLAGS_VLAN_STRIPPED) {
-		pkt_flags |= PKT_RX_VLAN_PKT;
+		pkt_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		mbuf->vlan_tci = enic_cq_rx_desc_vlan(cqrd);
 	} else {
 		mbuf->vlan_tci = 0;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index c833aa3..eea246b 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -88,7 +88,7 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union i40e_rx_desc *rxdp)
 {
 	if (rte_le_to_cpu_64(rxdp->wb.qword1.status_error_len) &
 		(1 << I40E_RX_DESC_STATUS_L2TAG1P_SHIFT)) {
-		mb->ol_flags |= PKT_RX_VLAN_PKT;
+		mb->ol_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		mb->vlan_tci =
 			rte_le_to_cpu_16(rxdp->wb.qword0.lo_dword.l2tag1);
 		PMD_RX_LOG(DEBUG, "Descriptor l2tag1: %u",
@@ -99,7 +99,7 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union i40e_rx_desc *rxdp)
 #ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
 	if (rte_le_to_cpu_16(rxdp->wb.qword2.ext_status) &
 		(1 << I40E_RX_DESC_EXT_STATUS_L2TAG2P_SHIFT)) {
-		mb->ol_flags |= PKT_RX_QINQ_PKT;
+		mb->ol_flags |= PKT_RX_QINQ_STRIPPED;
 		mb->vlan_tci_outer = mb->vlan_tci;
 		mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_2);
 		PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u",
diff --git a/drivers/net/i40e/i40e_rxtx_vec.c b/drivers/net/i40e/i40e_rxtx_vec.c
index eef80d9..634bd39 100644
--- a/drivers/net/i40e/i40e_rxtx_vec.c
+++ b/drivers/net/i40e/i40e_rxtx_vec.c
@@ -154,7 +154,7 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	/* map rss and vlan type to rss hash and vlan flag */
 	const __m128i vlan_flags = _mm_set_epi8(0, 0, 0, 0,
 			0, 0, 0, 0,
-			0, 0, 0, PKT_RX_VLAN_PKT,
+			0, 0, 0, PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED,
 			0, 0, 0, 0);
 
 	const __m128i rss_flags = _mm_set_epi8(0, 0, 0, 0,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a2b170b..5f3e047 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1636,6 +1636,7 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 {
 	struct ixgbe_hwstrip *hwstrip =
 		IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(dev->data->dev_private);
+	struct ixgbe_rx_queue *rxq;
 
 	if (queue >= IXGBE_MAX_RX_QUEUE_NUM)
 		return;
@@ -1644,6 +1645,16 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on)
 		IXGBE_SET_HWSTRIP(hwstrip, queue);
 	else
 		IXGBE_CLEAR_HWSTRIP(hwstrip, queue);
+
+	if (queue >= dev->data->nb_rx_queues)
+		return;
+
+	rxq = dev->data->rx_queues[queue];
+
+	if (on)
+		rxq->vlan_flags = PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
+	else
+		rxq->vlan_flags = PKT_RX_VLAN_PKT;
 }
 
 static void
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 9c6eaf2..5a7064c 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1221,7 +1221,7 @@ ixgbe_rxd_pkt_info_to_pkt_flags(uint16_t pkt_info)
 }
 
 static inline uint64_t
-rx_desc_status_to_pkt_flags(uint32_t rx_status)
+rx_desc_status_to_pkt_flags(uint32_t rx_status, uint64_t vlan_flags)
 {
 	uint64_t pkt_flags;
 
@@ -1230,7 +1230,7 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status)
 	 * Do not check whether L3/L4 rx checksum done by NIC or not,
 	 * That can be found from rte_eth_rxmode.hw_ip_checksum flag
 	 */
-	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  PKT_RX_VLAN_PKT : 0;
+	pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ?  vlan_flags : 0;
 
 #ifdef RTE_LIBRTE_IEEE1588
 	if (rx_status & IXGBE_RXD_STAT_TMST)
@@ -1287,6 +1287,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 	uint32_t pkt_info[LOOK_AHEAD];
 	int i, j, nb_rx = 0;
 	uint32_t status;
+	uint64_t vlan_flags = rxq->vlan_flags;
 
 	/* get references to current descriptor and S/W ring entry */
 	rxdp = &rxq->rx_ring[rxq->rx_tail];
@@ -1328,7 +1329,8 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 			mb->vlan_tci = rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
 
 			/* convert descriptor fields to rte mbuf flags */
-			pkt_flags = rx_desc_status_to_pkt_flags(s[j]);
+			pkt_flags = rx_desc_status_to_pkt_flags(s[j],
+				vlan_flags);
 			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
 			pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags
 					((uint16_t)pkt_info[j]);
@@ -1544,6 +1546,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	uint16_t nb_rx;
 	uint16_t nb_hold;
 	uint64_t pkt_flags;
+	uint64_t vlan_flags;
 
 	nb_rx = 0;
 	nb_hold = 0;
@@ -1551,6 +1554,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	rx_id = rxq->rx_tail;
 	rx_ring = rxq->rx_ring;
 	sw_ring = rxq->sw_ring;
+	vlan_flags = rxq->vlan_flags;
 	while (nb_rx < nb_pkts) {
 		/*
 		 * The order of operations here is important as the DD status
@@ -1660,7 +1664,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		/* Only valid if PKT_RX_VLAN_PKT set in pkt_flags */
 		rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan);
 
-		pkt_flags = rx_desc_status_to_pkt_flags(staterr);
+		pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_flags);
 		pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr);
 		pkt_flags = pkt_flags |
 			ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
@@ -1753,7 +1757,7 @@ ixgbe_fill_cluster_head_buf(
 	 */
 	head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
 	pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
-	pkt_flags = rx_desc_status_to_pkt_flags(staterr);
+	pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_flags);
 	pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
 	pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 	head->ol_flags = pkt_flags;
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 3691a19..2608b36 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -146,6 +146,8 @@ struct ixgbe_rx_queue {
 	uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
 	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
 	uint8_t             rx_deferred_start; /**< not in global dev start. */
+	/** flags to set in mbuf when a vlan is detected. */
+	uint64_t            vlan_flags;
 	/** need to alloc dummy mbuf, for wraparound when scanning hw ring */
 	struct rte_mbuf fake_mbuf;
 	/** hold packets to return to application */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
index e97ea82..12190d2 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
@@ -140,10 +140,9 @@ ixgbe_rxq_rearm(struct ixgbe_rx_queue *rxq)
  */
 #ifdef RTE_IXGBE_RX_OLFLAGS_ENABLE
 
-#define VTAG_SHIFT     (3)
-
 static inline void
-desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
+desc_to_olflags_v(__m128i descs[4], uint8_t vlan_flags,
+	struct rte_mbuf **rx_pkts)
 {
 	__m128i ptype0, ptype1, vtag0, vtag1;
 	union {
@@ -151,12 +150,6 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 		uint64_t dword;
 	} vol;
 
-	/* pkt type + vlan olflags mask */
-	const __m128i pkttype_msk = _mm_set_epi16(
-			0x0000, 0x0000, 0x0000, 0x0000,
-			PKT_RX_VLAN_PKT, PKT_RX_VLAN_PKT,
-			PKT_RX_VLAN_PKT, PKT_RX_VLAN_PKT);
-
 	/* mask everything except rss type */
 	const __m128i rsstype_msk = _mm_set_epi16(
 			0x0000, 0x0000, 0x0000, 0x0000,
@@ -168,6 +161,19 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 			PKT_RX_RSS_HASH, 0, PKT_RX_RSS_HASH, 0,
 			PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, 0);
 
+	/* mask everything except vlan present bit */
+	const __m128i vlan_msk = _mm_set_epi16(
+			0x0000, 0x0000,
+			0x0000, 0x0000,
+			IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
+			IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP);
+	/* map vlan present (0x8) to ol_flags */
+	const __m128i vlan_map = _mm_set_epi8(
+		0, 0, 0, 0,
+		0, 0, 0, vlan_flags,
+		0, 0, 0, 0,
+		0, 0, 0, 0);
+
 	ptype0 = _mm_unpacklo_epi16(descs[0], descs[1]);
 	ptype1 = _mm_unpacklo_epi16(descs[2], descs[3]);
 	vtag0 = _mm_unpackhi_epi16(descs[0], descs[1]);
@@ -178,8 +184,8 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	ptype0 = _mm_shuffle_epi8(rss_flags, ptype0);
 
 	vtag1 = _mm_unpacklo_epi32(vtag0, vtag1);
-	vtag1 = _mm_srli_epi16(vtag1, VTAG_SHIFT);
-	vtag1 = _mm_and_si128(vtag1, pkttype_msk);
+	vtag1 = _mm_and_si128(vtag1, vlan_msk);
+	vtag1 = _mm_shuffle_epi8(vlan_map, vtag1);
 
 	vtag1 = _mm_or_si128(ptype0, vtag1);
 	vol.dword = _mm_cvtsi128_si64(vtag1);
@@ -221,6 +227,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 				0, 0            /* ignore pkt_type field */
 			);
 	__m128i dd_check, eop_check;
+	uint8_t vlan_flags;
 
 	/* nb_pkts shall be less equal than RTE_IXGBE_MAX_RX_BURST */
 	nb_pkts = RTE_MIN(nb_pkts, RTE_IXGBE_MAX_RX_BURST);
@@ -270,6 +277,10 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	 */
 	sw_ring = &rxq->sw_ring[rxq->rx_tail];
 
+	/* ensure these 2 flags are in the lower 8 bits */
+	RTE_BUILD_BUG_ON((PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED) > UINT8_MAX);
+	vlan_flags = rxq->vlan_flags & UINT8_MAX;
+
 	/* A. load 4 packet in one loop
 	 * [A*. mask out 4 unused dirty field in desc]
 	 * B. copy 4 mbuf point from swring to rx_pkts
@@ -330,7 +341,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		sterr_tmp1 = _mm_unpackhi_epi32(descs[1], descs[0]);
 
 		/* set ol_flags with vlan packet type */
-		desc_to_olflags_v(descs, &rx_pkts[pos]);
+		desc_to_olflags_v(descs, vlan_flags, &rx_pkts[pos]);
 
 		/* D.2 pkt 3,4 set in_port/nb_seg and remove crc */
 		pkt_mb4 = _mm_add_epi16(pkt_mb4, crc_adjust);
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 29bfcec..d5b2286 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -1051,7 +1051,8 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 			pkt_buf->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
 #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
 			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
-				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT;
+				pkt_buf->ol_flags |= PKT_RX_VLAN_PKT |
+					PKT_RX_VLAN_STRIPPED;
 				pkt_buf->vlan_tci = vlan_tci;
 			}
 #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
@@ -1207,7 +1208,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 			seg->ol_flags = rxq_cq_to_ol_flags(rxq, flags);
 #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS
 			if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) {
-				seg->ol_flags |= PKT_RX_VLAN_PKT;
+				seg->ol_flags |= PKT_RX_VLAN_PKT |
+					PKT_RX_VLAN_STRIPPED;
 				seg->vlan_tci = vlan_tci;
 			}
 #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index ea5a2a3..5c9f350 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1800,7 +1800,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		if ((rxds->rxd.flags & PCIE_DESC_RX_VLAN) &&
 		    (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) {
 			mb->vlan_tci = rte_cpu_to_le_32(rxds->rxd.vlan);
-			mb->ol_flags |= PKT_RX_VLAN_PKT;
+			mb->ol_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED;
 		}
 
 		/* Adding the mbuff to the mbuff array passed by the app */
diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index 9fe8752..ccafc0c 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -579,7 +579,7 @@ vmxnet3_rx_offload(const Vmxnet3_RxCompDesc *rcd, struct rte_mbuf *rxm)
 {
 	/* Check for hardware stripped VLAN tag */
 	if (rcd->ts) {
-		rxm->ol_flags |= PKT_RX_VLAN_PKT;
+		rxm->ol_flags |= (PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED);
 		rxm->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci);
 	}
 
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index eec1456..2ece742 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -258,8 +258,10 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
 	/* case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW"; */
 	/* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */
 	/* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */
+	case PKT_RX_VLAN_STRIPPED: return "PKT_RX_VLAN_STRIPPED";
 	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
 	case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST";
+	case PKT_RX_QINQ_STRIPPED: return "PKT_RX_QINQ_STRIPPED";
 	default: return NULL;
 	}
 }
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 11fa06d..8798c41 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -79,7 +79,15 @@ extern "C" {
  * Keep these flags synchronized with rte_get_rx_ol_flag_name() and
  * rte_get_tx_ol_flag_name().
  */
-#define PKT_RX_VLAN_PKT      (1ULL << 0)  /**< RX packet is a 802.1q VLAN packet. */
+
+/**
+ * RX packet is a 802.1q VLAN packet. This flag was set by PMDs when
+ * the packet is recognized as a VLAN, but the behavior between PMDs
+ * was not the same. This flag is kept for some time to avoid breaking
+ * applications and should be replaced by PKT_RX_VLAN_STRIPPED.
+ */
+#define PKT_RX_VLAN_PKT      (1ULL << 0)
+
 #define PKT_RX_RSS_HASH      (1ULL << 1)  /**< RX packet with RSS hash result. */
 #define PKT_RX_FDIR          (1ULL << 2)  /**< RX packet with FDIR match indicate. */
 #define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt. is not OK. */
@@ -89,11 +97,37 @@ extern "C" {
 #define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer overflow. */
 #define PKT_RX_RECIP_ERR     (0ULL << 0)  /**< Hardware processing error. */
 #define PKT_RX_MAC_ERR       (0ULL << 0)  /**< MAC error. */
+
+/**
+ * A vlan has been stripped by the hardware and its tci is saved in
+ * mbuf->vlan_tci. This can only happen if vlan stripping is enabled
+ * in the RX configuration of the PMD.
+ */
+#define PKT_RX_VLAN_STRIPPED (1ULL << 6)
+
+/* hole, some bits can be reused here  */
+
 #define PKT_RX_IEEE1588_PTP  (1ULL << 9)  /**< RX IEEE1588 L2 Ethernet PT Packet. */
 #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588 L2/L4 timestamped packet.*/
 #define PKT_RX_FDIR_ID       (1ULL << 13) /**< FD id reported if FDIR match. */
 #define PKT_RX_FDIR_FLX      (1ULL << 14) /**< Flexible bytes reported if FDIR match. */
-#define PKT_RX_QINQ_PKT      (1ULL << 15)  /**< RX packet with double VLAN stripped. */
+
+/**
+ * The 2 vlans have been stripped by the hardware and their tci are
+ * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer).
+ * This can only happen if vlan stripping is enabled in the RX
+ * configuration of the PMD. If this flag is set, PKT_RX_VLAN_STRIPPED
+ * must also be set.
+ */
+#define PKT_RX_QINQ_STRIPPED (1ULL << 15)
+
+/**
+ * Deprecated.
+ * RX packet with double VLAN stripped.
+ * This flag is replaced by PKT_RX_QINQ_STRIPPED.
+ */
+#define PKT_RX_QINQ_PKT      PKT_RX_QINQ_STRIPPED
+
 /* add new RX flags here */
 
 /* add new TX flags here */
@@ -761,7 +795,10 @@ struct rte_mbuf {
 
 	/*
 	 * The packet type, which is the combination of outer/inner L2, L3, L4
-	 * and tunnel types.
+	 * and tunnel types. The packet_type is about data really present in the
+	 * mbuf. Example: if vlan stripping is enabled, a received vlan packet
+	 * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the
+	 * vlan is stripped from the data.
 	 */
 	union {
 		uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */
@@ -778,7 +815,8 @@ struct rte_mbuf {
 
 	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
 	uint16_t data_len;        /**< Amount of data in segment buffer. */
-	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
+	/** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */
+	uint16_t vlan_tci;
 
 	union {
 		uint32_t rss;     /**< RSS hash result if RSS enabled */
@@ -804,7 +842,8 @@ struct rte_mbuf {
 
 	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
 
-	uint16_t vlan_tci_outer;  /**< Outer VLAN Tag Control Identifier (CPU order) */
+	/** Outer VLAN TCI (CPU order), valid if PKT_RX_QINQ_STRIPPED is set. */
+	uint16_t vlan_tci_outer;
 
 	/* second cache line - fields only used in slow path or on TX */
 	MARKER cacheline1 __rte_cache_min_aligned;
-- 
2.8.0.rc3

Re: [PATCH v3] mbuf: new flag when Vlan is stripped

[Ananyev, Konstantin]

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, June 15, 2016 12:48 PM
> To: dev@dpdk.org
> Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; adrien.mazarguil@6wind.com; rahul.lakkireddy@chelsio.com;
> alejandro.lucero@netronome.com; sony.chacko@qlogic.com
> Subject: [PATCH v3] mbuf: new flag when Vlan is stripped
> 
> The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
> PMDs not advertising the same flags in similar conditions.
> 
> Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
> and PKT_RX_QINQ_STRIPPED that are better defined:
> 
>   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
>   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
>   is enabled in the RX configuration of the PMD.
> 
> For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
> It should be removed from applications and PMDs in a future revision.
> 
> This patch also updates the drivers. For PKT_RX_VLAN_PKT:
> 
> - e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already
>   had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is
>   required.
> - fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than
>   PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k.
> - ixgbe: modification done (vector and normal), the old flag was set
>   when a vlan was recognized, even if vlan stripping was disabled.
> - the other drivers do not support vlan stripping.
> 
> For PKT_RX_QINQ_PKT, it was only supported on i40e, and the behavior was
> already correct, so we can reuse the same bit value for
> PKT_RX_QINQ_STRIPPED.
> 
> [1] http://dpdk.org/ml/archives/dev/2016-April/037837.html,
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Re: [PATCH v3] mbuf: new flag when Vlan is stripped

[Thomas Monjalon]

2016-06-15 12:33, Ananyev, Konstantin:
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> > The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in
> > PMDs not advertising the same flags in similar conditions.
> > 
> > Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED
> > and PKT_RX_QINQ_STRIPPED that are better defined:
> > 
> >   PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its
> >   tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping
> >   is enabled in the RX configuration of the PMD.
> > 
> > For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated.
> > It should be removed from applications and PMDs in a future revision.
[...]
> > For PKT_RX_QINQ_PKT, it was only supported on i40e, and the behavior was
> > already correct, so we can reuse the same bit value for
> > PKT_RX_QINQ_STRIPPED.
> > 
> > [1] http://dpdk.org/ml/archives/dev/2016-April/037837.html,
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks