* [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
@ 2016-07-21 8:24 Xiao Wang
2016-07-21 8:48 ` Chen, Jing D
2016-07-22 9:53 ` Thomas Monjalon
0 siblings, 2 replies; 8+ messages in thread
From: Xiao Wang @ 2016-07-21 8:24 UTC (permalink / raw)
To: dev; +Cc: jing.d.chen, xueqin.lin, 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>
---
drivers/net/fm10k/fm10k_ethdev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
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)
--
1.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
2016-07-21 8:24 [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config Xiao Wang
@ 2016-07-21 8:48 ` Chen, Jing D
2016-07-21 9:35 ` Wang, Xiao W
2016-07-22 9:53 ` Thomas Monjalon
1 sibling, 1 reply; 8+ messages in thread
From: Chen, Jing D @ 2016-07-21 8:48 UTC (permalink / raw)
To: Wang, Xiao W, dev; +Cc: Lin, Xueqin
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
2016-07-21 8:48 ` Chen, Jing D
@ 2016-07-21 9:35 ` Wang, Xiao W
2016-07-22 8:21 ` Thomas Monjalon
0 siblings, 1 reply; 8+ messages in thread
From: Wang, Xiao W @ 2016-07-21 9:35 UTC (permalink / raw)
To: Chen, Jing D, dev; +Cc: Lin, Xueqin
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
2016-07-21 9:35 ` Wang, Xiao W
@ 2016-07-22 8:21 ` Thomas Monjalon
2016-07-22 8:23 ` Chen, Jing D
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2016-07-22 8:21 UTC (permalink / raw)
To: Chen, Jing D; +Cc: dev, Wang, Xiao W, Lin, Xueqin
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
2016-07-22 8:21 ` Thomas Monjalon
@ 2016-07-22 8:23 ` Chen, Jing D
2016-07-22 8:28 ` Thomas Monjalon
0 siblings, 1 reply; 8+ messages in thread
From: Chen, Jing D @ 2016-07-22 8:23 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev, Wang, Xiao W, Lin, Xueqin
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
2016-07-22 8:23 ` Chen, Jing D
@ 2016-07-22 8:28 ` Thomas Monjalon
2016-07-22 9:05 ` Chen, Jing D
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2016-07-22 8:28 UTC (permalink / raw)
To: Chen, Jing D; +Cc: dev, Wang, Xiao W, Lin, Xueqin
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
2016-07-22 8:28 ` Thomas Monjalon
@ 2016-07-22 9:05 ` Chen, Jing D
0 siblings, 0 replies; 8+ messages in thread
From: Chen, Jing D @ 2016-07-22 9:05 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev, Wang, Xiao W, Lin, Xueqin
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config
2016-07-21 8:24 [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config Xiao Wang
2016-07-21 8:48 ` Chen, Jing D
@ 2016-07-22 9:53 ` Thomas Monjalon
1 sibling, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2016-07-22 9:53 UTC (permalink / raw)
To: Xiao Wang; +Cc: dev, jing.d.chen, xueqin.lin
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-07-22 9:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-21 8:24 [dpdk-dev] [PATCH] net/fm10k: fix RSS hash config Xiao Wang
2016-07-21 8:48 ` Chen, Jing D
2016-07-21 9:35 ` Wang, Xiao W
2016-07-22 8:21 ` Thomas Monjalon
2016-07-22 8:23 ` Chen, Jing D
2016-07-22 8:28 ` Thomas Monjalon
2016-07-22 9:05 ` Chen, Jing D
2016-07-22 9:53 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).