DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Xiaoyu Min <jackmin@mellanox.com>,
	Wenzhuo Lu <wenzhuo.lu@intel.com>,
	Jingjing Wu <jingjing.wu@intel.com>,
	Bernard Iremonger <bernard.iremonger@intel.com>,
	John McNamara <john.mcnamara@intel.com>,
	Marko Kovacevic <marko.kovacevic@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: add cmdline option to set Rx mq mode
Date: Tue, 5 May 2020 17:06:30 +0100	[thread overview]
Message-ID: <8a562aef-6d7b-4713-7fb9-e4da12d6bf56@intel.com> (raw)
In-Reply-To: <127483b3dbf4840a80c6177a6230d1f3e8f40fb2.1588251701.git.jackmin@mellanox.com>

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
> 


  parent reply	other threads:[~2020-05-05 16:06 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-29 13:04 [dpdk-dev] [PATCH] " 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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8a562aef-6d7b-4713-7fb9-e4da12d6bf56@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=jackmin@mellanox.com \
    --cc=jingjing.wu@intel.com \
    --cc=john.mcnamara@intel.com \
    --cc=marko.kovacevic@intel.com \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).