DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/failsafe: report valid RSS RETA size
@ 2020-05-27 14:34 Andrew Rybchenko
  2020-05-27 15:07 ` Gaëtan Rivet
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Rybchenko @ 2020-05-27 14:34 UTC (permalink / raw)
  To: Gaetan Rivet; +Cc: dev, Ian Dolzhansky, Stephen Hemminger, stable

From: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>

Failsafe driver has been indicating zero for RSS redirection table size
after device info reporting had been reworked. Report proper value.

Fixes: 4586be3743d4 ("net/failsafe: fix reported device info")
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: stable@dpdk.org

Signed-off-by: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/failsafe/failsafe_ops.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index e046cfe6aa..45da9378c3 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -1068,6 +1068,13 @@ fs_dev_merge_info(struct rte_eth_dev_info *info,
 	info->rx_queue_offload_capa &= sinfo->rx_queue_offload_capa;
 	info->tx_queue_offload_capa &= sinfo->tx_queue_offload_capa;
 	info->flow_type_rss_offloads &= sinfo->flow_type_rss_offloads;
+
+	/*
+	 * RETA size is a GCD of RETA sizes indicated by sub-devices.
+	 * Each of these sizes is a power of 2, so use the lower one.
+	 */
+	info->reta_size = RTE_MIN(info->reta_size, sinfo->reta_size);
+
 	info->hash_key_size = RTE_MIN(info->hash_key_size,
 				      sinfo->hash_key_size);
 
@@ -1128,6 +1135,7 @@ fs_dev_infos_get(struct rte_eth_dev *dev,
 	infos->max_hash_mac_addrs = UINT32_MAX;
 	infos->max_vfs = UINT16_MAX;
 	infos->max_vmdq_pools = UINT16_MAX;
+	infos->reta_size = UINT16_MAX;
 	infos->hash_key_size = UINT8_MAX;
 
 	/*
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] net/failsafe: report valid RSS RETA size
  2020-05-27 14:34 [dpdk-dev] [PATCH] net/failsafe: report valid RSS RETA size Andrew Rybchenko
@ 2020-05-27 15:07 ` Gaëtan Rivet
  2020-05-27 15:30   ` Andrew Rybchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Gaëtan Rivet @ 2020-05-27 15:07 UTC (permalink / raw)
  To: Andrew Rybchenko; +Cc: dev, Ian Dolzhansky, Stephen Hemminger, stable

On 27/05/20 15:34 +0100, Andrew Rybchenko wrote:
> From: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
> 
> Failsafe driver has been indicating zero for RSS redirection table size
> after device info reporting had been reworked. Report proper value.
> 
> Fixes: 4586be3743d4 ("net/failsafe: fix reported device info")
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/failsafe/failsafe_ops.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> index e046cfe6aa..45da9378c3 100644
> --- a/drivers/net/failsafe/failsafe_ops.c
> +++ b/drivers/net/failsafe/failsafe_ops.c
> @@ -1068,6 +1068,13 @@ fs_dev_merge_info(struct rte_eth_dev_info *info,
>  	info->rx_queue_offload_capa &= sinfo->rx_queue_offload_capa;
>  	info->tx_queue_offload_capa &= sinfo->tx_queue_offload_capa;
>  	info->flow_type_rss_offloads &= sinfo->flow_type_rss_offloads;
> +
> +	/*
> +	 * RETA size is a GCD of RETA sizes indicated by sub-devices.
> +	 * Each of these sizes is a power of 2, so use the lower one.
> +	 */
> +	info->reta_size = RTE_MIN(info->reta_size, sinfo->reta_size);
> +
>  	info->hash_key_size = RTE_MIN(info->hash_key_size,
>  				      sinfo->hash_key_size);
>  
> @@ -1128,6 +1135,7 @@ fs_dev_infos_get(struct rte_eth_dev *dev,
>  	infos->max_hash_mac_addrs = UINT32_MAX;
>  	infos->max_vfs = UINT16_MAX;
>  	infos->max_vmdq_pools = UINT16_MAX;
> +	infos->reta_size = UINT16_MAX;
>  	infos->hash_key_size = UINT8_MAX;
>  
>  	/*
> -- 
> 2.17.1
> 

Hello Andrew, Ian,

The reta_size info is linked to being able to update the RSS RETA.

I don't think it is a bug for the moment to report 0, as long as
failsafe does not support RETA update. Now, the reta_update ops could be
quickly implemented in failsafe, but that should be a new feature.

Did you hit an issue with it?

Regards,
-- 
Gaëtan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] net/failsafe: report valid RSS RETA size
  2020-05-27 15:07 ` Gaëtan Rivet
