* [PATCH v4] common/sfc_efx/base: set return code in case of the error [not found] <20230531070810.93131-1-artemii.morozov@arknetworks.am> @ 2023-06-22 9:31 ` Artemii Morozov 2023-06-22 11:27 ` Ferruh Yigit 2023-06-22 12:31 ` [PATCH v5] common/sfc_efx/base: fix Rx queue creation without RSS hash prefix Artemii Morozov 1 sibling, 1 reply; 6+ messages in thread From: Artemii Morozov @ 2023-06-22 9:31 UTC (permalink / raw) To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, stable, Andy Moreton If the prefix for the RSS hash was not chosen the ENOTSUP error should be returned. Before this patch the zero code was returned in case of an error. Fixes: f784cdc5cbb1 ("common/sfc_efx/base: provide control to deliver RSS hash") Cc: stable@dpdk.org Signed-off-by: Artemii Morozov <artemii.morozov@arknetworks.am> Reviewed-by: Andy Moreton <amoreton@xilinx.com> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> --- v4: add Cc: stable@dpdk.org and transform rss to RSS v3: update commit log as fix commit v2: don't use capital letters in email drivers/common/sfc_efx/base/efx_rx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/common/sfc_efx/base/efx_rx.c b/drivers/common/sfc_efx/base/efx_rx.c index 68f42f5cac..61726a9f0b 100644 --- a/drivers/common/sfc_efx/base/efx_rx.c +++ b/drivers/common/sfc_efx/base/efx_rx.c @@ -937,8 +937,10 @@ efx_rx_qcreate_internal( rss_hash_field = &erplp->erpl_fields[EFX_RX_PREFIX_FIELD_RSS_HASH]; - if (rss_hash_field->erpfi_width_bits == 0) + if (rss_hash_field->erpfi_width_bits == 0) { + rc = ENOTSUP; goto fail5; + } } enp->en_rx_qcount++; -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] common/sfc_efx/base: set return code in case of the error 2023-06-22 9:31 ` [PATCH v4] common/sfc_efx/base: set return code in case of the error Artemii Morozov @ 2023-06-22 11:27 ` Ferruh Yigit 0 siblings, 0 replies; 6+ messages in thread From: Ferruh Yigit @ 2023-06-22 11:27 UTC (permalink / raw) To: Artemii Morozov, dev; +Cc: Andrew Rybchenko, stable, Andy Moreton On 6/22/2023 10:31 AM, Artemii Morozov wrote: > If the prefix for the RSS hash was not chosen the > ENOTSUP error should be returned. Before this patch > the zero code was returned in case of an error. > What is the impact of the problem to user? Previously 'efx_rx_qcreate_internal()' was returning success although it fails, and 'erpp' was not set. There are multiple callers of 'efx_rx_qcreate_internal()', those functions will assume functions succeed when it failed, what kind of problem is this cause? Does it cause a crash, or failure in the Rx queue creation, or no impact at all, can you please describe? Impact information is not always easy to understand from code change or low level details, but most of the times that is what users are interested. Like fixing a return value can be a simple refactoring without any functional impact at all, or it can be causing a crash and making driver completely unusable for some cases, although code change can be similar these two cases has different priority. So commit message/title should highlight the impact, starting from 'fix' keyword. Assume it cause Rx queue creation failure, commit can be something like: ``` common/sfc_efx/base: fix Rx queue creation without RSS hash prefix If the prefix for the RSS hash was not chosen the ENOTSUP error should be returned. Before this patch success was returned for this case causing Rx queue creation to fail. Fixing return value to indicate failure. Fixes: ... Cc: ... ... ``` > Fixes: f784cdc5cbb1 ("common/sfc_efx/base: provide control to deliver RSS hash") > Cc: stable@dpdk.org > > Signed-off-by: Artemii Morozov <artemii.morozov@arknetworks.am> > Reviewed-by: Andy Moreton <amoreton@xilinx.com> > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> > --- > v4: add Cc: stable@dpdk.org and transform rss to RSS > > v3: update commit log as fix commit > > v2: don't use capital letters in email > > drivers/common/sfc_efx/base/efx_rx.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/common/sfc_efx/base/efx_rx.c b/drivers/common/sfc_efx/base/efx_rx.c > index 68f42f5cac..61726a9f0b 100644 > --- a/drivers/common/sfc_efx/base/efx_rx.c > +++ b/drivers/common/sfc_efx/base/efx_rx.c > @@ -937,8 +937,10 @@ efx_rx_qcreate_internal( > > rss_hash_field = > &erplp->erpl_fields[EFX_RX_PREFIX_FIELD_RSS_HASH]; > - if (rss_hash_field->erpfi_width_bits == 0) > + if (rss_hash_field->erpfi_width_bits == 0) { > + rc = ENOTSUP; > goto fail5; > + } > } > > enp->en_rx_qcount++; ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5] common/sfc_efx/base: fix Rx queue creation without RSS hash prefix [not found] <20230531070810.93131-1-artemii.morozov@arknetworks.am> 2023-06-22 9:31 ` [PATCH v4] common/sfc_efx/base: set return code in case of the error Artemii Morozov @ 2023-06-22 12:31 ` Artemii Morozov 2023-06-22 13:13 ` Ferruh Yigit 2023-06-22 15:06 ` Ferruh Yigit 1 sibling, 2 replies; 6+ messages in thread From: Artemii Morozov @ 2023-06-22 12:31 UTC (permalink / raw) To: dev; +Cc: Ferruh Yigit, Andrew Rybchenko, stable, Andy Moreton If the prefix for the RSS hash was not chosen the ENOTSUP error should be returned. Before this patch success was returned for this case causing Rx queue creation to fail. Fixing return value to indicate failure. Fixes: f784cdc5cbb1 ("common/sfc_efx/base: provide control to deliver RSS hash") Cc: stable@dpdk.org Signed-off-by: Artemii Morozov <artemii.morozov@arknetworks.am> Reviewed-by: Andy Moreton <amoreton@xilinx.com> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> --- v5: update commit message v4: add Cc: stable@dpdk.org and transform rss to RSS v3: update commit log as fix commit v2: don't use capital letters in email drivers/common/sfc_efx/base/efx_rx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/common/sfc_efx/base/efx_rx.c b/drivers/common/sfc_efx/base/efx_rx.c index 68f42f5cac..61726a9f0b 100644 --- a/drivers/common/sfc_efx/base/efx_rx.c +++ b/drivers/common/sfc_efx/base/efx_rx.c @@ -937,8 +937,10 @@ efx_rx_qcreate_internal( rss_hash_field = &erplp->erpl_fields[EFX_RX_PREFIX_FIELD_RSS_HASH]; - if (rss_hash_field->erpfi_width_bits == 0) + if (rss_hash_field->erpfi_width_bits == 0) { + rc = ENOTSUP; goto fail5; + } } enp->en_rx_qcount++; -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] common/sfc_efx/base: fix Rx queue creation without RSS hash prefix 2023-06-22 12:31 ` [PATCH v5] common/sfc_efx/base: fix Rx queue creation without RSS hash prefix Artemii Morozov @ 2023-06-22 13:13 ` Ferruh Yigit 2023-06-22 13:52 ` Artemii Morozov 2023-06-22 15:06 ` Ferruh Yigit 1 sibling, 1 reply; 6+ messages in thread From: Ferruh Yigit @ 2023-06-22 13:13 UTC (permalink / raw) To: Artemii Morozov, dev; +Cc: Andrew Rybchenko, stable, Andy Moreton On 6/22/2023 1:31 PM, Artemii Morozov wrote: > If the prefix for the RSS hash was not chosen the ENOTSUP error should > be returned. > > Before this patch success was returned for this case causing Rx queue > creation to fail. > > Fixing return value to indicate failure. > It looks like you used sample commit log as it is, so can you please confirm this is the actual case (because this was my assumption without really knowing the actual problem), problem is not crash or undefined behavior because of 'erpp' was not set etc..? > Fixes: f784cdc5cbb1 ("common/sfc_efx/base: provide control to deliver RSS hash") > Cc: stable@dpdk.org > > Signed-off-by: Artemii Morozov <artemii.morozov@arknetworks.am> > Reviewed-by: Andy Moreton <amoreton@xilinx.com> > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> > --- > v5: update commit message > > v4: add Cc: stable@dpdk.org and transform rss to RSS > > v3: update commit log as fix commit > > v2: don't use capital letters in email > > drivers/common/sfc_efx/base/efx_rx.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/common/sfc_efx/base/efx_rx.c b/drivers/common/sfc_efx/base/efx_rx.c > index 68f42f5cac..61726a9f0b 100644 > --- a/drivers/common/sfc_efx/base/efx_rx.c > +++ b/drivers/common/sfc_efx/base/efx_rx.c > @@ -937,8 +937,10 @@ efx_rx_qcreate_internal( > > rss_hash_field = > &erplp->erpl_fields[EFX_RX_PREFIX_FIELD_RSS_HASH]; > - if (rss_hash_field->erpfi_width_bits == 0) > + if (rss_hash_field->erpfi_width_bits == 0) { > + rc = ENOTSUP; > goto fail5; > + } > } > > enp->en_rx_qcount++; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] common/sfc_efx/base: fix Rx queue creation without RSS hash prefix 2023-06-22 13:13 ` Ferruh Yigit @ 2023-06-22 13:52 ` Artemii Morozov 0 siblings, 0 replies; 6+ messages in thread From: Artemii Morozov @ 2023-06-22 13:52 UTC (permalink / raw) To: Ferruh Yigit, dev; +Cc: Andrew Rybchenko, stable, Andy Moreton On 6/22/23 17:13, Ferruh Yigit wrote: > On 6/22/2023 1:31 PM, Artemii Morozov wrote: >> If the prefix for the RSS hash was not chosen the ENOTSUP error should >> be returned. >> >> Before this patch success was returned for this case causing Rx queue >> creation to fail. >> >> Fixing return value to indicate failure. >> > It looks like you used sample commit log as it is, so can you please > confirm this is the actual case (because this was my assumption without > really knowing the actual problem), problem is not crash or undefined > behavior because of 'erpp' was not set etc..? The main problem is that if the RSS prefix was requested and an error occurred when creating the Rx queue, then this is ignored. Now the commit message reflects this. > >> Fixes: f784cdc5cbb1 ("common/sfc_efx/base: provide control to deliver RSS hash") >> Cc: stable@dpdk.org >> >> Signed-off-by: Artemii Morozov <artemii.morozov@arknetworks.am> >> Reviewed-by: Andy Moreton <amoreton@xilinx.com> >> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> >> --- >> v5: update commit message >> >> v4: add Cc: stable@dpdk.org and transform rss to RSS >> >> v3: update commit log as fix commit >> >> v2: don't use capital letters in email >> >> drivers/common/sfc_efx/base/efx_rx.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/common/sfc_efx/base/efx_rx.c b/drivers/common/sfc_efx/base/efx_rx.c >> index 68f42f5cac..61726a9f0b 100644 >> --- a/drivers/common/sfc_efx/base/efx_rx.c >> +++ b/drivers/common/sfc_efx/base/efx_rx.c >> @@ -937,8 +937,10 @@ efx_rx_qcreate_internal( >> >> rss_hash_field = >> &erplp->erpl_fields[EFX_RX_PREFIX_FIELD_RSS_HASH]; >> - if (rss_hash_field->erpfi_width_bits == 0) >> + if (rss_hash_field->erpfi_width_bits == 0) { >> + rc = ENOTSUP; >> goto fail5; >> + } >> } >> >> enp->en_rx_qcount++; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] common/sfc_efx/base: fix Rx queue creation without RSS hash prefix 2023-06-22 12:31 ` [PATCH v5] common/sfc_efx/base: fix Rx queue creation without RSS hash prefix Artemii Morozov 2023-06-22 13:13 ` Ferruh Yigit @ 2023-06-22 15:06 ` Ferruh Yigit 1 sibling, 0 replies; 6+ messages in thread From: Ferruh Yigit @ 2023-06-22 15:06 UTC (permalink / raw) To: Artemii Morozov, dev; +Cc: Andrew Rybchenko, stable, Andy Moreton On 6/22/2023 1:31 PM, Artemii Morozov wrote: > If the prefix for the RSS hash was not chosen the ENOTSUP error should > be returned. > > Before this patch success was returned for this case causing Rx queue > creation to fail. > > Fixing return value to indicate failure. > > Fixes: f784cdc5cbb1 ("common/sfc_efx/base: provide control to deliver RSS hash") > Cc: stable@dpdk.org > > Signed-off-by: Artemii Morozov <artemii.morozov@arknetworks.am> > Reviewed-by: Andy Moreton <amoreton@xilinx.com> > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> > Applied to dpdk-next-net/main, thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-22 15:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20230531070810.93131-1-artemii.morozov@arknetworks.am>
2023-06-22 9:31 ` [PATCH v4] common/sfc_efx/base: set return code in case of the error Artemii Morozov
2023-06-22 11:27 ` Ferruh Yigit
2023-06-22 12:31 ` [PATCH v5] common/sfc_efx/base: fix Rx queue creation without RSS hash prefix Artemii Morozov
2023-06-22 13:13 ` Ferruh Yigit
2023-06-22 13:52 ` Artemii Morozov
2023-06-22 15:06 ` 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).