On 8/27/2021 9:17 AM, Jie Wang wrote:
> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
> device configuration info.
>
> Signed-off-by: Jie Wang <jie1x.w...@intel.com>
> ---
> lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++++++++
> lib/ethdev/rte_ethdev.h | 21 +++++++++++++++++++++
> lib/ethdev/version.map | 3 +++
> 3 files changed, 52 insertions(+)
>
Can you add a release notes update for this?
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 9d95cd11e1..bbe24af662 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3458,6 +3458,34 @@ rte_eth_dev_info_get(uint16_t port_id, struct
> rte_eth_dev_info *dev_info)
> return 0;
> }
>
> +int
> +rte_eth_dev_conf_info_get(uint16_t port_id,
> + struct rte_eth_conf *dev_conf_info)
> +{
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> + dev = &rte_eth_devices[port_id];
> +
> + if (dev_conf_info == NULL) {
> + RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u configuration
> info to NULL\n",
Can you break the line after 'ERR', to reduce the column length?
> + port_id);
> + return -EINVAL;
> + }
> +
> + /*
> + * Init dev_conf_info before port_id check since caller does not have
> + * return status and does not know if get is successful or not.
> + */
> + memset(dev_conf_info, 0, sizeof(struct rte_eth_conf));
> +
I didn't get the reasoning for the memset, won't it be overwriten anyway by
below 'memcpy'?
> + /* copy dev->data->dev_conf to dev_conf_info */
> + memcpy(dev_conf_info, &dev->data->dev_conf,
> + sizeof(struct rte_eth_conf));
> +
> + return 0;
> +}
> +
> int
> rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
> uint32_t *ptypes, int num)
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index d2b27c351f..3c5b02a133 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -3058,6 +3058,27 @@ int rte_eth_macaddr_get(uint16_t port_id, struct
> rte_ether_addr *mac_addr);
> */
> int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info
> *dev_info);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Retrieve the contextual information of an Ethernet device.
> + *
'rte_eth_dev_info_get()' is using "contextual information" but for this API can
we say 'configuration'?
> + * @param port_id
> + * The port identifier of the Ethernet device.
> + * @param dev_conf_info
> + * A pointer to a structure of type *rte_eth_dev* to be filled with
> + * the contextual information of the Ethernet device.
Just to make it more clear, what do you think to highlight that memory should be
allocated by the caller?
> + * @return
> + * - (0) if successful.
> + * - (-ENOTSUP) if support for dev_infos_get() does not exist for the
> device.
I think '-ENOTSUP' is not returned by the API.
> + * - (-ENODEV) if *port_id* invalid.
> + * - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_dev_conf_info_get(uint16_t port_id,
> + struct rte_eth_conf *dev_conf_info);
> +
> /**
> * Retrieve the firmware version of a device.
> *
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 44d30b05ae..40539f99f9 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -249,6 +249,9 @@ EXPERIMENTAL {
> rte_mtr_meter_policy_delete;
> rte_mtr_meter_policy_update;
> rte_mtr_meter_policy_validate;
> +
> + # added in 21.11
> + rte_eth_dev_conf_info_get;
> };
>
> INTERNAL {
>
