DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] app/testpmd: add cmdline option to set Rx mq mode
@ 2020-04-29 13:04 Xiaoyu Min
  2020-04-30  9:16 ` Iremonger, Bernard
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Xiaoyu Min @ 2020-04-29 13:04 UTC (permalink / raw)
  To: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, John McNamara,
	Marko Kovacevic
  Cc: dev

One new cmdline option `--rx-mq-mode` is added in order to have the
possibility to check whether PMD handle the mq mode correctly or not.

The reason is some NICs need to do different settings based on different
RX mq mode, i.e RSS or not.

With this support in testpmd, the above scenario can be tested easily.

Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
 app/test-pmd/parameters.c             | 12 ++++++++++++
 app/test-pmd/testpmd.c                | 17 ++++++++++++++---
 app/test-pmd/testpmd.h                |  3 +++
 doc/guides/testpmd_app_ug/run_app.rst |  7 +++++++
 4 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 30c1753c32..a9dd58e96b 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -212,6 +212,7 @@ usage(char* progname)
 	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
 	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
 	       "valid only with --mp-alloc=anon\n");
+	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv)
 		{ "noisy-lkup-num-reads",	1, 0, 0 },
 		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
 		{ "no-iova-contig",             0, 0, 0 },
+		{ "rx-mq-mode",                 1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv)
 			}
 			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
 				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
+
+			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
+				char *end = NULL;
+				n = strtoul(optarg, &end, 16);
+				if (n >= 0)
+					rx_mq_mode = (enum rte_eth_rx_mq_mode)n;
+				else
+					rte_exit(EXIT_FAILURE,
+						 "rx-mq-mode must be >= 0\n");
+			}
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfd..9536d6ee27 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -482,6 +482,11 @@ uint8_t bitrate_enabled;
 struct gro_status gro_ports[RTE_MAX_ETHPORTS];
 uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
 
+/*
+ * RX mq mode value set in the commandline
+ */
+enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
+
 /* Forward function declarations */
 static void setup_attached_port(portid_t pi);
 static void map_port_queue_stats_mapping_registers(portid_t pi,
@@ -3337,7 +3342,9 @@ init_port_config(void)
 
 		if (port->dcb_flag == 0) {
 			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
-				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
+				port->dev_conf.rxmode.mq_mode =
+					(enum rte_eth_rx_mq_mode)
+						(rx_mq_mode & ETH_MQ_RX_RSS);
 			else
 				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
 		}
@@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
 		}
 
 		/* set DCB mode of RX and TX of multiple queues */
-		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
+		eth_conf->rxmode.mq_mode =
+				(enum rte_eth_rx_mq_mode)
+					(rx_mq_mode & ETH_MQ_RX_VMDQ_DCB);
 		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
 	} else {
 		struct rte_eth_dcb_rx_conf *rx_conf =
@@ -3458,7 +3467,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
 			tx_conf->dcb_tc[i] = i % num_tcs;
 		}
 
-		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
+		eth_conf->rxmode.mq_mode =
+				(enum rte_eth_rx_mq_mode)
+					(rx_mq_mode & ETH_MQ_RX_DCB_RSS);
 		eth_conf->rx_adv_conf.rss_conf = rss_conf;
 		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
 	}
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7ff4c5dba3..32bb324c94 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -602,6 +602,9 @@ struct mplsoudp_decap_conf {
 };
 extern struct mplsoudp_decap_conf mplsoudp_decap_conf;
 
+/* RX mq mode parameter. */
+extern enum rte_eth_rx_mq_mode rx_mq_mode;
+
 static inline unsigned int
 lcore_num(void)
 {
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 727ef52b8f..4f46299e68 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -481,3 +481,10 @@ The command line options are:
 
     Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
     The default value is 0.
+
+*   ``--rx-mq-mode``
+
+    Set the hexadecimal bitmask of RX queue mq mode.
+    The default value is 0x7::
+
+       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG
-- 
2.26.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: add cmdline option to set Rx mq mode
  2020-04-29 13:04 [dpdk-dev] [PATCH] app/testpmd: add cmdline option to set Rx mq mode Xiaoyu Min
@ 2020-04-30  9:16 ` Iremonger, Bernard
  2020-04-30 12:13   ` Xiaoyu Min
  2020-04-30 13:07 ` [dpdk-dev] [PATCH v2] " Xiaoyu Min
  2020-05-07  0:51 ` [dpdk-dev] [PATCH v3] " Xiaoyu Min
  2 siblings, 1 reply; 14+ messages in thread
From: Iremonger, Bernard @ 2020-04-30  9:16 UTC (permalink / raw)
  To: Xiaoyu Min, Lu, Wenzhuo, Wu, Jingjing, Mcnamara, John, Kovacevic, Marko
  Cc: dev

Hi Xiaoyu,

> -----Original Message-----
> From: Xiaoyu Min <jackmin@mellanox.com>
> Sent: Wednesday, April 29, 2020 2:04 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH] app/testpmd: add cmdline option to set Rx mq mode
> 
> One new cmdline option `--rx-mq-mode` is added in order to have the
> possibility to check whether PMD handle the mq mode correctly or not.
> 
> The reason is some NICs need to do different settings based on different RX
> mq mode, i.e RSS or not.
> 
> With this support in testpmd, the above scenario can be tested easily.
> 
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
>  app/test-pmd/parameters.c             | 12 ++++++++++++
>  app/test-pmd/testpmd.c                | 17 ++++++++++++++---
>  app/test-pmd/testpmd.h                |  3 +++
>  doc/guides/testpmd_app_ug/run_app.rst |  7 +++++++
>  4 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index
> 30c1753c32..a9dd58e96b 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -212,6 +212,7 @@ usage(char* progname)
>  	printf("  --noisy-lkup-num-writes=N: do N random reads and writes
> per packet\n");
>  	printf("  --no-iova-contig: mempool memory can be IOVA non
> contiguous. "
>  	       "valid only with --mp-alloc=anon\n");
> +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq
> mode\n");
>  }
> 
>  #ifdef RTE_LIBRTE_CMDLINE
> @@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv)
>  		{ "noisy-lkup-num-reads",	1, 0, 0 },
>  		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
>  		{ "no-iova-contig",             0, 0, 0 },
> +		{ "rx-mq-mode",                 1, 0, 0 },
>  		{ 0, 0, 0, 0 },
>  	};
> 
> @@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv)
>  			}
>  			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
>  				mempool_flags =
> MEMPOOL_F_NO_IOVA_CONTIG;
> +
> +			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
> +				char *end = NULL;
> +				n = strtoul(optarg, &end, 16);
> +				if (n >= 0)
> +					rx_mq_mode = (enum
> rte_eth_rx_mq_mode)n;
> +				else
> +					rte_exit(EXIT_FAILURE,
> +						 "rx-mq-mode must be >=
> 0\n");
> +			}
>  			break;
>  		case 'h':
>  			usage(argv[0]);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 99bacddbfd..9536d6ee27 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -482,6 +482,11 @@ uint8_t bitrate_enabled;  struct gro_status
> gro_ports[RTE_MAX_ETHPORTS];  uint8_t gro_flush_cycles =
> GRO_DEFAULT_FLUSH_CYCLES;
> 
> +/*
> + * RX mq mode value set in the commandline  */ enum
> rte_eth_rx_mq_mode
> +rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
> +
>  /* Forward function declarations */
>  static void setup_attached_port(portid_t pi);  static void
> map_port_queue_stats_mapping_registers(portid_t pi, @@ -3337,7 +3342,9
> @@ init_port_config(void)
> 
>  		if (port->dcb_flag == 0) {
>  			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> -				port->dev_conf.rxmode.mq_mode =
> ETH_MQ_RX_RSS;
> +				port->dev_conf.rxmode.mq_mode =
> +					(enum rte_eth_rx_mq_mode)
> +						(rx_mq_mode &
> ETH_MQ_RX_RSS);
>  			else
>  				port->dev_conf.rxmode.mq_mode =
> ETH_MQ_RX_NONE;
>  		}
> @@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct
> rte_eth_conf *eth_conf,
>  		}
> 
>  		/* set DCB mode of RX and TX of multiple queues */
> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
> +		eth_conf->rxmode.mq_mode =
> +				(enum rte_eth_rx_mq_mode)
> +					(rx_mq_mode &
> ETH_MQ_RX_VMDQ_DCB);
>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
>  	} else {
>  		struct rte_eth_dcb_rx_conf *rx_conf = @@ -3458,7 +3467,9
> @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
>  			tx_conf->dcb_tc[i] = i % num_tcs;
>  		}
> 
> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
> +		eth_conf->rxmode.mq_mode =
> +				(enum rte_eth_rx_mq_mode)
> +					(rx_mq_mode &
> ETH_MQ_RX_DCB_RSS);
>  		eth_conf->rx_adv_conf.rss_conf = rss_conf;
>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
>  	}
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 7ff4c5dba3..32bb324c94 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -602,6 +602,9 @@ struct mplsoudp_decap_conf {  };  extern struct
> mplsoudp_decap_conf mplsoudp_decap_conf;
> 
> +/* RX mq mode parameter. */
> +extern enum rte_eth_rx_mq_mode rx_mq_mode;
> +
>  static inline unsigned int
>  lcore_num(void)
>  {
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst
> b/doc/guides/testpmd_app_ug/run_app.rst
> index 727ef52b8f..4f46299e68 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -481,3 +481,10 @@ The command line options are:
> 
>      Enable to create mempool which is not IOVA contiguous. Valid only with --
> mp-alloc=anon.
>      The default value is 0.
> +
> +*   ``--rx-mq-mode``
> +
> +    Set the hexadecimal bitmask of RX queue mq mode.
> +    The default value is 0x7::
> +
> +       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG |
> ETH_MQ_RX_VMDQ_FLAG
> --
> 2.26.0

Should there be an update to the release notes to announce this new tespmd command line option?

Regards,

Bernard.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: add cmdline option to set Rx mq mode
  2020-04-30  9:16 ` Iremonger, Bernard
