From: "Zhang, Xiao" <xiao.zhang@intel.com>
To: Ori Kam <orika@mellanox.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-stable] app/testpmd: fix PPPOES flow API
Date: Mon, 30 Mar 2020 02:08:50 +0000 [thread overview]
Message-ID: <17e9c85bd1ee4ce190fbd4d1be26105e@intel.com> (raw)
In-Reply-To: <AM6PR05MB5176CFF8751C508A9037B241DBCA0@AM6PR05MB5176.eurprd05.prod.outlook.com>
Hi Ori,
> -----Original Message-----
> From: Ori Kam <orika@mellanox.com>
> Sent: Sunday, March 29, 2020 8:46 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,
>
> > -----Original Message-----
> > From: Zhang, Xiao <xiao.zhang@intel.com>
> > Sent: Sunday, March 29, 2020 3:00 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 6:19 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,
> > >
> > > > -----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?
> >
> > Yes, my previous explanation was not clear. The protocol ID is in the
> > beginning of the payload in PPP Session Stage according 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 part 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 <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?
> >
> > I think I was using this, am I using it incorrectly?
> >
> > + .args = 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 the 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 00:11:22:33:44:55 / pppoes proto_id is 21
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 00:11:22:33:44:55 / proto_id
proto_id [TOKEN]: match PPPoE session protocol identifier
/ [TOKEN]: specify next pattern item
testpmd> flow create 0 ingress pattern eth dst is 00:11:22:33:44:55 / proto_id proto_id
proto_id [TOKEN]: match PPPoE session protocol identifier
/ [TOKEN]: specify next pattern item
testpmd> flow create 0 ingress pattern eth dst is 00:11:22:33:44:55 / proto_id proto_id proto_id
proto_id [TOKEN]: match PPPoE session protocol identifier
/ [TOKEN]: specify next pattern item
testpmd> flow create 0 ingress pattern eth dst is 00:11:22:33:44:55 / proto_id proto_id proto_id
>
> > >
> > > > >
> > > > > > },
> > > > > > [ITEM_HIGIG2] = {
> > > > > > .name = "higig2",
> > > > > > --
> > > > > > 2.17.1
next prev parent reply other threads:[~2020-03-30 2:08 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
2020-03-29 12:00 ` Zhang, Xiao
2020-03-29 12:45 ` Ori Kam
2020-03-30 2:08 ` Zhang, Xiao [this message]
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-stable] [v2] " Xiao Zhang
2020-04-05 15:12 ` Ori Kam
2020-04-08 14:44 ` 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=17e9c85bd1ee4ce190fbd4d1be26105e@intel.com \
--to=xiao.zhang@intel.com \
--cc=dev@dpdk.org \
--cc=orika@mellanox.com \
--cc=qi.z.zhang@intel.com \
--cc=stable@dpdk.org \
--cc=wei.zhao1@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).