Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v6 01/15] rtnetlink: provide permanent hardware address in RTM_NEWLINK
From: Jiri Pirko @ 2019-07-02 11:57 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: David Miller, netdev, Jakub Kicinski, Andrew Lunn,
	Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
	linux-kernel
In-Reply-To: <b6e0aefbcb58297b3ec0a12ee4be8e5194eee61a.1562067622.git.mkubecek@suse.cz>

Tue, Jul 02, 2019 at 01:49:44PM CEST, mkubecek@suse.cz wrote:
>Permanent hardware address of a network device was traditionally provided
>via ethtool ioctl interface but as Jiri Pirko pointed out in a review of
>ethtool netlink interface, rtnetlink is much more suitable for it so let's
>add it to the RTM_NEWLINK message.
>
>Add IFLA_PERM_ADDRESS attribute to RTM_NEWLINK messages unless the
>permanent address is all zeros (i.e. device driver did not fill it). As
>permanent address is not modifiable, reject userspace requests containing
>IFLA_PERM_ADDRESS attribute.
>
>Note: we already provide permanent hardware address for bond slaves;
>unfortunately we cannot drop that attribute for backward compatibility
>reasons.
>
>v5 -> v6: only add the attribute if permanent address is not zero
>
>Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* [PATCH net-next v6 01/15] rtnetlink: provide permanent hardware address in RTM_NEWLINK
From: Michal Kubecek @ 2019-07-02 11:49 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Permanent hardware address of a network device was traditionally provided
via ethtool ioctl interface but as Jiri Pirko pointed out in a review of
ethtool netlink interface, rtnetlink is much more suitable for it so let's
add it to the RTM_NEWLINK message.

Add IFLA_PERM_ADDRESS attribute to RTM_NEWLINK messages unless the
permanent address is all zeros (i.e. device driver did not fill it). As
permanent address is not modifiable, reject userspace requests containing
IFLA_PERM_ADDRESS attribute.

Note: we already provide permanent hardware address for bond slaves;
unfortunately we cannot drop that attribute for backward compatibility
reasons.

v5 -> v6: only add the attribute if permanent address is not zero

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 include/uapi/linux/if_link.h | 1 +
 net/core/rtnetlink.c         | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 6f75bda2c2d7..1c79d6283a4d 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -167,6 +167,7 @@ enum {
 	IFLA_NEW_IFINDEX,
 	IFLA_MIN_MTU,
 	IFLA_MAX_MTU,
+	IFLA_PERM_ADDRESS,
 	__IFLA_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1ee6460f8275..9aae53e8914e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1027,6 +1027,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + nla_total_size(4)  /* IFLA_CARRIER_DOWN_COUNT */
 	       + nla_total_size(4)  /* IFLA_MIN_MTU */
 	       + nla_total_size(4)  /* IFLA_MAX_MTU */
+	       + nla_total_size(MAX_ADDR_LEN) /* IFLA_PERM_ADDRESS */
 	       + 0;
 }
 
@@ -1691,6 +1692,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
 	    nla_put_s32(skb, IFLA_NEW_IFINDEX, new_ifindex) < 0)
 		goto nla_put_failure;
 
+	if (memchr_inv(dev->perm_addr, '\0', dev->addr_len) &&
+	    nla_put(skb, IFLA_PERM_ADDRESS, dev->addr_len, dev->perm_addr))
+		goto nla_put_failure;
 
 	rcu_read_lock();
 	if (rtnl_fill_link_af(skb, dev, ext_filter_mask))
@@ -1750,6 +1754,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
 	[IFLA_MIN_MTU]		= { .type = NLA_U32 },
 	[IFLA_MAX_MTU]		= { .type = NLA_U32 },
+	[IFLA_PERM_ADDRESS]	= { .type = NLA_REJECT },
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 15/15] ethtool: provide link state in SETTINGS_GET request
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Add information about device link state (as provided by ETHTOOL_GLINK ioctl
command) into the SETTINGS_GET reply when ETHTOOL_IM_SETTINGS_LINKSTATE
bit is set in the request info mask.

We cannot use NLA_FLAG for link state as we need three states: off, on and
unknown. The attribute is encapsulated in a nest to allow future extensions
(e.g. link down reason or more detailed link state information).

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.txt |  5 ++-
 include/uapi/linux/ethtool_netlink.h         | 14 +++++++-
 net/ethtool/common.c                         |  8 +++++
 net/ethtool/common.h                         |  3 ++
 net/ethtool/ioctl.c                          |  8 ++---
 net/ethtool/settings.c                       | 37 ++++++++++++++++++++
 6 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
index 05bc8f5f8654..dc06e33329a4 100644
--- a/Documentation/networking/ethtool-netlink.txt
+++ b/Documentation/networking/ethtool-netlink.txt
@@ -250,6 +250,7 @@ Info mask bits meaning:
 
     ETHTOOL_IM_SETTINGS_LINKINFO	link settings
     ETHTOOL_IM_SETTINGS_LINKMODES	link modes and related
+    ETHTOOL_IM_SETTINGS_LINKSTATE	link state
 
 Response contents:
 
@@ -266,6 +267,8 @@ Response contents:
         ETHTOOL_A_LINKMODES_PEER	    (bitset)	    partner link modes
         ETHTOOL_A_LINKMODES_SPEED	    (u32)	    link speed (Mb/s)
         ETHTOOL_A_LINKMODES_DUPLEX	    (u8)	    duplex mode
+    ETHTOOL_A_SETTINGS_LINK_STATE	(nested)	link state
+        ETHTOOL_A_LINKSTATE_LINK	    (u8)	    link on/off/unknown
 
 Most of the attributes and their values have the same meaning as matching
 members of the corresponding ioctl structures. For ETHTOOL_A_LINKMODES_OURS,
@@ -323,7 +326,7 @@ ETHTOOL_SWOL			n/a
 ETHTOOL_GMSGLVL			n/a
 ETHTOOL_SMSGLVL			n/a
 ETHTOOL_NWAY_RST		n/a
-ETHTOOL_GLINK			n/a
+ETHTOOL_GLINK			ETHNL_CMD_GET_SETTINGS
 ETHTOOL_GEEPROM			n/a
 ETHTOOL_SEEPROM			n/a
 ETHTOOL_GCOALESCE		n/a
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 8ccf66ed3f58..46c13455246f 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -158,6 +158,7 @@ enum {
 	ETHTOOL_A_SETTINGS_HEADER,		/* nest - _A_HEADER_* */
 	ETHTOOL_A_SETTINGS_LINK_INFO,		/* nest - _A_LINKINFO_* */
 	ETHTOOL_A_SETTINGS_LINK_MODES,		/* nest - _A_LINKMODES_* */
+	ETHTOOL_A_SETTINGS_LINK_STATE,		/* nest - _A_LINKSTATE_* */
 
 	/* add new constants above here */
 	__ETHTOOL_A_SETTINGS_CNT,
@@ -166,9 +167,11 @@ enum {
 
 #define ETHTOOL_IM_SETTINGS_LINKINFO		(1U << 0)
 #define ETHTOOL_IM_SETTINGS_LINKMODES		(1U << 1)
+#define ETHTOOL_IM_SETTINGS_LINKSTATE		(1U << 2)
 
 #define ETHTOOL_IM_SETTINGS_ALL (ETHTOOL_IM_SETTINGS_LINKINFO | \
-				 ETHTOOL_IM_SETTINGS_LINKMODES)
+				 ETHTOOL_IM_SETTINGS_LINKMODES | \
+				 ETHTOOL_IM_SETTINGS_LINKSTATE)
 
 #define ETHTOOL_RF_SETTINGS_ALL 0
 
@@ -198,6 +201,15 @@ enum {
 	ETHTOOL_A_LINKMODES_MAX = (__ETHTOOL_A_LINKMODES_CNT - 1)
 };
 
+enum {
+	ETHTOOL_A_LINKSTATE_UNSPEC,
+	ETHTOOL_A_LINKSTATE_LINK,		/* u8 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_LINKSTATE_CNT,
+	ETHTOOL_A_LINKSTATE_MAX = (__ETHTOOL_A_LINKSTATE_CNT - 1)
+};
+
 /* generic netlink info */
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index abb00b3a7e77..b06635ad2620 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -130,3 +130,11 @@ convert_legacy_settings_to_link_ksettings(
 		= legacy_settings->eth_tp_mdix_ctrl;
 	return retval;
 }
+
+int __ethtool_get_link(struct net_device *dev)
+{
+	if (!dev->ethtool_ops->get_link)
+		return -EOPNOTSUPP;
+
+	return netif_running(dev) && dev->ethtool_ops->get_link(dev);
+}
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index 0381936d8e1e..a2c1504576c2 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
@@ -14,6 +15,8 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN];
 extern const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
 