@ 2020-04-30 12:13   ` Xiaoyu Min
  0 siblings, 0 replies; 14+ messages in thread
From: Xiaoyu Min @ 2020-04-30 12:13 UTC (permalink / raw)
  To: Iremonger, Bernard
  Cc: Lu, Wenzhuo, Wu, Jingjing, Mcnamara, John, Kovacevic, Marko, dev

On Thu, 20-04-30, 09:16, Iremonger, Bernard wrote:
> Hi Xiaoyu,
Hey Bernard,

> 
> > -----Original Message-----
> > From: Xiaoyu Min <jackmin@mellanox.com>
> > Sent: Wednesday, April 29, 2020 2:04 PM
> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Iremonger, Bernard
> > <bernard.iremonger@intel.com>; Mcnamara, John
> > <john.mcnamara@intel.com>; Kovacevic, Marko
> > <marko.kovacevic@intel.com>
> > Cc: dev@dpdk.org
> > Subject: [PATCH] app/testpmd: add cmdline option to set Rx mq mode
> > 
> > One new cmdline option `--rx-mq-mode` is added in order to have the
> > possibility to check whether PMD handle the mq mode correctly or not.
> > 
> > The reason is some NICs need to do different settings based on different RX
> > mq mode, i.e RSS or not.
> > 
> > With this support in testpmd, the above scenario can be tested easily.
> > 
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > ---
> >  app/test-pmd/parameters.c             | 12 ++++++++++++
> >  app/test-pmd/testpmd.c                | 17 ++++++++++++++---
> >  app/test-pmd/testpmd.h                |  3 +++
> >  doc/guides/testpmd_app_ug/run_app.rst |  7 +++++++
> >  4 files changed, 36 insertions(+), 3 deletions(-)
> > 
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index
> > 30c1753c32..a9dd58e96b 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -212,6 +212,7 @@ usage(char* progname)
> >  	printf("  --noisy-lkup-num-writes=N: do N random reads and writes
> > per packet\n");
> >  	printf("  --no-iova-contig: mempool memory can be IOVA non
> > contiguous. "
> >  	       "valid only with --mp-alloc=anon\n");
> > +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq
> > mode\n");
> >  }
> > 
> >  #ifdef RTE_LIBRTE_CMDLINE
> > @@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv)
> >  		{ "noisy-lkup-num-reads",	1, 0, 0 },
> >  		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
> >  		{ "no-iova-contig",             0, 0, 0 },
> > +		{ "rx-mq-mode",                 1, 0, 0 },
> >  		{ 0, 0, 0, 0 },
> >  	};
> > 
> > @@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv)
> >  			}
> >  			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
> >  				mempool_flags =
> > MEMPOOL_F_NO_IOVA_CONTIG;
> > +
> > +			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
> > +				char *end = NULL;
> > +				n = strtoul(optarg, &end, 16);
> > +				if (n >= 0)
> > +					rx_mq_mode = (enum
> > rte_eth_rx_mq_mode)n;
> > +				else
> > +					rte_exit(EXIT_FAILURE,
> > +						 "rx-mq-mode must be >=
> > 0\n");
> > +			}
> >  			break;
> >  		case 'h':
> >  			usage(argv[0]);
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 99bacddbfd..9536d6ee27 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -482,6 +482,11 @@ uint8_t bitrate_enabled;  struct gro_status
> > gro_ports[RTE_MAX_ETHPORTS];  uint8_t gro_flush_cycles =
> > GRO_DEFAULT_FLUSH_CYCLES;
> > 
> > +/*
> > + * RX mq mode value set in the commandline  */ enum
> > rte_eth_rx_mq_mode
> > +rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
> > +
> >  /* Forward function declarations */
> >  static void setup_attached_port(portid_t pi);  static void
> > map_port_queue_stats_mapping_registers(portid_t pi, @@ -3337,7 +3342,9
> > @@ init_port_config(void)
> > 
> >  		if (port->dcb_flag == 0) {
> >  			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> > -				port->dev_conf.rxmode.mq_mode =
> > ETH_MQ_RX_RSS;
> > +				port->dev_conf.rxmode.mq_mode =
> > +					(enum rte_eth_rx_mq_mode)
> > +						(rx_mq_mode &
> > ETH_MQ_RX_RSS);
> >  			else
> >  				port->dev_conf.rxmode.mq_mode =
> > ETH_MQ_RX_NONE;
> >  		}
> > @@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct
> > rte_eth_conf *eth_conf,
> >  		}
> > 
> >  		/* set DCB mode of RX and TX of multiple queues */
> > -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
> > +		eth_conf->rxmode.mq_mode =
> > +				(enum rte_eth_rx_mq_mode)
> > +					(rx_mq_mode &
> > ETH_MQ_RX_VMDQ_DCB);
> >  		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
> >  	} else {
> >  		struct rte_eth_dcb_rx_conf *rx_conf = @@ -3458,7 +3467,9
> > @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
> >  			tx_conf->dcb_tc[i] = i % num_tcs;
> >  		}
> > 
> > -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
> > +		eth_conf->rxmode.mq_mode =
> > +				(enum rte_eth_rx_mq_mode)
> > +					(rx_mq_mode &
> > ETH_MQ_RX_DCB_RSS);
> >  		eth_conf->rx_adv_conf.rss_conf = rss_conf;
> >  		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
> >  	}
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > 7ff4c5dba3..32bb324c94 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -602,6 +602,9 @@ struct mplsoudp_decap_conf {  };  extern struct
> > mplsoudp_decap_conf mplsoudp_decap_conf;
> > 
> > +/* RX mq mode parameter. */
> > +extern enum rte_eth_rx_mq_mode rx_mq_mode;
> > +
> >  static inline unsigned int
> >  lcore_num(void)
> >  {
> > diff --git a/doc/guides/testpmd_app_ug/run_app.rst
> > b/doc/guides/testpmd_app_ug/run_app.rst
> > index 727ef52b8f..4f46299e68 100644
> > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > @@ -481,3 +481,10 @@ The command line options are:
> > 
> >      Enable to create mempool which is not IOVA contiguous. Valid only with --
> > mp-alloc=anon.
> >      The default value is 0.
> > +
> > +*   ``--rx-mq-mode``
> > +
> > +    Set the hexadecimal bitmask of RX queue mq mode.
> > +    The default value is 0x7::
> > +
> > +       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG |
> > ETH_MQ_RX_VMDQ_FLAG
> > --
> > 2.26.0
> 
> Should there be an update to the release notes to announce this new tespmd command line option?
It should. I'll update release note in v2 soon.

Thank you.

> 
> Regards,
> 
> Bernard.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode
  2020-04-29 13:04 [dpdk-dev] [PATCH] app/testpmd: add cmdline option to set Rx mq mode Xiaoyu Min
  2020-04-30  9:16 ` Iremonger, Bernard
@ 2020-04-30 13:07 ` Xiaoyu Min
  2020-04-30 13:33   ` Iremonger, Bernard
  2020-05-05 16:06   ` Ferruh Yigit
  2020-05-07  0:51 ` [dpdk-dev] [PATCH v3] " Xiaoyu Min
  2 siblings, 2 replies; 14+ messages in thread
From: Xiaoyu Min @ 2020-04-30 13:07 UTC (permalink / raw)
  To: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, John McNamara,
	Marko Kovacevic
  Cc: dev

One new cmdline option `--rx-mq-mode` is added in order to have the
possibility to check whether PMD handle the mq mode correctly or not.

The reason is some NICs need to do different settings based on different
RX mq mode, i.e RSS or not.

With this support in testpmd, the above scenario can be tested easily.

Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
 app/test-pmd/parameters.c              | 12 ++++++++++++
 app/test-pmd/testpmd.c                 | 17 ++++++++++++++---
 app/test-pmd/testpmd.h                 |  3 +++
 doc/guides/rel_notes/release_20_05.rst |  4 ++++
 doc/guides/testpmd_app_ug/run_app.rst  |  7 +++++++
 5 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 30c1753c32..a9dd58e96b 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -212,6 +212,7 @@ usage(char* progname)
 	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
 	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
 	       "valid only with --mp-alloc=anon\n");
+	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv)
 		{ "noisy-lkup-num-reads",	1, 0, 0 },
 		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
 		{ "no-iova-contig",             0, 0, 0 },
+		{ "rx-mq-mode",                 1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv)
 			}
 			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
 				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
+
+			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
+				char *end = NULL;
+				n = strtoul(optarg, &end, 16);
+				if (n >= 0)
+					rx_mq_mode = (enum rte_eth_rx_mq_mode)n;
+				else
+					rte_exit(EXIT_FAILURE,
+						 "rx-mq-mode must be >= 0\n");
+			}
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfd..9536d6ee27 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -482,6 +482,11 @@ uint8_t bitrate_enabled;
 struct gro_status gro_ports[RTE_MAX_ETHPORTS];
 uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
 
+/*
+ * RX mq mode value set in the commandline
+ */
+enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
+
 /* Forward function declarations */
 static void setup_attached_port(portid_t pi);
 static void map_port_queue_stats_mapping_registers(portid_t pi,
@@ -3337,7 +3342,9 @@ init_port_config(void)
 
 		if (port->dcb_flag == 0) {
 			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
-				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
+				port->dev_conf.rxmode.mq_mode =
+					(enum rte_eth_rx_mq_mode)
+						(rx_mq_mode & ETH_MQ_RX_RSS);
 			else
 				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
 		}
@@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
 		}
 
 		/* set DCB mode of RX and TX of multiple queues */
-		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
+		eth_conf->rxmode.mq_mode =
+				(enum rte_eth_rx_mq_mode)
+					(rx_mq_mode & ETH_MQ_RX_VMDQ_DCB);
 		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
 	} else {
 		struct rte_eth_dcb_rx_conf *rx_conf =
@@ -3458,7 +3467,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
 			tx_conf->dcb_tc[i] = i % num_tcs;
 		}
 
