DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ori Kam <orika@mellanox.com>
To: "Zhang, Xiao" <xiao.zhang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Wang, Ying A" <ying.a.wang@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"Zhao1, Wei" <wei.zhao1@intel.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-dev] app/testpmd: fix PPPOES flow API
Date: Sun, 29 Mar 2020 10:18:36 +0000	[thread overview]
Message-ID: <AM6PR05MB51760E3F164A7E49BC0D29D8DBCA0@AM6PR05MB5176.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <2966f158164c411e897b3ab741787eea@intel.com>

Hi Xiao,

> -----Original Message-----
> From: Zhang, Xiao <xiao.zhang@intel.com>
> Sent: Sunday, March 29, 2020 12:06 PM
> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> Cc: Wang, Ying A <ying.a.wang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>; stable@dpdk.org
> Subject: RE: app/testpmd: fix PPPOES flow API
> 
> Hi Ori,
> 
> > -----Original Message-----
> > From: Ori Kam <orika@mellanox.com>
> > Sent: Sunday, March 29, 2020 2:28 PM
> > To: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org
> > Cc: Wang, Ying A <ying.a.wang@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>;
> stable@dpdk.org
> > Subject: RE: app/testpmd: fix PPPOES flow API
> >
> > 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?

> >
> > 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 <xiao.zhang@intel.com>
> > > Sent: Friday, March 27, 2020 11:19 AM
> > > To: dev@dpdk.org
> > > Cc: Ori Kam <orika@mellanox.com>; ying.a.wang@intel.com;
> > > qi.z.zhang@intel.com; wei.zhao1@intel.com; Xiao Zhang
> > > <xiao.zhang@intel.com>; 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 fix this issue.
> > >
> > > Fixes: 226c6e60c35b ("ethdev: add PPPoE to flow API")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> > > ---
> > >  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[] = {
> > >  	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[] = {
> > >
> > >  static const enum index item_pppoes[] = {
> > >  	ITEM_PPPOE_SEID,
> > > -	ITEM_NEXT,
> > > -	ZERO,
> > > -};
> > > -
> > > -static const enum index item_pppoe_proto_id[] = {
> > >  	ITEM_PPPOE_PROTO_ID,
> > >  	ITEM_NEXT,
> > >  	ZERO,
> > > @@ -2643,10 +2637,9 @@ static const struct token token_list[] = {
> > >  	[ITEM_PPPOE_PROTO_ID] = {
> > >  		.name = "proto_id",
> > >  		.help = "match PPPoE session protocol identifier",
> > > -		.priv = PRIV_ITEM(PPPOE_PROTO_ID,
> > > -				sizeof(struct rte_flow_item_pppoe_proto_id)),
> > > -		.next = NEXT(item_pppoe_proto_id),
> > > -		.call = parse_vc,
> > > +		.next = NEXT(item_pppoes, NEXT_ENTRY(UNSIGNED),
> > > item_param),
> > > +		.args = 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 identifier. */
> 1362 };
> 

Yes. Why don't you use this one?

> >
> > >  	},
> > >  	[ITEM_HIGIG2] = {
> > >  		.name = "higig2",
> > > --
> > > 2.17.1


  reply	other threads:[~2020-03-29 10:18 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-27  8:19 Xiao Zhang
2020-03-29  6:27 ` Ori Kam
2020-03-29  9:06   ` Zhang, Xiao
2020-03-29 10:18     ` Ori Kam [this message]
2020-03-29 12:00       ` Zhang, Xiao
2020-03-29 12:45         ` Ori Kam
2020-03-30  2:08           ` Zhang, Xiao
2020-03-30  7:42             ` Ori Kam
2020-03-30  8:49               ` Zhao1, Wei
2020-03-30  8:57                 ` Ori Kam
2020-03-30 14:22                   ` Zhang, Xiao
2020-03-31  6:55                     ` Ori Kam
2020-03-31  8:13                       ` Zhang, Xiao
2020-03-31  9:05                         ` Ori Kam
2020-03-31 13:04                           ` Zhang, Xiao
2020-03-31 13:29 ` [dpdk-dev] [v2] " Xiao Zhang
2020-04-05 15:12   ` Ori Kam
2020-04-08 14:44     ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM6PR05MB51760E3F164A7E49BC0D29D8DBCA0@AM6PR05MB5176.eurprd05.prod.outlook.com \
    --to=orika@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=qi.z.zhang@intel.com \
    --cc=stable@dpdk.org \
    --cc=wei.zhao1@intel.com \
    --cc=xiao.zhang@intel.com \
    --cc=ying.a.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).