+int __ethtool_get_link(struct net_device *dev);
+
 bool convert_legacy_settings_to_link_ksettings(
 	struct ethtool_link_ksettings *link_ksettings,
 	const struct ethtool_cmd *legacy_settings);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 504ab2f7009c..853b8c21a5e5 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1359,12 +1359,12 @@ static int ethtool_nway_reset(struct net_device *dev)
 static int ethtool_get_link(struct net_device *dev, char __user *useraddr)
 {
 	struct ethtool_value edata = { .cmd = ETHTOOL_GLINK };
+	int link = __ethtool_get_link(dev);
 
-	if (!dev->ethtool_ops->get_link)
-		return -EOPNOTSUPP;
-
-	edata.data = netif_running(dev) && dev->ethtool_ops->get_link(dev);
+	if (link < 0)
+		return link;
 
+	edata.data = link;
 	if (copy_to_user(useraddr, &edata, sizeof(edata)))
 		return -EFAULT;
 	return 0;
diff --git a/net/ethtool/settings.c b/net/ethtool/settings.c
index 2fc961297076..079d3776df71 100644
--- a/net/ethtool/settings.c
+++ b/net/ethtool/settings.c
@@ -11,6 +11,7 @@ struct settings_data {
 	struct ethnl_reply_data		repdata_base;
 	struct ethtool_link_ksettings	ksettings;
 	struct ethtool_link_settings	*lsettings;
+	int				link;
 	bool				lpm_empty;
 };
 
@@ -113,6 +114,7 @@ settings_get_policy[ETHTOOL_A_SETTINGS_MAX + 1] = {
 	[ETHTOOL_A_SETTINGS_HEADER]	= { .type = NLA_NESTED },
 	[ETHTOOL_A_SETTINGS_LINK_INFO]	= { .type = NLA_REJECT },
 	[ETHTOOL_A_SETTINGS_LINK_MODES]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_SETTINGS_LINK_STATE]	= { .type = NLA_REJECT },
 };
 
 static int ethnl_get_link_ksettings(struct genl_info *info,
@@ -140,6 +142,7 @@ static int settings_prepare(struct ethnl_req_info *req_info,
 
 	data->lsettings = &data->ksettings.base;
 	data->lpm_empty = true;
+	data->link = -EOPNOTSUPP;
 
 	ret = ethnl_before_ops(dev);
 	if (ret < 0)
@@ -162,6 +165,8 @@ static int settings_prepare(struct ethnl_req_info *req_info,
 		ethnl_bitmap_to_u32(data->ksettings.link_modes.lp_advertising,
 				    __ETHTOOL_LINK_MODE_MASK_NWORDS);
 	}
+	if (req_mask & ETHTOOL_IM_SETTINGS_LINKSTATE)
+		data->link = __ethtool_get_link(dev);
 	ethnl_after_ops(dev);
 
 	data->repdata_base.info_mask = req_mask;
@@ -212,6 +217,13 @@ settings_linkmodes_size(const struct ethtool_link_ksettings *ksettings,
 	return nla_total_size(len);
 }
 
+static int settings_linkstate_size(int link)
+{
+	if (link < 0)
+		return nla_total_size(0);
+	return nla_total_size(nla_total_size(sizeof(u8)));
+}
+
 /* reply_size() handler
  *
  * To keep things simple, reserve space for some attributes which may not
@@ -235,6 +247,8 @@ static int settings_size(const struct ethnl_req_info *req_info)
 			return ret;
 		len += ret;
 	}
+	if (info_mask & ETHTOOL_IM_SETTINGS_LINKSTATE)
+		len += settings_linkstate_size(data->link);
 
 	return len;
 }
@@ -310,6 +324,23 @@ settings_fill_linkmodes(struct sk_buff *skb,
 	return -EMSGSIZE;
 }
 
+static int settings_fill_linkstate(struct sk_buff *skb, int link)
+{
+	struct nlattr *nest;
+
+	nest = nla_nest_start(skb, ETHTOOL_A_SETTINGS_LINK_STATE);
+	if (!nest)
+		return -EMSGSIZE;
+	if (link >= 0 && nla_put_u8(skb, ETHTOOL_A_LINKSTATE_LINK, !!link))
+		goto nla_put_failure;
+	nla_nest_end(skb, nest);
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -EMSGSIZE;
+}
+
 /* fill_reply() handler */
 static int settings_fill(struct sk_buff *skb,
 			 const struct ethnl_req_info *req_info)
@@ -331,6 +362,11 @@ static int settings_fill(struct sk_buff *skb,
 		if (ret < 0)
 			return ret;
 	}
+	if (info_mask & ETHTOOL_IM_SETTINGS_LINKSTATE) {
+		ret = settings_fill_linkstate(skb, data->link);
+		if (ret < 0)
+			return ret;
+	}
 
 	return 0;
 }
@@ -389,6 +425,7 @@ settings_set_policy[ETHTOOL_A_SETTINGS_MAX + 1] = {
 	[ETHTOOL_A_SETTINGS_HEADER]		= { .type = NLA_NESTED },
 	[ETHTOOL_A_SETTINGS_LINK_INFO]		= { .type = NLA_NESTED },
 	[ETHTOOL_A_SETTINGS_LINK_MODES]		= { .type = NLA_NESTED },
+	[ETHTOOL_A_SETTINGS_LINK_STATE]		= { .type = NLA_REJECT },
 };
 
 static int ethnl_set_link_ksettings(struct genl_info *info,
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 14/15] ethtool: set link settings and link modes with SETTINGS_SET request
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Implement SETTINGS_SET netlink request allowing to set link settings and
advertised link modes as an alternative to ETHTOOL_SLINKSETTINGS and
ETHTOOL_SSET ioctl commands.

Only values which are intended to be set are to be included in the request.
Omitted attributes will be left at current values.

ETHTOOL_A_SETTINGS_LINK_MODES nested attribute is used to set (or modify)
advertised link modes and related settings (autonegotiation, speed and
duplex). Kernel implements logic which was already partially implemented in
ethtool for ioctl interface: if autonegotiation is on (either it was on
already or the request turns it on), no link mode change is requested (no
ETHTOOL_A_LINKMODES_OURS attribute) and speed or duplex are provided,
advertised link modes are set to supported modes matching requested speed
and/or duplex.

ETHTOOL_A_SETTINGS_LINK_INFO nested attribute is used to set physical port,
phy MDIO address and MDI(-X) control. An attempt to modify other attributes
provided by corresponding GET request is rejected.

When any data is modified, ETHTOOL_MSG_SETTINGS_NTF message in the same
format as a reply to GET request is sent to notify userspace about the
changes. The same notification is also sent when these settings are
modified using the ioctl interface.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.txt |  34 +-
 include/uapi/linux/ethtool_netlink.h         |   2 +
 net/ethtool/ioctl.c                          |  18 +-
 net/ethtool/netlink.c                        |  11 +
 net/ethtool/netlink.h                        |   2 +
 net/ethtool/settings.c                       | 332 +++++++++++++++++++
 6 files changed, 395 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
index 1d803488e02c..05bc8f5f8654 100644
--- a/Documentation/networking/ethtool-netlink.txt
+++ b/Documentation/networking/ethtool-netlink.txt
@@ -152,11 +152,13 @@ Userspace to kernel:
 
     ETHTOOL_MSG_STRSET_GET		get string set
     ETHTOOL_MSG_SETTINGS_GET		get device settings
+    ETHTOOL_MSG_SETTINGS_SET		set device settings
 
 Kernel to userspace:
 
     ETHTOOL_MSG_STRSET_GET_REPLY	string set contents
     ETHTOOL_MSG_SETTINGS_GET_REPLY	device settings
+    ETHTOOL_MSG_SETTINGS_NTF		device settings notification
 
 "GET" requests are sent by userspace applications to retrieve device
 information. They usually do not contain any message specific attributes.
@@ -275,6 +277,34 @@ to them are broadcasted as notifications on change of these settings using
 netlink or ioctl ethtool interface.
 
 
+SETTINGS_SET
+------------
+
+SETTINGS_SET request allows setting some of the data reported by SETTINGS_GET.
+Request flags, info_mask and index are ignored. These attributes are allowed
+to be passed with SETTINGS_SET request:
+
+    ETHTOOL_A_SETTINGS_HEADER		(nested)	request header
+    ETHTOOL_A_SETTINGS_LINK_INFO	(nested)	link settings
+        ETHTOOL_A_LINKINFO_PORT		    (u8)	    physical port
+        ETHTOOL_A_LINKINFO_PHYADDR	    (u8)	    MDIO address of phy
+        ETHTOOL_A_LINKINFO_TP_MDIX_CTRL	    (u8)	    MDI(-X) control
+    ETHTOOL_A_SETTINGS_LINK_MODES	(nested)	link modes
+        ETHTOOL_A_LINKMODES_AUTONEG	    (u8)	    autonegotiation
+        ETHTOOL_A_LINKMODES_OURS	    (bitset)	    advertised link modes
+        ETHTOOL_A_LINKMODES_SPEED	    (u32)	    link speed (Mb/s)
+        ETHTOOL_A_LINKMODES_DUPLEX	    (u8)	    duplex mode
+
+ETHTOOL_A_LINKMODES_OURS bit set allows setting advertised link modes. If
+autonegotiation is on (either set now or kept from before), advertised modes
+are not changed (no ETHTOOL_A_LINKMODES_OURS attribute) and at least one of
+speed and duplex is specified, kernel adjusts advertised modes to all
+supported modes matching speed, duplex or both (whatever is specified). This
+autoselection is done on ethtool side with ioctl interface, netlink interface
+is supposed to allow requesting changes without knowing what exactly kernel
+supports.
+
+
 Request translation
 -------------------
 
@@ -285,7 +315,7 @@ have their netlink replacement yet.
 ioctl command			netlink command
 ---------------------------------------------------------------------
 ETHTOOL_GSET			ETHTOOL_MSG_SETTINGS_GET
-ETHTOOL_SSET			n/a
+ETHTOOL_SSET			ETHTOOL_MSG_SETTINGS_SET
 ETHTOOL_GDRVINFO		n/a
 ETHTOOL_GREGS			n/a
 ETHTOOL_GWOL			n/a
@@ -359,7 +389,7 @@ ETHTOOL_STUNABLE		n/a
 ETHTOOL_GPHYSTATS		n/a
 ETHTOOL_PERQUEUE		n/a
 ETHTOOL_GLINKSETTINGS		ETHTOOL_MSG_SETTINGS_GET
-ETHTOOL_SLINKSETTINGS		n/a
+ETHTOOL_SLINKSETTINGS		ETHTOOL_MSG_SETTINGS_SET
 ETHTOOL_PHY_GTUNABLE		n/a
 ETHTOOL_PHY_STUNABLE		n/a
 ETHTOOL_GFECPARAM		n/a
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index a046dd8da50e..8ccf66ed3f58 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -16,6 +16,7 @@ enum {
 	ETHTOOL_MSG_USER_NONE,
 	ETHTOOL_MSG_STRSET_GET,
 	ETHTOOL_MSG_SETTINGS_GET,
+	ETHTOOL_MSG_SETTINGS_SET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -27,6 +28,7 @@ enum {
 	ETHTOOL_MSG_KERNEL_NONE,
 	ETHTOOL_MSG_STRSET_GET_REPLY,
 	ETHTOOL_MSG_SETTINGS_GET_REPLY,
+	ETHTOOL_MSG_SETTINGS_NTF,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index ed53e07d619e..504ab2f7009c 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -26,6 +26,7 @@
 #include <net/devlink.h>
 #include <net/xdp_sock.h>
 #include <net/flow_offload.h>
+#include <linux/ethtool_netlink.h>
 
 #include "common.h"
 
@@ -565,7 +566,13 @@ static int ethtool_set_link_ksettings(struct net_device *dev,
 	    != link_ksettings.base.link_mode_masks_nwords)
 		return -EINVAL;
 
-	return dev->ethtool_ops->set_link_ksettings(dev, &link_ksettings);
+	err = dev->ethtool_ops->set_link_ksettings(dev, &link_ksettings);
+	if (err >= 0)
+		ethtool_notify(dev, NULL, ETHTOOL_MSG_SETTINGS_NTF,
+			       ETHTOOL_IM_SETTINGS_LINKINFO |
+			       ETHTOOL_IM_SETTINGS_LINKMODES,
+			       NULL);
+	return err;
 }
 
 /* Query device for its ethtool_cmd settings.
@@ -614,6 +621,7 @@ static int ethtool_set_settings(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_link_ksettings link_ksettings;
 	struct ethtool_cmd cmd;
+	int ret;
 
 	ASSERT_RTNL();
 
@@ -626,7 +634,13 @@ static int ethtool_set_settings(struct net_device *dev, void __user *useraddr)
 		return -EINVAL;
 	link_ksettings.base.link_mode_masks_nwords =
 		__ETHTOOL_LINK_MODE_MASK_NU32;
-	return dev->ethtool_ops->set_link_ksettings(dev, &link_ksettings);
+	ret = dev->ethtool_ops->set_link_ksettings(dev, &link_ksettings);
+	if (ret >= 0)
+		ethtool_notify(dev, NULL, ETHTOOL_MSG_SETTINGS_NTF,
+			       ETHTOOL_IM_SETTINGS_LINKINFO |
+			       ETHTOOL_IM_SETTINGS_LINKMODES,
+			       NULL);
+	return ret;
 }
 
 static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 9ff17ef05023..69b6dfe2a1c8 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -612,6 +612,11 @@ static int ethnl_get_done(struct netlink_callback *cb)
 
 static const struct get_request_ops *ethnl_std_notify_to_ops(unsigned int cmd)
 {
+	switch (cmd) {
+	case ETHTOOL_MSG_SETTINGS_NTF:
+		return &settings_request_ops;
+	};
+
 	WARN_ONCE(1, "unexpected notification type %u\n", cmd);
 	return NULL;
 }
@@ -679,6 +684,7 @@ typedef void (*ethnl_notify_handler_t)(struct net_device *dev,
 				       const void *data);
 
 static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
+	[ETHTOOL_MSG_SETTINGS_NTF]	= ethnl_std_notify,
 };
 
 void ethtool_notify(struct net_device *dev, struct netlink_ext_ack *extack,
@@ -714,6 +720,11 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.dumpit	= ethnl_get_dumpit,
 		.done	= ethnl_get_done,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_SETTINGS_SET,
+		.flags	= GENL_UNS_ADMIN_PERM,
+		.doit	= ethnl_set_settings,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 6512d9d508bf..43fdf11cfc6d 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -297,4 +297,6 @@ struct get_request_ops {
 extern const struct get_request_ops strset_request_ops;
 extern const struct get_request_ops settings_request_ops;
 
+int ethnl_set_settings(struct sk_buff *skb, struct genl_info *info);
+
 #endif /* _NET_ETHTOOL_NETLINK_H */
diff --git a/net/ethtool/settings.c b/net/ethtool/settings.c
index 11ec30b9d48b..2fc961297076 100644
--- a/net/ethtool/settings.c
+++ b/net/ethtool/settings.c
@@ -14,6 +14,99 @@ struct settings_data {
 	bool				lpm_empty;
 };
 
+struct link_mode_info {
+	int				speed;
+	u8				duplex;
+};
+
+#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex) \
+	[ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \
+		.speed	= SPEED_ ## _speed, \
+		.duplex	= __DUPLEX_ ## _duplex \
+	}
+#define __DUPLEX_Half DUPLEX_HALF
+#define __DUPLEX_Full DUPLEX_FULL
+#define __DEFINE_SPECIAL_MODE_PARAMS(_mode) \
+	[ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = { \
+		.speed	= SPEED_UNKNOWN, \
+		.duplex	= DUPLEX_UNKNOWN, \
+	}
+
+static const struct link_mode_info link_mode_params[] = {
+	__DEFINE_LINK_MODE_PARAMS(10, T, Half),
+	__DEFINE_LINK_MODE_PARAMS(10, T, Full),
+	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
+	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
+	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
+	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(Autoneg),
+	__DEFINE_SPECIAL_MODE_PARAMS(TP),
+	__DEFINE_SPECIAL_MODE_PARAMS(AUI),
+	__DEFINE_SPECIAL_MODE_PARAMS(MII),
+	__DEFINE_SPECIAL_MODE_PARAMS(FIBRE),
+	__DEFINE_SPECIAL_MODE_PARAMS(BNC),
+	__DEFINE_LINK_MODE_PARAMS(10000, T, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(Pause),
+	__DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
+	__DEFINE_LINK_MODE_PARAMS(2500, X, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(Backplane),
+	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
+	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
+		.speed	= SPEED_10000,
+		.duplex = DUPLEX_FULL,
+	},
+	__DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full),
+	__DEFINE_LINK_MODE_PARAMS(20000, KR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(40000, KR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(40000, CR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(40000, SR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(40000, LR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(56000, KR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(56000, CR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(56000, SR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(56000, LR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(25000, CR, Full),
+	__DEFINE_LINK_MODE_PARAMS(25000, KR, Full),
+	__DEFINE_LINK_MODE_PARAMS(25000, SR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, CR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, KR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, KR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, SR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, CR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, SR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(1000, X, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, CR, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, SR, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, LR, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, LRM, Full),
+	__DEFINE_LINK_MODE_PARAMS(10000, ER, Full),
+	__DEFINE_LINK_MODE_PARAMS(2500, T, Full),
+	__DEFINE_LINK_MODE_PARAMS(5000, T, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(FEC_NONE),
+	__DEFINE_SPECIAL_MODE_PARAMS(FEC_RS),
+	__DEFINE_SPECIAL_MODE_PARAMS(FEC_BASER),
+	__DEFINE_LINK_MODE_PARAMS(50000, KR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, SR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, CR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, Full),
+	__DEFINE_LINK_MODE_PARAMS(50000, DR, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, KR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, SR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, CR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(100000, DR2, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, KR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, SR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, DR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(200000, CR4, Full),
+	__DEFINE_LINK_MODE_PARAMS(100, T1, Full),
+	__DEFINE_LINK_MODE_PARAMS(1000, T1, Full),
+};
+
 static const struct nla_policy
 settings_get_policy[ETHTOOL_A_SETTINGS_MAX + 1] = {
 	[ETHTOOL_A_SETTINGS_UNSPEC]	= { .type = NLA_REJECT },
@@ -257,3 +350,242 @@ const struct get_request_ops settings_request_ops = {
 	.reply_size		= settings_size,
 	.fill_reply		= settings_fill,
 };
+
+/* SET_SETTINGS */
+
+static const struct nla_policy settings_hdr_policy[ETHTOOL_A_HEADER_MAX + 1] = {
+	[ETHTOOL_A_HEADER_UNSPEC]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_HEADER_DEV_INDEX]		= { .type = NLA_U32 },
+	[ETHTOOL_A_HEADER_DEV_NAME]		= { .type = NLA_NUL_STRING,
+						    .len = IFNAMSIZ - 1 },
+	[ETHTOOL_A_HEADER_INFOMASK]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_HEADER_GFLAGS]		= { .type = NLA_U32 },
+	[ETHTOOL_A_HEADER_RFLAGS]		= { .type = NLA_REJECT },
+};
+
+static const struct nla_policy
+linkinfo_set_policy[ETHTOOL_A_LINKINFO_MAX + 1] = {
+	[ETHTOOL_A_LINKINFO_UNSPEC]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_LINKINFO_PORT]		= { .type = NLA_U8 },
+	[ETHTOOL_A_LINKINFO_PHYADDR]		= { .type = NLA_U8 },
+	[ETHTOOL_A_LINKINFO_TP_MDIX]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL]	= { .type = NLA_U8 },
+	[ETHTOOL_A_LINKINFO_TRANSCEIVER]	= { .type = NLA_REJECT },
+};
+
+static const struct nla_policy
+linkmodes_set_policy[ETHTOOL_A_LINKMODES_MAX + 1] = {
+	[ETHTOOL_A_LINKMODES_UNSPEC]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_LINKMODES_AUTONEG]		= { .type = NLA_U8 },
+	[ETHTOOL_A_LINKMODES_OURS]		= { .type = NLA_NESTED },
+	[ETHTOOL_A_LINKMODES_PEER]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_LINKMODES_SPEED]		= { .type = NLA_U32 },
+	[ETHTOOL_A_LINKMODES_DUPLEX]		= { .type = NLA_U8 },
+};
+
+static const struct nla_policy
+settings_set_policy[ETHTOOL_A_SETTINGS_MAX + 1] = {
+	[ETHTOOL_A_SETTINGS_UNSPEC]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_SETTINGS_HEADER]		= { .type = NLA_NESTED },
+	[ETHTOOL_A_SETTINGS_LINK_INFO]		= { .type = NLA_NESTED },
+	[ETHTOOL_A_SETTINGS_LINK_MODES]		= { .type = NLA_NESTED },
+};
+
+static int ethnl_set_link_ksettings(struct genl_info *info,
+				    struct net_device *dev,
+				    struct ethtool_link_ksettings *ksettings)
+{
+	int ret = dev->ethtool_ops->set_link_ksettings(dev, ksettings);
+
+	if (ret < 0)
+		GENL_SET_ERR_MSG(info, "link settings update failed");
+	return ret;
+}
+
+/* Set advertised link modes to all supported modes matching requested speed
+ * and duplex values. Called when autonegotiation is on, speed or duplex is
+ * requested but no link mode change. This is done in userspace with ioctl()
+ * interface, move it into kernel for netlink.
+ * Returns true if advertised modes bitmap was modified.
+ */
+static bool settings_auto_linkmodes(struct ethtool_link_ksettings *ksettings,
+				    bool req_speed, bool req_duplex)
+{
+	unsigned long *advertising = ksettings->link_modes.advertising;
+	unsigned long *supported = ksettings->link_modes.supported;
+	DECLARE_BITMAP(old_adv, __ETHTOOL_LINK_MODE_MASK_NBITS);
+	unsigned int i;
+
+	BUILD_BUG_ON(ARRAY_SIZE(link_mode_params) !=
+		     __ETHTOOL_LINK_MODE_MASK_NBITS);
+
+	bitmap_copy(old_adv, advertising, __ETHTOOL_LINK_MODE_MASK_NBITS);
+
+	for (i = 0; i < __ETHTOOL_LINK_MODE_MASK_NBITS; i++) {
+		const struct link_mode_info *info = &link_mode_params[i];
+
+		if (info->speed == SPEED_UNKNOWN)
+			continue;
+		if (test_bit(i, supported) &&
+		    (!req_speed || info->speed == ksettings->base.speed) &&
+		    (!req_duplex || info->duplex == ksettings->base.duplex))
+			set_bit(i, advertising);
+		else
+			clear_bit(i, advertising);
+	}
+
+	return !bitmap_equal(old_adv, advertising,
+			     __ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+
+static int settings_update_linkinfo(struct genl_info *info, struct nlattr *nest,
+				    struct ethtool_link_settings *lsettings)
+{
+	struct nlattr *tb[ETHTOOL_A_LINKINFO_MAX + 1];
+	int ret;
+
+	if (!nest)
+		return 0;
+	ret = nla_parse_nested(tb, ETHTOOL_A_LINKINFO_MAX, nest,
+			       linkinfo_set_policy, info->extack);
+	if (ret < 0)
+		return ret;
+
+	ret = 0;
+	if (ethnl_update_u8(&lsettings->port, tb[ETHTOOL_A_LINKINFO_PORT]))
+		ret = 1;
+	if (ethnl_update_u8(&lsettings->phy_address,
+			    tb[ETHTOOL_A_LINKINFO_PHYADDR]))
+		ret = 1;
+	if (ethnl_update_u8(&lsettings->eth_tp_mdix_ctrl,
+			    tb[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL]))
+		ret = 1;
+
+	return ret;
+}
+
+static int settings_update_linkmodes(struct genl_info *info,
+				     const struct nlattr *nest,
+				     struct ethtool_link_ksettings *ksettings)
+{
+	struct ethtool_link_settings *lsettings = &ksettings->base;
+	struct nlattr *tb[ETHTOOL_A_LINKMODES_MAX + 1];
+	bool req_speed, req_duplex;
+	bool mod = false;
+	int ret;
+
+	if (!nest)
+		return 0;
+	ret = nla_parse_nested(tb, ETHTOOL_A_LINKMODES_MAX, nest,
+			       linkmodes_set_policy, info->extack);
+	if (ret < 0)
+		return ret;
+	req_speed = tb[ETHTOOL_A_LINKMODES_SPEED];
+	req_duplex = tb[ETHTOOL_A_LINKMODES_DUPLEX];
+
+	if (ethnl_update_u8(&lsettings->autoneg,
+			    tb[ETHTOOL_A_LINKMODES_AUTONEG]))
+		mod = true;
+	if (ethnl_update_bitset(ksettings->link_modes.advertising, NULL,
+				__ETHTOOL_LINK_MODE_MASK_NBITS,
+				tb[ETHTOOL_A_LINKMODES_OURS],
+				&ret, link_mode_names, false, info))
+		mod = true;
+	if (ret < 0)
+		return ret;
+	if (ethnl_update_u32(&lsettings->speed, tb[ETHTOOL_A_LINKMODES_SPEED]))
+		mod = true;
+	if (ethnl_update_u8(&lsettings->duplex, tb[ETHTOOL_A_LINKMODES_DUPLEX]))
+		mod = true;
+
+	if (!tb[ETHTOOL_A_LINKMODES_OURS] && lsettings->autoneg &&
+	    (req_speed || req_duplex) &&
+	    settings_auto_linkmodes(ksettings, req_speed, req_duplex))
+		mod = true;
+
+	return mod;
+}
+
+/* Update device settings using ->set_link_ksettings() callback */
+static int ethnl_update_ksettings(struct genl_info *info, struct nlattr **tb,
+				  struct net_device *dev, u32 *req_mask)
+{
+	struct ethtool_link_ksettings ksettings = {};
+	struct ethtool_link_settings *lsettings;
+	u32 mod_mask = 0;
+	int ret;
+
+	ret = ethnl_get_link_ksettings(info, dev, &ksettings);
+	if (ret < 0)
+		return ret;
+	lsettings = &ksettings.base;
+
+	ret = settings_update_linkinfo(info, tb[ETHTOOL_A_SETTINGS_LINK_INFO],
+				       lsettings);
+	if (ret < 0)
+		return ret;
+	if (ret)
+		mod_mask |= ETHTOOL_IM_SETTINGS_LINKINFO;
+
+	ret = settings_update_linkmodes(info, tb[ETHTOOL_A_SETTINGS_LINK_MODES],
+					&ksettings);
+	if (ret < 0)
+		return ret;
+	if (ret)
+		mod_mask |= ETHTOOL_IM_SETTINGS_LINKMODES;
+
+	if (mod_mask) {
+		ret = ethnl_set_link_ksettings(info, dev, &ksettings);
+		if (ret < 0)
+			return ret;
+		*req_mask |= mod_mask;
+	}
+
+	return 0;
+}
+
+int ethnl_set_settings(struct sk_buff *skb, struct genl_info *info)
+{
+	struct nlattr *tb[ETHTOOL_A_SETTINGS_MAX + 1];
+	struct ethnl_req_info req_info = {};
+	struct net_device *dev;
+	u32 req_mask = 0;
+	int ret;
+
+	ret = nlmsg_parse(info->nlhdr, GENL_HDRLEN, tb,
+			  ETHTOOL_A_SETTINGS_MAX, settings_set_policy,
+			  info->extack);
+	if (ret < 0)
+		return ret;
+	ret = ethnl_parse_header(&req_info, tb[ETHTOOL_A_SETTINGS_HEADER],
+				 genl_info_net(info), info->extack,
+				 settings_hdr_policy, true);
+	if (ret < 0)
+		return ret;
+	dev = req_info.dev;
+
+	rtnl_lock();
+	ret = ethnl_before_ops(dev);
+	if (ret < 0)
+		goto out_rtnl;
+	if (tb[ETHTOOL_A_SETTINGS_LINK_INFO] ||
+	    tb[ETHTOOL_A_SETTINGS_LINK_MODES]) {
+		ret = -EOPNOTSUPP;
+		if (!dev->ethtool_ops->get_link_ksettings)
+			goto out_ops;
+		ret = ethnl_update_ksettings(info, tb, dev, &req_mask);
+		if (ret < 0)
+			goto out_ops;
+	}
+	ret = 0;
+
+out_ops:
+	if (req_mask)
+		ethtool_notify(dev, NULL, ETHTOOL_MSG_SETTINGS_NTF, req_mask,
+			       NULL);
+	ethnl_after_ops(dev);
+out_rtnl:
+	rtnl_unlock();
+	dev_put(dev);
+	return ret;
+}
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 13/15] ethtool: add standard notification handler
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

The ethtool netlink notifications have the same format as related GET
replies so that if generic GET handling framework is used to process GET
requests, its callbacks and instance of struct get_request_ops can be
also used to compose corresponding notification message.

Provide function ethnl_std_notify() to be used as notification handler in
ethnl_notify_handlers table.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 net/ethtool/netlink.c | 74 +++++++++++++++++++++++++++++++++++++++++++
 net/ethtool/netlink.h |  3 +-
 2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 6c0cfa9001a1..9ff17ef05023 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -7,6 +7,7 @@
 static struct genl_family ethtool_genl_family;
 
 static bool ethnl_ok __read_mostly;
+static u32 ethnl_bcast_seq;
 
 #define __LINK_MODE_NAME(speed, type, duplex) \
 	#speed "base" #type "/" #duplex
@@ -258,6 +259,18 @@ struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
 	return NULL;
 }
 
+static void *ethnl_bcastmsg_put(struct sk_buff *skb, u8 cmd)
+{
+	return genlmsg_put(skb, 0, ++ethnl_bcast_seq, &ethtool_genl_family, 0,
+			   cmd);
+}
+
+static int ethnl_multicast(struct sk_buff *skb, struct net_device *dev)
+{
+	return genlmsg_multicast_netns(&ethtool_genl_family, dev_net(dev), skb,
+				       0, ETHNL_MCGRP_MONITOR, GFP_KERNEL);
+}
+
 /* GET request helpers */
 
 /**
@@ -597,6 +610,67 @@ static int ethnl_get_done(struct netlink_callback *cb)
 	return 0;
 }
 
+static const struct get_request_ops *ethnl_std_notify_to_ops(unsigned int cmd)
+{
+	WARN_ONCE(1, "unexpected notification type %u\n", cmd);
+	return NULL;
+}
+
+/* generic notification handler */
+static void ethnl_std_notify(struct net_device *dev,
+			     struct netlink_ext_ack *extack, unsigned int cmd,
+			     u32 req_mask, const void *data)
+{
+	const struct get_request_ops *ops;
+	struct ethnl_req_info *req_info;
+	struct sk_buff *skb;
+	void *reply_payload;
+	int reply_len;
+	int ret;
+
+	ops = ethnl_std_notify_to_ops(cmd);
+	if (!ops)
+		return;
+
+	req_info = ethnl_alloc_get_data(ops);
+	if (!req_info)
+		return;
+	req_info->dev = dev;
+	req_info->req_mask = req_mask;
+	req_info->global_flags |= ETHTOOL_RF_COMPACT;
+
+	ethnl_init_reply_data(req_info, ops, dev);
+	ret = ops->prepare_data(req_info, NULL);
+	if (ret < 0)
+		goto err_data;
+	reply_len = ops->reply_size(req_info);
+	if (reply_len < 0)
+		goto err_data;
+	skb = genlmsg_new(reply_len, GFP_KERNEL);
+	if (!skb)
+		goto err_data;
+	reply_payload = ethnl_bcastmsg_put(skb, cmd);
+	if (!reply_payload)
+		goto err_skb;
+
+	ret = ethnl_fill_reply_header(skb, dev, ops->hdr_attr);
+	if (ret < 0)
+		goto err_skb;
+	ret = ops->fill_reply(skb, req_info);
+	if (ret < 0)
+		goto err_skb;
+	ethnl_free_get_data(ops, req_info);
+	genlmsg_end(skb, reply_payload);
+
+	ethnl_multicast(skb, dev);
+	return;
+
+err_skb:
+	nlmsg_free(skb);
+err_data:
+	ethnl_free_get_data(ops, req_info);
+}
+
 /* notifications */
 
 typedef void (*ethnl_notify_handler_t)(struct net_device *dev,
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 27832a3956c8..6512d9d508bf 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -265,7 +265,8 @@ static inline void ethnl_after_ops(struct net_device *dev)
  * infrastructure. When used, a pointer to an instance of this structure is to
  * be added to &get_requests array and generic handlers ethnl_get_doit(),
  * ethnl_get_dumpit(), ethnl_get_start() and ethnl_get_done() used in
- * @ethnl_genl_ops
+ * @ethnl_genl_ops; ethnl_std_notify() can be used in @ethnl_notify_handlers
+ * to send notifications of the corresponding type.
  */
 struct get_request_ops {
 	u8			request_cmd;
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 12/15] ethtool: provide link settings and link modes in SETTINGS_GET request
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Implement SETTINGS_GET netlink request to get link settings and link mode
information provided by ETHTOOL_GLINKSETTINGS and ETHTOOL_GSET ioctl
commands.

The information in SETTINGS_GET_REPLY message sent as reply is divided into
two parts and client can use info mask in request header to select only
one of them:

  - ETHTOOL_IM_SETTINGS_LINKINFO: physical port, phy MDIO address, MDI(-X)
    status, MDI(-X) control and transceiver
  - ETHTOOL_IM_SETTINGS_LINKMODES: supported and advertised link modes,
    autonegotiation state, link speed and duplex

SETTINGS_GET request can be used with NLM_F_DUMP (and without device
identification) to request the information for all devices in current
network namespace providing the data.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.txt |  48 +++-
 include/linux/ethtool_netlink.h              |   3 +
 include/uapi/linux/ethtool_netlink.h         |  49 ++++
 net/ethtool/Makefile                         |   2 +-
 net/ethtool/common.c                         |  48 ++++
 net/ethtool/common.h                         |   4 +
 net/ethtool/ioctl.c                          |  48 ----
 net/ethtool/netlink.c                        |   8 +
 net/ethtool/netlink.h                        |   1 +
 net/ethtool/settings.c                       | 259 +++++++++++++++++++
 10 files changed, 419 insertions(+), 51 deletions(-)
 create mode 100644 net/ethtool/settings.c

diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
index 21e5030734aa..1d803488e02c 100644
--- a/Documentation/networking/ethtool-netlink.txt
+++ b/Documentation/networking/ethtool-netlink.txt
@@ -151,10 +151,12 @@ according to message purpose:
 Userspace to kernel:
 
     ETHTOOL_MSG_STRSET_GET		get string set
+    ETHTOOL_MSG_SETTINGS_GET		get device settings
 
 Kernel to userspace:
 
     ETHTOOL_MSG_STRSET_GET_REPLY	string set contents
+    ETHTOOL_MSG_SETTINGS_GET_REPLY	device settings
 
 "GET" requests are sent by userspace applications to retrieve device
 information. They usually do not contain any message specific attributes.
@@ -231,6 +233,48 @@ Flag ETHTOOL_A_STRSET_COUNTS tells kernel to only return string counts of the
 sets, not the actual strings.
 
 
+SETTINGS_GET
+------------
+
+SETTINGS_GET request retrieves information provided by ETHTOOL_GLINKSETTINGS,
+ETHTOOL_GWOL, ETHTOOL_GMSGLVL and ETHTOOL_GLINK ioctl requests. The request
+doesn't use any attributes.
+
+Request attributes:
+
+    ETHTOOL_A_SETTINGS_HEADER		(nested)	request header
+
+Info mask bits meaning:
+
+    ETHTOOL_IM_SETTINGS_LINKINFO	link settings
+    ETHTOOL_IM_SETTINGS_LINKMODES	link modes and related
+
+Response contents:
+
+    ETHTOOL_A_SETTINGS_HEADER		(nested)	reply header
+    ETHTOOL_A_SETTINGS_LINK_INFO	(nested)	link settings
+        ETHTOOL_A_LINKINFO_PORT		    (u8)	    physical port
+        ETHTOOL_A_LINKINFO_PHYADDR	    (u8)	    phy MDIO address
+        ETHTOOL_A_LINKINFO_TP_MDIX	    (u8)	    MDI(-X) status
+        ETHTOOL_A_LINKINFO_TP_MDIX_CTRL	    (u8)	    MDI(-X) control
+        ETHTOOL_A_LINKINFO_TRANSCEIVER	    (u8)	    transceiver
+    ETHTOOL_A_SETTINGS_LINK_MODES	(nested)	link modes
+        ETHTOOL_A_LINKMODES_AUTONEG	    (u8)	    autonegotiation status
+        ETHTOOL_A_LINKMODES_OURS	    (bitset)	    advertised link modes
+        ETHTOOL_A_LINKMODES_PEER	    (bitset)	    partner link modes
+        ETHTOOL_A_LINKMODES_SPEED	    (u32)	    link speed (Mb/s)
+        ETHTOOL_A_LINKMODES_DUPLEX	    (u8)	    duplex mode
+
+Most of the attributes and their values have the same meaning as matching
+members of the corresponding ioctl structures. For ETHTOOL_A_LINKMODES_OURS,
+value represents advertised modes and mask represents supported modes.
+ETHTOOL_A_LINKMODES_PEER in the reply is a bit list.
+
+SETTINGS_GET requests allow dumps and messages in the same format as response
+to them are broadcasted as notifications on change of these settings using
+netlink or ioctl ethtool interface.
+
+
 Request translation
 -------------------
 
@@ -240,7 +284,7 @@ have their netlink replacement yet.
 
 ioctl command			netlink command
 ---------------------------------------------------------------------
-ETHTOOL_GSET			n/a
+ETHTOOL_GSET			ETHTOOL_MSG_SETTINGS_GET
 ETHTOOL_SSET			n/a
 ETHTOOL_GDRVINFO		n/a
 ETHTOOL_GREGS			n/a
@@ -314,7 +358,7 @@ ETHTOOL_GTUNABLE		n/a
 ETHTOOL_STUNABLE		n/a
 ETHTOOL_GPHYSTATS		n/a
 ETHTOOL_PERQUEUE		n/a
-ETHTOOL_GLINKSETTINGS		n/a
+ETHTOOL_GLINKSETTINGS		ETHTOOL_MSG_SETTINGS_GET
 ETHTOOL_SLINKSETTINGS		n/a
 ETHTOOL_PHY_GTUNABLE		n/a
 ETHTOOL_PHY_STUNABLE		n/a
diff --git a/include/linux/ethtool_netlink.h b/include/linux/ethtool_netlink.h
index 2a15e64a16f3..e770e6e9acca 100644
--- a/include/linux/ethtool_netlink.h
+++ b/include/linux/ethtool_netlink.h
@@ -7,6 +7,9 @@
 #include <linux/ethtool.h>
 #include <linux/netdevice.h>
 
+#define __ETHTOOL_LINK_MODE_MASK_NWORDS \
+	DIV_ROUND_UP(__ETHTOOL_LINK_MODE_MASK_NBITS, 32)
+
 enum ethtool_multicast_groups {
 	ETHNL_MCGRP_MONITOR,
 };
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 11b8519d2c1d..a046dd8da50e 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -15,6 +15,7 @@
 enum {
 	ETHTOOL_MSG_USER_NONE,
 	ETHTOOL_MSG_STRSET_GET,
+	ETHTOOL_MSG_SETTINGS_GET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -25,6 +26,7 @@ enum {
 enum {
 	ETHTOOL_MSG_KERNEL_NONE,
 	ETHTOOL_MSG_STRSET_GET_REPLY,
+	ETHTOOL_MSG_SETTINGS_GET_REPLY,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
@@ -147,6 +149,53 @@ enum {
 
 #define ETHTOOL_RF_STRSET_ALL (ETHTOOL_RF_STRSET_COUNTS)
 
+/* SETTINGS */
+
+enum {
+	ETHTOOL_A_SETTINGS_UNSPEC,
+	ETHTOOL_A_SETTINGS_HEADER,		/* nest - _A_HEADER_* */
+	ETHTOOL_A_SETTINGS_LINK_INFO,		/* nest - _A_LINKINFO_* */
+	ETHTOOL_A_SETTINGS_LINK_MODES,		/* nest - _A_LINKMODES_* */
+
+	/* add new constants above here */
+	__ETHTOOL_A_SETTINGS_CNT,
+	ETHTOOL_A_SETTINGS_MAX = (__ETHTOOL_A_SETTINGS_CNT - 1)
+};
+
+#define ETHTOOL_IM_SETTINGS_LINKINFO		(1U << 0)
+#define ETHTOOL_IM_SETTINGS_LINKMODES		(1U << 1)
+
+#define ETHTOOL_IM_SETTINGS_ALL (ETHTOOL_IM_SETTINGS_LINKINFO | \
+				 ETHTOOL_IM_SETTINGS_LINKMODES)
+
+#define ETHTOOL_RF_SETTINGS_ALL 0
+
+enum {
+	ETHTOOL_A_LINKINFO_UNSPEC,
+	ETHTOOL_A_LINKINFO_PORT,		/* u8 */
+	ETHTOOL_A_LINKINFO_PHYADDR,		/* u8 */
+	ETHTOOL_A_LINKINFO_TP_MDIX,		/* u8 */
+	ETHTOOL_A_LINKINFO_TP_MDIX_CTRL,	/* u8 */
+	ETHTOOL_A_LINKINFO_TRANSCEIVER,		/* u8 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_LINKINFO_CNT,
+	ETHTOOL_A_LINKINFO_MAX = (__ETHTOOL_A_LINKINFO_CNT - 1)
+};
+
+enum {
+	ETHTOOL_A_LINKMODES_UNSPEC,
+	ETHTOOL_A_LINKMODES_AUTONEG,		/* u8 */
+	ETHTOOL_A_LINKMODES_OURS,		/* bitset */
+	ETHTOOL_A_LINKMODES_PEER,		/* bitset */
+	ETHTOOL_A_LINKMODES_SPEED,		/* u32 */
+	ETHTOOL_A_LINKMODES_DUPLEX,		/* u8 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_LINKMODES_CNT,
+	ETHTOOL_A_LINKMODES_MAX = (__ETHTOOL_A_LINKMODES_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 11ceb00821b3..1155e5e9ef69 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -4,4 +4,4 @@ obj-y				+= ioctl.o common.o
 
 obj-$(CONFIG_ETHTOOL_NETLINK)	+= ethtool_nl.o
 
-ethtool_nl-y	:= netlink.o bitset.o strset.o
+ethtool_nl-y	:= netlink.o bitset.o strset.o settings.o
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index b0ce420e994e..abb00b3a7e77 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -82,3 +82,51 @@ phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
 	[ETHTOOL_PHY_DOWNSHIFT]	= "phy-downshift",
 	[ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down",
 };
+
+/* return false if legacy contained non-0 deprecated fields
+ * maxtxpkt/maxrxpkt. rest of ksettings always updated
+ */
+bool
+convert_legacy_settings_to_link_ksettings(
+	struct ethtool_link_ksettings *link_ksettings,
+	const struct ethtool_cmd *legacy_settings)
+{
+	bool retval = true;
+
+	memset(link_ksettings, 0, sizeof(*link_ksettings));
+
+	/* This is used to tell users that driver is still using these
+	 * deprecated legacy fields, and they should not use
+	 * %ETHTOOL_GLINKSETTINGS/%ETHTOOL_SLINKSETTINGS
+	 */
+	if (legacy_settings->maxtxpkt ||
+	    legacy_settings->maxrxpkt)
+		retval = false;
+
+	ethtool_convert_legacy_u32_to_link_mode(
+		link_ksettings->link_modes.supported,
+		legacy_settings->supported);
+	ethtool_convert_legacy_u32_to_link_mode(
+		link_ksettings->link_modes.advertising,
+		legacy_settings->advertising);
+	ethtool_convert_legacy_u32_to_link_mode(
+		link_ksettings->link_modes.lp_advertising,
+		legacy_settings->lp_advertising);
+	link_ksettings->base.speed
+		= ethtool_cmd_speed(legacy_settings);
+	link_ksettings->base.duplex
+		= legacy_settings->duplex;
+	link_ksettings->base.port
+		= legacy_settings->port;
+	link_ksettings->base.phy_address
+		= legacy_settings->phy_address;
+	link_ksettings->base.autoneg
+		= legacy_settings->autoneg;
+	link_ksettings->base.mdio_support
+		= legacy_settings->mdio_support;
+	link_ksettings->base.eth_tp_mdix
+		= legacy_settings->eth_tp_mdix;
+	link_ksettings->base.eth_tp_mdix_ctrl
+		= legacy_settings->eth_tp_mdix_ctrl;
+	return retval;
+}
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index 41b2efc1e4e1..0381936d8e1e 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -14,4 +14,8 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN];
 extern const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
 
+bool convert_legacy_settings_to_link_ksettings(
+	struct ethtool_link_ksettings *link_ksettings,
+	const struct ethtool_cmd *legacy_settings);
+
 #endif /* _ETHTOOL_COMMON_H */
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index b35366dd9997..ed53e07d619e 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -352,54 +352,6 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
 }
 EXPORT_SYMBOL(ethtool_convert_link_mode_to_legacy_u32);
 
-/* return false if legacy contained non-0 deprecated fields
- * maxtxpkt/maxrxpkt. rest of ksettings always updated
- */
-static bool
-convert_legacy_settings_to_link_ksettings(
-	struct ethtool_link_ksettings *link_ksettings,
-	const struct ethtool_cmd *legacy_settings)
-{
-	bool retval = true;
-
-	memset(link_ksettings, 0, sizeof(*link_ksettings));
-
-	/* This is used to tell users that driver is still using these
-	 * deprecated legacy fields, and they should not use
-	 * %ETHTOOL_GLINKSETTINGS/%ETHTOOL_SLINKSETTINGS
-	 */
-	if (legacy_settings->maxtxpkt ||
-	    legacy_settings->maxrxpkt)
-		retval = false;
-
-	ethtool_convert_legacy_u32_to_link_mode(
-		link_ksettings->link_modes.supported,
-		legacy_settings->supported);
-	ethtool_convert_legacy_u32_to_link_mode(
-		link_ksettings->link_modes.advertising,
-		legacy_settings->advertising);
-	ethtool_convert_legacy_u32_to_link_mode(
-		link_ksettings->link_modes.lp_advertising,
-		legacy_settings->lp_advertising);
-	link_ksettings->base.speed
-		= ethtool_cmd_speed(legacy_settings);
-	link_ksettings->base.duplex
-		= legacy_settings->duplex;
-	link_ksettings->base.port
-		= legacy_settings->port;
-	link_ksettings->base.phy_address
-		= legacy_settings->phy_address;
-	link_ksettings->base.autoneg
-		= legacy_settings->autoneg;
-	link_ksettings->base.mdio_support
-		= legacy_settings->mdio_support;
-	link_ksettings->base.eth_tp_mdix
-		= legacy_settings->eth_tp_mdix;
-	link_ksettings->base.eth_tp_mdix_ctrl
-		= legacy_settings->eth_tp_mdix_ctrl;
-	return retval;
-}
-
 /* return false if ksettings link modes had higher bits
  * set. legacy_settings always updated (best effort)
  */
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index cba1f2259248..6c0cfa9001a1 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -280,6 +280,7 @@ struct ethnl_dump_ctx {
 
 static const struct get_request_ops *get_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_STRSET_GET]	= &strset_request_ops,
+	[ETHTOOL_MSG_SETTINGS_GET]	= &settings_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -632,6 +633,13 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.dumpit	= ethnl_get_dumpit,
 		.done	= ethnl_get_done,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_SETTINGS_GET,
+		.doit	= ethnl_get_doit,
+		.start	= ethnl_get_start,
+		.dumpit	= ethnl_get_dumpit,
+		.done	= ethnl_get_done,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index d85b1edc1b91..27832a3956c8 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -294,5 +294,6 @@ struct get_request_ops {
 /* request handlers */
 
 extern const struct get_request_ops strset_request_ops;
+extern const struct get_request_ops settings_request_ops;
 
 #endif /* _NET_ETHTOOL_NETLINK_H */
diff --git a/net/ethtool/settings.c b/net/ethtool/settings.c
new file mode 100644
index 000000000000..11ec30b9d48b
--- /dev/null
+++ b/net/ethtool/settings.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
+
+#include "netlink.h"
+#include "common.h"
+#include "bitset.h"
+
+struct settings_data {
+	struct ethnl_req_info		reqinfo_base;
+
+	/* everything below here will be reset for each device in dumps */
+	struct ethnl_reply_data		repdata_base;
+	struct ethtool_link_ksettings	ksettings;
+	struct ethtool_link_settings	*lsettings;
+	bool				lpm_empty;
+};
+
+static const struct nla_policy
+settings_get_policy[ETHTOOL_A_SETTINGS_MAX + 1] = {
+	[ETHTOOL_A_SETTINGS_UNSPEC]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_SETTINGS_HEADER]	= { .type = NLA_NESTED },
+	[ETHTOOL_A_SETTINGS_LINK_INFO]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_SETTINGS_LINK_MODES]	= { .type = NLA_REJECT },
+};
+
+static int ethnl_get_link_ksettings(struct genl_info *info,
+				    struct net_device *dev,
+				    struct ethtool_link_ksettings *ksettings)
+{
+	int ret;
+
+	ret = __ethtool_get_link_ksettings(dev, ksettings);
+
+	if (ret < 0 && info)
+		GENL_SET_ERR_MSG(info, "failed to retrieve link settings");
+	return ret;
+}
+
+/* prepare_data() handler */
+static int settings_prepare(struct ethnl_req_info *req_info,
+			    struct genl_info *info)
+{
+	struct settings_data *data =
+		container_of(req_info, struct settings_data, reqinfo_base);
+	struct net_device *dev = data->repdata_base.dev;
+	u32 req_mask = req_info->req_mask;
+	int ret;
+
+	data->lsettings = &data->ksettings.base;
+	data->lpm_empty = true;
+
+	ret = ethnl_before_ops(dev);
+	if (ret < 0)
+		return ret;
+	if (req_mask &
+	    (ETHTOOL_IM_SETTINGS_LINKINFO | ETHTOOL_IM_SETTINGS_LINKMODES)) {
+		ret = ethnl_get_link_ksettings(info, dev, &data->ksettings);
+		if (ret < 0)
+			req_mask &= ~(ETHTOOL_IM_SETTINGS_LINKINFO |
+				      ETHTOOL_IM_SETTINGS_LINKMODES);
+	}
+	if (req_mask & ETHTOOL_IM_SETTINGS_LINKMODES) {
+		data->lpm_empty =
+			bitmap_empty(data->ksettings.link_modes.lp_advertising,
+				     __ETHTOOL_LINK_MODE_MASK_NBITS);
+		ethnl_bitmap_to_u32(data->ksettings.link_modes.supported,
+				    __ETHTOOL_LINK_MODE_MASK_NWORDS);
+		ethnl_bitmap_to_u32(data->ksettings.link_modes.advertising,
+				    __ETHTOOL_LINK_MODE_MASK_NWORDS);
+		ethnl_bitmap_to_u32(data->ksettings.link_modes.lp_advertising,
+				    __ETHTOOL_LINK_MODE_MASK_NWORDS);
+	}
+	ethnl_after_ops(dev);
+
+	data->repdata_base.info_mask = req_mask;
+	if (req_info->req_mask & ~req_mask && info)
+		GENL_SET_ERR_MSG(info,
+				 "not all requested data could be retrieved");
+	return 0;
+}
+
+static int settings_linkinfo_size(void)
+{
+	int len = 0;
+
+	/* port, phyaddr, mdix, mdixctrl, transcvr */
+	len += 5 * nla_total_size(sizeof(u8));
+	/* mdio_support */
+	len += nla_total_size(sizeof(struct nla_bitfield32));
+
+	/* nest */
+	return nla_total_size(len);
+}
+
+static int
+settings_linkmodes_size(const struct ethtool_link_ksettings *ksettings,
+			bool compact)
+{
+	unsigned int flags = compact ? ETHNL_BITSET_COMPACT : 0;
+	u32 *supported = (u32 *)ksettings->link_modes.supported;
+	u32 *advertising = (u32 *)ksettings->link_modes.advertising;
+	u32 *lp_advertising = (u32 *)ksettings->link_modes.lp_advertising;
+	int len = 0, ret;
+
+	/* speed, duplex, autoneg */
+	len += nla_total_size(sizeof(u32)) + 2 * nla_total_size(sizeof(u8));
+	ret = ethnl_bitset32_size(__ETHTOOL_LINK_MODE_MASK_NBITS, advertising,
+				  supported, link_mode_names, flags);
+	if (ret < 0)
+		return ret;
+	len += ret;
+	ret = ethnl_bitset32_size(__ETHTOOL_LINK_MODE_MASK_NBITS,
+				  lp_advertising, NULL, link_mode_names,
+				  flags & ETHNL_BITSET_LIST);
+	if (ret < 0)
+		return ret;
+	len += ret;
+
+	/* nest */
+	return nla_total_size(len);
+}
+
+/* reply_size() handler
+ *
+ * To keep things simple, reserve space for some attributes which may not
+ * be added to the message (e.g. ETHTOOL_A_SETTINGS_SOPASS); therefore the
+ * length returned may be bigger than the actual length of the message sent.
+ */
+static int settings_size(const struct ethnl_req_info *req_info)
+{
+	struct settings_data *data =
+		container_of(req_info, struct settings_data, reqinfo_base);
+	u32 info_mask = data->repdata_base.info_mask;
+	bool compact = req_info->global_flags & ETHTOOL_RF_COMPACT;
+	int len = 0, ret;
+
+	len += ethnl_reply_header_size();
+	if (info_mask & ETHTOOL_IM_SETTINGS_LINKINFO)
+		len += settings_linkinfo_size();
+	if (info_mask & ETHTOOL_IM_SETTINGS_LINKMODES) {
+		ret = settings_linkmodes_size(&data->ksettings, compact);
+		if (ret < 0)
+			return ret;
+		len += ret;
+	}
+
+	return len;
+}
+
+static int settings_fill_linkinfo(struct sk_buff *skb,
+				  const struct ethtool_link_settings *lsettings)
+{
+	struct nlattr *nest;
+
+	nest = nla_nest_start(skb, ETHTOOL_A_SETTINGS_LINK_INFO);
+	if (!nest)
+		return -EMSGSIZE;
+
+	if (nla_put_u8(skb, ETHTOOL_A_LINKINFO_PORT, lsettings->port) ||
+	    nla_put_u8(skb, ETHTOOL_A_LINKINFO_PHYADDR,
+		       lsettings->phy_address) ||
+	    nla_put_u8(skb, ETHTOOL_A_LINKINFO_TP_MDIX,
+		       lsettings->eth_tp_mdix) ||
+	    nla_put_u8(skb, ETHTOOL_A_LINKINFO_TP_MDIX_CTRL,
+		       lsettings->eth_tp_mdix_ctrl) ||
+	    nla_put_u8(skb, ETHTOOL_A_LINKINFO_TRANSCEIVER,
+		       lsettings->transceiver)) {
+		nla_nest_cancel(skb, nest);
+		return -EMSGSIZE;
+	}
+
+	nla_nest_end(skb, nest);
+	return 0;
+}
+
+static int
+settings_fill_linkmodes(struct sk_buff *skb,
+			const struct ethtool_link_ksettings *ksettings,
+			bool lpm_empty, bool compact)
+{
+	const u32 *supported = (const u32 *)ksettings->link_modes.supported;
+	const u32 *advertising = (const u32 *)ksettings->link_modes.advertising;
+	const u32 *lp_adv = (const u32 *)ksettings->link_modes.lp_advertising;
+	const unsigned int flags = compact ? ETHNL_BITSET_COMPACT : 0;
+	const struct ethtool_link_settings *lsettings = &ksettings->base;
+	struct nlattr *nest;
+	int ret;
+
+	nest = nla_nest_start(skb, ETHTOOL_A_SETTINGS_LINK_MODES);
+	if (!nest)
+		return -EMSGSIZE;
+	if (nla_put_u8(skb, ETHTOOL_A_LINKMODES_AUTONEG, lsettings->autoneg))
+		goto nla_put_failure;
+
+	ret = ethnl_put_bitset32(skb, ETHTOOL_A_LINKMODES_OURS,
+				 __ETHTOOL_LINK_MODE_MASK_NBITS, advertising,
+				 supported, link_mode_names, flags);
+	if (ret < 0)
+		goto nla_put_failure;
+	if (!lpm_empty) {
+		ret = ethnl_put_bitset32(skb, ETHTOOL_A_LINKMODES_PEER,
+					 __ETHTOOL_LINK_MODE_MASK_NBITS,
+					 lp_adv, NULL, link_mode_names,
+					 flags | ETHNL_BITSET_LIST);
+		if (ret < 0)
+			goto nla_put_failure;
+	}
+
+	if (nla_put_u32(skb, ETHTOOL_A_LINKMODES_SPEED, lsettings->speed) ||
+	    nla_put_u8(skb, ETHTOOL_A_LINKMODES_DUPLEX, lsettings->duplex))
+		goto nla_put_failure;
+
+	nla_nest_end(skb, nest);
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -EMSGSIZE;
+}
+
+/* fill_reply() handler */
+static int settings_fill(struct sk_buff *skb,
+			 const struct ethnl_req_info *req_info)
+{
+	const struct settings_data *data =
+		container_of(req_info, struct settings_data, reqinfo_base);
+	u32 info_mask = data->repdata_base.info_mask;
+	bool compact = req_info->global_flags & ETHTOOL_RF_COMPACT;
+	int ret;
+
+	if (info_mask & ETHTOOL_IM_SETTINGS_LINKINFO) {
+		ret = settings_fill_linkinfo(skb, data->lsettings);
+		if (ret < 0)
+			return ret;
+	}
+	if (info_mask & ETHTOOL_IM_SETTINGS_LINKMODES) {
+		ret = settings_fill_linkmodes(skb, &data->ksettings,
+					      data->lpm_empty, compact);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+const struct get_request_ops settings_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_SETTINGS_GET,
+	.reply_cmd		= ETHTOOL_MSG_SETTINGS_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_SETTINGS_HEADER,
+	.max_attr		= ETHTOOL_A_SETTINGS_MAX,
+	.data_size		= sizeof(struct settings_data),
+	.repdata_offset		= offsetof(struct settings_data, repdata_base),
+	.request_policy		= settings_get_policy,
+	.default_infomask	= ETHTOOL_IM_SETTINGS_ALL,
+	.all_reqflags		= ETHTOOL_RF_SETTINGS_ALL,
+
+	.prepare_data		= settings_prepare,
+	.reply_size		= settings_size,
+	.fill_reply		= settings_fill,
+};
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 11/15] ethtool: provide link mode names as a string set
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Unlike e.g. netdev features, the ethtool ioctl interface requires link mode
table to be in sync between kernel and userspace for userspace to be able
to display and set all link modes supported by kernel. The way arbitrary
length bitsets are implemented in netlink interface, this is no longer
needed.

To allow userspace to access all link modes running kernel supports, add
table of ethernet link mode names and make it available as a string set to
userspace GET_STRSET requests. Add build time check to make sure names
are defined for all modes declared in enum ethtool_link_mode_bit_indices.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 include/linux/ethtool.h      |  4 ++
 include/uapi/linux/ethtool.h |  2 +
 net/ethtool/netlink.c        | 83 ++++++++++++++++++++++++++++++++++++
 net/ethtool/netlink.h        |  2 +
 net/ethtool/strset.c         |  6 +++
 5 files changed, 97 insertions(+)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 95991e4300bf..5caef65d93d6 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -102,6 +102,10 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
 #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name)		\
 	DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
 
+/* compose link mode index from speed, type and duplex */
+#define ETHTOOL_LINK_MODE(speed, type, duplex) \
+	ETHTOOL_LINK_MODE_ ## speed ## base ## type ## _ ## duplex ## _BIT
+
 /* drivers must ignore base.cmd and base.link_mode_masks_nwords
  * fields, but they are allowed to overwrite them (will be ignored).
  */
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 4e4e28e77c7a..6ad298224352 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -571,6 +571,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
  * @ETH_SS_PHY_TUNABLES: PHY tunable names
+ * @ETH_SS_LINK_MODES: link mode names
  */
 enum ethtool_stringset {
 	ETH_SS_TEST		= 0,
@@ -582,6 +583,7 @@ enum ethtool_stringset {
 	ETH_SS_TUNABLES,
 	ETH_SS_PHY_STATS,
 	ETH_SS_PHY_TUNABLES,
+	ETH_SS_LINK_MODES,
 
 	ETH_SS_COUNT
 };
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 41d7fedd3dd6..cba1f2259248 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -8,6 +8,86 @@ static struct genl_family ethtool_genl_family;
 
 static bool ethnl_ok __read_mostly;
 
+#define __LINK_MODE_NAME(speed, type, duplex) \
+	#speed "base" #type "/" #duplex
+#define __DEFINE_LINK_MODE_NAME(speed, type, duplex) \
+	[ETHTOOL_LINK_MODE(speed, type, duplex)] = \
+	__LINK_MODE_NAME(speed, type, duplex)
+#define __DEFINE_SPECIAL_MODE_NAME(_mode, _name) \
+	[ETHTOOL_LINK_MODE_ ## _mode ## _BIT] = _name
+
+const char *const link_mode_names[] = {
+	__DEFINE_LINK_MODE_NAME(10, T, Half),
+	__DEFINE_LINK_MODE_NAME(10, T, Full),
+	__DEFINE_LINK_MODE_NAME(100, T, Half),
+	__DEFINE_LINK_MODE_NAME(100, T, Full),
+	__DEFINE_LINK_MODE_NAME(1000, T, Half),
+	__DEFINE_LINK_MODE_NAME(1000, T, Full),
+	__DEFINE_SPECIAL_MODE_NAME(Autoneg, "Autoneg"),
+	__DEFINE_SPECIAL_MODE_NAME(TP, "TP"),
+	__DEFINE_SPECIAL_MODE_NAME(AUI, "AUI"),
+	__DEFINE_SPECIAL_MODE_NAME(MII, "MII"),
+	__DEFINE_SPECIAL_MODE_NAME(FIBRE, "FIBRE"),
+	__DEFINE_SPECIAL_MODE_NAME(BNC, "BNC"),
+	__DEFINE_LINK_MODE_NAME(10000, T, Full),
+	__DEFINE_SPECIAL_MODE_NAME(Pause, "Pause"),
+	__DEFINE_SPECIAL_MODE_NAME(Asym_Pause, "Asym_Pause"),
+	__DEFINE_LINK_MODE_NAME(2500, X, Full),
+	__DEFINE_SPECIAL_MODE_NAME(Backplane, "Backplane"),
+	__DEFINE_LINK_MODE_NAME(1000, KX, Full),
+	__DEFINE_LINK_MODE_NAME(10000, KX4, Full),
+	__DEFINE_LINK_MODE_NAME(10000, KR, Full),
+	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = "10000baseR_FEC",
+	__DEFINE_LINK_MODE_NAME(20000, MLD2, Full),
+	__DEFINE_LINK_MODE_NAME(20000, KR2, Full),
+	__DEFINE_LINK_MODE_NAME(40000, KR4, Full),
+	__DEFINE_LINK_MODE_NAME(40000, CR4, Full),
+	__DEFINE_LINK_MODE_NAME(40000, SR4, Full),
+	__DEFINE_LINK_MODE_NAME(40000, LR4, Full),
+	__DEFINE_LINK_MODE_NAME(56000, KR4, Full),
+	__DEFINE_LINK_MODE_NAME(56000, CR4, Full),
+	__DEFINE_LINK_MODE_NAME(56000, SR4, Full),
+	__DEFINE_LINK_MODE_NAME(56000, LR4, Full),
+	__DEFINE_LINK_MODE_NAME(25000, CR, Full),
+	__DEFINE_LINK_MODE_NAME(25000, KR, Full),
+	__DEFINE_LINK_MODE_NAME(25000, SR, Full),
+	__DEFINE_LINK_MODE_NAME(50000, CR2, Full),
+	__DEFINE_LINK_MODE_NAME(50000, KR2, Full),
+	__DEFINE_LINK_MODE_NAME(100000, KR4, Full),
+	__DEFINE_LINK_MODE_NAME(100000, SR4, Full),
+	__DEFINE_LINK_MODE_NAME(100000, CR4, Full),
+	__DEFINE_LINK_MODE_NAME(100000, LR4_ER4, Full),
+	__DEFINE_LINK_MODE_NAME(50000, SR2, Full),
+	__DEFINE_LINK_MODE_NAME(1000, X, Full),
+	__DEFINE_LINK_MODE_NAME(10000, CR, Full),
+	__DEFINE_LINK_MODE_NAME(10000, SR, Full),
+	__DEFINE_LINK_MODE_NAME(10000, LR, Full),
+	__DEFINE_LINK_MODE_NAME(10000, LRM, Full),
+	__DEFINE_LINK_MODE_NAME(10000, ER, Full),
+	__DEFINE_LINK_MODE_NAME(2500, T, Full),
+	__DEFINE_LINK_MODE_NAME(5000, T, Full),
+	__DEFINE_SPECIAL_MODE_NAME(FEC_NONE, "None"),
+	__DEFINE_SPECIAL_MODE_NAME(FEC_RS, "RS"),
+	__DEFINE_SPECIAL_MODE_NAME(FEC_BASER, "BASER"),
+	__DEFINE_LINK_MODE_NAME(50000, KR, Full),
+	__DEFINE_LINK_MODE_NAME(50000, SR, Full),
+	__DEFINE_LINK_MODE_NAME(50000, CR, Full),
+	__DEFINE_LINK_MODE_NAME(50000, LR_ER_FR, Full),
+	__DEFINE_LINK_MODE_NAME(50000, DR, Full),
+	__DEFINE_LINK_MODE_NAME(100000, KR2, Full),
+	__DEFINE_LINK_MODE_NAME(100000, SR2, Full),
+	__DEFINE_LINK_MODE_NAME(100000, CR2, Full),
+	__DEFINE_LINK_MODE_NAME(100000, LR2_ER2_FR2, Full),
+	__DEFINE_LINK_MODE_NAME(100000, DR2, Full),
+	__DEFINE_LINK_MODE_NAME(200000, KR4, Full),
+	__DEFINE_LINK_MODE_NAME(200000, SR4, Full),
+	__DEFINE_LINK_MODE_NAME(200000, LR4_ER4_FR4, Full),
+	__DEFINE_LINK_MODE_NAME(200000, DR4, Full),
+	__DEFINE_LINK_MODE_NAME(200000, CR4, Full),
+	__DEFINE_LINK_MODE_NAME(100, T1, Full),
+	__DEFINE_LINK_MODE_NAME(1000, T1, Full),
+};
+
 static const struct nla_policy dflt_header_policy[ETHTOOL_A_HEADER_MAX + 1] = {
 	[ETHTOOL_A_HEADER_UNSPEC]	= { .type = NLA_REJECT },
 	[ETHTOOL_A_HEADER_DEV_INDEX]	= { .type = NLA_U32 },
@@ -575,6 +655,9 @@ static int __init ethnl_init(void)
 {
 	int ret;
 
+	BUILD_BUG_ON(ARRAY_SIZE(link_mode_names) !=
+		     __ETHTOOL_LINK_MODE_MASK_NBITS);
+
 	ret = genl_register_family(&ethtool_genl_family);
 	if (WARN(ret < 0, "ethtool: genetlink family registration failed"))
 		return ret;
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 2352fd9c17c3..d85b1edc1b91 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -10,6 +10,8 @@
 
 struct ethnl_req_info;
 
+extern const char *const link_mode_names[];
+
 int ethnl_parse_header(struct ethnl_req_info *req_info,
 		       const struct nlattr *nest, struct net *net,
 		       struct netlink_ext_ack *extack,
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index fd7229379158..514ef04709d3 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -67,6 +67,12 @@ static const struct strset_info info_template[] = {
 		.count		= ARRAY_SIZE(phy_tunable_strings),
 		.data		= { .legacy = phy_tunable_strings },
 	},
+	[ETH_SS_LINK_MODES] = {
+		.type		= ETH_SS_TYPE_SIMPLE,
+		.per_dev	= false,
+		.count		= __ETHTOOL_LINK_MODE_MASK_NBITS,
+		.data		= { .simple = link_mode_names },
+	},
 };
 
 struct strset_data {
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 10/15] ethtool: provide string sets with STRSET_GET request
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Requests a contents of one or more string sets, i.e. indexed arrays of
strings; this information is provided by ETHTOOL_GSSET_INFO and
ETHTOOL_GSTRINGS commands of ioctl interface. Unlike ioctl interface, all
information can be retrieved with one request and mulitple string sets can
be requested at once.

There are three types of requests:

  - no NLM_F_DUMP, no device: get "global" stringsets
  - no NLM_F_DUMP, with device: get string sets related to the device
  - NLM_F_DUMP, no device: get device related string sets for all devices

Client can request either all string sets of given type (global or device
related) or only specific sets. With ETHTOOL_A_STRSET_COUNTS flag set, only
set sizes (numbers of strings) are returned.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.txt |  57 ++-
 include/uapi/linux/ethtool.h                 |   2 +
 include/uapi/linux/ethtool_netlink.h         |  60 +++
 net/ethtool/Makefile                         |   2 +-
 net/ethtool/netlink.c                        |   8 +
 net/ethtool/netlink.h                        |   4 +
 net/ethtool/strset.c                         | 453 +++++++++++++++++++
 7 files changed, 583 insertions(+), 3 deletions(-)
 create mode 100644 net/ethtool/strset.c

diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
index 4636682c551f..21e5030734aa 100644
--- a/Documentation/networking/ethtool-netlink.txt
+++ b/Documentation/networking/ethtool-netlink.txt
@@ -148,6 +148,14 @@ according to message purpose:
     _ACT_REPLY  kernel reply to an ACT request
     _NTF	kernel notification
 
+Userspace to kernel:
+
+    ETHTOOL_MSG_STRSET_GET		get string set
+
+Kernel to userspace:
+
+    ETHTOOL_MSG_STRSET_GET_REPLY	string set contents
+
 "GET" requests are sent by userspace applications to retrieve device
 information. They usually do not contain any message specific attributes.
 Kernel replies with corresponding "GET_REPLY" message. For most types, "GET"
@@ -178,6 +186,51 @@ action also triggers a notification ("NTF" message).
 Later sections describe the format and semantics of these messages.
 
 
+STRSET_GET
+----------
+
+Requests contents of a string set as provided by ioctl commands
+ETHTOOL_GSSET_INFO and ETHTOOL_GSTRINGS. String sets are not user writeable so
+that the corresponding SET_STRSET message is only used in kernel replies.
+There are two types of string sets: global (independent of a device, e.g.
+device feature names) and device specific (e.g. device private flags).
+
+Request contents:
+
+    ETHTOOL_A_STRSET_HEADER		(nested)	request header
+    ETHTOOL_A_STRSET_STRINGSETS		(nested)	string set to request
+      ETHTOOL_A_STRINGSETS_STRINGSET+     (nested)	  one string set
+        ETHTOOL_A_STRINGSET_ID		    (u32)	    set id
+
+Request specific flag:
+
+    ETHTOOL_RF_STRSET_COUNTS		send only string counts in reply
+
+Kernel response contents:
+
+    ETHTOOL_A_STRSET_HEADER		(nested)	reply header
+    ETHTOOL_A_STRSET_STRINGSETS		(nested)	array of string sets
+      ETHTOOL_A_STRINGSETS_STRINGSET+     (nested)	  one string set
+        ETHTOOL_A_STRINGSET_ID		    (u32)	    set id
+        ETHTOOL_A_STRINGSET_COUNT	    (u32)	    number of strings
+        ETHTOOL_A_STRINGSET_STRINGS	    (nested)	    array of strings
+          ETHTOOL_A_STRINGS_STRING+           (nested)        one string
+            ETHTOOL_A_STRING_INDEX		(u32)		string index
+            ETHTOOL_A_STRING_VALUE		(string)	string value
+
+Device identification in request header is optional. Depending on its presence
+a and NLM_F_DUMP flag, there are three type of STRSET_GET requests:
+
+ - no NLM_F_DUMP, no device: get "global" stringsets
+ - no NLM_F_DUMP, with device: get string sets related to the device
+ - NLM_F_DUMP, no device: get device related string sets for all devices
+
+If there is no ETHTOOL_A_STRSET_STRINGSETS array, all string sets of requested
+type are returned, otherwise only those specified in the request.
+Flag ETHTOOL_A_STRSET_COUNTS tells kernel to only return string counts of the
+sets, not the actual strings.
+
+
 Request translation
 -------------------
 
@@ -212,7 +265,7 @@ ETHTOOL_STXCSUM			n/a
 ETHTOOL_GSG			n/a
 ETHTOOL_SSG			n/a
 ETHTOOL_TEST			n/a
-ETHTOOL_GSTRINGS		n/a
+ETHTOOL_GSTRINGS		ETHTOOL_MSG_STRSET_GET
 ETHTOOL_PHYS_ID			n/a
 ETHTOOL_GSTATS			n/a
 ETHTOOL_GTSO			n/a
@@ -240,7 +293,7 @@ ETHTOOL_FLASHDEV		n/a
 ETHTOOL_RESET			n/a
 ETHTOOL_SRXNTUPLE		n/a
 ETHTOOL_GRXNTUPLE		n/a
-ETHTOOL_GSSET_INFO		n/a
+ETHTOOL_GSSET_INFO		ETHTOOL_MSG_STRSET_GET
 ETHTOOL_GRXFHINDIR		n/a
 ETHTOOL_SRXFHINDIR		n/a
 ETHTOOL_GFEATURES		n/a
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index dd06302aa93e..4e4e28e77c7a 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -582,6 +582,8 @@ enum ethtool_stringset {
 	ETH_SS_TUNABLES,
 	ETH_SS_PHY_STATS,
 	ETH_SS_PHY_TUNABLES,
+
+	ETH_SS_COUNT
 };
 
 /**
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 8938a1f09057..11b8519d2c1d 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -14,6 +14,7 @@
 /* message types - userspace to kernel */
 enum {
 	ETHTOOL_MSG_USER_NONE,
+	ETHTOOL_MSG_STRSET_GET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -23,6 +24,7 @@ enum {
 /* message types - kernel to userspace */
 enum {
 	ETHTOOL_MSG_KERNEL_NONE,
+	ETHTOOL_MSG_STRSET_GET_REPLY,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
@@ -87,6 +89,64 @@ enum {
 	ETHTOOL_A_BITSET_MAX = (__ETHTOOL_A_BITSET_CNT - 1)
 };
 
+/* string sets */
+
+enum {
+	ETHTOOL_A_STRING_UNSPEC,
+	ETHTOOL_A_STRING_INDEX,			/* u32 */
+	ETHTOOL_A_STRING_VALUE,			/* string */
+
+	/* add new constants above here */
+	__ETHTOOL_A_STRING_CNT,
+	ETHTOOL_A_STRING_MAX = (__ETHTOOL_A_STRING_CNT - 1)
+};
+
+enum {
+	ETHTOOL_A_STRINGS_UNSPEC,
+	ETHTOOL_A_STRINGS_STRING,		/* nest - _A_STRINGS_* */
+
+	/* add new constants above here */
+	__ETHTOOL_A_STRINGS_CNT,
+	ETHTOOL_A_STRINGS_MAX = (__ETHTOOL_A_STRINGS_CNT - 1)
+};
+
+enum {
+	ETHTOOL_A_STRINGSET_UNSPEC,
+	ETHTOOL_A_STRINGSET_ID,			/* u32 */
+	ETHTOOL_A_STRINGSET_COUNT,		/* u32 */
+	ETHTOOL_A_STRINGSET_STRINGS,		/* nest - _A_STRINGS_* */
+
+	/* add new constants above here */
+	__ETHTOOL_A_STRINGSET_CNT,
+	ETHTOOL_A_STRINGSET_MAX = (__ETHTOOL_A_STRINGSET_CNT - 1)
+};
+
+/* STRSET */
+
+enum {
+	ETHTOOL_A_STRSET_UNSPEC,
+	ETHTOOL_A_STRSET_HEADER,		/* nest - _A_HEADER_* */
+	ETHTOOL_A_STRSET_STRINGSETS,		/* nest - _A_STRINGSETS_* */
+
+	/* add new constants above here */
+	__ETHTOOL_A_STRSET_CNT,
+	ETHTOOL_A_STRSET_MAX = (__ETHTOOL_A_STRSET_CNT - 1)
+};
+
+enum {
+	ETHTOOL_A_STRINGSETS_UNSPEC,
+	ETHTOOL_A_STRINGSETS_STRINGSET,		/* nest - _A_STRINGSET_* */
+
+	/* add new constants above here */
+	__ETHTOOL_A_STRINGSETS_CNT,
+	ETHTOOL_A_STRINGSETS_MAX = (__ETHTOOL_A_STRINGSETS_CNT - 1)
+};
+
+/* return only string counts, not the strings */
+#define ETHTOOL_RF_STRSET_COUNTS		(1 << 0)
+
+#define ETHTOOL_RF_STRSET_ALL (ETHTOOL_RF_STRSET_COUNTS)
+
 /* generic netlink info */
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 11782306593b..11ceb00821b3 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -4,4 +4,4 @@ obj-y				+= ioctl.o common.o
 
 obj-$(CONFIG_ETHTOOL_NETLINK)	+= ethtool_nl.o
 
-ethtool_nl-y	:= netlink.o bitset.o
+ethtool_nl-y	:= netlink.o bitset.o strset.o
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 6d326cc25aac..41d7fedd3dd6 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -199,6 +199,7 @@ struct ethnl_dump_ctx {
 };
 
 static const struct get_request_ops *get_requests[__ETHTOOL_MSG_USER_CNT] = {
+	[ETHTOOL_MSG_STRSET_GET]	= &strset_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -544,6 +545,13 @@ EXPORT_SYMBOL(ethtool_notify);
 /* genetlink setup */
 
 static const struct genl_ops ethtool_genl_ops[] = {
+	{
+		.cmd	= ETHTOOL_MSG_STRSET_GET,
+		.doit	= ethnl_get_doit,
+		.start	= ethnl_get_start,
+		.dumpit	= ethnl_get_dumpit,
+		.done	= ethnl_get_done,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 6a9695c3b0c6..2352fd9c17c3 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -289,4 +289,8 @@ struct get_request_ops {
 	void (*cleanup)(struct ethnl_req_info *req_info);
 };
 
+/* request handlers */
+
+extern const struct get_request_ops strset_request_ops;
+
 #endif /* _NET_ETHTOOL_NETLINK_H */
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
new file mode 100644
index 000000000000..fd7229379158
--- /dev/null
+++ b/net/ethtool/strset.c
@@ -0,0 +1,453 @@
+// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
+
+#include <linux/ethtool.h>
+#include <linux/phy.h>
+#include "netlink.h"
+#include "common.h"
+
+enum strset_type {
+	ETH_SS_TYPE_NONE,
+	ETH_SS_TYPE_LEGACY,
+	ETH_SS_TYPE_SIMPLE,
+};
+
+struct strset_info {
+	enum strset_type type;
+	bool per_dev;
+	bool free_data;
+	unsigned int count;
+	union {
+		const char (*legacy)[ETH_GSTRING_LEN];
+		const char * const *simple;
+		void *ptr;
+	} data;
+};
+
+static const struct strset_info info_template[] = {
+	[ETH_SS_TEST] = {
+		.type		= ETH_SS_TYPE_LEGACY,
+		.per_dev	= true,
+	},
+	[ETH_SS_STATS] = {
+		.type		= ETH_SS_TYPE_LEGACY,
+		.per_dev	= true,
+	},
+	[ETH_SS_PRIV_FLAGS] = {
+		.type		= ETH_SS_TYPE_LEGACY,
+		.per_dev	= true,
+	},
+	[ETH_SS_NTUPLE_FILTERS] = {
+		.type		= ETH_SS_TYPE_NONE,
+	},
+	[ETH_SS_FEATURES] = {
+		.type		= ETH_SS_TYPE_LEGACY,
+		.per_dev	= false,
+		.count		= ARRAY_SIZE(netdev_features_strings),
+		.data		= { .legacy = netdev_features_strings },
+	},
+	[ETH_SS_RSS_HASH_FUNCS] = {
+		.type		= ETH_SS_TYPE_LEGACY,
+		.per_dev	= false,
+		.count		= ARRAY_SIZE(rss_hash_func_strings),
+		.data		= { .legacy = rss_hash_func_strings },
+	},
+	[ETH_SS_TUNABLES] = {
+		.type		= ETH_SS_TYPE_LEGACY,
+		.per_dev	= false,
+		.count		= ARRAY_SIZE(tunable_strings),
+		.data		= { .legacy = tunable_strings },
+	},
+	[ETH_SS_PHY_STATS] = {
+		.type		= ETH_SS_TYPE_LEGACY,
+		.per_dev	= true,
+	},
+	[ETH_SS_PHY_TUNABLES] = {
+		.type		= ETH_SS_TYPE_LEGACY,
+		.per_dev	= false,
+		.count		= ARRAY_SIZE(phy_tunable_strings),
+		.data		= { .legacy = phy_tunable_strings },
+	},
+};
+
+struct strset_data {
+	struct ethnl_req_info		reqinfo_base;
+	u32				req_ids;
+	bool				counts_only;
+
+	/* everything below here will be reset for each device in dumps */
+	struct ethnl_reply_data		repdata_base;
+	struct strset_info		info[ETH_SS_COUNT];
+};
+
+static const struct nla_policy strset_get_policy[ETHTOOL_A_STRSET_MAX + 1] = {
+	[ETHTOOL_A_STRSET_UNSPEC]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_STRSET_HEADER]	= { .type = NLA_NESTED },
+	[ETHTOOL_A_STRSET_STRINGSETS]	= { .type = NLA_NESTED },
+};
+
+static const struct nla_policy
+get_stringset_policy[ETHTOOL_A_STRINGSET_MAX + 1] = {
+	[ETHTOOL_A_STRINGSET_UNSPEC]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_STRINGSET_ID]	= { .type = NLA_U32 },
+	[ETHTOOL_A_STRINGSET_COUNT]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_STRINGSET_STRINGS]	= { .type = NLA_REJECT },
+};
+
+/**
+ * strset_include() - test if a string set should be included in reply
+ * @data: pointer to request data structure
+ * @id:   id of string set to check (ETH_SS_* constants)
+ */
+static bool strset_include(const struct strset_data *data, u32 id)
+{
+	bool per_dev;
+
+	BUILD_BUG_ON(ETH_SS_COUNT >= BITS_PER_BYTE * sizeof(data->req_ids));
+
+	if (data->req_ids)
+		return data->req_ids & (1U << id);
+	per_dev = data->info[id].per_dev;
+	if (data->info[id].type == ETH_SS_TYPE_NONE)
+		return false;
+
+	return data->repdata_base.dev ? per_dev : !per_dev;
+}
+
+static int strset_get_id(const struct nlattr *nest, u32 *val,
+			 struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[ETHTOOL_A_STRINGSET_MAX + 1];
+	int ret;
+
+	ret = nla_parse_nested(tb, ETHTOOL_A_STRINGSET_MAX, nest,
+			       get_stringset_policy, extack);
+	if (ret < 0)
+		return ret;
+	if (!tb[ETHTOOL_A_STRINGSET_ID])
+		return -EINVAL;
+
+	*val = nla_get_u32(tb[ETHTOOL_A_STRINGSET_ID]);
+	return 0;
+}
+
+static const struct nla_policy strset_hdr_policy[ETHTOOL_A_HEADER_MAX + 1] = {
+	[ETHTOOL_A_HEADER_UNSPEC]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_HEADER_DEV_INDEX]	= { .type = NLA_U32 },
+	[ETHTOOL_A_HEADER_DEV_NAME]	= { .type = NLA_NUL_STRING,
+					    .len = IFNAMSIZ - 1 },
+	[ETHTOOL_A_HEADER_INFOMASK]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_HEADER_GFLAGS]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_HEADER_RFLAGS]	= { .type = NLA_U32 },
+};
+
+static const struct nla_policy
+strset_stringsets_policy[ETHTOOL_A_STRINGSETS_MAX + 1] = {
+	[ETHTOOL_A_STRINGSETS_UNSPEC]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_STRINGSETS_STRINGSET]	= { .type = NLA_NESTED },
+};
+
+/* parse_request() handler */
+static int strset_parse(struct ethnl_req_info *req_info,
+			struct nlattr **tb, struct netlink_ext_ack *extack)
+{
+	struct strset_data *data =
+		container_of(req_info, struct strset_data, reqinfo_base);
+	struct nlattr *nest = tb[ETHTOOL_A_STRSET_STRINGSETS];
+	struct nlattr *attr;
+	int rem, ret;
+
+	if (!nest)
+		return 0;
+	ret = nla_validate_nested(nest, ETHTOOL_A_STRINGSETS_MAX,
+				  strset_stringsets_policy, extack);
+	if (ret < 0)
+		return ret;
+
+	nla_for_each_nested(attr, nest, rem) {
+		u32 id;
+
+		if (WARN_ONCE(nla_type(attr) != ETHTOOL_A_STRINGSETS_STRINGSET,
+			      "unexpected attrtype %u in ETHTOOL_A_STRSET_STRINGSETS\n",
+			      nla_type(attr)))
+			return -EINVAL;
+
+		ret = strset_get_id(attr, &id, extack);
+		if (ret < 0)
+			return ret;
+		if (ret >= ETH_SS_COUNT) {
+			NL_SET_ERR_MSG_ATTR(extack, attr,
+					    "unknown string set id");
+			return -EOPNOTSUPP;
+		}
+
+		data->req_ids |= (1U << id);
+	}
+
+	return 0;
+}
+
+/* cleanup() handler - free allocated data (if any) */
+static void strset_cleanup(struct ethnl_req_info *req_info)
+{
+	struct strset_data *data =
+		container_of(req_info, struct strset_data, reqinfo_base);
+	unsigned int i;
+
+	for (i = 0; i < ETH_SS_COUNT; i++)
+		if (data->info[i].free_data) {
+			kfree(data->info[i].data.ptr);
+			data->info[i].data.ptr = NULL;
+			data->info[i].free_data = false;
+		}
+}
+
+static int strset_prepare_set(struct strset_info *info, struct net_device *dev,
+			      unsigned int id, bool counts_only)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	void *strings;
+	int count, ret;
+
+	if (id == ETH_SS_PHY_STATS && dev->phydev &&
+	    !ops->get_ethtool_phy_stats)
+		ret = phy_ethtool_get_sset_count(dev->phydev);
+	else if (ops->get_sset_count && ops->get_strings)
+		ret = ops->get_sset_count(dev, id);
+	else
+		ret = -EOPNOTSUPP;
+	if (ret <= 0) {
+		info->count = 0;
+		return 0;
+	}
+
+	count = ret;
+	if (!counts_only) {
+		strings = kcalloc(count, ETH_GSTRING_LEN, GFP_KERNEL);
+		if (!strings)
+			return -ENOMEM;
+		if (id == ETH_SS_PHY_STATS && dev->phydev &&
+		    !ops->get_ethtool_phy_stats)
+			phy_ethtool_get_strings(dev->phydev, strings);
+		else
+			ops->get_strings(dev, id, strings);
+		info->data.legacy = strings;
+		info->free_data = true;
+	}
+	info->count = count;
+
+	return 0;
+}
+
+/* prepare_data() handler */
+static int strset_prepare(struct ethnl_req_info *req_info,
+			  struct genl_info *info)
+{
+	struct strset_data *data =
+		container_of(req_info, struct strset_data, reqinfo_base);
+	struct net_device *dev = data->repdata_base.dev;
+	unsigned int i;
+	int ret;
+
+	BUILD_BUG_ON(ARRAY_SIZE(info_template) != ETH_SS_COUNT);
+	memcpy(&data->info, &info_template, sizeof(data->info));
+
+	if (!dev) {
+		for (i = 0; i < ETH_SS_COUNT; i++) {
+			if ((data->req_ids & (1U << i)) &&
+			    data->info[i].per_dev) {
+				if (info)
+					GENL_SET_ERR_MSG(info, "requested per device strings without dev");
+				return -EINVAL;
+			}
+		}
+	}
+
+	ret = ethnl_before_ops(dev);
+	if (ret < 0)
+		goto err_strset;
+	for (i = 0; i < ETH_SS_COUNT; i++) {
+		if (!strset_include(data, i) || !data->info[i].per_dev)
+			continue;
+		if (WARN_ONCE(data->info[i].type != ETH_SS_TYPE_LEGACY,
+			      "unexpected string set type %u",
+			      data->info[i].type))
+			goto err_ops;
+
+		ret = strset_prepare_set(&data->info[i], dev, i,
+					 data->counts_only);
+		if (ret < 0)
+			goto err_ops;
+	}
+	ethnl_after_ops(dev);
+
+	return 0;
+err_ops:
+	ethnl_after_ops(dev);
+err_strset:
+	strset_cleanup(req_info);
+	return ret;
+}
+
+/* calculate size of ETHTOOL_A_STRSET_STRINGSET nest for one string set */
+static int strset_set_size(const struct strset_info *info, bool counts_only)
+{
+	unsigned int len = 0;
+	unsigned int i;
+
+	if (info->count == 0)
+		return 0;
+	if (counts_only)
+		return nla_total_size(2 * nla_total_size(sizeof(u32)));
+
+	for (i = 0; i < info->count; i++) {
+		const char *str;
+
+		if (info->type == ETH_SS_TYPE_LEGACY)
+			str = info->data.legacy[i];
+		else
+			str = info->data.simple[i];
+
+		/* ETHTOOL_A_STRING_INDEX, ETHTOOL_A_STRING_VALUE, nest */
+		len += nla_total_size(nla_total_size(sizeof(u32)) +
+				      ethnl_str_size(str));
+	}
+	/* ETHTOOL_A_STRINGSET_ID, ETHTOOL_A_STRINGSET_COUNT */
+	len = 2 * nla_total_size(sizeof(u32)) + nla_total_size(len);
+
+	return nla_total_size(len);
+}
+
+/* reply_size() handler */
+static int strset_size(const struct ethnl_req_info *req_info)
+{
+	const struct strset_data *data =
+		container_of(req_info, struct strset_data, reqinfo_base);
+	unsigned int i;
+	int len = 0;
+	int ret;
+
+	len += ethnl_reply_header_size();
+	for (i = 0; i < ETH_SS_COUNT; i++) {
+		const struct strset_info *info = &data->info[i];
+
+		if (!strset_include(data, i) || info->type == ETH_SS_TYPE_NONE)
+			continue;
+
+		ret = strset_set_size(info, data->counts_only);
+		if (ret < 0)
+			return ret;
+		len += ret;
+	}
+
+	return len;
+}
+
+/* fill one string into reply */
+static int strset_fill_string(struct sk_buff *skb,
+			      const struct strset_info *info, u32 idx)
+{
+	struct nlattr *string;
+	const char *value;
+
+	if (info->type == ETH_SS_TYPE_LEGACY)
+		value = info->data.legacy[idx];
+	else
+		value = info->data.simple[idx];
+
+	string = nla_nest_start(skb, ETHTOOL_A_STRINGS_STRING);
+	if (!string)
+		return -EMSGSIZE;
+	if (nla_put_u32(skb, ETHTOOL_A_STRING_INDEX, idx) ||
+	    nla_put_string(skb, ETHTOOL_A_STRING_VALUE, value))
+		return -EMSGSIZE;
+	nla_nest_end(skb, string);
+
+	return 0;
+}
+
+/* fill one string set into reply */
+static int strset_fill_set(struct sk_buff *skb, const struct strset_data *data,
+			   u32 id)
+{
+	const struct strset_info *info = &data->info[id];
+	struct nlattr *strings;
+	struct nlattr *nest;
+	unsigned int i = (unsigned int)(-1);
+
+	if (info->type == ETH_SS_TYPE_NONE)
+		return -EOPNOTSUPP;
+	if (info->count == 0)
+		return 0;
+	nest = nla_nest_start(skb, ETHTOOL_A_STRINGSETS_STRINGSET);
+	if (!nest)
+		return -EMSGSIZE;
+
+	if (nla_put_u32(skb, ETHTOOL_A_STRINGSET_ID, id) ||
+	    nla_put_u32(skb, ETHTOOL_A_STRINGSET_COUNT, info->count))
+		goto nla_put_failure;
+
+	if (!data->counts_only) {
+		strings = nla_nest_start(skb, ETHTOOL_A_STRINGSET_STRINGS);
+		if (!strings)
+			goto nla_put_failure;
+		for (i = 0; i < info->count; i++) {
+			if (strset_fill_string(skb, info, i) < 0)
+				goto nla_put_failure;
+		}
+		nla_nest_end(skb, strings);
+	}
+
+	nla_nest_end(skb, nest);
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -EMSGSIZE;
+}
+
+/* fill_reply() handler */
+static int strset_fill(struct sk_buff *skb,
+		       const struct ethnl_req_info *req_info)
+{
+	const struct strset_data *data =
+		container_of(req_info, struct strset_data, reqinfo_base);
+	struct nlattr *nest;
+	unsigned int i;
+	int ret;
+
+	nest = nla_nest_start(skb, ETHTOOL_A_STRSET_STRINGSETS);
+	if (!nest)
+		return -EMSGSIZE;
+
+	for (i = 0; i < ETH_SS_COUNT; i++) {
+		if (strset_include(data, i)) {
+			ret = strset_fill_set(skb, data, i);
+			if (ret < 0)
+				goto nla_put_failure;
+		}
+	}
+
+	nla_nest_end(skb, nest);
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return ret;
+}
+
+const struct get_request_ops strset_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_STRSET_GET,
+	.reply_cmd		= ETHTOOL_MSG_STRSET_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_STRSET_HEADER,
+	.max_attr		= ETHTOOL_A_STRSET_MAX,
+	.data_size		= sizeof(struct strset_data),
+	.repdata_offset		= offsetof(struct strset_data, repdata_base),
+	.all_reqflags		= ETHTOOL_RF_STRSET_ALL,
+	.allow_nodev_do		= true,
+
+	.parse_request		= strset_parse,
+	.prepare_data		= strset_prepare,
+	.reply_size		= strset_size,
+	.fill_reply		= strset_fill,
+	.cleanup		= strset_cleanup,
+};
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 09/15] ethtool: generic handlers for GET requests
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Significant part of GET request processing is common for most request
types but unfortunately it cannot be easily separated from type specific
code as we need to alternate between common actions (parsing common request
header, allocating message and filling netlink/genetlink headers etc.) and
specific actions (querying the device, composing the reply). The processing
also happens in three different situations: "do" request, "dump" request
and notification, each doing things in slightly different way.

The request specific code is implemented in four or five callbacks defined
in an instance of struct get_request_ops:

  parse_request() - parse incoming message
  prepare_data()  - retrieve data from driver or NIC
  reply_size()    - estimate reply message size
  fill_reply()    - compose reply message
  cleanup()       - (optional) clean up additional data

Other members of struct get_request_ops describe the data structure holding
information from client request and data used to compose the message. The
standard handlers ethnl_get_doit(), ethnl_get_dumpit(), ethnl_get_start()
and ethnl_get_done() can be then used in genl_ops handler. Notification
handler will be introduced in a later patch.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 net/ethtool/netlink.c | 337 ++++++++++++++++++++++++++++++++++++++++++
 net/ethtool/netlink.h | 128 ++++++++++++++++
 2 files changed, 465 insertions(+)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index a7a0bfe1818c..6d326cc25aac 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -178,6 +178,343 @@ struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
 	return NULL;
 }
 
+/* GET request helpers */
+
+/**
+ * struct ethnl_dump_ctx - context structure for generic dumpit() callback
+ * @ops:      request ops of currently processed message type
+ * @req_info: parsed request header of processed request
+ * @pos_hash: saved iteration position - hashbucket
+ * @pos_idx:  saved iteration position - index
+ *
+ * These parameters are kept in struct netlink_callback as context preserved
+ * between iterations. They are initialized by ethnl_get_start() and used in
+ * ethnl_get_dumpit() and ethnl_get_done().
+ */
+struct ethnl_dump_ctx {
+	const struct get_request_ops	*ops;
+	struct ethnl_req_info		*req_info;
+	int				pos_hash;
+	int				pos_idx;
+};
+
+static const struct get_request_ops *get_requests[__ETHTOOL_MSG_USER_CNT] = {
+};
+
+static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
+{
+	return (struct ethnl_dump_ctx *)cb->ctx;
+}
+
+/**
+ * ethnl_alloc_get_data() - Allocate and initialize data for a GET request
+ * @ops: instance of struct get_request_ops describing size and layout
+ *
+ * This initializes only the first part (req_info), second part (reply_data)
+ * is initialized before filling the reply data into it (which is done for
+ * each iteration in dump requests).
+ *
+ * Return: pointer to allocated and initialized data, NULL on error
+ */
+static struct ethnl_req_info *
+ethnl_alloc_get_data(const struct get_request_ops *ops)
+{
+	struct ethnl_req_info *req_info;
+
+	req_info = kmalloc(ops->data_size, GFP_KERNEL);
+	if (!req_info)
+		return NULL;
+
+	memset(req_info, '\0', ops->repdata_offset);
+	req_info->reply_data =
+		(struct ethnl_reply_data *)((char *)req_info +
+					    ops->repdata_offset);
+
+	return req_info;
+}
+
+/**
+ * ethnl_free_get_data() - free GET request data
+ * @ops: instance of struct get_request_ops describing the layout
+ * @req_info: pointer to embedded struct ethnl_req_info (at offset 0)
+ *
+ * Calls ->cleanup() handler if defined and frees the data block.
+ */
+static void ethnl_free_get_data(const struct get_request_ops *ops,
+				struct ethnl_req_info *req_info)
+{
+	if (ops->cleanup)
+		ops->cleanup(req_info);
+	kfree(req_info);
+}
+
+/**
+ * ethnl_std_parse() - Parse request message
+ * @req_info:    pointer to structure to put data into
+ * @nlhdr:       pointer to request message header
+ * @net:         request netns
+ * @request_ops: struct request_ops for request type
+ * @extack:      netlink extack for error reporting
+ * @require_dev: fail if no device identiified in header
+ *
+ * Parse universal request header and call request specific ->parse_request()
+ * callback (if defined) to parse the rest of the message.
+ *
+ * Return: 0 on success or negative error code
+ */
+static int ethnl_std_parse(struct ethnl_req_info *req_info,
+			   const struct nlmsghdr *nlhdr, struct net *net,
+			   const struct get_request_ops *request_ops,
+			   struct netlink_ext_ack *extack, bool require_dev)
+{
+	struct nlattr **tb;
+	int ret;
+
+	tb = kmalloc_array(request_ops->max_attr + 1, sizeof(tb[0]),
+			   GFP_KERNEL);
+	if (!tb)
+		return -ENOMEM;
+
+	ret = nlmsg_parse(nlhdr, GENL_HDRLEN, tb, request_ops->max_attr,
+			  request_ops->request_policy, extack);
+	if (ret < 0)
+		goto out;
+	ret = ethnl_parse_header(req_info, tb[request_ops->hdr_attr], net,
+				 extack, request_ops->header_policy,
+				 require_dev);
+	if (ret < 0)
+		goto out;
+
+	if (request_ops->parse_request) {
+		ret = request_ops->parse_request(req_info, tb, extack);
+		if (ret < 0)
+			goto out;
+	}
+
+	if (req_info->req_mask == 0)
+		req_info->req_mask = request_ops->default_infomask;
+	if (req_info->req_flags & ~request_ops->all_reqflags) {
+		ret = -EOPNOTSUPP;
+		NL_SET_ERR_MSG_ATTR(extack, tb[ETHTOOL_A_HEADER_RFLAGS],
+				    "unsupported request specific flags");
+		goto out;
+	}
+
+	ret = 0;
+out:
+	kfree(tb);
+	return ret;
+}
+
+/**
+ * ethnl_init_reply_data() - Initialize reply data for GET request
+ * @req_info: pointer to embedded struct ethnl_req_info
+ * @ops:      instance of struct get_request_ops describing the layout
+ * @dev:      network device to initialize the reply for
+ *
+ * Fills the reply data part with zeros and sets the dev member. Must be called
+ * before calling the ->fill_reply() callback (for each iteration when handling
+ * dump requests).
+ */
+static void ethnl_init_reply_data(const struct ethnl_req_info *req_info,
+				  const struct get_request_ops *ops,
+				  struct net_device *dev)
+{
+	memset(req_info->reply_data, '\0',
+	       ops->data_size - ops->repdata_offset);
+	req_info->reply_data->dev = dev;
+}
+
+/* generic ->doit() handler for GET type requests */
+static int ethnl_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	const u8 cmd = info->genlhdr->cmd;
+	const struct get_request_ops *ops;
+	struct ethnl_req_info *req_info;
+	struct sk_buff *rskb;
+	void *reply_payload;
+	int reply_len;
+	int ret;
+
+	ops = get_requests[cmd];
+	if (WARN_ONCE(!ops, "cmd %u has no get_request_ops\n", cmd))
+		return -EOPNOTSUPP;
+	req_info = ethnl_alloc_get_data(ops);
+	if (!req_info)
+		return -ENOMEM;
+	ret = ethnl_std_parse(req_info, info->nlhdr, genl_info_net(info), ops,
+			      info->extack, !ops->allow_nodev_do);
+	if (ret < 0)
+		goto err_dev;
+	req_info->privileged = ethnl_is_privileged(skb);
+	ethnl_init_reply_data(req_info, ops, req_info->dev);
+
+	rtnl_lock();
+	ret = ops->prepare_data(req_info, info);
+	if (ret < 0)
+		goto err_rtnl;
+	reply_len = ops->reply_size(req_info);
+	if (ret < 0)
+		goto err_cleanup;
+	ret = -ENOMEM;
+	rskb = ethnl_reply_init(reply_len, req_info->dev, ops->reply_cmd,
+				ops->hdr_attr, info, &reply_payload);
+	if (!rskb)
+		goto err_cleanup;
+	ret = ops->fill_reply(rskb, req_info);
+	if (ret < 0)
+		goto err_msg;
+	rtnl_unlock();
+
+	genlmsg_end(rskb, reply_payload);
+	if (req_info->dev)
+		dev_put(req_info->dev);
+	ethnl_free_get_data(ops, req_info);
+	return genlmsg_reply(rskb, info);
+
+err_msg:
+	WARN_ONCE(ret == -EMSGSIZE,
+		  "calculated message payload length (%d) not sufficient\n",
+		  reply_len);
+	nlmsg_free(rskb);
+err_cleanup:
+	ethnl_free_get_data(ops, req_info);
+err_rtnl:
+	rtnl_unlock();
+err_dev:
+	if (req_info->dev)
+		dev_put(req_info->dev);
+	return ret;
+}
+
+static int ethnl_get_dump_one(struct sk_buff *skb,
+			      struct net_device *dev,
+			      const struct get_request_ops *ops,
+			      struct ethnl_req_info *req_info)
+{
+	int ret;
+
+	ethnl_init_reply_data(req_info, ops, dev);
+	rtnl_lock();
+	ret = ops->prepare_data(req_info, NULL);
+	if (ret < 0)
+		goto out;
+	ret = ethnl_fill_reply_header(skb, dev, ops->hdr_attr);
+	if (ret < 0)
+		goto out_cleanup;
+	ret = ops->fill_reply(skb, req_info);
+
+out_cleanup:
+	if (ops->cleanup)
+		ops->cleanup(req_info);
+out:
+	rtnl_unlock();
+	req_info->reply_data->dev = NULL;
+	return ret;
+}
+
+/* generic ->dumpit() handler for GET requests; device iteration copied from
+ * rtnl_dump_ifinfo()
+ */
+static int ethnl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
+	struct ethnl_req_info *req_info = ctx->req_info;
+	const struct get_request_ops *ops = ctx->ops;
+	struct net *net = sock_net(skb->sk);
+	int s_idx = ctx->pos_idx;
+	struct hlist_head *head;
+	struct net_device *dev;
+	int h, idx = 0;
+	int ret = 0;
+	void *ehdr;
+
+	for (h = ctx->pos_hash; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
+		idx = 0;
+		head = &net->dev_index_head[h];
+		hlist_for_each_entry(dev, head, index_hlist) {
+			if (idx < s_idx)
+				goto cont;
+			ehdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
+					   cb->nlh->nlmsg_seq,
+					   &ethtool_genl_family, 0,
+					   ops->reply_cmd);
+			ret = ethnl_get_dump_one(skb, dev, ops, req_info);
+			if (ret < 0) {
+				genlmsg_cancel(skb, ehdr);
+				if (ret == -EOPNOTSUPP)
+					goto cont;
+				if (likely(skb->len))
+					goto out;
+				goto out_err;
+			}
+			genlmsg_end(skb, ehdr);
+cont:
+			idx++;
+		}
+	}
+out:
+	ret = skb->len;
+out_err:
+	ctx->pos_hash = h;
+	ctx->pos_idx = idx;
+	cb->seq = net->dev_base_seq;
+	nl_dump_check_consistent(cb, nlmsg_hdr(skb));
+
+	return ret;
+}
+
+/* generic ->start() handler for GET requests */
+static int ethnl_get_start(struct netlink_callback *cb)
+{
+	struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
+	const struct get_request_ops *ops;
+	struct ethnl_req_info *req_info;
+	struct genlmsghdr *ghdr;
+	int ret;
+
+	BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
+
+	ghdr = nlmsg_data(cb->nlh);
+	ops = get_requests[ghdr->cmd];
+	if (WARN_ONCE(!ops, "cmd %u has no get_request_ops\n", ghdr->cmd))
+		return -EOPNOTSUPP;
+	req_info = ethnl_alloc_get_data(ops);
+	if (!req_info)
+		return -ENOMEM;
+
+	ret = ethnl_std_parse(req_info, cb->nlh, sock_net(cb->skb->sk), ops,
+			      cb->extack, false);
+	if (req_info->dev) {
+		/* We ignore device specification in dump requests but as the
+		 * same parser as for non-dump (doit) requests is used, it
+		 * would take reference to the device if it finds one
+		 */
+		dev_put(req_info->dev);
+		req_info->dev = NULL;
+	}
+	if (ret < 0)
+		return ret;
+	req_info->privileged = ethnl_is_privileged(cb->skb);
+
+	ctx->ops = ops;
+	ctx->req_info = req_info;
+	ctx->pos_hash = 0;
+	ctx->pos_idx = 0;
+
+	return 0;
+}
+
+/* generic ->done() handler for GET requests */
+static int ethnl_get_done(struct netlink_callback *cb)
+{
+	struct ethnl_dump_ctx *ctx = ethnl_dump_context(cb);
+
+	kfree(ctx->req_info);
+
+	return 0;
+}
+
 /* notifications */
 
 typedef void (*ethnl_notify_handler_t)(struct net_device *dev,
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 7f1b9ec1ace7..6a9695c3b0c6 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -143,8 +143,32 @@ static inline unsigned int ethnl_reply_header_size(void)
 			      nla_total_size(IFNAMSIZ));
 }
 
+/* GET request handling */
+
+struct ethnl_reply_data;
+
+/* The structure holding data for unified processing GET requests consists of
+ * two parts: request info and reply data. Request info is related to client
+ * request and for dump request it stays constant through all processing;
+ * reply data contains data for composing a reply message. When processing
+ * a dump request, request info is filled only once but reply data is filled
+ * from scratch for each reply message.
+ *
+ * +-----------------+-----------------+------------------+-----------------+
+ * | common_req_info |  specific info  | ethnl_reply_data |  specific data  |
+ * +-----------------+-----------------+------------------+-----------------+
+ * |<---------- request info --------->|<----------- reply data ----------->|
+ *
+ * Request info always starts at offset 0 with struct ethnl_req_info which
+ * holds information from parsing the common header. It may be followed by
+ * other members for request attributes specific for current message type.
+ * Reply data starts with struct ethnl_reply_data which may be followed by
+ * other members holding data needed to compose a message.
+ */
+
 /**
  * struct ethnl_req_info - base type of request information for GET requests
+ * @reply_data: pointer to reply data within the same block
  * @dev:          network device the request is for (may be null)
  * @req_mask:     request mask, bitmap of requested information
  * @global_flags: request flags common for all request types
@@ -154,6 +178,7 @@ static inline unsigned int ethnl_reply_header_size(void)
  * This is a common base, additional members may follow after this structure.
  */
 struct ethnl_req_info {
+	struct ethnl_reply_data		*reply_data;
 	struct net_device		*dev;
 	u32				req_mask;
 	u32				global_flags;
@@ -161,4 +186,107 @@ struct ethnl_req_info {
 	bool				privileged;
 };
 
+/**
+ * struct ethnl_reply_data - base type of reply data for GET requests
+ * @dev:       device for current reply message; in single shot requests it is
+ *             equal to &ethnl_req_info.dev; in dumps it's different for each
+ *             reply message
+ * @info_mask: bitmap of information actually provided in reply; it is a subset
+ *             of &ethnl_req_info.req_mask with cleared bits corresponding to
+ *             information which cannot be provided
+ *
+ * This structure is usually followed by additional members filled by
+ * ->prepare_data() and used by ->cleanup().
+ */
+struct ethnl_reply_data {
+	struct net_device		*dev;
+	u32				info_mask;
+};
+
+static inline int ethnl_before_ops(struct net_device *dev)
+{
+	if (dev && dev->ethtool_ops->begin)
+		return dev->ethtool_ops->begin(dev);
+	else
+		return 0;
+}
+
+static inline void ethnl_after_ops(struct net_device *dev)
+{
+	if (dev && dev->ethtool_ops->complete)
+		dev->ethtool_ops->complete(dev);
+}
+
+/**
+ * struct get_request_ops - unified handling of GET requests
+ * @request_cmd:      command id for request (GET)
+ * @reply_cmd:        command id for reply (GET_REPLY)
+ * @hdr_attr:         attribute type for request header
+ * @max_attr:         maximum (top level) attribute type
+ * @data_size:        total length of data structure
+ * @repdata_offset:   offset of "reply data" part (struct ethnl_reply_data)
+ * @request_policy:   netlink policy for message contents
+ * @header_policy:    (optional) netlink policy for request header
+ * @default_infomask: default infomask (to use if none specified)
+ * @all_reqflags:     allowed request specific flags
+ * @allow_nodev_do:   allow non-dump request with no device identification
+ * @parse_request:
+ *	Parse request except common header (struct ethnl_req_info). Common
+ *	header is already filled on entry, the rest up to @repdata_offset
+ *	is zero initialized. This callback should only modify type specific
+ *	request info by parsed attributes from request message.
+ * @prepare_data:
+ *	Retrieve and prepare data needed to compose a reply message. Calls to
+ *	ethtool_ops handlers should be limited to this callback. Common reply
+ *	data (struct ethnl_reply_data) is filled on entry, type specific part
+ *	after it is zero initialized. This callback should only modify the
+ *	type specific part of reply data. Device identification from struct
+ *	ethnl_reply_data is to be used as for dump requests, it iterates
+ *	through network devices which common_req_info::dev points to the
+ *	device from client request.
+ * @reply_size:
+ *	Estimate reply message size. Returned value must be sufficient for
+ *	message payload without common reply header. The callback may returned
+ *	estimate higher than actual message size if exact calculation would
+ *	not be worth the saved memory space.
+ * @fill_reply:
+ *	Fill reply message payload (except for common header) from reply data.
+ *	The callback must not generate more payload than previously called
+ *	->reply_size() estimated.
+ * @cleanup:
+ *	Optional cleanup called when reply data is no longer needed. Can be
+ *	used e.g. to free any additional data structures outside the main
+ *	structure which were allocated by ->prepare_data(). When processing
+ *	dump requests, ->cleanup() is called for each message.
+ *
+ * Description of variable parts of GET request handling when using the unified
+ * infrastructure. When used, a pointer to an instance of this structure is to
+ * be added to &get_requests array and generic handlers ethnl_get_doit(),
+ * ethnl_get_dumpit(), ethnl_get_start() and ethnl_get_done() used in
+ * @ethnl_genl_ops
+ */
+struct get_request_ops {
+	u8			request_cmd;
+	u8			reply_cmd;
+	u16			hdr_attr;
+	unsigned int		max_attr;
+	unsigned int		data_size;
+	unsigned int		repdata_offset;
+	const struct nla_policy *request_policy;
+	const struct nla_policy *header_policy;
+	u32			default_infomask;
+	u32			all_reqflags;
+	bool			allow_nodev_do;
+
+	int (*parse_request)(struct ethnl_req_info *req_info,
+			     struct nlattr **tb,
+			     struct netlink_ext_ack *extack);
+	int (*prepare_data)(struct ethnl_req_info *req_info,
+			    struct genl_info *info);
+	int (*reply_size)(const struct ethnl_req_info *req_info);
+	int (*fill_reply)(struct sk_buff *skb,
+			  const struct ethnl_req_info *req_info);
+	void (*cleanup)(struct ethnl_req_info *req_info);
+};
+
 #endif /* _NET_ETHTOOL_NETLINK_H */
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 08/15] ethtool: move string arrays into common file
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Introduce file net/ethtool/common.c for code shared by ioctl and netlink
ethtool interface. Move name tables of features, RSS hash functions,
tunables and PHY tunables into this file.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 net/ethtool/Makefile |  2 +-
 net/ethtool/common.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
 net/ethtool/common.h | 17 +++++++++
 net/ethtool/ioctl.c  | 83 ++-----------------------------------------
 4 files changed, 104 insertions(+), 82 deletions(-)
 create mode 100644 net/ethtool/common.c
 create mode 100644 net/ethtool/common.h

diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 482fdb9380fa..11782306593b 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-obj-y				+= ioctl.o
+obj-y				+= ioctl.o common.o
 
 obj-$(CONFIG_ETHTOOL_NETLINK)	+= ethtool_nl.o
 
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
new file mode 100644
index 000000000000..b0ce420e994e
--- /dev/null
+++ b/net/ethtool/common.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
+
+#include "common.h"
+
+const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
+	[NETIF_F_SG_BIT] =               "tx-scatter-gather",
+	[NETIF_F_IP_CSUM_BIT] =          "tx-checksum-ipv4",
+	[NETIF_F_HW_CSUM_BIT] =          "tx-checksum-ip-generic",
+	[NETIF_F_IPV6_CSUM_BIT] =        "tx-checksum-ipv6",
+	[NETIF_F_HIGHDMA_BIT] =          "highdma",
+	[NETIF_F_FRAGLIST_BIT] =         "tx-scatter-gather-fraglist",
+	[NETIF_F_HW_VLAN_CTAG_TX_BIT] =  "tx-vlan-hw-insert",
+
+	[NETIF_F_HW_VLAN_CTAG_RX_BIT] =  "rx-vlan-hw-parse",
+	[NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
+	[NETIF_F_HW_VLAN_STAG_TX_BIT] =  "tx-vlan-stag-hw-insert",
+	[NETIF_F_HW_VLAN_STAG_RX_BIT] =  "rx-vlan-stag-hw-parse",
+	[NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
+	[NETIF_F_VLAN_CHALLENGED_BIT] =  "vlan-challenged",
+	[NETIF_F_GSO_BIT] =              "tx-generic-segmentation",
+	[NETIF_F_LLTX_BIT] =             "tx-lockless",
+	[NETIF_F_NETNS_LOCAL_BIT] =      "netns-local",
+	[NETIF_F_GRO_BIT] =              "rx-gro",
+	[NETIF_F_GRO_HW_BIT] =           "rx-gro-hw",
+	[NETIF_F_LRO_BIT] =              "rx-lro",
+
+	[NETIF_F_TSO_BIT] =              "tx-tcp-segmentation",
+	[NETIF_F_GSO_ROBUST_BIT] =       "tx-gso-robust",
+	[NETIF_F_TSO_ECN_BIT] =          "tx-tcp-ecn-segmentation",
+	[NETIF_F_TSO_MANGLEID_BIT] =	 "tx-tcp-mangleid-segmentation",
+	[NETIF_F_TSO6_BIT] =             "tx-tcp6-segmentation",
+	[NETIF_F_FSO_BIT] =              "tx-fcoe-segmentation",
+	[NETIF_F_GSO_GRE_BIT] =		 "tx-gre-segmentation",
+	[NETIF_F_GSO_GRE_CSUM_BIT] =	 "tx-gre-csum-segmentation",
+	[NETIF_F_GSO_IPXIP4_BIT] =	 "tx-ipxip4-segmentation",
+	[NETIF_F_GSO_IPXIP6_BIT] =	 "tx-ipxip6-segmentation",
+	[NETIF_F_GSO_UDP_TUNNEL_BIT] =	 "tx-udp_tnl-segmentation",
+	[NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
+	[NETIF_F_GSO_PARTIAL_BIT] =	 "tx-gso-partial",
+	[NETIF_F_GSO_SCTP_BIT] =	 "tx-sctp-segmentation",
+	[NETIF_F_GSO_ESP_BIT] =		 "tx-esp-segmentation",
+	[NETIF_F_GSO_UDP_L4_BIT] =	 "tx-udp-segmentation",
+
+	[NETIF_F_FCOE_CRC_BIT] =         "tx-checksum-fcoe-crc",
+	[NETIF_F_SCTP_CRC_BIT] =        "tx-checksum-sctp",
+	[NETIF_F_FCOE_MTU_BIT] =         "fcoe-mtu",
+	[NETIF_F_NTUPLE_BIT] =           "rx-ntuple-filter",
+	[NETIF_F_RXHASH_BIT] =           "rx-hashing",
+	[NETIF_F_RXCSUM_BIT] =           "rx-checksum",
+	[NETIF_F_NOCACHE_COPY_BIT] =     "tx-nocache-copy",
+	[NETIF_F_LOOPBACK_BIT] =         "loopback",
+	[NETIF_F_RXFCS_BIT] =            "rx-fcs",
+	[NETIF_F_RXALL_BIT] =            "rx-all",
+	[NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
+	[NETIF_F_HW_TC_BIT] =		 "hw-tc-offload",
+	[NETIF_F_HW_ESP_BIT] =		 "esp-hw-offload",
+	[NETIF_F_HW_ESP_TX_CSUM_BIT] =	 "esp-tx-csum-hw-offload",
+	[NETIF_F_RX_UDP_TUNNEL_PORT_BIT] =	 "rx-udp_tunnel-port-offload",
+	[NETIF_F_HW_TLS_RECORD_BIT] =	"tls-hw-record",
+	[NETIF_F_HW_TLS_TX_BIT] =	 "tls-hw-tx-offload",
+	[NETIF_F_HW_TLS_RX_BIT] =	 "tls-hw-rx-offload",
+};
+
+const char
+rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = {
+	[ETH_RSS_HASH_TOP_BIT] =	"toeplitz",
+	[ETH_RSS_HASH_XOR_BIT] =	"xor",
+	[ETH_RSS_HASH_CRC32_BIT] =	"crc32",
+};
+
+const char
+tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
+	[ETHTOOL_ID_UNSPEC]     = "Unspec",
+	[ETHTOOL_RX_COPYBREAK]	= "rx-copybreak",
+	[ETHTOOL_TX_COPYBREAK]	= "tx-copybreak",
+	[ETHTOOL_PFC_PREVENTION_TOUT] = "pfc-prevention-tout",
+};
+
+const char
+phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
+	[ETHTOOL_ID_UNSPEC]     = "Unspec",
+	[ETHTOOL_PHY_DOWNSHIFT]	= "phy-downshift",
+	[ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down",
+};
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
new file mode 100644
index 000000000000..41b2efc1e4e1
--- /dev/null
+++ b/net/ethtool/common.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _ETHTOOL_COMMON_H
+#define _ETHTOOL_COMMON_H
+
+#include <linux/ethtool.h>
+
+extern const char
+netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN];
+extern const char
+rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN];
+extern const char
+tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN];
+extern const char
+phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
+
+#endif /* _ETHTOOL_COMMON_H */
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 6288e69e94fc..b35366dd9997 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -27,6 +27,8 @@
 #include <net/xdp_sock.h>
 #include <net/flow_offload.h>
 
+#include "common.h"
+
 /*
  * Some useful ethtool_ops methods that're device independent.
  * If we find that all drivers want to do the same thing here,
@@ -54,87 +56,6 @@ EXPORT_SYMBOL(ethtool_op_get_ts_info);
 
 #define ETHTOOL_DEV_FEATURE_WORDS	((NETDEV_FEATURE_COUNT + 31) / 32)
 
-static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
-	[NETIF_F_SG_BIT] =               "tx-scatter-gather",
-	[NETIF_F_IP_CSUM_BIT] =          "tx-checksum-ipv4",
-	[NETIF_F_HW_CSUM_BIT] =          "tx-checksum-ip-generic",
-	[NETIF_F_IPV6_CSUM_BIT] =        "tx-checksum-ipv6",
-	[NETIF_F_HIGHDMA_BIT] =          "highdma",
-	[NETIF_F_FRAGLIST_BIT] =         "tx-scatter-gather-fraglist",
-	[NETIF_F_HW_VLAN_CTAG_TX_BIT] =  "tx-vlan-hw-insert",
-
-	[NETIF_F_HW_VLAN_CTAG_RX_BIT] =  "rx-vlan-hw-parse",
-	[NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
-	[NETIF_F_HW_VLAN_STAG_TX_BIT] =  "tx-vlan-stag-hw-insert",
-	[NETIF_F_HW_VLAN_STAG_RX_BIT] =  "rx-vlan-stag-hw-parse",
-	[NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
-	[NETIF_F_VLAN_CHALLENGED_BIT] =  "vlan-challenged",
-	[NETIF_F_GSO_BIT] =              "tx-generic-segmentation",
-	[NETIF_F_LLTX_BIT] =             "tx-lockless",
-	[NETIF_F_NETNS_LOCAL_BIT] =      "netns-local",
-	[NETIF_F_GRO_BIT] =              "rx-gro",
-	[NETIF_F_GRO_HW_BIT] =           "rx-gro-hw",
-	[NETIF_F_LRO_BIT] =              "rx-lro",
-
-	[NETIF_F_TSO_BIT] =              "tx-tcp-segmentation",
-	[NETIF_F_GSO_ROBUST_BIT] =       "tx-gso-robust",
-	[NETIF_F_TSO_ECN_BIT] =          "tx-tcp-ecn-segmentation",
-	[NETIF_F_TSO_MANGLEID_BIT] =	 "tx-tcp-mangleid-segmentation",
-	[NETIF_F_TSO6_BIT] =             "tx-tcp6-segmentation",
-	[NETIF_F_FSO_BIT] =              "tx-fcoe-segmentation",
-	[NETIF_F_GSO_GRE_BIT] =		 "tx-gre-segmentation",
-	[NETIF_F_GSO_GRE_CSUM_BIT] =	 "tx-gre-csum-segmentation",
-	[NETIF_F_GSO_IPXIP4_BIT] =	 "tx-ipxip4-segmentation",
-	[NETIF_F_GSO_IPXIP6_BIT] =	 "tx-ipxip6-segmentation",
-	[NETIF_F_GSO_UDP_TUNNEL_BIT] =	 "tx-udp_tnl-segmentation",
-	[NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
-	[NETIF_F_GSO_PARTIAL_BIT] =	 "tx-gso-partial",
-	[NETIF_F_GSO_SCTP_BIT] =	 "tx-sctp-segmentation",
-	[NETIF_F_GSO_ESP_BIT] =		 "tx-esp-segmentation",
-	[NETIF_F_GSO_UDP_L4_BIT] =	 "tx-udp-segmentation",
-
-	[NETIF_F_FCOE_CRC_BIT] =         "tx-checksum-fcoe-crc",
-	[NETIF_F_SCTP_CRC_BIT] =        "tx-checksum-sctp",
-	[NETIF_F_FCOE_MTU_BIT] =         "fcoe-mtu",
-	[NETIF_F_NTUPLE_BIT] =           "rx-ntuple-filter",
-	[NETIF_F_RXHASH_BIT] =           "rx-hashing",
-	[NETIF_F_RXCSUM_BIT] =           "rx-checksum",
-	[NETIF_F_NOCACHE_COPY_BIT] =     "tx-nocache-copy",
-	[NETIF_F_LOOPBACK_BIT] =         "loopback",
-	[NETIF_F_RXFCS_BIT] =            "rx-fcs",
-	[NETIF_F_RXALL_BIT] =            "rx-all",
-	[NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
-	[NETIF_F_HW_TC_BIT] =		 "hw-tc-offload",
-	[NETIF_F_HW_ESP_BIT] =		 "esp-hw-offload",
-	[NETIF_F_HW_ESP_TX_CSUM_BIT] =	 "esp-tx-csum-hw-offload",
-	[NETIF_F_RX_UDP_TUNNEL_PORT_BIT] =	 "rx-udp_tunnel-port-offload",
-	[NETIF_F_HW_TLS_RECORD_BIT] =	"tls-hw-record",
-	[NETIF_F_HW_TLS_TX_BIT] =	 "tls-hw-tx-offload",
-	[NETIF_F_HW_TLS_RX_BIT] =	 "tls-hw-rx-offload",
-};
-
-static const char
-rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = {
-	[ETH_RSS_HASH_TOP_BIT] =	"toeplitz",
-	[ETH_RSS_HASH_XOR_BIT] =	"xor",
-	[ETH_RSS_HASH_CRC32_BIT] =	"crc32",
-};
-
-static const char
-tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
-	[ETHTOOL_ID_UNSPEC]     = "Unspec",
-	[ETHTOOL_RX_COPYBREAK]	= "rx-copybreak",
-	[ETHTOOL_TX_COPYBREAK]	= "tx-copybreak",
-	[ETHTOOL_PFC_PREVENTION_TOUT] = "pfc-prevention-tout",
-};
-
-static const char
-phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
-	[ETHTOOL_ID_UNSPEC]     = "Unspec",
-	[ETHTOOL_PHY_DOWNSHIFT]	= "phy-downshift",
-	[ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down",
-};
-
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_gfeatures cmd = {
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 07/15] ethtool: support for netlink notifications
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Add infrastructure for ethtool netlink notifications. There is only one
multicast group "monitor" which is used to notify userspace about changes
and actions performed. Notification messages (types using suffix _NTF)
share the format with replies to GET requests.

Notifications are supposed to be broadcasted on every configuration change,
whether it is done using the netlink interface or ioctl one. Netlink SET
requests only trigger a notification if some data is actually changed.

To trigger an ethtool notification, both ethtool netlink and external code
use ethtool_notify() helper. This helper requires RTNL to be held and may
sleep. Handlers sending messages for specific notification message types
are registered in ethnl_notify_handlers array. As notifications can be
triggered from other code, ethnl_ok flag is used to prevent an attempt to
send notification before genetlink family is registered.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 include/linux/ethtool_netlink.h      |  5 ++++
 include/linux/netdevice.h            | 12 ++++++++++
 include/uapi/linux/ethtool_netlink.h |  2 ++
 net/ethtool/netlink.c                | 35 ++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+)

diff --git a/include/linux/ethtool_netlink.h b/include/linux/ethtool_netlink.h
index 0412adb4f42f..2a15e64a16f3 100644
--- a/include/linux/ethtool_netlink.h
+++ b/include/linux/ethtool_netlink.h
@@ -5,5 +5,10 @@
 
 #include <uapi/linux/ethtool_netlink.h>
 #include <linux/ethtool.h>
+#include <linux/netdevice.h>
+
+enum ethtool_multicast_groups {
+	ETHNL_MCGRP_MONITOR,
+};
 
 #endif /* _LINUX_ETHTOOL_NETLINK_H_ */
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 88292953aa6f..c57d9917fd50 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4350,6 +4350,18 @@ struct netdev_notifier_bonding_info {
 void netdev_bonding_info_change(struct net_device *dev,
 				struct netdev_bonding_info *bonding_info);
 
+#if IS_ENABLED(CONFIG_ETHTOOL_NETLINK)
+void ethtool_notify(struct net_device *dev, struct netlink_ext_ack *extack,
+		    unsigned int cmd, u32 req_mask, const void *data);
+#else
+static inline void ethtool_notify(struct net_device *dev,
+				  struct netlink_ext_ack *extack,
+				  unsigned int cmd, u32 req_mask,
+				  const void *data)
+{
+}
+#endif
+
 static inline
 struct sk_buff *skb_gso_segment(struct sk_buff *skb, netdev_features_t features)
 {
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 805f314f4454..8938a1f09057 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -91,4 +91,6 @@ enum {
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
 
+#define ETHTOOL_MCGRP_MONITOR_NAME "monitor"
+
 #endif /* _UAPI_LINUX_ETHTOOL_NETLINK_H_ */
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index e13f29bbd625..a7a0bfe1818c 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -6,6 +6,8 @@
 
 static struct genl_family ethtool_genl_family;
 
+static bool ethnl_ok __read_mostly;
+
 static const struct nla_policy dflt_header_policy[ETHTOOL_A_HEADER_MAX + 1] = {
 	[ETHTOOL_A_HEADER_UNSPEC]	= { .type = NLA_REJECT },
 	[ETHTOOL_A_HEADER_DEV_INDEX]	= { .type = NLA_U32 },
@@ -176,11 +178,41 @@ struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
 	return NULL;
 }
 
+/* notifications */
+
+typedef void (*ethnl_notify_handler_t)(struct net_device *dev,
+				       struct netlink_ext_ack *extack,
+				       unsigned int cmd, u32 req_mask,
+				       const void *data);
+
+static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
+};
+
+void ethtool_notify(struct net_device *dev, struct netlink_ext_ack *extack,
+		    unsigned int cmd, u32 req_mask, const void *data)
+{
+	if (unlikely(!ethnl_ok))
+		return;
+	ASSERT_RTNL();
+
+	if (likely(cmd < ARRAY_SIZE(ethnl_notify_handlers) &&
+		   ethnl_notify_handlers[cmd]))
+		ethnl_notify_handlers[cmd](dev, extack, cmd, req_mask, data);
+	else
+		WARN_ONCE(1, "notification %u not implemented (dev=%s, req_mask=0x%x)\n",
+			  cmd, netdev_name(dev), req_mask);
+}
+EXPORT_SYMBOL(ethtool_notify);
+
 /* genetlink setup */
 
 static const struct genl_ops ethtool_genl_ops[] = {
 };
 
+static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
+	[ETHNL_MCGRP_MONITOR] = { .name = ETHTOOL_MCGRP_MONITOR_NAME },
+};
+
 static struct genl_family ethtool_genl_family = {
 	.name		= ETHTOOL_GENL_NAME,
 	.version	= ETHTOOL_GENL_VERSION,
@@ -188,6 +220,8 @@ static struct genl_family ethtool_genl_family = {
 	.parallel_ops	= true,
 	.ops		= ethtool_genl_ops,
 	.n_ops		= ARRAY_SIZE(ethtool_genl_ops),
+	.mcgrps		= ethtool_nl_mcgrps,
+	.n_mcgrps	= ARRAY_SIZE(ethtool_nl_mcgrps),
 };
 
 /* module setup */
@@ -199,6 +233,7 @@ static int __init ethnl_init(void)
 	ret = genl_register_family(&ethtool_genl_family);
 	if (WARN(ret < 0, "ethtool: genetlink family registration failed"))
 		return ret;
+	ethnl_ok = true;
 
 	return 0;
 }
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

The ethtool netlink code uses common framework for passing arbitrary
length bit sets to allow future extensions. A bitset can be a list (only
one bitmap) or can consist of value and mask pair (used e.g. when client
want to modify only some bits). A bitset can use one of two formats:
verbose (bit by bit) or compact.

Verbose format consists of bitset size (number of bits), list flag and
an array of bit nests, telling which bits are part of the list or which
bits are in the mask and which of them are to be set. In requests, bits
can be identified by index (position) or by name. In replies, kernel
provides both index and name. Verbose format is suitable for "one shot"
applications like standard ethtool command as it avoids the need to
either keep bit names (e.g. link modes) in sync with kernel or having to
add an extra roundtrip for string set request (e.g. for private flags).

Compact format uses one (list) or two (value/mask) arrays of 32-bit
words to store the bitmap(s). It is more suitable for long running
applications (ethtool in monitor mode or network management daemons)
which can retrieve the names once and then pass only compact bitmaps to
save space.

Userspace requests can use either format and ETHTOOL_RF_COMPACT flag in
request header tells kernel which format to use in reply. Notifications
always use compact format.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.txt |  61 ++
 include/uapi/linux/ethtool_netlink.h         |  35 ++
 net/ethtool/Makefile                         |   2 +-
 net/ethtool/bitset.c                         | 606 +++++++++++++++++++
 net/ethtool/bitset.h                         |  40 ++
 net/ethtool/netlink.h                        |   9 +
 6 files changed, 752 insertions(+), 1 deletion(-)
 create mode 100644 net/ethtool/bitset.c
 create mode 100644 net/ethtool/bitset.h

diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
index 97c369aa290b..4636682c551f 100644
--- a/Documentation/networking/ethtool-netlink.txt
+++ b/Documentation/networking/ethtool-netlink.txt
@@ -73,6 +73,67 @@ set, the behaviour is the same as (or closer to) the behaviour before it was
 introduced.
 
 
+Bit sets
+--------
+
+For short bitmaps of (reasonably) fixed length, standard NLA_BITFIELD32 type
+is used. For arbitrary length bitmaps, ethtool netlink uses a nested attribute
+with contents of one of two forms: compact (two binary bitmaps representing
+bit values and mask of affected bits) and bit-by-bit (list of bits identified
+by either index or name).
+
+Compact form: nested (bitset) atrribute contents:
+
+    ETHTOOL_A_BITSET_LIST	(flag)		no mask, only a list
+    ETHTOOL_A_BITSET_SIZE	(u32)		number of significant bits
+    ETHTOOL_A_BITSET_VALUE	(binary)	bitmap of bit values
+    ETHTOOL_A_BITSET_MASK	(binary)	bitmap of valid bits
+
+Value and mask must have length at least ETHTOOL_A_BITSET_SIZE bits rounded up
+to a multiple of 32 bits. They consist of 32-bit words in host byte order,
+words ordered from least significant to most significant (i.e. the same way as
+bitmaps are passed with ioctl interface).
+
+For compact form, ETHTOOL_A_BITSET_SIZE and ETHTOOL_A_BITSET_VALUE are
+mandatory.  Similar to BITFIELD32, a compact form bit set requests to set bits
+in the mask to 1 (if the bit is set in value) or 0 (if not) and preserve the
+rest. If ETHTOOL_A_BITSET_LIST is present, there is no mask and bitset
+represents a simple list of bits.
+
+Kernel bit set length may differ from userspace length if older application is
+used on newer kernel or vice versa. If userspace bitmap is longer, an error is
+issued only if the request actually tries to set values of some bits not
+recognized by kernel.
+
+Bit-by-bit form: nested (bitset) attribute contents:
+
+    ETHTOOL_A_BITSET_LIST	(flag)		no mask, only a list
+    ETHTOOL_A_BITSET_SIZE	(u32)		number of significant bits
+    ETHTOOL_A_BITSET_BIT	(nested)	array of bits
+	ETHTOOL_A_BITSET_BIT+   (nested)	one bit
+	    ETHTOOL_A_BIT_INDEX	(u32)		bit index (0 for LSB)
+	    ETHTOOL_A_BIT_NAME	(string)	bit name
+	    ETHTOOL_A_BIT_VALUE	(flag)		present if bit is set
+
+Bit size is optional for bit-by-bit form. ETHTOOL_A_BITSET_BITS nest can only
+contain ETHTOOL_A_BITS_BIT attributes but there can be an arbitrary number of
+them.  A bit may be identified by its index or by its name. When used in
+requests, listed bits are set to 0 or 1 according to ETHTOOL_A_BIT_VALUE, the
+rest is preserved. A request fails if index exceeds kernel bit length or if
+name is not recognized.
+
+When ETHTOOL_A_BITSET_LIST flag is present, bitset is interpreted as a simple
+bit list. ETHTOOL_A_BIT_VALUE attributes are not used in such case. Bit list
+represents a bitmap with listed bits set and the rest zero.
+
+In requests, application can use either form. Form used by kernel in reply is
+determined by a flag in flags field of request header. Semantics of value and
+mask depends on the attribute. General idea is that flags control request
+processing, info_mask control which parts of the information are returned in
+"get" request and index identifies a particular subcommand or an object to
+which the request applies.
+
+
 List of message types
 ---------------------
 
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index ffd7db0848ef..805f314f4454 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -52,6 +52,41 @@ enum {
 	ETHTOOL_A_HEADER_MAX = (__ETHTOOL_A_HEADER_CNT - 1)
 };
 
+/* bit sets */
+
+enum {
+	ETHTOOL_A_BIT_UNSPEC,
+	ETHTOOL_A_BIT_INDEX,			/* u32 */
+	ETHTOOL_A_BIT_NAME,			/* string */
+	ETHTOOL_A_BIT_VALUE,			/* flag */
+
+	/* add new constants above here */
+	__ETHTOOL_A_BIT_CNT,
+	ETHTOOL_A_BIT_MAX = (__ETHTOOL_A_BIT_CNT - 1)
+};
+
+enum {
+	ETHTOOL_A_BITS_UNSPEC,
+	ETHTOOL_A_BITS_BIT,
+
+	/* add new constants above here */
+	__ETHTOOL_A_BITS_CNT,
+	ETHTOOL_A_BITS_MAX = (__ETHTOOL_A_BITS_CNT - 1)
+};
+
+enum {
+	ETHTOOL_A_BITSET_UNSPEC,
+	ETHTOOL_A_BITSET_LIST,			/* flag */
+	ETHTOOL_A_BITSET_SIZE,			/* u32 */
+	ETHTOOL_A_BITSET_BITS,			/* nest - _A_BITS_* */
+	ETHTOOL_A_BITSET_VALUE,			/* binary */
+	ETHTOOL_A_BITSET_MASK,			/* binary */
+
+	/* add new constants above here */
+	__ETHTOOL_A_BITSET_CNT,
+	ETHTOOL_A_BITSET_MAX = (__ETHTOOL_A_BITSET_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 f30e0da88be5..482fdb9380fa 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -4,4 +4,4 @@ obj-y				+= ioctl.o
 
 obj-$(CONFIG_ETHTOOL_NETLINK)	+= ethtool_nl.o
 
-ethtool_nl-y	:= netlink.o
+ethtool_nl-y	:= netlink.o bitset.o
diff --git a/net/ethtool/bitset.c b/net/ethtool/bitset.c
new file mode 100644
index 000000000000..80bb6fbb1268
--- /dev/null
+++ b/net/ethtool/bitset.c
@@ -0,0 +1,606 @@
+// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
+
+#include <linux/ethtool_netlink.h>
+#include <linux/bitmap.h>
+#include "netlink.h"
+#include "bitset.h"
+
+static bool ethnl_test_bit(const void *val, unsigned int index, bool is_u32)
+{
+	if (!val)
+		return true;
+	else if (is_u32)
+		return ((const u32 *)val)[index / 32] & (1U << (index % 32));
+	else
+		return test_bit(index, val);
+}
+
+static void __bitmap_to_u32(u32 *dst, const void *src, unsigned int size,
+			    bool is_u32)
+{
+	unsigned int full_words = size / 32;
+	const u32 *src32 = src;
+
+	if (!is_u32) {
+		bitmap_to_arr32(dst, src, size);
+		return;
+	}
+
+	memcpy(dst, src32, full_words * sizeof(u32));
+	if (size % 32 != 0)
+		dst[full_words] = src32[full_words] & ((1U << (size % 32)) - 1);
+}
+
+/* convert standard kernel bitmap (long sized words) to ethtool one (u32 words)
+ * bitmap_to_arr32() is not guaranteed to do "in place" conversion correctly;
+ * moreover, we can use the fact that the conversion is no-op except for 64-bit
+ * big endian architectures
+ */
+#if BITS_PER_LONG == 64 && defined(__BIG_ENDIAN)
+void ethnl_bitmap_to_u32(unsigned long *bitmap, unsigned int nwords)
+{
+	u32 *dst = (u32 *)bitmap;
+	unsigned int i;
+
+	for (i = 0; i < nwords; i++) {
+		unsigned long tmp = READ_ONCE(bitmap[i]);
+
+		dst[2 * i] = tmp & 0xffffffff;
+		dst[2 * i + 1] = tmp >> 32;
+	}
+}
+#endif
+
+static const char *bit_name(const void *const names, bool legacy,
+			    unsigned int idx)
+{
+	const char (*const legacy_names)[ETH_GSTRING_LEN] =
+	       (const char (*const)[ETH_GSTRING_LEN])names;
+	const char *const *simple_names = names;
+
+	return legacy ? legacy_names[idx] : simple_names[idx];
+}
+
+/* calculate size for a bitset attribute
+ * see ethnl_put_bitset() for arguments
+ */
+static int __ethnl_bitset_size(unsigned int size, const void *val,
+			       const void *mask, const void *names,
+			       unsigned int flags)
+{
+	const bool legacy = flags & ETHNL_BITSET_LEGACY_NAMES;
+	const bool compact = flags & ETHNL_BITSET_COMPACT;
+	const bool is_list = flags & ETHNL_BITSET_LIST;
+	const bool is_u32 = flags & ETHNL_BITSET_U32;
+	unsigned int nwords = DIV_ROUND_UP(size, 32);
+	unsigned int len = 0;
+
+	if (WARN_ON(!compact && !names))
+		return -EINVAL;
+	/* list flag */
+	if (flags & ETHNL_BITSET_LIST)
+		len += nla_total_size(sizeof(u32));
+	/* size */
+	len += nla_total_size(sizeof(u32));
+
+	if (compact) {
+		/* values, mask */
+		len += 2 * nla_total_size(nwords * sizeof(u32));
+	} else {
+		unsigned int bits_len = 0;
+		unsigned int bit_len, i;
+
+		for (i = 0; i < size; i++) {
+			const char *name = bit_name(names, legacy, i) ?: "";
+
+			if ((is_list || mask) &&
+			    !ethnl_test_bit(is_list ? val : mask, i, is_u32))
+				continue;
+			/* index */
+			bit_len = nla_total_size(sizeof(u32));
+			/* name */
+			bit_len += ethnl_str_size(name);
+			/* value */
+			if (!is_list && ethnl_test_bit(val, i, is_u32))
+				bit_len += nla_total_size(0);
+
+			/* bit nest */
+			bits_len += nla_total_size(bit_len);
+		}
+		/* bits nest */
+		len += nla_total_size(bits_len);
+	}
+
+	/* outermost nest */
+	return nla_total_size(len);
+}
+
+int ethnl_bitset_size(unsigned int size, const unsigned long *val,
+		      const unsigned long *mask, const void *names,
+		      unsigned int flags)
+{
+	return __ethnl_bitset_size(size, val, mask, names,
+				   flags & ~ETHNL_BITSET_U32);
+}
+
+int ethnl_bitset32_size(unsigned int size, const u32 *val, const u32 *mask,
+			const void *names, unsigned int flags)
+{
+	return __ethnl_bitset_size(size, val, mask, names,
+				   flags | ETHNL_BITSET_U32);
+}
+
+/**
+ * __ethnl_put_bitset() - Put a bitset nest into a message
+ * @skb:      skb with the message
+ * @attrtype: attribute type for the bitset nest
+ * @size:     size of the set in bits
+ * @val:      bitset values
+ * @mask:     mask of valid bits; NULL is interpreted as "all bits"
+ * @names:    bit names (only used for verbose format)
+ * @flags:    combination of ETHNL_BITSET_* flags
+ *
+ * This is the actual implementation of putting a bitset nested attribute into
+ * a netlink message but callers are supposed to use either ethnl_put_bitset()
+ * for unsigned long based bitmaps or ethnl_put_bitset32() for u32 based ones.
+ * Cleans the nest up on error.
+ *
+ * Return:    0 on success, negative error value on error
+ */
+static int __ethnl_put_bitset(struct sk_buff *skb, int attrtype,
+			      unsigned int size, const void *val,
+			      const void *mask, const void *names,
+			      unsigned int flags)
+{
+	const bool legacy = flags & ETHNL_BITSET_LEGACY_NAMES;
+	const bool compact = flags & ETHNL_BITSET_COMPACT;
+	const bool is_list = flags & ETHNL_BITSET_LIST;
+	const bool is_u32 = flags & ETHNL_BITSET_U32;
+	struct nlattr *nest;
+	struct nlattr *attr;
+
+	if (WARN_ON(!compact && !names))
+		return -EINVAL;
+	nest = nla_nest_start(skb, attrtype);
+	if (!nest)
+		return -EMSGSIZE;
+
+	if (is_list && nla_put_flag(skb, ETHTOOL_A_BITSET_LIST))
+		goto nla_put_failure;
+	if (nla_put_u32(skb, ETHTOOL_A_BITSET_SIZE, size))
+		goto nla_put_failure;
+	if (compact) {
+		unsigned int bytesize = DIV_ROUND_UP(size, 32) * sizeof(u32);
+
+		attr = nla_reserve(skb, ETHTOOL_A_BITSET_VALUE, bytesize);
+		if (!attr)
+			goto nla_put_failure;
+		__bitmap_to_u32(nla_data(attr), val, size, is_u32);
+		if (mask) {
+			attr = nla_reserve(skb, ETHTOOL_A_BITSET_MASK,
+					   bytesize);
+			if (!attr)
+				goto nla_put_failure;
+			__bitmap_to_u32(nla_data(attr), mask, size, is_u32);
+		}
+	} else {
+		struct nlattr *bits;
+		unsigned int i;
+
+		bits = nla_nest_start(skb, ETHTOOL_A_BITSET_BITS);
+		if (!bits)
+			goto nla_put_failure;
+		for (i = 0; i < size; i++) {
+			const char *name = bit_name(names, legacy, i) ?: "";
+
+			if ((is_list || mask) &&
+			    !ethnl_test_bit(is_list ? val : mask, i, is_u32))
+				continue;
+			attr = nla_nest_start(skb, ETHTOOL_A_BITS_BIT);
+			if (!attr ||
+			    nla_put_u32(skb, ETHTOOL_A_BIT_INDEX, i) ||
+			    nla_put_string(skb, ETHTOOL_A_BIT_NAME, name))
+				goto nla_put_failure;
+			if (!is_list && ethnl_test_bit(val, i, is_u32) &&
+			    nla_put_flag(skb, ETHTOOL_A_BIT_VALUE))
+				goto nla_put_failure;
+			nla_nest_end(skb, attr);
+		}
+		nla_nest_end(skb, bits);
+	}
+
+	nla_nest_end(skb, nest);
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -EMSGSIZE;
+}
+
+int ethnl_put_bitset(struct sk_buff *skb, int attrtype, unsigned int size,
+		     const unsigned long *val, const unsigned long *mask,
+		     const void *names, unsigned int flags)
+{
+	return __ethnl_put_bitset(skb, attrtype, size, val, mask, names,
+				  flags & ~ETHNL_BITSET_U32);
+}
+
+int ethnl_put_bitset32(struct sk_buff *skb, int attrtype, unsigned int size,
+		       const u32 *val, const u32 *mask, const void *names,
+		       unsigned int flags)
+{
+	return __ethnl_put_bitset(skb, attrtype, size, val, mask, names,
+				  flags | ETHNL_BITSET_U32);
+}
+
+static const struct nla_policy bitset_policy[ETHTOOL_A_BITSET_MAX + 1] = {
+	[ETHTOOL_A_BITSET_UNSPEC]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_BITSET_LIST]		= { .type = NLA_FLAG },
+	[ETHTOOL_A_BITSET_SIZE]		= { .type = NLA_U32 },
+	[ETHTOOL_A_BITSET_BITS]		= { .type = NLA_NESTED },
+	[ETHTOOL_A_BITSET_VALUE]	= { .type = NLA_BINARY },
+	[ETHTOOL_A_BITSET_MASK]		= { .type = NLA_BINARY },
+};
+
+static const struct nla_policy bit_policy[ETHTOOL_A_BIT_MAX + 1] = {
+	[ETHTOOL_A_BIT_UNSPEC]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_BIT_INDEX]		= { .type = NLA_U32 },
+	[ETHTOOL_A_BIT_NAME]		= { .type = NLA_NUL_STRING },
+	[ETHTOOL_A_BIT_VALUE]		= { .type = NLA_FLAG },
+};
+
+static int ethnl_name_to_idx(const void *names, bool legacy,
+			     unsigned int n_names, const char *name,
+			     unsigned int name_len)
+{
+	unsigned int i;
+
+	for (i = 0; i < n_names; i++) {
+		const char *bname = bit_name(names, legacy, i);
+
+		if (bname && !strncmp(bname, name, name_len) &&
+		    strlen(bname) <= name_len)
+			return i;
+	}
+
+	return n_names;
+}
+
+static int ethnl_update_bit(unsigned long *bitmap, unsigned long *bitmask,
+			    unsigned int nbits, const struct nlattr *bit_attr,
+			    bool is_list, const void *names, bool legacy,
+			    struct genl_info *info)
+{
+	struct nlattr *tb[ETHTOOL_A_BIT_MAX + 1];
+	int ret, idx;
+
+	if (nla_type(bit_attr) != ETHTOOL_A_BITS_BIT) {
+		NL_SET_ERR_MSG_ATTR(info->extack, bit_attr,
+				    "ETHTOOL_A_BITSET_BITS can contain only ETHTOOL_A_BITS_BIT");
+		return -EINVAL;
+	}
+	ret = nla_parse_nested(tb, ETHTOOL_A_BIT_MAX, bit_attr, bit_policy,
+			       info->extack);
+	if (ret < 0)
+		return ret;
+
+	if (tb[ETHTOOL_A_BIT_INDEX]) {
+		const char *name;
+
+		idx = nla_get_u32(tb[ETHTOOL_A_BIT_INDEX]);
+		if (idx >= nbits) {
+			NL_SET_ERR_MSG_ATTR(info->extack,
+					    tb[ETHTOOL_A_BIT_INDEX],
+					    "bit index too high");
+			return -EOPNOTSUPP;
+		}
+		name = bit_name(names, legacy, idx);
+		if (tb[ETHTOOL_A_BIT_NAME] && name &&
+		    strncmp(nla_data(tb[ETHTOOL_A_BIT_NAME]), name,
+			    nla_len(tb[ETHTOOL_A_BIT_NAME]))) {
+			NL_SET_ERR_MSG_ATTR(info->extack, bit_attr,
+					    "bit index and name mismatch");
+			return -EINVAL;
+		}
+	} else if (tb[ETHTOOL_A_BIT_NAME]) {
+		idx = ethnl_name_to_idx(names, legacy, nbits,
+					nla_data(tb[ETHTOOL_A_BIT_NAME]),
+					nla_len(tb[ETHTOOL_A_BIT_NAME]));
+		if (idx >= nbits) {
+			NL_SET_ERR_MSG_ATTR(info->extack,
+					    tb[ETHTOOL_A_BIT_NAME],
+					    "bit name not found");
+			return -EOPNOTSUPP;
+		}
+	} else {
+		NL_SET_ERR_MSG_ATTR(info->extack, bit_attr,
+				    "neither bit index nor name specified");
+		return -EINVAL;
+	}
+
+	if (is_list || tb[ETHTOOL_A_BIT_VALUE])
+		set_bit(idx, bitmap);
+	else
+		clear_bit(idx, bitmap);
+	if (!is_list || bitmask)
+		set_bit(idx, bitmask);
+	return 0;
+}
+
+int ethnl_bitset_is_compact(const struct nlattr *bitset, bool *compact)
+{
+	struct nlattr *tb[ETHTOOL_A_BITSET_MAX + 1];
+	int ret;
+
+	ret = nla_parse_nested(tb, ETHTOOL_A_BITSET_MAX, bitset,
+			       bitset_policy, NULL);
+	if (ret < 0)
+		return ret;
+
+	if (tb[ETHTOOL_A_BITSET_BITS]) {
+		if (tb[ETHTOOL_A_BITSET_VALUE] || tb[ETHTOOL_A_BITSET_MASK])
+			return -EINVAL;
+		*compact = false;
+		return 0;
+	}
+	if (!tb[ETHTOOL_A_BITSET_SIZE] || !tb[ETHTOOL_A_BITSET_VALUE])
+		return -EINVAL;
+
+	*compact = true;
+	return 0;
+}
+
+/* 64-bit long endian is the only case when u32 based bitmap and unsigned long
+ * based bitmap layouts differ
+ */
+#if BITS_PER_LONG == 64 && defined(__BIG_ENDIAN)
+/* dst &= src */
+static void __bitmap_and_u32(unsigned long *dst, const u32 *src,
+			     unsigned int nbits)
+{
+	unsigned long op;
+
+	while (nbits >= BITS_PER_LONG) {
+		op = src[0] | ((unsigned long)src[1] << 32);
+		*dst &= op;
+
+		dst++;
+		src += 2;
+		nbits -= BITS_PER_LONG;
+	}
+
+	if (!nbits)
+		return;
+	op = src[0];
+	if (nbits > 32)
+		op |= ((unsigned long)src[1] << 32);
+	*dst = (op & BITMAP_LAST_WORD_MASK(nbits));
+}
+
+/* map1 == map2 */
+static bool __bitmap_equal_u32(const unsigned long *map1, const u32 *map2,
+			       unsigned int nbits)
+{
+	unsigned long dword;
+
+	while (nbits >= BITS_PER_LONG) {
+		dword = map2[0] | ((unsigned long)map2[1] << 32);
+		if (*map1 != dword)
+			return false;
+
+		map1++;
+		map2 += 2;
+		nbits -= BITS_PER_LONG;
+	}
+
+	if (!nbits)
+		return true;
+	dword = map2[0];
+	if (nbits > 32)
+		dword |= ((unsigned long)map2[1] << 32);
+	return !((*map1 ^ dword) & BITMAP_LAST_WORD_MASK(nbits));
+}
+#else
+/* On 32-bit and 64-bit LE, unsigned long and u32 bitmap layout is the same
+ * but we must not write past dst buffer if the number of words is odd.
+ */
+static void __bitmap_and_u32(unsigned long *dst, const u32 *src,
+			     unsigned int nbits)
+{
+	u32 *dst32 = (u32 *)dst;
+
+	while (nbits >= 32) {
+		*dst32++ &= *src++;
+		nbits -= 32;
+	}
+	if (!nbits)
+		return;
+	*dst32 &= (*src & ((1U << nbits) - 1));
+}
+
+static bool __bitmap_equal_u32(const unsigned long *map1, const u32 *map2,
+			       unsigned int nbits)
+{
+	unsigned int full_words = nbits / 32;
+	u32 last_word_mask;
+	u32 *map1_32 = (u32 *)map1;
+
+	if (memcmp(map1, map2, full_words * BITS_PER_BYTE))
+		return false;
+	if (!(nbits % 32))
+		return true;
+	last_word_mask = (1U << (nbits % 32)) - 1;
+	return !((map1_32[full_words] ^ map2[full_words]) & last_word_mask);
+}
+#endif
+
+/* copy unsigned long bitmap to unsigned long or u32 */
+static void __bitmap_to_any(void *dst, const unsigned long *src,
+			    unsigned int nbits, bool dst_is_u32)
+{
+	if (dst_is_u32)
+		bitmap_to_arr32(dst, src, nbits);
+	else
+		bitmap_copy(dst, src, nbits);
+}
+
+static bool __bitmap_equal_any(const unsigned long *map1, const void *map2,
+			       unsigned int nbits, bool is_u32)
+{
+	if (!is_u32)
+		return bitmap_equal(map1, map2, nbits);
+	else
+		return __bitmap_equal_u32(map1, map2, nbits);
+}
+
+/**
+ * __ethnl_update_bitset() - Apply a bitset nest to a bitmap
+ * @bitmap:  bitmap to update
+ * @bitmask: if not, mask from the nest is copied here
+ * @nbits:   size of the updated bitmap in bits
+ * @attr:    nest attribute to parse and apply
+ * @err:     pointer to variable to put error value (or 0 on success) to
+ * @names:   array of bit names; may be null for compact format
+ * @legacy:  true if @names is ioctl style array of char[32], false if it is
+ *           a simple array of (char *) strings
+ * @info:    genetlink info (also used for extack error reporting)
+ * @is_u32:  false: bitmaps are unsigned long based, true: u32 based bitmaps
+ *
+ * This is the actual implementation of bitset nested attribute parser but
+ * callers are supposed to use ethnl_update_bitset() for unsigned long based
+ * bitmaps or ethnl_update_bitset32() for u32 based ones.
+ *
+ * Return:   true if the bitmap contents was modified, false if not
+ */
+static bool __ethnl_update_bitset(void *bitmap, void *bitmask,
+				  unsigned int nbits, const struct nlattr *attr,
+				  int *err, const void *names, bool legacy,
+				  struct genl_info *info, bool is_u32)
+{
+	struct nlattr *tb[ETHTOOL_A_BITSET_MAX + 1];
+	unsigned int change_bits = 0;
+	unsigned int max_bits = 0;
+	unsigned long *val, *mask;
+	bool mod = false;
+	bool is_list;
+
+	*err = 0;
+	if (!attr)
+		return mod;
+	*err = nla_parse_nested(tb, ETHTOOL_A_BITSET_MAX, attr, bitset_policy,
+				info->extack);
+	if (*err < 0)
+		return mod;
+	*err = -EINVAL;
+	if (tb[ETHTOOL_A_BITSET_BITS] &&
+	    (tb[ETHTOOL_A_BITSET_VALUE] || tb[ETHTOOL_A_BITSET_MASK]))
+		return mod;
+	if (!tb[ETHTOOL_A_BITSET_BITS] &&
+	    (!tb[ETHTOOL_A_BITSET_SIZE] || !tb[ETHTOOL_A_BITSET_VALUE]))
+		return mod;
+	is_list = (tb[ETHTOOL_A_BITSET_LIST] != NULL);
+	if (is_list && tb[ETHTOOL_A_BITSET_MASK])
+		return mod;
+
+	/* To let new userspace to work with old kernel, we allow bitmaps
+	 * from userspace to be longer than kernel ones and only issue an
+	 * error if userspace actually tries to change a bit not existing
+	 * in kernel.
+	 */
+	if (tb[ETHTOOL_A_BITSET_SIZE])
+		change_bits = nla_get_u32(tb[ETHTOOL_A_BITSET_SIZE]);
+	max_bits = max_t(unsigned int, nbits, change_bits);
+	mask = bitmap_zalloc(max_bits, GFP_KERNEL);
+	val = bitmap_zalloc(max_bits, GFP_KERNEL);
+
+	if (tb[ETHTOOL_A_BITSET_BITS]) {
+		struct nlattr *bit_attr;
+		int rem;
+
+		if (is_list)
+			bitmap_fill(mask, nbits);
+		else if (is_u32)
+			bitmap_from_arr32(val, bitmap, nbits);
+		else
+			bitmap_copy(val, bitmap, nbits);
+		nla_for_each_nested(bit_attr, tb[ETHTOOL_A_BITSET_BITS], rem) {
+			*err = ethnl_update_bit(val, mask, nbits, bit_attr,
+						is_list, names, legacy, info);
+			if (*err < 0)
+				goto out;
+		}
+		if (bitmask)
+			__bitmap_to_any(bitmask, mask, nbits, is_u32);
+	} else {
+		unsigned int change_words = DIV_ROUND_UP(change_bits, 32);
+
+		*err = 0;
+		if (change_bits == 0 && tb[ETHTOOL_A_BITSET_MASK])
+			goto out;
+		*err = -EINVAL;
+		if (nla_len(tb[ETHTOOL_A_BITSET_VALUE]) <
+		    change_words * sizeof(u32))
+			goto out;
+		if (tb[ETHTOOL_A_BITSET_MASK] &&
+		    nla_len(tb[ETHTOOL_A_BITSET_MASK]) <
+		    change_words * sizeof(u32))
+			goto out;
+
+		bitmap_from_arr32(val, nla_data(tb[ETHTOOL_A_BITSET_VALUE]),
+				  change_bits);
+		if (tb[ETHTOOL_A_BITSET_MASK])
+			bitmap_from_arr32(mask,
+					  nla_data(tb[ETHTOOL_A_BITSET_MASK]),
+					  change_bits);
+		else
+			bitmap_fill(mask, nbits);
+
+		if (nbits < change_bits) {
+			unsigned int idx = find_next_bit(mask, max_bits, nbits);
+
+			*err = -EINVAL;
+			if (idx < max_bits)
+				goto out;
+		}
+
+		if (bitmask)
+			__bitmap_to_any(bitmask, mask, nbits, is_u32);
+		if (!is_list) {
+			bitmap_and(val, val, mask, nbits);
+			bitmap_complement(mask, mask, nbits);
+			if (is_u32)
+				__bitmap_and_u32(mask, bitmap, nbits);
+			else
+				bitmap_and(mask, mask, bitmap, nbits);
+			bitmap_or(val, val, mask, nbits);
+		}
+	}
+
+	mod = !__bitmap_equal_any(val, bitmap, nbits, is_u32);
+	if (mod)
+		__bitmap_to_any(bitmap, val, nbits, is_u32);
+
+	*err = 0;
+out:
+	bitmap_free(val);
+	bitmap_free(mask);
+	return mod;
+}
+
+bool ethnl_update_bitset(unsigned long *bitmap, unsigned long *bitmask,
+			 unsigned int nbits, const struct nlattr *attr,
+			 int *err, const void *names, bool legacy,
+			 struct genl_info *info)
+{
+	return __ethnl_update_bitset(bitmap, bitmask, nbits, attr, err, names,
+				     legacy, info, false);
+}
+
+bool ethnl_update_bitset32(u32 *bitmap, u32 *bitmask, unsigned int nbits,
+			   const struct nlattr *attr, int *err,
+			   const void *names, bool legacy,
+			   struct genl_info *info)
+{
+	return __ethnl_update_bitset(bitmap, bitmask, nbits, attr, err, names,
+				     legacy, info, true);
+}
diff --git a/net/ethtool/bitset.h b/net/ethtool/bitset.h
new file mode 100644
index 000000000000..761d0c47fe23
--- /dev/null
+++ b/net/ethtool/bitset.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _NET_ETHTOOL_BITSET_H
+#define _NET_ETHTOOL_BITSET_H
+
+/* when set, value and mask bitmaps are arrays of u32, when not, arrays of
+ * unsigned long
+ */
+#define ETHNL_BITSET_U32		BIT(0)
+/* generate a compact format bitset */
+#define ETHNL_BITSET_COMPACT		BIT(1)
+/* generate a bit list */
+#define ETHNL_BITSET_LIST		BIT(2)
+/* when set, names are interpreted as legacy string set (an array of
+ * char[ETH_GSTRING_LEN]), when not, as a simple array of char *
+ */
+#define ETHNL_BITSET_LEGACY_NAMES	BIT(3)
+
+int ethnl_bitset_is_compact(const struct nlattr *bitset, bool *compact);
+int ethnl_bitset_size(unsigned int size, const unsigned long *val,
+		      const unsigned long *mask, const void *names,
+		      unsigned int flags);
+int ethnl_bitset32_size(unsigned int size, const u32 *val, const u32 *mask,
+			const void *names, unsigned int flags);
+int ethnl_put_bitset(struct sk_buff *skb, int attrtype, unsigned int size,
+		     const unsigned long *val, const unsigned long *mask,
+		     const void *names, unsigned int flags);
+int ethnl_put_bitset32(struct sk_buff *skb, int attrtype, unsigned int size,
+		       const u32 *val, const u32 *mask, const void *names,
+		       unsigned int flags);
+bool ethnl_update_bitset(unsigned long *bitmap, unsigned long *bitmask,
+			 unsigned int nbits, const struct nlattr *attr,
+			 int *err, const void *names, bool legacy,
+			 struct genl_info *info);
+bool ethnl_update_bitset32(u32 *bitmap, u32 *bitmask, unsigned int nbits,
+			   const struct nlattr *attr, int *err,
+			   const void *names, bool legacy,
+			   struct genl_info *info);
+
+#endif /* _NET_ETHTOOL_BITSET_H */
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 5510eb7054b3..7f1b9ec1ace7 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -20,6 +20,15 @@ struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
 				 u16 hdr_attrtype, struct genl_info *info,
 				 void **ehdrp);
 
+#if BITS_PER_LONG == 64 && defined(__BIG_ENDIAN)
+void ethnl_bitmap_to_u32(unsigned long *bitmap, unsigned int nwords);
+#else
+static inline void ethnl_bitmap_to_u32(unsigned long *bitmap,
+				       unsigned int nwords)
+{
+}
+#endif
+
 static inline int ethnl_str_size(const char *s)
 {
 	return nla_total_size(strlen(s) + 1);
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 05/15] ethtool: helper functions for netlink interface
From: Michal Kubecek @ 2019-07-02 11:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Add common request/reply header definition and helpers to parse request
header and fill reply header. Provide ethnl_update_* helpers to update
structure members from request attributes (to be used for *_SET requests).

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 include/uapi/linux/ethtool_netlink.h |  23 ++++
 net/ethtool/netlink.c                | 173 +++++++++++++++++++++++++++
 net/ethtool/netlink.h                | 145 ++++++++++++++++++++++
 3 files changed, 341 insertions(+)

diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 9a0fbd4f85d9..ffd7db0848ef 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -29,6 +29,29 @@ enum {
 	ETHTOOL_MSG_KERNEL_MAX = (__ETHTOOL_MSG_KERNEL_CNT - 1)
 };
 
+/* request header */
+
+/* use compact bitsets in reply */
+#define ETHTOOL_RF_COMPACT		(1 << 0)
+/* provide optional reply for SET or ACT requests */
+#define ETHTOOL_RF_REPLY		(1 << 1)
+
+#define ETHTOOL_RF_ALL (ETHTOOL_RF_COMPACT | \
+			ETHTOOL_RF_REPLY)
+
+enum {
+	ETHTOOL_A_HEADER_UNSPEC,
+	ETHTOOL_A_HEADER_DEV_INDEX,		/* u32 */
+	ETHTOOL_A_HEADER_DEV_NAME,		/* string */
+	ETHTOOL_A_HEADER_INFOMASK,		/* u32 */
+	ETHTOOL_A_HEADER_GFLAGS,		/* u32 - ETHTOOL_RF_* */
+	ETHTOOL_A_HEADER_RFLAGS,		/* u32 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_HEADER_CNT,
+	ETHTOOL_A_HEADER_MAX = (__ETHTOOL_A_HEADER_CNT - 1)
+};
+
 /* generic netlink info */
 #define ETHTOOL_GENL_NAME "ethtool"
 #define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 3c98b41f04e5..e13f29bbd625 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -1,8 +1,181 @@
 // SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
 
+#include <net/sock.h>
 #include <linux/ethtool_netlink.h>
 #include "netlink.h"
 
+static struct genl_family ethtool_genl_family;
+
+static const struct nla_policy dflt_header_policy[ETHTOOL_A_HEADER_MAX + 1] = {
+	[ETHTOOL_A_HEADER_UNSPEC]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_HEADER_DEV_INDEX]	= { .type = NLA_U32 },
+	[ETHTOOL_A_HEADER_DEV_NAME]	= { .type = NLA_NUL_STRING,
+					    .len = IFNAMSIZ - 1 },
+	[ETHTOOL_A_HEADER_INFOMASK]	= { .type = NLA_U32 },
+	[ETHTOOL_A_HEADER_GFLAGS]	= { .type = NLA_U32 },
+	[ETHTOOL_A_HEADER_RFLAGS]	= { .type = NLA_U32 },
+};
+
+/**
+ * ethnl_parse_header() - parse request header
+ * @req_info:    structure to put results into
+ * @nest:        nest attribute with request header
+ * @net:         request netns
+ * @extack:      netlink extack for error reporting
+ * @policy:      netlink attribute policy to validate header; use
+ *               @dflt_header_policy (all attributes allowed) if null
+ * @require_dev: fail if no device identiified in header
+ *
+ * Parse request header in nested attribute @nest and puts results into
+ * the structure pointed to by @req_info. Extack from @info is used for error
+ * reporting. If req_info->dev is not null on return, reference to it has
+ * been taken. If error is returned, *req_info is null initialized and no
+ * reference is held.
+ *
+ * Return: 0 on success or negative error code
+ */
+int ethnl_parse_header(struct ethnl_req_info *req_info,
+		       const struct nlattr *nest, struct net *net,
+		       struct netlink_ext_ack *extack,
+		       const struct nla_policy *policy, bool require_dev)
+{
+	struct nlattr *tb[ETHTOOL_A_HEADER_MAX + 1];
+	const struct nlattr *devname_attr;
+	struct net_device *dev = NULL;
+	int ret;
+
+	if (!nest) {
+		NL_SET_ERR_MSG(extack, "request header missing");
+		return -EINVAL;
+	}
+	ret = nla_parse_nested(tb, ETHTOOL_A_HEADER_MAX, nest,
+			       policy ?: dflt_header_policy, extack);
+	if (ret < 0)
+		return ret;
+	devname_attr = tb[ETHTOOL_A_HEADER_DEV_NAME];
+
+	if (tb[ETHTOOL_A_HEADER_DEV_INDEX]) {
+		u32 ifindex = nla_get_u32(tb[ETHTOOL_A_HEADER_DEV_INDEX]);
+
+		dev = dev_get_by_index(net, ifindex);
+		if (!dev) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[ETHTOOL_A_HEADER_DEV_INDEX],
+					    "no device matches ifindex");
+			return -ENODEV;
+		}
+		/* if both ifindex and ifname are passed, they must match */
+		if (devname_attr &&
+		    strncmp(dev->name, nla_data(devname_attr), IFNAMSIZ)) {
+			dev_put(dev);
+			NL_SET_ERR_MSG_ATTR(extack, nest,
+					    "ifindex and name do not match");
+			return -ENODEV;
+		}
+	} else if (devname_attr) {
+		dev = dev_get_by_name(net, nla_data(devname_attr));
+		if (!dev) {
+			NL_SET_ERR_MSG_ATTR(extack, devname_attr,
+					    "no device matches name");
+			return -ENODEV;
+		}
+	} else if (require_dev) {
+		NL_SET_ERR_MSG_ATTR(extack, nest,
+				    "neither ifindex nor name specified");
+		return -EINVAL;
+	}
+
+	if (dev && !netif_device_present(dev)) {
+		dev_put(dev);
+		NL_SET_ERR_MSG(extack, "device not present");
+		return -ENODEV;
+	}
+
+	req_info->dev = dev;
+	ethnl_update_u32(&req_info->req_mask, tb[ETHTOOL_A_HEADER_INFOMASK]);
+	ethnl_update_u32(&req_info->global_flags, tb[ETHTOOL_A_HEADER_GFLAGS]);
+	ethnl_update_u32(&req_info->req_flags, tb[ETHTOOL_A_HEADER_RFLAGS]);
+
+	return 0;
+}
+
+/**
+ * ethnl_fill_reply_header() - Put standard header into a reply message
+ * @skb:      skb with the message
+ * @dev:      network device to describe in header
+ * @attrtype: attribute type to use for the nest
+ *
+ * Create a nested attribute with attributes describing given network device.
+ * Clean up on error.
+ *
+ * Return: 0 on success, error value (-EMSGSIZE only) on error
+ */
+int ethnl_fill_reply_header(struct sk_buff *skb, struct net_device *dev,
+			    u16 attrtype)
+{
+	struct nlattr *nest;
+
+	if (!dev)
+		return 0;
+	nest = nla_nest_start(skb, attrtype);
+	if (!nest)
+		return -EMSGSIZE;
+
+	if (nla_put_u32(skb, ETHTOOL_A_HEADER_DEV_INDEX, (u32)dev->ifindex) ||
+	    nla_put_string(skb, ETHTOOL_A_HEADER_DEV_NAME, dev->name))
+		goto nla_put_failure;
+	/* If more attributes are put into reply header, ethnl_header_size()
+	 * must be updated to account for them.
+	 */
+
+	nla_nest_end(skb, nest);
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(skb, nest);
+	return -EMSGSIZE;
+}
+
+/**
+ * ethnl_reply_init() - Create skb for a reply and fill device identification
+ * @payload: payload length (without netlink and genetlink header)
+ * @dev:     device the reply is about (may be null)
+ * @cmd:     ETHTOOL_MSG_* message type for reply
+ * @info:    genetlink info of the received packet we respond to
+ * @ehdrp:   place to store payload pointer returned by genlmsg_new()
+ *
+ * Return: pointer to allocated skb on success, NULL on error
+ */
+struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
+				 u16 hdr_attrtype, struct genl_info *info,
+				 void **ehdrp)
+{
+	struct sk_buff *skb;
+
+	skb = genlmsg_new(payload, GFP_KERNEL);
+	if (!skb)
+		goto err;
+	*ehdrp = genlmsg_put_reply(skb, info, &ethtool_genl_family, 0, cmd);
+	if (!*ehdrp)
+		goto err_free;
+
+	if (dev) {
+		int ret;
+
+		ret = ethnl_fill_reply_header(skb, dev, hdr_attrtype);
+		if (ret < 0)
+			goto err;
+	}
+	return skb;
+
+err_free:
+	nlmsg_free(skb);
+	if (info)
+		GENL_SET_ERR_MSG(info, "failed to setup reply message");
+err:
+	return NULL;
+}
+
 /* genetlink setup */
 
 static const struct genl_ops ethtool_genl_ops[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 257ae55ccc82..5510eb7054b3 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -6,5 +6,150 @@
 #include <linux/ethtool_netlink.h>
 #include <linux/netdevice.h>
 #include <net/genetlink.h>
+#include <net/sock.h>
+
+struct ethnl_req_info;
+
+int ethnl_parse_header(struct ethnl_req_info *req_info,
+		       const struct nlattr *nest, struct net *net,
+		       struct netlink_ext_ack *extack,
+		       const struct nla_policy *policy, bool require_dev);
+int ethnl_fill_reply_header(struct sk_buff *skb, struct net_device *dev,
+			    u16 attrtype);
+struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
+				 u16 hdr_attrtype, struct genl_info *info,
+				 void **ehdrp);
+
+static inline int ethnl_str_size(const char *s)
+{
+	return nla_total_size(strlen(s) + 1);
+}
+
+/* The ethnl_update_* helpers set value pointed to by @dst to the value of
+ * netlink attribute @attr (if attr is not null). They return true if *dst
+ * value was changed, false if not.
+ */
+static inline bool ethnl_update_u32(u32 *dst, struct nlattr *attr)
+{
+	u32 val;
+
+	if (!attr)
+		return false;
+	val = nla_get_u32(attr);
+	if (*dst == val)
+		return false;
+
+	*dst = val;
+	return true;
+}
+
+static inline bool ethnl_update_u8(u8 *dst, struct nlattr *attr)
+{
+	u8 val;
+
+	if (!attr)
+		return false;
+	val = nla_get_u8(attr);
+	if (*dst == val)
+		return false;
+
+	*dst = val;
+	return true;
+}
+
+/* update u32 value used as bool from NLA_U8 attribute */
+static inline bool ethnl_update_bool32(u32 *dst, struct nlattr *attr)
+{
+	u8 val;
+
+	if (!attr)
+		return false;
+	val = !!nla_get_u8(attr);
+	if (!!*dst == val)
+		return false;
+
+	*dst = val;
+	return true;
+}
+
+static inline bool ethnl_update_binary(u8 *dst, unsigned int len,
+				       struct nlattr *attr)
+{
+	if (!attr)
+		return false;
+	if (nla_len(attr) < len)
+		len = nla_len(attr);
+	if (!memcmp(dst, nla_data(attr), len))
+		return false;
+
+	memcpy(dst, nla_data(attr), len);
+	return true;
+}
+
+static inline bool ethnl_update_bitfield32(u32 *dst, struct nlattr *attr)
+{
+	struct nla_bitfield32 change;
+	u32 newval;
+
+	if (!attr)
+		return false;
+	change = nla_get_bitfield32(attr);
+	newval = (*dst & ~change.selector) | (change.value & change.selector);
+	if (*dst == newval)
+		return false;
+
+	*dst = newval;
+	return true;
+}
+
+/**
+ * ethnl_is_privileged() - check if request has sufficient privileges
+ * @skb: skb with client request
+ *
+ * Checks if client request has CAP_NET_ADMIN in its netns. Unlike the flags
+ * in genl_ops, this allows finer access control, e.g. allowing or denying
+ * the request based on its contents or witholding only part of the data
+ * from unprivileged users.
+ *
+ * Return: true if request is privileged, false if not
+ */
+static inline bool ethnl_is_privileged(struct sk_buff *skb)
+{
+	struct net *net = sock_net(skb->sk);
+
+	return netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN);
+}
+
+/**
+ * ethnl_reply_header_size() - total size of reply header
+ *
+ * This is an upper estimate so that we do not need to hold RTNL lock longer
+ * than necessary (to prevent rename between size estimate and composing the
+ * message). Accounts only for device ifindex and name as those are the only
+ * attributes ethnl_fill_reply_header() puts into the reply header.
+ */
+static inline unsigned int ethnl_reply_header_size(void)
+{
+	return nla_total_size(nla_total_size(sizeof(u32)) +
+			      nla_total_size(IFNAMSIZ));
+}
+
+/**
+ * struct ethnl_req_info - base type of request information for GET requests
+ * @dev:          network device the request is for (may be null)
+ * @req_mask:     request mask, bitmap of requested information
+ * @global_flags: request flags common for all request types
+ * @req_flags:    request flags specific for each request type
+ * @privileged:   privileged request (CAP_NET_ADMIN in netns)
+ *
+ * This is a common base, additional members may follow after this structure.
+ */
+struct ethnl_req_info {
+	struct net_device		*dev;
+	u32				req_mask;
+	u32				global_flags;
+	u32				req_flags;
+	bool				privileged;
+};
 
 #endif /* _NET_ETHTOOL_NETLINK_H */
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 04/15] ethtool: introduce ethtool netlink interface
From: Michal Kubecek @ 2019-07-02 11:49 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Basic genetlink and init infrastructure for the netlink interface, register
genetlink family "ethtool". Add CONFIG_ETHTOOL_NETLINK Kconfig option to
make the build optional. Add initial overall interface description into
Documentation/networking/ethtool-netlink.txt, further patches will add more
detailed information.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.txt | 208 +++++++++++++++++++
 include/linux/ethtool_netlink.h              |   9 +
 include/uapi/linux/ethtool_netlink.h         |  36 ++++
 net/Kconfig                                  |   8 +
 net/ethtool/Makefile                         |   6 +-
 net/ethtool/netlink.c                        |  33 +++
 net/ethtool/netlink.h                        |  10 +
 7 files changed, 309 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/networking/ethtool-netlink.txt
 create mode 100644 include/linux/ethtool_netlink.h
 create mode 100644 include/uapi/linux/ethtool_netlink.h
 create mode 100644 net/ethtool/netlink.c
 create mode 100644 net/ethtool/netlink.h

diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
new file mode 100644
index 000000000000..97c369aa290b
--- /dev/null
+++ b/Documentation/networking/ethtool-netlink.txt
@@ -0,0 +1,208 @@
+                        Netlink interface for ethtool
+                        =============================
+
+
+Basic information
+-----------------
+
+Netlink interface for ethtool uses generic netlink family "ethtool" (userspace
+application should use macros ETHTOOL_GENL_NAME and ETHTOOL_GENL_VERSION
+defined in <linux/ethtool_netlink.h> uapi header). This family does not use
+a specific header, all information in requests and replies is passed using
+netlink attributes.
+
+The ethtool netlink interface uses extended ACK for error and warning
+reporting, userspace application developers are encouraged to make these
+messages available to user in a suitable way.
+
+Requests can be divided into three categories: "get" (retrieving information),
+"set" (setting parameters) and "action" (invoking an action).
+
+All "set" and "action" type requests require admin privileges (CAP_NET_ADMIN
+in the namespace). Most "get" type requests are allowed for anyone but there
+are exceptions (where the response contains sensitive information). In some
+cases, the request as such is allowed for anyone but unprivileged users have
+attributes with sensitive information (e.g. wake-on-lan password) omitted.
+
+
+Conventions
+-----------
+
+Attributes which represent a boolean value usually use u8 type so that we can
+distinguish three states: "on", "off" and "not present" (meaning the
+information is not available in "get" requests or value is not to be changed
+in "set" requests). For these attributes, the "true" value should be passed as
+number 1 but any non-zero value should be understood as "true" by recipient.
+
+In the message structure descriptions below, if an attribute name is suffixed
+with "+", parent nest can contain multiple attributes of the same type. This
+implements an array of entries.
+
+
+Request header
+--------------
+
+Each request or reply message contains a nested attribute with common header.
+Structure of this header is
+
+    ETHTOOL_A_HEADER_DEV_INDEX	(u32)		device ifindex
+    ETHTOOL_A_HEADER_DEV_NAME	(string)	device name
+    ETHTOOL_A_HEADER_INFOMASK	(u32)		info mask
+    ETHTOOL_A_HEADER_GFLAGS	(u32)		flags common for all requests
+    ETHTOOL_A_HEADER_RFLAGS	(u32)		request specific flags
+
+ETHTOOL_A_HEADER_DEV_INDEX and ETHTOOL_A_HEADER_DEV_NAME identify the device
+message relates to. One of them is sufficient in requests, if both are used,
+they must identify the same device. Some requests, e.g. global string sets, do
+not require device identification. Most GET requests also allow dump requests
+without device identification to query the same information for all devices
+providing it (each device in a separate message).
+
+Optional info mask allows to ask only for a part of data provided by GET
+request types. If omitted or zero, all data is returned. The two flag bitmaps
+allow enabling requestoptions; ETHTOOL_A_HEADER_GFLAGS are global flags common
+for all request types, flags recognized in ETHTOOL_A_HEADER_RFLAGS and their
+interpretation are specific for each request type. Global flags are
+
+    ETHTOOL_RF_COMPACT		use compact format bitsets in reply
+    ETHTOOL_RF_REPLY		send optional reply (SET and ACT requests)
+
+Request specific flags are described with each request type. For both flag
+attributes, new flags should follow the general idea that if the flag is not
+set, the behaviour is the same as (or closer to) the behaviour before it was
+introduced.
+
+
+List of message types
+---------------------
+
+All constants identifying message types use ETHTOOL_CMD_ prefix and suffix
+according to message purpose:
+
+    _GET	userspace request to retrieve data
+    _SET	userspace request to set data
+    _ACT	userspace request to perform an action
+    _GET_REPLY	kernel reply to a GET request
+    _SET_REPLY	kernel reply to a SET request
+    _ACT_REPLY  kernel reply to an ACT request
+    _NTF	kernel notification
+
+"GET" requests are sent by userspace applications to retrieve device
+information. They usually do not contain any message specific attributes.
+Kernel replies with corresponding "GET_REPLY" message. For most types, "GET"
+request with NLM_F_DUMP and no device identification can be used to query the
+information for all devices supporting the request.
+
+If the data can be also modified, corresponding "SET" message with the same
+layout as "GET" reply is used to request changes. Only attributes where
+a change is requested are included in such request (also, not all attributes
+may be changed). Replies to most "SET" request consist only of error code and
+extack; if kernel provides additional data, it is sent in the form of
+corresponding "SET_REPLY" message (if ETHTOOL_RF_REPLY flag was set in request
+header).
+
+Data modification also triggers sending a "NTF" message with a notification.
+These usually bear only a subset of attributes which was affected by the
+change. The same notification is issued if the data is modified using other
+means (mostly ioctl ethtool interface). Unlike notifications from ethtool
+netlink code which are only sent if something actually changed, notifications
+triggered by ioctl interface may be sent even if the request did not actually
+change any data.
+
+"ACT" messages request kernel (driver) to perform a specific action. If some
+information is reported by kernel (as requested by ETHTOOL_RF_REPLY flag in
+request header), the reply takes form of an "ACT_REPLY" message. Performing an
+action also triggers a notification ("NTF" message).
+
+Later sections describe the format and semantics of these messages.
+
+
+Request translation
+-------------------
+
+The following table maps ioctl commands to netlink commands providing their
+functionality. Entries with "n/a" in right column are commands which do not
+have their netlink replacement yet.
+
+ioctl command			netlink command
+---------------------------------------------------------------------
+ETHTOOL_GSET			n/a
+ETHTOOL_SSET			n/a
+ETHTOOL_GDRVINFO		n/a
+ETHTOOL_GREGS			n/a
+ETHTOOL_GWOL			n/a
+ETHTOOL_SWOL			n/a
+ETHTOOL_GMSGLVL			n/a
+ETHTOOL_SMSGLVL			n/a
+ETHTOOL_NWAY_RST		n/a
+ETHTOOL_GLINK			n/a
+ETHTOOL_GEEPROM			n/a
+ETHTOOL_SEEPROM			n/a
+ETHTOOL_GCOALESCE		n/a
+ETHTOOL_SCOALESCE		n/a
+ETHTOOL_GRINGPARAM		n/a
+ETHTOOL_SRINGPARAM		n/a
+ETHTOOL_GPAUSEPARAM		n/a
+ETHTOOL_SPAUSEPARAM		n/a
+ETHTOOL_GRXCSUM			n/a
+ETHTOOL_SRXCSUM			n/a
+ETHTOOL_GTXCSUM			n/a
+ETHTOOL_STXCSUM			n/a
+ETHTOOL_GSG			n/a
+ETHTOOL_SSG			n/a
+ETHTOOL_TEST			n/a
+ETHTOOL_GSTRINGS		n/a
+ETHTOOL_PHYS_ID			n/a
+ETHTOOL_GSTATS			n/a
+ETHTOOL_GTSO			n/a
+ETHTOOL_STSO			n/a
+ETHTOOL_GPERMADDR		rtnetlink RTM_GETLINK
+ETHTOOL_GUFO			n/a
+ETHTOOL_SUFO			n/a
+ETHTOOL_GGSO			n/a
+ETHTOOL_SGSO			n/a
+ETHTOOL_GFLAGS			n/a
+ETHTOOL_SFLAGS			n/a
+ETHTOOL_GPFLAGS			n/a
+ETHTOOL_SPFLAGS			n/a
+ETHTOOL_GRXFH			n/a
+ETHTOOL_SRXFH			n/a
+ETHTOOL_GGRO			n/a
+ETHTOOL_SGRO			n/a
+ETHTOOL_GRXRINGS		n/a
+ETHTOOL_GRXCLSRLCNT		n/a
+ETHTOOL_GRXCLSRULE		n/a
+ETHTOOL_GRXCLSRLALL		n/a
+ETHTOOL_SRXCLSRLDEL		n/a
+ETHTOOL_SRXCLSRLINS		n/a
+ETHTOOL_FLASHDEV		n/a
+ETHTOOL_RESET			n/a
+ETHTOOL_SRXNTUPLE		n/a
+ETHTOOL_GRXNTUPLE		n/a
+ETHTOOL_GSSET_INFO		n/a
+ETHTOOL_GRXFHINDIR		n/a
+ETHTOOL_SRXFHINDIR		n/a
+ETHTOOL_GFEATURES		n/a
+ETHTOOL_SFEATURES		n/a
+ETHTOOL_GCHANNELS		n/a
+ETHTOOL_SCHANNELS		n/a
+ETHTOOL_SET_DUMP		n/a
+ETHTOOL_GET_DUMP_FLAG		n/a
+ETHTOOL_GET_DUMP_DATA		n/a
+ETHTOOL_GET_TS_INFO		n/a
+ETHTOOL_GMODULEINFO		n/a
+ETHTOOL_GMODULEEEPROM		n/a
+ETHTOOL_GEEE			n/a
+ETHTOOL_SEEE			n/a
+ETHTOOL_GRSSH			n/a
+ETHTOOL_SRSSH			n/a
+ETHTOOL_GTUNABLE		n/a
+ETHTOOL_STUNABLE		n/a
+ETHTOOL_GPHYSTATS		n/a
+ETHTOOL_PERQUEUE		n/a
+ETHTOOL_GLINKSETTINGS		n/a
+ETHTOOL_SLINKSETTINGS		n/a
+ETHTOOL_PHY_GTUNABLE		n/a
+ETHTOOL_PHY_STUNABLE		n/a
+ETHTOOL_GFECPARAM		n/a
+ETHTOOL_SFECPARAM		n/a
diff --git a/include/linux/ethtool_netlink.h b/include/linux/ethtool_netlink.h
new file mode 100644
index 000000000000..0412adb4f42f
--- /dev/null
+++ b/include/linux/ethtool_netlink.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _LINUX_ETHTOOL_NETLINK_H_
+#define _LINUX_ETHTOOL_NETLINK_H_
+
+#include <uapi/linux/ethtool_netlink.h>
+#include <linux/ethtool.h>
+
+#endif /* _LINUX_ETHTOOL_NETLINK_H_ */
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
new file mode 100644
index 000000000000..9a0fbd4f85d9
--- /dev/null
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * include/uapi/linux/ethtool_netlink.h - netlink interface for ethtool
+ *
+ * See Documentation/networking/ethtool-netlink.txt in kernel source tree for
+ * doucumentation of the interface.
+ */
+
+#ifndef _UAPI_LINUX_ETHTOOL_NETLINK_H_
+#define _UAPI_LINUX_ETHTOOL_NETLINK_H_
+
+#include <linux/ethtool.h>
+
+/* message types - userspace to kernel */
+enum {
+	ETHTOOL_MSG_USER_NONE,
+
+	/* add new constants above here */
+	__ETHTOOL_MSG_USER_CNT,
+	ETHTOOL_MSG_USER_MAX = (__ETHTOOL_MSG_USER_CNT - 1)
+};
+
+/* message types - kernel to userspace */
+enum {
+	ETHTOOL_MSG_KERNEL_NONE,
+
+	/* add new constants above here */
+	__ETHTOOL_MSG_KERNEL_CNT,
+	ETHTOOL_MSG_KERNEL_MAX = (__ETHTOOL_MSG_KERNEL_CNT - 1)
+};
+
+/* generic netlink info */
+#define ETHTOOL_GENL_NAME "ethtool"
+#define ETHTOOL_GENL_VERSION 1
+
+#endif /* _UAPI_LINUX_ETHTOOL_NETLINK_H_ */
diff --git a/net/Kconfig b/net/Kconfig
index 57f51a279ad6..65b760d26eec 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -447,6 +447,14 @@ config FAILOVER
 	  migration of VMs with direct attached VFs by failing over to the
 	  paravirtual datapath when the VF is unplugged.
 