-		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
+		eth_conf->rxmode.mq_mode =
+				(enum rte_eth_rx_mq_mode)
+					(rx_mq_mode & ETH_MQ_RX_DCB_RSS);
 		eth_conf->rx_adv_conf.rss_conf = rss_conf;
 		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
 	}
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7ff4c5dba3..32bb324c94 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -602,6 +602,9 @@ struct mplsoudp_decap_conf {
 };
 extern struct mplsoudp_decap_conf mplsoudp_decap_conf;
 
+/* RX mq mode parameter. */
+extern enum rte_eth_rx_mq_mode rx_mq_mode;
+
 static inline unsigned int
 lcore_num(void)
 {
diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
index b124c3f287..8026afb451 100644
--- a/doc/guides/rel_notes/release_20_05.rst
+++ b/doc/guides/rel_notes/release_20_05.rst
@@ -212,6 +212,10 @@ New Features
   * Added IPsec inbound load-distribution support for ipsec-secgw application
     using NIC load distribution feature(Flow Director).
 
+* **Updated testpmd application.**
+
+  * Added a new cmdline option ``--rx-mq-mode`` which can be used to test PMD's
+    behaviour on handling Rx mq mode.
 
 Removed Items
 -------------
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 727ef52b8f..4f46299e68 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -481,3 +481,10 @@ The command line options are:
 
     Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
     The default value is 0.
+
+*   ``--rx-mq-mode``
+
+    Set the hexadecimal bitmask of RX queue mq mode.
+    The default value is 0x7::
+
+       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG
-- 
2.26.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode
  2020-04-30 13:07 ` [dpdk-dev] [PATCH v2] " Xiaoyu Min
@ 2020-04-30 13:33   ` Iremonger, Bernard
  2020-05-05 16:06   ` Ferruh Yigit
  1 sibling, 0 replies; 14+ messages in thread
From: Iremonger, Bernard @ 2020-04-30 13:33 UTC (permalink / raw)
  To: Xiaoyu Min, Lu, Wenzhuo, Wu, Jingjing, Mcnamara, John, Kovacevic, Marko
  Cc: dev

> -----Original Message-----
> From: Xiaoyu Min <jackmin@mellanox.com>
> Sent: Thursday, April 30, 2020 2:08 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode
> 
> One new cmdline option `--rx-mq-mode` is added in order to have the
> possibility to check whether PMD handle the mq mode correctly or not.
> 
> The reason is some NICs need to do different settings based on different RX
> mq mode, i.e RSS or not.
> 
> With this support in testpmd, the above scenario can be tested easily.
> 
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode
  2020-04-30 13:07 ` [dpdk-dev] [PATCH v2] " Xiaoyu Min
  2020-04-30 13:33   ` Iremonger, Bernard
@ 2020-05-05 16:06   ` Ferruh Yigit
  2020-05-06  2:52     ` Xiaoyu Min
  1 sibling, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2020-05-05 16:06 UTC (permalink / raw)
  To: Xiaoyu Min, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger,
	John McNamara, Marko Kovacevic
  Cc: dev

On 4/30/2020 2:07 PM, Xiaoyu Min wrote:
> One new cmdline option `--rx-mq-mode` is added in order to have the
> possibility to check whether PMD handle the mq mode correctly or not.
> 
> The reason is some NICs need to do different settings based on different
> RX mq mode, i.e RSS or not.
> 
> With this support in testpmd, the above scenario can be tested easily.
> 
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
>  app/test-pmd/parameters.c              | 12 ++++++++++++
>  app/test-pmd/testpmd.c                 | 17 ++++++++++++++---
>  app/test-pmd/testpmd.h                 |  3 +++
>  doc/guides/rel_notes/release_20_05.rst |  4 ++++
>  doc/guides/testpmd_app_ug/run_app.rst  |  7 +++++++
>  5 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 30c1753c32..a9dd58e96b 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -212,6 +212,7 @@ usage(char* progname)
>  	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
>  	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
>  	       "valid only with --mp-alloc=anon\n");
> +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode\n");

Do you think does it worth to say the bitmask is for modes that can be enabled,
to remove need to look the code, not sure.

>  }
>  
>  #ifdef RTE_LIBRTE_CMDLINE
> @@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv)
>  		{ "noisy-lkup-num-reads",	1, 0, 0 },
>  		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
>  		{ "no-iova-contig",             0, 0, 0 },
> +		{ "rx-mq-mode",                 1, 0, 0 },
>  		{ 0, 0, 0, 0 },
>  	};
>  
> @@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv)
>  			}
>  			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
>  				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
> +
> +			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
> +				char *end = NULL;
> +				n = strtoul(optarg, &end, 16);
> +				if (n >= 0)
> +					rx_mq_mode = (enum rte_eth_rx_mq_mode)n;

Should we check if the provided value, 'n', is not out of the enum range?

> +				else
> +					rte_exit(EXIT_FAILURE,
> +						 "rx-mq-mode must be >= 0\n");
> +			}
>  			break;
>  		case 'h':
>  			usage(argv[0]);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 99bacddbfd..9536d6ee27 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -482,6 +482,11 @@ uint8_t bitrate_enabled;
>  struct gro_status gro_ports[RTE_MAX_ETHPORTS];
>  uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
>  
> +/*
> + * RX mq mode value set in the commandline

This is not the "RX mq mode value", the above help string seems more accurate,
"hexadecimal bitmask of RX mq mode". Can you please update here?

> + */
> +enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
> +
>  /* Forward function declarations */
>  static void setup_attached_port(portid_t pi);
>  static void map_port_queue_stats_mapping_registers(portid_t pi,
> @@ -3337,7 +3342,9 @@ init_port_config(void)
>  
>  		if (port->dcb_flag == 0) {
>  			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> -				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
> +				port->dev_conf.rxmode.mq_mode =
> +					(enum rte_eth_rx_mq_mode)

Do we need this enum type cast?

> +						(rx_mq_mode & ETH_MQ_RX_RSS);
>  			else
>  				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
>  		}
> @@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
>  		}
>  
>  		/* set DCB mode of RX and TX of multiple queues */
> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
> +		eth_conf->rxmode.mq_mode =
> +				(enum rte_eth_rx_mq_mode)
> +					(rx_mq_mode & ETH_MQ_RX_VMDQ_DCB);
>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
>  	} else {
>  		struct rte_eth_dcb_rx_conf *rx_conf =
> @@ -3458,7 +3467,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
>  			tx_conf->dcb_tc[i] = i % num_tcs;
>  		}
>  
> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
> +		eth_conf->rxmode.mq_mode =
> +				(enum rte_eth_rx_mq_mode)
> +					(rx_mq_mode & ETH_MQ_RX_DCB_RSS);
>  		eth_conf->rx_adv_conf.rss_conf = rss_conf;
>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
>  	}
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 7ff4c5dba3..32bb324c94 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -602,6 +602,9 @@ struct mplsoudp_decap_conf {
>  };
>  extern struct mplsoudp_decap_conf mplsoudp_decap_conf;
>  
> +/* RX mq mode parameter. */

The variable name gives as much context as the comment, may it be dropped?

> +extern enum rte_eth_rx_mq_mode rx_mq_mode;
> +
>  static inline unsigned int
>  lcore_num(void)
>  {
> diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
> index b124c3f287..8026afb451 100644
> --- a/doc/guides/rel_notes/release_20_05.rst
> +++ b/doc/guides/rel_notes/release_20_05.rst
> @@ -212,6 +212,10 @@ New Features
>    * Added IPsec inbound load-distribution support for ipsec-secgw application
>      using NIC load distribution feature(Flow Director).
>  
> +* **Updated testpmd application.**
> +
> +  * Added a new cmdline option ``--rx-mq-mode`` which can be used to test PMD's
> +    behaviour on handling Rx mq mode.
>  
>  Removed Items
>  -------------
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
> index 727ef52b8f..4f46299e68 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -481,3 +481,10 @@ The command line options are:
>  
>      Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
>      The default value is 0.
> +
> +*   ``--rx-mq-mode``
> +
> +    Set the hexadecimal bitmask of RX queue mq mode.

It is good to expand the 'mq' at least in the documentation, and I guess it is
"multi queue".

> +    The default value is 0x7::
> +
> +       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode
  2020-05-05 16:06   ` Ferruh Yigit
@ 2020-05-06  2:52     ` Xiaoyu Min
  2020-05-06  9:36       ` Ferruh Yigit
  0 siblings, 1 reply; 14+ messages in thread
From: Xiaoyu Min @ 2020-05-06  2:52 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, John McNamara,
	Marko Kovacevic, dev

On Tue, 20-05-05, 17:06, Ferruh Yigit wrote:
> On 4/30/2020 2:07 PM, Xiaoyu Min wrote:
> > One new cmdline option `--rx-mq-mode` is added in order to have the
> > possibility to check whether PMD handle the mq mode correctly or not.
> > 
> > The reason is some NICs need to do different settings based on different
> > RX mq mode, i.e RSS or not.
> > 
> > With this support in testpmd, the above scenario can be tested easily.
> > 
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > ---
> >  app/test-pmd/parameters.c              | 12 ++++++++++++
> >  app/test-pmd/testpmd.c                 | 17 ++++++++++++++---
> >  app/test-pmd/testpmd.h                 |  3 +++
> >  doc/guides/rel_notes/release_20_05.rst |  4 ++++
> >  doc/guides/testpmd_app_ug/run_app.rst  |  7 +++++++
> >  5 files changed, 40 insertions(+), 3 deletions(-)
> > 
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 30c1753c32..a9dd58e96b 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -212,6 +212,7 @@ usage(char* progname)
> >  	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
> >  	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
> >  	       "valid only with --mp-alloc=anon\n");
> > +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode\n");
> 
> Do you think does it worth to say the bitmask is for modes that can be enabled,
> to remove need to look the code, not sure.
So you mean something like:
+	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be enabled\n");
Is it right?

