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?
> */
> 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.
<...>
> -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.
<...>
> @@ -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.
>
> 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.
