On 8/28/19 12:51 PM, Jan Viktorin wrote:
On Tue, 27 Aug 2019 15:25:12 +0100
Andrew Rybchenko <arybche...@solarflare.com> wrote:
From: Ivan Ilchenko <ivan.ilche...@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...
The cover letter [1] was sent together with the patch.
[1] http://mails.dpdk.org/archives/dev/2019-August/141593.html
Signed-off-by: Ivan Ilchenko <ivan.ilche...@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybche...@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.
I agree that dev_infos_get() callback should be mandatory, but
what the function should do if the callback is not provided?
I think that a sane default contents is more harm than an error
(basically that's what we had before the patch).
Since the function may return error, caller should take it into
account anyway. Yes, some error codes could have special
handling, but default error handling is required in any case.