> 
> >  }
> >  
> >  #ifdef RTE_LIBRTE_CMDLINE
> > @@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv)
> >  		{ "noisy-lkup-num-reads",	1, 0, 0 },
> >  		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
> >  		{ "no-iova-contig",             0, 0, 0 },
> > +		{ "rx-mq-mode",                 1, 0, 0 },
> >  		{ 0, 0, 0, 0 },
> >  	};
> >  
> > @@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv)
> >  			}
> >  			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
> >  				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
> > +
> > +			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
> > +				char *end = NULL;
> > +				n = strtoul(optarg, &end, 16);
> > +				if (n >= 0)
> > +					rx_mq_mode = (enum rte_eth_rx_mq_mode)n;
> 
> Should we check if the provided value, 'n', is not out of the enum range?
OK, I'll add the check.

> 
> > +				else
> > +					rte_exit(EXIT_FAILURE,
> > +						 "rx-mq-mode must be >= 0\n");
> > +			}
> >  			break;
> >  		case 'h':
> >  			usage(argv[0]);
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> > index 99bacddbfd..9536d6ee27 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -482,6 +482,11 @@ uint8_t bitrate_enabled;
> >  struct gro_status gro_ports[RTE_MAX_ETHPORTS];
> >  uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
> >  
> > +/*
> > + * RX mq mode value set in the commandline
> 
> This is not the "RX mq mode value", the above help string seems more accurate,
> "hexadecimal bitmask of RX mq mode". Can you please update here?
Sure, I'll update.

> 
> > + */
> > +enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
> > +
> >  /* Forward function declarations */
> >  static void setup_attached_port(portid_t pi);
> >  static void map_port_queue_stats_mapping_registers(portid_t pi,
> > @@ -3337,7 +3342,9 @@ init_port_config(void)
> >  
> >  		if (port->dcb_flag == 0) {
> >  			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> > -				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
> > +				port->dev_conf.rxmode.mq_mode =
> > +					(enum rte_eth_rx_mq_mode)
> 
> Do we need this enum type cast?
Yes, we need this cast otherwise the coverity will complain about enum type
mixed with other type.

> 
> > +						(rx_mq_mode & ETH_MQ_RX_RSS);
> >  			else
> >  				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
> >  		}
> > @@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
> >  		}
> >  
> >  		/* set DCB mode of RX and TX of multiple queues */
> > -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
> > +		eth_conf->rxmode.mq_mode =
> > +				(enum rte_eth_rx_mq_mode)
> > +					(rx_mq_mode & ETH_MQ_RX_VMDQ_DCB);
> >  		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
> >  	} else {
> >  		struct rte_eth_dcb_rx_conf *rx_conf =
> > @@ -3458,7 +3467,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
> >  			tx_conf->dcb_tc[i] = i % num_tcs;
> >  		}
> >  
> > -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
> > +		eth_conf->rxmode.mq_mode =
> > +				(enum rte_eth_rx_mq_mode)
> > +					(rx_mq_mode & ETH_MQ_RX_DCB_RSS);
> >  		eth_conf->rx_adv_conf.rss_conf = rss_conf;
> >  		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
> >  	}
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> > index 7ff4c5dba3..32bb324c94 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -602,6 +602,9 @@ struct mplsoudp_decap_conf {
> >  };
> >  extern struct mplsoudp_decap_conf mplsoudp_decap_conf;
> >  
> > +/* RX mq mode parameter. */
> 
> The variable name gives as much context as the comment, may it be dropped?
OK, I'll dorp this comment.

