Re: [dpdk-dev] [dpdk-dev v2 3/4] app/testpmd: support GTP PDU type

[Ori Kam <orika@mellanox.com>]

Hi Jeff,

My name is Ori 😊

I'm not an expert in GTP so this is just my thinking and maybe I'm
missing something, this is why a good explanation helps 😊

> -----Original Message-----
> From: Jeff Guo <jia.guo@intel.com>
> Sent: Monday, March 30, 2020 11:30 AM
> To: Ori Kam <orika@mellanox.com>; xiaolong.ye@intel.com;
> qi.z.zhang@intel.com
> Cc: dev@dpdk.org; jingjing.wu@intel.com; yahui.cao@intel.com;
> simei.su@intel.com
> Subject: Re: [dpdk-dev] [dpdk-dev v2 3/4] app/testpmd: support GTP PDU type
> 
> hi, orika
> 
> 
> On 3/29/2020 4:44 PM, Ori Kam wrote:
> > Hi Jeff,
> >
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Jeff Guo
> >> Sent: Thursday, March 26, 2020 6:41 PM
> >> To: xiaolong.ye@intel.com; qi.z.zhang@intel.com
> >> Cc: dev@dpdk.org; jingjing.wu@intel.com; yahui.cao@intel.com;
> >> simei.su@intel.com; jia.guo@intel.com
> >> Subject: [dpdk-dev] [dpdk-dev v2 3/4] app/testpmd: support GTP PDU type
> >>
> >> Add gtp pdu type configure in the cmdline.
> > Why not use ITEM_GTP_PSC_PDU?
> 
> 
> I guess you mean ITEM_GTP_PSC_PDU_T, rihgt? We know  we have got
> ITEM_GTP_PSC_QFI/ITEM_GTP_PSC_PDU_T but not define the
> 
> spec for them, so what i use is add the spec into the ITEM_GTP_PSC_PDU_T
> to let the pdu type to be configured.
> 
Yes you are correct, from rte_flow we have the  RTE_FLOW_ITEM_TYPE_GTP_PSC
Item that include pdu_type. This is the field you need right?

In testpmd we have the ITEM_GTP_PSC_PDU_T which should support adding
the pdu type.
Basically you just need to type the following cmd line:
flow create 0 ingress pattern gtp_psc pdu_t is xxx
if this command is not working we need to understand why.



> 
> >> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> >> ---
> >> v1:
> >> no change
> >> ---
> >>   app/test-pmd/cmdline_flow.c | 11 ++++++++++-
> >>   1 file changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> >> index a78154502..c1bd02919 100644
> >> --- a/app/test-pmd/cmdline_flow.c
> >> +++ b/app/test-pmd/cmdline_flow.c
> >> @@ -49,6 +49,7 @@ enum index {
> >>   	PORT_ID,
> >>   	GROUP_ID,
> >>   	PRIORITY_LEVEL,
> >> +	GTP_PSC_PDU_T,
> >>
> >>   	/* Top-level command. */
> >>   	SET,
> >> @@ -1626,6 +1627,13 @@ static const struct token token_list[] = {
> >>   		.call = parse_int,
> >>   		.comp = comp_none,
> >>   	},
> >> +	[GTP_PSC_PDU_T] = {
> >> +		.name = "{GTPU pdu type}",
> >> +		.type = "INTEGER",
> >> +		.help = "gtpu pdu uplink/downlink identifier",
> >> +		.call = parse_int,
> >> +		.comp = comp_none,
> >> +	},
> > Why is this created at this level?
> > This looks like is should be written totally differently.
> 
> 
> As i said above,  the item we got but spec or say next token still need
> to be added, do you mean it should not in the group of Common tokens? If
> so, let me think about that, and please explicit your proposal if you
> already have one.
> 

Please see above response.

> 
> >>   	/* Top-level command. */
> >>   	[FLOW] = {
> >>   		.name = "flow",
> >> @@ -2615,7 +2623,8 @@ static const struct token token_list[] = {
> >>   	[ITEM_GTP_PSC_PDU_T] = {
> >>   		.name = "pdu_t",
> >>   		.help = "PDU type",
> >> -		.next = NEXT(item_gtp_psc, NEXT_ENTRY(UNSIGNED),
> >> item_param),
> >> +		.next = NEXT(item_gtp_psc, NEXT_ENTRY(GTP_PSC_PDU_T),
> >> +			     item_param),
> >>   		.args = ARGS(ARGS_ENTRY_HTON(struct
> >> rte_flow_item_gtp_psc,
> >>   					pdu_type)),
> >>   	},
> >> --
> >> 2.20.1