From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 019CDA0526; Wed, 22 Jul 2020 07:50:54 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9CE991BFBB; Wed, 22 Jul 2020 07:50:53 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 2F16C1BFBA; Wed, 22 Jul 2020 07:50:50 +0200 (CEST) IronPort-SDR: SGvXY7PkI/d7hdBJPp/mYm+dV01I5yBDEdUcf5b7xfmL3kJ0xCqgGaBNRydktoHkud2nernn7m LY4ADzQg+DlQ== X-IronPort-AV: E=McAfee;i="6000,8403,9689"; a="149440517" X-IronPort-AV: E=Sophos;i="5.75,381,1589266800"; d="scan'208";a="149440517" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jul 2020 22:50:49 -0700 IronPort-SDR: 9e9da1m8YG+lsByGuawj9mJmYZOncYCKoYWDTqu5MOgkuUtjjX9ArkZn6VyWBzsZjlSy4HzDmo gD0gvdh3C1NQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,381,1589266800"; d="scan'208";a="284111485" Received: from jguo15x-mobl.ccr.corp.intel.com (HELO [10.67.68.153]) ([10.67.68.153]) by orsmga003.jf.intel.com with ESMTP; 21 Jul 2020 22:50:47 -0700 To: "Xie, WeiX" , "Wang, ShougangX" , "dev@dpdk.org" Cc: "Xing, Beilei" , "stable@dpdk.org" References: <20200715063515.9262-1-shougangx.wang@intel.com> <20200721054920.29749-1-shougangx.wang@intel.com> <6FD6A7610D20924F885A4ECF34E8AC910463D264@CDSMSX102.ccr.corp.intel.com> From: Jeff Guo Message-ID: <334f916e-1aad-264f-e073-6e737e78c767@intel.com> Date: Wed, 22 Jul 2020 13:50:46 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <6FD6A7610D20924F885A4ECF34E8AC910463D264@CDSMSX102.ccr.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix incorrect hash look up table X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" hi, shougang On 7/21/2020 2:41 PM, Xie, WeiX wrote: > Tested-by: Zhang, XiX > > Regards, > Xie Wei > > > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shougang Wang > Sent: Tuesday, July 21, 2020 1:49 PM > To: dev@dpdk.org > Cc: Xing, Beilei ; Guo, Jia ; Wang, ShougangX ; 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. > > Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS flow") > Cc: stable@dpdk.org > > Signed-off-by: Shougang Wang > --- > 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*/ Please fix the checkpatch issue here. > static int i40e_rss_set_lut(struct i40e_pf *pf, In order to eliminate any confuse with current i40e_set_rss_lut, please change the name, such as "i40e_rss_lut_init" or other better naming. > + 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); Alignment should match open parenthesis. > + > + 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; Suggest set the 0 to ret and return ret at the end of this function, so ret could be use in all part. > > 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); Need to check return value. > > rss_info->conf.queue_num = 0; > memset(&rss_info->conf.queue, 0, sizeof(uint16_t)); > -- > 2.17.1 >