diff mbox series

[RFC,net-next,v3,12/21] ethtool: provide permanent hardware address in GET_INFO request

Message ID 4e5879e36289c526dc79db37e55e5fc7d89d15fe.1550513384.git.mkubecek@suse.cz
State RFC
Delegated to: David Miller
Headers show
Series ethtool netlink interface, part 1 | expand

Commit Message

Michal Kubecek Feb. 18, 2019, 6:22 p.m. UTC
Add information about permanent hadware address of a device (as provided by
ETHTOOL_GPERMADDR ioctl command) in GET_INFO reply if ETH_INFO_IM_PERMADDR
flag is set in the request.

There is no separate attribute for hardware address length as nla_len gives
this information. The reply also provides address type (net_device::type).

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.txt |  9 ++++-
 include/uapi/linux/ethtool_netlink.h         | 12 +++++-
 net/ethtool/info.c                           | 39 ++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)

Comments

Jiri Pirko Feb. 19, 2019, 10:24 a.m. UTC | #1
Mon, Feb 18, 2019 at 07:22:24PM CET, mkubecek@suse.cz wrote:
>Add information about permanent hadware address of a device (as provided by
>ETHTOOL_GPERMADDR ioctl command) in GET_INFO reply if ETH_INFO_IM_PERMADDR
>flag is set in the request.
>
>There is no separate attribute for hardware address length as nla_len gives
>this information. The reply also provides address type (net_device::type).
>
>Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
>---
> Documentation/networking/ethtool-netlink.txt |  9 ++++-
> include/uapi/linux/ethtool_netlink.h         | 12 +++++-
> net/ethtool/info.c                           | 39 ++++++++++++++++++++
> 3 files changed, 58 insertions(+), 2 deletions(-)
>
>diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
>index b6999a2167e8..1e615e111262 100644
>--- a/Documentation/networking/ethtool-netlink.txt
>+++ b/Documentation/networking/ethtool-netlink.txt
>@@ -239,6 +239,9 @@ Kernel response contents:
>         ETHA_DRVINFO_FWVERSION		(string)	firmware version
>         ETHA_DRVINFO_BUSINFO		(string)	device bus address
>         ETHA_DRVINFO_EROM_VER		(string)	expansion ROM version
>+    ETHA_INFO_PERMADDR		(nested)
>+        ETHA_PERMADDR_ADDRESS		(binary)	permanent HW address

I think this is a nice example of thing that should not be exposed with
ethtool but rather via rtnetlink, alongside with the actual hw address.

[...]
Michal Kubecek Feb. 19, 2019, 11:36 a.m. UTC | #2
On Tue, Feb 19, 2019 at 11:24:00AM +0100, Jiri Pirko wrote:
> Mon, Feb 18, 2019 at 07:22:24PM CET, mkubecek@suse.cz wrote:
> >Add information about permanent hadware address of a device (as provided by
> >ETHTOOL_GPERMADDR ioctl command) in GET_INFO reply if ETH_INFO_IM_PERMADDR
> >flag is set in the request.
> >
> >There is no separate attribute for hardware address length as nla_len gives
> >this information. The reply also provides address type (net_device::type).
> >
> >Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> >---
> > Documentation/networking/ethtool-netlink.txt |  9 ++++-
> > include/uapi/linux/ethtool_netlink.h         | 12 +++++-
> > net/ethtool/info.c                           | 39 ++++++++++++++++++++
> > 3 files changed, 58 insertions(+), 2 deletions(-)
> >
> >diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
> >index b6999a2167e8..1e615e111262 100644
> >--- a/Documentation/networking/ethtool-netlink.txt
> >+++ b/Documentation/networking/ethtool-netlink.txt
> >@@ -239,6 +239,9 @@ Kernel response contents:
> >         ETHA_DRVINFO_FWVERSION		(string)	firmware version
> >         ETHA_DRVINFO_BUSINFO		(string)	device bus address
> >         ETHA_DRVINFO_EROM_VER		(string)	expansion ROM version
> >+    ETHA_INFO_PERMADDR		(nested)
> >+        ETHA_PERMADDR_ADDRESS		(binary)	permanent HW address
> 
> I think this is a nice example of thing that should not be exposed with
> ethtool but rather via rtnetlink, alongside with the actual hw address.
> 
> [...]

I guess you are right. As we don't have to query the driver and just
read the information from struct net_device, rtnetlink does indeed seem
more appropriate.

Michal
diff mbox series

Patch

diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
index b6999a2167e8..1e615e111262 100644
--- a/Documentation/networking/ethtool-netlink.txt
+++ b/Documentation/networking/ethtool-netlink.txt
@@ -239,6 +239,9 @@  Kernel response contents:
         ETHA_DRVINFO_FWVERSION		(string)	firmware version
         ETHA_DRVINFO_BUSINFO		(string)	device bus address
         ETHA_DRVINFO_EROM_VER		(string)	expansion ROM version
+    ETHA_INFO_PERMADDR		(nested)
+        ETHA_PERMADDR_ADDRESS		(binary)	permanent HW address
+        ETHA_PERMADDR_TYPE		(u16)		dev->type
 
 The meaning of DRVINFO attributes follows the corresponding fields of
 ETHTOOL_GDRVINFO response. Second part with various counts and sizes is
