[dpdk-dev,v2] mbuf: new flag when Vlan is stripped
Commit Message
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(-)
Comments
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?
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
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
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
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
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
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
@@ -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) {
@@ -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.
@@ -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;
}
@@ -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)
@@ -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;
@@ -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",
@@ -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,
@@ -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
@@ -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;
@@ -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 */
@@ -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);
@@ -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 */
@@ -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 */
@@ -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);
}
@@ -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;
}
}
@@ -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;