> 
> > +extern enum rte_eth_rx_mq_mode rx_mq_mode;
> > +
> >  static inline unsigned int
> >  lcore_num(void)
> >  {
> > diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
> > index b124c3f287..8026afb451 100644
> > --- a/doc/guides/rel_notes/release_20_05.rst
> > +++ b/doc/guides/rel_notes/release_20_05.rst
> > @@ -212,6 +212,10 @@ New Features
> >    * Added IPsec inbound load-distribution support for ipsec-secgw application
> >      using NIC load distribution feature(Flow Director).
> >  
> > +* **Updated testpmd application.**
> > +
> > +  * Added a new cmdline option ``--rx-mq-mode`` which can be used to test PMD's
> > +    behaviour on handling Rx mq mode.
> >  
> >  Removed Items
> >  -------------
> > diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
> > index 727ef52b8f..4f46299e68 100644
> > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > @@ -481,3 +481,10 @@ The command line options are:
> >  
> >      Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
> >      The default value is 0.
> > +
> > +*   ``--rx-mq-mode``
> > +
> > +    Set the hexadecimal bitmask of RX queue mq mode.
> 
> It is good to expand the 'mq' at least in the documentation, and I guess it is
> "multi queue".
OK, I'll expand it.

Thank you
-Jack
> 
> > +    The default value is 0x7::
> > +
> > +       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG
> > 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode
  2020-05-06  2:52     ` Xiaoyu Min
@ 2020-05-06  9:36       ` Ferruh Yigit
  2020-05-06 16:04         ` Xiaoyu Min
  0 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2020-05-06  9:36 UTC (permalink / raw)
  To: Xiaoyu Min
  Cc: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, John McNamara,
	Marko Kovacevic, dev

On 5/6/2020 3:52 AM, Xiaoyu Min wrote:
> On Tue, 20-05-05, 17:06, Ferruh Yigit wrote:
>> On 4/30/2020 2:07 PM, Xiaoyu Min wrote:
>>> One new cmdline option `--rx-mq-mode` is added in order to have the
>>> possibility to check whether PMD handle the mq mode correctly or not.
>>>
>>> The reason is some NICs need to do different settings based on different
>>> RX mq mode, i.e RSS or not.
>>>
>>> With this support in testpmd, the above scenario can be tested easily.
>>>
>>> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
>>> ---
>>>  app/test-pmd/parameters.c              | 12 ++++++++++++
>>>  app/test-pmd/testpmd.c                 | 17 ++++++++++++++---
>>>  app/test-pmd/testpmd.h                 |  3 +++
>>>  doc/guides/rel_notes/release_20_05.rst |  4 ++++
>>>  doc/guides/testpmd_app_ug/run_app.rst  |  7 +++++++
>>>  5 files changed, 40 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>> index 30c1753c32..a9dd58e96b 100644
>>> --- a/app/test-pmd/parameters.c
>>> +++ b/app/test-pmd/parameters.c
>>> @@ -212,6 +212,7 @@ usage(char* progname)
>>>  	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
>>>  	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
>>>  	       "valid only with --mp-alloc=anon\n");
>>> +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode\n");
>>
>> Do you think does it worth to say the bitmask is for modes that can be enabled,
>> to remove need to look the code, not sure.
> So you mean something like:
> +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be enabled\n");
> Is it right?

Yes.

> 
>>
>>>  }
>>>  
>>>  #ifdef RTE_LIBRTE_CMDLINE
>>> @@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv)
>>>  		{ "noisy-lkup-num-reads",	1, 0, 0 },
>>>  		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
>>>  		{ "no-iova-contig",             0, 0, 0 },
>>> +		{ "rx-mq-mode",                 1, 0, 0 },
>>>  		{ 0, 0, 0, 0 },
>>>  	};
>>>  
>>> @@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv)
>>>  			}
>>>  			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
>>>  				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
>>> +
>>> +			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
>>> +				char *end = NULL;
>>> +				n = strtoul(optarg, &end, 16);
>>> +				if (n >= 0)
>>> +					rx_mq_mode = (enum rte_eth_rx_mq_mode)n;
>>
>> Should we check if the provided value, 'n', is not out of the enum range?
> OK, I'll add the check.
> 
>>
>>> +				else
>>> +					rte_exit(EXIT_FAILURE,
>>> +						 "rx-mq-mode must be >= 0\n");
>>> +			}
>>>  			break;
>>>  		case 'h':
>>>  			usage(argv[0]);
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>> index 99bacddbfd..9536d6ee27 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -482,6 +482,11 @@ uint8_t bitrate_enabled;
>>>  struct gro_status gro_ports[RTE_MAX_ETHPORTS];
>>>  uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
>>>  
>>> +/*
>>> + * RX mq mode value set in the commandline
>>
>> This is not the "RX mq mode value", the above help string seems more accurate,
>> "hexadecimal bitmask of RX mq mode". Can you please update here?
> Sure, I'll update.
> 
>>
>>> + */
>>> +enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
>>> +
>>>  /* Forward function declarations */
>>>  static void setup_attached_port(portid_t pi);
>>>  static void map_port_queue_stats_mapping_registers(portid_t pi,
>>> @@ -3337,7 +3342,9 @@ init_port_config(void)
>>>  
>>>  		if (port->dcb_flag == 0) {
>>>  			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
>>> -				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
>>> +				port->dev_conf.rxmode.mq_mode =
>>> +					(enum rte_eth_rx_mq_mode)
>>
>> Do we need this enum type cast?
> Yes, we need this cast otherwise the coverity will complain about enum type
> mixed with other type.

Here all variables looks an enum, 'mq_mode' type is enum, 'rx_mq_mode' is enum
and 'ETH_MQ_RX_RSS' is enum item. With which type it is mixed with?

I didn't run coverity for it, if it is giving warning OK to keep them, but can
you please double check with coverity scan?

> 
>>
>>> +						(rx_mq_mode & ETH_MQ_RX_RSS);
>>>  			else
>>>  				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
>>>  		}
>>> @@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
>>>  		}
>>>  
>>>  		/* set DCB mode of RX and TX of multiple queues */
>>> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
>>> +		eth_conf->rxmode.mq_mode =
>>> +				(enum rte_eth_rx_mq_mode)
>>> +					(rx_mq_mode & ETH_MQ_RX_VMDQ_DCB);
>>>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
>>>  	} else {
>>>  		struct rte_eth_dcb_rx_conf *rx_conf =
>>> @@ -3458,7 +3467,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
>>>  			tx_conf->dcb_tc[i] = i % num_tcs;
>>>  		}
>>>  
>>> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
>>> +		eth_conf->rxmode.mq_mode =
>>> +				(enum rte_eth_rx_mq_mode)
>>> +					(rx_mq_mode & ETH_MQ_RX_DCB_RSS);
>>>  		eth_conf->rx_adv_conf.rss_conf = rss_conf;
>>>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
>>>  	}
>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>>> index 7ff4c5dba3..32bb324c94 100644
>>> --- a/app/test-pmd/testpmd.h
>>> +++ b/app/test-pmd/testpmd.h
>>> @@ -602,6 +602,9 @@ struct mplsoudp_decap_conf {
>>>  };
>>>  extern struct mplsoudp_decap_conf mplsoudp_decap_conf;
>>>  
>>> +/* RX mq mode parameter. */
>>
>> The variable name gives as much context as the comment, may it be dropped?
> OK, I'll dorp this comment.
> 
>>
>>> +extern enum rte_eth_rx_mq_mode rx_mq_mode;
>>> +
>>>  static inline unsigned int
>>>  lcore_num(void)
>>>  {
>>> diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
>>> index b124c3f287..8026afb451 100644
>>> --- a/doc/guides/rel_notes/release_20_05.rst
>>> +++ b/doc/guides/rel_notes/release_20_05.rst
>>> @@ -212,6 +212,10 @@ New Features
>>>    * Added IPsec inbound load-distribution support for ipsec-secgw application
>>>      using NIC load distribution feature(Flow Director).
>>>  
>>> +* **Updated testpmd application.**
>>> +
>>> +  * Added a new cmdline option ``--rx-mq-mode`` which can be used to test PMD's
>>> +    behaviour on handling Rx mq mode.
>>>  
>>>  Removed Items
>>>  -------------
>>> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
>>> index 727ef52b8f..4f46299e68 100644
>>> --- a/doc/guides/testpmd_app_ug/run_app.rst
>>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
>>> @@ -481,3 +481,10 @@ The command line options are:
>>>  
>>>      Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
>>>      The default value is 0.
>>> +
>>> +*   ``--rx-mq-mode``
>>> +
>>> +    Set the hexadecimal bitmask of RX queue mq mode.
>>
>> It is good to expand the 'mq' at least in the documentation, and I guess it is
>> "multi queue".
> OK, I'll expand it.
> 
> Thank you
> -Jack
>>
>>> +    The default value is 0x7::
>>> +
>>> +       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG
>>>
>>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode
  2020-05-06  9:36       ` Ferruh Yigit
@ 2020-05-06 16:04         ` Xiaoyu Min
  2020-05-06 16:37           ` Ferruh Yigit
  0 siblings, 1 reply; 14+ messages in thread
From: Xiaoyu Min @ 2020-05-06 16:04 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, John McNamara,
	Marko Kovacevic, dev

On Wed, 20-05-06, 10:36, Ferruh Yigit wrote:
> On 5/6/2020 3:52 AM, Xiaoyu Min wrote:
> > On Tue, 20-05-05, 17:06, Ferruh Yigit wrote:
> >> On 4/30/2020 2:07 PM, Xiaoyu Min wrote:
> >>> One new cmdline option `--rx-mq-mode` is added in order to have the
> >>> possibility to check whether PMD handle the mq mode correctly or not.
> >>>
> >>> The reason is some NICs need to do different settings based on different
> >>> RX mq mode, i.e RSS or not.
> >>>
> >>> With this support in testpmd, the above scenario can be tested easily.
> >>>
> >>> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> >>> ---
> >>>  app/test-pmd/parameters.c              | 12 ++++++++++++
> >>>  app/test-pmd/testpmd.c                 | 17 ++++++++++++++---
> >>>  app/test-pmd/testpmd.h                 |  3 +++
> >>>  doc/guides/rel_notes/release_20_05.rst |  4 ++++
> >>>  doc/guides/testpmd_app_ug/run_app.rst  |  7 +++++++
> >>>  5 files changed, 40 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> >>> index 30c1753c32..a9dd58e96b 100644
> >>> --- a/app/test-pmd/parameters.c
> >>> +++ b/app/test-pmd/parameters.c
> >>> @@ -212,6 +212,7 @@ usage(char* progname)
> >>>  	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
> >>>  	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
> >>>  	       "valid only with --mp-alloc=anon\n");
> >>> +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode\n");
> >>
> >> Do you think does it worth to say the bitmask is for modes that can be enabled,
> >> to remove need to look the code, not sure.
> > So you mean something like:
> > +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be enabled\n");
> > Is it right?
> 
> Yes.
> 
> > 
> >>
> >>>  }
> >>>  
> >>>  #ifdef RTE_LIBRTE_CMDLINE
> >>> @@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv)
> >>>  		{ "noisy-lkup-num-reads",	1, 0, 0 },
> >>>  		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
> >>>  		{ "no-iova-contig",             0, 0, 0 },
> >>> +		{ "rx-mq-mode",                 1, 0, 0 },
> >>>  		{ 0, 0, 0, 0 },
> >>>  	};
> >>>  
> >>> @@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv)
> >>>  			}
> >>>  			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
> >>>  				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
> >>> +
> >>> +			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
> >>> +				char *end = NULL;
> >>> +				n = strtoul(optarg, &end, 16);
> >>> +				if (n >= 0)
> >>> +					rx_mq_mode = (enum rte_eth_rx_mq_mode)n;
> >>
> >> Should we check if the provided value, 'n', is not out of the enum range?
> > OK, I'll add the check.
> > 
> >>
> >>> +				else
> >>> +					rte_exit(EXIT_FAILURE,
> >>> +						 "rx-mq-mode must be >= 0\n");
> >>> +			}
> >>>  			break;
> >>>  		case 'h':
> >>>  			usage(argv[0]);
> >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> >>> index 99bacddbfd..9536d6ee27 100644
> >>> --- a/app/test-pmd/testpmd.c
> >>> +++ b/app/test-pmd/testpmd.c
> >>> @@ -482,6 +482,11 @@ uint8_t bitrate_enabled;
> >>>  struct gro_status gro_ports[RTE_MAX_ETHPORTS];
> >>>  uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
> >>>  
> >>> +/*
> >>> + * RX mq mode value set in the commandline
> >>
> >> This is not the "RX mq mode value", the above help string seems more accurate,
> >> "hexadecimal bitmask of RX mq mode". Can you please update here?
> > Sure, I'll update.
> > 
> >>
> >>> + */
> >>> +enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
> >>> +
> >>>  /* Forward function declarations */
> >>>  static void setup_attached_port(portid_t pi);
> >>>  static void map_port_queue_stats_mapping_registers(portid_t pi,
> >>> @@ -3337,7 +3342,9 @@ init_port_config(void)
> >>>  
> >>>  		if (port->dcb_flag == 0) {
> >>>  			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> >>> -				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
> >>> +				port->dev_conf.rxmode.mq_mode =
> >>> +					(enum rte_eth_rx_mq_mode)
> >>
> >> Do we need this enum type cast?
> > Yes, we need this cast otherwise the coverity will complain about enum type
> > mixed with other type.
> 
> Here all variables looks an enum, 'mq_mode' type is enum, 'rx_mq_mode' is enum
> and 'ETH_MQ_RX_RSS' is enum item. With which type it is mixed with?
> 
> I didn't run coverity for it, if it is giving warning OK to keep them, but can
> you please double check with coverity scan?
> 
As you asked, I double checked with coverity by removing the enum cast. The coverity
has the below warning [1] but this time the warning is supressed by filter(I guess
it's newly added). It seems coverity doesn't allow the bitwise operators(?)
Anyhow I prefer to keep the cast this time since it's already there.
What do you think?

[1]:
[2020-05-06T10:31:01.899Z] Detected 1 defect occurrence that passes the filter criteria.
[2020-05-06T10:31:01.899Z] There are 267 suppressed defects due to filters.
[2020-05-06T10:31:01.900Z] 
[2020-05-06T10:31:01.900Z] app/test-pmd/testpmd.c:3345: CID 154009 (#1 of 1):
[2020-05-06T10:31:01.901Z]   Type: Parse warning (PW.MIXED_ENUM_TYPE)
[2020-05-06T10:31:01.901Z]   Classification: Unclassified
[2020-05-06T10:31:01.901Z]   Severity: Unspecified
[2020-05-06T10:31:01.902Z]   Action: Undecided
[2020-05-06T10:31:01.902Z]   Owner: Unassigned
[2020-05-06T10:31:01.903Z]   Defect only exists locally.
[2020-05-06T10:31:01.903Z] app/test-pmd/testpmd.c:3345:
[2020-05-06T10:31:01.903Z]   1. mixed_enum_type: enumerated type mixed with another type
> > 
> >>
> >>> +						(rx_mq_mode & ETH_MQ_RX_RSS);
> >>>  			else
> >>>  				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
> >>>  		}
> >>> @@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
> >>>  		}
> >>>  
> >>>  		/* set DCB mode of RX and TX of multiple queues */
> >>> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
> >>> +		eth_conf->rxmode.mq_mode =
> >>> +				(enum rte_eth_rx_mq_mode)
> >>> +					(rx_mq_mode & ETH_MQ_RX_VMDQ_DCB);
> >>>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
> >>>  	} else {
> >>>  		struct rte_eth_dcb_rx_conf *rx_conf =
> >>> @@ -3458,7 +3467,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
> >>>  			tx_conf->dcb_tc[i] = i % num_tcs;
> >>>  		}
> >>>  
> >>> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
> >>> +		eth_conf->rxmode.mq_mode =
> >>> +				(enum rte_eth_rx_mq_mode)
> >>> +					(rx_mq_mode & ETH_MQ_RX_DCB_RSS);
> >>>  		eth_conf->rx_adv_conf.rss_conf = rss_conf;
> >>>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
> >>>  	}
> >>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> >>> index 7ff4c5dba3..32bb324c94 100644
> >>> --- a/app/test-pmd/testpmd.h
> >>> +++ b/app/test-pmd/testpmd.h
> >>> @@ -602,6 +602,9 @@ struct mplsoudp_decap_conf {
> >>>  };
> >>>  extern struct mplsoudp_decap_conf mplsoudp_decap_conf;
> >>>  
> >>> +/* RX mq mode parameter. */
> >>
> >> The variable name gives as much context as the comment, may it be dropped?
> > OK, I'll dorp this comment.
> > 
> >>
> >>> +extern enum rte_eth_rx_mq_mode rx_mq_mode;
> >>> +
> >>>  static inline unsigned int
> >>>  lcore_num(void)
> >>>  {
> >>> diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
> >>> index b124c3f287..8026afb451 100644
> >>> --- a/doc/guides/rel_notes/release_20_05.rst
> >>> +++ b/doc/guides/rel_notes/release_20_05.rst
> >>> @@ -212,6 +212,10 @@ New Features
> >>>    * Added IPsec inbound load-distribution support for ipsec-secgw application
> >>>      using NIC load distribution feature(Flow Director).
> >>>  
> >>> +* **Updated testpmd application.**
> >>> +
> >>> +  * Added a new cmdline option ``--rx-mq-mode`` which can be used to test PMD's
> >>> +    behaviour on handling Rx mq mode.
> >>>  
> >>>  Removed Items
> >>>  -------------
> >>> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
> >>> index 727ef52b8f..4f46299e68 100644
> >>> --- a/doc/guides/testpmd_app_ug/run_app.rst
> >>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> >>> @@ -481,3 +481,10 @@ The command line options are:
> >>>  
> >>>      Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
> >>>      The default value is 0.
> >>> +
> >>> +*   ``--rx-mq-mode``
> >>> +
> >>> +    Set the hexadecimal bitmask of RX queue mq mode.
> >>
> >> It is good to expand the 'mq' at least in the documentation, and I guess it is
> >> "multi queue".
> > OK, I'll expand it.
> > 
> > Thank you
> > -Jack
> >>
> >>> +    The default value is 0x7::
> >>> +
> >>> +       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG
> >>>
> >>
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode
  2020-05-06 16:04         ` Xiaoyu Min
@ 2020-05-06 16:37           ` Ferruh Yigit
  0 siblings, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2020-05-06 16:37 UTC (permalink / raw)
  To: Xiaoyu Min
  Cc: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, John McNamara,
	Marko Kovacevic, dev

On 5/6/2020 5:04 PM, Xiaoyu Min wrote:
> On Wed, 20-05-06, 10:36, Ferruh Yigit wrote:
>> On 5/6/2020 3:52 AM, Xiaoyu Min wrote:
>>> On Tue, 20-05-05, 17:06, Ferruh Yigit wrote:
>>>> On 4/30/2020 2:07 PM, Xiaoyu Min wrote:
>>>>> One new cmdline option `--rx-mq-mode` is added in order to have the
>>>>> possibility to check whether PMD handle the mq mode correctly or not.
>>>>>
>>>>> The reason is some NICs need to do different settings based on different
>>>>> RX mq mode, i.e RSS or not.
>>>>>
>>>>> With this support in testpmd, the above scenario can be tested easily.
>>>>>
>>>>> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
>>>>> ---
>>>>>  app/test-pmd/parameters.c              | 12 ++++++++++++
>>>>>  app/test-pmd/testpmd.c                 | 17 ++++++++++++++---
>>>>>  app/test-pmd/testpmd.h                 |  3 +++
>>>>>  doc/guides/rel_notes/release_20_05.rst |  4 ++++
>>>>>  doc/guides/testpmd_app_ug/run_app.rst  |  7 +++++++
>>>>>  5 files changed, 40 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>>>> index 30c1753c32..a9dd58e96b 100644
>>>>> --- a/app/test-pmd/parameters.c
>>>>> +++ b/app/test-pmd/parameters.c
>>>>> @@ -212,6 +212,7 @@ usage(char* progname)
>>>>>  	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
>>>>>  	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
>>>>>  	       "valid only with --mp-alloc=anon\n");
>>>>> +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode\n");
>>>>
>>>> Do you think does it worth to say the bitmask is for modes that can be enabled,
>>>> to remove need to look the code, not sure.
>>> So you mean something like:
>>> +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be enabled\n");
>>> Is it right?
>>
>> Yes.
>>
>>>
>>>>
>>>>>  }
>>>>>  
>>>>>  #ifdef RTE_LIBRTE_CMDLINE
>>>>> @@ -670,6 +671,7 @@ launch_args_parse(int argc, char** argv)
>>>>>  		{ "noisy-lkup-num-reads",	1, 0, 0 },
>>>>>  		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
>>>>>  		{ "no-iova-contig",             0, 0, 0 },
>>>>> +		{ "rx-mq-mode",                 1, 0, 0 },
>>>>>  		{ 0, 0, 0, 0 },
>>>>>  	};
>>>>>  
>>>>> @@ -1363,6 +1365,16 @@ launch_args_parse(int argc, char** argv)
>>>>>  			}
>>>>>  			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
>>>>>  				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
>>>>> +
>>>>> +			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
>>>>> +				char *end = NULL;
>>>>> +				n = strtoul(optarg, &end, 16);
>>>>> +				if (n >= 0)
>>>>> +					rx_mq_mode = (enum rte_eth_rx_mq_mode)n;
>>>>
>>>> Should we check if the provided value, 'n', is not out of the enum range?
>>> OK, I'll add the check.
>>>
>>>>
>>>>> +				else
>>>>> +					rte_exit(EXIT_FAILURE,
>>>>> +						 "rx-mq-mode must be >= 0\n");
>>>>> +			}
>>>>>  			break;
>>>>>  		case 'h':
>>>>>  			usage(argv[0]);
>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>>>> index 99bacddbfd..9536d6ee27 100644
>>>>> --- a/app/test-pmd/testpmd.c
>>>>> +++ b/app/test-pmd/testpmd.c
>>>>> @@ -482,6 +482,11 @@ uint8_t bitrate_enabled;
>>>>>  struct gro_status gro_ports[RTE_MAX_ETHPORTS];
>>>>>  uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
>>>>>  
>>>>> +/*
>>>>> + * RX mq mode value set in the commandline
>>>>
>>>> This is not the "RX mq mode value", the above help string seems more accurate,
>>>> "hexadecimal bitmask of RX mq mode". Can you please update here?
>>> Sure, I'll update.
>>>
>>>>
>>>>> + */
>>>>> +enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
>>>>> +
>>>>>  /* Forward function declarations */
>>>>>  static void setup_attached_port(portid_t pi);
>>>>>  static void map_port_queue_stats_mapping_registers(portid_t pi,
>>>>> @@ -3337,7 +3342,9 @@ init_port_config(void)
>>>>>  
>>>>>  		if (port->dcb_flag == 0) {
>>>>>  			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
>>>>> -				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
>>>>> +				port->dev_conf.rxmode.mq_mode =
>>>>> +					(enum rte_eth_rx_mq_mode)
>>>>
>>>> Do we need this enum type cast?
>>> Yes, we need this cast otherwise the coverity will complain about enum type
>>> mixed with other type.
>>
>> Here all variables looks an enum, 'mq_mode' type is enum, 'rx_mq_mode' is enum
>> and 'ETH_MQ_RX_RSS' is enum item. With which type it is mixed with?
>>
>> I didn't run coverity for it, if it is giving warning OK to keep them, but can
>> you please double check with coverity scan?
>>
> As you asked, I double checked with coverity by removing the enum cast. The coverity
> has the below warning [1] but this time the warning is supressed by filter(I guess
> it's newly added). It seems coverity doesn't allow the bitwise operators(?)
> Anyhow I prefer to keep the cast this time since it's already there.
> What do you think?

OK, thanks for re-checking.

> 
> [1]:
> [2020-05-06T10:31:01.899Z] Detected 1 defect occurrence that passes the filter criteria.
> [2020-05-06T10:31:01.899Z] There are 267 suppressed defects due to filters.
> [2020-05-06T10:31:01.900Z] 
> [2020-05-06T10:31:01.900Z] app/test-pmd/testpmd.c:3345: CID 154009 (#1 of 1):
> [2020-05-06T10:31:01.901Z]   Type: Parse warning (PW.MIXED_ENUM_TYPE)
> [2020-05-06T10:31:01.901Z]   Classification: Unclassified
> [2020-05-06T10:31:01.901Z]   Severity: Unspecified
> [2020-05-06T10:31:01.902Z]   Action: Undecided
> [2020-05-06T10:31:01.902Z]   Owner: Unassigned
> [2020-05-06T10:31:01.903Z]   Defect only exists locally.
> [2020-05-06T10:31:01.903Z] app/test-pmd/testpmd.c:3345:
> [2020-05-06T10:31:01.903Z]   1. mixed_enum_type: enumerated type mixed with another type
>>>
>>>>
>>>>> +						(rx_mq_mode & ETH_MQ_RX_RSS);
>>>>>  			else
>>>>>  				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
>>>>>  		}
>>>>> @@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
>>>>>  		}
>>>>>  
>>>>>  		/* set DCB mode of RX and TX of multiple queues */
>>>>> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
>>>>> +		eth_conf->rxmode.mq_mode =
>>>>> +				(enum rte_eth_rx_mq_mode)
>>>>> +					(rx_mq_mode & ETH_MQ_RX_VMDQ_DCB);
>>>>>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
>>>>>  	} else {
>>>>>  		struct rte_eth_dcb_rx_conf *rx_conf =
>>>>> @@ -3458,7 +3467,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
>>>>>  			tx_conf->dcb_tc[i] = i % num_tcs;
>>>>>  		}
>>>>>  
>>>>> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
>>>>> +		eth_conf->rxmode.mq_mode =
>>>>> +				(enum rte_eth_rx_mq_mode)
>>>>> +					(rx_mq_mode & ETH_MQ_RX_DCB_RSS);
>>>>>  		eth_conf->rx_adv_conf.rss_conf = rss_conf;
>>>>>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
>>>>>  	}
>>>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>>>>> index 7ff4c5dba3..32bb324c94 100644
>>>>> --- a/app/test-pmd/testpmd.h
>>>>> +++ b/app/test-pmd/testpmd.h
>>>>> @@ -602,6 +602,9 @@ struct mplsoudp_decap_conf {
>>>>>  };
>>>>>  extern struct mplsoudp_decap_conf mplsoudp_decap_conf;
>>>>>  
>>>>> +/* RX mq mode parameter. */
>>>>
>>>> The variable name gives as much context as the comment, may it be dropped?
>>> OK, I'll dorp this comment.
>>>
>>>>
>>>>> +extern enum rte_eth_rx_mq_mode rx_mq_mode;
>>>>> +
>>>>>  static inline unsigned int
>>>>>  lcore_num(void)
>>>>>  {
>>>>> diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
>>>>> index b124c3f287..8026afb451 100644
>>>>> --- a/doc/guides/rel_notes/release_20_05.rst
>>>>> +++ b/doc/guides/rel_notes/release_20_05.rst
>>>>> @@ -212,6 +212,10 @@ New Features
>>>>>    * Added IPsec inbound load-distribution support for ipsec-secgw application
>>>>>      using NIC load distribution feature(Flow Director).
>>>>>  
>>>>> +* **Updated testpmd application.**
>>>>> +
>>>>> +  * Added a new cmdline option ``--rx-mq-mode`` which can be used to test PMD's
>>>>> +    behaviour on handling Rx mq mode.
>>>>>  
>>>>>  Removed Items
>>>>>  -------------
>>>>> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
>>>>> index 727ef52b8f..4f46299e68 100644
>>>>> --- a/doc/guides/testpmd_app_ug/run_app.rst
>>>>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
>>>>> @@ -481,3 +481,10 @@ The command line options are:
>>>>>  
>>>>>      Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
>>>>>      The default value is 0.
>>>>> +
>>>>> +*   ``--rx-mq-mode``
>>>>> +
>>>>> +    Set the hexadecimal bitmask of RX queue mq mode.
>>>>
>>>> It is good to expand the 'mq' at least in the documentation, and I guess it is
>>>> "multi queue".
>>> OK, I'll expand it.
>>>
>>> Thank you
>>> -Jack
>>>>
>>>>> +    The default value is 0x7::
>>>>> +
>>>>> +       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG
>>>>>
>>>>
>>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v3] app/testpmd: add cmdline option to set Rx mq mode
  2020-04-29 13:04 [dpdk-dev] [PATCH] app/testpmd: add cmdline option to set Rx mq mode Xiaoyu Min
  2020-04-30  9:16 ` Iremonger, Bernard
  2020-04-30 13:07 ` [dpdk-dev] [PATCH v2] " Xiaoyu Min