+config ETHTOOL_NETLINK
+	bool "Netlink interface for ethtool"
+	default y
+	help
+	  An alternative userspace interface for ethtool based on generic
+	  netlink. It provides better extensibility and some new features,
+	  e.g. notification messages.
+
 endif   # if NET
 
 # Used by archs to tell that they support BPF JIT compiler plus which flavour.
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 3ebfab2bca66..f30e0da88be5 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -1,3 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
-obj-y		+= ioctl.o
+obj-y				+= ioctl.o
+
+obj-$(CONFIG_ETHTOOL_NETLINK)	+= ethtool_nl.o
+
+ethtool_nl-y	:= netlink.o
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
new file mode 100644
index 000000000000..3c98b41f04e5
--- /dev/null
+++ b/net/ethtool/netlink.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
+
+#include <linux/ethtool_netlink.h>
+#include "netlink.h"
+
+/* genetlink setup */
+
+static const struct genl_ops ethtool_genl_ops[] = {
+};
+
+static struct genl_family ethtool_genl_family = {
+	.name		= ETHTOOL_GENL_NAME,
+	.version	= ETHTOOL_GENL_VERSION,
+	.netnsok	= true,
+	.parallel_ops	= true,
+	.ops		= ethtool_genl_ops,
+	.n_ops		= ARRAY_SIZE(ethtool_genl_ops),
+};
+
+/* module setup */
+
+static int __init ethnl_init(void)
+{
+	int ret;
+
+	ret = genl_register_family(&ethtool_genl_family);
+	if (WARN(ret < 0, "ethtool: genetlink family registration failed"))
+		return ret;
+
+	return 0;
+}
+
+subsys_initcall(ethnl_init);
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
new file mode 100644
index 000000000000..257ae55ccc82
--- /dev/null
+++ b/net/ethtool/netlink.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _NET_ETHTOOL_NETLINK_H
+#define _NET_ETHTOOL_NETLINK_H
+
+#include <linux/ethtool_netlink.h>
+#include <linux/netdevice.h>
+#include <net/genetlink.h>
+
+#endif /* _NET_ETHTOOL_NETLINK_H */
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 03/15] ethtool: move to its own directory
From: Michal Kubecek @ 2019-07-02 11:49 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

