Re: [PATCH v2] mbuf: new flag when Vlan is stripped
From: Ananyev, Konstantin <hidden>
Date: 2016-06-13 14:42:38
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.
quoted hunk ↗ jump to hunk
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