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 6CB7BA0521; Thu, 23 Jul 2020 14:53:32 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E12F71BFFE; Thu, 23 Jul 2020 14:53:30 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id B1DA31BFFD; Thu, 23 Jul 2020 14:53:28 +0200 (CEST) IronPort-SDR: D2UG80dpyxLvL8H9YCz/sj32RNLBRcuvMNU1qb4kbQNliZnTESQVcfPzjmDaiP1eZXa1k+8m9g Cvby0FiV6j9A== X-IronPort-AV: E=McAfee;i="6000,8403,9690"; a="130584952" X-IronPort-AV: E=Sophos;i="5.75,386,1589266800"; d="scan'208";a="130584952" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Jul 2020 05:53:27 -0700 IronPort-SDR: z6TVyjogXOuov3jrfvyFgIG4S/GAWpyka+XPtemttViXpwYDslJ4Fs2DVoHYkeDxHB+haSqH+c Uyg0cGJiiwfA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,386,1589266800"; d="scan'208";a="488819830" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga005.fm.intel.com with ESMTP; 23 Jul 2020 05:53:27 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 23 Jul 2020 05:53:27 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX154.amr.corp.intel.com (10.18.116.70) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 23 Jul 2020 05:53:26 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.22]) by shsmsx102.ccr.corp.intel.com ([169.254.2.43]) with mapi id 14.03.0439.000; Thu, 23 Jul 2020 20:53:23 +0800 From: "Zhang, Qi Z" To: "Yang, Qiming" , "Wang, ShougangX" , "dev@dpdk.org" CC: "Xing, Beilei" , "Guo, Jia" , "Wang, ShougangX" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v3] net/i40e: fix incorrect hash look up table Thread-Index: AQHWYALWKcxnSmLLsU+pXE7TqxoMoKkT4vyAgAE8kSA= Date: Thu, 23 Jul 2020 12:53:23 +0000 Message-ID: <039ED4275CED7440929022BC67E7061154864F86@SHSMSX103.ccr.corp.intel.com> References: <20200715063515.9262-1-shougangx.wang@intel.com> <20200722081543.52396-1-shougangx.wang@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3] 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" > -----Original Message----- > From: dev On Behalf Of Yang, Qiming > Sent: Thursday, July 23, 2020 9:57 AM > To: Wang, ShougangX ; dev@dpdk.org > Cc: Xing, Beilei ; Guo, Jia ; W= ang, > ShougangX ; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3] net/i40e: fix incorrect hash look up t= able >=20 > I don't understand why you add new function and new function mostly do th= e > same thing. > Why don't add fix in original code. >=20 > > -----Original Message----- > > From: dev On Behalf Of Shougang Wang > > Sent: Wednesday, July 22, 2020 16:16 > > To: dev@dpdk.org > > Cc: Xing, Beilei ; Guo, Jia > > ; Wang, ShougangX ; > > stable@dpdk.org > > Subject: [dpdk-dev] [PATCH v3] net/i40e: fix incorrect hash look up > > table > > > > The hash look up table (LUT) is managed by global register but it is > > not initialized when RSS is disabled. Once user wants to enable RSS > > during runtime, the LUT will not be initialized. > > This patch fixes the issue by initializing the LUT whether RSS enabled = or not. > > > > Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS > > flow") > > Cc: stable@dpdk.org > > > > Signed-off-by: Shougang Wang > > --- > > v3: > > -Updated the time of initializing the look up table > > --- > > drivers/net/i40e/i40e_ethdev.c | 85 > > ++++++++++++++++++++-------------- > > 1 file changed, 49 insertions(+), 36 deletions(-) > > > > diff --git a/drivers/net/i40e/i40e_ethdev.c > > b/drivers/net/i40e/i40e_ethdev.c index 05d5f2861..e35590d96 100644 > > --- a/drivers/net/i40e/i40e_ethdev.c > > +++ b/drivers/net/i40e/i40e_ethdev.c > > @@ -8984,42 +8984,7 @@ i40e_pf_calc_configured_queues_num(struct > > i40e_pf *pf) static int i40e_pf_config_rss(struct i40e_pf *pf) { > > - struct i40e_hw *hw =3D I40E_PF_TO_HW(pf); > > struct rte_eth_rss_conf rss_conf; > > - uint32_t i, lut =3D 0; > > - uint16_t j, num; > > - > > - /* > > - * If both VMDQ and RSS enabled, not all of PF queues are > > configured. > > - * It's necessary to calculate the actual PF queues that are configur= ed. > > - */ > > - if (pf->dev_data->dev_conf.rxmode.mq_mode & > > ETH_MQ_RX_VMDQ_FLAG) > > - num =3D i40e_pf_calc_configured_queues_num(pf); > > - else > > - num =3D pf->dev_data->nb_rx_queues; > > - > > - num =3D RTE_MIN(num, I40E_MAX_Q_PER_TC); > > - PMD_INIT_LOG(INFO, "Max of contiguous %u PF queues are > > configured", > > - num); > > - > > - if (num =3D=3D 0) { > > - PMD_INIT_LOG(ERR, > > - "No PF queues are configured to enable RSS for port > > %u", > > - pf->dev_data->port_id); > > - return -ENOTSUP; > > - } > > - > > - if (pf->adapter->rss_reta_updated =3D=3D 0) { > > - for (i =3D 0, j =3D 0; i < hw->func_caps.rss_table_size; i++, j++) { > > - if (j =3D=3D num) > > - j =3D 0; > > - lut =3D (lut << 8) | (j & ((0x1 << > > - hw->func_caps.rss_table_entry_width) - 1)); > > - if ((i & 3) =3D=3D 3) > > - I40E_WRITE_REG(hw, I40E_PFQF_HLUT(i >> > > 2), > > - rte_bswap32(lut)); > > - } > > - } > > > > rss_conf =3D pf->dev_data->dev_conf.rx_adv_conf.rss_conf; > > if ((rss_conf.rss_hf & pf->adapter->flow_types_mask) =3D=3D 0) { @@ - > > 9195,12 +9160,60 @@ i40e_tunnel_filter_handle(struct rte_eth_dev *dev, > > return ret; > > } > > > > +/* Initialize the hash look up table */ static int > > +i40e_pf_init_rss_lut(struct i40e_pf *pf) { > > + struct i40e_hw *hw =3D I40E_PF_TO_HW(pf); > > + uint32_t lut =3D 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 configur= ed. > > + */ > > + if (pf->dev_data->dev_conf.rxmode.mq_mode & > > ETH_MQ_RX_VMDQ_FLAG) > > + num =3D i40e_pf_calc_configured_queues_num(pf); > > + else > > + num =3D pf->dev_data->nb_rx_queues; > > + > > + num =3D RTE_MIN(num, I40E_MAX_Q_PER_TC); > > + PMD_INIT_LOG(INFO, "Max of contiguous %u PF queues are > > configured", > > + num); > > + > > + if (num =3D=3D 0) { > > + PMD_INIT_LOG(ERR, > > + "No PF queues are configured to enable RSS for port > > %u", > > + pf->dev_data->port_id); > > + return -ENOTSUP; > > + } > > + > > + if (pf->adapter->rss_reta_updated =3D=3D 0) { > > + for (i =3D 0, j =3D 0; i < hw->func_caps.rss_table_size; i++, j++) { > > + if (j =3D=3D num) > > + j =3D 0; > > + lut =3D (lut << 8) | (j & ((0x1 << > > + hw->func_caps.rss_table_entry_width) - 1)); > > + if ((i & 3) =3D=3D 3) > > + I40E_WRITE_REG(hw, I40E_PFQF_HLUT(i >> > > 2), > > + rte_bswap32(lut)); > > + } > > + } > > + > > + return 0; > > +} > > + > > static int > > i40e_pf_config_mq_rx(struct i40e_pf *pf) { > > - int ret =3D 0; > > + int ret; > > enum rte_eth_rx_mq_mode mq_mode =3D pf->dev_data- > > >dev_conf.rxmode.mq_mode; > > > > + /* Initialize hash look up table */ > > + ret =3D i40e_pf_init_rss_lut(pf); > > + if (ret) > > + return ret; > > + > > /* RSS setup */ > > if (mq_mode & ETH_MQ_RX_RSS_FLAG) I agree with Qiming, if we move this check into i40e_pf_config_rss, dose th= at looks we don't need to create a new function? > > ret =3D i40e_pf_config_rss(pf); > > -- > > 2.17.1