@ 2020-05-27 15:30   ` Andrew Rybchenko
  2020-05-27 21:35     ` Gaëtan Rivet
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Rybchenko @ 2020-05-27 15:30 UTC (permalink / raw)
  To: Gaëtan Rivet; +Cc: dev, Ian Dolzhansky, Stephen Hemminger, stable

On 5/27/20 6:07 PM, Gaëtan Rivet wrote:
> On 27/05/20 15:34 +0100, Andrew Rybchenko wrote:
>> From: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
>>
>> Failsafe driver has been indicating zero for RSS redirection table size
>> after device info reporting had been reworked. Report proper value.
>>
>> Fixes: 4586be3743d4 ("net/failsafe: fix reported device info")
>> Cc: Stephen Hemminger <sthemmin@microsoft.com>
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>  drivers/net/failsafe/failsafe_ops.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
>> index e046cfe6aa..45da9378c3 100644
>> --- a/drivers/net/failsafe/failsafe_ops.c
>> +++ b/drivers/net/failsafe/failsafe_ops.c
>> @@ -1068,6 +1068,13 @@ fs_dev_merge_info(struct rte_eth_dev_info *info,
>>  	info->rx_queue_offload_capa &= sinfo->rx_queue_offload_capa;
>>  	info->tx_queue_offload_capa &= sinfo->tx_queue_offload_capa;
>>  	info->flow_type_rss_offloads &= sinfo->flow_type_rss_offloads;
>> +
>> +	/*
>> +	 * RETA size is a GCD of RETA sizes indicated by sub-devices.
>> +	 * Each of these sizes is a power of 2, so use the lower one.
>> +	 */
>> +	info->reta_size = RTE_MIN(info->reta_size, sinfo->reta_size);
>> +
>>  	info->hash_key_size = RTE_MIN(info->hash_key_size,
>>  				      sinfo->hash_key_size);
>>  
>> @@ -1128,6 +1135,7 @@ fs_dev_infos_get(struct rte_eth_dev *dev,
>>  	infos->max_hash_mac_addrs = UINT32_MAX;
>>  	infos->max_vfs = UINT16_MAX;
>>  	infos->max_vmdq_pools = UINT16_MAX;
>> +	infos->reta_size = UINT16_MAX;
>>  	infos->hash_key_size = UINT8_MAX;
>>  
>>  	/*
>> -- 
>> 2.17.1
>>
> 
> Hello Andrew, Ian,
> 
> The reta_size info is linked to being able to update the RSS RETA.
> 
> I don't think it is a bug for the moment to report 0, as long as
> failsafe does not support RETA update. Now, the reta_update ops could be
> quickly implemented in failsafe, but that should be a new feature.
> 
> Did you hit an issue with it?

Hello Gaetan,

in our case reta_size is required for RSS testing even if there
is no reta update and query API to estimate where a packet
should be delivered. It assumes that default reta spreads over
all Rx queue evenly. I.e. we need reta size and number of Rx
queues to built default reta.

May be it is really very testing specific, but IMO logical
anyway.

Andrew.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] net/failsafe: report valid RSS RETA size
  2020-05-27 15:30   ` Andrew Rybchenko
@ 2020-05-27 21:35     ` Gaëtan Rivet
  2020-05-27 22:02       ` Stephen Hemminger
  0 siblings, 1 reply; 6+ messages in thread
From: Gaëtan Rivet @ 2020-05-27 21:35 UTC (permalink / raw)
  To: Andrew Rybchenko; +Cc: dev, Ian Dolzhansky, Stephen Hemminger, stable

On 27/05/20 18:30 +0300, Andrew Rybchenko wrote:
> On 5/27/20 6:07 PM, Gaëtan Rivet wrote:
> > On 27/05/20 15:34 +0100, Andrew Rybchenko wrote:
> >> From: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
> >>
> >> Failsafe driver has been indicating zero for RSS redirection table size
> >> after device info reporting had been reworked. Report proper value.
> >>
> >> Fixes: 4586be3743d4 ("net/failsafe: fix reported device info")
> >> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >> ---
> >>  drivers/net/failsafe/failsafe_ops.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> >> index e046cfe6aa..45da9378c3 100644
> >> --- a/drivers/net/failsafe/failsafe_ops.c
> >> +++ b/drivers/net/failsafe/failsafe_ops.c
> >> @@ -1068,6 +1068,13 @@ fs_dev_merge_info(struct rte_eth_dev_info *info,
> >>  	info->rx_queue_offload_capa &= sinfo->rx_queue_offload_capa;
> >>  	info->tx_queue_offload_capa &= sinfo->tx_queue_offload_capa;
> >>  	info->flow_type_rss_offloads &= sinfo->flow_type_rss_offloads;
> >> +
> >> +	/*
> >> +	 * RETA size is a GCD of RETA sizes indicated by sub-devices.
> >> +	 * Each of these sizes is a power of 2, so use the lower one.
> >> +	 */
> >> +	info->reta_size = RTE_MIN(info->reta_size, sinfo->reta_size);
> >> +
> >>  	info->hash_key_size = RTE_MIN(info->hash_key_size,
> >>  				      sinfo->hash_key_size);
> >>  
> >> @@ -1128,6 +1135,7 @@ fs_dev_infos_get(struct rte_eth_dev *dev,
> >>  	infos->max_hash_mac_addrs = UINT32_MAX;
> >>  	infos->max_vfs = UINT16_MAX;
> >>  	infos->max_vmdq_pools = UINT16_MAX;
> >> +	infos->reta_size = UINT16_MAX;
> >>  	infos->hash_key_size = UINT8_MAX;
> >>  
> >>  	/*
> >> -- 
> >> 2.17.1
> >>
> > 
> > Hello Andrew, Ian,
> > 
> > The reta_size info is linked to being able to update the RSS RETA.
> > 
> > I don't think it is a bug for the moment to report 0, as long as
> > failsafe does not support RETA update. Now, the reta_update ops could be
> > quickly implemented in failsafe, but that should be a new feature.
> > 
> > Did you hit an issue with it?
> 
> Hello Gaetan,
> 
> in our case reta_size is required for RSS testing even if there
> is no reta update and query API to estimate where a packet
> should be delivered. It assumes that default reta spreads over
> all Rx queue evenly. I.e. we need reta size and number of Rx
> queues to built default reta.
> 
> May be it is really very testing specific, but IMO logical
> anyway.
> 
> Andrew.

Alright, given the size of the change there is no reason not to add it
if it can improve consistency.

I think the patch is fine otherwise, I'm just not sure it should be
marked as a bug fix, but backporting won't be hard.

Thanks,

Acked-by: Gaetan Rivet <grive@u256.net>

-- 
Gaëtan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] net/failsafe: report valid RSS RETA size
  2020-05-27 21:35     ` Gaëtan Rivet
@ 2020-05-27 22:02       ` Stephen Hemminger
  2020-06-02 18:10         ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2020-05-27 22:02 UTC (permalink / raw)
  To: Gaëtan Rivet
  Cc: Andrew Rybchenko, dev, Ian Dolzhansky, Stephen Hemminger, stable

On Wed, 27 May 2020 23:35:25 +0200
Gaëtan Rivet <grive@u256.net> wrote:

> On 27/05/20 18:30 +0300, Andrew Rybchenko wrote:
> > On 5/27/20 6:07 PM, Gaëtan Rivet wrote:  
> > > On 27/05/20 15:34 +0100, Andrew Rybchenko wrote:  
> > >> From: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
> > >>
> > >> Failsafe driver has been indicating zero for RSS redirection table size
> > >> after device info reporting had been reworked. Report proper value.
> > >>
> > >> Fixes: 4586be3743d4 ("net/failsafe: fix reported device info")
> > >> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> > >> Cc: stable@dpdk.org
> > >>
> > >> Signed-off-by: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
> > >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > >> ---
> > >>  drivers/net/failsafe/failsafe_ops.c | 8 ++++++++
> > >>  1 file changed, 8 insertions(+)
> > >>
> > >> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> > >> index e046cfe6aa..45da9378c3 100644
> > >> --- a/drivers/net/failsafe/failsafe_ops.c
> > >> +++ b/drivers/net/failsafe/failsafe_ops.c
> > >> @@ -1068,6 +1068,13 @@ fs_dev_merge_info(struct rte_eth_dev_info *info,
> > >>  	info->rx_queue_offload_capa &= sinfo->rx_queue_offload_capa;
> > >>  	info->tx_queue_offload_capa &= sinfo->tx_queue_offload_capa;
> > >>  	info->flow_type_rss_offloads &= sinfo->flow_type_rss_offloads;
> > >> +
> > >> +	/*
> > >> +	 * RETA size is a GCD of RETA sizes indicated by sub-devices.
> > >> +	 * Each of these sizes is a power of 2, so use the lower one.
> > >> +	 */
> > >> +	info->reta_size = RTE_MIN(info->reta_size, sinfo->reta_size);
> > >> +
> > >>  	info->hash_key_size = RTE_MIN(info->hash_key_size,
> > >>  				      sinfo->hash_key_size);
> > >>  
> > >> @@ -1128,6 +1135,7 @@ fs_dev_infos_get(struct rte_eth_dev *dev,
> > >>  	infos->max_hash_mac_addrs = UINT32_MAX;
> > >>  	infos->max_vfs = UINT16_MAX;
> > >>  	infos->max_vmdq_pools = UINT16_MAX;
> > >> +	infos->reta_size = UINT16_MAX;
> > >>  	infos->hash_key_size = UINT8_MAX;
> > >>  
> > >>  	/*
> > >> -- 
> > >> 2.17.1
> > >>  
> > > 
> > > Hello Andrew, Ian,
> > > 
> > > The reta_size info is linked to being able to update the RSS RETA.
> > > 
> > > I don't think it is a bug for the moment to report 0, as long as
> > > failsafe does not support RETA update. Now, the reta_update ops could be
> > > quickly implemented in failsafe, but that should be a new feature.
> > > 
> > > Did you hit an issue with it?  
> > 
> > Hello Gaetan,
> > 
> > in our case reta_size is required for RSS testing even if there
> > is no reta update and query API to estimate where a packet
> > should be delivered. It assumes that default reta spreads over
> > all Rx queue evenly. I.e. we need reta size and number of Rx
> > queues to built default reta.
> > 
> > May be it is really very testing specific, but IMO logical
> > anyway.
> > 
> > Andrew.  
> 
> Alright, given the size of the change there is no reason not to add it
> if it can improve consistency.
> 
> I think the patch is fine otherwise, I'm just not sure it should be
> marked as a bug fix, but backporting won't be hard.
> 
> Thanks,
> 
> Acked-by: Gaetan Rivet <grive@u256.net>
> 

Agreed, looks good to me, but probably doesn't matter much. Almost all hardware
only supports one reta_size anyway.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] net/failsafe: report valid RSS RETA size
  2020-05-27 22:02       ` Stephen Hemminger
@ 2020-06-02 18:10         ` Ferruh Yigit
  0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2020-06-02 18:10 UTC (permalink / raw)
  To: Stephen Hemminger, Gaëtan Rivet
  Cc: Andrew Rybchenko, dev, Ian Dolzhansky, Stephen Hemminger, stable

On 5/27/2020 11:02 PM, Stephen Hemminger wrote:
> On Wed, 27 May 2020 23:35:25 +0200
> Gaëtan Rivet <grive@u256.net> wrote:
> 
>> On 27/05/20 18:30 +0300, Andrew Rybchenko wrote:
>>> On 5/27/20 6:07 PM, Gaëtan Rivet wrote:  
>>>> On 27/05/20 15:34 +0100, Andrew Rybchenko wrote:  
>>>>> From: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
>>>>>
>>>>> Failsafe driver has been indicating zero for RSS redirection table size
>>>>> after device info reporting had been reworked. Report proper value.
>>>>>
>>>>> Fixes: 4586be3743d4 ("net/failsafe: fix reported device info")
>>>>> Cc: Stephen Hemminger <sthemmin@microsoft.com>
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Ian Dolzhansky <ian.dolzhansky@oktetlabs.ru>
>>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>>> ---
>>>>>  drivers/net/failsafe/failsafe_ops.c | 8 ++++++++
>>>>>  1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
>>>>> index e046cfe6aa..45da9378c3 100644
>>>>> --- a/drivers/net/failsafe/failsafe_ops.c
>>>>> +++ b/drivers/net/failsafe/failsafe_ops.c
>>>>> @@ -1068,6 +1068,13 @@ fs_dev_merge_info(struct rte_eth_dev_info *info,
>>>>>  	info->rx_queue_offload_capa &= sinfo->rx_queue_offload_capa;
>>>>>  	info->tx_queue_offload_capa &= sinfo->tx_queue_offload_capa;
>>>>>  	info->flow_type_rss_offloads &= sinfo->flow_type_rss_offloads;
>>>>> +
>>>>> +	/*
>>>>> +	 * RETA size is a GCD of RETA sizes indicated by sub-devices.
>>>>> +	 * Each of these sizes is a power of 2, so use the lower one.
>>>>> +	 */
>>>>> +	info->reta_size = RTE_MIN(info->reta_size, sinfo->reta_size);
>>>>> +
>>>>>  	info->hash_key_size = RTE_MIN(info->hash_key_size,
>>>>>  				      sinfo->hash_key_size);
>>>>>  
>>>>> @@ -1128,6 +1135,7 @@ fs_dev_infos_get(struct rte_eth_dev *dev,
>>>>>  	infos->max_hash_mac_addrs = UINT32_MAX;
>>>>>  	infos->max_vfs = UINT16_MAX;
>>>>>  	infos->max_vmdq_pools = UINT16_MAX;
>>>>> +	infos->reta_size = UINT16_MAX;
>>>>>  	infos->hash_key_size = UINT8_MAX;
>>>>>  
>>>>>  	/*
>>>>> -- 
>>>>> 2.17.1
>>>>>  
>>>>
>>>> Hello Andrew, Ian,
>>>>
>>>> The reta_size info is linked to being able to update the RSS RETA.
>>>>
>>>> I don't think it is a bug for the moment to report 0, as long as
>>>> failsafe does not support RETA update. Now, the reta_update ops could be
>>>> quickly implemented in failsafe, but that should be a new feature.
>>>>
>>>> Did you hit an issue with it?  
>>>
>>> Hello Gaetan,
>>>
>>> in our case reta_size is required for RSS testing even if there
>>> is no reta update and query API to estimate where a packet
>>> should be delivered. It assumes that default reta spreads over
>>> all Rx queue evenly. I.e. we need reta size and number of Rx
>>> queues to built default reta.
>>>
>>> May be it is really very testing specific, but IMO logical
>>> anyway.
>>>
>>> Andrew.  
>>
>> Alright, given the size of the change there is no reason not to add it
>> if it can improve consistency.
>>
>> I think the patch is fine otherwise, I'm just not sure it should be
>> marked as a bug fix, but backporting won't be hard.
>>
>> Thanks,
>>
>> Acked-by: Gaetan Rivet <grive@u256.net>
>>
> 
> Agreed, looks good to me, but probably doesn't matter much. Almost all hardware
> only supports one reta_size anyway.
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-06-02 18:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27 14:34 [dpdk-dev] [PATCH] net/failsafe: report valid RSS RETA size Andrew Rybchenko
2020-05-27 15:07 ` Gaëtan Rivet
2020-05-27 15:30   ` Andrew Rybchenko
2020-05-27 21:35     ` Gaëtan Rivet
2020-05-27 22:02       ` Stephen Hemminger
2020-06-02 18:10         ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit

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).