From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id CC0998D3D for ; Wed, 25 Apr 2018 18:10:43 +0200 (CEST) Received: by mail-wm0-f54.google.com with SMTP id 66so8070288wmd.3 for ; Wed, 25 Apr 2018 09:10:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=LGAKshcyOXroRTY1U39rMFndqbm0TJNOxZ3zUQKGY0g=; b=I9A0wphiLLu6W2e3ZCKxult7HEAhcdt/yhXjvIMViP2y80ZF/gZKo3oEYPRNkvKnz5 C1Ay8QAmRiB4yGfoAuw/iLQ4A03OU4xgj8MXzKuBIfSYdNuoOZgVoiFSOIwxbxjQzBRD xaf4aVDOeSQhwrG1UKyX+ak9h4KtmdB69nQ8nLRlLP/AOAdY7SLbinAD6xzJWNgTEFqb 83vtFHUA0XCVVls1nAnhaJAvN7DhgW0zaz0azB9u9ApKHjafpNqne552siQBXQryqQNj 3OA3/MBpDMiZ1H0JwDIbIbBjBBmpf1kSoSAqycc38RzuEDmwUOahtHkIRf8H7hkJlG9E 2Q8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=LGAKshcyOXroRTY1U39rMFndqbm0TJNOxZ3zUQKGY0g=; b=J+0LSaKfPLkuRRzKVksnE+i6hu9v/ByELyXE0UyTJFbm3k8mfnYAMKojLy/tXDOfKw 19c/4jf1R8CtCBonXrIq5+ClUfwNth5iEB022feH/JEAdLW1jwjd1xOHFf8C4xjDZuId /j9IVv/xtWjK5t90XFalN1s4PwjPIp5wYsJdRshc5hbGROFZ1a+6aSz6Ub6JNUsnSmcL MxBhFxzCiuAhP3Zk0A1sSu4Y384//07OmhUqYIAdsV0NK8l8VCutltI8w6XasWGYdMeA 04xlIap97ftZutdWmpr0BFYPHIBE1H0mmyHOYTZdgaeTGvHIXzq8YHNGSLga/tEf9jb2 7M2Q== X-Gm-Message-State: ALQs6tCe/9lbqo7p4m16FROA11s56zRofCxowTdLx7ln6UgPk6JECscd X3BH4ll0HYsmmG+y9kismT/X+g== X-Google-Smtp-Source: AB8JxZoKYHg5ytl2Aup8CSUGL+RS3+ZzrcQUCwvqwgoMykScfejqCr5AAYAtN3X0vBWfsX5lMj3xWQ== X-Received: by 10.28.139.11 with SMTP id n11mr1348667wmd.12.1524672643428; Wed, 25 Apr 2018 09:10:43 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id c50-v6sm31799548wrc.11.2018.04.25.09.10.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Apr 2018 09:10:42 -0700 (PDT) Date: Wed, 25 Apr 2018 18:10:28 +0200 From: Adrien Mazarguil To: Ferruh Yigit , dev@dpdk.org Cc: Jacek Siuda , Tomasz Duszynski , Dmitri Epshtein , Natalie Samsonov , Jianbo Liu Message-ID: <20180425161028.GT4957@6wind.com> References: <20180419100848.6178-1-adrien.mazarguil@6wind.com> <20180425151852.7676-1-adrien.mazarguil@6wind.com> <20180425151852.7676-11-adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180425151852.7676-11-adrien.mazarguil@6wind.com> Subject: Re: [dpdk-dev] [PATCH v6 10/16] ethdev: fix 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, 25 Apr 2018 16:10:44 -0000 On Wed, Apr 25, 2018 at 05:27:56PM +0200, 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. > > Fixes: b1a4b4cbc0a8 ("ethdev: introduce generic flow API") > Fixes: 99e7003831c3 ("net/ixgbe: parse L2 tunnel filter") > > Signed-off-by: Adrien Mazarguil > Acked-by: Andrew Rybchenko > 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 > > --- > > v6 changes: > > - Reworded patch title as a "fix" (not candidate for backports) since it > addresses a flaw in the original API definition. > - Updated API and ABI changes sections in release notes. > > 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 > diff --git a/drivers/net/mvpp2/mrvl_flow.c b/drivers/net/mvpp2/mrvl_flow.c > + if (mask->inner_type) { > + struct rte_flow_item_eth spec_eth = { > + .type = spec->inner_type, > + }; > + struct rte_flow_item_eth mask_eth = { > + .type = mask->inner_type, > + }; > + > + RTE_LOG(WARNING, PMD, "inner eth type mask is ignored\n"); > + ret = mrvl_parse_type(spec_eth, mask_eth, flow); Looks like there's a compilation issue left on the above line, which should obviously read: ret = mrvl_parse_type(&spec_eth, &mask_eth, flow); Ferruh, can you update it while applying? I'd hate to spam the world with v7 :) Thanks. -- Adrien Mazarguil 6WIND