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

[Ilya Maximets <i.maximets@samsung.com>]

On 03.06.2019 11:50, David Marchand wrote:
> 
> 
> On Thu, May 30, 2019 at 3:26 PM Ilya Maximets <i.maximets@samsung.com <mailto: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?

Do you mean replacing all the 'rte_dev_probe() != 0' with 'rte_dev_probe() < 0'
around the codebase?

> 
> 
>     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 <mailto: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 <mailto: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