From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <adrien.mazarguil@6wind.com>
Received: from mail-wr0-f177.google.com (mail-wr0-f177.google.com
[209.85.128.177]) by dpdk.org (Postfix) with ESMTP id 8574E1C7FE
for <dev@dpdk.org>; Thu, 5 Apr 2018 16:39:34 +0200 (CEST)
Received: by mail-wr0-f177.google.com with SMTP id d17so13439910wre.1
for <dev@dpdk.org>; Thu, 05 Apr 2018 07:39:34 -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=+smcYX5XvSZCdo662T3hSOXx0fIU5ZWHk/+8R98k9lo=;
b=evLrD4JmPSW26ylO0k1kYxpEQuKqtSy7YBHOzZeSlVAwo9NAIvrb7AvBg2UOlDxWkk
FXYaI+3URwfIXoL6Op2V2NtO9Y6aLzBpVFViEz2IZSHSPuL9KxGToq7NoVitPDO94OCX
l3cjhmhCmv4jbNfYDgXPTHCeB3Nui6Q+K4MP5wwksX2Lacxf+1mTsZ5af8eGgvU18sOT
s/NIuxG1xy/q6rku59hSlLfjCaRxFsNJigh5Mz9BuvpexqqOTdKD2vGNDJXeHzgmIvep
7eEYJZaU7YVyWKVDaQP3YC1T5ON7O7IylIsXYZm4ZkgyaSJq5+2SRA5xu3Y+R+tMgYJa
nLWA==
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=+smcYX5XvSZCdo662T3hSOXx0fIU5ZWHk/+8R98k9lo=;
b=TUx3eXYK9L5HYx3rycP+r/IJNvxC7+Fj0b7XgN4ktG/NfMDe33dl0vpXfOr7cYh9Nw
/aowfZk4lduX1kDzwz1KUGku2PveRym+avB0a4uN8LkcWgpnfoFa4Sf+pVLQJHrq42x7
kMi8BD+yWXXQM2X3i23DFb5wDT6ESwr2GPQuXtBDvGzjrIWH2dk7gdaH0b6I4AFPU+3X
AXiy4cy0/bD/I5oHSDbpCCnSHaJ7GHBBBhQuzZaxI0fCDmzDTdztcbN7cg3x0E12+9H/
NqH4ziXVIwnl0Nh0v1rUhqFyUrYg4LxbnWFiRj3Pr+wraiot35+B7JBzTftSCYK4izBQ
/0dg==
X-Gm-Message-State: AElRT7EUKNMPrFTX4cZwiXVvUvN3ckDJHnH78v0acAyJnFhzWKlOTyoY
XlfZQwodWFXF4VMM0CU9QWLUNQ==
X-Google-Smtp-Source: AIpwx4/R++DQKFSC8Ks9OUK+rIvTeaiTMpdQgzVqeKnw+N1ASC2LlevOZ+Hwt7I9ZYUNR6oK2rsmig==
X-Received: by 10.223.158.6 with SMTP id u6mr16159499wre.142.1522939174267;
Thu, 05 Apr 2018 07:39:34 -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 t196sm6074611wme.35.2018.04.05.07.39.33
(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Thu, 05 Apr 2018 07:39:33 -0700 (PDT)
Date: Thu, 5 Apr 2018 16:39:20 +0200
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: "Zhang, Qi Z" <qi.z.zhang@intel.com>
Cc: Thomas Monjalon <thomas@monjalon.net>,
"Yigit, Ferruh" <ferruh.yigit@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>, "Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>,
Ajit Khaparde <ajit.khaparde@broadcom.com>,
Somnath Kotur <somnath.kotur@broadcom.com>,
John Daley <johndale@cisco.com>, Hyong Youb Kim <hyonkim@cisco.com>,
"Xing, Beilei" <beilei.xing@intel.com>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
Nelio Laranjeiro <nelio.laranjeiro@6wind.com>,
Yongseok Koh <yskoh@mellanox.com>, Jacek Siuda <jck@semihalf.com>,
Tomasz Duszynski <tdu@semihalf.com>, Dmitri Epshtein <dima@marvell.com>,
Natalie Samsonov <nsamsono@marvell.com>, Jianbo Liu <jianbo.liu@arm.com>,
Andrew Rybchenko <arybchenko@solarflare.com>,
Pascal Mazon <pascal.mazon@6wind.com>
Message-ID: <20180405143920.GM4957@6wind.com>
References: <20180404150312.12304-1-adrien.mazarguil@6wind.com>
<20180404150312.12304-12-adrien.mazarguil@6wind.com>
<039ED4275CED7440929022BC67E7061153182FE4@SHSMSX103.ccr.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <039ED4275CED7440929022BC67E7061153182FE4@SHSMSX103.ccr.corp.intel.com>
Subject: Re: [dpdk-dev] [PATCH v1 11/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 <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 05 Apr 2018 14:39:34 -0000
On Thu, Apr 05, 2018 at 02:20:34PM +0000, Zhang, Qi Z wrote:
> Hi Adrien:
>
> > Hi PMD maintainers, while I'm pretty confident in these changes, I could not
> > validate them with all devices.
> >
> > It would be great if you could apply this patch, run testpmd, create VLAN flow
> > rules with/without inner EtherType as described and send matching traffic
> > while making sure nothing was broken in the process.
> >
> > Thanks!
> > ---
> > diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> > 1b336df74..c6dd889ad 100644
> > --- a/drivers/net/i40e/i40e_flow.c
> > +++ b/drivers/net/i40e/i40e_flow.c
> > @@ -2490,24 +2490,36 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev
> > *dev,
> > "Invalid MAC_addr mask.");
> > return -rte_errno;
> > }
> > + }
> > + if (eth_spec && eth_mask && eth_mask->type) {
> > + enum rte_flow_item_type next = (item + 1)->type;
> >
> > - if ((eth_mask->type & UINT16_MAX) ==
> > - UINT16_MAX) {
> > - input_set |= I40E_INSET_LAST_ETHER_TYPE;
> > - filter->input.flow.l2_flow.ether_type =
> > - eth_spec->type;
> > + if (eth_mask->type != RTE_BE16(0xffff)) {
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ITEM,
> > + item,
> > + "Invalid type mask.");
> > + return -rte_errno;
> > }
> >
> > ether_type = rte_be_to_cpu_16(eth_spec->type);
> > - if (ether_type == ETHER_TYPE_IPv4 ||
> > - ether_type == ETHER_TYPE_IPv6 ||
> > - ether_type == ETHER_TYPE_ARP ||
> > - ether_type == outer_tpid) {
> > +
> > + if ((next == RTE_FLOW_ITEM_TYPE_VLAN &&
> > + ether_type != outer_tpid) ||
> > + (next != RTE_FLOW_ITEM_TYPE_VLAN &&
> > + (ether_type == ETHER_TYPE_IPv4 ||
> > + ether_type == ETHER_TYPE_IPv6 ||
> > + ether_type == ETHER_TYPE_ARP ||
> > + ether_type == outer_tpid))) {
> > rte_flow_error_set(error, EINVAL,
> > RTE_FLOW_ERROR_TYPE_ITEM,
> > item,
> > "Unsupported ether_type.");
> > return -rte_errno;
> > + } else if (next != RTE_FLOW_ITEM_TYPE_VLAN) {
> > + input_set |= I40E_INSET_LAST_ETHER_TYPE;
> > + filter->input.flow.l2_flow.ether_type =
> > + eth_spec->type;
> > }
> > }
> >
> > @@ -2518,6 +2530,14 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev
> > *dev,
> > case RTE_FLOW_ITEM_TYPE_VLAN:
> > vlan_spec = item->spec;
> > vlan_mask = item->mask;
> > +
> > + if (input_set & I40E_INSET_LAST_ETHER_TYPE) {
> > + rte_flow_error_set(error, EINVAL,
> > + RTE_FLOW_ERROR_TYPE_ITEM,
> > + item,
> > + "Unsupported outer TPID.");
> > + return -rte_errno;
> > + }
>
> Please correct me I'm wrong, now I40E_INSET_LAST_ETHER_TYPE will only be set when next != RTE_FLOW_ITEM_TYPE_VLAN
> while, RTE_FLOW_ITEM_TYPE_VLAN will only be the next to RTE_FLOW_ITEM_TYPE_ETH for fdir, so above check seems unnecessary ?
You're right, it's unnecessary. I can remove this change or leave it as a
safety measure for future contributions, since when parsing a VLAN item
one is not necessarily aware of what happened in previous iterations.
How about an assertion check for debugging purposes instead?
RTE_ASSERT(!(input_set & I40E_INSET_LAST_ETHER_TYPE));
--
Adrien Mazarguil
6WIND