Re: [dpdk-dev] [PATCH v6 1/2] ethdev: add an API to get device configuration info

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 8/24/2021 7:19 PM, Jie Wang wrote:
> This patch adds a new API "rte_eth_dev_conf_info_get()" to help testpmd get
> device configuration info.
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h | 26 ++++++++++++++++++++++++++
>  lib/ethdev/version.map  |  3 +++
>  3 files changed, 56 insertions(+)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 9d95cd11e1..74184099a1 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3458,6 +3458,33 @@ 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_dev_conf_info *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 config info to NULL\n",
> +			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_dev_conf_info));
> +
> +	dev_conf_info->rx_offloads = dev->data->dev_conf.rxmode.offloads;
> +	dev_conf_info->tx_offloads = dev->data->dev_conf.txmode.offloads;
> +
> +	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..70a2db550f 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1587,6 +1587,15 @@ struct rte_eth_dev_info {
>  	void *reserved_ptrs[2];   /**< Reserved for future fields */
>  };
>  
> +/**
> + * Ethernet device configuration information structure.
> + * Used to retrieve information about configured device.
> + */
> +struct rte_eth_dev_conf_info {
> +	uint64_t rx_offloads; /**rxmode offloads */
> +	uint64_t tx_offloads; /**txmode offloads */
> +};

My concern is if we need to extend this struct later, when application wants to
get more current config from the dpdk layer, it will cause ABI break and will
need to wait next LTS.

And as this struct grow, it will be kind of duplication of the 'struct
rte_eth_conf'.

What do you think to reuse 'struct rte_eth_conf' in this API, to cover future needs?

> +
>  /**
>   * RX/TX queue states
>   */
> @@ -3058,6 +3067,23 @@ 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);
>  
> +/**
> + * Retrieve the contextual information of an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param dev_conf_info
> + *   A pointer to a structure of type *rte_eth_dev_conf_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.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +int rte_eth_dev_conf_info_get(uint16_t port_id,
> +				struct rte_eth_dev_conf_info *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 {
>