[dpdk-dev,v4,1/5] ethdev: add access to eeprom
Checks
Commit Message
add new APIs:
- rte_eth_dev_get_module_info
- rte_eth_dev_get_module_eeprom
Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
Acked-by: Remy Horton <remy.horton@intel.com>
---
Cc: remy.horton@intel.com
Cc: john.mcnamara@intel.com
Cc: marko.kovacevic@intel.com
Cc: thomas@monjalon.net
doc/guides/nics/features.rst | 11 ++++++++
lib/librte_ether/rte_dev_info.h | 18 +++++++++++++
lib/librte_ether/rte_ethdev.c | 26 +++++++++++++++++++
lib/librte_ether/rte_ethdev.h | 43 +++++++++++++++++++++++++++++++
lib/librte_ether/rte_ethdev_core.h | 12 +++++++++
lib/librte_ether/rte_ethdev_version.map | 2 ++
6 files changed, 112 insertions(+)
Comments
24/04/2018 12:17, Zijie Pan:
> +/**
> + * Placeholder for accessing plugin module eeprom
> + */
> +struct rte_dev_module_info {
> + uint32_t type; /**< Type of plugin module eeprom */
> + uint32_t eeprom_len; /**< Length of plugin module eeprom */
> +};
I am not sure "plugin module" is descriptive enough.
And I think the structure name should be rte_eth_dev_module_info
to make clear that we are talking about NIC modules.
Any better idea?
> +
> +/* EEPROM Standards for plug in modules */
> +#define RTE_ETH_MODULE_SFF_8079 0x1
> +#define RTE_ETH_MODULE_SFF_8079_LEN 256
> +#define RTE_ETH_MODULE_SFF_8472 0x2
> +#define RTE_ETH_MODULE_SFF_8472_LEN 512
> +#define RTE_ETH_MODULE_SFF_8636 0x3
> +#define RTE_ETH_MODULE_SFF_8636_LEN 256
> +#define RTE_ETH_MODULE_SFF_8436 0x4
> +#define RTE_ETH_MODULE_SFF_8436_LEN 256
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -3947,6 +3947,32 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
> return eth_err(port_id, (*dev->dev_ops->set_eeprom)(dev, info));
> }
>
> +int __rte_experimental
> +rte_eth_dev_get_module_info(uint16_t port_id,
> + struct rte_dev_module_info *modinfo)
> +{
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> + dev = &rte_eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_module_info, -ENOTSUP);
> + return (*dev->dev_ops->get_module_info)(dev, modinfo);
> +}
> +
> +int __rte_experimental
> +rte_eth_dev_get_module_eeprom(uint16_t port_id,
> + struct rte_dev_eeprom_info *info)
> +{
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> + dev = &rte_eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_module_eeprom, -ENOTSUP);
> + return (*dev->dev_ops->get_module_eeprom)(dev, info);
> +}
> +
> int
> rte_eth_dev_get_dcb_info(uint16_t port_id,
Please move this code after other EEPROM related functions.
> --- a/lib/librte_ether/rte_ethdev_version.map
> +++ b/lib/librte_ether/rte_ethdev_version.map
> @@ -229,5 +229,7 @@ EXPERIMENTAL {
> rte_mtr_policer_actions_update;
> rte_mtr_stats_read;
> rte_mtr_stats_update;
> + rte_eth_dev_get_module_info;
> + rte_eth_dev_get_module_eeprom;
This must be inserted in alphabetical order.
Thanks
Hi Thomas,
> > +/**
> > + * Placeholder for accessing plugin module eeprom
> > + */
> > +struct rte_dev_module_info {
> > + uint32_t type; /**< Type of plugin module eeprom */
> > + uint32_t eeprom_len; /**< Length of plugin module eeprom */
> > +};
>
> I am not sure "plugin module" is descriptive enough.
Here is the description when "man ethtool":
-m --dump-module-eeprom --module-info
Retrieves and if possible decodes the EEPROM from plugin modules, e.g SFP+, QSFP.
Is there any suggestion how to describe it?
> And I think the structure name should be rte_eth_dev_module_info
> to make clear that we are talking about NIC modules.
> Any better idea?
Yes, rte_eth_dev_module_info is better than rte_dev_module_info.
But the name of other structures should be also changed. And the file name
should be changed to rte_eth_dev_info.h.
Do we need to make a patch first to change the name of the file and structures?
> > +int __rte_experimental
> > +rte_eth_dev_get_module_eeprom(uint16_t port_id,
> > + struct rte_dev_eeprom_info *info)
> > +{
> > + struct rte_eth_dev *dev;
> > +
> > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +
> > + dev = &rte_eth_devices[port_id];
> > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_module_eeprom, -ENOTSUP);
> > + return (*dev->dev_ops->get_module_eeprom)(dev, info);
> > +}
> > +
> > int
> > rte_eth_dev_get_dcb_info(uint16_t port_id,
>
> Please move this code after other EEPROM related functions.
I put the functions after rte_eth_dev_set_eeprom(). Please apply the patch and check it.
> > --- a/lib/librte_ether/rte_ethdev_version.map
> > +++ b/lib/librte_ether/rte_ethdev_version.map
> > @@ -229,5 +229,7 @@ EXPERIMENTAL {
> > rte_mtr_policer_actions_update;
> > rte_mtr_stats_read;
> > rte_mtr_stats_update;
> > + rte_eth_dev_get_module_info;
> > + rte_eth_dev_get_module_eeprom;
>
> This must be inserted in alphabetical order.
I will update it in v5 patchset.
Thanks & Regards,
Zijie
25/04/2018 10:21, Zijie Pan:
> Hi Thomas,
>
> > > +/**
> > > + * Placeholder for accessing plugin module eeprom
> > > + */
> > > +struct rte_dev_module_info {
> > > + uint32_t type; /**< Type of plugin module eeprom */
> > > + uint32_t eeprom_len; /**< Length of plugin module eeprom */
> > > +};
> >
> > I am not sure "plugin module" is descriptive enough.
>
> Here is the description when "man ethtool":
> -m --dump-module-eeprom --module-info
> Retrieves and if possible decodes the EEPROM from plugin modules, e.g SFP+, QSFP.
> Is there any suggestion how to describe it?
No better idea.
> > And I think the structure name should be rte_eth_dev_module_info
> > to make clear that we are talking about NIC modules.
> > Any better idea?
>
> Yes, rte_eth_dev_module_info is better than rte_dev_module_info.
> But the name of other structures should be also changed. And the file name
> should be changed to rte_eth_dev_info.h.
> Do we need to make a patch first to change the name of the file and structures?
No you can just focus on your change.
If more clean-up is required in your opinion, you can do it later.
> > > +int __rte_experimental
> > > +rte_eth_dev_get_module_eeprom(uint16_t port_id,
> > > + struct rte_dev_eeprom_info *info)
> > > +{
> > > + struct rte_eth_dev *dev;
> > > +
> > > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > > +
> > > + dev = &rte_eth_devices[port_id];
> > > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_module_eeprom, -ENOTSUP);
> > > + return (*dev->dev_ops->get_module_eeprom)(dev, info);
> > > +}
> > > +
> > > int
> > > rte_eth_dev_get_dcb_info(uint16_t port_id,
> >
> > Please move this code after other EEPROM related functions.
> I put the functions after rte_eth_dev_set_eeprom(). Please apply the patch and check it.
OK
> > > --- a/lib/librte_ether/rte_ethdev_version.map
> > > +++ b/lib/librte_ether/rte_ethdev_version.map
> > > @@ -229,5 +229,7 @@ EXPERIMENTAL {
> > > rte_mtr_policer_actions_update;
> > > rte_mtr_stats_read;
> > > rte_mtr_stats_update;
> > > + rte_eth_dev_get_module_info;
> > > + rte_eth_dev_get_module_eeprom;
> >
> > This must be inserted in alphabetical order.
> I will update it in v5 patchset.
Please send v5 ASAP, thanks.
@@ -749,6 +749,17 @@ Supports getting/setting device eeprom data.
``rte_eth_dev_set_eeprom()``.
+.. _nic_features_module_eeprom_dump:
+
+Module EEPROM dump
+------------------
+
+Supports getting information and data of plugin module eeprom.
+
+* **[implements] eth_dev_ops**: ``get_module_info``, ``get_module_eeprom``.
+* **[related] API**: ``rte_eth_dev_get_module_info()``, ``rte_eth_dev_get_module_eeprom()``.
+
+
.. _nic_features_register_dump:
Registers dump
@@ -28,4 +28,22 @@ struct rte_dev_eeprom_info {
uint32_t magic; /**< Device-specific key, such as device-id */
};
+/**
+ * Placeholder for accessing plugin module eeprom
+ */
+struct rte_dev_module_info {
+ uint32_t type; /**< Type of plugin module eeprom */
+ uint32_t eeprom_len; /**< Length of plugin module eeprom */
+};
+
+/* EEPROM Standards for plug in modules */
+#define RTE_ETH_MODULE_SFF_8079 0x1
+#define RTE_ETH_MODULE_SFF_8079_LEN 256
+#define RTE_ETH_MODULE_SFF_8472 0x2
+#define RTE_ETH_MODULE_SFF_8472_LEN 512
+#define RTE_ETH_MODULE_SFF_8636 0x3
+#define RTE_ETH_MODULE_SFF_8636_LEN 256
+#define RTE_ETH_MODULE_SFF_8436 0x4
+#define RTE_ETH_MODULE_SFF_8436_LEN 256
+
#endif /* _RTE_DEV_INFO_H_ */
@@ -3947,6 +3947,32 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
return eth_err(port_id, (*dev->dev_ops->set_eeprom)(dev, info));
}
+int __rte_experimental
+rte_eth_dev_get_module_info(uint16_t port_id,
+ struct rte_dev_module_info *modinfo)
+{
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_module_info, -ENOTSUP);
+ return (*dev->dev_ops->get_module_info)(dev, modinfo);
+}
+
+int __rte_experimental
+rte_eth_dev_get_module_eeprom(uint16_t port_id,
+ struct rte_dev_eeprom_info *info)
+{
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_module_eeprom, -ENOTSUP);
+ return (*dev->dev_ops->get_module_eeprom)(dev, info);
+}
+
int
rte_eth_dev_get_dcb_info(uint16_t port_id,
struct rte_eth_dcb_info *dcb_info)
@@ -3311,6 +3311,49 @@ int rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id,
int rte_eth_dev_set_eeprom(uint16_t port_id, struct rte_dev_eeprom_info *info);
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Retrieve the type and size of plugin module EEPROM
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param modinfo
+ * The type and size of plugin module EEPROM.
+ * @return
+ * - (0) if successful.
+ * - (-ENOTSUP) if hardware doesn't support.
+ * - (-ENODEV) if *port_id* invalid.
+ * - (-EIO) if device is removed.
+ * - others depends on the specific operations implementation.
+ */
+int __rte_experimental
+rte_eth_dev_get_module_info(uint16_t port_id,
+ struct rte_dev_module_info *modinfo);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Retrieve the data of plugin module EEPROM
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param info
+ * The template includes the plugin module EEPROM attributes, and the
+ * buffer for return plugin module EEPROM data.
+ * @return
+ * - (0) if successful.
+ * - (-ENOTSUP) if hardware doesn't support.
+ * - (-ENODEV) if *port_id* invalid.
+ * - (-EIO) if device is removed.
+ * - others depends on the specific operations implementation.
+ */
+int __rte_experimental
+rte_eth_dev_get_module_eeprom(uint16_t port_id,
+ struct rte_dev_eeprom_info *info);
+
+/**
* Set the list of multicast addresses to filter on an Ethernet device.
*
* @param port_id
@@ -337,6 +337,14 @@ typedef int (*eth_set_eeprom_t)(struct rte_eth_dev *dev,
struct rte_dev_eeprom_info *info);
/**< @internal Program eeprom data */
+typedef int (*eth_get_module_info_t)(struct rte_eth_dev *dev,
+ struct rte_dev_module_info *modinfo);
+/**< @internal Retrieve type and size of plugin module eeprom */
+
+typedef int (*eth_get_module_eeprom_t)(struct rte_eth_dev *dev,
+ struct rte_dev_eeprom_info *info);
+/**< @internal Retrieve plugin module eeprom data */
+
typedef int (*eth_l2_tunnel_eth_type_conf_t)
(struct rte_eth_dev *dev, struct rte_eth_l2_tunnel_conf *l2_tunnel);
/**< @internal config l2 tunnel ether type */
@@ -467,6 +475,10 @@ struct eth_dev_ops {
eth_get_eeprom_t get_eeprom; /**< Get eeprom data. */
eth_set_eeprom_t set_eeprom; /**< Set eeprom. */
+ eth_get_module_info_t get_module_info;
+ /** Get plugin module eeprom attribute. */
+ eth_get_module_eeprom_t get_module_eeprom;
+ /** Get plugin module eeprom data. */
eth_filter_ctrl_t filter_ctrl; /**< common filter control. */
@@ -229,5 +229,7 @@ EXPERIMENTAL {
rte_mtr_policer_actions_update;
rte_mtr_stats_read;
rte_mtr_stats_update;
+ rte_eth_dev_get_module_info;
+ rte_eth_dev_get_module_eeprom;
} DPDK_18.02;