[v2,2/3] app/testpmd: support PPPoL2TPv2oUDP RSS Hash

Message ID 20211012102508.275790-3-jie1x.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series support PPPoL2TPv2oUDP RSS Hash |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jie Wang Oct. 12, 2021, 10:25 a.m. UTC
  Add support for test-pmd to parse protocol pattern L2TPv2 and PPP.

Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 app/test-pmd/cmdline_flow.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
  

Comments

Ori Kam Oct. 12, 2021, 3:31 p.m. UTC | #1
Hi Jie,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Jie Wang
> Sent: Tuesday, October 12, 2021 1:25 PM
> Subject: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support PPPoL2TPv2oUDP RSS Hash
> 
> Add support for test-pmd to parse protocol pattern L2TPv2 and PPP.
> 
> Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  app/test-pmd/cmdline_flow.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c index
> bb22294dd3..3c9bcabd97 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -299,6 +299,8 @@ enum index {
>  	ITEM_GENEVE_OPT_TYPE,
>  	ITEM_GENEVE_OPT_LENGTH,
>  	ITEM_GENEVE_OPT_DATA,
> +	ITEM_PPP,
> +	ITEM_L2TPV2,
>  	ITEM_INTEGRITY,
>  	ITEM_INTEGRITY_LEVEL,
>  	ITEM_INTEGRITY_VALUE,
> @@ -997,6 +999,8 @@ static const enum index next_item[] = {
>  	ITEM_AH,
>  	ITEM_PFCP,
>  	ITEM_ECPRI,
> +	ITEM_PPP,
> +	ITEM_L2TPV2,

Why in the middle?

>  	ITEM_GENEVE_OPT,
>  	ITEM_INTEGRITY,
>  	ITEM_CONNTRACK,
> @@ -1368,6 +1372,16 @@ static const enum index item_integrity_lv[] = {
>  	ZERO,
>  };
> 
> +static const enum index item_ppp[] = {
> +	ITEM_NEXT,
> +	ZERO,
> +};
> +
> +static const enum index item_l2tpv2[] = {
> +	ITEM_NEXT,
> +	ZERO,
> +};
> +
>  static const enum index next_action[] = {
>  	ACTION_END,
>  	ACTION_VOID,
> @@ -3579,6 +3593,20 @@ static const struct token token_list[] = {
>  				(sizeof(struct rte_flow_item_geneve_opt),
>  				ITEM_GENEVE_OPT_DATA_SIZE)),
>  	},
> +	[ITEM_PPP] = {
> +		.name = "ppp",
> +		.help = "match ppp header",
> +		.priv = PRIV_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
> +		.next = NEXT(item_ppp),
> +		.call = parse_vc,
> +	},
> +	[ITEM_L2TPV2] = {
> +		.name = "l2tpv2",
> +		.help = "match l2tpv2 header",
> +		.priv = PRIV_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
> +		.next = NEXT(item_l2tpv2),
> +		.call = parse_vc,
> +	},
>  	[ITEM_INTEGRITY] = {
>  		.name = "integrity",
>  		.help = "match packet integrity",
> @@ -8343,6 +8371,12 @@ flow_item_default_mask(const struct rte_flow_item *item)
>  	case RTE_FLOW_ITEM_TYPE_PFCP:
>  		mask = &rte_flow_item_pfcp_mask;
>  		break;
> +	case RTE_FLOW_ITEM_TYPE_L2TPV2:
> +		mask = &rte_flow_item_l2tpv2_mask;
> +		break;
> +	case RTE_FLOW_ITEM_TYPE_PPP:
> +		mask = &rte_flow_item_ppp_mask;
> +		break;
>  	default:
>  		break;
>  	}
> --
> 2.25.1

Maybe I'm missing something but I don't see that you added the ability to match on any
of the header fields value.
You also didn't update the code of encap (from my understanding this is a tunnel header)

Best,
Ori
  
Jie Wang Oct. 13, 2021, 8:15 a.m. UTC | #2
> -----Original Message-----
> From: Ori Kam <orika@nvidia.com>
> Sent: Tuesday, October 12, 2021 11:32 PM
> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru; Li, Xiaoyun
> <xiaoyun.li@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Wu, Wenjun1
> <wenjun1.wu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support PPPoL2TPv2oUDP
> RSS Hash
> 
> Hi Jie,
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Jie Wang
> > Sent: Tuesday, October 12, 2021 1:25 PM
> > Subject: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support PPPoL2TPv2oUDP
> > RSS Hash
> >
> > Add support for test-pmd to parse protocol pattern L2TPv2 and PPP.
> >
> > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > ---
> >  app/test-pmd/cmdline_flow.c | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index
> > bb22294dd3..3c9bcabd97 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -299,6 +299,8 @@ enum index {
> >  	ITEM_GENEVE_OPT_TYPE,
> >  	ITEM_GENEVE_OPT_LENGTH,
> >  	ITEM_GENEVE_OPT_DATA,
> > +	ITEM_PPP,
> > +	ITEM_L2TPV2,
> >  	ITEM_INTEGRITY,
> >  	ITEM_INTEGRITY_LEVEL,
> >  	ITEM_INTEGRITY_VALUE,
> > @@ -997,6 +999,8 @@ static const enum index next_item[] = {
> >  	ITEM_AH,
> >  	ITEM_PFCP,
> >  	ITEM_ECPRI,
> > +	ITEM_PPP,
> > +	ITEM_L2TPV2,
> 
> Why in the middle?
> 

Ok, I will update it.

> >  	ITEM_GENEVE_OPT,
> >  	ITEM_INTEGRITY,
> >  	ITEM_CONNTRACK,
> > @@ -1368,6 +1372,16 @@ static const enum index item_integrity_lv[] = {
> >  	ZERO,
> >  };
> >
> > +static const enum index item_ppp[] = {
> > +	ITEM_NEXT,
> > +	ZERO,
> > +};
> > +
> > +static const enum index item_l2tpv2[] = {
> > +	ITEM_NEXT,
> > +	ZERO,
> > +};
> > +
> >  static const enum index next_action[] = {
> >  	ACTION_END,
> >  	ACTION_VOID,
> > @@ -3579,6 +3593,20 @@ static const struct token token_list[] = {
> >  				(sizeof(struct rte_flow_item_geneve_opt),
> >  				ITEM_GENEVE_OPT_DATA_SIZE)),
> >  	},
> > +	[ITEM_PPP] = {
> > +		.name = "ppp",
> > +		.help = "match ppp header",
> > +		.priv = PRIV_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
> > +		.next = NEXT(item_ppp),
> > +		.call = parse_vc,
> > +	},
> > +	[ITEM_L2TPV2] = {
> > +		.name = "l2tpv2",
> > +		.help = "match l2tpv2 header",
> > +		.priv = PRIV_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
> > +		.next = NEXT(item_l2tpv2),
> > +		.call = parse_vc,
> > +	},
> >  	[ITEM_INTEGRITY] = {
> >  		.name = "integrity",
> >  		.help = "match packet integrity",
> > @@ -8343,6 +8371,12 @@ flow_item_default_mask(const struct
> rte_flow_item *item)
> >  	case RTE_FLOW_ITEM_TYPE_PFCP:
> >  		mask = &rte_flow_item_pfcp_mask;
> >  		break;
> > +	case RTE_FLOW_ITEM_TYPE_L2TPV2:
> > +		mask = &rte_flow_item_l2tpv2_mask;
> > +		break;
> > +	case RTE_FLOW_ITEM_TYPE_PPP:
> > +		mask = &rte_flow_item_ppp_mask;
> > +		break;
> >  	default:
> >  		break;
> >  	}
> > --
> > 2.25.1
> 
> Maybe I'm missing something but I don't see that you added the ability to match
> on any of the header fields value.
> You also didn't update the code of encap (from my understanding this is a tunnel
> header)
> 
> Best,
> Ori

Hi Ori,

This feature is only support for iavf enable PPPoL2TPv2oUDP rss. So it doesn't need to add the ability to match on any of the header fields value and the code of encap.

I'm not sure if it is necessary to add these.
  
Ori Kam Oct. 13, 2021, 9:15 a.m. UTC | #3
Hi Wang,

> -----Original Message-----
> From: Wang, Jie1X <jie1x.wang@intel.com>
> Sent: Wednesday, October 13, 2021 11:16 AM
>
> Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support PPPoL2TPv2oUDP RSS Hash
> 
> 
> 
> > -----Original Message-----
> > From: Ori Kam <orika@nvidia.com>
> > Sent: Tuesday, October 12, 2021 11:32 PM
> > To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; NBU-Contact-Thomas
> > Monjalon <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru; Li,
> > Xiaoyun <xiaoyun.li@intel.com>; Yang, SteveX <stevex.yang@intel.com>;
> > Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support
> > PPPoL2TPv2oUDP RSS Hash
> >
> > Hi Jie,
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Jie Wang
> > > Sent: Tuesday, October 12, 2021 1:25 PM
> > > Subject: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support
> > > PPPoL2TPv2oUDP RSS Hash
> > >
> > > Add support for test-pmd to parse protocol pattern L2TPv2 and PPP.
> > >
> > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > > ---
> > >  app/test-pmd/cmdline_flow.c | 34 ++++++++++++++++++++++++++++++++++
> > >  1 file changed, 34 insertions(+)
> > >
> > > diff --git a/app/test-pmd/cmdline_flow.c
> > > b/app/test-pmd/cmdline_flow.c index
> > > bb22294dd3..3c9bcabd97 100644
> > > --- a/app/test-pmd/cmdline_flow.c
> > > +++ b/app/test-pmd/cmdline_flow.c
> > > @@ -299,6 +299,8 @@ enum index {
> > >  	ITEM_GENEVE_OPT_TYPE,
> > >  	ITEM_GENEVE_OPT_LENGTH,
> > >  	ITEM_GENEVE_OPT_DATA,
> > > +	ITEM_PPP,
> > > +	ITEM_L2TPV2,
> > >  	ITEM_INTEGRITY,
> > >  	ITEM_INTEGRITY_LEVEL,
> > >  	ITEM_INTEGRITY_VALUE,
> > > @@ -997,6 +999,8 @@ static const enum index next_item[] = {
> > >  	ITEM_AH,
> > >  	ITEM_PFCP,
> > >  	ITEM_ECPRI,
> > > +	ITEM_PPP,
> > > +	ITEM_L2TPV2,
> >
> > Why in the middle?
> >
> 
> Ok, I will update it.
> 
> > >  	ITEM_GENEVE_OPT,
> > >  	ITEM_INTEGRITY,
> > >  	ITEM_CONNTRACK,
> > > @@ -1368,6 +1372,16 @@ static const enum index item_integrity_lv[] = {
> > >  	ZERO,
> > >  };
> > >
> > > +static const enum index item_ppp[] = {
> > > +	ITEM_NEXT,
> > > +	ZERO,
> > > +};
> > > +
> > > +static const enum index item_l2tpv2[] = {
> > > +	ITEM_NEXT,
> > > +	ZERO,
> > > +};
> > > +
> > >  static const enum index next_action[] = {
> > >  	ACTION_END,
> > >  	ACTION_VOID,
> > > @@ -3579,6 +3593,20 @@ static const struct token token_list[] = {
> > >  				(sizeof(struct rte_flow_item_geneve_opt),
> > >  				ITEM_GENEVE_OPT_DATA_SIZE)),
> > >  	},
> > > +	[ITEM_PPP] = {
> > > +		.name = "ppp",
> > > +		.help = "match ppp header",
> > > +		.priv = PRIV_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
> > > +		.next = NEXT(item_ppp),
> > > +		.call = parse_vc,
> > > +	},
> > > +	[ITEM_L2TPV2] = {
> > > +		.name = "l2tpv2",
> > > +		.help = "match l2tpv2 header",
> > > +		.priv = PRIV_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
> > > +		.next = NEXT(item_l2tpv2),
> > > +		.call = parse_vc,
> > > +	},
> > >  	[ITEM_INTEGRITY] = {
> > >  		.name = "integrity",
> > >  		.help = "match packet integrity", @@ -8343,6 +8371,12 @@
> > > flow_item_default_mask(const struct
> > rte_flow_item *item)
> > >  	case RTE_FLOW_ITEM_TYPE_PFCP:
> > >  		mask = &rte_flow_item_pfcp_mask;
> > >  		break;
> > > +	case RTE_FLOW_ITEM_TYPE_L2TPV2:
> > > +		mask = &rte_flow_item_l2tpv2_mask;
> > > +		break;
> > > +	case RTE_FLOW_ITEM_TYPE_PPP:
> > > +		mask = &rte_flow_item_ppp_mask;
> > > +		break;
> > >  	default:
> > >  		break;
> > >  	}
> > > --
> > > 2.25.1
> >
> > Maybe I'm missing something but I don't see that you added the ability
> > to match on any of the header fields value.
> > You also didn't update the code of encap (from my understanding this
> > is a tunnel
> > header)
> >
> > Best,
> > Ori
> 
> Hi Ori,
> 
> This feature is only support for iavf enable PPPoL2TPv2oUDP rss. So it doesn't need to add the ability
> to match on any of the header fields value and the code of encap.
> 
> I'm not sure if it is necessary to add these.

You added a lot of fields in the rte_flow and you don't give any way to test them. 
also Iike I said in previous patch what is the relation between matching items to RSS?
You didn't add it to the RSS possible support.

Best,
Ori
  
Jie Wang Oct. 13, 2021, 9:54 a.m. UTC | #4
> -----Original Message-----
> From: Ori Kam <orika@nvidia.com>
> Sent: Wednesday, October 13, 2021 5:15 PM
> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru; Li, Xiaoyun
> <xiaoyun.li@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Wu, Wenjun1
> <wenjun1.wu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support PPPoL2TPv2oUDP
> RSS Hash
> 
> Hi Wang,
> 
> > -----Original Message-----
> > From: Wang, Jie1X <jie1x.wang@intel.com>
> > Sent: Wednesday, October 13, 2021 11:16 AM
> >
> > Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support
> > PPPoL2TPv2oUDP RSS Hash
> >
> >
> >
> > > -----Original Message-----
> > > From: Ori Kam <orika@nvidia.com>
> > > Sent: Tuesday, October 12, 2021 11:32 PM
> > > To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> > > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; NBU-Contact-Thomas
> > > Monjalon <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru; Li,
> > > Xiaoyun <xiaoyun.li@intel.com>; Yang, SteveX
> > > <stevex.yang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> > > Beilei <beilei.xing@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support
> > > PPPoL2TPv2oUDP RSS Hash
> > >
> > > Hi Jie,
> > >
> > > > -----Original Message-----
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Jie Wang
> > > > Sent: Tuesday, October 12, 2021 1:25 PM
> > > > Subject: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support
> > > > PPPoL2TPv2oUDP RSS Hash
> > > >
> > > > Add support for test-pmd to parse protocol pattern L2TPv2 and PPP.
> > > >
> > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > > > ---
> > > >  app/test-pmd/cmdline_flow.c | 34
> > > > ++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 34 insertions(+)
> > > >
> > > > diff --git a/app/test-pmd/cmdline_flow.c
> > > > b/app/test-pmd/cmdline_flow.c index
> > > > bb22294dd3..3c9bcabd97 100644
> > > > --- a/app/test-pmd/cmdline_flow.c
> > > > +++ b/app/test-pmd/cmdline_flow.c
> > > > @@ -299,6 +299,8 @@ enum index {
> > > >  	ITEM_GENEVE_OPT_TYPE,
> > > >  	ITEM_GENEVE_OPT_LENGTH,
> > > >  	ITEM_GENEVE_OPT_DATA,
> > > > +	ITEM_PPP,
> > > > +	ITEM_L2TPV2,
> > > >  	ITEM_INTEGRITY,
> > > >  	ITEM_INTEGRITY_LEVEL,
> > > >  	ITEM_INTEGRITY_VALUE,
> > > > @@ -997,6 +999,8 @@ static const enum index next_item[] = {
> > > >  	ITEM_AH,
> > > >  	ITEM_PFCP,
> > > >  	ITEM_ECPRI,
> > > > +	ITEM_PPP,
> > > > +	ITEM_L2TPV2,
> > >
> > > Why in the middle?
> > >
> >
> > Ok, I will update it.
> >
> > > >  	ITEM_GENEVE_OPT,
> > > >  	ITEM_INTEGRITY,
> > > >  	ITEM_CONNTRACK,
> > > > @@ -1368,6 +1372,16 @@ static const enum index item_integrity_lv[] = {
> > > >  	ZERO,
> > > >  };
> > > >
> > > > +static const enum index item_ppp[] = {
> > > > +	ITEM_NEXT,
> > > > +	ZERO,
> > > > +};
> > > > +
> > > > +static const enum index item_l2tpv2[] = {
> > > > +	ITEM_NEXT,
> > > > +	ZERO,
> > > > +};
> > > > +
> > > >  static const enum index next_action[] = {
> > > >  	ACTION_END,
> > > >  	ACTION_VOID,
> > > > @@ -3579,6 +3593,20 @@ static const struct token token_list[] = {
> > > >  				(sizeof(struct rte_flow_item_geneve_opt),
> > > >  				ITEM_GENEVE_OPT_DATA_SIZE)),
> > > >  	},
> > > > +	[ITEM_PPP] = {
> > > > +		.name = "ppp",
> > > > +		.help = "match ppp header",
> > > > +		.priv = PRIV_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
> > > > +		.next = NEXT(item_ppp),
> > > > +		.call = parse_vc,
> > > > +	},
> > > > +	[ITEM_L2TPV2] = {
> > > > +		.name = "l2tpv2",
> > > > +		.help = "match l2tpv2 header",
> > > > +		.priv = PRIV_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
> > > > +		.next = NEXT(item_l2tpv2),
> > > > +		.call = parse_vc,
> > > > +	},
> > > >  	[ITEM_INTEGRITY] = {
> > > >  		.name = "integrity",
> > > >  		.help = "match packet integrity", @@ -8343,6 +8371,12 @@
> > > > flow_item_default_mask(const struct
> > > rte_flow_item *item)
> > > >  	case RTE_FLOW_ITEM_TYPE_PFCP:
> > > >  		mask = &rte_flow_item_pfcp_mask;
> > > >  		break;
> > > > +	case RTE_FLOW_ITEM_TYPE_L2TPV2:
> > > > +		mask = &rte_flow_item_l2tpv2_mask;
> > > > +		break;
> > > > +	case RTE_FLOW_ITEM_TYPE_PPP:
> > > > +		mask = &rte_flow_item_ppp_mask;
> > > > +		break;
> > > >  	default:
> > > >  		break;
> > > >  	}
> > > > --
> > > > 2.25.1
> > >
> > > Maybe I'm missing something but I don't see that you added the
> > > ability to match on any of the header fields value.
> > > You also didn't update the code of encap (from my understanding this
> > > is a tunnel
> > > header)
> > >
> > > Best,
> > > Ori
> >
> > Hi Ori,
> >
> > This feature is only support for iavf enable PPPoL2TPv2oUDP rss. So it
> > doesn't need to add the ability to match on any of the header fields value and
> the code of encap.
> >
> > I'm not sure if it is necessary to add these.
> 
> You added a lot of fields in the rte_flow and you don't give any way to test them.
> also Iike I said in previous patch what is the relation between matching items to
> RSS?
> You didn't add it to the RSS possible support.
> 
> Best,
> Ori

The feature apply a RSS rule on L2TP data packet (include PPP over L2TP) with inner IP / port as input set.
It doesn't need match any L2TP field.
  
Ori Kam Oct. 13, 2021, 10:19 a.m. UTC | #5
> -----Original Message-----
> From: Wang, Jie1X <jie1x.wang@intel.com>
> Sent: Wednesday, October 13, 2021 12:54 PM
> Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support PPPoL2TPv2oUDP RSS Hash
> 
> 
> 
> > -----Original Message-----
> > From: Ori Kam <orika@nvidia.com>
> > Sent: Wednesday, October 13, 2021 5:15 PM
> > To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; NBU-Contact-Thomas
> > Monjalon <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru; Li,
> > Xiaoyun <xiaoyun.li@intel.com>; Yang, SteveX <stevex.yang@intel.com>;
> > Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support
> > PPPoL2TPv2oUDP RSS Hash
> >
> > Hi Wang,
> >
> > > -----Original Message-----
> > > From: Wang, Jie1X <jie1x.wang@intel.com>
> > > Sent: Wednesday, October 13, 2021 11:16 AM
> > >
> > > Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support
> > > PPPoL2TPv2oUDP RSS Hash
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ori Kam <orika@nvidia.com>
> > > > Sent: Tuesday, October 12, 2021 11:32 PM
> > > > To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> > > > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; NBU-Contact-Thomas
> > > > Monjalon <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru; Li,
> > > > Xiaoyun <xiaoyun.li@intel.com>; Yang, SteveX
> > > > <stevex.yang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> > > > Xing, Beilei <beilei.xing@intel.com>; Wu, Wenjun1
> > > > <wenjun1.wu@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support
> > > > PPPoL2TPv2oUDP RSS Hash
> > > >
> > > > Hi Jie,
> > > >
> > > > > -----Original Message-----
> > > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Jie Wang
> > > > > Sent: Tuesday, October 12, 2021 1:25 PM
> > > > > Subject: [dpdk-dev] [PATCH v2 2/3] app/testpmd: support
> > > > > PPPoL2TPv2oUDP RSS Hash
> > > > >
> > > > > Add support for test-pmd to parse protocol pattern L2TPv2 and PPP.
> > > > >
> > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > > > > ---
> > > > >  app/test-pmd/cmdline_flow.c | 34
> > > > > ++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 34 insertions(+)
> > > > >
> > > > > diff --git a/app/test-pmd/cmdline_flow.c
> > > > > b/app/test-pmd/cmdline_flow.c index
> > > > > bb22294dd3..3c9bcabd97 100644
> > > > > --- a/app/test-pmd/cmdline_flow.c
> > > > > +++ b/app/test-pmd/cmdline_flow.c
> > > > > @@ -299,6 +299,8 @@ enum index {
> > > > >  	ITEM_GENEVE_OPT_TYPE,
> > > > >  	ITEM_GENEVE_OPT_LENGTH,
> > > > >  	ITEM_GENEVE_OPT_DATA,
> > > > > +	ITEM_PPP,
> > > > > +	ITEM_L2TPV2,
> > > > >  	ITEM_INTEGRITY,
> > > > >  	ITEM_INTEGRITY_LEVEL,
> > > > >  	ITEM_INTEGRITY_VALUE,
> > > > > @@ -997,6 +999,8 @@ static const enum index next_item[] = {
> > > > >  	ITEM_AH,
> > > > >  	ITEM_PFCP,
> > > > >  	ITEM_ECPRI,
> > > > > +	ITEM_PPP,
> > > > > +	ITEM_L2TPV2,
> > > >
> > > > Why in the middle?
> > > >
> > >
> > > Ok, I will update it.
> > >
> > > > >  	ITEM_GENEVE_OPT,
> > > > >  	ITEM_INTEGRITY,
> > > > >  	ITEM_CONNTRACK,
> > > > > @@ -1368,6 +1372,16 @@ static const enum index item_integrity_lv[] = {
> > > > >  	ZERO,
> > > > >  };
> > > > >
> > > > > +static const enum index item_ppp[] = {
> > > > > +	ITEM_NEXT,
> > > > > +	ZERO,
> > > > > +};
> > > > > +
> > > > > +static const enum index item_l2tpv2[] = {
> > > > > +	ITEM_NEXT,
> > > > > +	ZERO,
> > > > > +};
> > > > > +
> > > > >  static const enum index next_action[] = {
> > > > >  	ACTION_END,
> > > > >  	ACTION_VOID,
> > > > > @@ -3579,6 +3593,20 @@ static const struct token token_list[] = {
> > > > >  				(sizeof(struct rte_flow_item_geneve_opt),
> > > > >  				ITEM_GENEVE_OPT_DATA_SIZE)),
> > > > >  	},
> > > > > +	[ITEM_PPP] = {
> > > > > +		.name = "ppp",
> > > > > +		.help = "match ppp header",
> > > > > +		.priv = PRIV_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
> > > > > +		.next = NEXT(item_ppp),
> > > > > +		.call = parse_vc,
> > > > > +	},
> > > > > +	[ITEM_L2TPV2] = {
> > > > > +		.name = "l2tpv2",
> > > > > +		.help = "match l2tpv2 header",
> > > > > +		.priv = PRIV_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
> > > > > +		.next = NEXT(item_l2tpv2),
> > > > > +		.call = parse_vc,
> > > > > +	},
> > > > >  	[ITEM_INTEGRITY] = {
> > > > >  		.name = "integrity",
> > > > >  		.help = "match packet integrity", @@ -8343,6 +8371,12 @@
> > > > > flow_item_default_mask(const struct
> > > > rte_flow_item *item)
> > > > >  	case RTE_FLOW_ITEM_TYPE_PFCP:
> > > > >  		mask = &rte_flow_item_pfcp_mask;
> > > > >  		break;
> > > > > +	case RTE_FLOW_ITEM_TYPE_L2TPV2:
> > > > > +		mask = &rte_flow_item_l2tpv2_mask;
> > > > > +		break;
> > > > > +	case RTE_FLOW_ITEM_TYPE_PPP:
> > > > > +		mask = &rte_flow_item_ppp_mask;
> > > > > +		break;
> > > > >  	default:
> > > > >  		break;
> > > > >  	}
> > > > > --
> > > > > 2.25.1
> > > >
> > > > Maybe I'm missing something but I don't see that you added the
> > > > ability to match on any of the header fields value.
> > > > You also didn't update the code of encap (from my understanding
> > > > this is a tunnel
> > > > header)
> > > >
> > > > Best,
> > > > Ori
> > >
> > > Hi Ori,
> > >
> > > This feature is only support for iavf enable PPPoL2TPv2oUDP rss. So
> > > it doesn't need to add the ability to match on any of the header
> > > fields value and
> > the code of encap.
> > >
> > > I'm not sure if it is necessary to add these.
> >
> > You added a lot of fields in the rte_flow and you don't give any way to test them.
> > also Iike I said in previous patch what is the relation between
> > matching items to RSS?
> > You didn't add it to the RSS possible support.
> >
> > Best,
> > Ori
> 
> The feature apply a RSS rule on L2TP data packet (include PPP over L2TP) with inner IP / port as input
> set.
> It doesn't need match any L2TP field.

Just to be clear when you say RSS on mean RSS on ipv4+udp for example not on the ppp header itself right?
If so then the title should just state that you are adding new items (also for other patches)
since I can use this item to jump to other group or count or any other action. This patch dosesn't have anything to do with hash.

I understand that from your point of view you don't need to match on any field, but you added such fields and no one
can test it and no PMD support this so this feature is not valid.

Best,
Ori
  

Patch

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index bb22294dd3..3c9bcabd97 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -299,6 +299,8 @@  enum index {
 	ITEM_GENEVE_OPT_TYPE,
 	ITEM_GENEVE_OPT_LENGTH,
 	ITEM_GENEVE_OPT_DATA,
+	ITEM_PPP,
+	ITEM_L2TPV2,
 	ITEM_INTEGRITY,
 	ITEM_INTEGRITY_LEVEL,
 	ITEM_INTEGRITY_VALUE,
@@ -997,6 +999,8 @@  static const enum index next_item[] = {
 	ITEM_AH,
 	ITEM_PFCP,
 	ITEM_ECPRI,
+	ITEM_PPP,
+	ITEM_L2TPV2,
 	ITEM_GENEVE_OPT,
 	ITEM_INTEGRITY,
 	ITEM_CONNTRACK,
@@ -1368,6 +1372,16 @@  static const enum index item_integrity_lv[] = {
 	ZERO,
 };
 
+static const enum index item_ppp[] = {
+	ITEM_NEXT,
+	ZERO,
+};
+
+static const enum index item_l2tpv2[] = {
+	ITEM_NEXT,
+	ZERO,
+};
+
 static const enum index next_action[] = {
 	ACTION_END,
 	ACTION_VOID,
@@ -3579,6 +3593,20 @@  static const struct token token_list[] = {
 				(sizeof(struct rte_flow_item_geneve_opt),
 				ITEM_GENEVE_OPT_DATA_SIZE)),
 	},
+	[ITEM_PPP] = {
+		.name = "ppp",
+		.help = "match ppp header",
+		.priv = PRIV_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
+		.next = NEXT(item_ppp),
+		.call = parse_vc,
+	},
+	[ITEM_L2TPV2] = {
+		.name = "l2tpv2",
+		.help = "match l2tpv2 header",
+		.priv = PRIV_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
+		.next = NEXT(item_l2tpv2),
+		.call = parse_vc,
+	},
 	[ITEM_INTEGRITY] = {
 		.name = "integrity",
 		.help = "match packet integrity",
@@ -8343,6 +8371,12 @@  flow_item_default_mask(const struct rte_flow_item *item)
 	case RTE_FLOW_ITEM_TYPE_PFCP:
 		mask = &rte_flow_item_pfcp_mask;
 		break;
+	case RTE_FLOW_ITEM_TYPE_L2TPV2:
+		mask = &rte_flow_item_l2tpv2_mask;
+		break;
+	case RTE_FLOW_ITEM_TYPE_PPP:
+		mask = &rte_flow_item_ppp_mask;
+		break;
 	default:
 		break;
 	}