[dpdk-dev] ethdev: force RSS offload rules again

Message ID 20180531132245.19424-1-ferruh.yigit@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Ferruh Yigit May 31, 2018, 1:22 p.m. UTC
  PMDs should provide supported RSS hash functions via
dev_info.flow_type_rss_offloads variable.

There is a check in ethdev if requested RSS hash function is supported
by PMD or not.
This check has been relaxed in previous release to not return an error
when a non supported has function requested [1], this has been done to
not break the applications.

Adding the error return back.
PMDs need to provide correct list of supported hash functions and
applications need to take care this information before configuring
the RSS otherwise they will get an error from APIs:
rte_eth_dev_rss_hash_update()
rte_eth_dev_configure()

[1] af7551e2bfce ("ethdev: remove error return on RSS hash check")

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Xueming Li <xuemingl@mellanox.com>
Cc: Shahaf Shuler <shahafs@mellanox.com>
Cc: Wei Dai <wei.dai@intel.com>
Cc: Qi Zhang <qi.z.zhang@intel.com>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Shahaf Shuler June 3, 2018, 10:41 a.m. UTC | #1
Thursday, May 31, 2018 4:23 PM, Ferruh Yigit:
> Subject: [PATCH] ethdev: force RSS offload rules again
> 
> PMDs should provide supported RSS hash functions via
> dev_info.flow_type_rss_offloads variable.
> 
> There is a check in ethdev if requested RSS hash function is supported by
> PMD or not.
> This check has been relaxed in previous release to not return an error when a
> non supported has function requested [1], this has been done to not break
> the applications.
> 
> Adding the error return back.
> PMDs need to provide correct list of supported hash functions and
> applications need to take care this information before configuring the RSS
> otherwise they will get an error from APIs:
> rte_eth_dev_rss_hash_update()
> rte_eth_dev_configure()

Are the current app/examples in DPDK tree behave accordingly?
  
Ferruh Yigit June 3, 2018, 2:15 p.m. UTC | #2
On 6/3/2018 11:41 AM, Shahaf Shuler wrote:
> Thursday, May 31, 2018 4:23 PM, Ferruh Yigit:
>> Subject: [PATCH] ethdev: force RSS offload rules again
>>
>> PMDs should provide supported RSS hash functions via
>> dev_info.flow_type_rss_offloads variable.
>>
>> There is a check in ethdev if requested RSS hash function is supported by
>> PMD or not.
>> This check has been relaxed in previous release to not return an error when a
>> non supported has function requested [1], this has been done to not break
>> the applications.
>>
>> Adding the error return back.
>> PMDs need to provide correct list of supported hash functions and
>> applications need to take care this information before configuring the RSS
>> otherwise they will get an error from APIs:
>> rte_eth_dev_rss_hash_update()
>> rte_eth_dev_configure()
> 
> Are the current app/examples in DPDK tree behave accordingly?  

I tried a few which were good but I don't know about all.

That is why we should merge this patch early so that we can detect and fix ones
fails.
  
Shahaf Shuler June 4, 2018, 7:56 a.m. UTC | #3
Sunday, June 3, 2018 5:15 PM, Ferruh Yigit:
> Subject: Re: [PATCH] ethdev: force RSS offload rules again

> 

> On 6/3/2018 11:41 AM, Shahaf Shuler wrote:

> > Thursday, May 31, 2018 4:23 PM, Ferruh Yigit:

> >> Subject: [PATCH] ethdev: force RSS offload rules again

> >>

> >> PMDs should provide supported RSS hash functions via

> >> dev_info.flow_type_rss_offloads variable.

> >>

> >> There is a check in ethdev if requested RSS hash function is

> >> supported by PMD or not.

> >> This check has been relaxed in previous release to not return an

> >> error when a non supported has function requested [1], this has been

> >> done to not break the applications.

> >>

> >> Adding the error return back.

> >> PMDs need to provide correct list of supported hash functions and

> >> applications need to take care this information before configuring

> >> the RSS otherwise they will get an error from APIs:

> >> rte_eth_dev_rss_hash_update()

> >> rte_eth_dev_configure()

> >

> > Are the current app/examples in DPDK tree behave accordingly?

> 

> I tried a few which were good but I don't know about all.

> 

> That is why we should merge this patch early so that we can detect and fix

> ones fails.


Acked-by: Shahaf Shuler <shahafs@mellanox.com>
  
Thomas Monjalon June 13, 2018, 3:19 p.m. UTC | #4
04/06/2018 09:56, Shahaf Shuler:
> Sunday, June 3, 2018 5:15 PM, Ferruh Yigit:
> > Subject: Re: [PATCH] ethdev: force RSS offload rules again
> > 
> > On 6/3/2018 11:41 AM, Shahaf Shuler wrote:
> > > Thursday, May 31, 2018 4:23 PM, Ferruh Yigit:
> > >> Subject: [PATCH] ethdev: force RSS offload rules again
> > >>
> > >> PMDs should provide supported RSS hash functions via
> > >> dev_info.flow_type_rss_offloads variable.
> > >>
> > >> There is a check in ethdev if requested RSS hash function is
> > >> supported by PMD or not.
> > >> This check has been relaxed in previous release to not return an
> > >> error when a non supported has function requested [1], this has been
> > >> done to not break the applications.
> > >>
> > >> Adding the error return back.
> > >> PMDs need to provide correct list of supported hash functions and
> > >> applications need to take care this information before configuring
> > >> the RSS otherwise they will get an error from APIs:
> > >> rte_eth_dev_rss_hash_update()
> > >> rte_eth_dev_configure()
> > >
> > > Are the current app/examples in DPDK tree behave accordingly?
> > 
> > I tried a few which were good but I don't know about all.
> > 
> > That is why we should merge this patch early so that we can detect and fix
> > ones fails.
> 
> Acked-by: Shahaf Shuler <shahafs@mellanox.com>

Acked-by: Thomas Monjalon <thomas@monjalon.net>
  

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 66e311676..a9977df97 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1194,6 +1194,7 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 				    port_id,
 				    dev_conf->rx_adv_conf.rss_conf.rss_hf,
 				    dev_info.flow_type_rss_offloads);
+		return -EINVAL;
 	}
 
 	/*
@@ -2928,6 +2929,7 @@  rte_eth_dev_rss_hash_update(uint16_t port_id,
 				    port_id,
 				    rss_conf->rss_hf,
 				    dev_info.flow_type_rss_offloads);
+		return -EINVAL;
 	}
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rss_hash_update, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,