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? > > > > 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? > > > > > }, > > > [ITEM_HIGIG2] = { > > > .name = "higig2", > > > -- > > > 2.17.1
