Hi Xiao, > -----Original Message----- > From: Zhang, Xiao <[email protected]> > Sent: Sunday, March 29, 2020 3:00 PM > To: Ori Kam <[email protected]>; [email protected] > Cc: Wang, Ying A <[email protected]>; Zhang, Qi Z > <[email protected]>; Zhao1, Wei <[email protected]>; [email protected] > Subject: RE: app/testpmd: fix PPPOES flow API > > Hi Ori, > > > -----Original Message----- > > From: Ori Kam <[email protected]> > > Sent: Sunday, March 29, 2020 6:19 PM > > To: Zhang, Xiao <[email protected]>; [email protected] > > Cc: Wang, Ying A <[email protected]>; Zhang, Qi Z > > <[email protected]>; Zhao1, Wei <[email protected]>; > [email protected] > > Subject: RE: app/testpmd: fix PPPOES flow API > > > > Hi Xiao, > > > > > -----Original Message----- > > > From: Zhang, Xiao <[email protected]> > > > Sent: Sunday, March 29, 2020 12:06 PM > > > To: Ori Kam <[email protected]>; [email protected] > > > Cc: Wang, Ying A <[email protected]>; Zhang, Qi Z > > > <[email protected]>; Zhao1, Wei <[email protected]>; > > > [email protected] > > > Subject: RE: app/testpmd: fix PPPOES flow API > > > > > > Hi Ori, > > > > > > > -----Original Message----- > > > > From: Ori Kam <[email protected]> > > > > Sent: Sunday, March 29, 2020 2:28 PM > > > > To: Zhang, Xiao <[email protected]>; [email protected] > > > > Cc: Wang, Ying A <[email protected]>; Zhang, Qi Z > > > > <[email protected]>; Zhao1, Wei <[email protected]>; > > > [email protected] > > > > 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? > > > > > > > > > > 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 <[email protected]> > > > > > Sent: Friday, March 27, 2020 11:19 AM > > > > > To: [email protected] > > > > > Cc: Ori Kam <[email protected]>; [email protected]; > > > > > [email protected]; [email protected]; Xiao Zhang > > > > > <[email protected]>; [email protected] > > > > > 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: [email protected] > > > > > > > > > > Signed-off-by: Xiao Zhang <[email protected]> > > > > > --- > > > > > 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. > > > > > > > > > > > }, > > > > > [ITEM_HIGIG2] = { > > > > > .name = "higig2", > > > > > -- > > > > > 2.17.1
