From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 64DF3A0487
	for <public@inbox.dpdk.org>; Thu,  4 Jul 2019 04:43:12 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 2135A1F28;
	Thu,  4 Jul 2019 04:43:12 +0200 (CEST)
Received: from EUR02-AM5-obe.outbound.protection.outlook.com
 (mail-eopbgr00059.outbound.protection.outlook.com [40.107.0.59])
 by dpdk.org (Postfix) with ESMTP id 17F851E20
 for <dev@dpdk.org>; Thu,  4 Jul 2019 04:43:10 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;
 s=selector2;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=+zLjtlMmcSh8m4vOj9RPwcsWBzKHP4G550lmexeCbyI=;
 b=JqCgBdqu6FfzPAvo/rkLUxHw2uV5zaAWMXKkYR1ySi7QoQlB5av2EkUIpE3+U4o1Z4jhTf0E7eQ2CPWB2NuvG1fokQ7thUG6J/fO9mTTgJgIFIZv6hgF4rRJNrl0iI0+23DI1NbaQDK1af3w5tp9ESxJWyCx95bJQDualtQMZ9E=
Received: from AM6PR05MB6567.eurprd05.prod.outlook.com (20.179.6.215) by
 AM6PR05MB5013.eurprd05.prod.outlook.com (20.177.35.14) with Microsoft SMTP
 Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.2032.20; Thu, 4 Jul 2019 02:43:08 +0000
Received: from AM6PR05MB6567.eurprd05.prod.outlook.com
 ([fe80::496b:bd1c:863a:ed47]) by AM6PR05MB6567.eurprd05.prod.outlook.com
 ([fe80::496b:bd1c:863a:ed47%3]) with mapi id 15.20.2052.010; Thu, 4 Jul 2019
 02:43:08 +0000
From: Jack Min <jackmin@mellanox.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
CC: Ori Kam <orika@mellanox.com>, John McNamara <john.mcnamara@intel.com>,
 Marko Kovacevic <marko.kovacevic@intel.com>, Thomas Monjalon
 <thomas@monjalon.net>, Ferruh Yigit <ferruh.yigit@intel.com>, Andrew
 Rybchenko <arybchenko@solarflare.com>, "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [PATCH v4 1/4] ethdev: add GRE key field to flow API
Thread-Index: AQHVMbOBeqiSW/lYt0Ohlp4ilvQb/Ka5wPkA
Date: Thu, 4 Jul 2019 02:43:08 +0000
Message-ID: <20190704024258.sdt3cbzjdcdp7nqg@mellanox.com>
References: <20190624154018.128379-1-jackmin@mellanox.com>
 <cover.1562058723.git.jackmin@mellanox.com>
 <f639cc93de14d44a1f213aba59a7e4991ca10bb7.1562058723.git.jackmin@mellanox.com>
 <20190703152507.GH4512@6wind.com>
In-Reply-To: <20190703152507.GH4512@6wind.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-clientproxiedby: HK0PR01CA0072.apcprd01.prod.exchangelabs.com
 (2603:1096:203:a6::36) To AM6PR05MB6567.eurprd05.prod.outlook.com
 (2603:10a6:20b:bc::23)
authentication-results: spf=none (sender IP is )
 smtp.mailfrom=jackmin@mellanox.com; 
x-ms-exchange-messagesentrepresentingtype: 1
x-originating-ip: [139.226.40.4]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 02854337-7ac7-42a5-0b25-08d7002958ce
x-ms-office365-filtering-ht: Tenant
x-microsoft-antispam: BCL:0; PCL:0;
 RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(4618075)(2017052603328)(7193020);
 SRVR:AM6PR05MB5013; 
x-ms-traffictypediagnostic: AM6PR05MB5013:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <AM6PR05MB50130150F3467DBB3B995FF3CCFA0@AM6PR05MB5013.eurprd05.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:6790;
x-forefront-prvs: 0088C92887
x-forefront-antispam-report: SFV:NSPM;
 SFS:(10009020)(4636009)(39860400002)(366004)(396003)(136003)(346002)(376002)(189003)(199004)(1076003)(36756003)(8676002)(81166006)(81156014)(2906002)(8936002)(52116002)(66556008)(478600001)(64756008)(66446008)(73956011)(66946007)(25786009)(6916009)(14454004)(68736007)(66476007)(6512007)(71200400001)(476003)(11346002)(316002)(26005)(2616005)(102836004)(53936002)(7736002)(99286004)(446003)(6486002)(86362001)(71190400001)(305945005)(54906003)(4326008)(5660300002)(6246003)(486006)(66066001)(3846002)(229853002)(186003)(6116002)(76176011)(386003)(6436002)(6506007)(14444005)(256004);
 DIR:OUT; SFP:1101; SCL:1; SRVR:AM6PR05MB5013;
 H:AM6PR05MB6567.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en;
 PTR:InfoNoRecords; MX:1; A:1; 
