examples/multi_process: add options to control port configuration

Message ID 20220217151755.442306-1-wenwux.ma@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series examples/multi_process: add options to control port configuration |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

Ma, WenwuX Feb. 17, 2022, 3:17 p.m. UTC
  The default values of rx mq_mode and rx offloads for port
will cause symmetric_mp startup failure if the port do
not support rss or csum. Therefore, we added two new options
--rx-mq-mode and --rx-offloads, through which the user can set
the values appropriately according to the situation to make
app startup normally.

Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
---
 examples/multi_process/symmetric_mp/main.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
  

Comments

Bruce Richardson Feb. 17, 2022, 9:06 a.m. UTC | #1
On Thu, Feb 17, 2022 at 03:17:55PM +0000, Wenwu Ma wrote:
> The default values of rx mq_mode and rx offloads for port will cause
> symmetric_mp startup failure if the port do not support rss or csum.
> Therefore, we added two new options --rx-mq-mode and --rx-offloads,
> through which the user can set the values appropriately according to the
> situation to make app startup normally.
> 
> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> ---

The idea seems reasonable enough, but I think the implementation requiring
the user to pass in special "magic numbers" for the offload values is not a
good idea. Perhaps add in a separate flag for "no-csum" to disable that.

For the no-rss case, can you explain how you would see this app being used
in the absense of RSS support to distribute traffic among the separate
processes?
  
Ma, WenwuX Feb. 18, 2022, 6:49 a.m. UTC | #2
> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: 2022年2月17日 17:06
> To: Ma, WenwuX <wenwux.ma@intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org; Hu,
> Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He,
> Xingguang <xingguang.he@intel.com>
> Subject: Re: [PATCH] examples/multi_process: add options to control port
> configuration
> 
> On Thu, Feb 17, 2022 at 03:17:55PM +0000, Wenwu Ma wrote:
> > The default values of rx mq_mode and rx offloads for port will cause
> > symmetric_mp startup failure if the port do not support rss or csum.
> > Therefore, we added two new options --rx-mq-mode and --rx-offloads,
> > through which the user can set the values appropriately according to
> > the situation to make app startup normally.
> >
> > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> ---
> 
> The idea seems reasonable enough, but I think the implementation requiring
> the user to pass in special "magic numbers" for the offload values is not a
> good idea. Perhaps add in a separate flag for "no-csum" to disable that.
> 
> For the no-rss case, can you explain how you would see this app being used
> in the absense of RSS support to distribute traffic among the separate
> processes?

When app run in qemu vm and the backend is dpdk vhost, it will report error below:
"virtio_dev_configure(): RSS support requested but not supported by the device"
  
Bruce Richardson Feb. 18, 2022, 9:41 a.m. UTC | #3
On Fri, Feb 18, 2022 at 06:49:19AM +0000, Ma, WenwuX wrote:
> 
> 
> > -----Original Message-----
> > From: Richardson, Bruce <bruce.richardson@intel.com>
> > Sent: 2022年2月17日 17:06
> > To: Ma, WenwuX <wenwux.ma@intel.com>
> > Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org; Hu,
> > Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He,
> > Xingguang <xingguang.he@intel.com>
> > Subject: Re: [PATCH] examples/multi_process: add options to control port
> > configuration
> >
> > On Thu, Feb 17, 2022 at 03:17:55PM +0000, Wenwu Ma wrote:
> > > The default values of rx mq_mode and rx offloads for port will cause
> > > symmetric_mp startup failure if the port do not support rss or csum.
> > > Therefore, we added two new options --rx-mq-mode and --rx-offloads,
> > > through which the user can set the values appropriately according to
> > > the situation to make app startup normally.
> > >
> > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> ---
> >
> > The idea seems reasonable enough, but I think the implementation requiring
> > the user to pass in special "magic numbers" for the offload values is not a
> > good idea. Perhaps add in a separate flag for "no-csum" to disable that.
> >
> > For the no-rss case, can you explain how you would see this app being used
> > in the absense of RSS support to distribute traffic among the separate
> > processes?
> 
> When app run in qemu vm and the backend is dpdk vhost, it will report error below:
> "virtio_dev_configure(): RSS support requested but not supported by the device"

Sure, I understand that. But how does it make sense to run multiple copies
of the app if RSS cannot be used to spread the traffic between the
instances?
  
