From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id E3DF94B79 for ; Mon, 13 Jun 2016 16:42:38 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 13 Jun 2016 07:42:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,466,1459839600"; d="scan'208";a="986454209" Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by fmsmga001.fm.intel.com with ESMTP; 13 Jun 2016 07:42:35 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.51]) by IRSMSX154.ger.corp.intel.com ([169.254.12.28]) with mapi id 14.03.0248.002; Mon, 13 Jun 2016 15:42:34 +0100 From: "Ananyev, Konstantin" To: Olivier Matz , "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" Thread-Topic: [PATCH v2] mbuf: new flag when Vlan is stripped Thread-Index: AQHRuCT1tJhClSk81UqMy5GQmX82V5/nje2w Date: Mon, 13 Jun 2016 14:42:34 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836B70190@irsmsx105.ger.corp.intel.com> References: <1463993205-5623-1-git-send-email-olivier.matz@6wind.com> <1464359593-3534-1-git-send-email-olivier.matz@6wind.com> In-Reply-To: <1464359593-3534-1-git-send-email-olivier.matz@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2] mbuf: new flag when Vlan is stripped X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Jun 2016 14:42:39 -0000 Hi Olivier, >=20 > The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in > PMDs not advertising the same flags in similar conditions. >=20 > Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED > and PKT_RX_QINQ_STRIPPED that are better defined: >=20 > 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. >=20 > 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.=20 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 su= pported 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 >=20 > -#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; >=20 > - /* pkt type + vlan olflags mask */ > - const __m128i pkttype_msk =3D _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 =3D _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); >=20 > + /* mask everything except vlan present bit */ > + const __m128i vlan_msk =3D _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 =3D _mm_set_epi8( > + 0, 0, 0, 0, > + 0, 0, 0, vlan_flags, > + 0, 0, 0, 0, > + 0, 0, 0, 0); > + > ptype0 =3D _mm_unpacklo_epi16(descs[0], descs[1]); > ptype1 =3D _mm_unpacklo_epi16(descs[2], descs[3]); > vtag0 =3D _mm_unpackhi_epi16(descs[0], descs[1]); > @@ -178,8 +184,8 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf *= *rx_pkts) > ptype0 =3D _mm_shuffle_epi8(rss_flags, ptype0); >=20 > vtag1 =3D _mm_unpacklo_epi32(vtag0, vtag1); > - vtag1 =3D _mm_srli_epi16(vtag1, VTAG_SHIFT); > - vtag1 =3D _mm_and_si128(vtag1, pkttype_msk); > + vtag1 =3D _mm_and_si128(vtag1, vlan_msk); > + vtag1 =3D _mm_shuffle_epi8(vlan_map, vtag1); >=20 > vtag1 =3D _mm_or_si128(ptype0, vtag1); > vol.dword =3D _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; >=20 > /* nb_pkts shall be less equal than RTE_IXGBE_MAX_RX_BURST */ > nb_pkts =3D RTE_MIN(nb_pkts, RTE_IXGBE_MAX_RX_BURST); > @@ -270,6 +277,11 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struc= t rte_mbuf **rx_pkts, > */ > sw_ring =3D &rxq->sw_ring[rxq->rx_tail]; >=20 > + /* ensure these 2 flags are in the lower 8 bits */ > + RTE_BUILD_BUG_ON(((PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED) & > + 0xffffffffffffff00ULL) !=3D 0ULL); I suppose your need here: RTE_BUILD_BUG_ON(((PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED) & UINT8_MAX) =3D= =3D=20 PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED)); To make sure both flags are inside first 8 bits? =09 > + vlan_flags =3D 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