received-spf: None (protection.outlook.com: mellanox.com does not designate
 permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: xasxPGyJ4Qu3d/gf5Zp2ioGT6j3i2f22Kne/w9EC6jnfLJjWazzZirDQhrF7czYAYPNsKJlw/wJA6QXrlVgR2YIlzagCmGUn54DvZTRowaMDe1tNrP//R7cafc2Jdqadrk/vrM+5uTc2rR1m3aZMX/cweSEYkzjoUYmLjuXQw0Zbom5ia8aUvW1i384z6GlVsfUCUJIDCYEZ/RjRIxpNGmbzbuAYXZb7syF6zS6LiEk97FHMMwgVMoo0gV1HRKmyPkL2Kys9rI1ehagtgJQipxG9kMzMIKOaXZV4AU4/jkvLI6/zMIKHv04axp3Uk3ocrgE2JSkq8nYNlhPZ4is/0yTIKbsLKehvhP3z0V/ML2HktGxWHpzZq2thUyhovCyTcyrjV0SRSKWdY+W+Y1KabmZUlatUSEgcYRfABX6CvvY=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1C64B48025C0B8439349CB87AB163385@eurprd05.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 02854337-7ac7-42a5-0b25-08d7002958ce
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Jul 2019 02:43:08.4149 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: jackmin@mellanox.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR05MB5013
Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to 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://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Wed, 19-07-03, 17:25, Adrien Mazarguil wrote:
> On Tue, Jul 02, 2019 at 05:45:52PM +0800, Xiaoyu Min wrote:
> > Add new rte_flow_item_gre_key in order to match the optional key field.
> >=20
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
>=20
> OK with adding this feature, however I still have a bunch of comments bel=
ow.
>=20
> > ---
> >  doc/guides/prog_guide/rte_flow.rst | 8 ++++++++
> >  lib/librte_ethdev/rte_flow.c       | 1 +
> >  lib/librte_ethdev/rte_flow.h       | 7 +++++++
> >  3 files changed, 16 insertions(+)
> >=20
> > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide=
/rte_flow.rst
> > index a34d012e55..f4b7baa3c3 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -980,6 +980,14 @@ Matches a GRE header.
> >  - ``protocol``: protocol type.
> >  - Default ``mask`` matches protocol only.
> > =20
> > +Item: ``GRE_KEY``
> > +^^^^^^^^^^^^^^^^^
> > +
> > +Matches a GRE key field.
> > +This should be preceded by item ``GRE``
>=20
> Nit: missing ending "."
>=20

Ok, I'll add it.

> > +
> > +- Value to be matched is a big-endian 32 bit integer
> > +
> >  Item: ``FUZZY``
> >  ^^^^^^^^^^^^^^^
> > =20
> > diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.=
c
> > index 3277be1edb..f3e56d0bbe 100644
> > --- a/lib/librte_ethdev/rte_flow.c
> > +++ b/lib/librte_ethdev/rte_flow.c
> > @@ -55,6 +55,7 @@ static const struct rte_flow_desc_data rte_flow_desc_=
item[] =3D {
> >  	MK_FLOW_ITEM(NVGRE, sizeof(struct rte_flow_item_nvgre)),
> >  	MK_FLOW_ITEM(MPLS, sizeof(struct rte_flow_item_mpls)),
> >  	MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)),
> > +	MK_FLOW_ITEM(GRE_KEY, sizeof(rte_be32_t)),
>=20
> Hmm? Adding a new item in the middle?
>=20

I'll add it at the end.

> >  	MK_FLOW_ITEM(FUZZY, sizeof(struct rte_flow_item_fuzzy)),
> >  	MK_FLOW_ITEM(GTP, sizeof(struct rte_flow_item_gtp)),
> >  	MK_FLOW_ITEM(GTPC, sizeof(struct rte_flow_item_gtp)),
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.=
h
> > index f3a8fb103f..5d3702a44c 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -289,6 +289,13 @@ enum rte_flow_item_type {
> >  	 */
> >  	RTE_FLOW_ITEM_TYPE_GRE,
> > =20
> > +	/**
> > +	 * Matches a GRE optional key field.
> > +	 *
> > +	 * The value should a big-endian 32bit integer.
> > +	 */
> > +	RTE_FLOW_ITEM_TYPE_GRE_KEY,
> > +
>=20
> Same comment. While I understand the intent to group GRE and GRE_KEY, doi=
ng
> so causes ABI breakage by shifting the value of all subsequent pattern
> items (see IPV6 and IPV6_EXT for instance).
>=20

Oh, I was't aware of this. Thank you for explaination.

> We could later decide to sort them while knowingly breaking ABI on purpos=
e,
> however right now there's no choice but adding new pattern items and acti=
ons
> at the end of their respective enums, please do that.
>=20

Yes, I'll do this.

> --=20
> Adrien Mazarguil
> 6WIND