From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 52C0E1C09A for ; Wed, 11 Apr 2018 14:46:08 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 4CE82B40063; Wed, 11 Apr 2018 12:46:06 +0000 (UTC) Received: from [192.168.38.17] (84.52.114.114) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Wed, 11 Apr 2018 13:45:55 +0100 To: Adrien Mazarguil , Thomas Monjalon , Ferruh Yigit , CC: Wenzhuo Lu , Jingjing Wu , Ajit Khaparde , Somnath Kotur , John Daley , Hyong Youb Kim , Beilei Xing , Qi Zhang , Konstantin Ananyev , Nelio Laranjeiro , Yongseok Koh , Tomasz Duszynski , Dmitri Epshtein , Natalie Samsonov , Jianbo Liu , Pascal Mazon References: <20180406131736.19145-1-adrien.mazarguil@6wind.com> <20180410162022.9101-1-adrien.mazarguil@6wind.com> <20180410162022.9101-11-adrien.mazarguil@6wind.com> From: Andrew Rybchenko Message-ID: <26297b1e-fea2-1995-25b4-9d17d5a1a39e@solarflare.com> Date: Wed, 11 Apr 2018 15:45:51 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180410162022.9101-11-adrien.mazarguil@6wind.com> Content-Language: en-GB X-Originating-IP: [84.52.114.114] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23776.003 X-TM-AS-Result: No--24.398400-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1523450767-CfZynjrdw3gU Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v3 10/16] ethdev: refine TPID handling in flow API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Apr 2018 12:46:08 -0000 On 04/10/2018 07:36 PM, Adrien Mazarguil wrote: > TPID handling in rte_flow VLAN and E_TAG pattern item definitions is not > consistent with the normal stacking order of pattern items, which is > confusing to applications. > > Problem is that when followed by one of these layers, the EtherType field > of the preceding layer keeps its "inner" definition, and the "outer" TPID > is provided by the subsequent layer, the reverse of how a packet looks like > on the wire: > > Wire: [ ETH TPID = A | VLAN EtherType = B | B DATA ] > rte_flow: [ ETH EtherType = B | VLAN TPID = A | B DATA ] > > Worse, when QinQ is involved, the stacking order of VLAN layers is > unspecified. It is unclear whether it should be reversed (innermost to > outermost) as well given TPID applies to the previous layer: > > Wire: [ ETH TPID = A | VLAN TPID = B | VLAN EtherType = C | C DATA ] > rte_flow 1: [ ETH EtherType = C | VLAN TPID = B | VLAN TPID = A | C DATA ] > rte_flow 2: [ ETH EtherType = C | VLAN TPID = A | VLAN TPID = B | C DATA ] > > While specifying EtherType/TPID is hopefully rarely necessary, the stacking > order in case of QinQ and the lack of documentation remain an issue. > > This patch replaces TPID in the VLAN pattern item with an inner > EtherType/TPID as is usually done everywhere else (e.g. struct vlan_hdr), > clarifies documentation and updates all relevant code. > > It breaks ABI compatibility for the following public functions: > > - rte_flow_copy() > - rte_flow_create() > - rte_flow_query() > - rte_flow_validate() > > Summary of changes for PMDs that implement ETH, VLAN or E_TAG pattern > items: > > - bnxt: EtherType matching is supported with and without VLAN, but TPID > matching is not and triggers an error. > > - e1000: EtherType matching is only supported with the ETHERTYPE filter, > which does not support VLAN matching, therefore no impact. > > - enic: same as bnxt. > > - i40e: same as bnxt with existing FDIR limitations on allowed EtherType > values. The remaining filter types (VXLAN, NVGRE, QINQ) do not support > EtherType matching. > > - ixgbe: same as e1000, with additional minor change to rely on the new > E-Tag macro definition. > > - mlx4: EtherType/TPID matching is not supported, no impact. > > - mlx5: same as bnxt. > > - mvpp2: same as bnxt. > > - sfc: same as bnxt. > > - tap: same as bnxt. > > Signed-off-by: Adrien Mazarguil > Cc: Ferruh Yigit > Cc: Thomas Monjalon > Cc: Wenzhuo Lu > Cc: Jingjing Wu > Cc: Ajit Khaparde > Cc: Somnath Kotur > Cc: John Daley > Cc: Hyong Youb Kim > Cc: Beilei Xing > Cc: Qi Zhang > Cc: Konstantin Ananyev > Cc: Nelio Laranjeiro > Cc: Yongseok Koh > Cc: Tomasz Duszynski > Cc: Dmitri Epshtein > Cc: Natalie Samsonov > Cc: Jianbo Liu > Cc: Andrew Rybchenko > Cc: Pascal Mazon > > --- > > v3 changes: > > Updated mrvl to mvpp2. > > Moved unrelated default TCI mask update to separate patch. > > Fixed sfc according to Andrew's comments [1], which made so much sense that > I standardized on the same behavior for all other PMDs: matching outer TPID > is never supported when a VLAN pattern item is present. > > This is done because many devices accept several TPIDs but do not provide > means to match a given one explicitly, it's all or nothing, and that makes > the resulting flow rule inaccurate. > > [1] http://dpdk.org/ml/archives/dev/2018-April/095870.html > --- > app/test-pmd/cmdline_flow.c | 17 +++---- > doc/guides/nics/tap.rst | 2 +- > doc/guides/prog_guide/rte_flow.rst | 19 ++++++-- > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +- > drivers/net/bnxt/bnxt_filter.c | 35 +++++++++++--- > drivers/net/enic/enic_flow.c | 19 +++++--- > drivers/net/i40e/i40e_flow.c | 60 ++++++++++++++++++++---- > drivers/net/ixgbe/ixgbe_ethdev.c | 3 +- > drivers/net/mlx5/mlx5_flow.c | 13 ++++- > drivers/net/mvpp2/mrvl_flow.c | 26 +++++++--- > drivers/net/sfc/sfc_flow.c | 18 +++++++ > drivers/net/tap/tap_flow.c | 14 ++++-- > lib/librte_ether/rte_flow.h | 22 ++++++--- > lib/librte_net/rte_ether.h | 1 + > 14 files changed, 198 insertions(+), 55 deletions(-) Generic and net/sfc Acked-by: Andrew Rybchenko