| Message ID | 1614884228-8542-2-git-send-email-moshe@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ethtool: Extend module EEPROM dump API | expand |
On Thu, Mar 04, 2021 at 10:57AM-0800, Moshe Shemesh wrote: > From: Vladyslav Tarasiuk <vladyslavt@nvidia.com> > > Define get_module_eeprom_data_by_page() ethtool callback and > implement netlink infrastructure. > > get_module_eeprom_data_by_page() allows network drivers to dump a > part of module's EEPROM specified by page and bank numbers along with > offset and length. It is effectively a netlink replacement for > get_module_info() and get_module_eeprom() pair, which is needed due to > emergence of complex non-linear EEPROM layouts. > > Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com> > --- > include/linux/ethtool.h | 7 +- > include/uapi/linux/ethtool.h | 26 +++++ > include/uapi/linux/ethtool_netlink.h | 19 ++++ > net/ethtool/Makefile | 2 +- > net/ethtool/eeprom.c | 157 +++++++++++++++++++++++++++ > net/ethtool/netlink.c | 10 ++ > net/ethtool/netlink.h | 2 + > 7 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 > net/ethtool/eeprom.c > > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index > ec4cd3921c67..2f65aae5f492 100644 > --- a/include/linux/ethtool.h > +++ b/include/linux/ethtool.h > @@ -81,6 +81,7 @@ enum { > #define ETH_RSS_HASH_NO_CHANGE 0 > > struct net_device; > +struct netlink_ext_ack; > > /* Some generic methods drivers may use in their ethtool_ops */ > u32 ethtool_op_get_link(struct net_device *dev); @@ -410,6 +411,8 @@ > struct ethtool_pause_stats { > * @get_ethtool_phy_stats: Return extended statistics about the PHY > device. > * This is only useful if the device maintains PHY statistics and > * cannot use the standard PHY library helpers. > + * @get_module_eeprom_data_by_page: Get a region of plug-in module > EEPROM data > + * from specified page. Returns a negative error code or zero. > * > * All operations are optional (i.e. the function pointer may be set > * to %NULL) and callers must take this into account. Callers must @@ -515,6 > +518,9 @@ struct ethtool_ops { > const struct ethtool_tunable *, void *); > int (*set_phy_tunable)(struct net_device *, > const struct ethtool_tunable *, const void > *); > + int (*get_module_eeprom_data_by_page)(struct net_device > *dev, > + const struct > ethtool_eeprom_data *page, > + struct netlink_ext_ack > *extack); > }; > > int ethtool_check_ops(const struct ethtool_ops *ops); @@ -538,7 +544,6 > @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev, > const struct ethtool_link_ksettings *cmd, > u32 *dev_speed, u8 *dev_duplex); > > -struct netlink_ext_ack; > struct phy_device; > struct phy_tdr_config; > > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index > cde753bb2093..2459571fc1d1 100644 > --- a/include/uapi/linux/ethtool.h > +++ b/include/uapi/linux/ethtool.h > @@ -340,6 +340,28 @@ struct ethtool_eeprom { > __u8 data[0]; > }; > > +/** > + * struct ethtool_eeprom_data - EEPROM dump from specified page > + * @offset: Offset within the specified EEPROM page to begin read, in > bytes. Note here that bytes at offset 0-127 are the same for every page of the module, only bytes at offset 128 and higher are actually paged. > + * @length: Number of bytes to read. > + * @page: Page number to read from. > + * @bank: Page bank number to read from, if applicable by EEPROM spec. > + * @i2c_address: I2C address of a page. Value less than 0x7f expected. > Most > + * EEPROMs use 0x50 or 0x51. The standards are all very clear, the only legal values are 0x50 and 0x51. It isn't 'expected', it is required. I suggest that 0xA0 and 0xA2 also be silently accepted, and translated to 0x50 and 0x51 respectively. Some of the specs use A0/A2 instead of 0x50/0x51. They actually mean the same thing. > + * @data: Pointer to buffer with EEPROM data of @length size. > + * > + * This can be used to manage pages during EEPROM dump in ethtool and > +pass > + * required information to the driver. > + */ > +struct ethtool_eeprom_data { > + __u32 offset; > + __u32 length; > + __u32 page; > + __u32 bank; > + __u32 i2c_address; > + __u8 *data; > +}; > + > /** > * struct ethtool_eee - Energy Efficient Ethernet information > * @cmd: ETHTOOL_{G,S}EEE > @@ -1865,6 +1887,10 @@ static inline int ethtool_validate_duplex(__u8 > duplex) > #define ETH_MODULE_SFF_8636_MAX_LEN 640 > #define ETH_MODULE_SFF_8436_MAX_LEN 640 > > +#define ETH_MODULE_EEPROM_MAX_LEN 640 Please don't add this MAX_LEN constant. Even better, remove the two above it as well. The proper value for all 3 of these MAX_LEN items is the architectural limit imposed by the 8 bit page register plus the constant lower page (hence 257*128 bytes). The 8436 and 8636 specs do not actually limit these devices to 640 bytes (3 pages). There is no MAX_LEN listed for SFF_8472. If there is one, it should actually be 259 * 128 bytes (to account for 256 more bytes on the unpaged 0x50 i2c address). Nor is there one for CMIS. The maximum architected length for CMIS is (257*128) + (127 * 16 * 128). That's the QSFP max length plus 127 more banks of 16 pages. > +#define ETH_MODULE_EEPROM_PAGE_LEN 256 > +#define ETH_MODULE_MAX_I2C_ADDRESS 0x7f Actually there are only two legal values for the i2c address (0x50, 0x51). Rather than defining a MAX address, consider defining the legal values, or... is it used at all? Leave it out? > + > /* Reset flags */ > /* The reset() operation must clear the flags for the components which > * were actually reset. On successful return, the flags indicate the diff --git > a/include/uapi/linux/ethtool_netlink.h > b/include/uapi/linux/ethtool_netlink.h > index a286635ac9b8..60dd848d0b54 100644 > --- a/include/uapi/linux/ethtool_netlink.h > +++ b/include/uapi/linux/ethtool_netlink.h > @@ -42,6 +42,7 @@ enum { > ETHTOOL_MSG_CABLE_TEST_ACT, > ETHTOOL_MSG_CABLE_TEST_TDR_ACT, > ETHTOOL_MSG_TUNNEL_INFO_GET, > + ETHTOOL_MSG_EEPROM_DATA_GET, > > /* add new constants above here */ > __ETHTOOL_MSG_USER_CNT, > @@ -80,6 +81,7 @@ enum { > ETHTOOL_MSG_CABLE_TEST_NTF, > ETHTOOL_MSG_CABLE_TEST_TDR_NTF, > ETHTOOL_MSG_TUNNEL_INFO_GET_REPLY, > + ETHTOOL_MSG_EEPROM_DATA_GET_REPLY, > > /* add new constants above here */ > __ETHTOOL_MSG_KERNEL_CNT, > @@ -629,6 +631,23 @@ enum { > ETHTOOL_A_TUNNEL_INFO_MAX = > (__ETHTOOL_A_TUNNEL_INFO_CNT - 1) }; > > +/* MODULE EEPROM DATA */ > + > +enum { > + ETHTOOL_A_EEPROM_DATA_UNSPEC, > + ETHTOOL_A_EEPROM_DATA_HEADER, > + > + ETHTOOL_A_EEPROM_DATA_OFFSET, > + ETHTOOL_A_EEPROM_DATA_LENGTH, > + ETHTOOL_A_EEPROM_DATA_PAGE, > + ETHTOOL_A_EEPROM_DATA_BANK, > + ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS, > + ETHTOOL_A_EEPROM_DATA, > + > + __ETHTOOL_A_EEPROM_DATA_CNT, > + ETHTOOL_A_EEPROM_DATA_MAX = > (__ETHTOOL_A_EEPROM_DATA_CNT - 1) }; > + > /* generic netlink info */ > #define ETHTOOL_GENL_NAME "ethtool" > #define ETHTOOL_GENL_VERSION 1 > diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile index > 7a849ff22dad..d604346bc074 100644 > --- a/net/ethtool/Makefile > +++ b/net/ethtool/Makefile > @@ -7,4 +7,4 @@ obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_nl.o > ethtool_nl-y := netlink.o bitset.o strset.o linkinfo.o linkmodes.o \ > linkstate.o debug.o wol.o features.o privflags.o rings.o \ > channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \ > - tunnels.o > + tunnels.o eeprom.o > diff --git a/net/ethtool/eeprom.c b/net/ethtool/eeprom.c new file mode > 100644 index 000000000000..2618a55b9a40 > --- /dev/null > +++ b/net/ethtool/eeprom.c > @@ -0,0 +1,157 @@ > +// SPDX-License-Identifier: GPL-2.0-only > + > +#include <linux/ethtool.h> > +#include "netlink.h" > +#include "common.h" > + > +struct eeprom_data_req_info { > + struct ethnl_req_info base; > + u32 offset; > + u32 length; > + u32 page; > + u32 bank; > + u32 i2c_address; > +}; > + > +struct eeprom_data_reply_data { > + struct ethnl_reply_data base; > + u32 length; > + u32 i2c_address; > + u8 *data; > +}; > + > +#define EEPROM_DATA_REQINFO(__req_base) \ > + container_of(__req_base, struct eeprom_data_req_info, base) > + > +#define EEPROM_DATA_REPDATA(__reply_base) \ > + container_of(__reply_base, struct eeprom_data_reply_data, base) > + > +static int eeprom_data_prepare_data(const struct ethnl_req_info > *req_base, > + struct ethnl_reply_data *reply_base, > + struct genl_info *info) > +{ > + struct eeprom_data_reply_data *reply = > EEPROM_DATA_REPDATA(reply_base); > + struct eeprom_data_req_info *request = > EEPROM_DATA_REQINFO(req_base); > + struct ethtool_eeprom_data page_data = {0}; > + struct net_device *dev = reply_base->dev; > + int err; > + > + if (!dev->ethtool_ops->get_module_eeprom_data_by_page) > + return -EOPNOTSUPP; > + > + page_data.offset = request->offset; > + page_data.length = request->length; > + page_data.i2c_address = request->i2c_address; > + page_data.page = request->page; > + page_data.bank = request->bank; > + page_data.data = kmalloc(page_data.length, GFP_KERNEL); > + if (!page_data.data) > + return -ENOMEM; > + err = ethnl_ops_begin(dev); > + if (err) > + goto err_free; > + > + err = dev->ethtool_ops->get_module_eeprom_data_by_page(dev, > &page_data, > + info->extack); > + if (err) > + goto err_ops; > + > + reply->length = page_data.length; > + reply->i2c_address = page_data.i2c_address; > + reply->data = page_data.data; > + > + ethnl_ops_complete(dev); The two error paths below kfree(page_data.data). Does someone else free this memory when there is no error? > + return 0; > + > +err_ops: > + ethnl_ops_complete(dev); > +err_free: > + kfree(page_data.data); > + return err; > +} > + > +static int eeprom_data_parse_request(struct ethnl_req_info *req_info, > struct nlattr **tb, > + struct netlink_ext_ack *extack) { > + struct eeprom_data_req_info *request = > EEPROM_DATA_REQINFO(req_info); > + > + if (!tb[ETHTOOL_A_EEPROM_DATA_OFFSET] || > + !tb[ETHTOOL_A_EEPROM_DATA_LENGTH] || > + !tb[ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS]) > + return -EINVAL; > + > + request->i2c_address = > nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS]); > + if (request->i2c_address > ETH_MODULE_MAX_I2C_ADDRESS) > + return -EINVAL; I would be much more restrictive, with one flexibility... if (request->i2c_address == 0xA0) request->i2c_address = 0x50; if (request->i2c_address == 0xA2) request->i2c_address = 0x51; if (request->i2c_address < 0x50) || (request->i2c_address > 0x51) return -EINVAL; > + > + request->offset = > nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_OFFSET]); > + request->length = > nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_LENGTH]); > + if (request->length > ETH_MODULE_EEPROM_MAX_LEN) > + return -EINVAL; This is really problematic as there are MANY different max values, within the specs, for the various EEPROMs being generically supported here. I would leave it to the drivers to handle out-of-range requests. If you really want to check, you need to know which spec the module supports, whether it supports pages, and whether it supports banks. I have not found a register that actually reports the number of supported pages that an eeprom supports. The specs should have included that :-(. > + if (tb[ETHTOOL_A_EEPROM_DATA_PAGE] && > + request->offset + request->length > > ETH_MODULE_EEPROM_PAGE_LEN) > + return -EINVAL; Why does this stanza depend on DATA_PAGE? In this new data structure, no requests can cross the 256 byte page boundary. I suggest, rather then -EINVAL, you should reduce the length to reach the end of the page: if (request->offset + request->length) > ETH_MODULE_EEPROM_PAGE_LEN) request->length = ETH_MODULE_EEPROM_PAGE_LEN - request->offset. Note that this matches the choice you made to truncate rather than error out in fallback_set_parms(). > + > + if (tb[ETHTOOL_A_EEPROM_DATA_PAGE]) > + request->page = > nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_PAGE]); > + if (tb[ETHTOOL_A_EEPROM_DATA_BANK]) > + request->bank = > nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_BANK]); Other checks: Page and bank have to be between 0 and 255 (inclusive), they go into an 8 bit register in the eeprom. Offset and length can't be negative. > + > + return 0; > +} > + > +static int eeprom_data_reply_size(const struct ethnl_req_info *req_base, > + const struct ethnl_reply_data *reply_base) { > + const struct eeprom_data_req_info *request = > +EEPROM_DATA_REQINFO(req_base); > + > + return nla_total_size(sizeof(u32)) + /* _EEPROM_DATA_LENGTH */ > + nla_total_size(sizeof(u32)) + /* _EEPROM_DATA_I2C_ADDRESS > */ > + nla_total_size(sizeof(u8) * request->length); /* _EEPROM_DATA > +*/ } > + > +static int eeprom_data_fill_reply(struct sk_buff *skb, > + const struct ethnl_req_info *req_base, > + const struct ethnl_reply_data *reply_base) { > + struct eeprom_data_reply_data *reply = > +EEPROM_DATA_REPDATA(reply_base); > + > + if (nla_put_u32(skb, ETHTOOL_A_EEPROM_DATA_LENGTH, reply- > >length) || > + nla_put_u32(skb, ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS, > reply->i2c_address) || > + nla_put(skb, ETHTOOL_A_EEPROM_DATA, reply->length, reply- > >data)) > + return -EMSGSIZE; > + > + return 0; > +} > + > +static void eeprom_data_cleanup_data(struct ethnl_reply_data > +*reply_base) { > + struct eeprom_data_reply_data *reply = > +EEPROM_DATA_REPDATA(reply_base); > + > + kfree(reply->data); > +} > + > +const struct ethnl_request_ops ethnl_eeprom_data_request_ops = { > + .request_cmd = ETHTOOL_MSG_EEPROM_DATA_GET, > + .reply_cmd = > ETHTOOL_MSG_EEPROM_DATA_GET_REPLY, > + .hdr_attr = ETHTOOL_A_EEPROM_DATA_HEADER, > + .req_info_size = sizeof(struct eeprom_data_req_info), > + .reply_data_size = sizeof(struct eeprom_data_reply_data), > + > + .parse_request = eeprom_data_parse_request, > + .prepare_data = eeprom_data_prepare_data, > + .reply_size = eeprom_data_reply_size, > + .fill_reply = eeprom_data_fill_reply, > + .cleanup_data = eeprom_data_cleanup_data, > +}; > + > +const struct nla_policy ethnl_eeprom_data_get_policy[] = { > + [ETHTOOL_A_EEPROM_DATA_HEADER] = > NLA_POLICY_NESTED(ethnl_header_policy), > + [ETHTOOL_A_EEPROM_DATA_OFFSET] = { .type = > NLA_U32 }, > + [ETHTOOL_A_EEPROM_DATA_LENGTH] = { .type = > NLA_U32 }, > + [ETHTOOL_A_EEPROM_DATA_PAGE] = { .type = NLA_U32 }, > + [ETHTOOL_A_EEPROM_DATA_BANK] = { .type = NLA_U32 }, > + [ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS] = { .type = NLA_U32 }, > + [ETHTOOL_A_EEPROM_DATA] = { .type = > NLA_BINARY }, > +}; > + > diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index > 50d3c8896f91..ff2528bee192 100644 > --- a/net/ethtool/netlink.c > +++ b/net/ethtool/netlink.c > @@ -245,6 +245,7 @@ > ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = { > [ETHTOOL_MSG_PAUSE_GET] = > ðnl_pause_request_ops, > [ETHTOOL_MSG_EEE_GET] = ðnl_eee_request_ops, > [ETHTOOL_MSG_TSINFO_GET] = ðnl_tsinfo_request_ops, > + [ETHTOOL_MSG_EEPROM_DATA_GET] = > ðnl_eeprom_data_request_ops, > }; > > static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback > *cb) @@ -912,6 +913,15 @@ static const struct genl_ops ethtool_genl_ops[] > = { > .policy = ethnl_tunnel_info_get_policy, > .maxattr = ARRAY_SIZE(ethnl_tunnel_info_get_policy) - 1, > }, > + { > + .cmd = ETHTOOL_MSG_EEPROM_DATA_GET, > + .doit = ethnl_default_doit, > + .start = ethnl_default_start, > + .dumpit = ethnl_default_dumpit, > + .done = ethnl_default_done, > + .policy = ethnl_eeprom_data_get_policy, > + .maxattr = ARRAY_SIZE(ethnl_eeprom_data_get_policy) - 1, > + }, > }; > > static const struct genl_multicast_group ethtool_nl_mcgrps[] = { diff --git > a/net/ethtool/netlink.h b/net/ethtool/netlink.h index > 6eabd58d81bf..60954c7b4dfe 100644 > --- a/net/ethtool/netlink.h > +++ b/net/ethtool/netlink.h > @@ -344,6 +344,7 @@ extern const struct ethnl_request_ops > ethnl_coalesce_request_ops; extern const struct ethnl_request_ops > ethnl_pause_request_ops; extern const struct ethnl_request_ops > ethnl_eee_request_ops; extern const struct ethnl_request_ops > ethnl_tsinfo_request_ops; > +extern const struct ethnl_request_ops ethnl_eeprom_data_request_ops; > > extern const struct nla_policy > ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1]; extern const struct > nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1]; @@ > -375,6 +376,7 @@ extern const struct nla_policy > ethnl_tsinfo_get_policy[ETHTOOL_A_TSINFO_HEADER + extern const struct > nla_policy ethnl_cable_test_act_policy[ETHTOOL_A_CABLE_TEST_HEADER + > 1]; extern const struct nla_policy > ethnl_cable_test_tdr_act_policy[ETHTOOL_A_CABLE_TEST_TDR_CFG + 1]; > extern const struct nla_policy > ethnl_tunnel_info_get_policy[ETHTOOL_A_TUNNEL_INFO_HEADER + 1]; > +extern const struct nla_policy > +ethnl_eeprom_data_get_policy[ETHTOOL_A_EEPROM_DATA + 1]; > > int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info); int > ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info); > -- > 2.18.2
> > + * @length: Number of bytes to read. > > + * @page: Page number to read from. > > + * @bank: Page bank number to read from, if applicable by EEPROM spec. > > + * @i2c_address: I2C address of a page. Value less than 0x7f expected. > > Most > > + * EEPROMs use 0x50 or 0x51. > > The standards are all very clear Our experience so far is that manufactures of SFP modules like to ignore the standard. And none of the standards seem to cover copper modules, which have additional registers at some other page. Admittedly, they cannot be mapped as pages, you need some proprietary protocol to map MDIO onto I2C. But i would not be surprised to find some SFP that maps the FLASH of the microcontroller onto an address, which we might be able to read out using this API. So i suggested we keep it generic, allowing access to these proprietary registers at other addresses. And if there is nothing there, you probably get a 1/2 page of 0xff. > I suggest that 0xA0 and 0xA2 also be silently accepted, and > translated to 0x50 and 0x51 respectively. No, i don't like having two different values mean the same thing. It just leads to confusion. And userspace is going to be confused when it asks for 0xA0 but the reply says it is for 0x50. The Linux I2C subsystem does not magically map 8bit addresses in 7bit addresses. We should follow what the Linux I2C subsystem does. > > + > > + request->offset = > > nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_OFFSET]); > > + request->length = > > nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_LENGTH]); > > + if (request->length > ETH_MODULE_EEPROM_MAX_LEN) > > + return -EINVAL; > > This is really problematic as there are MANY different max values, within > the specs I agree. We should only be returning one 1/2 page as a maximum. So it should be limited to 128 bytes. And offset+length should not go beyond the end of a 1/2 page. > > + if (tb[ETHTOOL_A_EEPROM_DATA_PAGE]) > > + request->page = > > nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_PAGE]); > > + if (tb[ETHTOOL_A_EEPROM_DATA_BANK]) > > + request->bank = > > nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_BANK]); > > Other checks: > > Page and bank have to be between 0 and 255 (inclusive), they > go into an 8 bit register in the eeprom. Yes, a u8 would be a better type here. Andrew
> +/* MODULE EEPROM DATA */ > + > +enum { > + ETHTOOL_A_EEPROM_DATA_UNSPEC, > + ETHTOOL_A_EEPROM_DATA_HEADER, > + > + ETHTOOL_A_EEPROM_DATA_OFFSET, > + ETHTOOL_A_EEPROM_DATA_LENGTH, > + ETHTOOL_A_EEPROM_DATA_PAGE, > + ETHTOOL_A_EEPROM_DATA_BANK, > + ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS, > + ETHTOOL_A_EEPROM_DATA, If you look at all the other such enums in ethtool_netlink, you will see a comment indicating the type. Please add them here as well. Please also update Documentation/networking/ethtool-netlink.rst. Andrew
On 3/5/2021 2:50 AM, Don Bollinger wrote: > > On Thu, Mar 04, 2021 at 10:57AM-0800, Moshe Shemesh wrote: >> From: Vladyslav Tarasiuk <vladyslavt@nvidia.com> >> >> Define get_module_eeprom_data_by_page() ethtool callback and >> implement netlink infrastructure. >> >> get_module_eeprom_data_by_page() allows network drivers to dump a >> part of module's EEPROM specified by page and bank numbers along with >> offset and length. It is effectively a netlink replacement for >> get_module_info() and get_module_eeprom() pair, which is needed due to >> emergence of complex non-linear EEPROM layouts. >> >> Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com> >> --- >> include/linux/ethtool.h | 7 +- >> include/uapi/linux/ethtool.h | 26 +++++ >> include/uapi/linux/ethtool_netlink.h | 19 ++++ >> net/ethtool/Makefile | 2 +- >> net/ethtool/eeprom.c | 157 +++++++++++++++++++++++++++ >> net/ethtool/netlink.c | 10 ++ >> net/ethtool/netlink.h | 2 + >> 7 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 >> net/ethtool/eeprom.c >> >> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index >> ec4cd3921c67..2f65aae5f492 100644 >> --- a/include/linux/ethtool.h >> +++ b/include/linux/ethtool.h >> @@ -81,6 +81,7 @@ enum { >> #define ETH_RSS_HASH_NO_CHANGE 0 >> >> struct net_device; >> +struct netlink_ext_ack; >> >> /* Some generic methods drivers may use in their ethtool_ops */ >> u32 ethtool_op_get_link(struct net_device *dev); @@ -410,6 +411,8 @@ >> struct ethtool_pause_stats { >> * @get_ethtool_phy_stats: Return extended statistics about the PHY >> device. >> * This is only useful if the device maintains PHY statistics and >> * cannot use the standard PHY library helpers. >> + * @get_module_eeprom_data_by_page: Get a region of plug-in module >> EEPROM data >> + * from specified page. Returns a negative error code or zero. >> * >> * All operations are optional (i.e. the function pointer may be set >> * to %NULL) and callers must take this into account. Callers must @@ > -515,6 >> +518,9 @@ struct ethtool_ops { >> const struct ethtool_tunable *, void *); >> int (*set_phy_tunable)(struct net_device *, >> const struct ethtool_tunable *, const > void >> *); >> + int (*get_module_eeprom_data_by_page)(struct net_device >> *dev, >> + const struct >> ethtool_eeprom_data *page, >> + struct netlink_ext_ack >> *extack); >> }; >> >> int ethtool_check_ops(const struct ethtool_ops *ops); @@ -538,7 +544,6 >> @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev, >> const struct ethtool_link_ksettings > *cmd, >> u32 *dev_speed, u8 *dev_duplex); >> >> -struct netlink_ext_ack; >> struct phy_device; >> struct phy_tdr_config; >> >> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h > index >> cde753bb2093..2459571fc1d1 100644 >> --- a/include/uapi/linux/ethtool.h >> +++ b/include/uapi/linux/ethtool.h >> @@ -340,6 +340,28 @@ struct ethtool_eeprom { >> __u8 data[0]; >> }; >> >> +/** >> + * struct ethtool_eeprom_data - EEPROM dump from specified page >> + * @offset: Offset within the specified EEPROM page to begin read, in >> bytes. > Note here that bytes at offset 0-127 are the same for every page of the > module, only bytes at offset 128 and higher are actually paged. Right, but still having the offset relative to 128 will be confusing. >> + * @length: Number of bytes to read. >> + * @page: Page number to read from. >> + * @bank: Page bank number to read from, if applicable by EEPROM spec. >> + * @i2c_address: I2C address of a page. Value less than 0x7f expected. >> Most >> + * EEPROMs use 0x50 or 0x51. > The standards are all very clear, the only legal values are 0x50 and 0x51. > It isn't 'expected', it is required. I suggest that 0xA0 and 0xA2 also be > silently accepted, and translated to 0x50 and 0x51 respectively. Some > of the specs use A0/A2 instead of 0x50/0x51. They actually mean the > same thing. > >> + * @data: Pointer to buffer with EEPROM data of @length size. >> + * >> + * This can be used to manage pages during EEPROM dump in ethtool and >> +pass >> + * required information to the driver. >> + */ >> +struct ethtool_eeprom_data { >> + __u32 offset; >> + __u32 length; >> + __u32 page; >> + __u32 bank; >> + __u32 i2c_address; >> + __u8 *data; >> +}; >> + >> /** >> * struct ethtool_eee - Energy Efficient Ethernet information >> * @cmd: ETHTOOL_{G,S}EEE >> @@ -1865,6 +1887,10 @@ static inline int ethtool_validate_duplex(__u8 >> duplex) >> #define ETH_MODULE_SFF_8636_MAX_LEN 640 >> #define ETH_MODULE_SFF_8436_MAX_LEN 640 >> >> +#define ETH_MODULE_EEPROM_MAX_LEN 640 > Please don't add this MAX_LEN constant. Even better, remove > the two above it as well. These constants are relevant to the ioctl function and used by the drivers. For netlink new KAPI I will remove the new MAX_LEN per your explanation, thanks. > The proper value for all 3 of these MAX_LEN items is the > architectural limit imposed by the 8 bit page register plus the constant > lower page (hence 257*128 bytes). The 8436 and 8636 > specs do not actually limit these devices to 640 bytes (3 pages). > > There is no MAX_LEN listed for SFF_8472. If there is one, it should > actually be 259 * 128 bytes (to account for 256 more bytes on the > unpaged 0x50 i2c address). > > Nor is there one for CMIS. The maximum > architected length for CMIS is (257*128) + (127 * 16 * 128). That's > the QSFP max length plus 127 more banks of 16 pages. > >> +#define ETH_MODULE_EEPROM_PAGE_LEN 256 >> +#define ETH_MODULE_MAX_I2C_ADDRESS 0x7f > Actually there are only two legal values for the i2c address (0x50, 0x51). > Rather than defining a MAX address, consider defining the legal values, > or... is it used at all? Leave it out? As Andrew commented there might be usage of other i2c adresses, I will keep it. >> + >> /* Reset flags */ >> /* The reset() operation must clear the flags for the components which >> * were actually reset. On successful return, the flags indicate the > diff --git >> a/include/uapi/linux/ethtool_netlink.h >> b/include/uapi/linux/ethtool_netlink.h >> index a286635ac9b8..60dd848d0b54 100644 >> --- a/include/uapi/linux/ethtool_netlink.h >> +++ b/include/uapi/linux/ethtool_netlink.h >> @@ -42,6 +42,7 @@ enum { >> ETHTOOL_MSG_CABLE_TEST_ACT, >> ETHTOOL_MSG_CABLE_TEST_TDR_ACT, >> ETHTOOL_MSG_TUNNEL_INFO_GET, >> + ETHTOOL_MSG_EEPROM_DATA_GET, >> >> /* add new constants above here */ >> __ETHTOOL_MSG_USER_CNT, >> @@ -80,6 +81,7 @@ enum { >> ETHTOOL_MSG_CABLE_TEST_NTF, >> ETHTOOL_MSG_CABLE_TEST_TDR_NTF, >> ETHTOOL_MSG_TUNNEL_INFO_GET_REPLY, >> + ETHTOOL_MSG_EEPROM_DATA_GET_REPLY, >> >> /* add new constants above here */ >> __ETHTOOL_MSG_KERNEL_CNT, >> @@ -629,6 +631,23 @@ enum { >> ETHTOOL_A_TUNNEL_INFO_MAX = >> (__ETHTOOL_A_TUNNEL_INFO_CNT - 1) }; >> >> +/* MODULE EEPROM DATA */ >> + >> +enum { >> + ETHTOOL_A_EEPROM_DATA_UNSPEC, >> + ETHTOOL_A_EEPROM_DATA_HEADER, >> + >> + ETHTOOL_A_EEPROM_DATA_OFFSET, >> + ETHTOOL_A_EEPROM_DATA_LENGTH, >> + ETHTOOL_A_EEPROM_DATA_PAGE, >> + ETHTOOL_A_EEPROM_DATA_BANK, >> + ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS, >> + ETHTOOL_A_EEPROM_DATA, >> + >> + __ETHTOOL_A_EEPROM_DATA_CNT, >> + ETHTOOL_A_EEPROM_DATA_MAX = >> (__ETHTOOL_A_EEPROM_DATA_CNT - 1) }; >> + >> /* generic netlink info */ >> #define ETHTOOL_GENL_NAME "ethtool" >> #define ETHTOOL_GENL_VERSION 1 >> diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile index >> 7a849ff22dad..d604346bc074 100644 >> --- a/net/ethtool/Makefile >> +++ b/net/ethtool/Makefile >> @@ -7,4 +7,4 @@ obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_nl.o >> ethtool_nl-y := netlink.o bitset.o strset.o linkinfo.o linkmodes.o \ >> linkstate.o debug.o wol.o features.o privflags.o rings.o > \ >> channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o > \ >> - tunnels.o >> + tunnels.o eeprom.o >> diff --git a/net/ethtool/eeprom.c b/net/ethtool/eeprom.c new file mode >> 100644 index 000000000000..2618a55b9a40 >> --- /dev/null >> +++ b/net/ethtool/eeprom.c >> @@ -0,0 +1,157 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> + >> +#include <linux/ethtool.h> >> +#include "netlink.h" >> +#include "common.h" >> + >> +struct eeprom_data_req_info { >> + struct ethnl_req_info base; >> + u32 offset; >> + u32 length; >> + u32 page; >> + u32 bank; >> + u32 i2c_address; >> +}; >> + >> +struct eeprom_data_reply_data { >> + struct ethnl_reply_data base; >> + u32 length; >> + u32 i2c_address; >> + u8 *data; >> +}; >> + >> +#define EEPROM_DATA_REQINFO(__req_base) \ >> + container_of(__req_base, struct eeprom_data_req_info, base) >> + >> +#define EEPROM_DATA_REPDATA(__reply_base) \ >> + container_of(__reply_base, struct eeprom_data_reply_data, base) >> + >> +static int eeprom_data_prepare_data(const struct ethnl_req_info >> *req_base, >> + struct ethnl_reply_data *reply_base, >> + struct genl_info *info) >> +{ >> + struct eeprom_data_reply_data *reply = >> EEPROM_DATA_REPDATA(reply_base); >> + struct eeprom_data_req_info *request = >> EEPROM_DATA_REQINFO(req_base); >> + struct ethtool_eeprom_data page_data = {0}; >> + struct net_device *dev = reply_base->dev; >> + int err; >> + >> + if (!dev->ethtool_ops->get_module_eeprom_data_by_page) >> + return -EOPNOTSUPP; >> + >> + page_data.offset = request->offset; >> + page_data.length = request->length; >> + page_data.i2c_address = request->i2c_address; >> + page_data.page = request->page; >> + page_data.bank = request->bank; >> + page_data.data = kmalloc(page_data.length, GFP_KERNEL); >> + if (!page_data.data) >> + return -ENOMEM; >> + err = ethnl_ops_begin(dev); >> + if (err) >> + goto err_free; >> + >> + err = dev->ethtool_ops->get_module_eeprom_data_by_page(dev, >> &page_data, >> + > info->extack); >> + if (err) >> + goto err_ops; >> + >> + reply->length = page_data.length; >> + reply->i2c_address = page_data.i2c_address; >> + reply->data = page_data.data; >> + >> + ethnl_ops_complete(dev); > The two error paths below kfree(page_data.data). Does someone else > free this memory when there is no error? eeprom_data_cleanup_data() >> + return 0; >> + >> +err_ops: >> + ethnl_ops_complete(dev); >> +err_free: >> + kfree(page_data.data); >> + return err; >> +} >> + >> +static int eeprom_data_parse_request(struct ethnl_req_info *req_info, >> struct nlattr **tb, >> + struct netlink_ext_ack *extack) { >> + struct eeprom_data_req_info *request = >> EEPROM_DATA_REQINFO(req_info); >> + >> + if (!tb[ETHTOOL_A_EEPROM_DATA_OFFSET] || >> + !tb[ETHTOOL_A_EEPROM_DATA_LENGTH] || >> + !tb[ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS]) >> + return -EINVAL; >> + >> + request->i2c_address = >> nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS]); >> + if (request->i2c_address > ETH_MODULE_MAX_I2C_ADDRESS) >> + return -EINVAL; > I would be much more restrictive, with one flexibility... > > if (request->i2c_address == 0xA0) request->i2c_address = 0x50; > if (request->i2c_address == 0xA2) request->i2c_address = 0x51; > if (request->i2c_address < 0x50) || (request->i2c_address > 0x51) > return -EINVAL; > >> + >> + request->offset = >> nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_OFFSET]); >> + request->length = >> nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_LENGTH]); >> + if (request->length > ETH_MODULE_EEPROM_MAX_LEN) >> + return -EINVAL; > This is really problematic as there are MANY different max values, within > the specs, for the various EEPROMs being generically supported here. > I would leave it to the drivers to handle out-of-range requests. If you > really want to check, you need to know which spec the module supports, > whether it supports pages, and whether it supports banks. I have not > found a register that actually reports the number of supported pages > that an eeprom supports. The specs should have included that :-(. Will remove this check per your explanation, thanks. >> + if (tb[ETHTOOL_A_EEPROM_DATA_PAGE] && >> + request->offset + request->length > >> ETH_MODULE_EEPROM_PAGE_LEN) >> + return -EINVAL; > Why does this stanza depend on DATA_PAGE? In this new data > structure, no requests can cross the 256 byte page boundary. > > I suggest, rather then -EINVAL, you should reduce the length to reach > the end of the page: > > if (request->offset + request->length) > ETH_MODULE_EEPROM_PAGE_LEN) > request->length = ETH_MODULE_EEPROM_PAGE_LEN - > request->offset. Backward compatibility, users use the command without page and bank as it was till now expect to get data that can cross page as it worked till now, I can't break it. > Note that this matches the choice you made to truncate rather than > error out in fallback_set_parms(). We truncate in fallback_set_params() to eeprom_len. length = modinfo->eeprom_len - offset; >> + >> + if (tb[ETHTOOL_A_EEPROM_DATA_PAGE]) >> + request->page = >> nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_PAGE]); >> + if (tb[ETHTOOL_A_EEPROM_DATA_BANK]) >> + request->bank = >> nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_BANK]); > Other checks: > > Page and bank have to be between 0 and 255 (inclusive), they > go into an 8 bit register in the eeprom. Will add, thanks. > Offset and length can't be negative. Already are u32. >> + >> + return 0; >> +} >> + >> +static int eeprom_data_reply_size(const struct ethnl_req_info *req_base, >> + const struct ethnl_reply_data *reply_base) > { >> + const struct eeprom_data_req_info *request = >> +EEPROM_DATA_REQINFO(req_base); >> + >> + return nla_total_size(sizeof(u32)) + /* _EEPROM_DATA_LENGTH */ >> + nla_total_size(sizeof(u32)) + /* _EEPROM_DATA_I2C_ADDRESS >> */ >> + nla_total_size(sizeof(u8) * request->length); /* _EEPROM_DATA >> +*/ } >> + >> +static int eeprom_data_fill_reply(struct sk_buff *skb, >> + const struct ethnl_req_info *req_base, >> + const struct ethnl_reply_data *reply_base) > { >> + struct eeprom_data_reply_data *reply = >> +EEPROM_DATA_REPDATA(reply_base); >> + >> + if (nla_put_u32(skb, ETHTOOL_A_EEPROM_DATA_LENGTH, reply- >>> length) || >> + nla_put_u32(skb, ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS, >> reply->i2c_address) || >> + nla_put(skb, ETHTOOL_A_EEPROM_DATA, reply->length, reply- >>> data)) >> + return -EMSGSIZE; >> + >> + return 0; >> +} >> + >> +static void eeprom_data_cleanup_data(struct ethnl_reply_data >> +*reply_base) { >> + struct eeprom_data_reply_data *reply = >> +EEPROM_DATA_REPDATA(reply_base); >> + >> + kfree(reply->data); >> +} >> + >> +const struct ethnl_request_ops ethnl_eeprom_data_request_ops = { >> + .request_cmd = ETHTOOL_MSG_EEPROM_DATA_GET, >> + .reply_cmd = >> ETHTOOL_MSG_EEPROM_DATA_GET_REPLY, >> + .hdr_attr = ETHTOOL_A_EEPROM_DATA_HEADER, >> + .req_info_size = sizeof(struct eeprom_data_req_info), >> + .reply_data_size = sizeof(struct eeprom_data_reply_data), >> + >> + .parse_request = eeprom_data_parse_request, >> + .prepare_data = eeprom_data_prepare_data, >> + .reply_size = eeprom_data_reply_size, >> + .fill_reply = eeprom_data_fill_reply, >> + .cleanup_data = eeprom_data_cleanup_data, >> +}; >> + >> +const struct nla_policy ethnl_eeprom_data_get_policy[] = { >> + [ETHTOOL_A_EEPROM_DATA_HEADER] = >> NLA_POLICY_NESTED(ethnl_header_policy), >> + [ETHTOOL_A_EEPROM_DATA_OFFSET] = { .type = >> NLA_U32 }, >> + [ETHTOOL_A_EEPROM_DATA_LENGTH] = { .type = >> NLA_U32 }, >> + [ETHTOOL_A_EEPROM_DATA_PAGE] = { .type = NLA_U32 }, >> + [ETHTOOL_A_EEPROM_DATA_BANK] = { .type = NLA_U32 }, >> + [ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS] = { .type = NLA_U32 }, >> + [ETHTOOL_A_EEPROM_DATA] = { .type = >> NLA_BINARY }, >> +}; >> + >> diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index >> 50d3c8896f91..ff2528bee192 100644 >> --- a/net/ethtool/netlink.c >> +++ b/net/ethtool/netlink.c >> @@ -245,6 +245,7 @@ >> ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = { >> [ETHTOOL_MSG_PAUSE_GET] = >> ðnl_pause_request_ops, >> [ETHTOOL_MSG_EEE_GET] = ðnl_eee_request_ops, >> [ETHTOOL_MSG_TSINFO_GET] = ðnl_tsinfo_request_ops, >> + [ETHTOOL_MSG_EEPROM_DATA_GET] = >> ðnl_eeprom_data_request_ops, >> }; >> >> static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback >> *cb) @@ -912,6 +913,15 @@ static const struct genl_ops ethtool_genl_ops[] >> = { >> .policy = ethnl_tunnel_info_get_policy, >> .maxattr = ARRAY_SIZE(ethnl_tunnel_info_get_policy) - 1, >> }, >> + { >> + .cmd = ETHTOOL_MSG_EEPROM_DATA_GET, >> + .doit = ethnl_default_doit, >> + .start = ethnl_default_start, >> + .dumpit = ethnl_default_dumpit, >> + .done = ethnl_default_done, >> + .policy = ethnl_eeprom_data_get_policy, >> + .maxattr = ARRAY_SIZE(ethnl_eeprom_data_get_policy) - 1, >> + }, >> }; >> >> static const struct genl_multicast_group ethtool_nl_mcgrps[] = { diff > --git >> a/net/ethtool/netlink.h b/net/ethtool/netlink.h index >> 6eabd58d81bf..60954c7b4dfe 100644 >> --- a/net/ethtool/netlink.h >> +++ b/net/ethtool/netlink.h >> @@ -344,6 +344,7 @@ extern const struct ethnl_request_ops >> ethnl_coalesce_request_ops; extern const struct ethnl_request_ops >> ethnl_pause_request_ops; extern const struct ethnl_request_ops >> ethnl_eee_request_ops; extern const struct ethnl_request_ops >> ethnl_tsinfo_request_ops; >> +extern const struct ethnl_request_ops ethnl_eeprom_data_request_ops; >> >> extern const struct nla_policy >> ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1]; extern const struct >> nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1]; @@ >> -375,6 +376,7 @@ extern const struct nla_policy >> ethnl_tsinfo_get_policy[ETHTOOL_A_TSINFO_HEADER + extern const struct >> nla_policy ethnl_cable_test_act_policy[ETHTOOL_A_CABLE_TEST_HEADER + >> 1]; extern const struct nla_policy >> ethnl_cable_test_tdr_act_policy[ETHTOOL_A_CABLE_TEST_TDR_CFG + 1]; >> extern const struct nla_policy >> ethnl_tunnel_info_get_policy[ETHTOOL_A_TUNNEL_INFO_HEADER + 1]; >> +extern const struct nla_policy >> +ethnl_eeprom_data_get_policy[ETHTOOL_A_EEPROM_DATA + 1]; >> >> int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info); int >> ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info); >> -- >> 2.18.2
On 3/5/2021 3:58 AM, Andrew Lunn wrote: > >> +/* MODULE EEPROM DATA */ >> + >> +enum { >> + ETHTOOL_A_EEPROM_DATA_UNSPEC, >> + ETHTOOL_A_EEPROM_DATA_HEADER, >> + >> + ETHTOOL_A_EEPROM_DATA_OFFSET, >> + ETHTOOL_A_EEPROM_DATA_LENGTH, >> + ETHTOOL_A_EEPROM_DATA_PAGE, >> + ETHTOOL_A_EEPROM_DATA_BANK, >> + ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS, >> + ETHTOOL_A_EEPROM_DATA, > If you look at all the other such enums in ethtool_netlink, you will > see a comment indicating the type. Please add them here as well. > > Please also update Documentation/networking/ethtool-netlink.rst. Sure, will add. > Andrew
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index ec4cd3921c67..2f65aae5f492 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -81,6 +81,7 @@ enum { #define ETH_RSS_HASH_NO_CHANGE 0 struct net_device; +struct netlink_ext_ack; /* Some generic methods drivers may use in their ethtool_ops */ u32 ethtool_op_get_link(struct net_device *dev); @@ -410,6 +411,8 @@ struct ethtool_pause_stats { * @get_ethtool_phy_stats: Return extended statistics about the PHY device. * This is only useful if the device maintains PHY statistics and * cannot use the standard PHY library helpers. + * @get_module_eeprom_data_by_page: Get a region of plug-in module EEPROM data + * from specified page. Returns a negative error code or zero. * * All operations are optional (i.e. the function pointer may be set * to %NULL) and callers must take this into account. Callers must @@ -515,6 +518,9 @@ struct ethtool_ops { const struct ethtool_tunable *, void *); int (*set_phy_tunable)(struct net_device *, const struct ethtool_tunable *, const void *); + int (*get_module_eeprom_data_by_page)(struct net_device *dev, + const struct ethtool_eeprom_data *page, + struct netlink_ext_ack *extack); }; int ethtool_check_ops(const struct ethtool_ops *ops); @@ -538,7 +544,6 @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev, const struct ethtool_link_ksettings *cmd, u32 *dev_speed, u8 *dev_duplex); -struct netlink_ext_ack; struct phy_device; struct phy_tdr_config; diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index cde753bb2093..2459571fc1d1 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -340,6 +340,28 @@ struct ethtool_eeprom { __u8 data[0]; }; +/** + * struct ethtool_eeprom_data - EEPROM dump from specified page + * @offset: Offset within the specified EEPROM page to begin read, in bytes. + * @length: Number of bytes to read. + * @page: Page number to read from. + * @bank: Page bank number to read from, if applicable by EEPROM spec. + * @i2c_address: I2C address of a page. Value less than 0x7f expected. Most + * EEPROMs use 0x50 or 0x51. + * @data: Pointer to buffer with EEPROM data of @length size. + * + * This can be used to manage pages during EEPROM dump in ethtool and pass + * required information to the driver. + */ +struct ethtool_eeprom_data { + __u32 offset; + __u32 length; + __u32 page; + __u32 bank; + __u32 i2c_address; + __u8 *data; +}; + /** * struct ethtool_eee - Energy Efficient Ethernet information * @cmd: ETHTOOL_{G,S}EEE @@ -1865,6 +1887,10 @@ static inline int ethtool_validate_duplex(__u8 duplex) #define ETH_MODULE_SFF_8636_MAX_LEN 640 #define ETH_MODULE_SFF_8436_MAX_LEN 640 +#define ETH_MODULE_EEPROM_MAX_LEN 640 +#define ETH_MODULE_EEPROM_PAGE_LEN 256 +#define ETH_MODULE_MAX_I2C_ADDRESS 0x7f + /* Reset flags */ /* The reset() operation must clear the flags for the components which * were actually reset. On successful return, the flags indicate the diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h index a286635ac9b8..60dd848d0b54 100644 --- a/include/uapi/linux/ethtool_netlink.h +++ b/include/uapi/linux/ethtool_netlink.h @@ -42,6 +42,7 @@ enum { ETHTOOL_MSG_CABLE_TEST_ACT, ETHTOOL_MSG_CABLE_TEST_TDR_ACT, ETHTOOL_MSG_TUNNEL_INFO_GET, + ETHTOOL_MSG_EEPROM_DATA_GET, /* add new constants above here */ __ETHTOOL_MSG_USER_CNT, @@ -80,6 +81,7 @@ enum { ETHTOOL_MSG_CABLE_TEST_NTF, ETHTOOL_MSG_CABLE_TEST_TDR_NTF, ETHTOOL_MSG_TUNNEL_INFO_GET_REPLY, + ETHTOOL_MSG_EEPROM_DATA_GET_REPLY, /* add new constants above here */ __ETHTOOL_MSG_KERNEL_CNT, @@ -629,6 +631,23 @@ enum { ETHTOOL_A_TUNNEL_INFO_MAX = (__ETHTOOL_A_TUNNEL_INFO_CNT - 1) }; +/* MODULE EEPROM DATA */ + +enum { + ETHTOOL_A_EEPROM_DATA_UNSPEC, + ETHTOOL_A_EEPROM_DATA_HEADER, + + ETHTOOL_A_EEPROM_DATA_OFFSET, + ETHTOOL_A_EEPROM_DATA_LENGTH, + ETHTOOL_A_EEPROM_DATA_PAGE, + ETHTOOL_A_EEPROM_DATA_BANK, + ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS, + ETHTOOL_A_EEPROM_DATA, + + __ETHTOOL_A_EEPROM_DATA_CNT, + ETHTOOL_A_EEPROM_DATA_MAX = (__ETHTOOL_A_EEPROM_DATA_CNT - 1) +}; + /* generic netlink info */ #define ETHTOOL_GENL_NAME "ethtool" #define ETHTOOL_GENL_VERSION 1 diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile index 7a849ff22dad..d604346bc074 100644 --- a/net/ethtool/Makefile +++ b/net/ethtool/Makefile @@ -7,4 +7,4 @@ obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_nl.o ethtool_nl-y := netlink.o bitset.o strset.o linkinfo.o linkmodes.o \ linkstate.o debug.o wol.o features.o privflags.o rings.o \ channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \ - tunnels.o + tunnels.o eeprom.o diff --git a/net/ethtool/eeprom.c b/net/ethtool/eeprom.c new file mode 100644 index 000000000000..2618a55b9a40 --- /dev/null +++ b/net/ethtool/eeprom.c @@ -0,0 +1,157 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include <linux/ethtool.h> +#include "netlink.h" +#include "common.h" + +struct eeprom_data_req_info { + struct ethnl_req_info base; + u32 offset; + u32 length; + u32 page; + u32 bank; + u32 i2c_address; +}; + +struct eeprom_data_reply_data { + struct ethnl_reply_data base; + u32 length; + u32 i2c_address; + u8 *data; +}; + +#define EEPROM_DATA_REQINFO(__req_base) \ + container_of(__req_base, struct eeprom_data_req_info, base) + +#define EEPROM_DATA_REPDATA(__reply_base) \ + container_of(__reply_base, struct eeprom_data_reply_data, base) + +static int eeprom_data_prepare_data(const struct ethnl_req_info *req_base, + struct ethnl_reply_data *reply_base, + struct genl_info *info) +{ + struct eeprom_data_reply_data *reply = EEPROM_DATA_REPDATA(reply_base); + struct eeprom_data_req_info *request = EEPROM_DATA_REQINFO(req_base); + struct ethtool_eeprom_data page_data = {0}; + struct net_device *dev = reply_base->dev; + int err; + + if (!dev->ethtool_ops->get_module_eeprom_data_by_page) + return -EOPNOTSUPP; + + page_data.offset = request->offset; + page_data.length = request->length; + page_data.i2c_address = request->i2c_address; + page_data.page = request->page; + page_data.bank = request->bank; + page_data.data = kmalloc(page_data.length, GFP_KERNEL); + if (!page_data.data) + return -ENOMEM; + err = ethnl_ops_begin(dev); + if (err) + goto err_free; + + err = dev->ethtool_ops->get_module_eeprom_data_by_page(dev, &page_data, + info->extack); + if (err) + goto err_ops; + + reply->length = page_data.length; + reply->i2c_address = page_data.i2c_address; + reply->data = page_data.data; + + ethnl_ops_complete(dev); + return 0; + +err_ops: + ethnl_ops_complete(dev); +err_free: + kfree(page_data.data); + return err; +} + +static int eeprom_data_parse_request(struct ethnl_req_info *req_info, struct nlattr **tb, + struct netlink_ext_ack *extack) +{ + struct eeprom_data_req_info *request = EEPROM_DATA_REQINFO(req_info); + + if (!tb[ETHTOOL_A_EEPROM_DATA_OFFSET] || + !tb[ETHTOOL_A_EEPROM_DATA_LENGTH] || + !tb[ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS]) + return -EINVAL; + + request->i2c_address = nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS]); + if (request->i2c_address > ETH_MODULE_MAX_I2C_ADDRESS) + return -EINVAL; + + request->offset = nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_OFFSET]); + request->length = nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_LENGTH]); + if (request->length > ETH_MODULE_EEPROM_MAX_LEN) + return -EINVAL; + if (tb[ETHTOOL_A_EEPROM_DATA_PAGE] && + request->offset + request->length > ETH_MODULE_EEPROM_PAGE_LEN) + return -EINVAL; + + if (tb[ETHTOOL_A_EEPROM_DATA_PAGE]) + request->page = nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_PAGE]); + if (tb[ETHTOOL_A_EEPROM_DATA_BANK]) + request->bank = nla_get_u32(tb[ETHTOOL_A_EEPROM_DATA_BANK]); + + return 0; +} + +static int eeprom_data_reply_size(const struct ethnl_req_info *req_base, + const struct ethnl_reply_data *reply_base) +{ + const struct eeprom_data_req_info *request = EEPROM_DATA_REQINFO(req_base); + + return nla_total_size(sizeof(u32)) + /* _EEPROM_DATA_LENGTH */ + nla_total_size(sizeof(u32)) + /* _EEPROM_DATA_I2C_ADDRESS */ + nla_total_size(sizeof(u8) * request->length); /* _EEPROM_DATA */ +} + +static int eeprom_data_fill_reply(struct sk_buff *skb, + const struct ethnl_req_info *req_base, + const struct ethnl_reply_data *reply_base) +{ + struct eeprom_data_reply_data *reply = EEPROM_DATA_REPDATA(reply_base); + + if (nla_put_u32(skb, ETHTOOL_A_EEPROM_DATA_LENGTH, reply->length) || + nla_put_u32(skb, ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS, reply->i2c_address) || + nla_put(skb, ETHTOOL_A_EEPROM_DATA, reply->length, reply->data)) + return -EMSGSIZE; + + return 0; +} + +static void eeprom_data_cleanup_data(struct ethnl_reply_data *reply_base) +{ + struct eeprom_data_reply_data *reply = EEPROM_DATA_REPDATA(reply_base); + + kfree(reply->data); +} + +const struct ethnl_request_ops ethnl_eeprom_data_request_ops = { + .request_cmd = ETHTOOL_MSG_EEPROM_DATA_GET, + .reply_cmd = ETHTOOL_MSG_EEPROM_DATA_GET_REPLY, + .hdr_attr = ETHTOOL_A_EEPROM_DATA_HEADER, + .req_info_size = sizeof(struct eeprom_data_req_info), + .reply_data_size = sizeof(struct eeprom_data_reply_data), + + .parse_request = eeprom_data_parse_request, + .prepare_data = eeprom_data_prepare_data, + .reply_size = eeprom_data_reply_size, + .fill_reply = eeprom_data_fill_reply, + .cleanup_data = eeprom_data_cleanup_data, +}; + +const struct nla_policy ethnl_eeprom_data_get_policy[] = { + [ETHTOOL_A_EEPROM_DATA_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy), + [ETHTOOL_A_EEPROM_DATA_OFFSET] = { .type = NLA_U32 }, + [ETHTOOL_A_EEPROM_DATA_LENGTH] = { .type = NLA_U32 }, + [ETHTOOL_A_EEPROM_DATA_PAGE] = { .type = NLA_U32 }, + [ETHTOOL_A_EEPROM_DATA_BANK] = { .type = NLA_U32 }, + [ETHTOOL_A_EEPROM_DATA_I2C_ADDRESS] = { .type = NLA_U32 }, + [ETHTOOL_A_EEPROM_DATA] = { .type = NLA_BINARY }, +}; + diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index 50d3c8896f91..ff2528bee192 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -245,6 +245,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = { [ETHTOOL_MSG_PAUSE_GET] = ðnl_pause_request_ops, [ETHTOOL_MSG_EEE_GET] = ðnl_eee_request_ops, [ETHTOOL_MSG_TSINFO_GET] = ðnl_tsinfo_request_ops, + [ETHTOOL_MSG_EEPROM_DATA_GET] = ðnl_eeprom_data_request_ops, }; static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb) @@ -912,6 +913,15 @@ static const struct genl_ops ethtool_genl_ops[] = { .policy = ethnl_tunnel_info_get_policy, .maxattr = ARRAY_SIZE(ethnl_tunnel_info_get_policy) - 1, }, + { + .cmd = ETHTOOL_MSG_EEPROM_DATA_GET, + .doit = ethnl_default_doit, + .start = ethnl_default_start, + .dumpit = ethnl_default_dumpit, + .done = ethnl_default_done, + .policy = ethnl_eeprom_data_get_policy, + .maxattr = ARRAY_SIZE(ethnl_eeprom_data_get_policy) - 1, + }, }; static const struct genl_multicast_group ethtool_nl_mcgrps[] = { diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h index 6eabd58d81bf..60954c7b4dfe 100644 --- a/net/ethtool/netlink.h +++ b/net/ethtool/netlink.h @@ -344,6 +344,7 @@ extern const struct ethnl_request_ops ethnl_coalesce_request_ops; extern const struct ethnl_request_ops ethnl_pause_request_ops; extern const struct ethnl_request_ops ethnl_eee_request_ops; extern const struct ethnl_request_ops ethnl_tsinfo_request_ops; +extern const struct ethnl_request_ops ethnl_eeprom_data_request_ops; extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1]; extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1]; @@ -375,6 +376,7 @@ extern const struct nla_policy ethnl_tsinfo_get_policy[ETHTOOL_A_TSINFO_HEADER + extern const struct nla_policy ethnl_cable_test_act_policy[ETHTOOL_A_CABLE_TEST_HEADER + 1]; extern const struct nla_policy ethnl_cable_test_tdr_act_policy[ETHTOOL_A_CABLE_TEST_TDR_CFG + 1]; extern const struct nla_policy ethnl_tunnel_info_get_policy[ETHTOOL_A_TUNNEL_INFO_HEADER + 1]; +extern const struct nla_policy ethnl_eeprom_data_get_policy[ETHTOOL_A_EEPROM_DATA + 1]; int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info); int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info);