@ 2020-05-07  0:51 ` Xiaoyu Min
  2020-05-07 10:23   ` Iremonger, Bernard
  2 siblings, 1 reply; 14+ messages in thread
From: Xiaoyu Min @ 2020-05-07  0:51 UTC (permalink / raw)
  To: ferruh.yigit, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger,
	John McNamara, Marko Kovacevic
  Cc: dev

One new cmdline option `--rx-mq-mode` is added in order to have the
possibility to check whether PMD handle the mq mode correctly or not.

The reason is some NICs need to do different settings based on different
RX mq mode, i.e RSS or not.

With this support in testpmd, the above scenario can be tested easily.

Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
 app/test-pmd/parameters.c              | 15 +++++++++++++++
 app/test-pmd/testpmd.c                 | 17 ++++++++++++++---
 app/test-pmd/testpmd.h                 |  2 ++
 doc/guides/rel_notes/release_20_05.rst |  4 ++++
 doc/guides/testpmd_app_ug/run_app.rst  |  7 +++++++
 5 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 30c1753c32..9f80fe898f 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -212,6 +212,8 @@ usage(char* progname)
 	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
 	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
 	       "valid only with --mp-alloc=anon\n");
+	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be "
+	       "enabled\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -670,6 +672,7 @@ launch_args_parse(int argc, char** argv)
 		{ "noisy-lkup-num-reads",	1, 0, 0 },
 		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
 		{ "no-iova-contig",             0, 0, 0 },
