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 5626DA0487
	for <public@inbox.dpdk.org>; Fri,  5 Jul 2019 11:06:49 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 99B801BDE2;
	Fri,  5 Jul 2019 11:06:48 +0200 (CEST)
Received: from EUR03-VE1-obe.outbound.protection.outlook.com
 (mail-eopbgr50044.outbound.protection.outlook.com [40.107.5.44])
 by dpdk.org (Postfix) with ESMTP id DEF471B9C9
 for <dev@dpdk.org>; Fri,  5 Jul 2019 11:06:46 +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=nu4DPT3GxUr2duMMZhX8OJ0N+U//B1FnOnZH6OL1424=;
 b=DrhmQaDTTchX/dBoqkdKL24mLSdwsOZeT13FUg9jvYg11cDzwTMycs1rjgXoo/1QyZ78E6AssvPZ+GiiqzPKZG8yVCzyjUfGWwO5KLkGkrseIBuksfHl+uRwFRoSPeOyOqAm4/JWXmxspChmGtZwE4hB9s6644hKbUnpE4EWMEU=
Received: from AM6PR05MB6567.eurprd05.prod.outlook.com (20.179.6.215) by
 AM6PR05MB5459.eurprd05.prod.outlook.com (20.177.188.82) with Microsoft SMTP
 Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.2052.18; Fri, 5 Jul 2019 09:06:45 +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.019; Fri, 5 Jul 2019
 09:06:45 +0000
From: Jack Min <jackmin@mellanox.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
CC: Ori Kam <orika@mellanox.com>, Slava Ovsiienko <viacheslavo@mellanox.com>, 
 Wenzhuo Lu <wenzhuo.lu@intel.com>, Jingjing Wu <jingjing.wu@intel.com>,
 Bernard Iremonger <bernard.iremonger@intel.com>, John McNamara
 <john.mcnamara@intel.com>, Marko Kovacevic <marko.kovacevic@intel.com>,
 "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [PATCH v6 4/4] app/testpmd: match GRE's key and present bits
Thread-Index: AQHVMw/XKY1xZ2pj7kS0t60L78j4YKa7u8SA
Date: Fri, 5 Jul 2019 09:06:45 +0000
Message-ID: <20190705090635.43zvptgrwfbj7goj@mellanox.com>
References: <20190624154018.128379-1-jackmin@mellanox.com>
 <cover.1562292465.git.jackmin@mellanox.com>
 <c162b5bba3015a468f372337e7655966588a51ef.1562292465.git.jackmin@mellanox.com>
 <20190705085835.GQ4512@6wind.com>
In-Reply-To: <20190705085835.GQ4512@6wind.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-clientproxiedby: HK2PR02CA0185.apcprd02.prod.outlook.com
 (2603:1096:201:21::21) 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: 7233d5fb-9ad6-4434-8569-08d701281a6c
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:AM6PR05MB5459; 
x-ms-traffictypediagnostic: AM6PR05MB5459:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <AM6PR05MB545993E96C831ACE0B2E2657CCF50@AM6PR05MB5459.eurprd05.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:9508;
x-forefront-prvs: 008960E8EC
x-forefront-antispam-report: SFV:NSPM;
 SFS:(10009020)(4636009)(366004)(136003)(346002)(376002)(39860400002)(396003)(189003)(199004)(6506007)(386003)(76176011)(102836004)(316002)(54906003)(99286004)(52116002)(53936002)(6246003)(71190400001)(71200400001)(66066001)(5660300002)(36756003)(26005)(2906002)(66946007)(66556008)(66446008)(66476007)(73956011)(14444005)(256004)(64756008)(14454004)(186003)(229853002)(6116002)(3846002)(6512007)(4326008)(81156014)(6486002)(446003)(81166006)(8936002)(6916009)(11346002)(305945005)(2616005)(68736007)(476003)(7736002)(86362001)(1076003)(6436002)(486006)(8676002)(478600001)(25786009);
 DIR:OUT; SFP:1101; SCL:1; SRVR:AM6PR05MB5459;
 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: i96shQavzoQ7v1qcXpngjP5wiasGdWotI2q/R6NkGUWfh6Ff4uc5cmNT6IfZi78usog2q1GICZTOAZd0DUAluuPOKlJb9R5jCncU6NghyNTPQy9e8alN4JMZWLlZ4H5Q/BAl3KpmqONk9UN9wrhJ37wKsL+Z0Z2jkUiu94PcetuIsxPoWWTh/11bpHYX/a1G2hFgOpxXvV4K4MX15Jf8v8FDbDkDwmb7uzJg4cJRWA2LIXGuGYDt2mITEEN1ioRK3b2v3VtUD6UsLSjpqGb+cTBQuOJla1OJu2ssbz3KDcwc8l8MupObaXNgButPprMUkofDJUYeTC2/PWsI9/NEj6M+3MTz5vPLFw2zu7bXgUY5ttdCC7CMBC7Va4bE7BNYKN+26juiPbfMbxjZJUyKRuLtZUy8H1MhbPHmBS1KmsU=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <61D36324C0C90A4E8F1FF7AF42EE520F@eurprd05.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7233d5fb-9ad6-4434-8569-08d701281a6c
