diff mbox series

[v6,1/2] ethdev: add an API to get device configuration info

Message ID 20210824181929.142691-2-jie1x.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v6,1/2] ethdev: add an API to get device configuration info |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing fail build patch failure

Commit Message

Jie Wang Aug. 24, 2021, 6:19 p.m. UTC 
  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(+)

Comments

Ferruh Yigit Aug. 25, 2021, 8:07 p.m. UTC | #1 
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 {
>
Ajit Khaparde Aug. 26, 2021, 6 a.m. UTC | #2 
On Wed, Aug 25, 2021 at 1:08 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> 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?
+1
rte_eth_conf gives all the information needed and more for future enhancements!

>
> > +
> >  /**
> >   * 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 {
> >
>
diff mbox series

Patch

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 */
+};
+
 /**
  * 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 {