[v9,1/3] ethdev: add an API to get device configuration info
Checks
Commit Message
This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
device configuration info.
Cc: stable@dpdk.org
Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
lib/ethdev/rte_ethdev.c | 23 +++++++++++++++++++++++
lib/ethdev/rte_ethdev.h | 21 +++++++++++++++++++++
lib/ethdev/version.map | 3 +++
3 files changed, 47 insertions(+)
Comments
26/09/2021 11:20, Jie Wang:
> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
> device configuration info.
>
> Cc: stable@dpdk.org
No we don't backport new features.
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
[...]
> + * Retrieve the configuration 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_conf* to be filled with
> + * the configuration of the Ethernet device.
> + * And the memory of the structure should be allocated by the caller.
> + * @return
> + * - (0) if successful.
> + * - (-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);
It does not make sense to me.
rte_eth_conf is passed by the app to rte_eth_dev_configure.
Why the app would need to get the same info back?
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, September 27, 2021 2:19 PM
> To: Wang, Jie1X <jie1x.wang@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>;
> andrew.rybchenko@oktetlabs.ru; Li, Xiaoyun <xiaoyun.li@intel.com>; Yang,
> SteveX <stevex.yang@intel.com>
> Subject: Re: [PATCH v9 1/3] ethdev: add an API to get device configuration info
>
> 26/09/2021 11:20, Jie Wang:
> > This patch adds a new API "rte_eth_dev_conf_info_get()" to help users
> > get device configuration info.
> >
> > Cc: stable@dpdk.org
>
> No we don't backport new features.
Ok, I'll remove 'Cc: stable@dpdk.org'.
>
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> [...]
> > + * Retrieve the configuration 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_conf* to be filled with
> > + * the configuration of the Ethernet device.
> > + * And the memory of the structure should be allocated by the caller.
> > + * @return
> > + * - (0) if successful.
> > + * - (-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);
>
> It does not make sense to me.
> rte_eth_conf is passed by the app to rte_eth_dev_configure.
> Why the app would need to get the same info back?
>
>
In rte_eth_dev_configure, dev->data->dev_conf copies the info from port->dev_conf, and then the driver updates it. It doesn't same as port->dev_conf.
We need to get the updated device configuration.
27/09/2021 09:21, Wang, Jie1X:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 26/09/2021 11:20, Jie Wang:
> > > This patch adds a new API "rte_eth_dev_conf_info_get()" to help users
> > > get device configuration info.
> > [...]
> > > + * Retrieve the configuration 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_conf* to be filled with
> > > + * the configuration of the Ethernet device.
> > > + * And the memory of the structure should be allocated by the caller.
> > > + * @return
> > > + * - (0) if successful.
> > > + * - (-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);
> >
> > It does not make sense to me.
> > rte_eth_conf is passed by the app to rte_eth_dev_configure.
> > Why the app would need to get the same info back?
> >
> >
>
> In rte_eth_dev_configure, dev->data->dev_conf copies the info from port->dev_conf, and then the driver updates it. It doesn't same as port->dev_conf.
> We need to get the updated device configuration.
OK I see.
Please update the commit log to explain this.
On 9/27/2021 8:56 AM, Thomas Monjalon wrote:
> 27/09/2021 09:21, Wang, Jie1X:
>> From: Thomas Monjalon <thomas@monjalon.net>
>>> 26/09/2021 11:20, Jie Wang:
>>>> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users
>>>> get device configuration info.
>>> [...]
>>>> + * Retrieve the configuration 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_conf* to be filled with
>>>> + * the configuration of the Ethernet device.
>>>> + * And the memory of the structure should be allocated by the caller.
>>>> + * @return
>>>> + * - (0) if successful.
>>>> + * - (-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);
>>>
>>> It does not make sense to me.
>>> rte_eth_conf is passed by the app to rte_eth_dev_configure.
>>> Why the app would need to get the same info back?
>>>
>>>
>>
>> In rte_eth_dev_configure, dev->data->dev_conf copies the info from port->dev_conf, and then the driver updates it. It doesn't same as port->dev_conf.
>> We need to get the updated device configuration.
>
> OK I see.
> Please update the commit log to explain this.
>
Also either an application needs to keep copy of the configuration (like testpmd
does), or won't have any way to know device configuration details.
And for the apps that keeps the configuration, it has a risk that application
copy and device copy of the configuration diverged, as Jie mentioned.
I think it makes sense to have a way to get the configuration from device, small
applications can rely on it without keeping a copy of a config at all.
And for testpmd, we have aligned with Xiaoyun to rely on the device
configuration more, in a way:
- When to display a config, use device copy as much as possible
- Use app copy of config to accumulate user config change requests to apply them
later, sync app config with device config after config applied.
On 9/26/2021 10:20 AM, Jie Wang wrote:
> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
> device configuration info.
>
> Cc: stable@dpdk.org
>
Since this is a new API, I think we can request it to be backported.
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
<...>
> @@ -247,6 +247,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;
Not sure about the 'info' part in the API, what about 'rte_eth_dev_conf_get()'?
04/10/2021 13:20, Ferruh Yigit:
> On 9/27/2021 8:56 AM, Thomas Monjalon wrote:
> > 27/09/2021 09:21, Wang, Jie1X:
> >> From: Thomas Monjalon <thomas@monjalon.net>
> >>> 26/09/2021 11:20, Jie Wang:
> >>>> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users
> >>>> get device configuration info.
> >>> [...]
> >>>> + * Retrieve the configuration 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_conf* to be filled with
> >>>> + * the configuration of the Ethernet device.
> >>>> + * And the memory of the structure should be allocated by the caller.
> >>>> + * @return
> >>>> + * - (0) if successful.
> >>>> + * - (-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);
> >>>
> >>> It does not make sense to me.
> >>> rte_eth_conf is passed by the app to rte_eth_dev_configure.
> >>> Why the app would need to get the same info back?
> >>>
> >>>
> >>
> >> In rte_eth_dev_configure, dev->data->dev_conf copies the info from port->dev_conf, and then the driver updates it. It doesn't same as port->dev_conf.
> >> We need to get the updated device configuration.
> >
> > OK I see.
> > Please update the commit log to explain this.
> >
>
> Also either an application needs to keep copy of the configuration (like testpmd
> does), or won't have any way to know device configuration details.
> And for the apps that keeps the configuration, it has a risk that application
> copy and device copy of the configuration diverged, as Jie mentioned.
>
> I think it makes sense to have a way to get the configuration from device, small
> applications can rely on it without keeping a copy of a config at all.
>
> And for testpmd, we have aligned with Xiaoyun to rely on the device
> configuration more, in a way:
> - When to display a config, use device copy as much as possible
> - Use app copy of config to accumulate user config change requests to apply them
> later, sync app config with device config after config applied.
Makes sense, thanks.
04/10/2021 13:22, Ferruh Yigit:
> On 9/26/2021 10:20 AM, Jie Wang wrote:
> > This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
> > device configuration info.
> >
> > Cc: stable@dpdk.org
> >
>
> Since this is a new API, I think we can request it to be backported.
We cannot.
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
>
> <...>
>
> > @@ -247,6 +247,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;
>
> Not sure about the 'info' part in the API, what about 'rte_eth_dev_conf_get()'?
+1
On 10/4/2021 12:26 PM, Thomas Monjalon wrote:
> 04/10/2021 13:22, Ferruh Yigit:
>> On 9/26/2021 10:20 AM, Jie Wang wrote:
>>> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
>>> device configuration info.
>>>
>>> Cc: stable@dpdk.org
>>>
>>
>> Since this is a new API, I think we can request it to be backported.
>
> We cannot.
>
Of course, it is a typo in my end, I mean "we can NOT request ..."
>>> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
>>
>> <...>
>>
>>> @@ -247,6 +247,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;
>>
>> Not sure about the 'info' part in the API, what about 'rte_eth_dev_conf_get()'?
>
> +1
>
>
@@ -3457,6 +3457,29 @@ 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",
+ port_id);
+ return -EINVAL;
+ }
+
+ /* 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)
@@ -3068,6 +3068,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 configuration 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_conf* to be filled with
+ * the configuration of the Ethernet device.
+ * And the memory of the structure should be allocated by the caller.
+ * @return
+ * - (0) if successful.
+ * - (-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.
*
@@ -247,6 +247,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 {