X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Jul 2019 09:06:45.3451 (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: AM6PR05MB5459
Subject: Re: [dpdk-dev] [PATCH v6 4/4] app/testpmd: match GRE's key and
	present bits
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 Fri, 19-07-05, 10:58, Adrien Mazarguil wrote:
> On Fri, Jul 05, 2019 at 10:14:45AM +0800, Xiaoyu Min wrote:
> > support matching on GRE key and present bits (C,K,S)
> >=20
> > example testpmd command could be:
> >   testpmd>flow create 0 ingress group 1 pattern eth / ipv4 /
> > 	  gre / gre_key value is 0x12345678 / end
> > 	  actions rss queues 1 0 end / mark id 196 / end
> >=20
> > Which will match GRE packet with k present bit set and key value is
> > 0x12345678.
> >=20
> > Acked-by: Ori Kam <orika@mellanox.com>
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
>=20
> A few more nits below.
>=20
> [...]
> > @@ -1898,6 +1915,50 @@ static const struct token token_list[] =3D {
> >  		.args =3D ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gre,
> >  					     protocol)),
> >  	},
> > +	[ITEM_GRE_C_RSVD0_VER] =3D {
> > +		.name =3D "c_rsvd0_ver",
> > +		.help =3D "GRE's first word (bit0 - bit15)",
>=20
> Help strings on existing fields should ideally be the same as their
> counterparts in rte_flow.h (shortened if necessary, not starting with a c=
ap
> and not ending "."), in this case for instance:
>=20
Didn't know this before.

>  .help =3D
>          "checksum (1b), undefined (1b), key bit (1b),"
>          " sequence number (1b), reserved 0 (9b),"
>          " version (3b)",
>=20
I'll update.

> > +		.next =3D NEXT(item_gre, NEXT_ENTRY(UNSIGNED), item_param),
> > +		.args =3D ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_gre,
> > +					     c_rsvd0_ver)),
> > +	},
> > +	[ITEM_GRE_C_BIT] =3D {
> > +		.name =3D "c_bit",
> > +		.help =3D "GRE's C present bit",
>=20
> A bit odd, here's a suggestion:
>=20
>  "checksum bit (C)".
>=20
I'll update.

> > +		.next =3D NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
> > +		.args =3D ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
> > +						  c_rsvd0_ver,
> > +						  "\x80\x00\x00\x00")),
> > +	},
> > +	[ITEM_GRE_S_BIT] =3D {
> > +		.name =3D "s_bit",
> > +		.help =3D "GRE's S present bit",
>=20
> Ditto:
>=20
>  "sequence number bit (S)"
>=20
Ditto.

> > +		.next =3D NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
> > +		.args =3D ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
> > +						  c_rsvd0_ver,
> > +						  "\x10\x00\x00\x00")),
> > +	},
> > +	[ITEM_GRE_K_BIT] =3D {
> > +		.name =3D "k_bit",
> > +		.help =3D "GRE's K present bit",
>=20
> Ditto:
>=20
>  "key bit (K)"
>=20
Ditto.

> > +		.next =3D NEXT(item_gre, NEXT_ENTRY(BOOLEAN), item_param),
> > +		.args =3D ARGS(ARGS_ENTRY_MASK_HTON(struct rte_flow_item_gre,
> > +						  c_rsvd0_ver,
> > +						  "\x20\x00\x00\x00")),
> > +	},
> > +	[ITEM_GRE_KEY] =3D {
> > +		.name =3D "gre_key",
> > +		.help =3D "match GRE Key",
>=20
> Nit: no caps for "Key" =3D> "match GRE key"
>=20
OK.

> > +		.priv =3D PRIV_ITEM(GRE_KEY, sizeof(rte_be32_t)),
> > +		.next =3D NEXT(item_gre_key),
> > +		.call =3D parse_vc,
> > +	},
> > +	[ITEM_GRE_KEY_VALUE] =3D {
> > +		.name =3D "value",
> > +		.help =3D "GRE key value",
>=20
> No need to repeat "GRE" here since it's already in GRE context:
>=20
>  "key value"
>=20
OK.

> > +		.next =3D NEXT(item_gre_key, NEXT_ENTRY(UNSIGNED), item_param),
> > +		.args =3D ARGS(ARG_ENTRY_HTON(rte_be32_t)),
> > +	},
>=20
> Also ITEM_GRE_KEY and ITEM_GRE_KEY_VALUE should come after ITEM_META_DATA=
 to
> keep the same order as everywhere else.
>=20
Yes, it should be.=20

> Then assuming all the suggested changes are made:
>=20
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>=20
Thank you!

> Note I did not look at mlx5 patches, please make sure someone has reviewe=
d
> them. Thanks.
>=20
Yes, Slava will review them.

> --=20
> Adrien Mazarguil
> 6WIND