On 9/4/19 7:40 PM, Ferruh Yigit wrote:
On 9/3/2019 2:56 PM, Andrew Rybchenko wrote:
From: Ivan Ilchenko <ivan.ilche...@oktetlabs.com>
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.
Signed-off-by: Ivan Ilchenko <ivan.ilche...@oktetlabs.com>
Signed-off-by: Andrew Rybchenko <arybche...@solarflare.com>
<...>
@@ -1144,7 +1143,9 @@ struct rte_eth_dev *
The diff output seems not able to detect the function/block which makes it
harder to review, same patch I am getting a diff output like below:
@@ -1144,7 +1143,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t
nb_rx_q,
uint16_t nb_tx_q,
Can you please either check a newer version of git, as far as I can see you are
using '1.8.3.1', or can you please try by removing/updating '.gitattributes'
file?
Newer Git version solves the problem.
*/
memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf));
- rte_eth_dev_info_get(port_id, &dev_info);
+ ret = rte_eth_dev_info_get(port_id, &dev_info);
+ if (ret != 0)
+ return ret;
'dev->data->dev_conf' should be restored from 'orig_conf' on failure, before
return.
Thanks, will fix in v3.
<...>
-void
+int
rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
{
struct rte_eth_dev *dev;
@@ -2558,7 +2566,7 @@ struct rte_eth_dev *
*/
memset(dev_info, 0, sizeof(struct rte_eth_dev_info));
- RTE_ETH_VALID_PORTID_OR_RET(port_id);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
dev = &rte_eth_devices[port_id];
dev_info->rx_desc_lim = lim;
@@ -2567,13 +2575,15 @@ struct rte_eth_dev *
dev_info->min_mtu = RTE_ETHER_MIN_MTU;
dev_info->max_mtu = UINT16_MAX;
- RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
(*dev->dev_ops->dev_infos_get)(dev, dev_info);
dev_info->driver_name = dev->device->driver->name;
dev_info->nb_rx_queues = dev->data->nb_rx_queues;
dev_info->nb_tx_queues = dev->data->nb_tx_queues;
dev_info->dev_flags = &dev->data->dev_flags;
+
+ return 0;
Should API use "return eth_err(port_id, ret);" syntax, as other APIs do,
I am not very clear about the 'eth_err()', Thomas can provide better
description/suggestion on this.
In this particular case eth_err() is irrelevant. It makes sense when
callback is updated to return int and I'll use it there.
In any case better description would be useful.
<...>
@@ -3100,9 +3118,13 @@ struct rte_eth_dev *
struct rte_eth_dev_info dev_info;
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
unsigned i;
+ int ret;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
- rte_eth_dev_info_get(port_id, &dev_info);
+
+ ret = rte_eth_dev_info_get(port_id, &dev_info);
+ if (ret != 0)
+ return ret;
This is 'get_mac_addr_index()' static function, and it should either send
negative value for error, or 'index' value in case of success. So should we be
sure here only negative value sent? Incase 'rte_eth_dev_info_get()' returns
positive value in the future.
In fact, I think there is no necessity to check port_id here since it is
a static
function and port_id is now checked in rte_eth_dev_info_get() anyway.
Also, I think it is better to return -1 here since it is bad to mix
-errno and -1
return codes. -errno would be useful if caller really takes it into
account and
have different processing for -ENOENT which makes sense in this case.
for (i = 0; i < dev_info.max_mac_addrs; i++)
if (memcmp(addr, &dev->data->mac_addrs[i],
@@ -3233,8 +3255,14 @@ struct rte_eth_dev *
struct rte_eth_dev_info dev_info;
struct rte_eth_dev *dev = &rte_eth_devices[port_id];
unsigned i;
+ int ret;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ ret = rte_eth_dev_info_get(port_id, &dev_info);
+ if (ret != 0)
+ return ret;
Same comment as above, for this 'get_hash_mac_addr_index()' static function.
Same as above.
Many thanks for review.
