patches for DPDK stable branches
 help / color / mirror / Atom feed
From: "Zhang, Qi Z" <qi.z.zhang@intel.com>
To: "Wang, ShougangX" <shougangx.wang@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "Xing, Beilei" <beilei.xing@intel.com>,
	"Guo, Jia" <jia.guo@intel.com>,
	"Wang, ShougangX" <shougangx.wang@intel.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v2] net/i40e: fix incorrect hash look up table
Date: Wed, 22 Jul 2020 05:31:01 +0000
Message-ID: <039ED4275CED7440929022BC67E7061154863E7D@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20200721054920.29749-1-shougangx.wang@intel.com>



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Shougang Wang
> Sent: Tuesday, July 21, 2020 1:49 PM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Guo, Jia <jia.guo@intel.com>; Wang,
> ShougangX <shougangx.wang@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] net/i40e: fix incorrect hash look up table
> 
> The hash look up table(LUT) will not be initializing when starting testpmd with
> --disable-rss. So that some invalid queue indexes may still in the LUT. When
> enable RSS by creating RSS rule, some packets will not be into the valid queues.
> This patch fixes this issue by initializing the LUT when creating an RSS rule.

Could you explain why you only initialize the LUT when creating an RSS rule but not at dev_init or dev_start?
What if user configure LUT table before create a RSS rule? Does that mean the LUT table will be flushed?

