[PATCH] app/test-pipeline: relax RSS hash requirement

[PATCH] app/test-pipeline: relax RSS hash requirement

[Feifei Wang]

For some drivers which can not support the configured RSS hash functions,
the thread reports 'invalid rss_hf' when doing device configure.

For example, i40e driver can not support 'RTE_ETH_RSS_IPV4',
'RTE_ETH_RSS_IPV6' and 'RTE_ETH_RSS_NONFRAG_IPV6_OTHER', thus it can not
run successfully in test-pipeline with XL710 NIC and reports the issue:
-------------------------------------------------------------
Ethdev port_id=0 invalid rss_hf: 0xa38c, valid value: 0x7ef8
PANIC in app_init_ports():
Cannot init NIC port 0 (-22)
-------------------------------------------------------------

To fix this, referring to l3fwd operation, adjust the 'rss_hf' based on
device capability and just report a warning.

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Trevor Tao <trevor.tao@arm.com>
---
 app/test-pipeline/init.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
index d146c44be0..84a1734519 100644
--- a/app/test-pipeline/init.c
+++ b/app/test-pipeline/init.c
@@ -188,21 +188,41 @@ static void
 app_init_ports(void)
 {
 	uint32_t i;
+	struct rte_eth_dev_info dev_info;
+
 
 	/* Init NIC ports, then start the ports */
 	for (i = 0; i < app.n_ports; i++) {
 		uint16_t port;
 		int ret;
+		struct rte_eth_conf local_port_conf = port_conf;
 
 		port = app.ports[i];
 		RTE_LOG(INFO, USER1, "Initializing NIC port %u ...\n", port);
 
+		ret = rte_eth_dev_info_get(port, &dev_info);
+		if (ret != 0)
+			rte_panic("Error during getting device (port %u) info: %s\n",
+					port, rte_strerror(-ret));
+
 		/* Init port */
+		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
+			dev_info.flow_type_rss_offloads;
+		if (local_port_conf.rx_adv_conf.rss_conf.rss_hf !=
+				port_conf.rx_adv_conf.rss_conf.rss_hf) {
+			printf("Warning:"
+				"Port %u modified RSS hash function based on hardware support,"
+				"requested:%#"PRIx64" configured:%#"PRIx64"\n",
+				port,
+				port_conf.rx_adv_conf.rss_conf.rss_hf,
+				local_port_conf.rx_adv_conf.rss_conf.rss_hf);
+		}
+
 		ret = rte_eth_dev_configure(
 			port,
 			1,
 			1,
-			&port_conf);
+			&local_port_conf);
 		if (ret < 0)
 			rte_panic("Cannot init NIC port %u (%d)\n", port, ret);
 
-- 
2.25.1

Re: [PATCH] app/test-pipeline: relax RSS hash requirement

[Thomas Monjalon]

Cristian, any comment?

26/06/2023 09:45, Feifei Wang:
> For some drivers which can not support the configured RSS hash functions,
> the thread reports 'invalid rss_hf' when doing device configure.
> 
> For example, i40e driver can not support 'RTE_ETH_RSS_IPV4',
> 'RTE_ETH_RSS_IPV6' and 'RTE_ETH_RSS_NONFRAG_IPV6_OTHER', thus it can not
> run successfully in test-pipeline with XL710 NIC and reports the issue:
> -------------------------------------------------------------
> Ethdev port_id=0 invalid rss_hf: 0xa38c, valid value: 0x7ef8
> PANIC in app_init_ports():
> Cannot init NIC port 0 (-22)
> -------------------------------------------------------------
> 
> To fix this, referring to l3fwd operation, adjust the 'rss_hf' based on
> device capability and just report a warning.
> 
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Trevor Tao <trevor.tao@arm.com>
> ---
>  app/test-pipeline/init.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
> index d146c44be0..84a1734519 100644
> --- a/app/test-pipeline/init.c
> +++ b/app/test-pipeline/init.c
> @@ -188,21 +188,41 @@ static void
>  app_init_ports(void)
>  {
>  	uint32_t i;
> +	struct rte_eth_dev_info dev_info;
> +
>  
>  	/* Init NIC ports, then start the ports */
>  	for (i = 0; i < app.n_ports; i++) {
>  		uint16_t port;
>  		int ret;
> +		struct rte_eth_conf local_port_conf = port_conf;
>  
>  		port = app.ports[i];
>  		RTE_LOG(INFO, USER1, "Initializing NIC port %u ...\n", port);
>  
> +		ret = rte_eth_dev_info_get(port, &dev_info);
> +		if (ret != 0)
> +			rte_panic("Error during getting device (port %u) info: %s\n",
> +					port, rte_strerror(-ret));
> +
>  		/* Init port */
> +		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
> +			dev_info.flow_type_rss_offloads;
> +		if (local_port_conf.rx_adv_conf.rss_conf.rss_hf !=
> +				port_conf.rx_adv_conf.rss_conf.rss_hf) {
> +			printf("Warning:"
> +				"Port %u modified RSS hash function based on hardware support,"
> +				"requested:%#"PRIx64" configured:%#"PRIx64"\n",
> +				port,
> +				port_conf.rx_adv_conf.rss_conf.rss_hf,
> +				local_port_conf.rx_adv_conf.rss_conf.rss_hf);
> +		}
> +
>  		ret = rte_eth_dev_configure(
>  			port,
>  			1,
>  			1,
> -			&port_conf);
> +			&local_port_conf);
>  		if (ret < 0)
>  			rte_panic("Cannot init NIC port %u (%d)\n", port, ret);
>  
> 

Re: [PATCH] app/test-pipeline: relax RSS hash requirement

[lihuisong (C)]

在 2023/6/26 15:45, Feifei Wang 写道:
> For some drivers which can not support the configured RSS hash functions,
> the thread reports 'invalid rss_hf' when doing device configure.
>
> For example, i40e driver can not support 'RTE_ETH_RSS_IPV4',
> 'RTE_ETH_RSS_IPV6' and 'RTE_ETH_RSS_NONFRAG_IPV6_OTHER', thus it can not
> run successfully in test-pipeline with XL710 NIC and reports the issue:
> -------------------------------------------------------------
> Ethdev port_id=0 invalid rss_hf: 0xa38c, valid value: 0x7ef8
> PANIC in app_init_ports():
> Cannot init NIC port 0 (-22)
> -------------------------------------------------------------
>
> To fix this, referring to l3fwd operation, adjust the 'rss_hf' based on
> device capability and just report a warning.
>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Trevor Tao <trevor.tao@arm.com>
> ---
>   app/test-pipeline/init.c | 22 +++++++++++++++++++++-
>   1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
> index d146c44be0..84a1734519 100644
> --- a/app/test-pipeline/init.c
> +++ b/app/test-pipeline/init.c
> @@ -188,21 +188,41 @@ static void
>   app_init_ports(void)
>   {
>   	uint32_t i;
> +	struct rte_eth_dev_info dev_info;
> +
please delete this blank line.
>   
>   	/* Init NIC ports, then start the ports */
>   	for (i = 0; i < app.n_ports; i++) {
>   		uint16_t port;
>   		int ret;
> +		struct rte_eth_conf local_port_conf = port_conf;
>   
>   		port = app.ports[i];
>   		RTE_LOG(INFO, USER1, "Initializing NIC port %u ...\n", port);
>   
> +		ret = rte_eth_dev_info_get(port, &dev_info);
> +		if (ret != 0)
> +			rte_panic("Error during getting device (port %u) info: %s\n",
> +					port, rte_strerror(-ret));
> +
>   		/* Init port */
> +		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
> +			dev_info.flow_type_rss_offloads;
> +		if (local_port_conf.rx_adv_conf.rss_conf.rss_hf !=
> +				port_conf.rx_adv_conf.rss_conf.rss_hf) {
> +			printf("Warning:"
> +				"Port %u modified RSS hash function based on hardware support,"
> +				"requested:%#"PRIx64" configured:%#"PRIx64"\n",
> +				port,
> +				port_conf.rx_adv_conf.rss_conf.rss_hf,
> +				local_port_conf.rx_adv_conf.rss_conf.rss_hf);
> +		}
> +
>   		ret = rte_eth_dev_configure(
>   			port,
>   			1,
>   			1,
> -			&port_conf);
> +			&local_port_conf);
>   		if (ret < 0)
>   			rte_panic("Cannot init NIC port %u (%d)\n", port, ret);
>   
This treatment is very common.
Acked-by: Huisong Li <lihuisong@huawei.com>

RE: [PATCH] app/test-pipeline: relax RSS hash requirement

[Feifei Wang]

> -----Original Message-----
> From: lihuisong (C) <lihuisong@huawei.com>
> Sent: Friday, July 14, 2023 3:50 PM
> To: Feifei Wang <Feifei.Wang2@arm.com>; Cristian Dumitrescu
> <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Trevor Tao <Trevor.Tao@arm.com>
> Subject: Re: [PATCH] app/test-pipeline: relax RSS hash requirement
> 
> 
> 在 2023/6/26 15:45, Feifei Wang 写道:
> > For some drivers which can not support the configured RSS hash
> > functions, the thread reports 'invalid rss_hf' when doing device configure.
> >
> > For example, i40e driver can not support 'RTE_ETH_RSS_IPV4',
> > 'RTE_ETH_RSS_IPV6' and 'RTE_ETH_RSS_NONFRAG_IPV6_OTHER', thus it
> can
> > not run successfully in test-pipeline with XL710 NIC and reports the issue:
> > -------------------------------------------------------------
> > Ethdev port_id=0 invalid rss_hf: 0xa38c, valid value: 0x7ef8 PANIC in
> > app_init_ports():
> > Cannot init NIC port 0 (-22)
> > -------------------------------------------------------------
> >
> > To fix this, referring to l3fwd operation, adjust the 'rss_hf' based
> > on device capability and just report a warning.
> >
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Trevor Tao <trevor.tao@arm.com>
> > ---
> >   app/test-pipeline/init.c | 22 +++++++++++++++++++++-
> >   1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c index
> > d146c44be0..84a1734519 100644
> > --- a/app/test-pipeline/init.c
> > +++ b/app/test-pipeline/init.c
> > @@ -188,21 +188,41 @@ static void
> >   app_init_ports(void)
> >   {
> >   	uint32_t i;
> > +	struct rte_eth_dev_info dev_info;
> > +
> please delete this blank line.

Thanks for the reviewing. I will change this and update a new version.

> >
> >   	/* Init NIC ports, then start the ports */
> >   	for (i = 0; i < app.n_ports; i++) {
> >   		uint16_t port;
> >   		int ret;
> > +		struct rte_eth_conf local_port_conf = port_conf;
> >
> >   		port = app.ports[i];
> >   		RTE_LOG(INFO, USER1, "Initializing NIC port %u ...\n", port);
> >
> > +		ret = rte_eth_dev_info_get(port, &dev_info);
> > +		if (ret != 0)
> > +			rte_panic("Error during getting device (port %u)
> info: %s\n",
> > +					port, rte_strerror(-ret));
> > +
> >   		/* Init port */
> > +		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
> > +			dev_info.flow_type_rss_offloads;
> > +		if (local_port_conf.rx_adv_conf.rss_conf.rss_hf !=
> > +				port_conf.rx_adv_conf.rss_conf.rss_hf) {
> > +			printf("Warning:"
> > +				"Port %u modified RSS hash function based
> on hardware support,"
> > +				"requested:%#"PRIx64"
> configured:%#"PRIx64"\n",
> > +				port,
> > +				port_conf.rx_adv_conf.rss_conf.rss_hf,
> > +				local_port_conf.rx_adv_conf.rss_conf.rss_hf);
> > +		}
> > +
> >   		ret = rte_eth_dev_configure(
> >   			port,
> >   			1,
> >   			1,
> > -			&port_conf);
> > +			&local_port_conf);
> >   		if (ret < 0)
> >   			rte_panic("Cannot init NIC port %u (%d)\n", port,
> ret);
> >
> This treatment is very common.
> Acked-by: Huisong Li <lihuisong@huawei.com>

[PATCH v2] app/test-pipeline: relax RSS hash requirement

[Feifei Wang]

For some drivers which can not support the configured RSS hash functions,
the thread reports 'invalid rss_hf' when doing device configure.

For example, i40e driver can not support 'RTE_ETH_RSS_IPV4',
'RTE_ETH_RSS_IPV6' and 'RTE_ETH_RSS_NONFRAG_IPV6_OTHER', thus it can not
run successfully in test-pipeline with XL710 NIC and reports the issue:
-------------------------------------------------------------
Ethdev port_id=0 invalid rss_hf: 0xa38c, valid value: 0x7ef8
PANIC in app_init_ports():
Cannot init NIC port 0 (-22)
-------------------------------------------------------------

To fix this, referring to l3fwd operation, adjust the 'rss_hf' based on
device capability and just report a warning.

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Trevor Tao <trevor.tao@arm.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pipeline/init.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
index d146c44be0..558f0e428d 100644
--- a/app/test-pipeline/init.c
+++ b/app/test-pipeline/init.c
@@ -188,21 +188,40 @@ static void
 app_init_ports(void)
 {
 	uint32_t i;
+	struct rte_eth_dev_info dev_info;
 
 	/* Init NIC ports, then start the ports */
 	for (i = 0; i < app.n_ports; i++) {
 		uint16_t port;
 		int ret;
+		struct rte_eth_conf local_port_conf = port_conf;
 
 		port = app.ports[i];
 		RTE_LOG(INFO, USER1, "Initializing NIC port %u ...\n", port);
 
+		ret = rte_eth_dev_info_get(port, &dev_info);
+		if (ret != 0)
+			rte_panic("Error during getting device (port %u) info: %s\n",
+					port, rte_strerror(-ret));
+
 		/* Init port */
+		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
+			dev_info.flow_type_rss_offloads;
+		if (local_port_conf.rx_adv_conf.rss_conf.rss_hf !=
+				port_conf.rx_adv_conf.rss_conf.rss_hf) {
+			printf("Warning:"
+				"Port %u modified RSS hash function based on hardware support,"
+				"requested:%#"PRIx64" configured:%#"PRIx64"\n",
+				port,
+				port_conf.rx_adv_conf.rss_conf.rss_hf,
+				local_port_conf.rx_adv_conf.rss_conf.rss_hf);
+		}
+
 		ret = rte_eth_dev_configure(
 			port,
 			1,
 			1,
-			&port_conf);
+			&local_port_conf);
 		if (ret < 0)
 			rte_panic("Cannot init NIC port %u (%d)\n", port, ret);
 
-- 
2.25.1

[PATCH v3 0/3] fix test-pipeline issues

[Feifei Wang]

For test-pipeline application, there are some problems with the normal
operation of the program and security issues. These patches can fix
these issues.

v3: fix SIGINT handling issue and add dev close operation

Feifei Wang (3):
  app/test-pipeline: relax RSS hash requirement
  app/test-pipeline: fix SIGINT handling issue
  app/test-pipeline: add dev close operation

 app/test-pipeline/init.c              |  22 ++++-
 app/test-pipeline/main.c              |  33 +++++++
 app/test-pipeline/main.h              |   2 +
 app/test-pipeline/pipeline_acl.c      |   6 +-
 app/test-pipeline/pipeline_hash.c     | 110 ++++++++++-----------
 app/test-pipeline/pipeline_lpm.c      |   6 +-
 app/test-pipeline/pipeline_lpm_ipv6.c |   6 +-
 app/test-pipeline/pipeline_stub.c     |   6 +-
 app/test-pipeline/runtime.c           | 132 ++++++++++++++------------
 9 files changed, 198 insertions(+), 125 deletions(-)

-- 
2.25.1

[PATCH v3 1/3] app/test-pipeline: relax RSS hash requirement

[Feifei Wang]

For some drivers which can not support the configured RSS hash functions,
the thread reports 'invalid rss_hf' when doing device configure.

For example, i40e driver can not support 'RTE_ETH_RSS_IPV4',
'RTE_ETH_RSS_IPV6' and 'RTE_ETH_RSS_NONFRAG_IPV6_OTHER', thus it can not
run successfully in test-pipeline with XL710 NIC and reports the issue:
-------------------------------------------------------------
Ethdev port_id=0 invalid rss_hf: 0xa38c, valid value: 0x7ef8
PANIC in app_init_ports():
Cannot init NIC port 0 (-22)
-------------------------------------------------------------

To fix this, referring to l3fwd operation, adjust the 'rss_hf' based on
device capability and just report a warning.

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Trevor Tao <trevor.tao@arm.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pipeline/init.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c
index d146c44be0..84a1734519 100644
--- a/app/test-pipeline/init.c
+++ b/app/test-pipeline/init.c
@@ -188,21 +188,41 @@ static void
 app_init_ports(void)
 {
 	uint32_t i;
+	struct rte_eth_dev_info dev_info;
+
 
 	/* Init NIC ports, then start the ports */
 	for (i = 0; i < app.n_ports; i++) {
 		uint16_t port;
 		int ret;
+		struct rte_eth_conf local_port_conf = port_conf;
 
 		port = app.ports[i];
 		RTE_LOG(INFO, USER1, "Initializing NIC port %u ...\n", port);
 
+		ret = rte_eth_dev_info_get(port, &dev_info);
+		if (ret != 0)
+			rte_panic("Error during getting device (port %u) info: %s\n",
+					port, rte_strerror(-ret));
+
 		/* Init port */
+		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
+			dev_info.flow_type_rss_offloads;
+		if (local_port_conf.rx_adv_conf.rss_conf.rss_hf !=
+				port_conf.rx_adv_conf.rss_conf.rss_hf) {
+			printf("Warning:"
+				"Port %u modified RSS hash function based on hardware support,"
+				"requested:%#"PRIx64" configured:%#"PRIx64"\n",
+				port,
+				port_conf.rx_adv_conf.rss_conf.rss_hf,
+				local_port_conf.rx_adv_conf.rss_conf.rss_hf);
+		}
+
 		ret = rte_eth_dev_configure(
 			port,
 			1,
 			1,
-			&port_conf);
+			&local_port_conf);
 		if (ret < 0)
 			rte_panic("Cannot init NIC port %u (%d)\n", port, ret);
 
-- 
2.25.1

[PATCH v3 2/3] app/test-pipeline: fix SIGINT handling issue

[Feifei Wang]

For test-pipeline, if the main core receive SIGINT signal, it will kill
all the threads immediately and not wait other threads to finish their
jobs.

To fix this, add 'signal_handler' function.

Fixes: 48f31ca50cc4 ("app/pipeline: packet framework benchmark")
Cc: cristian.dumitrescu@intel.com
Cc: stable@dpdk.org

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Matthew Dirba <Matthew.Dirba@arm.com>
---
 app/test-pipeline/main.c              |  14 +++
 app/test-pipeline/main.h              |   2 +
 app/test-pipeline/pipeline_acl.c      |   6 +-
 app/test-pipeline/pipeline_hash.c     | 110 ++++++++++-----------
 app/test-pipeline/pipeline_lpm.c      |   6 +-
 app/test-pipeline/pipeline_lpm_ipv6.c |   6 +-
 app/test-pipeline/pipeline_stub.c     |   6 +-
 app/test-pipeline/runtime.c           | 132 ++++++++++++++------------
 8 files changed, 158 insertions(+), 124 deletions(-)

diff --git a/app/test-pipeline/main.c b/app/test-pipeline/main.c
index 1e16794183..8633933fd9 100644
--- a/app/test-pipeline/main.c
+++ b/app/test-pipeline/main.c
@@ -10,6 +10,7 @@
 #include <string.h>
 #include <sys/queue.h>
 #include <stdarg.h>
+#include <signal.h>
 #include <errno.h>
 #include <getopt.h>
 #include <unistd.h>
@@ -41,6 +42,15 @@
 
 #include "main.h"
 
+bool force_quit;
+
+static void
+signal_handler(int signum)
+{
+	if (signum == SIGINT || signum == SIGTERM)
+		force_quit = true;
+}
+
 int
 main(int argc, char **argv)
 {
@@ -54,6 +64,10 @@ main(int argc, char **argv)
 	argc -= ret;
 	argv += ret;
 
+	force_quit = false;
+	signal(SIGINT, signal_handler);
+	signal(SIGTERM, signal_handler);
+
 	/* Parse application arguments (after the EAL ones) */
 	ret = app_parse_args(argc, argv);
 	if (ret < 0) {
diff --git a/app/test-pipeline/main.h b/app/test-pipeline/main.h
index 59dcfddbf4..9df157de22 100644
--- a/app/test-pipeline/main.h
+++ b/app/test-pipeline/main.h
@@ -60,6 +60,8 @@ struct app_params {
 
 extern struct app_params app;
 
+extern bool force_quit;
+
 int app_parse_args(int argc, char **argv);
 void app_print_usage(void);
 void app_init(void);
diff --git a/app/test-pipeline/pipeline_acl.c b/app/test-pipeline/pipeline_acl.c
index 2f04868e3e..9eb4053e23 100644
--- a/app/test-pipeline/pipeline_acl.c
+++ b/app/test-pipeline/pipeline_acl.c
@@ -236,14 +236,16 @@ app_main_loop_worker_pipeline_acl(void) {
 
 	/* Run-time */
 #if APP_FLUSH == 0
-	for ( ; ; )
+	while (!force_quit)
 		rte_pipeline_run(p);
 #else
-	for (i = 0; ; i++) {
+	i = 0;
+	while (!force_quit) {
 		rte_pipeline_run(p);
 
 		if ((i & APP_FLUSH) == 0)
 			rte_pipeline_flush(p);
+		i++;
 	}
 #endif
 }
diff --git a/app/test-pipeline/pipeline_hash.c b/app/test-pipeline/pipeline_hash.c
index 2dd8928d43..cab9c20980 100644
--- a/app/test-pipeline/pipeline_hash.c
+++ b/app/test-pipeline/pipeline_hash.c
@@ -366,14 +366,16 @@ app_main_loop_worker_pipeline_hash(void) {
 
 	/* Run-time */
 #if APP_FLUSH == 0
-	for ( ; ; )
+	while (!force_quit)
 		rte_pipeline_run(p);
 #else
-	for (i = 0; ; i++) {
+	i = 0;
+	while (!force_quit) {
 		rte_pipeline_run(p);
 
 		if ((i & APP_FLUSH) == 0)
 			rte_pipeline_flush(p);
+		i++;
 	}
 #endif
 }
@@ -411,59 +413,61 @@ app_main_loop_rx_metadata(void) {
 	RTE_LOG(INFO, USER1, "Core %u is doing RX (with meta-data)\n",
 		rte_lcore_id());
 
-	for (i = 0; ; i = ((i + 1) & (app.n_ports - 1))) {
-		uint16_t n_mbufs;
-
-		n_mbufs = rte_eth_rx_burst(
-			app.ports[i],
-			0,
-			app.mbuf_rx.array,
-			app.burst_size_rx_read);
-
-		if (n_mbufs == 0)
-			continue;
-
-		for (j = 0; j < n_mbufs; j++) {
-			struct rte_mbuf *m;
-			uint8_t *m_data, *key;
-			struct rte_ipv4_hdr *ip_hdr;
-			struct rte_ipv6_hdr *ipv6_hdr;
-			uint32_t ip_dst;
-			uint8_t *ipv6_dst;
-			uint32_t *signature, *k32;
-
-			m = app.mbuf_rx.array[j];
-			m_data = rte_pktmbuf_mtod(m, uint8_t *);
-			signature = RTE_MBUF_METADATA_UINT32_PTR(m,
-					APP_METADATA_OFFSET(0));
-			key = RTE_MBUF_METADATA_UINT8_PTR(m,
-					APP_METADATA_OFFSET(32));
-
-			if (RTE_ETH_IS_IPV4_HDR(m->packet_type)) {
-				ip_hdr = (struct rte_ipv4_hdr *)
-					&m_data[sizeof(struct rte_ether_hdr)];
-				ip_dst = ip_hdr->dst_addr;
-
-				k32 = (uint32_t *) key;
-				k32[0] = ip_dst & 0xFFFFFF00;
-			} else if (RTE_ETH_IS_IPV6_HDR(m->packet_type)) {
-				ipv6_hdr = (struct rte_ipv6_hdr *)
-					&m_data[sizeof(struct rte_ether_hdr)];
-				ipv6_dst = ipv6_hdr->dst_addr;
-
-				memcpy(key, ipv6_dst, 16);
-			} else
+	while (!force_quit) {
+		for (i = 0; i < app.n_ports; i++) {
+			uint16_t n_mbufs;
+
+			n_mbufs = rte_eth_rx_burst(
+				app.ports[i],
+				0,
+				app.mbuf_rx.array,
+				app.burst_size_rx_read);
+
+			if (n_mbufs == 0)
 				continue;
 
-			*signature = test_hash(key, NULL, 0, 0);
+			for (j = 0; j < n_mbufs; j++) {
+				struct rte_mbuf *m;
+				uint8_t *m_data, *key;
+				struct rte_ipv4_hdr *ip_hdr;
+				struct rte_ipv6_hdr *ipv6_hdr;
+				uint32_t ip_dst;
+				uint8_t *ipv6_dst;
+				uint32_t *signature, *k32;
+
+				m = app.mbuf_rx.array[j];
+				m_data = rte_pktmbuf_mtod(m, uint8_t *);
+				signature = RTE_MBUF_METADATA_UINT32_PTR(m,
+						APP_METADATA_OFFSET(0));
+				key = RTE_MBUF_METADATA_UINT8_PTR(m,
+						APP_METADATA_OFFSET(32));
+
+				if (RTE_ETH_IS_IPV4_HDR(m->packet_type)) {
+					ip_hdr = (struct rte_ipv4_hdr *)
+						&m_data[sizeof(struct rte_ether_hdr)];
+					ip_dst = ip_hdr->dst_addr;
+
+					k32 = (uint32_t *) key;
+					k32[0] = ip_dst & 0xFFFFFF00;
+				} else if (RTE_ETH_IS_IPV6_HDR(m->packet_type)) {
+					ipv6_hdr = (struct rte_ipv6_hdr *)
+						&m_data[sizeof(struct rte_ether_hdr)];
+					ipv6_dst = ipv6_hdr->dst_addr;
+
+					memcpy(key, ipv6_dst, 16);
+				} else
+					continue;
+
+				*signature = test_hash(key, NULL, 0, 0);
+			}
+
+			do {
+				ret = rte_ring_sp_enqueue_bulk(
+					app.rings_rx[i],
+					(void **) app.mbuf_rx.array,
+					n_mbufs,
+					NULL);
+			} while (ret == 0 && !force_quit);
 		}
-
-		do {
-			ret = rte_ring_sp_enqueue_bulk(
-				app.rings_rx[i],
-				(void **) app.mbuf_rx.array,
-				n_mbufs,
-				NULL);
-		} while (ret == 0);
 	}
 }
diff --git a/app/test-pipeline/pipeline_lpm.c b/app/test-pipeline/pipeline_lpm.c
index 854319174b..8a59bd0042 100644
--- a/app/test-pipeline/pipeline_lpm.c
+++ b/app/test-pipeline/pipeline_lpm.c
@@ -160,14 +160,16 @@ app_main_loop_worker_pipeline_lpm(void) {
 
 	/* Run-time */
 #if APP_FLUSH == 0
-	for ( ; ; )
+	while (!force_quit)
 		rte_pipeline_run(p);
 #else
-	for (i = 0; ; i++) {
+	i = 0;
+	while (!force_quit) {
 		rte_pipeline_run(p);
 
 		if ((i & APP_FLUSH) == 0)
 			rte_pipeline_flush(p);
+		i++;
 	}
 #endif
 }
diff --git a/app/test-pipeline/pipeline_lpm_ipv6.c b/app/test-pipeline/pipeline_lpm_ipv6.c
index 18d4f018f1..207ffbeff0 100644
--- a/app/test-pipeline/pipeline_lpm_ipv6.c
+++ b/app/test-pipeline/pipeline_lpm_ipv6.c
@@ -158,14 +158,16 @@ app_main_loop_worker_pipeline_lpm_ipv6(void) {
 
 	/* Run-time */
 #if APP_FLUSH == 0
-	for ( ; ; )
+	while (!force_quit)
 		rte_pipeline_run(p);
 #else
-	for (i = 0; ; i++) {
+	i = 0;
+	while (!force_quit) {
 		rte_pipeline_run(p);
 
 		if ((i & APP_FLUSH) == 0)
 			rte_pipeline_flush(p);
+		i++;
 	}
 #endif
 }
diff --git a/app/test-pipeline/pipeline_stub.c b/app/test-pipeline/pipeline_stub.c
index b6750d51bf..48a638aad7 100644
--- a/app/test-pipeline/pipeline_stub.c
+++ b/app/test-pipeline/pipeline_stub.c
@@ -122,14 +122,16 @@ app_main_loop_worker_pipeline_stub(void) {
 
 	/* Run-time */
 #if APP_FLUSH == 0
-	for ( ; ; )
+	while (!force_quit)
 		rte_pipeline_run(p);
 #else
-	for (i = 0; ; i++) {
+	i = 0;
+	while (!force_quit) {
 		rte_pipeline_run(p);
 
 		if ((i & APP_FLUSH) == 0)
 			rte_pipeline_flush(p);
+		i++;
 	}
 #endif
 }
diff --git a/app/test-pipeline/runtime.c b/app/test-pipeline/runtime.c
index d939a85d7e..752f783370 100644
--- a/app/test-pipeline/runtime.c
+++ b/app/test-pipeline/runtime.c
@@ -48,24 +48,26 @@ app_main_loop_rx(void) {
 
 	RTE_LOG(INFO, USER1, "Core %u is doing RX\n", rte_lcore_id());
 
-	for (i = 0; ; i = ((i + 1) & (app.n_ports - 1))) {
-		uint16_t n_mbufs;
-
-		n_mbufs = rte_eth_rx_burst(
-			app.ports[i],
-			0,
-			app.mbuf_rx.array,
-			app.burst_size_rx_read);
-
-		if (n_mbufs == 0)
-			continue;
-
-		do {
-			ret = rte_ring_sp_enqueue_bulk(
-				app.rings_rx[i],
-				(void **) app.mbuf_rx.array,
-				n_mbufs, NULL);
-		} while (ret == 0);
+	while (!force_quit) {
+		for (i = 0; i < app.n_ports; i++) {
+			uint16_t n_mbufs;
+
+			n_mbufs = rte_eth_rx_burst(
+				app.ports[i],
+				0,
+				app.mbuf_rx.array,
+				app.burst_size_rx_read);
+
+			if (n_mbufs == 0)
+				continue;
+
+			do {
+				ret = rte_ring_sp_enqueue_bulk(
+					app.rings_rx[i],
+					(void **) app.mbuf_rx.array,
+					n_mbufs, NULL);
+			} while (ret == 0 && !force_quit);
+		}
 	}
 }
 
@@ -82,25 +84,27 @@ app_main_loop_worker(void) {
 	if (worker_mbuf == NULL)
 		rte_panic("Worker thread: cannot allocate buffer space\n");
 
-	for (i = 0; ; i = ((i + 1) & (app.n_ports - 1))) {
-		int ret;
-
-		ret = rte_ring_sc_dequeue_bulk(
-			app.rings_rx[i],
-			(void **) worker_mbuf->array,
-			app.burst_size_worker_read,
-			NULL);
+	while (!force_quit) {
+		for (i = 0; i < app.n_ports; i++) {
+			int ret;
 
-		if (ret == 0)
-			continue;
-
-		do {
-			ret = rte_ring_sp_enqueue_bulk(
-				app.rings_tx[i ^ 1],
+			ret = rte_ring_sc_dequeue_bulk(
+				app.rings_rx[i],
 				(void **) worker_mbuf->array,
-				app.burst_size_worker_write,
+				app.burst_size_worker_read,
 				NULL);
-		} while (ret == 0);
+
+			if (ret == 0)
+				continue;
+
+			do {
+				ret = rte_ring_sp_enqueue_bulk(
+					app.rings_tx[i ^ 1],
+					(void **) worker_mbuf->array,
+					app.burst_size_worker_write,
+					NULL);
+			} while (ret == 0 && !force_quit);
+		}
 	}
 }
 
@@ -110,45 +114,47 @@ app_main_loop_tx(void) {
 
 	RTE_LOG(INFO, USER1, "Core %u is doing TX\n", rte_lcore_id());
 
-	for (i = 0; ; i = ((i + 1) & (app.n_ports - 1))) {
-		uint16_t n_mbufs, n_pkts;
-		int ret;
+	while (!force_quit) {
+		for (i = 0; i < app.n_ports; i++) {
+			uint16_t n_mbufs, n_pkts;
+			int ret;
 
-		n_mbufs = app.mbuf_tx[i].n_mbufs;
+			n_mbufs = app.mbuf_tx[i].n_mbufs;
 
-		ret = rte_ring_sc_dequeue_bulk(
-			app.rings_tx[i],
-			(void **) &app.mbuf_tx[i].array[n_mbufs],
-			app.burst_size_tx_read,
-			NULL);
+			ret = rte_ring_sc_dequeue_bulk(
+				app.rings_tx[i],
+				(void **) &app.mbuf_tx[i].array[n_mbufs],
+				app.burst_size_tx_read,
+				NULL);
 
-		if (ret == 0)
-			continue;
+			if (ret == 0)
+				continue;
 
-		n_mbufs += app.burst_size_tx_read;
+			n_mbufs += app.burst_size_tx_read;
 
-		if (n_mbufs < app.burst_size_tx_write) {
-			app.mbuf_tx[i].n_mbufs = n_mbufs;
-			continue;
-		}
+			if (n_mbufs < app.burst_size_tx_write) {
+				app.mbuf_tx[i].n_mbufs = n_mbufs;
+				continue;
+			}
 
-		n_pkts = rte_eth_tx_burst(
-			app.ports[i],
-			0,
-			app.mbuf_tx[i].array,
-			n_mbufs);
+			n_pkts = rte_eth_tx_burst(
+				app.ports[i],
+				0,
+				app.mbuf_tx[i].array,
+				n_mbufs);
 
-		if (n_pkts < n_mbufs) {
-			uint16_t k;
+			if (n_pkts < n_mbufs) {
+				uint16_t k;
 
-			for (k = n_pkts; k < n_mbufs; k++) {
-				struct rte_mbuf *pkt_to_free;
+				for (k = n_pkts; k < n_mbufs; k++) {
+					struct rte_mbuf *pkt_to_free;
 
-				pkt_to_free = app.mbuf_tx[i].array[k];
-				rte_pktmbuf_free(pkt_to_free);
+					pkt_to_free = app.mbuf_tx[i].array[k];
+					rte_pktmbuf_free(pkt_to_free);
+				}
 			}
-		}
 
-		app.mbuf_tx[i].n_mbufs = 0;
+			app.mbuf_tx[i].n_mbufs = 0;
+		}
 	}
 }
-- 
2.25.1

[PATCH v3 3/3] app/test-pipeline: add dev close operation

[Feifei Wang]

For test-pipeline, there is dev start operation, but when thread need to
exit, there is no dev close operation. This is not safe, to fix this,
add dev close operation.

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 app/test-pipeline/main.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/app/test-pipeline/main.c b/app/test-pipeline/main.c
index 8633933fd9..73f6d31f82 100644
--- a/app/test-pipeline/main.c
+++ b/app/test-pipeline/main.c
@@ -55,6 +55,7 @@ int
 main(int argc, char **argv)
 {
 	uint32_t lcore;
+	uint32_t i;
 	int ret;
 
 	/* Init EAL */
@@ -85,6 +86,24 @@ main(int argc, char **argv)
 			return -1;
 	}
 
+	/*Close ports */
+	for (i = 0; i < app.n_ports; i++) {
+		uint16_t port;
+		int ret;
+
+		port = app.ports[i];
+		printf("Closing port %d...", port);
+		ret = rte_eth_dev_stop(port);
+		if (ret != 0)
+			printf("rte_eth_dev_stop: err=%d, port=%u\n",
+					 ret, port);
+		rte_eth_dev_close(port);
+		printf("Done\n");
+	}
+
+	/* Clean up the EAL */
+	rte_eal_cleanup();
+
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH v3 0/3] fix test-pipeline issues

[Thomas Monjalon]

Cristian, please could you review this series please?

12/09/2023 08:39, Feifei Wang:
> For test-pipeline application, there are some problems with the normal
> operation of the program and security issues. These patches can fix
> these issues.
> 
> v3: fix SIGINT handling issue and add dev close operation
> 
> Feifei Wang (3):
>   app/test-pipeline: relax RSS hash requirement
>   app/test-pipeline: fix SIGINT handling issue
>   app/test-pipeline: add dev close operation
> 
>  app/test-pipeline/init.c              |  22 ++++-
>  app/test-pipeline/main.c              |  33 +++++++
>  app/test-pipeline/main.h              |   2 +
>  app/test-pipeline/pipeline_acl.c      |   6 +-
>  app/test-pipeline/pipeline_hash.c     | 110 ++++++++++-----------
>  app/test-pipeline/pipeline_lpm.c      |   6 +-
>  app/test-pipeline/pipeline_lpm_ipv6.c |   6 +-
>  app/test-pipeline/pipeline_stub.c     |   6 +-
>  app/test-pipeline/runtime.c           | 132 ++++++++++++++------------
>  9 files changed, 198 insertions(+), 125 deletions(-)

RE: [PATCH v3 0/3] fix test-pipeline issues

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Feifei Wang <feifei.wang2@arm.com>
> Sent: Tuesday, September 12, 2023 7:39 AM
> Cc: dev@dpdk.org; nd@arm.com; Feifei Wang <feifei.wang2@arm.com>
> Subject: [PATCH v3 0/3] fix test-pipeline issues
> 
> For test-pipeline application, there are some problems with the normal
> operation of the program and security issues. These patches can fix
> these issues.
> 
> v3: fix SIGINT handling issue and add dev close operation
> 
> Feifei Wang (3):
>   app/test-pipeline: relax RSS hash requirement
>   app/test-pipeline: fix SIGINT handling issue
>   app/test-pipeline: add dev close operation
> 
>  app/test-pipeline/init.c              |  22 ++++-
>  app/test-pipeline/main.c              |  33 +++++++
>  app/test-pipeline/main.h              |   2 +
>  app/test-pipeline/pipeline_acl.c      |   6 +-
>  app/test-pipeline/pipeline_hash.c     | 110 ++++++++++-----------
>  app/test-pipeline/pipeline_lpm.c      |   6 +-
>  app/test-pipeline/pipeline_lpm_ipv6.c |   6 +-
>  app/test-pipeline/pipeline_stub.c     |   6 +-
>  app/test-pipeline/runtime.c           | 132 ++++++++++++++------------
>  9 files changed, 198 insertions(+), 125 deletions(-)
> 
> --
> 2.25.1

Series-acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Re: [PATCH v3 0/3] fix test-pipeline issues

[Thomas Monjalon]

> > Feifei Wang (3):
> >   app/test-pipeline: relax RSS hash requirement
> >   app/test-pipeline: fix SIGINT handling issue
> >   app/test-pipeline: add dev close operation
> > 
> Series-acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Applied with few minor cleanups.