| Message ID | 4dcd60f25efe368ada4e0c035dc1d7612ab59132.1532953989.git.mkubecek@suse.cz |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | ethtool netlink interface (WiP) | expand |
Mon, Jul 30, 2018 at 02:53:27PM CEST, mkubecek@suse.cz wrote: [...] >+/* GET_DRVINFO / SET_DRVINFO */ >+ >+enum { >+ ETHA_DRVINFO_UNSPEC, >+ ETHA_DRVINFO_DEV, /* nest - ETHA_DEV_* */ >+ ETHA_DRVINFO_DRIVER, /* string */ >+ ETHA_DRVINFO_VERSION, /* string */ >+ ETHA_DRVINFO_FWVERSION, /* string */ >+ ETHA_DRVINFO_BUSINFO, /* string */ >+ ETHA_DRVINFO_EROM_VER, /* string */ >+ ETHA_DRVINFO_N_PRIV_FLAGS, /* u32 */ >+ ETHA_DRVINFO_N_STATS, /* u32 */ >+ ETHA_DRVINFO_TESTINFO_LEN, /* u32 */ >+ ETHA_DRVINFO_EEDUMP_LEN, /* u32 */ >+ ETHA_DRVINFO_REGDUMP_LEN, /* u32 */ This is a nice example of why 1:1 ioctl->netlink conversion would be a big mistake. I understand that for ioclt, getting lengths of various things is important. Userspace can prepare buffer for next ioctl which would actually do dump transfer. However in netlink, this is totally pointless as the dump goes into userspace in multiple netlink messages. We need to figure out the netlink uapi from scratch. [...]
On Mon, Jul 30, 2018 at 02:53:27PM +0200, Michal Kubecek wrote: > Requests the same information as ETHTOOL_GDRVINFO command in ioct > interface. This is read-only so that corresponding SET_DRVINFO exists but > is only used in kernel replies. > > Signed-off-by: Michal Kubecek <mkubecek@suse.cz> > --- > Documentation/networking/ethtool-netlink.txt | 38 +++++- > include/uapi/linux/ethtool_netlink.h | 22 ++++ > net/ethtool/Makefile | 4 +- > net/ethtool/common.c | 43 ++++++ > net/ethtool/common.h | 3 + > net/ethtool/drvinfo.c | 131 +++++++++++++++++++ > net/ethtool/ioctl.c | 42 +----- > net/ethtool/netlink.c | 8 ++ > 8 files changed, 252 insertions(+), 39 deletions(-) > create mode 100644 net/ethtool/drvinfo.c > > diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt > index 8b43f41a8140..1e3d5ffc97ab 100644 > --- a/Documentation/networking/ethtool-netlink.txt > +++ b/Documentation/networking/ethtool-netlink.txt > @@ -121,6 +121,8 @@ List of message types > ETHNL_CMD_EVENT notification only > ETHNL_CMD_GET_STRSET > ETHNL_CMD_SET_STRSET response only > + ETHNL_CMD_GET_DRVINFO > + ETHNL_CMD_SET_DRVINFO response only > > All constants use ETHNL_CMD_ prefix, usually followed by "GET", "SET" or "ACT" > to indicate the type. > @@ -156,6 +158,40 @@ and also multiple events of the same type (e.g. two or more newly registered > devices). > > > +GET_DRVINFO > +----------- > + > +GET_DRVINFO request corresponds to ETHTOOL_GDRVINFO ioctl command and provides > +basic driver information. > + > +Request contents: > + > + ETHA_DRVINFO_DEV (nested) device identification > + > +Kernel response contents: > + > + ETHA_DRVINFO_DEV (nested) device identification > + ETHA_DRVINFO_DRIVER (string) driver name > + ETHA_DRVINFO_VERSION (string) driver version Driver version is generally useless. Is version X.Y.Z of a driver the same when backported to some ancient enterprise kernel with 1000s of patches? Now seems like a good opportunity to drop it. > + ETHA_DRVINFO_FWVERSION (string) firmware version > + ETHA_DRVINFO_BUSINFO (string) device bus address > + ETHA_DRVINFO_EROM_VER (string) expansion ROM version > + ETHA_DRVINFO_N_PRIV_FLAGS (u32) number of private flags > + ETHA_DRVINFO_N_STATS (u32) number of device stats I know there is at least one driver that has somewhat dynamic number of statistics. It would be better to have the strings and the values in the same message, so there is no need to first get the number of strings, allocate the memory, get the strings, then get the values. > + ETHA_DRVINFO_TESTINFO_LEN (u32) number of test results In theory, this also does not need to be fixed. > + ETHA_DRVINFO_EEDUMP_LEN (u32) EEPROM dump size > + ETHA_DRVINFO_REGDUMP_LEN (u32) register dump size I would suggest removing all these _LEN properties, and let netlink return what it needs to return. Andrew
On Mon, Jul 30, 2018 at 03:21:07PM +0200, Jiri Pirko wrote: > Mon, Jul 30, 2018 at 02:53:27PM CEST, mkubecek@suse.cz wrote: > > [...] > > >+/* GET_DRVINFO / SET_DRVINFO */ > >+ > >+enum { > >+ ETHA_DRVINFO_UNSPEC, > >+ ETHA_DRVINFO_DEV, /* nest - ETHA_DEV_* */ > >+ ETHA_DRVINFO_DRIVER, /* string */ > >+ ETHA_DRVINFO_VERSION, /* string */ > >+ ETHA_DRVINFO_FWVERSION, /* string */ > >+ ETHA_DRVINFO_BUSINFO, /* string */ > >+ ETHA_DRVINFO_EROM_VER, /* string */ > >+ ETHA_DRVINFO_N_PRIV_FLAGS, /* u32 */ > >+ ETHA_DRVINFO_N_STATS, /* u32 */ > >+ ETHA_DRVINFO_TESTINFO_LEN, /* u32 */ > >+ ETHA_DRVINFO_EEDUMP_LEN, /* u32 */ > >+ ETHA_DRVINFO_REGDUMP_LEN, /* u32 */ > > This is a nice example of why 1:1 ioctl->netlink conversion would be > a big mistake. > > I understand that for ioclt, getting lengths of various things is > important. Userspace can prepare buffer for next ioctl which would > actually do dump transfer. However in netlink, this is totally pointless > as the dump goes into userspace in multiple netlink messages. Right, I already mentioned this in the ToDo part of cover letter. It makes indeed little sense to put this information here - and even less to put only dome of the counts and lengths. Michal Kubecek
On Mon, Jul 30, 2018 at 04:28:25PM +0200, Andrew Lunn wrote: > On Mon, Jul 30, 2018 at 02:53:27PM +0200, Michal Kubecek wrote: > > > + ETHA_DRVINFO_FWVERSION (string) firmware version > > + ETHA_DRVINFO_BUSINFO (string) device bus address > > + ETHA_DRVINFO_EROM_VER (string) expansion ROM version > > + ETHA_DRVINFO_N_PRIV_FLAGS (u32) number of private flags > > + ETHA_DRVINFO_N_STATS (u32) number of device stats > > I know there is at least one driver that has somewhat dynamic number > of statistics. It would be better to have the strings and the values > in the same message, so there is no need to first get the number of > strings, allocate the memory, get the strings, then get the values. > > > + ETHA_DRVINFO_TESTINFO_LEN (u32) number of test results > > In theory, this also does not need to be fixed. This is interesting. It would mean current (ioctl) ethtool approach with string set may not work correctly either. On the other hand, this should not be a problem for netlink interface. Statistics are unlikely to appear in notifications and daemons collecting them periodically will have to learn with it. Adding test name to notification "test started" or "test finished" seems quite natural. > > + ETHA_DRVINFO_EEDUMP_LEN (u32) EEPROM dump size > > + ETHA_DRVINFO_REGDUMP_LEN (u32) register dump size > > I would suggest removing all these _LEN properties, and let netlink > return what it needs to return. Agreed. Michal Kubecek
> This is interesting. It would mean current (ioctl) ethtool approach with > string set may not work correctly either. Hi Michal For the statistics, it is a bit of a corner case. One of the Ethernet switches in DSA can have two different PHYs linked to one MAC. One PHY is built in, the second is connected via a SERDES interface. Which every gets link first is used. However, the SERDES interface has additional statistics counters. So if the SERDES is in use, we return more statistics. If somebody was to plug in the cable at just the wrong/right time, the count of statistics could be different to the number of statistics. Another corner case i can think of. Some drivers return statistics per queue. And there is an ioctl to change the number of queues.... I could also imaging tests being similar. There are more loopback tests you can do with a SERDES which you cannot do with a built in PHY. But so far, i've not seen anything like that. Andrew
On Mon, Jul 30, 2018 at 05:48:03PM +0200, Andrew Lunn wrote: > > For the statistics, it is a bit of a corner case. One of the Ethernet > switches in DSA can have two different PHYs linked to one MAC. One PHY > is built in, the second is connected via a SERDES interface. Which > every gets link first is used. However, the SERDES interface has > additional statistics counters. So if the SERDES is in use, we return > more statistics. If somebody was to plug in the cable at just the > wrong/right time, the count of statistics could be different to the > number of statistics. > > Another corner case i can think of. Some drivers return statistics per > queue. And there is an ioctl to change the number of queues.... > > I could also imaging tests being similar. There are more loopback > tests you can do with a SERDES which you cannot do with a built in > PHY. But so far, i've not seen anything like that. Thank you for the explanation. What I have in mind is that there are two different types of userspace applications: one shot and running long term. It can be seen in my series in the way e.g. link modes are handled. There are two formats in which a bitset can be passed: verbose (list of bits with names) or compact (just bitmaps). For one shot queries (e.g. "ethtool eth0") it's easier to use verbose format so that you get names with bit values in one package. But for long running applications processing many messages ("ethtool --monitor" or config management daemons), it's more practical to load the names once and pass only data with each notification or response. I suppose in the scenarios you mentioned the driver would be able to trigger a notification whenever the list changes so that userspace application could reload the string set. Michal Kubecek
On Mon, 30 Jul 2018 14:53:27 +0200 (CEST), Michal Kubecek wrote: > +GET_DRVINFO > +----------- > + > +GET_DRVINFO request corresponds to ETHTOOL_GDRVINFO ioctl command and provides > +basic driver information. > + > +Request contents: > + > + ETHA_DRVINFO_DEV (nested) device identification > + > +Kernel response contents: > + > + ETHA_DRVINFO_DEV (nested) device identification > + ETHA_DRVINFO_DRIVER (string) driver name > + ETHA_DRVINFO_VERSION (string) driver version > + ETHA_DRVINFO_FWVERSION (string) firmware version FWIW I think fwinfo belongs to devlink, and should be split. Most modern drivers provide versions of multiple FW components smooshed into a single string. Perhaps it's time to allow this facility to carry multiple key: value entries? > + ETHA_DRVINFO_BUSINFO (string) device bus address I wonder if some of this information is also not duplicated with sysfs. There should be a link from the netdev to a sysfs device. Same for driver name. I'm probably missing some uses (in embedded world?) and will be corrected.. :) > + ETHA_DRVINFO_EROM_VER (string) expansion ROM version > + ETHA_DRVINFO_N_PRIV_FLAGS (u32) number of private flags > + ETHA_DRVINFO_N_STATS (u32) number of device stats > + ETHA_DRVINFO_TESTINFO_LEN (u32) number of test results > + ETHA_DRVINFO_EEDUMP_LEN (u32) EEPROM dump size > + ETHA_DRVINFO_REGDUMP_LEN (u32) register dump size
diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt index 8b43f41a8140..1e3d5ffc97ab 100644 --- a/Documentation/networking/ethtool-netlink.txt +++ b/Documentation/networking/ethtool-netlink.txt @@ -121,6 +121,8 @@ List of message types ETHNL_CMD_EVENT notification only ETHNL_CMD_GET_STRSET ETHNL_CMD_SET_STRSET response only + ETHNL_CMD_GET_DRVINFO + ETHNL_CMD_SET_DRVINFO response only All constants use ETHNL_CMD_ prefix, usually followed by "GET", "SET" or "ACT" to indicate the type. @@ -156,6 +158,40 @@ and also multiple events of the same type (e.g. two or more newly registered devices). +GET_DRVINFO +----------- + +GET_DRVINFO request corresponds to ETHTOOL_GDRVINFO ioctl command and provides +basic driver information. + +Request contents: + + ETHA_DRVINFO_DEV (nested) device identification + +Kernel response contents: + + ETHA_DRVINFO_DEV (nested) device identification + ETHA_DRVINFO_DRIVER (string) driver name + ETHA_DRVINFO_VERSION (string) driver version + ETHA_DRVINFO_FWVERSION (string) firmware version + ETHA_DRVINFO_BUSINFO (string) device bus address + ETHA_DRVINFO_EROM_VER (string) expansion ROM version + ETHA_DRVINFO_N_PRIV_FLAGS (u32) number of private flags + ETHA_DRVINFO_N_STATS (u32) number of device stats + ETHA_DRVINFO_TESTINFO_LEN (u32) number of test results + ETHA_DRVINFO_EEDUMP_LEN (u32) EEPROM dump size + ETHA_DRVINFO_REGDUMP_LEN (u32) register dump size + +The meaning of these follows the corresponding fields of ETHTOOL_GDRVINFO +response. + +All information is read only, SET_DRVINFO request is not implemented +(ETHNL_CMD_SET_DRVINFO messages are sent only by kernel in response to +GET_DRVINFO requests). + +GET_DRVINFO requests allow dumps. + + Request translation ------------------- @@ -167,7 +203,7 @@ ioctl command netlink command --------------------------------------------------------------------- ETHTOOL_GSET n/a ETHTOOL_SSET n/a -ETHTOOL_GDRVINFO n/a +ETHTOOL_GDRVINFO ETHNL_CMD_GET_DRVINFO ETHTOOL_GREGS n/a ETHTOOL_GWOL n/a ETHTOOL_SWOL n/a diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h index 5177c1940c2b..df4de61fac48 100644 --- a/include/uapi/linux/ethtool_netlink.h +++ b/include/uapi/linux/ethtool_netlink.h @@ -10,6 +10,8 @@ enum { ETHNL_CMD_EVENT, /* only for notifications */ ETHNL_CMD_GET_STRSET, ETHNL_CMD_SET_STRSET, /* only for reply */ + ETHNL_CMD_GET_DRVINFO, + ETHNL_CMD_SET_DRVINFO, /* only for reply */ __ETHNL_CMD_MAX, ETHNL_CMD_MAX = (__ETHNL_CMD_MAX - 1) @@ -124,6 +126,26 @@ enum { ETHA_STRSET_MAX = (__ETHA_STRSET_MAX - 1) }; +/* GET_DRVINFO / SET_DRVINFO */ + +enum { + ETHA_DRVINFO_UNSPEC, + ETHA_DRVINFO_DEV, /* nest - ETHA_DEV_* */ + ETHA_DRVINFO_DRIVER, /* string */ + ETHA_DRVINFO_VERSION, /* string */ + ETHA_DRVINFO_FWVERSION, /* string */ + ETHA_DRVINFO_BUSINFO, /* string */ + ETHA_DRVINFO_EROM_VER, /* string */ + ETHA_DRVINFO_N_PRIV_FLAGS, /* u32 */ + ETHA_DRVINFO_N_STATS, /* u32 */ + ETHA_DRVINFO_TESTINFO_LEN, /* u32 */ + ETHA_DRVINFO_EEDUMP_LEN, /* u32 */ + ETHA_DRVINFO_REGDUMP_LEN, /* u32 */ + + __ETHA_DRVINFO_MAX, + ETHA_DRVINFO_MAX = (__ETHA_DRVINFO_MAX - 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 ba260d5b53b2..2e840ae0ba1e 100644 --- a/net/ethtool/Makefile +++ b/net/ethtool/Makefile @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 -obj-y += ioctl.o +obj-y += ioctl.o common.o obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_nl.o -ethtool_nl-y := netlink.o strset.o +ethtool_nl-y := netlink.o strset.o drvinfo.o diff --git a/net/ethtool/common.c b/net/ethtool/common.c index 208259c51b73..1dc4a6515996 100644 --- a/net/ethtool/common.c +++ b/net/ethtool/common.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#include <linux/rtnetlink.h> #include "common.h" const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = { @@ -85,3 +86,45 @@ phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = { [ETHTOOL_PHY_DOWNSHIFT] = "phy-downshift", }; EXPORT_SYMBOL(phy_tunable_strings); + +int __ethtool_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info) +{ + const struct ethtool_ops *ops = dev->ethtool_ops; + + memset(info, 0, sizeof(*info)); + info->cmd = ETHTOOL_GDRVINFO; + if (ops->get_drvinfo) { + ops->get_drvinfo(dev, info); + } else if (dev->dev.parent && dev->dev.parent->driver) { + strlcpy(info->bus_info, dev_name(dev->dev.parent), + sizeof(info->bus_info)); + strlcpy(info->driver, dev->dev.parent->driver->name, + sizeof(info->driver)); + } else { + return -EOPNOTSUPP; + } + + /* this method of obtaining string set info is deprecated; + * Use ETHTOOL_GSSET_INFO instead. + */ + if (ops->get_sset_count) { + int rc; + + rc = ops->get_sset_count(dev, ETH_SS_TEST); + if (rc >= 0) + info->testinfo_len = rc; + rc = ops->get_sset_count(dev, ETH_SS_STATS); + if (rc >= 0) + info->n_stats = rc; + rc = ops->get_sset_count(dev, ETH_SS_PRIV_FLAGS); + if (rc >= 0) + info->n_priv_flags = rc; + } + if (ops->get_regs_len) + info->regdump_len = ops->get_regs_len(dev); + if (ops->get_eeprom_len) + info->eedump_len = ops->get_eeprom_len(dev); + + return 0; +} +EXPORT_SYMBOL(__ethtool_get_drvinfo); diff --git a/net/ethtool/common.h b/net/ethtool/common.h index 45c6492e4aee..0f768c1be527 100644 --- a/net/ethtool/common.h +++ b/net/ethtool/common.h @@ -3,6 +3,7 @@ #ifndef _ETHTOOL_COMMON_H #define _ETHTOOL_COMMON_H +#include <linux/netdevice.h> #include <linux/ethtool.h> extern const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]; @@ -10,4 +11,6 @@ extern const char rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LE extern const char tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN]; extern const char phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN]; +int __ethtool_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info); + #endif /* _ETHTOOL_COMMON_H */ diff --git a/net/ethtool/drvinfo.c b/net/ethtool/drvinfo.c new file mode 100644 index 000000000000..2bdaf6d7f28c --- /dev/null +++ b/net/ethtool/drvinfo.c @@ -0,0 +1,131 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ + +#include "netlink.h" +#include "common.h" + +static const struct nla_policy get_drvinfo_policy[ETHA_DRVINFO_MAX + 1] = { + [ETHA_DRVINFO_DEV] = { .type = NLA_NESTED }, +}; + +static int prepare_drvinfo(struct ethtool_drvinfo *data, struct genl_info *info, + struct net_device *dev) +{ + int ret; + + memset(data, '\0', sizeof(*data)); + rtnl_lock(); + ret = __ethtool_get_drvinfo(dev, data); + rtnl_unlock(); + if (ret < 0) { + ETHNL_SET_ERRMSG(info, "failed to retrieve driver info"); + return ret; + } + + return 0; +} + +static int drvinfo_size(struct ethtool_drvinfo *drvinfo) +{ + int len = 0; + + len += ethnl_str_ifne_size(drvinfo->driver); + len += ethnl_str_ifne_size(drvinfo->version); + len += ethnl_str_ifne_size(drvinfo->fw_version); + len += ethnl_str_ifne_size(drvinfo->bus_info); + len += ethnl_str_ifne_size(drvinfo->erom_version); + /* n_priv_flags, n_stats, testinfo_len, eedump_len, regdump_len */ + len += 5 * nla_total_size(sizeof(u32)); + + return len; +} + +static int fill_drvinfo(struct sk_buff *skb, struct net_device *dev, + struct ethtool_drvinfo *drvinfo) +{ + int ret; + + ret = -EMSGSIZE; + if (ethnl_put_str_ifne(skb, ETHA_DRVINFO_DRIVER, drvinfo->driver) || + ethnl_put_str_ifne(skb, ETHA_DRVINFO_VERSION, drvinfo->version) || + ethnl_put_str_ifne(skb, ETHA_DRVINFO_FWVERSION, + drvinfo->fw_version) || + ethnl_put_str_ifne(skb, ETHA_DRVINFO_BUSINFO, drvinfo->bus_info) || + ethnl_put_str_ifne(skb, ETHA_DRVINFO_EROM_VER, + drvinfo->erom_version) || + nla_put_u32(skb, ETHA_DRVINFO_N_PRIV_FLAGS, + drvinfo->n_priv_flags) || + nla_put_u32(skb, ETHA_DRVINFO_N_STATS, drvinfo->n_stats) || + nla_put_u32(skb, ETHA_DRVINFO_TESTINFO_LEN, + drvinfo->testinfo_len) || + nla_put_u32(skb, ETHA_DRVINFO_EEDUMP_LEN, drvinfo->eedump_len) || + nla_put_u32(skb, ETHA_DRVINFO_REGDUMP_LEN, drvinfo->regdump_len)) + return ret; + + return 0; +} + +int ethnl_get_drvinfo(struct sk_buff *skb, struct genl_info *info) +{ + struct nlattr *tb[ETHA_DRVINFO_MAX + 1]; + struct ethtool_drvinfo drvinfo; + struct net_device *dev; + struct sk_buff *rskb; + int reply_len; + void *ehdr; + int ret; + + ret = genlmsg_parse(info->nlhdr, ðtool_genl_family, tb, + ETHA_DRVINFO_MAX, get_drvinfo_policy, info->extack); + if (ret < 0) + return ret; + dev = ethnl_dev_get(info, tb[ETHA_DRVINFO_DEV]); + if (IS_ERR(dev)) + return PTR_ERR(dev); + + ret = prepare_drvinfo(&drvinfo, info, dev); + if (ret < 0) + goto err_dev; + reply_len = drvinfo_size(&drvinfo); + rskb = ethnl_reply_init(reply_len, dev, ETHNL_CMD_SET_DRVINFO, + ETHA_DRVINFO_DEV, info, &ehdr); + if (!rskb) + return -ENOMEM; + ret = fill_drvinfo(rskb, dev, &drvinfo); + if (ret < 0) + goto err; + + genlmsg_end(rskb, ehdr); + dev_put(dev); + return genlmsg_reply(rskb, info); + +err: + WARN_ONCE(ret == -EMSGSIZE, + "calculated message payload length (%d) not sufficient\n", + reply_len); + nlmsg_free(rskb); +err_dev: + dev_put(dev); + return ret; +} + +static int drvinfo_dump(struct sk_buff *skb, struct netlink_callback *cb, + struct net_device *dev) +{ + struct ethtool_drvinfo drvinfo; + int ret; + + ret = prepare_drvinfo(&drvinfo, NULL, dev); + if (ret < 0) + return ret; + ret = ethnl_fill_dev(skb, dev, ETHA_DRVINFO_DEV); + ret = fill_drvinfo(skb, dev, &drvinfo); + return ret; +} + +int ethnl_drvinfo_start(struct netlink_callback *cb) +{ + cb->args[0] = (long)drvinfo_dump; + cb->args[1] = ETHNL_CMD_SET_DRVINFO; + + return 0; +} diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index a91b597073f8..7b5831d35bca 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -27,6 +27,7 @@ #include <linux/rtnetlink.h> #include <linux/sched/signal.h> #include <linux/net.h> +#include "common.h" /* * Some useful ethtool_ops methods that're device independent. @@ -768,45 +769,14 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev, void __user *useraddr) { struct ethtool_drvinfo info; - const struct ethtool_ops *ops = dev->ethtool_ops; - - memset(&info, 0, sizeof(info)); - info.cmd = ETHTOOL_GDRVINFO; - if (ops->get_drvinfo) { - ops->get_drvinfo(dev, &info); - } else if (dev->dev.parent && dev->dev.parent->driver) { - strlcpy(info.bus_info, dev_name(dev->dev.parent), - sizeof(info.bus_info)); - strlcpy(info.driver, dev->dev.parent->driver->name, - sizeof(info.driver)); - } else { - return -EOPNOTSUPP; - } - - /* - * this method of obtaining string set info is deprecated; - * Use ETHTOOL_GSSET_INFO instead. - */ - if (ops->get_sset_count) { - int rc; - - rc = ops->get_sset_count(dev, ETH_SS_TEST); - if (rc >= 0) - info.testinfo_len = rc; - rc = ops->get_sset_count(dev, ETH_SS_STATS); - if (rc >= 0) - info.n_stats = rc; - rc = ops->get_sset_count(dev, ETH_SS_PRIV_FLAGS); - if (rc >= 0) - info.n_priv_flags = rc; - } - if (ops->get_regs_len) - info.regdump_len = ops->get_regs_len(dev); - if (ops->get_eeprom_len) - info.eedump_len = ops->get_eeprom_len(dev); + int rc; + rc = __ethtool_get_drvinfo(dev, &info); + if (rc < 0) + return rc; if (copy_to_user(useraddr, &info, sizeof(info))) return -EFAULT; + return 0; } diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index 237a2cb40be4..305baa02ff70 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -670,8 +670,10 @@ static struct notifier_block ethnl_netdev_notifier = { /* genetlink setup */ int ethnl_get_strset(struct sk_buff *skb, struct genl_info *info); +int ethnl_get_drvinfo(struct sk_buff *skb, struct genl_info *info); int ethnl_strset_start(struct netlink_callback *cb); +int ethnl_drvinfo_start(struct netlink_callback *cb); int ethnl_strset_done(struct netlink_callback *cb); @@ -683,6 +685,12 @@ static const struct genl_ops ethtool_genl_ops[] = { .dumpit = ethnl_dumpit, .done = ethnl_strset_done, }, + { + .cmd = ETHNL_CMD_GET_DRVINFO, + .doit = ethnl_get_drvinfo, + .start = ethnl_drvinfo_start, + .dumpit = ethnl_dumpit, + }, }; static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
Requests the same information as ETHTOOL_GDRVINFO command in ioct interface. This is read-only so that corresponding SET_DRVINFO exists but is only used in kernel replies. Signed-off-by: Michal Kubecek <mkubecek@suse.cz> --- Documentation/networking/ethtool-netlink.txt | 38 +++++- include/uapi/linux/ethtool_netlink.h | 22 ++++ net/ethtool/Makefile | 4 +- net/ethtool/common.c | 43 ++++++ net/ethtool/common.h | 3 + net/ethtool/drvinfo.c | 131 +++++++++++++++++++ net/ethtool/ioctl.c | 42 +----- net/ethtool/netlink.c | 8 ++ 8 files changed, 252 insertions(+), 39 deletions(-) create mode 100644 net/ethtool/drvinfo.c