> 
> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 134 ++++++++++++++++-----------------
>  1 file changed, 63 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 393b5320f..e56543393 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -13070,6 +13070,55 @@ i40e_rss_conf_init(struct i40e_rte_flow_rss_conf
> *out,
>  	return 0;
>  }
> 
> +/* If conf is NULL, function will init hash LUT with default
> +configration*/ static int i40e_rss_set_lut(struct i40e_pf *pf,
> +		 struct i40e_rte_flow_rss_conf *conf) {
> +	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> +	uint32_t lut = 0;
> +	uint16_t j, num;
> +	uint32_t i;
> +
> +	/* If both VMDQ and RSS enabled, not all of PF queues are configured.
> +	 * It's necessary to calculate the actual PF queues that are configured.
> +	 */
> +	if (pf->dev_data->dev_conf.rxmode.mq_mode &
> ETH_MQ_RX_VMDQ_FLAG)
> +		num = i40e_pf_calc_configured_queues_num(pf);
> +	else
> +		num = pf->dev_data->nb_rx_queues;
> +
> +	if (conf == NULL)
> +		num = RTE_MIN(num, I40E_MAX_Q_PER_TC);
> +	else
> +		num = RTE_MIN(num, conf->conf.queue_num);
> +	PMD_DRV_LOG(INFO, "Max of contiguous %u PF queues are configured",
> +			num);
> +
> +	if (num == 0) {
> +		PMD_DRV_LOG(ERR,
> +			"No PF queues are configured to enable RSS for port %u",
> +			pf->dev_data->port_id);
> +		return -ENOTSUP;
> +	}
> +
> +	/* Fill in redirection table */
> +	for (i = 0, j = 0; i < hw->func_caps.rss_table_size; i++, j++) {
> +		if (j == num)
> +			j = 0;
> +		if (conf == NULL)
> +			lut = (lut << 8) | (j & ((0x1 <<
> +				hw->func_caps.rss_table_entry_width) - 1));
> +		else
> +			lut = (lut << 8) | (conf->conf.queue[j] & ((0x1 <<
> +			hw->func_caps.rss_table_entry_width) - 1));
> +		if ((i & 3) == 3)
> +			I40E_WRITE_REG(hw, I40E_PFQF_HLUT(i >> 2), lut);
> +	}
> +
> +	return 0;
> +}
> +
>  /* Write HENA register to enable hash */  static int
> i40e_rss_hash_set(struct i40e_pf *pf, struct i40e_rte_flow_rss_conf *rss_conf)
> @@ -13318,12 +13367,24 @@ static int  i40e_rss_enable_hash(struct i40e_pf
> *pf,
>  		struct i40e_rte_flow_rss_conf *conf)
>  {
> +	enum rte_eth_rx_mq_mode mq_mode =
> +pf->dev_data->dev_conf.rxmode.mq_mode;
>  	struct i40e_rte_flow_rss_conf *rss_info = &pf->rss_info;
>  	struct i40e_rte_flow_rss_conf rss_conf;
> +	int ret;
> 
>  	if (!(conf->conf.types & pf->adapter->flow_types_mask))
>  		return -ENOTSUP;
> 
> +	/* If the RSS is disabled before this, the LUT is uninitialized.
> +	 * So it is necessary to initialize it here.
> +	 */
> +	if (!(mq_mode & ETH_MQ_RX_RSS_FLAG)
> && !pf->rss_info.conf.queue_num &&
> +	    !pf->adapter->rss_reta_updated) {
> +		ret = i40e_rss_set_lut(pf, NULL);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	memset(&rss_conf, 0, sizeof(rss_conf));
>  	rte_memcpy(&rss_conf, conf, sizeof(rss_conf));
> 
> @@ -13362,39 +13423,7 @@ static int
>  i40e_rss_config_queue_region(struct i40e_pf *pf,
>  		struct i40e_rte_flow_rss_conf *conf)
>  {
> -	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> -	uint32_t lut = 0;
> -	uint16_t j, num;
> -	uint32_t i;
> -
> -	/* If both VMDQ and RSS enabled, not all of PF queues are configured.
> -	 * It's necessary to calculate the actual PF queues that are configured.
> -	 */
> -	if (pf->dev_data->dev_conf.rxmode.mq_mode &
> ETH_MQ_RX_VMDQ_FLAG)
> -		num = i40e_pf_calc_configured_queues_num(pf);
> -	else
> -		num = pf->dev_data->nb_rx_queues;
> -
> -	num = RTE_MIN(num, conf->conf.queue_num);
> -	PMD_DRV_LOG(INFO, "Max of contiguous %u PF queues are configured",
> -			num);
> -
> -	if (num == 0) {
> -		PMD_DRV_LOG(ERR,
> -			"No PF queues are configured to enable RSS for port %u",
> -			pf->dev_data->port_id);
> -		return -ENOTSUP;
> -	}
> -
> -	/* Fill in redirection table */
> -	for (i = 0, j = 0; i < hw->func_caps.rss_table_size; i++, j++) {
> -		if (j == num)
> -			j = 0;
> -		lut = (lut << 8) | (conf->conf.queue[j] & ((0x1 <<
> -			hw->func_caps.rss_table_entry_width) - 1));
> -		if ((i & 3) == 3)
> -			I40E_WRITE_REG(hw, I40E_PFQF_HLUT(i >> 2), lut);
> -	}
> +	i40e_rss_set_lut(pf, conf);
> 
>  	i40e_rss_mark_invalid_rule(pf, conf);
> 
> @@ -13491,46 +13520,9 @@ i40e_rss_disable_hash(struct i40e_pf *pf,  static
> int  i40e_rss_clear_queue_region(struct i40e_pf *pf)  {
> -	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
>  	struct i40e_rte_flow_rss_conf *rss_info = &pf->rss_info;
> -	uint16_t queue[I40E_MAX_Q_PER_TC];
> -	uint32_t num_rxq, i;
> -	uint32_t lut = 0;
> -	uint16_t j, num;
> -
> -	num_rxq = RTE_MIN(pf->dev_data->nb_rx_queues,
> I40E_MAX_Q_PER_TC);
> 
> -	for (j = 0; j < num_rxq; j++)
> -		queue[j] = j;
> -
> -	/* If both VMDQ and RSS enabled, not all of PF queues are configured.
> -	 * It's necessary to calculate the actual PF queues that are configured.
> -	 */
> -	if (pf->dev_data->dev_conf.rxmode.mq_mode &
> ETH_MQ_RX_VMDQ_FLAG)
> -		num = i40e_pf_calc_configured_queues_num(pf);
> -	else
> -		num = pf->dev_data->nb_rx_queues;
> -
> -	num = RTE_MIN(num, num_rxq);
> -	PMD_DRV_LOG(INFO, "Max of contiguous %u PF queues are configured",
> -			num);
> -
> -	if (num == 0) {
> -		PMD_DRV_LOG(ERR,
> -			"No PF queues are configured to enable RSS for port %u",
> -			pf->dev_data->port_id);
> -		return -ENOTSUP;
> -	}
> -
> -	/* Fill in redirection table */
> -	for (i = 0, j = 0; i < hw->func_caps.rss_table_size; i++, j++) {
> -		if (j == num)
> -			j = 0;
> -		lut = (lut << 8) | (queue[j] & ((0x1 <<
> -			hw->func_caps.rss_table_entry_width) - 1));
> -		if ((i & 3) == 3)
> -			I40E_WRITE_REG(hw, I40E_PFQF_HLUT(i >> 2), lut);
> -	}
> +	i40e_rss_set_lut(pf, NULL);
> 
>  	rss_info->conf.queue_num = 0;
>  	memset(&rss_info->conf.queue, 0, sizeof(uint16_t));
> --
> 2.17.1


  parent reply	other threads:[~2020-07-22  5:31 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-15  6:35 [dpdk-stable] [PATCH] " Shougang Wang
2020-07-15  9:07 ` [dpdk-stable] [dpdk-dev] " Chen, BoX C
2020-07-18  4:07   ` Jeff Guo
2020-07-21  5:49 ` [dpdk-stable] [PATCH v2] " Shougang Wang
2020-07-21  6:41   ` [dpdk-stable] [dpdk-dev] " Xie, WeiX
2020-07-22  5:50     ` Jeff Guo
2020-07-22  6:39       ` Wang, ShougangX
2020-07-22  5:31   ` Zhang, Qi Z [this message]
2020-07-22  6:20     ` Wang, ShougangX
2020-07-22  8:15 ` [dpdk-stable] [PATCH v3] " Shougang Wang
2020-07-23  1:57   ` [dpdk-stable] [dpdk-dev] " Yang, Qiming
2020-07-23 12:53     ` Zhang, Qi Z
2020-07-24  2:34       ` Wang, ShougangX
2020-07-24  2:47 ` [dpdk-stable] [PATCH v4] " Shougang Wang
2020-07-24  3:57   ` Jeff Guo
2020-07-24  4:49     ` Wang, ShougangX
2020-07-24  5:07   ` [dpdk-stable] [dpdk-dev] " Yang, Qiming
2020-07-24  5:42     ` Wang, ShougangX
2020-07-24  8:12 ` [dpdk-stable] [PATCH v5] " Shougang Wang
2020-07-24  9:00   ` [dpdk-stable] [dpdk-dev] " Xie, WeiX
2020-07-24  9:38 ` [dpdk-stable] [PATCH v6] " Shougang Wang
2020-07-24  9:54   ` Jeff Guo
2020-07-24  9:58     ` [dpdk-stable] [dpdk-dev] " Zhang, Qi Z

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=039ED4275CED7440929022BC67E7061154863E7D@SHSMSX103.ccr.corp.intel.com \
    --to=qi.z.zhang@intel.com \
    --cc=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=jia.guo@intel.com \
    --cc=shougangx.wang@intel.com \
    --cc=stable@dpdk.org \
    /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

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git