+		{ "rx-mq-mode",                 1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1363,6 +1366,18 @@ launch_args_parse(int argc, char** argv)
 			}
 			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
 				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
+
+			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
+				char *end = NULL;
+				n = strtoul(optarg, &end, 16);
+				if (n >= 0 && n <= ETH_MQ_RX_VMDQ_DCB_RSS)
+					rx_mq_mode = (enum rte_eth_rx_mq_mode)n;
+				else
+					rte_exit(EXIT_FAILURE,
+						 "rx-mq-mode must be >= 0 and "
+						 " <= %d\n",
+						 ETH_MQ_RX_VMDQ_DCB_RSS);
+			}
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfd..808cfae463 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -482,6 +482,11 @@ uint8_t bitrate_enabled;
 struct gro_status gro_ports[RTE_MAX_ETHPORTS];
 uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
 
+/*
+ * hexadecimal bitmask of RX mq mode can be enabled.
+ */
+enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
+
 /* Forward function declarations */
 static void setup_attached_port(portid_t pi);
 static void map_port_queue_stats_mapping_registers(portid_t pi,
@@ -3337,7 +3342,9 @@ init_port_config(void)
 
 		if (port->dcb_flag == 0) {
 			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
-				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_RSS;
+				port->dev_conf.rxmode.mq_mode =
+					(enum rte_eth_rx_mq_mode)
+						(rx_mq_mode & ETH_MQ_RX_RSS);
 			else
 				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
 		}
@@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
 		}
 
 		/* set DCB mode of RX and TX of multiple queues */
-		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
+		eth_conf->rxmode.mq_mode =
+				(enum rte_eth_rx_mq_mode)
+					(rx_mq_mode & ETH_MQ_RX_VMDQ_DCB);
 		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
 	} else {
 		struct rte_eth_dcb_rx_conf *rx_conf =
@@ -3458,7 +3467,9 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
 			tx_conf->dcb_tc[i] = i % num_tcs;
 		}
 
-		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
+		eth_conf->rxmode.mq_mode =
+				(enum rte_eth_rx_mq_mode)
+					(rx_mq_mode & ETH_MQ_RX_DCB_RSS);
 		eth_conf->rx_adv_conf.rss_conf = rss_conf;
 		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
 	}
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7ff4c5dba3..e47d1a40f4 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -602,6 +602,8 @@ struct mplsoudp_decap_conf {
 };
 extern struct mplsoudp_decap_conf mplsoudp_decap_conf;
 
+extern enum rte_eth_rx_mq_mode rx_mq_mode;
+
 static inline unsigned int
 lcore_num(void)
 {
diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
index 777d36e582..c94d84a14c 100644
--- a/doc/guides/rel_notes/release_20_05.rst
+++ b/doc/guides/rel_notes/release_20_05.rst
@@ -216,6 +216,10 @@ New Features
   * Added IPsec inbound load-distribution support for ipsec-secgw application
     using NIC load distribution feature(Flow Director).
 
+* **Updated testpmd application.**
+
+  * Added a new cmdline option ``--rx-mq-mode`` which can be used to test PMD's
+    behaviour on handling Rx mq mode.
 
 Removed Items
 -------------
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 727ef52b8f..d586f2e35d 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -481,3 +481,10 @@ The command line options are:
 
     Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
     The default value is 0.
+
+*   ``--rx-mq-mode``
+
+    Set the hexadecimal bitmask of RX multi queue mode which can be enabled.
+    The default value is 0x7::
+
+       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG
-- 
2.26.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v3] app/testpmd: add cmdline option to set Rx mq mode
  2020-05-07  0:51 ` [dpdk-dev] [PATCH v3] " Xiaoyu Min
@ 2020-05-07 10:23   ` Iremonger, Bernard
  2020-05-07 11:56     ` Ferruh Yigit
  2020-05-07 11:56     ` Ferruh Yigit
  0 siblings, 2 replies; 14+ messages in thread
From: Iremonger, Bernard @ 2020-05-07 10:23 UTC (permalink / raw)
  To: Xiaoyu Min, Yigit, Ferruh, Lu, Wenzhuo, Wu, Jingjing, Mcnamara,
	John, Kovacevic, Marko
  Cc: dev


Hi Xiaoyu,

