Re: [dpdk-stable] app/testpmd: fix PPPOES flow API

[Ori Kam <orika@mellanox.com>]

Hi Xiao,

Is the proto_id part of the basic header or not?

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.

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?

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