Ma, WenwuX Feb. 18, 2022, 10:10 a.m. UTC | #4
> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: 2022年2月18日 17:42
> To: Ma, WenwuX <wenwux.ma@intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org; Hu,
> Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He,
> Xingguang <xingguang.he@intel.com>
> Subject: Re: [PATCH] examples/multi_process: add options to control port
> configuration
> 
> On Fri, Feb 18, 2022 at 06:49:19AM +0000, Ma, WenwuX wrote:
> >
> >
> > > -----Original Message-----
> > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > Sent: 2022年2月17日 17:06
> > > To: Ma, WenwuX <wenwux.ma@intel.com>
> > > Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org; Hu,
> > > Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He,
> > > Xingguang <xingguang.he@intel.com>
> > > Subject: Re: [PATCH] examples/multi_process: add options to control
> > > port configuration
> > >
> > > On Thu, Feb 17, 2022 at 03:17:55PM +0000, Wenwu Ma wrote:
> > > > The default values of rx mq_mode and rx offloads for port will
> > > > cause symmetric_mp startup failure if the port do not support rss or
> csum.
> > > > Therefore, we added two new options --rx-mq-mode and
> > > > --rx-offloads, through which the user can set the values
> > > > appropriately according to the situation to make app startup normally.
> > > >
> > > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> ---
> > >
> > > The idea seems reasonable enough, but I think the implementation
> > > requiring the user to pass in special "magic numbers" for the
> > > offload values is not a good idea. Perhaps add in a separate flag for "no-
> csum" to disable that.
> > >
> > > For the no-rss case, can you explain how you would see this app
> > > being used in the absense of RSS support to distribute traffic among
> > > the separate processes?
> >
> > When app run in qemu vm and the backend is dpdk vhost, it will report
> error below:
> > "virtio_dev_configure(): RSS support requested but not supported by the
> device"
> 
> Sure, I understand that. But how does it make sense to run multiple copies of
> the app if RSS cannot be used to spread the traffic between the instances?

in test case, vhost backend has 2 queues, and we set fwd as txonly, and in vm, we run app as follows

./examples/multi_process/symmetric_mp/build/symmetric_mp -l 1 -n 4 --proc-type=auto -- -p 3 --num-procs=2 --proc-id=0
./examples/multi_process/symmetric_mp/build/symmetric_mp -l 2 -n 4 --proc-type=secondary -- -p 3 --num-procs=2 --proc-id=1

So,  instance 0 receive pkts in queue 0, and instance 1 receive pkts in queue 1.
  
Bruce Richardson Feb. 18, 2022, 10:22 a.m. UTC | #5
On Fri, Feb 18, 2022 at 10:10:58AM +0000, Ma, WenwuX wrote:
> 
> 
> > -----Original Message-----
> > From: Richardson, Bruce <bruce.richardson@intel.com>
> > Sent: 2022年2月18日 17:42
> > To: Ma, WenwuX <wenwux.ma@intel.com>
> > Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org; Hu,
> > Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He,
> > Xingguang <xingguang.he@intel.com>
> > Subject: Re: [PATCH] examples/multi_process: add options to control port
> > configuration
> >
> > On Fri, Feb 18, 2022 at 06:49:19AM +0000, Ma, WenwuX wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > > Sent: 2022年2月17日 17:06
> > > > To: Ma, WenwuX <wenwux.ma@intel.com>
> > > > Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org; Hu,
> > > > Jiayu <jiayu.hu@intel.com>; Wang, Yinan <yinan.wang@intel.com>; He,
> > > > Xingguang <xingguang.he@intel.com>
> > > > Subject: Re: [PATCH] examples/multi_process: add options to control
> > > > port configuration
> > > >
> > > > On Thu, Feb 17, 2022 at 03:17:55PM +0000, Wenwu Ma wrote:
> > > > > The default values of rx mq_mode and rx offloads for port will
> > > > > cause symmetric_mp startup failure if the port do not support rss or
> > csum.
> > > > > Therefore, we added two new options --rx-mq-mode and
> > > > > --rx-offloads, through which the user can set the values
> > > > > appropriately according to the situation to make app startup normally.
> > > > >
> > > > > Signed-off-by: Wenwu Ma <wenwux.ma@intel.com> ---
> > > >
> > > > The idea seems reasonable enough, but I think the implementation
> > > > requiring the user to pass in special "magic numbers" for the
> > > > offload values is not a good idea. Perhaps add in a separate flag for "no-
> > csum" to disable that.
> > > >
> > > > For the no-rss case, can you explain how you would see this app
> > > > being used in the absense of RSS support to distribute traffic among
> > > > the separate processes?
> > >
> > > When app run in qemu vm and the backend is dpdk vhost, it will report
> > error below:
> > > "virtio_dev_configure(): RSS support requested but not supported by the
> > device"
> >
> > Sure, I understand that. But how does it make sense to run multiple copies of
> > the app if RSS cannot be used to spread the traffic between the instances?
> 
> in test case, vhost backend has 2 queues, and we set fwd as txonly, and in vm, we run app as follows
> 
> ./examples/multi_process/symmetric_mp/build/symmetric_mp -l 1 -n 4 --proc-type=auto -- -p 3 --num-procs=2 --proc-id=0
> ./examples/multi_process/symmetric_mp/build/symmetric_mp -l 2 -n 4 --proc-type=secondary -- -p 3 --num-procs=2 --proc-id=1
> 
> So,  instance 0 receive pkts in queue 0, and instance 1 receive pkts in queue 1.
> 
Interesting. So with vhost/virtio, the distribution of the traffic to
queues in entirely controls by the backend without the virtio PMD being
able to control it?

