| Message ID | 1469089470-5764-1-git-send-email-xiao.w.wang@intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 12F533989; Thu, 21 Jul 2016 10:24:42 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id CEDB23979 for <dev@dpdk.org>; Thu, 21 Jul 2016 10:24:39 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 21 Jul 2016 01:24:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.28,398,1464678000"; d="scan'208"; a="1011100776" Received: from shvmail01.sh.intel.com ([10.239.29.42]) by fmsmga001.fm.intel.com with ESMTP; 21 Jul 2016 01:24:36 -0700 Received: from shecgisg004.sh.intel.com (shecgisg004.sh.intel.com [10.239.29.89]) by shvmail01.sh.intel.com with ESMTP id u6L8OYrX024085; Thu, 21 Jul 2016 16:24:34 +0800 Received: from shecgisg004.sh.intel.com (localhost [127.0.0.1]) by shecgisg004.sh.intel.com (8.13.6/8.13.6/SuSE Linux 0.8) with ESMTP id u6L8OV5v005799; Thu, 21 Jul 2016 16:24:33 +0800 Received: (from xiaowan1@localhost) by shecgisg004.sh.intel.com (8.13.6/8.13.6/Submit) id u6L8OVEc005795; Thu, 21 Jul 2016 16:24:31 +0800 From: Xiao Wang <xiao.w.wang@intel.com> To: dev@dpdk.org Cc: jing.d.chen@intel.com, xueqin.lin@intel.com, Xiao Wang <xiao.w.wang@intel.com> Date: Thu, 21 Jul 2016 16:24:30 +0800 Message-Id: <1469089470-5764-1-git-send-email-xiao.w.wang@intel.com> X-Mailer: git-send-email 1.7.4.1 Subject: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Xiao Wang
July 21, 2016, 8:24 a.m. UTC
Sometimes app just wants to update the RSS hash function and no RSS key
update is needed, but fm10k pmd will return EINVAL for this case.
If the rss_key is NULL, we don't need to check the rss_key_len.
Fixes: 57033cdf8fdc ("fm10k: add PF RSS")
Reported-by: Xueqin Lin <xueqin.lin@intel.com>
Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
---
drivers/net/fm10k/fm10k_ethdev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Comments
Hi, > diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c > index 144b2de..01f4a72 100644 > --- a/drivers/net/fm10k/fm10k_ethdev.c > +++ b/drivers/net/fm10k/fm10k_ethdev.c > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev *dev, > > PMD_INIT_FUNC_TRACE(); > > - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > - FM10K_RSSRK_ENTRIES_PER_REG) > + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > + FM10K_RSSRK_ENTRIES_PER_REG)) > return -EINVAL; > > if (hf == 0) It's also possible that app wants to update rss key and not expect to update hash function. Is that indicate we shouldn't return error in case hf == 0?
Hi Mark, > -----Original Message----- > From: Chen, Jing D > Sent: Thursday, July 21, 2016 4:48 PM > To: Wang, Xiao W <xiao.w.wang@intel.com>; dev@dpdk.org > Cc: Lin, Xueqin <xueqin.lin@intel.com> > Subject: RE: [PATCH] net/fm10k: fix RSS hash config > > Hi, > > > diff --git a/drivers/net/fm10k/fm10k_ethdev.c > > b/drivers/net/fm10k/fm10k_ethdev.c > > index 144b2de..01f4a72 100644 > > --- a/drivers/net/fm10k/fm10k_ethdev.c > > +++ b/drivers/net/fm10k/fm10k_ethdev.c > > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev *dev, > > > > PMD_INIT_FUNC_TRACE(); > > > > - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > - FM10K_RSSRK_ENTRIES_PER_REG) > > + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > + FM10K_RSSRK_ENTRIES_PER_REG)) > > return -EINVAL; > > > > if (hf == 0) > > It's also possible that app wants to update rss key and not expect to update hash > function. > Is that indicate we shouldn't return error in case hf == 0? > If the app just wants to update RSS key, it needs to read out the RSS config first, then change only the key field. This is what testpmd does for this operation. hf == 0 will disable RSS feature, I think we should return error to protect multi-queue. Best Regards, Xiao
2016-07-21 09:35, Wang, Xiao W: > From: Chen, Jing D > > > --- a/drivers/net/fm10k/fm10k_ethdev.c > > > +++ b/drivers/net/fm10k/fm10k_ethdev.c > > > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev *dev, > > > > > > PMD_INIT_FUNC_TRACE(); > > > > > > - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > > - FM10K_RSSRK_ENTRIES_PER_REG) > > > + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > > + FM10K_RSSRK_ENTRIES_PER_REG)) > > > return -EINVAL; > > > > > > if (hf == 0) > > > > It's also possible that app wants to update rss key and not expect to update hash > > function. > > Is that indicate we shouldn't return error in case hf == 0? > > > > If the app just wants to update RSS key, it needs to read out the RSS config first, then > change only the key field. This is what testpmd does for this operation. > > hf == 0 will disable RSS feature, I think we should return error to protect multi-queue. Jing, do you confirm we can apply this patch, please?
Hi, Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Friday, July 22, 2016 4:22 PM > To: Chen, Jing D <jing.d.chen@intel.com> > Cc: dev@dpdk.org; Wang, Xiao W <xiao.w.wang@intel.com>; Lin, Xueqin > <xueqin.lin@intel.com> > Subject: Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config > > 2016-07-21 09:35, Wang, Xiao W: > > From: Chen, Jing D > > > > --- a/drivers/net/fm10k/fm10k_ethdev.c > > > > +++ b/drivers/net/fm10k/fm10k_ethdev.c > > > > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev *dev, > > > > > > > > PMD_INIT_FUNC_TRACE(); > > > > > > > > - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > > > - FM10K_RSSRK_ENTRIES_PER_REG) > > > > + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > > > + FM10K_RSSRK_ENTRIES_PER_REG)) > > > > return -EINVAL; > > > > > > > > if (hf == 0) > > > > > > It's also possible that app wants to update rss key and not expect to update hash > > > function. > > > Is that indicate we shouldn't return error in case hf == 0? > > > > > > > If the app just wants to update RSS key, it needs to read out the RSS config first, > then > > change only the key field. This is what testpmd does for this operation. > > > > hf == 0 will disable RSS feature, I think we should return error to protect multi- > queue. > > Jing, do you confirm we can apply this patch, please? I think we need some rework or more explanations here.
2016-07-22 08:23, Chen, Jing D: > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > 2016-07-21 09:35, Wang, Xiao W: > > > From: Chen, Jing D > > > > > --- a/drivers/net/fm10k/fm10k_ethdev.c > > > > > +++ b/drivers/net/fm10k/fm10k_ethdev.c > > > > > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev *dev, > > > > > > > > > > PMD_INIT_FUNC_TRACE(); > > > > > > > > > > - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > > > > - FM10K_RSSRK_ENTRIES_PER_REG) > > > > > + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > > > > + FM10K_RSSRK_ENTRIES_PER_REG)) > > > > > return -EINVAL; > > > > > > > > > > if (hf == 0) > > > > > > > > It's also possible that app wants to update rss key and not expect to update hash > > > > function. > > > > Is that indicate we shouldn't return error in case hf == 0? > > > > > > > > > > If the app just wants to update RSS key, it needs to read out the RSS config first, > > then > > > change only the key field. This is what testpmd does for this operation. > > > > > > hf == 0 will disable RSS feature, I think we should return error to protect multi- > > queue. > > > > Jing, do you confirm we can apply this patch, please? > I think we need some rework or more explanations here. It is not reasonnable to wait RC5 for such a fix. Either it is not important and postponed to 16.11 or you submit a v2 very shortly for 16.07. Please advise
Hi, Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Friday, July 22, 2016 4:29 PM > To: Chen, Jing D <jing.d.chen@intel.com> > Cc: dev@dpdk.org; Wang, Xiao W <xiao.w.wang@intel.com>; Lin, Xueqin > <xueqin.lin@intel.com> > Subject: Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config > > 2016-07-22 08:23, Chen, Jing D: > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > > 2016-07-21 09:35, Wang, Xiao W: > > > > From: Chen, Jing D > > > > > > --- a/drivers/net/fm10k/fm10k_ethdev.c > > > > > > +++ b/drivers/net/fm10k/fm10k_ethdev.c > > > > > > @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev > *dev, > > > > > > > > > > > > PMD_INIT_FUNC_TRACE(); > > > > > > > > > > > > - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > > > > > - FM10K_RSSRK_ENTRIES_PER_REG) > > > > > > + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * > > > > > > + FM10K_RSSRK_ENTRIES_PER_REG)) > > > > > > return -EINVAL; > > > > > > > > > > > > if (hf == 0) > > > > > > > > > > It's also possible that app wants to update rss key and not expect to update > hash > > > > > function. > > > > > Is that indicate we shouldn't return error in case hf == 0? > > > > > > > > > > > > > If the app just wants to update RSS key, it needs to read out the RSS config first, > > > then > > > > change only the key field. This is what testpmd does for this operation. > > > > > > > > hf == 0 will disable RSS feature, I think we should return error to protect > multi- > > > queue. > > > > > > Jing, do you confirm we can apply this patch, please? > > I think we need some rework or more explanations here. > > It is not reasonnable to wait RC5 for such a fix. > Either it is not important and postponed to 16.11 or you submit > a v2 very shortly for 16.07. > Please advise Please kindly merge.
2016-07-21 16:24, Xiao Wang: > Sometimes app just wants to update the RSS hash function and no RSS key > update is needed, but fm10k pmd will return EINVAL for this case. > > If the rss_key is NULL, we don't need to check the rss_key_len. > > Fixes: 57033cdf8fdc ("fm10k: add PF RSS") > > Reported-by: Xueqin Lin <xueqin.lin@intel.com> > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com> Applied, thanks
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c index 144b2de..01f4a72 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -2159,8 +2159,8 @@ fm10k_rss_hash_update(struct rte_eth_dev *dev, PMD_INIT_FUNC_TRACE(); - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * - FM10K_RSSRK_ENTRIES_PER_REG) + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * + FM10K_RSSRK_ENTRIES_PER_REG)) return -EINVAL; if (hf == 0) @@ -2202,8 +2202,8 @@ fm10k_rss_hash_conf_get(struct rte_eth_dev *dev, PMD_INIT_FUNC_TRACE(); - if (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * - FM10K_RSSRK_ENTRIES_PER_REG) + if (key && (rss_conf->rss_key_len < FM10K_RSSRK_SIZE * + FM10K_RSSRK_ENTRIES_PER_REG)) return -EINVAL; if (key != NULL)