Re: [dpdk-dev] [PATCH v20.11 2/2] eal: improve device probing API

[Maxime Coquelin <maxime.coquelin@redhat.com>]

Hi Gaëtan,

On 6/10/20 2:06 PM, Gaëtan Rivet wrote:
> Hello Maxime,
> 
> On 08/06/20 17:53 +0200, Maxime Coquelin wrote:
>> This patch makes rte_dev_probe() to return the
>> rte_device pointer on success and NULL on error
>> instead of returning 0 on success and negative
>> value on error.
>>
>> The goal is to avoid that the calling application
>> iterates the devices list afterwards to retrieve
>> the pointer. Retrieving the pointer is required
>> for calling rte_dev_remove() later.
>>
> 
> I agree with the idea. I recall starting to do it on the legacy functions
> (rte_eal_hotplug_*), but it was scrapped for API compat.
> 
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  app/test-pmd/testpmd.c                 |  2 +-
>>  drivers/net/failsafe/failsafe.c        |  5 +++--
>>  lib/librte_eal/common/eal_common_dev.c | 18 ++++++++++--------
>>  lib/librte_eal/include/rte_dev.h       |  4 ++--
>>  4 files changed, 16 insertions(+), 13 deletions(-)
>>
...
>>  
>>  int
>> diff --git a/lib/librte_eal/include/rte_dev.h b/lib/librte_eal/include/rte_dev.h
>> index c8d985fb5c..9cf7c7fd71 100644
>> --- a/lib/librte_eal/include/rte_dev.h
>> +++ b/lib/librte_eal/include/rte_dev.h
>> @@ -148,9 +148,9 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
>>   * @param devargs
>>   *   Device arguments including bus, class and driver properties.
>>   * @return
>> - *   0 on success, negative on error.
>> + *   Generic device pointer on success, NULL on error.
> 
> If rte_errno is set, mapping codes to meanings would be helpful here.

Actually David made me the same comment before I post the patch.
I am fine with setting rte_errno. If we do that, I think we should have
fixed error code in rte_dev_probe() and not propagate error codes from
functions it calls. Otherwise it's likely the API doc will be outdated
quite rapidly.

What do you think?

> Acked-by: Gaetan Rivet <grive@u256.net>
> 

Great, could you also ack the deprecation notice, as this is the part
that needs to be merged into v20.08?

Thanks!
Maxime