[dpdk-dev] [PATCH 01/51] ethdev: change rte_eth_dev_info_get() return value to int

[Jan Viktorin]

On Tue, 27 Aug 2019 15:25:12 +0100
Andrew Rybchenko <arybchenko at solarflare.com> wrote:

> From: Ivan Ilchenko <Ivan.Ilchenko at oktetlabs.ru>
> 
> Change rte_eth_dev_info_get() return value from void to int and return
> negative errno values in case of error conditions.
> Modify rte_eth_dev_info_get() usage across the ethdev according
> to new return type.

Hello Andrew,

I didn't find any cover letter describing the true intentions of this
patchset. Anyway, see below a short comment...

> 
> Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko at oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
> ---
>  doc/guides/rel_notes/deprecation.rst   |  1 -
>  doc/guides/rel_notes/release_19_11.rst |  5 ++-
>  lib/librte_ethdev/rte_ethdev.c         | 71
> ++++++++++++++++++++++++---------- lib/librte_ethdev/rte_ethdev.h
>     |  6 ++- 4 files changed, 60 insertions(+), 23 deletions(-)

[...]

> b/lib/librte_ethdev/rte_ethdev.h index dc6596b..09c278d 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -2366,8 +2366,12 @@ int
> rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id,
>   * @param dev_info
>   *   A pointer to a structure of type *rte_eth_dev_info* to be
> filled with
>   *   the contextual information of the Ethernet device.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if support for dev_infos_get() does not exist for
> the device.

I believe that allowing PMDs to return -ENOTSUP is not a good idea.
At the moment, all PMDs provides this kind of information. It is not
always very reliable piece of information but for me, it is a piece of
gold I would not like to loose when configuring devices.

I think it should be mandatory for all PMDs to provide this function.
Another possible way, give a sane default contents of this structure.
But, please, do not return -ENOTSUP.

Regards
Jan

> + *   - (-ENODEV) if *port_id* invalid.
>   */
> -void rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info
> *dev_info); +int rte_eth_dev_info_get(uint16_t port_id, struct
> rte_eth_dev_info *dev_info); 
>  /**
>   * Retrieve the firmware version of a device.