The ethtool netlink interface is going to be split into multiple files so
that it will be more convenient to put all of them in a separate directory
net/ethtool. Start by moving current ethtool.c with ioctl interface into
this directory and renaming it to ioctl.c.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/Makefile                            | 2 +-
 net/core/Makefile                       | 2 +-
 net/ethtool/Makefile                    | 3 +++
 net/{core/ethtool.c => ethtool/ioctl.c} | 0
 4 files changed, 5 insertions(+), 2 deletions(-)
 create mode 100644 net/ethtool/Makefile
 rename net/{core/ethtool.c => ethtool/ioctl.c} (100%)

diff --git a/net/Makefile b/net/Makefile
index 449fc0b221f8..848303d98d3d 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -13,7 +13,7 @@ obj-$(CONFIG_NET)		+= $(tmp-y)
 
 # LLC has to be linked before the files in net/802/
 obj-$(CONFIG_LLC)		+= llc/
-obj-$(CONFIG_NET)		+= ethernet/ 802/ sched/ netlink/ bpf/
+obj-$(CONFIG_NET)		+= ethernet/ 802/ sched/ netlink/ bpf/ ethtool/
 obj-$(CONFIG_NETFILTER)		+= netfilter/
 obj-$(CONFIG_INET)		+= ipv4/
 obj-$(CONFIG_TLS)		+= tls/
