From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1F8B4A0562; Tue, 31 Mar 2020 08:55:29 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2EEFE1BFC1; Tue, 31 Mar 2020 08:55:28 +0200 (CEST) Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2042.outbound.protection.outlook.com [40.107.21.42]) by dpdk.org (Postfix) with ESMTP id 718CB2C6D; Tue, 31 Mar 2020 08:55:26 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Kr7wtV9jCHeXbPd3zPcjE2DX1Kvn8DVsx/tv2fsu/Jh9WgmNaK+WdH3RBP3+2lxatgV9R85S8dpEcvYUYeGqFYQWCiQ1YZjiFJ/F/hkJveHyS8klbuyRR8Tzi6Wvzs0ORtSpQqA9GZdlQZ2g7I8mBdVxPcEsEfgjeXetfbv3AtMVmwsyrNwc87F7SJLeIweWfy8aZRyxx9drvlDx9DdDpOxkAQRK7iUXz2HOCXE+9xMUFL275YJxrtweOmvyRpjUe152geFmMP6/HaRX7mgZ76HVElCjQu4/bFlxWR/0c0MG93Im5ISlBtH2PlGhpwt3Pzat2c2GqOUL45tdp6lWdA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ypOOeP87lyYRhYkG3lqS68T/fpIzL840sSSopcMbuJ0=; b=I/QbK7EWBfUMbgbvF4dDKw//sUiiz87hBtmwcW2eI/kZ8ky0loKaXTWK2nHn2iJ/DYukqQNsP1veSgA8H94SdtSZhc58vuhs6X3AYJ45lEGjq/3jVPzM+gQtpSivwZkQM9duv4DDQyC1YXYOJAD8SbfoP/a7YXBaIBrQ5pXSSSwCRjknHF31zwUVYuSmqV9KKTmOin+jqGQX5gEknBCfZBXT8XtqoTzNajTdUh3r+O9ErNqsdmq79wkIH/mVTZaq3++GmLgM10F+/z3cBCzUUQLKN6MTsUgZxPYQULO9VhtFCz+IlKizOjU+Bw2Fw0sbsge+79qYxnuy78y/ecc9zg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=mellanox.com; dmarc=pass action=none header.from=mellanox.com; dkim=pass header.d=mellanox.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ypOOeP87lyYRhYkG3lqS68T/fpIzL840sSSopcMbuJ0=; b=ixcOE6G4hpBaWRbgH+BEHlbuB5iZDys2IZc3M4fh/FpvcKkKyRreb3yM3A30cmPRVYSXsEFaUt+hylrkDwnafonmO5phF7awKnqHlKK15LoPgTjsZzslbaNRXVGKpr5L8gu55IIfQCAwhgp2z/3MB3viYNUfFWPYRD8TcWdMGtw= Received: from AM6PR05MB5176.eurprd05.prod.outlook.com (20.177.196.158) by AM6PR05MB6229.eurprd05.prod.outlook.com (20.178.92.160) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2856.19; Tue, 31 Mar 2020 06:55:24 +0000 Received: from AM6PR05MB5176.eurprd05.prod.outlook.com ([fe80::f5cd:b10f:5f1b:4b22]) by AM6PR05MB5176.eurprd05.prod.outlook.com ([fe80::f5cd:b10f:5f1b:4b22%7]) with mapi id 15.20.2856.019; Tue, 31 Mar 2020 06:55:24 +0000 From: Ori Kam To: "Zhang, Xiao" , "Zhao1, Wei" , "dev@dpdk.org" CC: "Wang, Ying A" , "Zhang, Qi Z" , "stable@dpdk.org" Thread-Topic: app/testpmd: fix PPPOES flow API Thread-Index: AQHWBBGaeM8mPPPhN06u+6qAGH787KhfHGRwgAAuDYCAABH+kIAAHq0AgAALNhCAAOHZAIAAW3SAgAAUf4CAAABTEIAAXMIAgAEOjQA= Date: Tue, 31 Mar 2020 06:55:24 +0000 Message-ID: References: <20200327081926.6154-1-xiao.zhang@intel.com> <2966f158164c411e897b3ab741787eea@intel.com> <2723defc86e04f0aaeb42a14183b4b5f@intel.com> <17e9c85bd1ee4ce190fbd4d1be26105e@intel.com> <704bf77207994d6182d5c4df625004e9@intel.com> In-Reply-To: <704bf77207994d6182d5c4df625004e9@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=orika@mellanox.com; x-originating-ip: [185.175.35.255] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 5c93ad60-169a-407f-b945-08d7d5407cff x-ms-traffictypediagnostic: AM6PR05MB6229: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 0359162B6D x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AM6PR05MB5176.eurprd05.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(396003)(346002)(366004)(376002)(136003)(39860400002)(66446008)(66476007)(86362001)(66946007)(5660300002)(30864003)(52536014)(2906002)(55016002)(9686003)(186003)(76116006)(6506007)(7696005)(478600001)(81156014)(8676002)(81166006)(110136005)(64756008)(66556008)(54906003)(966005)(4326008)(8936002)(33656002)(26005)(316002)(71200400001)(21314003); DIR:OUT; SFP:1101; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: E7zko94HNfCZjoDkcZ0bmg5eX4Dl6+SVEg2KPxiWr6a/Bih78nmlF2Xcc0I/ygBgZA/VoDddyzQ83S07xrOCaqUlx+oKuSB9ibYEyIzkaTt8YsdmjBfgnrhIc2hQPjU/uywywvbYeLN7vKtMQq9kj2thou29hK0SzRXgyRs3QTiQvQFxGZhsBYnDZucWXEgaT1PLWHMTLr6+HjYS+SDYCjLoXcIo/rC6SdG5rYHvqk3wgvBMqy+iCQhH4+iiTW6g6msqyJYe/fLlc33mH98Y1z0WZHC5Pev2bYlIpLGFyUXckVau0YP4mWou924ctagK9MnhaUOGc08v7nWYQ51/77mJsB+s1h+STqbZMoSwIdkJw35IlUCUfW/tDO/WQhR9Fr+pZQ3fSkz03j0nyOAJC3KJ7JEEm3Wy6Pgs6DBGD86g5c3lyknMGwloxBS+N98KAtnqVvjC6ALSxnQri8fjB3TTQW2rRjzKK5Gn+83U7/hWaXzCfs9jpwcuRwkNq8y5vHNqCF5uZn0IvUwvjqLt8PLeez+yROj5+whLr5aIJlrb5v6/S6/a3r1mMLEiIkBT x-ms-exchange-antispam-messagedata: ZheuD3vRvMXjlj7Qn89iNVFBmoQHFq5qL2uLYA/j10dh+tnfH3n7GHQF/FOrW1Yw1iok5h3imV93sUeCaASNum2XjiTrquVGlpFBXiwfotwgRIzpWYJrpu/ZV5ijZAQSDJ0a62V7VRkUzmmA/G4iFA== x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 5c93ad60-169a-407f-b945-08d7d5407cff X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Mar 2020 06:55:24.6372 (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: /9aTDvNdPMTU/8MvYZGuo6AjQjcfqHS25/rFLHpSY0DW6UYJj4TbyjhP+sQvv13gP4q1+IyTOvs33WIQ1Lalpg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR05MB6229 Subject: Re: [dpdk-dev] app/testpmd: fix PPPOES 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Zhang, Xiao >=20 > Hi Ori, >=20 > > -----Original Message----- > > From: Ori Kam > > > > Hi Xiao, > > > > > -----Original Message----- > > > From: Zhao1, Wei > > > > > > Hi, Ori > > > > > > > -----Original Message----- > > > > From: Ori Kam > > > > > > > > Hi Xiao > > > > > > > > > -----Original Message----- > > > > > From: Zhang, Xiao > > > > > > > > > > Hi Ori, > > > > > > > > > > > -----Original Message----- > > > > > > From: Ori Kam > > > > > > > > > > > > Hi Xiao, > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Zhang, Xiao > > > > > > > > > > > > > > Hi Ori, > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Ori Kam > > > > > > > > > > > > > > > > Hi Xiao, > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > From: Zhang, Xiao > > > > > > > > > Sent: Sunday, March 29, 2020 12:06 PM > > > > > > > > > To: Ori Kam ; dev@dpdk.org > > > > > > > > > Cc: Wang, Ying A ; Zhang, Qi Z > > > > > > > > > ; Zhao1, Wei ; > > > > > > > > > stable@dpdk.org > > > > > > > > > Subject: RE: app/testpmd: fix PPPOES flow API > > > > > > > > > > > > > > > > > > Hi Ori, > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > From: Ori Kam > > > > > > > > > > > > > > > > > > > > Hi Xiao, > > > > > > > > > > > > > > > > > > > > Is the proto_id part of the basic header or not? > > > > > > > > > > > > > > > > > > Proto_id is part of PPPOE session header, > > > > > > > > > > > > > > > > > > > > > > > > > Where is the porto_id located? Inside the payload? > > > > > > > > > > > > > > Yes, my previous explanation was not clear. The protocol ID i= s > > > > > > > in the beginning of the payload in PPP Session Stage accordin= g to > > RFC2516. > > > > > > > > > > > > > > 1 > > > > 2 3 > > > > > > > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 = 5 > > > > > > > 6 7 > > > > > > > 8 9 > > > > > > > 0 1 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-= +-+-+-+- > + > > > > > > > | VER | TYPE | CODE | > > > > SESSION_ID | > > > > > > > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-= +-+-+ > > > > > > > | LENGTH | > > > > payload ~ > > > > > > > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > > > > > > > > > > > > > > > > > > Yes this is what I thought, does proto_id must be the first par= t > > > > > > of the > > > > payload? > > > > > > > > > > It must be the first part of the payload for PPP Session Stage, > > > > > not all PPPOE packets. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From the spec it looks like a different header. > > > > > > > > > > > > > > > > > > > > If it is part of the original header then all > > > > > > > > > > documentations and rte_structs > > > > > > > > > should > > > > > > > > > > be changed, to reflect this. > > > > > > > > > > > > > > > > > > > > It will be very helpful if the patch message would > > > > > > > > > > explain the bug and why it > > > > > > > > > was > > > > > > > > > > changed. > > > > > > > > > > > > > > > > > > Okay, will add more message. The next value of the > > > > > > > ITEM_PPPOE_PROTO_ID > > > > > > > > > should be unsigned value but not item list. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Also please see inline other comment. > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > Ori > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > > From: Xiao Zhang > > > > > > > > > > > Sent: Friday, March 27, 2020 11:19 AM > > > > > > > > > > > To: dev@dpdk.org > > > > > > > > > > > Cc: Ori Kam ; > > > > > > > > > > > ying.a.wang@intel.com; qi.z.zhang@intel.com; > > > > > > > > > > > wei.zhao1@intel.com; Xiao Zhang > > > > > > > > > > > ; stable@dpdk.org > > > > > > > > > > > Subject: app/testpmd: fix PPPOES flow API > > > > > > > > > > > > > > > > > > > > > > The command line to create RTE flow for specific > > > > > > > > > > > proto_id of PPPOES is not correct. This patch is to f= ix this > issue. > > > > > > > > > > > > > > > > > > > > > > Fixes: 226c6e60c35b ("ethdev: add PPPoE to flow API") > > > > > > > > > > > Cc: stable@dpdk.org > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Xiao Zhang > > > > > > > > > > > --- > > > > > > > > > > > app/test-pmd/cmdline_flow.c | 13 +++---------- > > > > > > > > > > > 1 file changed, 3 insertions(+), 10 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/app/test-pmd/cmdline_flow.c > > > > > > > > > > > b/app/test-pmd/cmdline_flow.c index > > > > > > > > > > > a78154502..c25a2598d > > > > > > > > > > > 100644 > > > > > > > > > > > --- a/app/test-pmd/cmdline_flow.c > > > > > > > > > > > +++ b/app/test-pmd/cmdline_flow.c > > > > > > > > > > > @@ -768,7 +768,6 @@ static const enum index next_item= [] > =3D { > > > > > > > > > > > ITEM_GTP_PSC, > > > > > > > > > > > ITEM_PPPOES, > > > > > > > > > > > ITEM_PPPOED, > > > > > > > > > > > - ITEM_PPPOE_PROTO_ID, > > > > > > > > > > > ITEM_HIGIG2, > > > > > > > > > > > ITEM_TAG, > > > > > > > > > > > ITEM_L2TPV3OIP, > > > > > > > > > > > @@ -1030,11 +1029,6 @@ static const enum index > > > > > > > > > > > item_pppoed[] =3D { > > > > > > > > > > > > > > > > > > > > > > static const enum index item_pppoes[] =3D { > > > > > > > > > > > ITEM_PPPOE_SEID, > > > > > > > > > > > - ITEM_NEXT, > > > > > > > > > > > - ZERO, > > > > > > > > > > > -}; > > > > > > > > > > > - > > > > > > > > > > > -static const enum index item_pppoe_proto_id[] =3D { > > > > > > > > > > > ITEM_PPPOE_PROTO_ID, > > > > > > > > > > > ITEM_NEXT, > > > > > > > > > > > ZERO, > > > > > > > > > > > @@ -2643,10 +2637,9 @@ static const struct token > > > > > > > > > > > token_list[] > > > =3D > > > > { > > > > > > > > > > > [ITEM_PPPOE_PROTO_ID] =3D { > > > > > > > > > > > .name =3D "proto_id", > > > > > > > > > > > .help =3D "match PPPoE session protocol > > identifier", > > > > > > > > > > > - .priv =3D PRIV_ITEM(PPPOE_PROTO_ID, > > > > > > > > > > > - sizeof(struct > > > > > > > rte_flow_item_pppoe_proto_id)), > > > > > > > > > > > - .next =3D NEXT(item_pppoe_proto_id), > > > > > > > > > > > - .call =3D parse_vc, > > > > > > > > > > > + .next =3D NEXT(item_pppoes, > > NEXT_ENTRY(UNSIGNED), > > > > > > > > > > > item_param), > > > > > > > > > > > + .args =3D ARGS(ARGS_ENTRY_HTON > > > > > > > > > > > + (struct > > rte_flow_item_pppoe_proto_id, > > > > > > > proto_id)), > > > > > > > > > > > > > > > > > > > > Where is the memory for this proto_id is defined? > > > > > > > > > > > > > > > > > > Do you mean this? > > > > > > > > > lib/librte_ethdev/rte_flow.h > > > > > > > > > 1360 struct rte_flow_item_pppoe_proto_id { > > > > > > > > > 1361 rte_be16_t proto_id; /**< PPP protocol ident= ifier. */ > > > > > > > > > 1362 }; > > > > > > > > > > > > > > > > > > > > > > > > > Yes. Why don't you use this one? > > > > > > > > > > > > > > I think I was using this, am I using it incorrectly? > > > > > > > > > > > > > > + .args =3D ARGS(ARGS_ENTRY_HTON > > > > > > > + (struct rte_flow_item_pppoe_proto_id, > > > proto_id)), > > > > > > > > > > > > > > > > > > > Yes but there is no space to save this data since you deleted t= he priv. > > > > > > I think you are trying to implement something like > > > > > > RTE_FLOW_ITEM_TYPE_IPV6_EXT. > > > > > > > > > > > > And I don't understand what was the problem with the previous > > > > > implementation. > > > > > > > > > > I deleted the priv because it changed to a subcommand in pppoes, > > > > > the command line will be like this: > > > > > testpmd> flow create 0 ingress pattern eth dst is > > > > > testpmd> 00:11:22:33:44:55 / pppoes > > > > > proto_id is 21 > > > > > > > > > > > > > The issue is that the pppoe struct doesn't have definition to the p= roto_id. > > > > If you wish a possible solution will be to add it to the pppoe > > > > struct, I'm not > > > sure > > > > if this is the correct approach since this field is not a must. > > > > > > > > Like I said there are examples on how to work with extended headers= , > > > > which I think are more correct, buy may be the problem is that the > > > > pppoe struct is > > > not > > > > aligned and this result in an issue when adding the last bytes. > > > > > > > > > There is a defination of RTE_FLOW_ITEM_TYPE_PPPOE_PROTO_ID, do you > > > mean make use of that? > > > That is the reason for use extended header for this? > > > But that seems as you say, has some bug. > > > > > > > I understand there is a bug, the question how to solve it. > > I suggested two approaches. Add the proto_id to the pppoe struct, but t= his > > means that we will add a new member that is not part of the original > definition. > > Maybe the issue is in the PMD and it needs to understand that the proto= _id > > should be located in a different offset. > > In any case it doesn't look like the current fix the right one. >=20 > From my understanding, you mean there are two approaches. One is adding > proto_id to pppoe struct. But you don't prefer this one since proto_id is= not a > must. I am not clear about the other one. >=20 The solution should be just like the pdu_type which is part of the gtp_psc. You can find also my comments on this, in the ML. I think it is exactly the same case. Example line for pdu type: flow create 0 ingress pattern gtp_psc pdu_t is x= xx The thread https://patches.dpdk.org/patch/67198/ > And also how do you suggest the command line be for proto_id? > "proto_id is 0x0021" or "pppoes proto_id is 0x0021"? If the former just l= ike > what it was, I think it maybe a little confused. If the latter (as proto_= id is part of > pppoes), do we still need to put proto_id in rte_flow_item_pppoe? >=20 > Thanks, > Xiao >=20 > > > > > > > > > > > > > > The previous implementation would be infinite loop for proto_id > > > > > command and can not specific the value for proto_id. > > > > > testpmd> flow create 0 ingress pattern eth dst is > > > > > testpmd> 00:11:22:33:44:55 / proto_id > > > > > proto_id [TOKEN]: match PPPoE session protocol identifier / [TO= KEN]: > > > > > specify next pattern item > > > > > testpmd> flow create 0 ingress pattern eth dst is > > > > > testpmd> 00:11:22:33:44:55 / proto_id > > > > > proto_id > > > > > proto_id [TOKEN]: match PPPoE session protocol identifier / [TO= KEN]: > > > > > specify next pattern item > > > > > testpmd> flow create 0 ingress pattern eth dst is > > > > > testpmd> 00:11:22:33:44:55 / proto_id > > > > > proto_id proto_id > > > > > proto_id [TOKEN]: match PPPoE session protocol identifier / [TO= KEN]: > > > > > specify next pattern item > > > > > testpmd> flow create 0 ingress pattern eth dst is > > > > > testpmd> 00:11:22:33:44:55 / proto_id > > > > > proto_id proto_id > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > }, > > > > > > > > > > > [ITEM_HIGIG2] =3D { > > > > > > > > > > > .name =3D "higig2", > > > > > > > > > > > -- > > > > > > > > > > > 2.17.1