Re: [dpdk-dev] [dpdk-stable] [PATCH] eal: fix positive error codes from probe/remove

[David Marchand <david.marchand@redhat.com>]

On Thu, May 30, 2019 at 3:26 PM Ilya Maximets <i.maximets@samsung.com>
wrote:

> According to API, 'rte_dev_probe()' and 'rte_dev_remove()' and their
> 'hotplug' equivalents must return 0 or negative error code. Bus code
>

About this first part, existing callers in dpdk are not consistent with the
api which might explain why this was not seen earlier.
How about fixing the existing callers?


returns positive values if device wasn't recognized by any driver, so
> the result of 'bus->plug/unplug()' must be converted.
>

The problem is in local_dev_probe() (resp. local_dev_remove()) itself,
since this internal api announces it should return < 0 on error.



> Positive on remove means that device not found by driver.
> Positive on probe means that there are no suitable buses/drivers,
> i.e. device is not supported.
>
> CC: stable@dpdk.org
> Fixes: a3ee360f4440 ("eal: add hotplug add/remove device")
> Fixes: 244d5130719c ("eal: enable hotplug on multi-process")
>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  lib/librte_eal/common/eal_common_dev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_dev.c
> b/lib/librte_eal/common/eal_common_dev.c
> index 824b8f926..f9cae8e26 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -233,7 +233,7 @@ rte_dev_probe(const char *devargs)
>                  * process.
>                  */
>                 if (ret != -EEXIST)
> -                       return ret;
> +                       return (ret < 0) ? ret : -ENOTSUP;
>         }
>
>         /* primary send attach sync request to secondary. */
> @@ -319,7 +319,7 @@ local_dev_remove(struct rte_device *dev)
>         if (ret) {
>                 RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
>                         dev->name);
> -               return ret;
> +               return (ret < 0) ? ret : -ENOENT;
>         }
>
>         return 0;
> --
> 2.17.1
>
>

-- 
David Marchand