[dpdk-dev] [PATCH v2] mbuf: new flag when Vlan is stripped
Ananyev, Konstantin
konstantin.ananyev at intel.com
Mon Jun 13 16:42:34 CEST 2016
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
More information about the dev
mailing list