Re: [dpdk-dev] [PATCH v11 6/7] app/testpmd: force shared Rx queue polled on same core

["Li, Xiaoyun" <xiaoyun.li@intel.com>]

Hi

> -----Original Message-----
> From: Xueming Li <xuemingl@nvidia.com>
> Sent: Wednesday, October 20, 2021 15:53
> To: dev@dpdk.org; Zhang, Yuying <yuying.zhang@intel.com>
> Cc: xuemingl@nvidia.com; Jerin Jacob <jerinjacobk@gmail.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Viacheslav Ovsiienko
> <viacheslavo@nvidia.com>; Thomas Monjalon <thomas@monjalon.net>; Lior
> Margalit <lmargalit@nvidia.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Li, Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH v11 6/7] app/testpmd: force shared Rx queue polled on same
> core
> 
> Shared Rx queue must be polled on same core. This patch checks and stops
> forwarding if shared RxQ being scheduled on multiple cores.
> 
> It's suggested to use same number of Rx queues and polling cores.
> 
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> ---
>  app/test-pmd/config.c  | 103
> +++++++++++++++++++++++++++++++++++++++++
>  app/test-pmd/testpmd.c |   4 +-
>  app/test-pmd/testpmd.h |   2 +
>  3 files changed, 108 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> fa951a86704..1f1307178be 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2915,6 +2915,109 @@ port_rss_hash_key_update(portid_t port_id, char
> rss_type[], uint8_t *hash_key,
>  	}
>  }
> 
> +/*
> + * Check whether a shared rxq scheduled on other lcores.
> + */
> +static bool
> +fwd_stream_on_other_lcores(uint16_t domain_id, lcoreid_t src_lc,
> +			   portid_t src_port, queueid_t src_rxq,
> +			   uint32_t share_group, queueid_t share_rxq) {
> +	streamid_t sm_id;
> +	streamid_t nb_fs_per_lcore;
> +	lcoreid_t  nb_fc;
> +	lcoreid_t  lc_id;
> +	struct fwd_stream *fs;
> +	struct rte_port *port;
> +	struct rte_eth_dev_info *dev_info;
> +	struct rte_eth_rxconf *rxq_conf;
> +
> +	nb_fc = cur_fwd_config.nb_fwd_lcores;
> +	/* Check remaining cores. */
> +	for (lc_id = src_lc + 1; lc_id < nb_fc; lc_id++) {
> +		sm_id = fwd_lcores[lc_id]->stream_idx;
> +		nb_fs_per_lcore = fwd_lcores[lc_id]->stream_nb;
> +		for (; sm_id < fwd_lcores[lc_id]->stream_idx + nb_fs_per_lcore;
> +		     sm_id++) {
> +			fs = fwd_streams[sm_id];
> +			port = &ports[fs->rx_port];
> +			dev_info = &port->dev_info;
> +			rxq_conf = &port->rx_conf[fs->rx_queue];
> +			if ((dev_info->dev_capa &
> RTE_ETH_DEV_CAPA_RXQ_SHARE)
> +			    == 0)
> +				/* Not shared rxq. */
> +				continue;
> +			if (domain_id != port->dev_info.switch_info.domain_id)
> +				continue;
> +			if (rxq_conf->share_group != share_group)
> +				continue;
> +			if (rxq_conf->share_qid != share_rxq)
> +				continue;
> +			printf("Shared Rx queue group %u queue %hu can't be
> scheduled on different cores:\n",
> +			       share_group, share_rxq);
> +			printf("  lcore %hhu Port %hu queue %hu\n",
> +			       src_lc, src_port, src_rxq);
> +			printf("  lcore %hhu Port %hu queue %hu\n",
> +			       lc_id, fs->rx_port, fs->rx_queue);
> +			printf("Please use --nb-cores=%hu to limit number of
> forwarding cores\n",
> +			       nb_rxq);
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +/*
> + * Check shared rxq configuration.
> + *
> + * Shared group must not being scheduled on different core.
> + */
> +bool
> +pkt_fwd_shared_rxq_check(void)
> +{
> +	streamid_t sm_id;
> +	streamid_t nb_fs_per_lcore;
> +	lcoreid_t  nb_fc;
> +	lcoreid_t  lc_id;
> +	struct fwd_stream *fs;
> +	uint16_t domain_id;
> +	struct rte_port *port;
> +	struct rte_eth_dev_info *dev_info;
> +	struct rte_eth_rxconf *rxq_conf;
> +
> +	nb_fc = cur_fwd_config.nb_fwd_lcores;
> +	/*
> +	 * Check streams on each core, make sure the same switch domain +
> +	 * group + queue doesn't get scheduled on other cores.
> +	 */
> +	for (lc_id = 0; lc_id < nb_fc; lc_id++) {
> +		sm_id = fwd_lcores[lc_id]->stream_idx;
> +		nb_fs_per_lcore = fwd_lcores[lc_id]->stream_nb;
> +		for (; sm_id < fwd_lcores[lc_id]->stream_idx + nb_fs_per_lcore;
> +		     sm_id++) {
> +			fs = fwd_streams[sm_id];
> +			/* Update lcore info stream being scheduled. */
> +			fs->lcore = fwd_lcores[lc_id];
> +			port = &ports[fs->rx_port];
> +			dev_info = &port->dev_info;
> +			rxq_conf = &port->rx_conf[fs->rx_queue];
> +			if ((dev_info->dev_capa &
> RTE_ETH_DEV_CAPA_RXQ_SHARE)
> +			    == 0)
> +				/* Not shared rxq. */
> +				continue;
> +			/* Check shared rxq not scheduled on remaining cores.

The check will be done anyway just if dev has the capability of share_rxq.
But what if user wants normal queue config when they are using the dev which has the share_q capability?
You should limit the check only when "rxq_share > 0".

> */
> +			domain_id = port->dev_info.switch_info.domain_id;
> +			if (fwd_stream_on_other_lcores(domain_id, lc_id,
> +						       fs->rx_port,
> +						       fs->rx_queue,
> +						       rxq_conf->share_group,
> +						       rxq_conf->share_qid))
> +				return false;
> +		}
> +	}
> +	return true;
> +}
> +
>  /*
>   * Setup forwarding configuration for each logical core.
>   */
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 123142ed110..f3f81ef561f 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2236,10 +2236,12 @@ start_packet_forwarding(int with_tx_first)
> 
>  	fwd_config_setup();
> 
> +	pkt_fwd_config_display(&cur_fwd_config);
> +	if (!pkt_fwd_shared_rxq_check())

Same comment as above
This check should only happens if user enables "--rxq-share=[X]".
You can limit the check here too.
If (rxq_share > 0 && !pkt_fwd_shared_rxq_check())

> +		return;
>  	if(!no_flush_rx)
>  		flush_fwd_rx_queues();
> 
> -	pkt_fwd_config_display(&cur_fwd_config);
>  	rxtx_config_display();
> 
>  	fwd_stats_reset();
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 3dfaaad94c0..f121a2da90c 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -144,6 +144,7 @@ struct fwd_stream {
>  	uint64_t     core_cycles; /**< used for RX and TX processing */
>  	struct pkt_burst_stats rx_burst_stats;
>  	struct pkt_burst_stats tx_burst_stats;
> +	struct fwd_lcore *lcore; /**< Lcore being scheduled. */
>  };
> 
>  /**
> @@ -795,6 +796,7 @@ void port_summary_header_display(void);
>  void rx_queue_infos_display(portid_t port_idi, uint16_t queue_id);  void
> tx_queue_infos_display(portid_t port_idi, uint16_t queue_id);  void
> fwd_lcores_config_display(void);
> +bool pkt_fwd_shared_rxq_check(void);
>  void pkt_fwd_config_display(struct fwd_config *cfg);  void
> rxtx_config_display(void);  void fwd_config_setup(void);
> --
> 2.33.0