Anyway, if we want to allow running the app without rss, I would echo the
comment I made about "csum", in that we probably just want a suitably named
flag for it, rather than just requiring magic numbers from the user.

Two options therefore I'd suggest:
1. just make the app ignore errors with csum and RSS - if they are not
supported by a NIC, just log a warning and run the app anyway,
reconfiguring the NIC without them. Only quit the app if the second
reconfiguration fails.
2. Add explicit --no-csum and --no-rss flags.

I would actually tend towards the first option as more usable, but the
second could work too.

/Bruce
  

Patch

diff --git a/examples/multi_process/symmetric_mp/main.c b/examples/multi_process/symmetric_mp/main.c
index 050337765f..df41abe682 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -52,6 +52,8 @@ 
 
 #define PARAM_PROC_ID "proc-id"
 #define PARAM_NUM_PROCS "num-procs"
+#define PARAM_RX_OFFLOADS "rx-offloads"
+#define PARAM_RX_MQ_MODE "rx-mq-mode"
 
 /* for each lcore, record the elements of the ports array to use */
 struct lcore_ports{
@@ -69,6 +71,8 @@  struct port_stats{
 
 static int proc_id = -1;
 static unsigned num_procs = 0;
+static uint64_t rx_offloads = RTE_ETH_RX_OFFLOAD_CHECKSUM;
+static enum rte_eth_rx_mq_mode rx_mq_mode = RTE_ETH_MQ_RX_RSS;
 
 static uint16_t ports[RTE_MAX_ETHPORTS];
 static unsigned num_ports = 0;
@@ -84,9 +88,13 @@  smp_usage(const char *prgname, const char *errmsg)
 	printf("\n%s [EAL options] -- -p <port mask> "
 			"--"PARAM_NUM_PROCS" <n>"
 			" --"PARAM_PROC_ID" <id>\n"
+			" --"PARAM_RX_OFFLOADS" <offload>\n"
+			" --"PARAM_RX_MQ_MODE" <mode>\n"
 			"-p         : a hex bitmask indicating what ports are to be used\n"
 			"--num-procs: the number of processes which will be used\n"
 			"--proc-id  : the id of the current process (id < num-procs)\n"
+			"--rx-offloads : rx offload capabilities of ports\n"
+			"--rx-mq-mode : the multi-queue packet distribution mode, e.g. RSS.\n"
 			"\n",
 			prgname);
 	exit(1);
@@ -119,6 +127,8 @@  smp_parse_args(int argc, char **argv)
 	static struct option lgopts[] = {
 			{PARAM_NUM_PROCS, 1, 0, 0},
 			{PARAM_PROC_ID, 1, 0, 0},
+			{PARAM_RX_OFFLOADS, 1, 0, 0},
+			{PARAM_RX_MQ_MODE, 1, 0, 0},
 			{NULL, 0, 0, 0}
 	};
 
@@ -137,6 +147,10 @@  smp_parse_args(int argc, char **argv)
 				num_procs = atoi(optarg);
 			else if (strncmp(lgopts[option_index].name, PARAM_PROC_ID, 7) == 0)
 				proc_id = atoi(optarg);
+			else if (strncmp(lgopts[option_index].name, PARAM_RX_OFFLOADS, 11) == 0)
+				rx_offloads = atoi(optarg);
+			else if (strncmp(lgopts[option_index].name, PARAM_RX_MQ_MODE, 10) == 0)
+				rx_mq_mode = atoi(optarg);
 			break;
 
 		default:
@@ -175,9 +189,9 @@  smp_port_init(uint16_t port, struct rte_mempool *mbuf_pool,
 {
 	struct rte_eth_conf port_conf = {
 			.rxmode = {
-				.mq_mode	= RTE_ETH_MQ_RX_RSS,
+				.mq_mode	= rx_mq_mode,
 				.split_hdr_size = 0,
-				.offloads = RTE_ETH_RX_OFFLOAD_CHECKSUM,
+				.offloads = rx_offloads,
 			},
 			.rx_adv_conf = {
 				.rss_conf = {