app/testpmd: fix flex item flush

PATCH Applied Gregory Etelson 3 participants 6 messages Oct 26, 2021 – Nov 5, 2021

Share & tools

Gregory Etelson

author

2021-10-26 15:20 UTC #0 lore raw

PATCH 1/1 app/testpmd: fix flex item flush

Testpmd provides 2 sets of flex item create and destroy functions One for hosts with JSON library. These functions parse flex item configuration stored in JSON file and create or destroy flex item object. The second functions set is for hosts without JSON library for compilation compatibility.

On hosts without JSON library, current implementation issues "no JSON library" notification on port close.
The notification was triggered by port destructors that include flex items flush routine.

The patch introduces single implementation for testpmd flex item destroy.

Fixes 59f3a8acbcdb ("app/testpmd: add flex item commands")

Signed-off-by Gregory Etelson <[email protected]>

---
 app/test-pmd/cmd_flex_item.c | 72 ++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 37 deletions(-)

app/test-pmd/cmd_flex_item.c +35 ~~-37~~

index 45103e45a8..908bcb3f47 100644
@@ -20,13 +20,6 @@
 struct flex_item *flex_items[RTE_MAX_ETHPORTS][FLEX_MAX_PARSERS_NUM];
 struct flex_pattern flex_patterns[FLEX_MAX_PATTERNS_NUM];
 
-#ifdef RTE_HAS_JANSSON
-static __rte_always_inline bool
-match_strkey(const char *key, const char *pattern)
-{
-	return strncmp(key, pattern, strlen(key)) == 0;
-}
-
 static struct flex_item *
 flex_parser_fetch(uint16_t port_id, uint16_t flex_id)
 {

@@ -41,30 +34,11 @@ flex_parser_fetch(uint16_t port_id, uint16_t flex_id)
 	return flex_items[port_id][flex_id];
 }
 
-void
-flex_item_destroy(portid_t port_id, uint16_t flex_id)
+#ifdef RTE_HAS_JANSSON
+static __rte_always_inline bool
+match_strkey(const char *key, const char *pattern)
 {
-	int ret;
-	struct rte_flow_error error;
-	struct flex_item *fp = flex_parser_fetch(port_id, flex_id);
-	if (fp == FLEX_PARSER_ERR) {
-		printf("Bad parameters: port_id=%u flex_id=%u\n",
-		       port_id, flex_id);
-		return;
-	}
-	if (!fp)
-		return;
-	ret = rte_flow_flex_item_release(port_id, fp->flex_handle, &error);
-	if (!ret) {
-		free(fp);
-		flex_items[port_id][flex_id] = NULL;
-		printf("port-%u: released flex item #%u\n",
-		       port_id, flex_id);
-
-	} else {
-		printf("port-%u: cannot release flex item #%u: %s\n",
-		       port_id, flex_id, error.message);
-	}
+	return strncmp(key, pattern, strlen(key)) == 0;
 }
 
 static int

@@ -399,15 +373,37 @@ void flex_item_create(__rte_unused portid_t port_id,
 		      __rte_unused uint16_t flex_id,
 		      __rte_unused const char *filename)
 {
-	printf("no JSON library\n");
+	printf("cannot create flex item - no JSON library configured\n");
 }
+#endif /* RTE_HAS_JANSSON */
 
-void flex_item_destroy(__rte_unused portid_t port_id,
-		       __rte_unused uint16_t flex_id)
+void
+flex_item_destroy(portid_t port_id, uint16_t flex_id)
 {
-	printf("no JSON library\n");
+	int ret;
+	struct rte_flow_error error;
+	struct flex_item *fp = flex_parser_fetch(port_id, flex_id);
+	if (!flex_id)
+		return;
+	if (fp == FLEX_PARSER_ERR) {
+		printf("Bad parameters: port_id=%u flex_id=%u\n",
+		       port_id, flex_id);
+		return;
+	}
+	if (!fp)
+		return;
+	ret = rte_flow_flex_item_release(port_id, fp->flex_handle, &error);
+	if (!ret) {
+		free(fp);
+		flex_items[port_id][flex_id] = NULL;
+		printf("port-%u: released flex item #%u\n",
+		       port_id, flex_id);
+
+	} else {
+		printf("port-%u: cannot release flex item #%u: %s\n",
+		       port_id, flex_id, error.message);
+	}
 }
-#endif /* RTE_HAS_JANSSON */
 
 void
 port_flex_item_flush(portid_t port_id)

@@ -415,8 +411,10 @@ port_flex_item_flush(portid_t port_id)
 	uint16_t i;
 
 	for (i = 0; i < FLEX_MAX_PARSERS_NUM; i++) {
-		flex_item_destroy(port_id, i);
-		flex_items[port_id][i] = NULL;
+		if (flex_items[port_id][i] != NULL) {
+			flex_item_destroy(port_id, i);
+			flex_items[port_id][i] = NULL;
+		}
 	}
 }

Signature · 1 line

2.33.1

Ferruh Yigit

2021-11-03 19:13 UTC #1 lore raw

Quoted 11 lines · Gregory Etelson

Testpmd provides 2 sets of flex item create and destroy functions One for hosts with JSON library. These functions parse flex item configuration stored in JSON file and create or destroy flex item object. The second functions set is for hosts without JSON library for compilation compatibility.
On hosts without JSON library, current implementation issues "no JSON library" notification on port close.
The notification was triggered by port destructors that include flex items flush routine.

Quoted 7 lines

The patch introduces single implementation for testpmd flex item destroy.
Fixes 59f3a8acbcdb ("app/testpmd: add flex item commands")
Signed-off-by Gregory Etelson<[email protected]>
---
  app/test-pmd/cmd_flex_item.c | 72 ++++++++++++++++++------------------

