From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 396522C49 for ; Thu, 9 Mar 2017 02:59:53 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Mar 2017 17:59:53 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,266,1486454400"; d="scan'208";a="1106431634" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga001.jf.intel.com with ESMTP; 08 Mar 2017 17:59:52 -0800 Received: from fmsmsx122.amr.corp.intel.com (10.18.125.37) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 8 Mar 2017 17:59:51 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx122.amr.corp.intel.com (10.18.125.37) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 8 Mar 2017 17:59:51 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.177]) with mapi id 14.03.0248.002; Thu, 9 Mar 2017 09:59:49 +0800 From: "Lu, Wenzhuo" To: Le Scouarnec Nicolas , "Adrien Mazarguil" CC: users , Jan Medala , Evgeny Schemeilin , Stephen Hurd , Jerin Jacob , Rahul Lakkireddy , John Daley , Matej Vido , "Zhang, Helin" , "Ananyev, Konstantin" , "Wu, Jingjing" , "Chen, Jing D" , "Alejandro Lucero" , Harish Patil , Rasesh Mody , "Andrew Rybchenko" , Nelio Laranjeiro , Vasily Philipov , "Pascal Mazon" , Thomas Monjalon Thread-Topic: Issues with ixgbe and rte_flow Thread-Index: AQHSly9VSVacWJmR5EC8Uqr0D2ovhaGKMkmAgAB1ebX//+itgIAAChGAgAElwrA= Date: Thu, 9 Mar 2017 01:59:48 +0000 Message-ID: <6A0DE07E22DDAD4C9103DF62FEBC09093B56DC90@shsmsx102.ccr.corp.intel.com> References: <6A0DE07E22DDAD4C9103DF62FEBC09093B56D514@shsmsx102.ccr.corp.intel.com> , <20170308154131.GQ3790@6wind.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 09 Mar 2017 11:17:33 +0100 Subject: Re: [dpdk-users] Issues with ixgbe and rte_flow X-BeenThere: users@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK usage discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Mar 2017 01:59:54 -0000 Hi Le Scouarnec, > -----Original Message----- > From: Le Scouarnec Nicolas [mailto:Nicolas.LeScouarnec@technicolor.com] > Sent: Thursday, March 9, 2017 12:18 AM > To: Adrien Mazarguil > Cc: Lu, Wenzhuo; users; Jan Medala; Evgeny Schemeilin; Stephen Hurd; Jeri= n > Jacob; Rahul Lakkireddy; John Daley; Matej Vido; Zhang, Helin; Ananyev, > Konstantin; Wu, Jingjing; Chen, Jing D; Alejandro Lucero; Harish Patil; R= asesh > Mody; Andrew Rybchenko; Nelio Laranjeiro; Vasily Philipov; Pascal Mazon; > Thomas Monjalon > Subject: Re: Issues with ixgbe and rte_flow >=20 > Removing dev to avoid flooding too much people. >=20 > Talking about API/ABI breakage,=A0PMD maintainers / users might be intere= sted by > this comment that got stripped from the initial message: I also have a si= de > comment which might be more related to the general rte_flow API than to t= he > specific implementation in ixgbe. I don't know if it is specific to ixgbe= 's > implementation but, as a user, the rte_flow_error returned was not very u= seful > for it does return the error of the last tried filter-type (L2 tunnel in = ixgbe), and > not the error of the filter-type that my setup should use (flow director)= . I had to > change the order in which filters are tried in ixgbe code to get a more u= seful > error message. As NICs can have several filter-types, It would be be usef= ul to the > user if rte_flow_validate/create could return the errors for all filter t= ypes tried > although that would require to change the rte_flow API and returning an a= rray > of rte_flow_error and not a single struct. It's a good suggestion. I remember we have some discussion about how to fe= edback the error to the APP. I think the reason why we don't make it too co= mplex because it's the first step of generic API. Now we see some feedback= from the users, we can keep optimizing it :) And about the tpid, ethertype. I have a thought that why we need it as it's= duplicate with the item type. I think the initial design is just following= the IEEE spec to define the structures so we will not miss anything. But w= hy not do some optimization. For VLAN the tpid must be 0x8100, for IPv4, th= e ethertype must be 0x0800. So why bothering let APP provide them and drive= r check them? Seems we can just remove these fields from the structures, it= can make things simpler. Adrien, as you're the maintainer of rte_flow, any thought about these ideas= ? Thanks. >=20 >=20 > Best regards, > Nicolas Le Scouarnec >=20 >=20 > From: Adrien Mazarguil > Sent: Wednesday, March 8, 2017 4:41 PM > To: Le Scouarnec Nicolas > Cc: Lu, Wenzhuo; dev; users; Jan Medala; Evgeny Schemeilin; Stephen Hurd; > Jerin Jacob; Rahul Lakkireddy; John Daley; Matej Vido; Helin Zhang; Konst= antin > Ananyev; Jingjing Wu; Jing Chen; Alejandro Lucero; Harish Patil; Rasesh M= ody; > Andrew Rybchenko; Nelio Laranjeiro; Vasily Philipov; Pascal Mazon; Thoma= s > Monjalon > Subject: Re: Issues with ixgbe and rte_flow >=20 > ** WARNING: This mail is from an external source ** >=20 >=20 > CC'ing users@dpdk.org since this issue primarily affects rte_flow users, = and > several PMD maintainers to get their opinion on the matter, see below. >=20 > On Wed, Mar 08, 2017 at 09:24:26AM +0000, Le Scouarnec Nicolas wrote: > > My response is inline bellow, and further comment on the code excerpt > > also > > > > > > From: Lu, Wenzhuo > > Sent: Wednesday, March 8, 2017 4:16 AM > > To: Le Scouarnec Nicolas; dev@dpdk.org; Adrien Mazarguil > > (adrien.mazarguil@6wind.com) > > Cc: Yigit, Ferruh > > Subject: RE: Issues with ixgbe and rte_flow > > > > >> I have been using the new API rte_flow to program filtering on an > > >> X540 (ixgbe) NIC. My goal is to send packets from different VLANs > > >> to different queues (filtering which should be supported by flow > > >> director as far as I understand). I enclosed the setup code at the b= ottom of > this email. > > >> For reference, here is the setup code I use > > >> > > >>=A0=A0=A0=A0=A0=A0 vlan_spec.tci =3D vlan_be; > > >>=A0=A0=A0=A0=A0=A0 vlan_spec.tpid =3D 0; > > >> > > >>=A0=A0=A0=A0=A0=A0 vlan_mask.tci =3D rte_cpu_to_be_16(0x0fff); > > >>=A0=A0=A0=A0=A0=A0 vlan_mask.tpid =3D=A0 0; > > > > >To my opinion, this setting is not right. As we know, vlan tag is inse= rted > between MAC source address and Ether type. > > >So if we have a MAC+VLAN+IPv4 packet, the vlan_spec.tpid should be 0x8= 100, > the eth_spec.type should be 0x0800. > > >+ Adrien, the author. He can correct me if I'm wrong. >=20 > That's right, however the confusion is understandable, perhaps the > documentation should be clearer. It currently states what follows without > describing the reason: >=20 > =A0/** > =A0 * RTE_FLOW_ITEM_TYPE_VLAN > =A0 * > =A0 * Matches an 802.1Q/ad VLAN tag. > =A0 * > =A0 * This type normally follows either RTE_FLOW_ITEM_TYPE_ETH or > =A0 * RTE_FLOW_ITEM_TYPE_VLAN. > =A0 */ >=20 > > Ok, I apologize, you're right. Being more used to the software-side tha= n to the > hardware-side, I misunderstood struct rte_flow_item_vlan and though it wa= s the > "equivalent" of struct vlan_hdr, in which case the vlan_hdr contains the = type of > the encapsulated frame. > > > > (=A0 /** > >=A0 * Ethernet VLAN Header. > >=A0 * Contains the 16-bit VLAN Tag Control Identifier and the Ethernet > >type > >=A0 * of the encapsulated frame. > >=A0 */ > > struct vlan_hdr { > >=A0=A0=A0=A0=A0=A0 uint16_t vlan_tci; /**< Priority (3) + CFI (1) + Iden= tifier Code > >(12) */ > >=A0=A0=A0=A0=A0=A0 uint16_t eth_proto;/**< Ethernet type of encapsulated= frame. */ > >} __attribute__((__packed__));=A0=A0=A0=A0=A0=A0=A0 ) >=20 > Indeed, struct vlan_hdr and struct rte_flow_item_vlan are not mapped at t= he > same offset; the former includes EtherType of the inner packet (eth_proto= ), > while the latter describes the inserted VLAN header itself starting with = TPID. >=20 > This approach was chosen for rte_flow for consistency with the fact each > pattern item describes exactly one protocol header, even though in the ca= se of > VLAN and other layer 2.5 protocols, some happen to be embedded. > IPv4/IPv6 options will be provided as separate items in a similar fashion= . >=20 > It also allows adding/removing VLAN tags to an existing rule without modi= fying > the EtherType of the inner frame. >=20 > Now assuming you're not the only one facing that issue, if the current de= finition > does not make sense, perhaps we can update the API before it's too late. = I'll > attempt to summarize it with an example below. >=20 > In any case, matching nonspecific VLAN-tagged and QinQ UDPv4 packets in > testpmd is written as: >=20 > =A0flow create 0 pattern eth / vlan / ipv4 / udp / end actions queue 1 / = end > =A0flow create 0 pattern eth / vlan / vlan / ipv4 / udp / end actions que= ue 1 / end >=20 > However, with the current API described above, specifying inner/outer > EtherTypes for the above packets yields (as a reminder, 0x8100 stands for= VLAN, > 0x8000 for IPv4 and 0x88A8 for QinQ): >=20 > #1 >=20 > =A0flow create 0 pattern eth type is 0x8000 / vlan tpid is 0x8100 / ipv4 = / udp / > actions queue 1 / end > =A0flow create 0 pattern eth type is 0x8000 / vlan tpid is 0x88A8 / vlan = tpid is > 0x8100 / ipv4 / udp / actions queue 1 / end >=20 > Instead of the arguably more accurate (renaming "tpid" to "inner_type" fo= r > clarity): >=20 > #2 >=20 > =A0flow create 0 pattern eth type is 0x8100 / vlan type is 0x8000 / ipv4 = / udp / > actions queue 1 / end > =A0flow create 0 pattern eth type is 0x88A8 / vlan inner_type is 0x8100 /= vlan > inner_type is 0x8000 / ipv4 / udp / actions queue 1 / end >=20 > So, should the VLAN item be updated to behave as described in #2? >=20 > Note: doing so will cause a serious API/ABI breakage, I know it was not > supposed to happen according to the rte_flow sales pitch, but hey. >=20 > -- > Adrien Mazarguil > 6WIND >=20