@@ -246,6 +249,10 @@  omitted as these are not really needed (and if they are, they can be easily
 found by different means). Driver version is also omitted as it is rather
 misleading in most cases.
 
+There is no separate attribute for permanent address length as the length can
+be determined from attribute length (nla_len()). ETHA_PERMADDR_TYPE provides
+net_device::type value to give a hint about what kind of address device has.
+
 GET_INFO requests allow dumps.
 
 
@@ -288,7 +295,7 @@  ETHTOOL_PHYS_ID			n/a
 ETHTOOL_GSTATS			n/a
 ETHTOOL_GTSO			n/a
 ETHTOOL_STSO			n/a
-ETHTOOL_GPERMADDR		n/a
+ETHTOOL_GPERMADDR		ETHNL_CMD_GET_INFO
 ETHTOOL_GUFO			n/a
 ETHTOOL_SUFO			n/a
 ETHTOOL_GGSO			n/a
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index fdae12b6c6b6..fb756b6a8592 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -153,8 +153,9 @@  enum {
 };
 
 #define ETH_INFO_IM_DRVINFO			0x01
+#define ETH_INFO_IM_PERMADDR			0x02
 
-#define ETH_INFO_IM_ALL				0x01
+#define ETH_INFO_IM_ALL				0x03
 
 enum {
 	ETHA_DRVINFO_UNSPEC,
@@ -167,6 +168,15 @@  enum {
 	ETHA_DRVINFO_MAX = (__ETHA_DRVINFO_CNT - 1)
 };
 
+enum {
+	ETHA_PERMADDR_UNSPEC,
+	ETHA_PERMADDR_ADDRESS,			/* binary */
+	ETHA_PERMADDR_TYPE,			/* u16 */
+
+	__ETHA_PERMADDR_CNT,
+	ETHA_PERMADDR_MAX = (__ETHA_PERMADDR_CNT - 1)
+};
+
 /* generic netlink info */
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/info.c b/net/ethtool/info.c
index 1a2e78d238e3..05dbd87ebc41 100644
--- a/net/ethtool/info.c
+++ b/net/ethtool/info.c
@@ -18,6 +18,7 @@  static const struct nla_policy get_info_policy[ETHA_INFO_MAX + 1] = {
 	[ETHA_INFO_INFOMASK]		= { .type = NLA_U32 },
 	[ETHA_INFO_COMPACT]		= { .type = NLA_FLAG },
 	[ETHA_INFO_DRVINFO]		= { .type = NLA_REJECT },
+	[ETHA_INFO_PERMADDR]		= { .type = NLA_REJECT },
 };
 
 static int parse_info(struct common_req_info *req_info, struct sk_buff *skb,
@@ -86,16 +87,29 @@  static int drvinfo_size(const struct ethtool_drvinfo *drvinfo)
 	return nla_total_size(len);
 }
 
+static int permaddr_size(const struct net_device *dev)
+{
+	int len = 0;
+
+	len += nla_total_size(dev->addr_len);
+	len += nla_total_size(sizeof(u16));
+
+	return nla_total_size(len);
+}
+
 static int info_size(const struct common_req_info *req_info)
 {
 	const struct info_data *data =
 		container_of(req_info, struct info_data, reqinfo_base);
+	const struct net_device *dev = data->repdata_base.dev;
 	u32 info_mask = data->repdata_base.info_mask;
 	int len = 0;
 
 	len += dev_ident_size();
 	if (info_mask & ETH_INFO_IM_DRVINFO)
 		len += drvinfo_size(&data->drvinfo);
+	if (info_mask & ETH_INFO_IM_PERMADDR)
+		len += permaddr_size(dev);
 
 	return len;
 }
@@ -124,6 +138,26 @@  static int fill_drvinfo(struct sk_buff *skb,
 	return ret;
 }
 
+static int fill_permaddr(struct sk_buff *skb, const struct net_device *dev)
+{
+	struct nlattr *nest = ethnl_nest_start(skb, ETHA_INFO_PERMADDR);
+	int ret;
+
+	if (!nest)
+		return -EMSGSIZE;
+	ret = -EMSGSIZE;
+	if (nla_put(skb, ETHA_PERMADDR_ADDRESS, dev->addr_len, dev->perm_addr))
+		goto err;
+	if (nla_put_u16(skb, ETHA_PERMADDR_TYPE, dev->type))
+		goto err;
+
+	nla_nest_end(skb, nest);
+	return 0;
+err:
+	nla_nest_cancel(skb, nest);
+	return ret;
+}
+
 static int fill_info(struct sk_buff *skb,
 		     const struct common_req_info *req_info)
 {
@@ -137,6 +171,11 @@  static int fill_info(struct sk_buff *skb,
 		if (ret < 0)
 			return ret;
 	}
+	if (info_mask & ETH_INFO_IM_PERMADDR) {
+		ret = fill_permaddr(skb, data->repdata_base.dev);
+		if (ret < 0)
+			return ret;
+	}
 
 	return 0;
 }