Hi Gregory, Ori,

I just recognized that 'cmd_flex_item.c' doesn't have an explicit maintainer, which makes it fallback to Xialyoun, but I don't think that is related to existing coverage.

Since the file is related to rte_flow and only user of it (also author of it) is nvidia, can one of you take the maintainership of the file?

It is easier to add it to 'Flow API' block in MAINTAINERS file, that Ori already is the maintainer, if that suits.

And perhaps, if you guys are OK, Gregory can be added to 'Flow API' too, it is good to have multiple maintainers.

Thanks,
ferruh

Ferruh Yigit

2021-11-05 15:57 UTC #2 lore raw

In reply to Gregory Etelson

Quoted 11 lines · Gregory Etelson

Testpmd provides 2 sets of flex item create and destroy functions One for hosts with JSON library. These functions parse flex item configuration stored in JSON file and create or destroy flex item object. The second functions set is for hosts without JSON library for compilation compatibility.
On hosts without JSON library, current implementation issues "no JSON library" notification on port close.
The notification was triggered by port destructors that include flex items flush routine.

Quoted 5 lines

The patch introduces single implementation for testpmd flex item destroy.
Fixes 59f3a8acbcdb ("app/testpmd: add flex item commands")
Signed-off-by Gregory Etelson<[email protected]>

Hi Viacheslav, Ori,

Can you please help on reviewing this patch?

Slava Ovsiienko

Reviewed-by

2021-11-05 17:03 UTC #3 lore raw

Quoted 24 lines

-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Friday, November 5, 2021 17:57
To: Gregory Etelson <[email protected]>; Slava Ovsiienko <[email protected]>; Ori Kam <[email protected]>
Cc: Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Xiaoyun Li <[email protected]>; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix flex item flush
Quoted 14 lines · Gregory Etelson
Testpmd provides 2 sets of flex item create and destroy functions One for hosts with JSON library. These functions parse flex item configuration stored in JSON file and create or destroy flex item object. The second functions set is for hosts without JSON library for compilation compatibility.
On hosts without JSON library, current implementation issues "no JSON library" notification on port close.
The notification was triggered by port destructors that include flex items flush routine.
The patch introduces single implementation for testpmd flex item destroy.

Quoted 6 lines

Quoted 2 lines
Fixes 59f3a8acbcdb ("app/testpmd: add flex item commands")
Signed-off-by Gregory Etelson<[email protected]>
Hi Viacheslav, Ori,
Can you please help on reviewing this patch?

Reviewed-by Viacheslav Ovsiienko <[email protected]>

I've already reviewed this one internally, so just posting the confirmation. As for maintainership of cmd_flex_item.c - we talked with Ori, he don't mind if I would take the responsibility for this. What should we do for that? Send patch to update MAINTAINERS ?

With best regards,
Slava

Ferruh Yigit

2021-11-05 18:57 UTC #4 lore raw

Quoted 29 lines · Slava Ovsiienko

Quoted 29 lines
-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Friday, November 5, 2021 17:57
To: Gregory Etelson <[email protected]>; Slava Ovsiienko <[email protected]>; Ori Kam <[email protected]>
Cc: Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Xiaoyun Li <[email protected]>; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix flex item flush
Quoted 11 lines · Gregory Etelson
Testpmd provides 2 sets of flex item create and destroy functions One for hosts with JSON library. These functions parse flex item configuration stored in JSON file and create or destroy flex item object. The second functions set is for hosts without JSON library for compilation compatibility.
On hosts without JSON library, current implementation issues "no JSON library" notification on port close.
The notification was triggered by port destructors that include flex items flush routine.
Quoted 5 lines
The patch introduces single implementation for testpmd flex item destroy.
Fixes 59f3a8acbcdb ("app/testpmd: add flex item commands")
Signed-off-by Gregory Etelson<[email protected]>
Hi Viacheslav, Ori,

Quoted 8 lines

Quoted 1 line
Can you please help on reviewing this patch?
Reviewed-by Viacheslav Ovsiienko <[email protected]>
I've already reviewed this one internally, so just posting the confirmation. As for maintainership of cmd_flex_item.c - we talked with Ori, he don't mind if I would take the responsibility for this. What should we do for that? Send patch to update MAINTAINERS ?

Great, thanks.
Yes a maintainers file update is required.

Ferruh Yigit

Applied

2021-11-05 21:21 UTC #5 lore raw

In reply to Slava Ovsiienko

Quoted 24 lines · Slava Ovsiienko

Quoted 24 lines
-----Original Message-----
From: Ferruh Yigit <[email protected]>
Sent: Friday, November 5, 2021 17:57
To: Gregory Etelson <[email protected]>; Slava Ovsiienko <[email protected]>; Ori Kam <[email protected]>
Cc: Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Xiaoyun Li <[email protected]>; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix flex item flush
Quoted 14 lines · Gregory Etelson
Testpmd provides 2 sets of flex item create and destroy functions One for hosts with JSON library. These functions parse flex item configuration stored in JSON file and create or destroy flex item object. The second functions set is for hosts without JSON library for compilation compatibility.
On hosts without JSON library, current implementation issues "no JSON library" notification on port close.
The notification was triggered by port destructors that include flex items flush routine.
The patch introduces single implementation for testpmd flex item destroy.

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
Fixes 59f3a8acbcdb ("app/testpmd: add flex item commands")
Signed-off-by Gregory Etelson<[email protected]>
Hi Viacheslav, Ori,
Can you please help on reviewing this patch?
Reviewed-by Viacheslav Ovsiienko <[email protected]>

Applied to dpdk-next-net/main, thanks.