diff --git a/net/core/Makefile b/net/core/Makefile
index a104dc8faafc..3e2c378e5f31 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -8,7 +8,7 @@ obj-y := sock.o request_sock.o skbuff.o datagram.o stream.o scm.o \
 
 obj-$(CONFIG_SYSCTL) += sysctl_net_core.o
 
-obj-y		     += dev.o ethtool.o dev_addr_lists.o dst.o netevent.o \
+obj-y		     += dev.o dev_addr_lists.o dst.o netevent.o \
 			neighbour.o rtnetlink.o utils.o link_watch.o filter.o \
 			sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \
 			fib_notifier.o xdp.o flow_offload.o
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
new file mode 100644
index 000000000000..3ebfab2bca66
--- /dev/null
+++ b/net/ethtool/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-y		+= ioctl.o
diff --git a/net/core/ethtool.c b/net/ethtool/ioctl.c
similarity index 100%
rename from net/core/ethtool.c
rename to net/ethtool/ioctl.c
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 02/15] netlink: rename nl80211_validate_nested() to nla_validate_nested()
From: Michal Kubecek @ 2019-07-02 11:49 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <cover.1562067622.git.mkubecek@suse.cz>

Function nl80211_validate_nested() is not specific to nl80211, it's
a counterpart to nla_validate_nested_deprecated() with strict validation.
For consistency with other validation and parse functions, rename it to
nla_validate_nested().

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 include/net/netlink.h  | 8 ++++----
 net/wireless/nl80211.c | 3 +--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index e4650e5b64a1..edb36bf29261 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -1736,7 +1736,7 @@ static inline void nla_nest_cancel(struct sk_buff *skb, struct nlattr *start)
 }
 
 /**
- * nla_validate_nested - Validate a stream of nested attributes
+ * __nla_validate_nested - Validate a stream of nested attributes
  * @start: container attribute
  * @maxtype: maximum attribute type to be expected
  * @policy: validation policy
@@ -1759,9 +1759,9 @@ static inline int __nla_validate_nested(const struct nlattr *start, int maxtype,
 }
 
 static inline int
-nl80211_validate_nested(const struct nlattr *start, int maxtype,
-			const struct nla_policy *policy,
-			struct netlink_ext_ack *extack)
+nla_validate_nested(const struct nlattr *start, int maxtype,
+		    const struct nla_policy *policy,
+		    struct netlink_ext_ack *extack)
 {
 	return __nla_validate_nested(start, maxtype, policy,
 				     NL_VALIDATE_STRICT, extack);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fc83dd179c1a..ac371d40530d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -12707,8 +12707,7 @@ static int nl80211_vendor_check_policy(const struct wiphy_vendor_command *vcmd,
 		return -EINVAL;
 	}
 
-	return nl80211_validate_nested(attr, vcmd->maxattr, vcmd->policy,
-				       extack);
+	return nla_validate_nested(attr, vcmd->maxattr, vcmd->policy, extack);
 }
 
 static int nl80211_vendor_cmd(struct sk_buff *skb, struct genl_info *info)
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next v6 00/15] ethtool netlink interface, part 1
From: Michal Kubecek @ 2019-07-02 11:49 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Jakub Kicinski, Jiri Pirko, Andrew Lunn, Florian Fainelli,
	John Linville, Stephen Hemminger, Johannes Berg, linux-kernel

This is first part of netlink based alternative userspace interface for
ethtool. It aims to address some long known issues with the ioctl
interface, mainly lack of extensibility, raciness, limited error reporting
and absence of notifications. The goal is to allow userspace ethtool
utility to provide all features it currently does but without using the
ioctl interface. However, some features provided by ethtool ioctl API will
be available through other netlink interfaces (rtnetlink, devlink) if it's
more appropriate.

The interface uses generic netlink family "ethtool" and provides multicast
group "monitor" which is used for notifications. Documentation for the
interface is in Documentation/networking/ethtool-netlink.txt file. The
netlink interface is optional, it is built when CONFIG_ETHTOOL_NETLINK
(bool) option is enabled.

There are three types of request messages distinguished by suffix "_GET"
(query for information), "_SET" (modify parameters) and "_ACT" (perform an
action). Kernel reply messages have name with additional suffix "_REPLY"
(e.g. ETHTOOL_MSG_SETTINGS_GET_REPLY). Most "_SET" and "_ACT" message types
do not have matching reply type as only some of them need additional reply
data beyond numeric error code and extack.

Basic concepts:

- make extensions easier not only by allowing new attributes but also by
  imposing as few artificial limits as possible, e.g. by using arbitrary
  size bit sets for most bitmap attributes or by not using fixed size
  strings
- use extack for error reporting and warnings
- send netlink notifications on changes (even if they were done using the
  ioctl interface) and actions
- avoid the racy read/modify/write cycle between kernel and userspace by
  sending only attributes which userspace wants to change; there is still
  a read/modify/write cycle between generic kernel code and ethtool_ops
  handler in NIC driver but it is only in kernel and under a lock
- reduce the number of name lists that need to be kept in sync between
  kernel and userspace (e.g. recognized link modes)
- where feasible, allow dump requests to query specific information for all
  network devices
- as the lack of extensibility of the ioctl interface led to having too
  many commands, group some of them together to one netlink message but
  allow querying only part(s) of the information (using "info mask" bitmap)
  and modifying only some of the parameters (by providing only some
  attributes)
- as parsing and generating netlink messages is more complicated than
  simply copying data structures between userspace API and ethtool_ops
  handlers (which most ioctl commands do), split the code into multiple
  files in net/ethtool directory; move net/core/ethtool.c also to this
  directory and rename it to ioctl.c

The full (work in progress) series, together with the (userspace) ethtool
counterpart can be found at https://github.com/mkubecek/ethnl

Main changes between v5 and v6:

- use ETHTOOL_MSG_ prefix for message types
- replace ETHA_ prefix for netlink attributes by ETHTOOL_A_
- replace ETH_x_IM_y for infomask bits by ETHTOOL_IM_x_y
- split GET reply types from SET requests and notifications
- split kernel and userspace message types into different enums
- remove INFO_GET requests from submitted part
- drop EVENT notifications (use rtnetlink and on-demand string set load)
- reorganize patches to reduce the number of intermitent warnings
- unify request/reply header and its processing
- another nest around strings in a string set for consistency
- more consistent identifier naming
- coding style cleanup
- get rid of some of the helpers
- set bad attribute in extack where applicable
- various bug fixes
- improve documentation and code comments, more kerneldoc comments
- more verbose commit messages

Changes between v4 and v5:

- do not panic on failed initialization, only WARN()

Main changes between RFC v3 and v4:

- use more kerneldoc style comments
- strict attribute policy checking
- use macros for tables of link mode names and parameters
- provide permanent hardware address in rtnetlink
- coding style cleanup
- split too long patches, reorder
- wrap more ETHA_SETTINGS_* attributes in nests
- add also some SET_* implementation into submitted part

Main changes between RFC v2 and RFC v3:

- do not allow building as a module (no netdev notifiers needed)
- drop some obsolete fields
- add permanent hw address, timestamping and private flags support
- rework bitset handling to get rid of variable length arrays
- notify monitor on device renames
- restructure GET_SETTINGS/SET_SETTINGS messages
- split too long patches and submit only first part of the series

Main changes between RFC v1 and RFC v2:

- support dumps for all "get" requests
- provide notifications for changes related to supported request types
- support getting string sets (both global and per device)
- support getting/setting device features
- get rid of family specific header, everything passed as attributes
- split netlink code into multiple files in net/ethtool/ directory


Michal Kubecek (15):
  rtnetlink: provide permanent hardware address in RTM_NEWLINK
  netlink: rename nl80211_validate_nested() to nla_validate_nested()
  ethtool: move to its own directory
  ethtool: introduce ethtool netlink interface
  ethtool: helper functions for netlink interface
  ethtool: netlink bitset handling
  ethtool: support for netlink notifications
  ethtool: move string arrays into common file
  ethtool: generic handlers for GET requests
  ethtool: provide string sets with STRSET_GET request
  ethtool: provide link mode names as a string set
  ethtool: provide link settings and link modes in SETTINGS_GET request
  ethtool: add standard notification handler
  ethtool: set link settings and link modes with SETTINGS_SET request
  ethtool: provide link state in SETTINGS_GET request

 Documentation/networking/ethtool-netlink.txt | 399 ++++++++++
 include/linux/ethtool.h                      |   4 +
 include/linux/ethtool_netlink.h              |  17 +
 include/linux/netdevice.h                    |  12 +
 include/net/netlink.h                        |   8 +-
 include/uapi/linux/ethtool.h                 |   4 +
 include/uapi/linux/ethtool_netlink.h         | 219 ++++++
 include/uapi/linux/if_link.h                 |   1 +
 net/Kconfig                                  |   8 +
 net/Makefile                                 |   2 +-
 net/core/Makefile                            |   2 +-
 net/core/rtnetlink.c                         |   5 +
 net/ethtool/Makefile                         |   7 +
 net/ethtool/bitset.c                         | 606 +++++++++++++++
 net/ethtool/bitset.h                         |  40 +
 net/ethtool/common.c                         | 140 ++++
 net/ethtool/common.h                         |  24 +
 net/{core/ethtool.c => ethtool/ioctl.c}      | 157 +---
 net/ethtool/netlink.c                        | 762 +++++++++++++++++++
 net/ethtool/netlink.h                        | 302 ++++++++
 net/ethtool/settings.c                       | 628 +++++++++++++++
 net/ethtool/strset.c                         | 459 +++++++++++
 net/wireless/nl80211.c                       |   3 +-
 23 files changed, 3666 insertions(+), 143 deletions(-)
 create mode 100644 Documentation/networking/ethtool-netlink.txt
 create mode 100644 include/linux/ethtool_netlink.h
 create mode 100644 include/uapi/linux/ethtool_netlink.h
 create mode 100644 net/ethtool/Makefile
 create mode 100644 net/ethtool/bitset.c
 create mode 100644 net/ethtool/bitset.h
 create mode 100644 net/ethtool/common.c
 create mode 100644 net/ethtool/common.h
 rename net/{core/ethtool.c => ethtool/ioctl.c} (93%)
 create mode 100644 net/ethtool/netlink.c
 create mode 100644 net/ethtool/netlink.h
 create mode 100644 net/ethtool/settings.c
 create mode 100644 net/ethtool/strset.c

-- 
2.22.0


^ permalink raw reply

* Re: [Patch net] xfrm: remove a duplicated assignment
From: Steffen Klassert @ 2019-07-02 11:48 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, Florian Westphal
In-Reply-To: <20190629191714.5808-1-xiyou.wangcong@gmail.com>

On Sat, Jun 29, 2019 at 12:17:14PM -0700, Cong Wang wrote:
> Fixes: 30846090a746 ("xfrm: policy: add sequence count to sync with hash resize")
> Cc: Florian Westphal <fw@strlen.de>
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied, thanks Cong!

^ permalink raw reply

* Re: [PATCH bpf-next v2 1/6] xsk: replace ndo_xsk_async_xmit with ndo_xsk_wakeup
From: Maxim Mikityanskiy @ 2019-07-02 11:42 UTC (permalink / raw)
  To: Magnus Karlsson, ast@kernel.org
  Cc: bjorn.topel@intel.com, daniel@iogearbox.net,
	netdev@vger.kernel.org, brouer@redhat.com, bpf@vger.kernel.org,
	bruce.richardson@intel.com, ciara.loftus@intel.com,
	jakub.kicinski@netronome.com, xiaolong.ye@intel.com,
	qi.z.zhang@intel.com, sridhar.samudrala@intel.com,
	kevin.laatz@intel.com, ilias.apalodimas@linaro.org,
	kiran.patil@intel.com, axboe@kernel.dk,
	maciej.fijalkowski@intel.com, maciejromanfijalkowski@gmail.com,
	intel-wired-lan@lists.osuosl.org
In-Reply-To: <1562059288-26773-2-git-send-email-magnus.karlsson@intel.com>

On 2019-07-02 12:21, Magnus Karlsson wrote:
> This commit replaces ndo_xsk_async_xmit with ndo_xsk_wakeup. This new
> ndo provides the same functionality as before but with the addition of
> a new flags field that is used to specifiy if Rx, Tx or both should be
> woken up. The previous ndo only woke up Tx, as implied by the
> name. The i40e and ixgbe drivers (which are all the supported ones)
> are updated with this new interface.

This API change will break build of mlx5 - XSK support for mlx5 was merged.

> This new ndo will be used by the new need_wakeup functionality of XDP
> sockets that need to be able to wake up both Rx and Tx driver
> processing.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>


^ permalink raw reply

* Re: [PATCH v5 net-next 6/6] net: ethernet: ti: cpsw: add XDP support
From: Ivan Khoronzhuk @ 2019-07-02 11:37 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: grygorii.strashko, davem, ast, linux-kernel, linux-omap,
	ilias.apalodimas, netdev, daniel, jakub.kicinski, john.fastabend
In-Reply-To: <20190701181901.150c0b71@carbon>

On Mon, Jul 01, 2019 at 06:19:01PM +0200, Jesper Dangaard Brouer wrote:
>On Sun, 30 Jun 2019 20:23:48 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> +static int cpsw_ndev_create_xdp_rxq(struct cpsw_priv *priv, int ch)
>> +{
>> +	struct cpsw_common *cpsw = priv->cpsw;
>> +	int ret, new_pool = false;
>> +	struct xdp_rxq_info *rxq;
>> +
>> +	rxq = &priv->xdp_rxq[ch];
>> +
>> +	ret = xdp_rxq_info_reg(rxq, priv->ndev, ch);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (!cpsw->page_pool[ch]) {
>> +		ret =  cpsw_create_rx_pool(cpsw, ch);
>> +		if (ret)
>> +			goto err_rxq;
>> +
>> +		new_pool = true;
>> +	}
>> +
>> +	ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_PAGE_POOL,
>> +					 cpsw->page_pool[ch]);
>> +	if (!ret)
>> +		return 0;
>> +
>> +	if (new_pool) {
>> +		page_pool_free(cpsw->page_pool[ch]);
>> +		cpsw->page_pool[ch] = NULL;
>> +	}
>> +
>> +err_rxq:
>> +	xdp_rxq_info_unreg(rxq);
>> +	return ret;
>> +}
>
>Looking at this, and Ilias'es XDP-netsec error handling path, it might
>be a mistake that I removed page_pool_destroy() and instead put the
>responsibility on xdp_rxq_info_unreg().
As for me this is started not from page_pool_free, but rather from calling
unreg_mem_model from rxq_info_unreg. Then, if page_pool_free is hidden
it looks more a while normal to move all chain to be self destroyed.

>
>As here, we have to detect if page_pool_create() was a success, and then
>if xdp_rxq_info_reg_mem_model() was a failure, explicitly call
>page_pool_free() because the xdp_rxq_info_unreg() call cannot "free"
>the page_pool object given it was not registered.
Yes, it looked a little bit ugly from the beginning, but, frankly,
I have got used to this already.

>
>Ivan's patch in[1], might be a better approach, which forced all
>drivers to explicitly call page_pool_free(), even-though it just
>dec-refcnt and the real call to page_pool_free() happened via
>xdp_rxq_info_unreg().
>
>To better handle error path, I would re-introduce page_pool_destroy(),
So, you might to do it later as I understand, and not for my special
case but becouse it makes error path to look a little bit more pretty.
I'm perfectly fine with this, and better you add this, for now my
implementation requires only "xdp: allow same allocator usage" patch,
but if you insist I can resend also patch in question afterwards my
series is applied (with modification to cpsw & netsec & mlx5 & page_pool).

What's your choice? I can add to your series patch needed for cpsw to
avoid some misuse.

>as a driver API, that would gracefully handle NULL-pointer case, and
>then call page_pool_free() with the atomic_dec_and_test().  (It should
>hopefully simplify the error handling code a bit)
>
>[1] https://lore.kernel.org/netdev/20190625175948.24771-2-ivan.khoronzhuk@linaro.org/
>
>
>> +void cpsw_ndev_destroy_xdp_rxqs(struct cpsw_priv *priv)
>> +{
>> +	struct cpsw_common *cpsw = priv->cpsw;
>> +	struct xdp_rxq_info *rxq;
>> +	int i;
>> +
>> +	for (i = 0; i < cpsw->rx_ch_num; i++) {
>> +		rxq = &priv->xdp_rxq[i];
>> +		if (xdp_rxq_info_is_reg(rxq))
>> +			xdp_rxq_info_unreg(rxq);
>> +	}
>> +}
>
>Are you sure you need to test xdp_rxq_info_is_reg() here?
Yes it's required in my case as it's used in error path where
an rx queue can be even not registered and no need in this warn.

>
>You should just call xdp_rxq_info_unreg(rxq), if you know that this rxq
>should be registered.  If your assumption failed, you will get a
>WARNing, and discover your driver level bug.  This is one of the ways
>the API is designed to "detect" misuse of the API.  (I found this
>rather useful, when I converted the approx 12 drivers using this
>xdp_rxq_info API).
>
>-- 
>Best regards,
>  Jesper Dangaard Brouer
>  MSc.CS, Principal Kernel Engineer at Red Hat
>  LinkedIn: http://www.linkedin.com/in/brouer

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* Re: [iproute2] Can't create ip6 tunnel device
From: Andrea Claudi @ 2019-07-02 11:19 UTC (permalink / raw)
  To: Ji Jianwen; +Cc: netdev, Stephen Hemminger, maheshb
In-Reply-To: <CAPpH65x3_adKR5DfBznwg-6k=ZrTE3EZwGcKziiZx6hqcHkgbA@mail.gmail.com>

On Tue, Jul 2, 2019 at 12:55 PM Andrea Claudi <aclaudi@redhat.com> wrote:
>
> On Tue, Jul 2, 2019 at 12:27 PM Ji Jianwen <jijianwen@gmail.com> wrote:
> >
> > It seems this issue was introduced by commit below, I am able to run
> > the command successfully mentioned at previous mail without it.
> >
> > commit ba126dcad20e6d0e472586541d78bdd1ac4f1123 (HEAD)
> > Author: Mahesh Bandewar <maheshb@google.com>
> > Date:   Thu Jun 6 16:44:26 2019 -0700
> >
> >     ip6tunnel: fix 'ip -6 {show|change} dev <name>' cmds
> >
>
> From what I can see, before this commit we have in p->name the tunnel
> iface name (in Jianwen example, ip6tnl1), while after this p->name
> contains the iface name specified after "dev".
> Probably the strlcpy() should be limited to the {show|change} cases?
>
> Regards,
> Andrea
>
> > On Tue, Jul 2, 2019 at 2:53 PM Ji Jianwen <jijianwen@gmail.com> wrote:
> > >
> > > Hello  there,
> > >
> > > I got error when creating ip6 tunnel device on a rhel-8.0.0 system.
> > >
> > > Here are the steps to reproduce the issue.
> > > # # uname -r
> > > 4.18.0-80.el8.x86_64
> > > # dnf install -y libcap-devel bison flex git gcc
> > > # git clone git://git.kernel.org/pub/scm/network/iproute2/iproute2.git
> > > # cd iproute2  &&  git log --pretty=oneline --abbrev-commit
> > > d0272f54 (HEAD -> master, origin/master, origin/HEAD) devlink: fix
> > > libc and kernel headers collision
> > > ee09370a devlink: fix format string warning for 32bit targets
> > > 68c46872 ip address: do not set mngtmpaddr option for IPv4 addresses
> > > e4448b6c ip address: do not set home option for IPv4 addresses
> > > ....
> > >
> > > # ./configure && make && make install
> > > # ip -6 tunnel add ip6tnl1 mode ip6ip6 remote 2001:db8:ffff:100::2
> > > local 2001:db8:ffff:100::1 hoplimit 1 tclass 0x0 dev eno1   --->
> > > please replace eno1 with the network card name of your system
> > > add tunnel "ip6tnl0" failed: File exists
> > >
> > > Please help take a look. Thanks!
> > >
> > > Br,
> > > Jianwen

Jianwen, can you please check if this patch solves your issue?

--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -298,7 +298,7 @@ static int parse_args(int argc, char **argv, int
cmd, struct ip6_tnl_parm2 *p)
                p->link = ll_name_to_index(medium);
                if (!p->link)
                        return nodev(medium);
-               else
+               else if (cmd != SIOCADDTUNNEL)
                        strlcpy(p->name, medium, sizeof(p->name));
        }
        return 0;

Thanks in advance!

^ permalink raw reply

* Re: [iproute2] Can't create ip6 tunnel device
From: Andrea Claudi @ 2019-07-02 10:55 UTC (permalink / raw)
  To: Ji Jianwen; +Cc: netdev, Stephen Hemminger, maheshb
In-Reply-To: <CAGWhr0Bg3mnaddCg=RexXgUGeP5EyqiU63n_c9NAgyfx-wpJ2Q@mail.gmail.com>

On Tue, Jul 2, 2019 at 12:27 PM Ji Jianwen <jijianwen@gmail.com> wrote:
>
> It seems this issue was introduced by commit below, I am able to run
> the command successfully mentioned at previous mail without it.
>
> commit ba126dcad20e6d0e472586541d78bdd1ac4f1123 (HEAD)
> Author: Mahesh Bandewar <maheshb@google.com>
> Date:   Thu Jun 6 16:44:26 2019 -0700
>
>     ip6tunnel: fix 'ip -6 {show|change} dev <name>' cmds
>

From what I can see, before this commit we have in p->name the tunnel
iface name (in Jianwen example, ip6tnl1), while after this p->name
contains the iface name specified after "dev".
Probably the strlcpy() should be limited to the {show|change} cases?

Regards,
Andrea

> On Tue, Jul 2, 2019 at 2:53 PM Ji Jianwen <jijianwen@gmail.com> wrote:
> >
> > Hello  there,
> >
> > I got error when creating ip6 tunnel device on a rhel-8.0.0 system.
> >
> > Here are the steps to reproduce the issue.
> > # # uname -r
> > 4.18.0-80.el8.x86_64
> > # dnf install -y libcap-devel bison flex git gcc
> > # git clone git://git.kernel.org/pub/scm/network/iproute2/iproute2.git
> > # cd iproute2  &&  git log --pretty=oneline --abbrev-commit
> > d0272f54 (HEAD -> master, origin/master, origin/HEAD) devlink: fix
> > libc and kernel headers collision
> > ee09370a devlink: fix format string warning for 32bit targets
> > 68c46872 ip address: do not set mngtmpaddr option for IPv4 addresses
> > e4448b6c ip address: do not set home option for IPv4 addresses
> > ....
> >
> > # ./configure && make && make install
> > # ip -6 tunnel add ip6tnl1 mode ip6ip6 remote 2001:db8:ffff:100::2
> > local 2001:db8:ffff:100::1 hoplimit 1 tclass 0x0 dev eno1   --->
> > please replace eno1 with the network card name of your system
> > add tunnel "ip6tnl0" failed: File exists
> >
> > Please help take a look. Thanks!
> >
> > Br,
> > Jianwen

^ permalink raw reply

* [PATCH ipsec] xfrm: policy: fix bydst hlist corruption on hash rebuild
From: Florian Westphal @ 2019-07-02 10:46 UTC (permalink / raw)
  To: netdev
  Cc: steffen.klassert, syzkaller-bugs, Florian Westphal,
	syzbot+0165480d4ef07360eeda
In-Reply-To: <000000000000db481c058c462e4c@google.com>

syzbot reported following spat:

BUG: KASAN: use-after-free in __write_once_size include/linux/compiler.h:221
BUG: KASAN: use-after-free in hlist_del_rcu include/linux/rculist.h:455
BUG: KASAN: use-after-free in xfrm_hash_rebuild+0xa0d/0x1000 net/xfrm/xfrm_policy.c:1318
Write of size 8 at addr ffff888095e79c00 by task kworker/1:3/8066
Workqueue: events xfrm_hash_rebuild
Call Trace:
 __write_once_size include/linux/compiler.h:221 [inline]
 hlist_del_rcu include/linux/rculist.h:455 [inline]
 xfrm_hash_rebuild+0xa0d/0x1000 net/xfrm/xfrm_policy.c:1318
 process_one_work+0x814/0x1130 kernel/workqueue.c:2269
Allocated by task 8064:
 __kmalloc+0x23c/0x310 mm/slab.c:3669
 kzalloc include/linux/slab.h:742 [inline]
 xfrm_hash_alloc+0x38/0xe0 net/xfrm/xfrm_hash.c:21
 xfrm_policy_init net/xfrm/xfrm_policy.c:4036 [inline]
 xfrm_net_init+0x269/0xd60 net/xfrm/xfrm_policy.c:4120
 ops_init+0x336/0x420 net/core/net_namespace.c:130
 setup_net+0x212/0x690 net/core/net_namespace.c:316

The faulting address is the address of the old chain head,
free'd by xfrm_hash_resize().

In xfrm_hash_rehash(), chain heads get re-initialized without
any hlist_del_rcu:

 for (i = hmask; i >= 0; i--)
    INIT_HLIST_HEAD(odst + i);

Then, hlist_del_rcu() gets called on the about to-be-reinserted policy
when iterating the per-net list of policies.

hlist_del_rcu() will then make chain->first be nonzero again:

static inline void __hlist_del(struct hlist_node *n)
{
   struct hlist_node *next = n->next;   // address of next element in list
   struct hlist_node **pprev = n->pprev;// location of previous elem, this
                                        // can point at chain->first
        WRITE_ONCE(*pprev, next);       // chain->first points to next elem
        if (next)
                next->pprev = pprev;

Then, when we walk chainlist to find insertion point, we may find a
non-empty list even though we're supposedly reinserting the first
policy to an empty chain.

To fix this first unlink all exact and inexact policies instead of
zeroing the list heads.

Add the commands equivalent to the syzbot reproducer to xfrm_policy.sh,
without fix KASAN catches the corruption as it happens, SLUB poisoning
detects it a bit later.

Reported-by: syzbot+0165480d4ef07360eeda@syzkaller.appspotmail.com
Fixes: 1548bc4e0512 ("xfrm: policy: delete inexact policies from inexact list on hash rebuild")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/xfrm/xfrm_policy.c                     | 12 ++++++----
 tools/testing/selftests/net/xfrm_policy.sh | 27 +++++++++++++++++++++-
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index b1694d5d15d3..82be7780bbe8 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1280,13 +1280,17 @@ static void xfrm_hash_rebuild(struct work_struct *work)
 
 		hlist_for_each_entry_safe(policy, n,
 					  &net->xfrm.policy_inexact[dir],
-					  bydst_inexact_list)
+					  bydst_inexact_list) {
+			hlist_del_rcu(&policy->bydst);
 			hlist_del_init(&policy->bydst_inexact_list);
+		}
 
 		hmask = net->xfrm.policy_bydst[dir].hmask;
 		odst = net->xfrm.policy_bydst[dir].table;
-		for (i = hmask; i >= 0; i--)
-			INIT_HLIST_HEAD(odst + i);
+		for (i = hmask; i >= 0; i--) {
+			hlist_for_each_entry_safe(policy, n, odst + i, bydst)
+				hlist_del_rcu(&policy->bydst);
+		}
 		if ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) {
 			/* dir out => dst = remote, src = local */
 			net->xfrm.policy_bydst[dir].dbits4 = rbits4;
