* [PATCH] net/failsafe: fix failsafe bus uninit return value
@ 2017-08-29 7:51 Raslan Darawsheh
2017-08-29 8:18 ` Gaëtan Rivet
0 siblings, 1 reply; 7+ messages in thread
From: Raslan Darawsheh @ 2017-08-29 7:51 UTC (permalink / raw)
To: thomas, gaetan.rivet; +Cc: dev
fs_bus_uninit is always returning 0 no matter what was the status
of each sub device bus_uninit value.
Will now return the first sub device fail value in case it fails.
Fixes: a46f8d58 ("net/failsafe: add fail-safe PMD")
Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
---
drivers/net/failsafe/failsafe_eal.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index c8f4318..4295347 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -90,19 +90,20 @@ fs_bus_uninit(struct rte_eth_dev *dev)
{
struct sub_device *sdev = NULL;
uint8_t i;
- int ret;
+ int ret = 0, sdev_ret;
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
- ret = rte_eal_hotplug_remove(sdev->bus->name,
+ sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
sdev->dev->name);
- if (ret) {
+ if (sdev_ret) {
ERROR("Failed to remove requested device %s",
sdev->dev->name);
+ ret = (ret ? ret : sdev_ret);
continue;
}
sdev->state = DEV_PROBED - 1;
}
- return 0;
+ return ret;
}
int
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] net/failsafe: fix failsafe bus uninit return value
2017-08-29 7:51 [PATCH] net/failsafe: fix failsafe bus uninit return value Raslan Darawsheh
@ 2017-08-29 8:18 ` Gaëtan Rivet
2017-08-29 8:55 ` [PATCH v2] " Raslan Darawsheh
0 siblings, 1 reply; 7+ messages in thread
From: Gaëtan Rivet @ 2017-08-29 8:18 UTC (permalink / raw)
To: Raslan Darawsheh; +Cc: thomas, dev
Hi Raslan,
On Tue, Aug 29, 2017 at 10:51:29AM +0300, Raslan Darawsheh wrote:
> fs_bus_uninit is always returning 0 no matter what was the status
> of each sub device bus_uninit value.
An error on fs_bus_uninit should not stop failsafe_eal_uninit from
finishing its operation.
So for the sake of internal consistency for this scope, it might be
acceptable to yield an error.
However, if you do so, you then need to ignore the error within
failsafe_eal_uninit, so that the fail-safe device state is also properly
set.
something like:
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index c8f4318..d144719 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -111,8 +111,6 @@ failsafe_eal_uninit(struct rte_eth_dev *dev)
int ret;
ret = fs_bus_uninit(dev);
- if (ret)
- return ret;
PRIV(dev)->state = DEV_PROBED - 1;
- return 0;
+ return ret;
}
>
> Will now return the first sub device fail value in case it fails.
>
> Fixes: a46f8d58 ("net/failsafe: add fail-safe PMD")
>
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> ---
> drivers/net/failsafe/failsafe_eal.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
> index c8f4318..4295347 100644
> --- a/drivers/net/failsafe/failsafe_eal.c
> +++ b/drivers/net/failsafe/failsafe_eal.c
> @@ -90,19 +90,20 @@ fs_bus_uninit(struct rte_eth_dev *dev)
> {
> struct sub_device *sdev = NULL;
> uint8_t i;
> - int ret;
> + int ret = 0, sdev_ret;
No second declaration on the same line if one is initialized please.
>
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
> - ret = rte_eal_hotplug_remove(sdev->bus->name,
> + sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
> sdev->dev->name);
> - if (ret) {
> + if (sdev_ret) {
> ERROR("Failed to remove requested device %s",
> sdev->dev->name);
I think printing the error code might be useful, at least it would not
be completely lost.
> + ret = (ret ? ret : sdev_ret);
ret is meaningless anyway, as it might represent any sub_device removal
error. I'd set it to -1 if sdev_ret is displayed to avoid any
unnecessary confusion.
> continue;
> }
> sdev->state = DEV_PROBED - 1;
> }
> - return 0;
> + return ret;
> }
>
> int
> --
> 2.7.4
>
Best regards,
--
Gaëtan Rivet
6WIND
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2] net/failsafe: fix failsafe bus uninit return value
2017-08-29 8:18 ` Gaëtan Rivet
@ 2017-08-29 8:55 ` Raslan Darawsheh
2017-08-29 9:02 ` Gaëtan Rivet
2017-08-29 9:08 ` [PATCH v3] " Raslan Darawsheh
0 siblings, 2 replies; 7+ messages in thread
From: Raslan Darawsheh @ 2017-08-29 8:55 UTC (permalink / raw)
To: thomas, gaetan.rivet; +Cc: dev
fs_bus_uninit is always returning 0 no matter what was the status
of each sub device bus_uninit value.
Fixes: a46f8d58 ("net/failsafe: add fail-safe PMD")
Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
---
drivers/net/failsafe/failsafe_eal.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index c8f4318..f6e44ca 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -90,19 +90,21 @@ fs_bus_uninit(struct rte_eth_dev *dev)
{
struct sub_device *sdev = NULL;
uint8_t i;
- int ret;
+ int sdev_ret;
+ int ret = 0;
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
- ret = rte_eal_hotplug_remove(sdev->bus->name,
- sdev->dev->name);
- if (ret) {
- ERROR("Failed to remove requested device %s",
- sdev->dev->name);
+ sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
+ sdev->dev->name);
+ if (sdev_ret) {
+ ERROR("Failed to remove requested device %s"
+ "(errno: %d)", sdev->dev->name, sdev_ret);
+ ret = -1;
continue;
}
sdev->state = DEV_PROBED - 1;
}
- return 0;
+ return ret;
}
int
@@ -111,8 +113,6 @@ failsafe_eal_uninit(struct rte_eth_dev *dev)
int ret;
ret = fs_bus_uninit(dev);
- if (ret)
- return ret;
PRIV(dev)->state = DEV_PROBED - 1;
- return 0;
+ return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] net/failsafe: fix failsafe bus uninit return value
2017-08-29 8:55 ` [PATCH v2] " Raslan Darawsheh
@ 2017-08-29 9:02 ` Gaëtan Rivet
2017-08-29 9:08 ` [PATCH v3] " Raslan Darawsheh
1 sibling, 0 replies; 7+ messages in thread
From: Gaëtan Rivet @ 2017-08-29 9:02 UTC (permalink / raw)
To: Raslan Darawsheh; +Cc: thomas, dev
On Tue, Aug 29, 2017 at 11:55:45AM +0300, Raslan Darawsheh wrote:
> fs_bus_uninit is always returning 0 no matter what was the status
> of each sub device bus_uninit value.
>
> Fixes: a46f8d58 ("net/failsafe: add fail-safe PMD")
>
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> ---
> drivers/net/failsafe/failsafe_eal.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
> index c8f4318..f6e44ca 100644
> --- a/drivers/net/failsafe/failsafe_eal.c
> +++ b/drivers/net/failsafe/failsafe_eal.c
> @@ -90,19 +90,21 @@ fs_bus_uninit(struct rte_eth_dev *dev)
> {
> struct sub_device *sdev = NULL;
> uint8_t i;
> - int ret;
> + int sdev_ret;
> + int ret = 0;
>
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
> - ret = rte_eal_hotplug_remove(sdev->bus->name,
> - sdev->dev->name);
> - if (ret) {
> - ERROR("Failed to remove requested device %s",
> - sdev->dev->name);
> + sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
> + sdev->dev->name);
> + if (sdev_ret) {
> + ERROR("Failed to remove requested device %s"
> + "(errno: %d)", sdev->dev->name, sdev_ret);
The error string should be on one line to help grepping the issue.
The code displayed is not errno, it should not be named as such.
+ ERROR("Failed to remove requested device %s (%d)",
+ sdev->dev->name, sdev_ret);
> + ret = -1;
> continue;
> }
> sdev->state = DEV_PROBED - 1;
> }
> - return 0;
> + return ret;
> }
>
> int
> @@ -111,8 +113,6 @@ failsafe_eal_uninit(struct rte_eth_dev *dev)
> int ret;
>
> ret = fs_bus_uninit(dev);
> - if (ret)
> - return ret;
> PRIV(dev)->state = DEV_PROBED - 1;
> - return 0;
> + return ret;
> }
> --
> 2.7.4
>
Otherwise,
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v3] net/failsafe: fix failsafe bus uninit return value
2017-08-29 8:55 ` [PATCH v2] " Raslan Darawsheh
2017-08-29 9:02 ` Gaëtan Rivet
@ 2017-08-29 9:08 ` Raslan Darawsheh
2017-08-29 9:28 ` Gaëtan Rivet
1 sibling, 1 reply; 7+ messages in thread
From: Raslan Darawsheh @ 2017-08-29 9:08 UTC (permalink / raw)
To: thomas, gaetan.rivet; +Cc: dev
fs_bus_uninit is always returning 0 no matter what was the status
of each sub device bus_uninit value.
Fixes: a46f8d58 ("net/failsafe: add fail-safe PMD")
Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
---
drivers/net/failsafe/failsafe_eal.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index c8f4318..aeb87a0 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -90,19 +90,20 @@ fs_bus_uninit(struct rte_eth_dev *dev)
{
struct sub_device *sdev = NULL;
uint8_t i;
- int ret;
+ int sdev_ret;
+ int ret = 0;
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
- ret = rte_eal_hotplug_remove(sdev->bus->name,
- sdev->dev->name);
- if (ret) {
- ERROR("Failed to remove requested device %s",
- sdev->dev->name);
+ sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
+ sdev->dev->name);
+ if (sdev_ret) {
+ ERROR("Failed to remove requested device %s (err: %d)",
+ sdev->dev->name, sdev_ret);
continue;
}
sdev->state = DEV_PROBED - 1;
}
- return 0;
+ return ret;
}
int
@@ -111,8 +112,6 @@ failsafe_eal_uninit(struct rte_eth_dev *dev)
int ret;
ret = fs_bus_uninit(dev);
- if (ret)
- return ret;
PRIV(dev)->state = DEV_PROBED - 1;
- return 0;
+ return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v3] net/failsafe: fix failsafe bus uninit return value
2017-08-29 9:08 ` [PATCH v3] " Raslan Darawsheh
@ 2017-08-29 9:28 ` Gaëtan Rivet
2017-08-30 9:09 ` Ferruh Yigit
0 siblings, 1 reply; 7+ messages in thread
From: Gaëtan Rivet @ 2017-08-29 9:28 UTC (permalink / raw)
To: Raslan Darawsheh; +Cc: thomas, dev
On Tue, Aug 29, 2017 at 12:08:08PM +0300, Raslan Darawsheh wrote:
> fs_bus_uninit is always returning 0 no matter what was the status
> of each sub device bus_uninit value.
>
> Fixes: a46f8d58 ("net/failsafe: add fail-safe PMD")
>
No need to send another version, but for next time: you could have
included my ack at this point :).
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Thanks,
> ---
> drivers/net/failsafe/failsafe_eal.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
> index c8f4318..aeb87a0 100644
> --- a/drivers/net/failsafe/failsafe_eal.c
> +++ b/drivers/net/failsafe/failsafe_eal.c
> @@ -90,19 +90,20 @@ fs_bus_uninit(struct rte_eth_dev *dev)
> {
> struct sub_device *sdev = NULL;
> uint8_t i;
> - int ret;
> + int sdev_ret;
> + int ret = 0;
>
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
> - ret = rte_eal_hotplug_remove(sdev->bus->name,
> - sdev->dev->name);
> - if (ret) {
> - ERROR("Failed to remove requested device %s",
> - sdev->dev->name);
> + sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
> + sdev->dev->name);
> + if (sdev_ret) {
> + ERROR("Failed to remove requested device %s (err: %d)",
> + sdev->dev->name, sdev_ret);
> continue;
> }
> sdev->state = DEV_PROBED - 1;
> }
> - return 0;
> + return ret;
> }
>
> int
> @@ -111,8 +112,6 @@ failsafe_eal_uninit(struct rte_eth_dev *dev)
> int ret;
>
> ret = fs_bus_uninit(dev);
> - if (ret)
> - return ret;
> PRIV(dev)->state = DEV_PROBED - 1;
> - return 0;
> + return ret;
> }
> --
> 2.7.4
>
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v3] net/failsafe: fix failsafe bus uninit return value
2017-08-29 9:28 ` Gaëtan Rivet
@ 2017-08-30 9:09 ` Ferruh Yigit
0 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2017-08-30 9:09 UTC (permalink / raw)
To: Gaëtan Rivet, Raslan Darawsheh; +Cc: thomas, dev
On 8/29/2017 10:28 AM, Gaëtan Rivet wrote:
> On Tue, Aug 29, 2017 at 12:08:08PM +0300, Raslan Darawsheh wrote:
>> fs_bus_uninit is always returning 0 no matter what was the status
>> of each sub device bus_uninit value.
>>
>> Fixes: a46f8d58 ("net/failsafe: add fail-safe PMD")
Cc: stable@dpdk.org
>>
>> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-08-30 9:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-29 7:51 [PATCH] net/failsafe: fix failsafe bus uninit return value Raslan Darawsheh
2017-08-29 8:18 ` Gaëtan Rivet
2017-08-29 8:55 ` [PATCH v2] " Raslan Darawsheh
2017-08-29 9:02 ` Gaëtan Rivet
2017-08-29 9:08 ` [PATCH v3] " Raslan Darawsheh
2017-08-29 9:28 ` Gaëtan Rivet
2017-08-30 9:09 ` Ferruh Yigit
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.