[PATCH] app/testpmd: fix parsing for connection tracking item

[PATCH] app/testpmd: fix parsing for connection tracking item

[Maayan Kashani]

In command line translation there were missing fields for
connection tracking item, as a result this item was not parsed
and was missing from the items list received from test-pmd.

Fixes: 4d07cbefe3ba ("app/testpmd: add commands for conntrack")
Cc: stable@dpdk.org
Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..1f9d5ebd05 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -5797,9 +5797,12 @@ static const struct token token_list[] = {
 	[ITEM_CONNTRACK] = {
 		.name = "conntrack",
 		.help = "conntrack state",
+		.priv = PRIV_ITEM(CONNTRACK,
+				  sizeof(struct rte_flow_item_conntrack)),
 		.next = NEXT(NEXT_ENTRY(ITEM_NEXT), NEXT_ENTRY(COMMON_UNSIGNED),
 			     item_param),
 		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_conntrack, flags)),
+		.call = parse_vc,
 	},
 	[ITEM_PORT_REPRESENTOR] = {
 		.name = "port_representor",
-- 
2.25.1

Re: [PATCH] app/testpmd: fix parsing for connection tracking item

[Ferruh Yigit]

On 6/2/2024 11:23 AM, Maayan Kashani wrote:
> In command line translation there were missing fields for
> connection tracking item, as a result this item was not parsed
> and was missing from the items list received from test-pmd.
> 
> Fixes: 4d07cbefe3ba ("app/testpmd: add commands for conntrack")
> Cc: stable@dpdk.org
> Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
>

Hi Maayan,

I can't really trace why .priv and 'parse_vc' is required, can you
please help understanding by providing more data?

What was the old testpmd command and what was old behavior with that
command, why it was wrong?
What is the new behavior and why this update was needed, how new code
solved the issue?

testpmd flow command is still somewhat cryptic to me, and without
running code and debugging it is very hard for me to detect the missing
parts.


> ---
>  app/test-pmd/cmdline_flow.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 60ee9337cf..1f9d5ebd05 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -5797,9 +5797,12 @@ static const struct token token_list[] = {
>  	[ITEM_CONNTRACK] = {
>  		.name = "conntrack",
>  		.help = "conntrack state",
> +		.priv = PRIV_ITEM(CONNTRACK,
> +				  sizeof(struct rte_flow_item_conntrack)),
>  		.next = NEXT(NEXT_ENTRY(ITEM_NEXT), NEXT_ENTRY(COMMON_UNSIGNED),
>  			     item_param),
>  		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_conntrack, flags)),
> +		.call = parse_vc,
>  	},
>  	[ITEM_PORT_REPRESENTOR] = {
>  		.name = "port_representor",

RE: [PATCH] app/testpmd: fix parsing for connection tracking item

[Maayan Kashani]

Sure,
This is an example of using connection tracking with testpmd:

start
    set verbose 1
    set conntrack com peer 1 is_orig 1 enable 1 live 1 sack 1 cack 0 last_dir 0 liberal 0 state 1 max_ack_win 7 r_lim 5 last_win 510 last_seq 2632987379 last_ack 2532480967 last_end 2632987379 last_index 0x8
    set conntrack orig scale 7 fin 0 acked 1 unack_data 0 sent_end 2632987379 reply_end 2633016339 max_win 28960 max_ack 2632987379
    set conntrack rply scale 7 fin 0 acked 1 unack_data 0 sent_end 2532480967 reply_end 2532546247 max_win 65280 max_ack 2532480967
    flow indirect_action 0 create ingress action conntrack / end

    flow create 0 group 3 ingress pattern eth / ipv4 / tcp / end actions indirect 0 / jump group 5 / end

    flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 1 / end actions queue index 5 / end

before my changes, conntracking was missed in the flow items list i got from the parsing of testpmd for the second flow created.
I just aligned the parsing of connection tracking to be the same as other items.(e.g ITEM_PORT_REPRESENTOR)


Regards,
Maayan Kashani

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Wednesday, 12 June 2024 19:35
> To: Maayan Kashani <mkashani@nvidia.com>; dev@dpdk.org
> Cc: Dariusz Sosnowski <dsosnowski@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>; stable@dpdk.org; Ori Kam <orika@nvidia.com>;
> Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang
> <yuying.zhang@intel.com>; Bing Zhao <bingz@nvidia.com>
> Subject: Re: [PATCH] app/testpmd: fix parsing for connection tracking item
> 
> External email: Use caution opening links or attachments
> 
> 
> On 6/2/2024 11:23 AM, Maayan Kashani wrote:
> > In command line translation there were missing fields for connection
> > tracking item, as a result this item was not parsed and was missing
> > from the items list received from test-pmd.
> >
> > Fixes: 4d07cbefe3ba ("app/testpmd: add commands for conntrack")
> > Cc: stable@dpdk.org
> > Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
> >
> 
> Hi Maayan,
> 
> I can't really trace why .priv and 'parse_vc' is required, can you please help
> understanding by providing more data?
> 
> What was the old testpmd command and what was old behavior with that
> command, why it was wrong?
> What is the new behavior and why this update was needed, how new code
> solved the issue?
> 
> testpmd flow command is still somewhat cryptic to me, and without running
> code and debugging it is very hard for me to detect the missing parts.
> 
> 
> > ---
> >  app/test-pmd/cmdline_flow.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 60ee9337cf..1f9d5ebd05 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -5797,9 +5797,12 @@ static const struct token token_list[] = {
> >       [ITEM_CONNTRACK] = {
> >               .name = "conntrack",
> >               .help = "conntrack state",
> > +             .priv = PRIV_ITEM(CONNTRACK,
> > +                               sizeof(struct
> > + rte_flow_item_conntrack)),
> >               .next = NEXT(NEXT_ENTRY(ITEM_NEXT),
> NEXT_ENTRY(COMMON_UNSIGNED),
> >                            item_param),
> >               .args = ARGS(ARGS_ENTRY(struct rte_flow_item_conntrack,
> > flags)),
> > +             .call = parse_vc,
> >       },
> >       [ITEM_PORT_REPRESENTOR] = {
> >               .name = "port_representor",