@@ -1315,8 +1319,6 @@ static void xfrm_hash_rebuild(struct work_struct *work)
 		chain = policy_hash_bysel(net, &policy->selector,
 					  policy->family, dir);
 
-		hlist_del_rcu(&policy->bydst);
-
 		if (!chain) {
 			void *p = xfrm_policy_inexact_insert(policy, dir, 0);
 
diff --git a/tools/testing/selftests/net/xfrm_policy.sh b/tools/testing/selftests/net/xfrm_policy.sh
index 71d7fdc513c1..5445943bf07f 100755
--- a/tools/testing/selftests/net/xfrm_policy.sh
+++ b/tools/testing/selftests/net/xfrm_policy.sh
@@ -257,6 +257,29 @@ check_exceptions()
 	return $lret
 }
 
+check_hthresh_repeat()
+{
+	local log=$1
+	i=0
+
+	for i in $(seq 1 10);do
+		ip -net ns1 xfrm policy update src e000:0001::0000 dst ff01::0014:0000:0001 dir in tmpl src :: dst :: proto esp mode tunnel priority 100 action allow || break
+		ip -net ns1 xfrm policy set hthresh6 0 28 || break
+
+		ip -net ns1 xfrm policy update src e000:0001::0000 dst ff01::01 dir in tmpl src :: dst :: proto esp mode tunnel priority 100 action allow || break
+		ip -net ns1 xfrm policy set hthresh6 0 28 || break
+	done
+
+	if [ $i -ne 10 ] ;then
+		echo "FAIL: $log" 1>&2
+		ret=1
+		return 1
+	fi
+
+	echo "PASS: $log"
+	return 0
+}
+
 #check for needed privileges
 if [ "$(id -u)" -ne 0 ];then
 	echo "SKIP: Need root privileges"
