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