> -----Original Message-----
> From: Xiaoyu Min <jackmin@mellanox.com>
> Sent: Thursday, May 7, 2020 1:52 AM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger,
> Bernard <bernard.iremonger@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v3] app/testpmd: add cmdline option to set Rx mq mode
> 
> One new cmdline option `--rx-mq-mode` is added in order to have the
> possibility to check whether PMD handle the mq mode correctly or not.
> 
> The reason is some NICs need to do different settings based on different RX
> mq mode, i.e RSS or not.
> 
> With this support in testpmd, the above scenario can be tested easily.
> 
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
>  app/test-pmd/parameters.c              | 15 +++++++++++++++
>  app/test-pmd/testpmd.c                 | 17 ++++++++++++++---
>  app/test-pmd/testpmd.h                 |  2 ++
>  doc/guides/rel_notes/release_20_05.rst |  4 ++++
> doc/guides/testpmd_app_ug/run_app.rst  |  7 +++++++
>  5 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index
> 30c1753c32..9f80fe898f 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -212,6 +212,8 @@ usage(char* progname)
>  	printf("  --noisy-lkup-num-writes=N: do N random reads and writes
> per packet\n");
>  	printf("  --no-iova-contig: mempool memory can be IOVA non
> contiguous. "
>  	       "valid only with --mp-alloc=anon\n");
> +	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode
> can be "
> +	       "enabled\n");
>  }
> 
>  #ifdef RTE_LIBRTE_CMDLINE
> @@ -670,6 +672,7 @@ launch_args_parse(int argc, char** argv)
>  		{ "noisy-lkup-num-reads",	1, 0, 0 },
>  		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
>  		{ "no-iova-contig",             0, 0, 0 },
> +		{ "rx-mq-mode",                 1, 0, 0 },
>  		{ 0, 0, 0, 0 },
>  	};
> 
> @@ -1363,6 +1366,18 @@ launch_args_parse(int argc, char** argv)
>  			}
>  			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
>  				mempool_flags =
> MEMPOOL_F_NO_IOVA_CONTIG;
> +
> +			if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
> +				char *end = NULL;
> +				n = strtoul(optarg, &end, 16);
> +				if (n >= 0 && n <=
> ETH_MQ_RX_VMDQ_DCB_RSS)
> +					rx_mq_mode = (enum
> rte_eth_rx_mq_mode)n;
> +				else
> +					rte_exit(EXIT_FAILURE,
> +						 "rx-mq-mode must be >= 0
> and "
> +						 " <= %d\n",
> +

Better not to split error string over two lines (passes checkpatch for me)

> ETH_MQ_RX_VMDQ_DCB_RSS);
> +			}
>  			break;
>  		case 'h':
>  			usage(argv[0]);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 99bacddbfd..808cfae463 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -482,6 +482,11 @@ uint8_t bitrate_enabled;  struct gro_status
> gro_ports[RTE_MAX_ETHPORTS];  uint8_t gro_flush_cycles =
> GRO_DEFAULT_FLUSH_CYCLES;
> 
> +/*
> + * hexadecimal bitmask of RX mq mode can be enabled.
> + */
> +enum rte_eth_rx_mq_mode rx_mq_mode =
> ETH_MQ_RX_VMDQ_DCB_RSS;
> +
>  /* Forward function declarations */
>  static void setup_attached_port(portid_t pi);  static void
> map_port_queue_stats_mapping_registers(portid_t pi, @@ -3337,7 +3342,9
> @@ init_port_config(void)
> 
>  		if (port->dcb_flag == 0) {
>  			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
> -				port->dev_conf.rxmode.mq_mode =
> ETH_MQ_RX_RSS;
> +				port->dev_conf.rxmode.mq_mode =
> +					(enum rte_eth_rx_mq_mode)
> +						(rx_mq_mode &
> ETH_MQ_RX_RSS);
>  			else
>  				port->dev_conf.rxmode.mq_mode =
> ETH_MQ_RX_NONE;
>  		}
> @@ -3438,7 +3445,9 @@ get_eth_dcb_conf(portid_t pid, struct
> rte_eth_conf *eth_conf,
>  		}
> 
>  		/* set DCB mode of RX and TX of multiple queues */
> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_VMDQ_DCB;
> +		eth_conf->rxmode.mq_mode =
> +				(enum rte_eth_rx_mq_mode)
> +					(rx_mq_mode &
> ETH_MQ_RX_VMDQ_DCB);
>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_VMDQ_DCB;
>  	} else {
>  		struct rte_eth_dcb_rx_conf *rx_conf = @@ -3458,7 +3467,9
> @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
>  			tx_conf->dcb_tc[i] = i % num_tcs;
>  		}
> 
> -		eth_conf->rxmode.mq_mode = ETH_MQ_RX_DCB_RSS;
> +		eth_conf->rxmode.mq_mode =
> +				(enum rte_eth_rx_mq_mode)
> +					(rx_mq_mode &
> ETH_MQ_RX_DCB_RSS);
>  		eth_conf->rx_adv_conf.rss_conf = rss_conf;
>  		eth_conf->txmode.mq_mode = ETH_MQ_TX_DCB;
>  	}
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 7ff4c5dba3..e47d1a40f4 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -602,6 +602,8 @@ struct mplsoudp_decap_conf {  };  extern struct
> mplsoudp_decap_conf mplsoudp_decap_conf;
> 
> +extern enum rte_eth_rx_mq_mode rx_mq_mode;
> +
>  static inline unsigned int
>  lcore_num(void)
>  {
> diff --git a/doc/guides/rel_notes/release_20_05.rst
> b/doc/guides/rel_notes/release_20_05.rst
> index 777d36e582..c94d84a14c 100644
> --- a/doc/guides/rel_notes/release_20_05.rst
> +++ b/doc/guides/rel_notes/release_20_05.rst
> @@ -216,6 +216,10 @@ New Features
>    * Added IPsec inbound load-distribution support for ipsec-secgw
> application
>      using NIC load distribution feature(Flow Director).
> 
> +* **Updated testpmd application.**
> +
> +  * Added a new cmdline option ``--rx-mq-mode`` which can be used to test
> PMD's
> +    behaviour on handling Rx mq mode.
> 
>  Removed Items
>  -------------
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst
> b/doc/guides/testpmd_app_ug/run_app.rst
> index 727ef52b8f..d586f2e35d 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -481,3 +481,10 @@ The command line options are:
> 
>      Enable to create mempool which is not IOVA contiguous. Valid only with --
> mp-alloc=anon.
>      The default value is 0.
> +
> +*   ``--rx-mq-mode``
> +
> +    Set the hexadecimal bitmask of RX multi queue mode which can be
> enabled.
> +    The default value is 0x7::
> +
> +       ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG |
> ETH_MQ_RX_VMDQ_FLAG
> --
> 2.26.2

Otherwise

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v3] app/testpmd: add cmdline option to set Rx mq mode
  2020-05-07 10:23   ` Iremonger, Bernard
@ 2020-05-07 11:56     ` Ferruh Yigit
  2020-05-07 11:56     ` Ferruh Yigit
  1 sibling, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2020-05-07 11:56 UTC (permalink / raw)
  To: Iremonger, Bernard, Xiaoyu Min, Lu, Wenzhuo, Wu, Jingjing,
	Mcnamara, John, Kovacevic, Marko
  Cc: dev

On 5/7/2020 11:23 AM, Iremonger, Bernard wrote:
> 
> Hi Xiaoyu,
> 
>> -----Original Message-----
>> From: Xiaoyu Min <jackmin@mellanox.com>
>> Sent: Thursday, May 7, 2020 1:52 AM
>> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger,
>> Bernard <bernard.iremonger@intel.com>; Mcnamara, John
>> <john.mcnamara@intel.com>; Kovacevic, Marko
>> <marko.kovacevic@intel.com>
>> Cc: dev@dpdk.org
>> Subject: [PATCH v3] app/testpmd: add cmdline option to set Rx mq mode
>>
>> One new cmdline option `--rx-mq-mode` is added in order to have the
>> possibility to check whether PMD handle the mq mode correctly or not.
>>
>> The reason is some NICs need to do different settings based on different RX
>> mq mode, i.e RSS or not.
>>
>> With this support in testpmd, the above scenario can be tested easily.
>>
>> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>

<...>

>> @@ -1363,6 +1366,18 @@ launch_args_parse(int argc, char** argv)
>>  }
>>  if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
>>  mempool_flags =
>> MEMPOOL_F_NO_IOVA_CONTIG;
>> +
>> +if (!strcmp(lgopts[opt_idx].name, "rx-mq-mode")) {
>> +char *end = NULL;
>> +n = strtoul(optarg, &end, 16);
>> +if (n >= 0 && n <=
>> ETH_MQ_RX_VMDQ_DCB_RSS)
>> +rx_mq_mode = (enum
>> rte_eth_rx_mq_mode)n;
>> +else
>> +rte_exit(EXIT_FAILURE,
>> + "rx-mq-mode must be >= 0
>> and "
>> + " <= %d\n",
>> +
> 
> Better not to split error string over two lines (passes checkpatch for me)
> 

+1, will fix while merging.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v3] app/testpmd: add cmdline option to set Rx mq mode
  2020-05-07 10:23   ` Iremonger, Bernard
  2020-05-07 11:56     ` Ferruh Yigit
@ 2020-05-07 11:56     ` Ferruh Yigit
  1 sibling, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2020-05-07 11:56 UTC (permalink / raw)
  To: Iremonger, Bernard, Xiaoyu Min, Lu, Wenzhuo, Wu, Jingjing,
	Mcnamara, John, Kovacevic, Marko
  Cc: dev

On 5/7/2020 11:23 AM, Iremonger, Bernard wrote:
> 
> Hi Xiaoyu,
> 
>> -----Original Message-----
>> From: Xiaoyu Min <jackmin@mellanox.com>
>> Sent: Thursday, May 7, 2020 1:52 AM
>> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger,
>> Bernard <bernard.iremonger@intel.com>; Mcnamara, John
>> <john.mcnamara@intel.com>; Kovacevic, Marko
>> <marko.kovacevic@intel.com>
>> Cc: dev@dpdk.org
>> Subject: [PATCH v3] app/testpmd: add cmdline option to set Rx mq mode
>>
>> One new cmdline option `--rx-mq-mode` is added in order to have the
>> possibility to check whether PMD handle the mq mode correctly or not.
>>
>> The reason is some NICs need to do different settings based on different RX
>> mq mode, i.e RSS or not.
>>
>> With this support in testpmd, the above scenario can be tested easily.
>>
>> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> 
> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-05-07 11:56 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29 13:04 [dpdk-dev] [PATCH] app/testpmd: add cmdline option to set Rx mq mode Xiaoyu Min
2020-04-30  9:16 ` Iremonger, Bernard
2020-04-30 12:13   ` Xiaoyu Min
2020-04-30 13:07 ` [dpdk-dev] [PATCH v2] " Xiaoyu Min
2020-04-30 13:33   ` Iremonger, Bernard
2020-05-05 16:06   ` Ferruh Yigit
2020-05-06  2:52     ` Xiaoyu Min
2020-05-06  9:36       ` Ferruh Yigit
2020-05-06 16:04         ` Xiaoyu Min
2020-05-06 16:37           ` Ferruh Yigit
2020-05-07  0:51 ` [dpdk-dev] [PATCH v3] " Xiaoyu Min
2020-05-07 10:23   ` Iremonger, Bernard
2020-05-07 11:56     ` Ferruh Yigit
2020-05-07 11:56     ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).