@@ -404,7 +427,9 @@ for n in ns3 ns4;do
 	ip -net $n xfrm policy set hthresh4 32 32 hthresh6 128 128
 	sleep $((RANDOM%5))
 done
-check_exceptions "exceptions and block policies after hresh change to normal"
+check_exceptions "exceptions and block policies after htresh change to normal"
+
+check_hthresh_repeat "policies with repeated htresh change"
 
 for i in 1 2 3 4;do ip netns del ns$i;done
 
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH v5 net-next 1/6] xdp: allow same allocator usage
From: Ivan Khoronzhuk @ 2019-07-02 10:27 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: grygorii.strashko, hawk, davem, ast, linux-kernel, linux-omap,
	xdp-newbies, ilias.apalodimas, netdev, daniel, jakub.kicinski,
	john.fastabend
In-Reply-To: <20190701134059.71757892@carbon>

On Mon, Jul 01, 2019 at 01:40:59PM +0200, Jesper Dangaard Brouer wrote:
>
>I'm very skeptical about this approach.
>
>On Sun, 30 Jun 2019 20:23:43 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> XDP rxqs can be same for ndevs running under same rx napi softirq.
>> But there is no ability to register same allocator for both rxqs,
>> by fact it can same rxq but has different ndev as a reference.
>
>This description is not very clear. It can easily be misunderstood.
>
>It is an absolute requirement that each RX-queue have their own
>page_pool object/allocator. (This where the performance comes from) as
>the page_pool have NAPI protected array for alloc and XDP_DROP recycle.
>
>Your driver/hardware seems to have special case, where a single
>RX-queue can receive packets for two different net_device'es.
>
>Do you violate this XDP devmap redirect assumption[1]?
>[1] https://github.com/torvalds/linux/blob/v5.2-rc7/kernel/bpf/devmap.c#L324-L329
Seems that yes, but that's used only for trace for now.
As it runs under napi and flush clear dev_rx i must do it right in the
rx_handler. So next patchset version will have one patch less.

Thanks!

>
>
>> Due to last changes allocator destroy can be defered till the moment
>> all packets are recycled by destination interface, afterwards it's
>> freed. In order to schedule allocator destroy only after all users are
>> unregistered, add refcnt to allocator object and schedule to destroy
>> only it reaches 0.
>
>The guiding principles when designing an API, is to make it easy to
>use, but also make it hard to misuse.
>
>Your API change makes it easy to misuse the API.  As it make it easy to
>(re)use the allocator pointer (likely page_pool) for multiple
>xdp_rxq_info structs.  It is only valid for your use-case, because you
>have hardware where a single RX-queue delivers to two different
>net_devices.  For other normal use-cases, this will be a violation.
>
>If I was a user of this API, and saw your xdp_allocator_get(), then I
>would assume that this was the normal case.  As minimum, we need to add
>a comment in the code, about this specific/intended use-case.  I
>through about detecting the misuse, by adding a queue_index to
>xdp_mem_allocator, that can be checked against, when calling
>xdp_rxq_info_reg_mem_model() with another xdp_rxq_info struct (to catch
>the obvious mistake where queue_index mismatch).

I can add, but not sure if it has or can have some conflicts with another
memory allocators, now or in future. Main here to not became a cornerstone
in some not obvious use-cases.

So, for now, let it be in this way:

--- a/include/net/xdp_priv.h
+++ b/include/net/xdp_priv.h
@@ -19,6 +19,7 @@ struct xdp_mem_allocator {
        struct delayed_work defer_wq;
        unsigned long defer_warn;
        unsigned long refcnt;
+       u32 queue_index;
 };

 #endif /* __LINUX_NET_XDP_PRIV_H__ */
diff --git a/net/core/xdp.c b/net/core/xdp.c
index a44621190fdc..c4bf29810f4d 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -324,7 +324,7 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
        return true;
 }

-static struct xdp_mem_allocator *xdp_allocator_get(void *allocator)
+static struct xdp_mem_allocator *xdp_allocator_find(void *allocator)
 {
        struct xdp_mem_allocator *xae, *xa = NULL;
        struct rhashtable_iter iter;
@@ -336,7 +336,6 @@ static struct xdp_mem_allocator *xdp_allocator_get(void *allocator)

                while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
                        if (xae->allocator == allocator) {
-                               xae->refcnt++;
                                xa = xae;
                                break;
                        }
@@ -386,9 +385,13 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
                }
        }

-       xdp_alloc = xdp_allocator_get(allocator);
+       xdp_alloc = xdp_allocator_find(allocator);
        if (xdp_alloc) {
+               if (xdp_alloc->queue_index != xdp_rxq->queue_index)
+                       return -EINVAL;
+
                xdp_rxq->mem.id = xdp_alloc->mem.id;
+               xdp_alloc->refcnt++;
                return 0;
        }

@@ -406,6 +409,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
        xdp_alloc->mem  = xdp_rxq->mem;
        xdp_alloc->allocator = allocator;
        xdp_alloc->refcnt = 1;
+       xdp_alloc->queue_index = xdp_rxq->queue_index;

        /* Insert allocator into ID lookup table */
        ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);

Jesper, are you Ok with this version?

>
>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  include/net/xdp_priv.h |  1 +
>>  net/core/xdp.c         | 46 ++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
>> index 6a8cba6ea79a..995b21da2f27 100644
>> --- a/include/net/xdp_priv.h
>> +++ b/include/net/xdp_priv.h
>> @@ -18,6 +18,7 @@ struct xdp_mem_allocator {
>>  	struct rcu_head rcu;
>>  	struct delayed_work defer_wq;
>>  	unsigned long defer_warn;
>> +	unsigned long refcnt;
>>  };
>>
>>  #endif /* __LINUX_NET_XDP_PRIV_H__ */
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index b29d7b513a18..a44621190fdc 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -98,6 +98,18 @@ bool __mem_id_disconnect(int id, bool force)
>>  		WARN(1, "Request remove non-existing id(%d), driver bug?", id);
>>  		return true;
>>  	}
>> +
>> +	/* to avoid calling hash lookup twice, decrement refcnt here till it
>> +	 * reaches zero, then it can be called from workqueue afterwards.
>> +	 */
>> +	if (xa->refcnt)
>> +		xa->refcnt--;
>> +
>> +	if (xa->refcnt) {
>> +		mutex_unlock(&mem_id_lock);
>> +		return true;
>> +	}
>> +
>>  	xa->disconnect_cnt++;
>>
>>  	/* Detects in-flight packet-pages for page_pool */
>> @@ -312,6 +324,33 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
>>  	return true;
>>  }
>>
>> +static struct xdp_mem_allocator *xdp_allocator_get(void *allocator)
>
>API wise, when you have "get" operation, you usually also have a "put"
>operation...

It's not part of external API.
I propose to rename it on xdp_allocator_find() as in above diff.
What do you say?

>
>> +{
>> +	struct xdp_mem_allocator *xae, *xa = NULL;
>> +	struct rhashtable_iter iter;
>> +
>> +	mutex_lock(&mem_id_lock);
>> +	rhashtable_walk_enter(mem_id_ht, &iter);
>> +	do {
>> +		rhashtable_walk_start(&iter);
>> +
>> +		while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
>> +			if (xae->allocator == allocator) {
>> +				xae->refcnt++;
>> +				xa = xae;
>> +				break;
>> +			}
>> +		}
>> +
>> +		rhashtable_walk_stop(&iter);
>> +
>> +	} while (xae == ERR_PTR(-EAGAIN));
>> +	rhashtable_walk_exit(&iter);
>> +	mutex_unlock(&mem_id_lock);
>> +
>> +	return xa;
>> +}
>> +
>>  int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>>  			       enum xdp_mem_type type, void *allocator)
>>  {
>> @@ -347,6 +386,12 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>>  		}
>>  	}
>>
>> +	xdp_alloc = xdp_allocator_get(allocator);
>> +	if (xdp_alloc) {
>> +		xdp_rxq->mem.id = xdp_alloc->mem.id;
>> +		return 0;
>> +	}
>> +
>
>The allocator pointer (in-practice) becomes the identifier for the
>mem.id (which rhashtable points to xdp_mem_allocator object).

So, you have no obj against it?

[...]

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply related

* Re: [iproute2] Can't create ip6 tunnel device
From: Ji Jianwen @ 2019-07-02 10:26 UTC (permalink / raw)
  To: netdev; +Cc: stephen, maheshb
In-Reply-To: <CAGWhr0AOApbf4-4RJHibUaKa8MmOfGS+uH6Rx4x1PQGZXRbCOQ@mail.gmail.com>

It seems this issue was introduced by commit below, I am able to run
the command successfully mentioned at previous mail without it.

commit ba126dcad20e6d0e472586541d78bdd1ac4f1123 (HEAD)
Author: Mahesh Bandewar <maheshb@google.com>
Date:   Thu Jun 6 16:44:26 2019 -0700

    ip6tunnel: fix 'ip -6 {show|change} dev <name>' cmds

On Tue, Jul 2, 2019 at 2:53 PM Ji Jianwen <jijianwen@gmail.com> wrote:
>
> Hello  there,
>
> I got error when creating ip6 tunnel device on a rhel-8.0.0 system.
>
> Here are the steps to reproduce the issue.
> # # uname -r
> 4.18.0-80.el8.x86_64
> # dnf install -y libcap-devel bison flex git gcc
> # git clone git://git.kernel.org/pub/scm/network/iproute2/iproute2.git
> # cd iproute2  &&  git log --pretty=oneline --abbrev-commit
> d0272f54 (HEAD -> master, origin/master, origin/HEAD) devlink: fix
> libc and kernel headers collision
> ee09370a devlink: fix format string warning for 32bit targets
> 68c46872 ip address: do not set mngtmpaddr option for IPv4 addresses
> e4448b6c ip address: do not set home option for IPv4 addresses
> ....
>
> # ./configure && make && make install
> # ip -6 tunnel add ip6tnl1 mode ip6ip6 remote 2001:db8:ffff:100::2
> local 2001:db8:ffff:100::1 hoplimit 1 tclass 0x0 dev eno1   --->
> please replace eno1 with the network card name of your system
> add tunnel "ip6tnl0" failed: File exists
>
> Please help take a look. Thanks!
>
> Br,
> Jianwen

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox