[PATCH net-next 0/7] ethtool netlink interface, part 2

[PATCH net-next 0/7] ethtool netlink interface, part 2

[Michal Kubecek]

This shorter series adds support for getting and setting of wake-on-lan
settings and message mask (originally message level). Together with the
code already in net-next, this will allow full implementation of
"ethtool <dev>" and "ethtool -s <dev> ...".

Older versions of the ethtool netlink series allowed getting WoL settings
by unprivileged users and only filtered out the password but this was
a source of controversy so for now, ETHTOOL_MSG_WOL_GET request always
requires CAP_NET_ADMIN as ETHTOOL_GWOL ioctl request does.

Michal Kubecek (7):
  ethtool: fix kernel-doc descriptions
  ethtool: provide message mask with DEBUG_GET request
  ethtool: set message mask with DEBUG_SET request
  ethtool: add DEBUG_NTF notification
  ethtool: provide WoL settings with WOL_GET request
  ethtool: set wake-on-lan settings with WOL_SET request
  ethtool: add WOL_NTF notification

 Documentation/networking/ethtool-netlink.rst | 110 +++++++++++-
 include/linux/netdevice.h                    |  56 ++++--
 include/uapi/linux/ethtool.h                 |   6 +
 include/uapi/linux/ethtool_netlink.h         |  33 ++++
 net/ethtool/Makefile                         |   2 +-
 net/ethtool/common.c                         |  31 ++++
 net/ethtool/common.h                         |   2 +
 net/ethtool/debug.c                          | 134 ++++++++++++++
 net/ethtool/ioctl.c                          |   3 +
 net/ethtool/netlink.c                        |  57 ++++--
 net/ethtool/netlink.h                        |   4 +
 net/ethtool/strset.c                         |  11 ++
 net/ethtool/wol.c                            | 177 +++++++++++++++++++
 13 files changed, 592 insertions(+), 34 deletions(-)
 create mode 100644 net/ethtool/debug.c
 create mode 100644 net/ethtool/wol.c

-- 
2.25.0

[PATCH net-next 1/7] ethtool: fix kernel-doc descriptions

[Michal Kubecek]

Fix missing or incorrect function argument and struct member descriptions.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 net/ethtool/netlink.c | 26 ++++++++++++++------------
 net/ethtool/strset.c  |  1 +
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 86b79f9bc08d..0af43bbdb9b2 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -134,11 +134,12 @@ int ethnl_fill_reply_header(struct sk_buff *skb, struct net_device *dev,
 
 /**
  * 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()
+ * @payload:      payload length (without netlink and genetlink header)
+ * @dev:          device the reply is about (may be null)
+ * @cmd:          ETHTOOL_MSG_* message type for reply
+ * @hdr_attrtype: attribute type for common header
+ * @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
  */
@@ -188,10 +189,11 @@ static int ethnl_multicast(struct sk_buff *skb, struct net_device *dev)
 
 /**
  * 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
+ * @ops:        request ops of currently processed message type
+ * @req_info:   parsed request header of processed request
+ * @reply_data: data needed to compose the reply
+ * @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_default_start() and used
@@ -268,9 +270,9 @@ static int ethnl_default_parse(struct ethnl_req_info *req_info,
 
 /**
  * ethnl_init_reply_data() - Initialize reply data for GET request
- * @req_info: pointer to embedded struct ethnl_req_info
- * @ops:      instance of struct ethnl_request_ops describing the layout
- * @dev:      network device to initialize the reply for
+ * @reply_data: pointer to embedded struct ethnl_reply_data
+ * @ops:        instance of struct ethnl_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
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index 82a059c13c1c..948d967f1eca 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -85,6 +85,7 @@ get_stringset_policy[ETHTOOL_A_STRINGSET_MAX + 1] = {
 
 /**
  * strset_include() - test if a string set should be included in reply
+ * @info: parsed client request
  * @data: pointer to request data structure
  * @id:   id of string set to check (ETH_SS_* constants)
  */
-- 
2.25.0

[PATCH net-next 2/7] ethtool: provide message mask with DEBUG_GET request

[Michal Kubecek]

Implement DEBUG_GET request to get debugging settings for a device. At the
moment, only message mask corresponding to message level as reported by
ETHTOOL_GMSGLVL ioctl request is provided. (It is called message level in
ioctl interface but almost all drivers interpret it as a bit mask.)

As part of the implementation, provide symbolic names for message mask bits
as ETH_SS_MSG_CLASSES string set.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.rst | 34 ++++++++-
 include/linux/netdevice.h                    | 56 ++++++++++----
 include/uapi/linux/ethtool.h                 |  2 +
 include/uapi/linux/ethtool_netlink.h         | 14 ++++
 net/ethtool/Makefile                         |  2 +-
 net/ethtool/common.c                         | 19 +++++
 net/ethtool/common.h                         |  1 +
 net/ethtool/debug.c                          | 80 ++++++++++++++++++++
 net/ethtool/netlink.c                        |  8 ++
 net/ethtool/netlink.h                        |  1 +
 net/ethtool/strset.c                         |  5 ++
 11 files changed, 205 insertions(+), 17 deletions(-)
 create mode 100644 net/ethtool/debug.c

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index c60afba69e3c..76a411d3102d 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -185,6 +185,7 @@ Userspace to kernel:
   ``ETHTOOL_MSG_LINKMODES_GET``         get link modes info
   ``ETHTOOL_MSG_LINKMODES_SET``         set link modes info
   ``ETHTOOL_MSG_LINKSTATE_GET``         get link state
+  ``ETHTOOL_MSG_DEBUG_GET``             get debugging settings
   ===================================== ================================
 
 Kernel to userspace:
@@ -196,6 +197,7 @@ Kernel to userspace:
   ``ETHTOOL_MSG_LINKMODES_GET_REPLY``   link modes info
   ``ETHTOOL_MSG_LINKMODES_NTF``         link modes notification
   ``ETHTOOL_MSG_LINKSTATE_GET_REPLY``   link state info
+  ``ETHTOOL_MSG_DEBUG_GET_REPLY``       debugging settings
   ===================================== ================================
 
 ``GET`` requests are sent by userspace applications to retrieve device
@@ -423,6 +425,36 @@ define their own handler.
 devices supporting the request).
 
 
+DEBUG_GET
+=========
+
+Requests debugging settings of a device. At the moment, only message mask is
+provided.
+
+Request contents:
+
+  ====================================  ======  ==========================
+  ``ETHTOOL_A_DEBUG_HEADER``            nested  request header
+  ====================================  ======  ==========================
+
+Kernel response contents:
+
+  ====================================  ======  ==========================
+  ``ETHTOOL_A_DEBUG_HEADER``            nested  reply header
+  ``ETHTOOL_A_DEBUG_MSGMASK``           bitset  message mask
+  ====================================  ======  ==========================
+
+The message mask (``ETHTOOL_A_DEBUG_MSGMASK``) is equal to message level as
+provided by ``ETHTOOL_GMSGLVL`` and set by ``ETHTOOL_SMSGLVL`` in ioctl
+interface. While it is called message level there for historical reasons, most
+drivers and almost all newer drivers use it as a mask of enabled message
+classes (represented by ``NETIF_MSG_*`` constants); therefore netlink
+interface follows its actual use in practice.
+
+``DEBUG_GET`` allows dump requests (kernel returns reply messages for all
+devices supporting the request).
+
+
 Request translation
 ===================
 
@@ -441,7 +473,7 @@ have their netlink replacement yet.
   ``ETHTOOL_GREGS``                   n/a
   ``ETHTOOL_GWOL``                    n/a
   ``ETHTOOL_SWOL``                    n/a
-  ``ETHTOOL_GMSGLVL``                 n/a
+  ``ETHTOOL_GMSGLVL``                 ``ETHTOOL_MSG_DEBUG_GET``
   ``ETHTOOL_SMSGLVL``                 n/a
   ``ETHTOOL_NWAY_RST``                n/a
   ``ETHTOOL_GLINK``                   ``ETHTOOL_MSG_LINKSTATE_GET``
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 78e9c6c1b131..bdec77b7bec3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3894,22 +3894,48 @@ void netif_device_attach(struct net_device *dev);
  */
 
 enum {
-	NETIF_MSG_DRV		= 0x0001,
-	NETIF_MSG_PROBE		= 0x0002,
-	NETIF_MSG_LINK		= 0x0004,
-	NETIF_MSG_TIMER		= 0x0008,
-	NETIF_MSG_IFDOWN	= 0x0010,
-	NETIF_MSG_IFUP		= 0x0020,
-	NETIF_MSG_RX_ERR	= 0x0040,
-	NETIF_MSG_TX_ERR	= 0x0080,
-	NETIF_MSG_TX_QUEUED	= 0x0100,
-	NETIF_MSG_INTR		= 0x0200,
-	NETIF_MSG_TX_DONE	= 0x0400,
-	NETIF_MSG_RX_STATUS	= 0x0800,
-	NETIF_MSG_PKTDATA	= 0x1000,
-	NETIF_MSG_HW		= 0x2000,
-	NETIF_MSG_WOL		= 0x4000,
+	NETIF_MSG_DRV_BIT,
+	NETIF_MSG_PROBE_BIT,
+	NETIF_MSG_LINK_BIT,
+	NETIF_MSG_TIMER_BIT,
+	NETIF_MSG_IFDOWN_BIT,
+	NETIF_MSG_IFUP_BIT,
+	NETIF_MSG_RX_ERR_BIT,
+	NETIF_MSG_TX_ERR_BIT,
+	NETIF_MSG_TX_QUEUED_BIT,
+	NETIF_MSG_INTR_BIT,
+	NETIF_MSG_TX_DONE_BIT,
+	NETIF_MSG_RX_STATUS_BIT,
+	NETIF_MSG_PKTDATA_BIT,
+	NETIF_MSG_HW_BIT,
+	NETIF_MSG_WOL_BIT,
+
+	/* When you add a new bit above, update netif_msg_class_names array
+	 * in net/ethtool/common.c
+	 */
+	NETIF_MSG_CLASS_COUNT,
 };
+/* Both ethtool_ops interface and internal driver implementation use u32 */
+static_assert(NETIF_MSG_CLASS_COUNT <= 32);
+
+#define __NETIF_MSG_BIT(bit)	((u32)1 << (bit))
+#define __NETIF_MSG(name)	__NETIF_MSG_BIT(NETIF_MSG_ ## name ## _BIT)
+
+#define NETIF_MSG_DRV		__NETIF_MSG(DRV)
+#define NETIF_MSG_PROBE		__NETIF_MSG(PROBE)
+#define NETIF_MSG_LINK		__NETIF_MSG(LINK)
+#define NETIF_MSG_TIMER		__NETIF_MSG(TIMER)
+#define NETIF_MSG_IFDOWN	__NETIF_MSG(IFDOWN)
+#define NETIF_MSG_IFUP		__NETIF_MSG(IFUP)
+#define NETIF_MSG_RX_ERR	__NETIF_MSG(RX_ERR)
+#define NETIF_MSG_TX_ERR	__NETIF_MSG(TX_ERR)
+#define NETIF_MSG_TX_QUEUED	__NETIF_MSG(TX_QUEUED)
+#define NETIF_MSG_INTR		__NETIF_MSG(INTR)
+#define NETIF_MSG_TX_DONE	__NETIF_MSG(TX_DONE)
+#define NETIF_MSG_RX_STATUS	__NETIF_MSG(RX_STATUS)
+#define NETIF_MSG_PKTDATA	__NETIF_MSG(PKTDATA)
+#define NETIF_MSG_HW		__NETIF_MSG(HW)
+#define NETIF_MSG_WOL		__NETIF_MSG(WOL)
 
 #define netif_msg_drv(p)	((p)->msg_enable & NETIF_MSG_DRV)
 #define netif_msg_probe(p)	((p)->msg_enable & NETIF_MSG_PROBE)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 116bcbf09c74..456fb2aa0fad 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -594,6 +594,7 @@ struct ethtool_pauseparam {
  * @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
+ * @ETH_SS_MSG_CLASSES: debug message class names
  */
 enum ethtool_stringset {
 	ETH_SS_TEST		= 0,
@@ -606,6 +607,7 @@ enum ethtool_stringset {
 	ETH_SS_PHY_STATS,
 	ETH_SS_PHY_TUNABLES,
 	ETH_SS_LINK_MODES,
+	ETH_SS_MSG_CLASSES,
 
 	/* add new constants above here */
 	ETH_SS_COUNT
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 02f82f42a889..0d39e04567cb 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -20,6 +20,7 @@ enum {
 	ETHTOOL_MSG_LINKMODES_GET,
 	ETHTOOL_MSG_LINKMODES_SET,
 	ETHTOOL_MSG_LINKSTATE_GET,
+	ETHTOOL_MSG_DEBUG_GET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -35,6 +36,7 @@ enum {
 	ETHTOOL_MSG_LINKMODES_GET_REPLY,
 	ETHTOOL_MSG_LINKMODES_NTF,
 	ETHTOOL_MSG_LINKSTATE_GET_REPLY,
+	ETHTOOL_MSG_DEBUG_GET_REPLY,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
@@ -195,6 +197,18 @@ enum {
 	ETHTOOL_A_LINKSTATE_MAX = __ETHTOOL_A_LINKSTATE_CNT - 1
 };
 
+/* DEBUG */
+
+enum {
+	ETHTOOL_A_DEBUG_UNSPEC,
+	ETHTOOL_A_DEBUG_HEADER,			/* nest - _A_HEADER_* */
+	ETHTOOL_A_DEBUG_MSGMASK,		/* bitset */
+
+	/* add new constants above here */
+	__ETHTOOL_A_DEBUG_CNT,
+	ETHTOOL_A_DEBUG_MAX = __ETHTOOL_A_DEBUG_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 9a1332fb0cc6..c120c820a4f5 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -5,4 +5,4 @@ obj-y				+= ioctl.o common.o
 obj-$(CONFIG_ETHTOOL_NETLINK)	+= ethtool_nl.o
 
 ethtool_nl-y	:= netlink.o bitset.o strset.o linkinfo.o linkmodes.o \
-		   linkstate.o
+		   linkstate.o debug.o
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index e621b1694d2f..93a46c640181 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -170,6 +170,25 @@ const char link_mode_names[][ETH_GSTRING_LEN] = {
 };
 static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
 
+const char netif_msg_class_names[][ETH_GSTRING_LEN] = {
+	[NETIF_MSG_DRV_BIT]		= "drv",
+	[NETIF_MSG_PROBE_BIT]		= "probe",
+	[NETIF_MSG_LINK_BIT]		= "link",
+	[NETIF_MSG_TIMER_BIT]		= "timer",
+	[NETIF_MSG_IFDOWN_BIT]		= "ifdown",
+	[NETIF_MSG_IFUP_BIT]		= "ifup",
+	[NETIF_MSG_RX_ERR_BIT]		= "rx_err",
+	[NETIF_MSG_TX_ERR_BIT]		= "tx_err",
+	[NETIF_MSG_TX_QUEUED_BIT]	= "tx_queued",
+	[NETIF_MSG_INTR_BIT]		= "intr",
+	[NETIF_MSG_TX_DONE_BIT]		= "tx_done",
+	[NETIF_MSG_RX_STATUS_BIT]	= "rx_status",
+	[NETIF_MSG_PKTDATA_BIT]		= "pktdata",
+	[NETIF_MSG_HW_BIT]		= "hw",
+	[NETIF_MSG_WOL_BIT]		= "wol",
+};
+static_assert(ARRAY_SIZE(netif_msg_class_names) == NETIF_MSG_CLASS_COUNT);
+
 /* return false if legacy contained non-0 deprecated fields
  * maxtxpkt/maxrxpkt. rest of ksettings always updated
  */
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index 5c5f7dc90cd4..064c5c3aa990 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -19,6 +19,7 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN];
 extern const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
 extern const char link_mode_names[][ETH_GSTRING_LEN];
+extern const char netif_msg_class_names[][ETH_GSTRING_LEN];
 
 int __ethtool_get_link(struct net_device *dev);
 
diff --git a/net/ethtool/debug.c b/net/ethtool/debug.c
new file mode 100644
index 000000000000..cc121a23be5f
--- /dev/null
+++ b/net/ethtool/debug.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "netlink.h"
+#include "common.h"
+#include "bitset.h"
+
+struct debug_req_info {
+	struct ethnl_req_info		base;
+};
+
+struct debug_reply_data {
+	struct ethnl_reply_data		base;
+	u32				msg_mask;
+};
+
+#define DEBUG_REPDATA(__reply_base) \
+	container_of(__reply_base, struct debug_reply_data, base)
+
+static const struct nla_policy
+debug_get_policy[ETHTOOL_A_DEBUG_MAX + 1] = {
+	[ETHTOOL_A_DEBUG_UNSPEC]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_DEBUG_HEADER]	= { .type = NLA_NESTED },
+	[ETHTOOL_A_DEBUG_MSGMASK]	= { .type = NLA_REJECT },
+};
+
+static int debug_prepare_data(const struct ethnl_req_info *req_base,
+			      struct ethnl_reply_data *reply_base,
+			      struct genl_info *info)
+{
+	struct debug_reply_data *data = DEBUG_REPDATA(reply_base);
+	struct net_device *dev = reply_base->dev;
+	int ret;
+
+	if (!dev->ethtool_ops->get_msglevel)
+		return -EOPNOTSUPP;
+
+	ret = ethnl_ops_begin(dev);
+	if (ret < 0)
+		return ret;
+	data->msg_mask = dev->ethtool_ops->get_msglevel(dev);
+	ethnl_ops_complete(dev);
+
+	return 0;
+}
+
+static int debug_reply_size(const struct ethnl_req_info *req_base,
+			    const struct ethnl_reply_data *reply_base)
+{
+	const struct debug_reply_data *data = DEBUG_REPDATA(reply_base);
+	bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+
+	return ethnl_bitset32_size(&data->msg_mask, NULL, NETIF_MSG_CLASS_COUNT,
+				   netif_msg_class_names, compact);
+}
+
+static int debug_fill_reply(struct sk_buff *skb,
+			    const struct ethnl_req_info *req_base,
+			    const struct ethnl_reply_data *reply_base)
+{
+	const struct debug_reply_data *data = DEBUG_REPDATA(reply_base);
+	bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+
+	return ethnl_put_bitset32(skb, ETHTOOL_A_DEBUG_MSGMASK, &data->msg_mask,
+				  NULL, NETIF_MSG_CLASS_COUNT,
+				  netif_msg_class_names, compact);
+}
+
+const struct ethnl_request_ops ethnl_debug_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_DEBUG_GET,
+	.reply_cmd		= ETHTOOL_MSG_DEBUG_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_DEBUG_HEADER,
+	.max_attr		= ETHTOOL_A_DEBUG_MAX,
+	.req_info_size		= sizeof(struct debug_req_info),
+	.reply_data_size	= sizeof(struct debug_reply_data),
+	.request_policy		= debug_get_policy,
+
+	.prepare_data		= debug_prepare_data,
+	.reply_size		= debug_reply_size,
+	.fill_reply		= debug_fill_reply,
+};
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 0af43bbdb9b2..bdaf583e392b 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -213,6 +213,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_LINKINFO_GET]	= &ethnl_linkinfo_request_ops,
 	[ETHTOOL_MSG_LINKMODES_GET]	= &ethnl_linkmodes_request_ops,
 	[ETHTOOL_MSG_LINKSTATE_GET]	= &ethnl_linkstate_request_ops,
+	[ETHTOOL_MSG_DEBUG_GET]		= &ethnl_debug_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -664,6 +665,13 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.dumpit	= ethnl_default_dumpit,
 		.done	= ethnl_default_done,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_DEBUG_GET,
+		.doit	= ethnl_default_doit,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index da9d6521a4eb..9bd8ef671501 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -334,6 +334,7 @@ extern const struct ethnl_request_ops ethnl_strset_request_ops;
 extern const struct ethnl_request_ops ethnl_linkinfo_request_ops;
 extern const struct ethnl_request_ops ethnl_linkmodes_request_ops;
 extern const struct ethnl_request_ops ethnl_linkstate_request_ops;
+extern const struct ethnl_request_ops ethnl_debug_request_ops;
 
 int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info);
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index 948d967f1eca..7a45c25355b8 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -50,6 +50,11 @@ static const struct strset_info info_template[] = {
 		.count		= __ETHTOOL_LINK_MODE_MASK_NBITS,
 		.strings	= link_mode_names,
 	},
+	[ETH_SS_MSG_CLASSES] = {
+		.per_dev	= false,
+		.count		= NETIF_MSG_CLASS_COUNT,
+		.strings	= netif_msg_class_names,
+	},
 };
 
 struct strset_req_info {
-- 
2.25.0

[PATCH net-next 3/7] ethtool: set message mask with DEBUG_SET request

[Michal Kubecek]

Implement DEBUG_SET netlink request to set debugging settings for a device.
At the moment, only message mask corresponding to message level as set by
ETHTOOL_SMSGLVL ioctl request can be set. (It is called message level in
ioctl interface but almost all drivers interpret it as a bit mask.)

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.rst | 20 +++++++-
 include/uapi/linux/ethtool_netlink.h         |  1 +
 net/ethtool/debug.c                          | 53 ++++++++++++++++++++
 net/ethtool/netlink.c                        |  5 ++
 net/ethtool/netlink.h                        |  1 +
 5 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 76a411d3102d..58473c05319f 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -186,6 +186,7 @@ Userspace to kernel:
   ``ETHTOOL_MSG_LINKMODES_SET``         set link modes info
   ``ETHTOOL_MSG_LINKSTATE_GET``         get link state
   ``ETHTOOL_MSG_DEBUG_GET``             get debugging settings
+  ``ETHTOOL_MSG_DEBUG_SET``             set debugging settings
   ===================================== ================================
 
 Kernel to userspace:
@@ -455,6 +456,23 @@ interface follows its actual use in practice.
 devices supporting the request).
 
 
+DEBUG_SET
+=========
+
+Set or update debugging settings of a device. At the moment, only message mask
+is supported.
+
+Request contents:
+
+  ====================================  ======  ==========================
+  ``ETHTOOL_A_DEBUG_HEADER``            nested  request header
+  ``ETHTOOL_A_DEBUG_MSGMASK``           bitset  message mask
+  ====================================  ======  ==========================
+
+``ETHTOOL_A_DEBUG_MSGMASK`` bit set allows setting or modifying mask of
+enabled debugging message types for the device.
+
+
 Request translation
 ===================
 
@@ -474,7 +492,7 @@ have their netlink replacement yet.
   ``ETHTOOL_GWOL``                    n/a
   ``ETHTOOL_SWOL``                    n/a
   ``ETHTOOL_GMSGLVL``                 ``ETHTOOL_MSG_DEBUG_GET``
-  ``ETHTOOL_SMSGLVL``                 n/a
+  ``ETHTOOL_SMSGLVL``                 ``ETHTOOL_MSG_DEBUG_SET``
   ``ETHTOOL_NWAY_RST``                n/a
   ``ETHTOOL_GLINK``                   ``ETHTOOL_MSG_LINKSTATE_GET``
   ``ETHTOOL_GEEPROM``                 n/a
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 0d39e04567cb..8f0b6fd277d5 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -21,6 +21,7 @@ enum {
 	ETHTOOL_MSG_LINKMODES_SET,
 	ETHTOOL_MSG_LINKSTATE_GET,
 	ETHTOOL_MSG_DEBUG_GET,
+	ETHTOOL_MSG_DEBUG_SET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
diff --git a/net/ethtool/debug.c b/net/ethtool/debug.c
index cc121a23be5f..6fc3ef8113c0 100644
--- a/net/ethtool/debug.c
+++ b/net/ethtool/debug.c
@@ -78,3 +78,56 @@ const struct ethnl_request_ops ethnl_debug_request_ops = {
 	.reply_size		= debug_reply_size,
 	.fill_reply		= debug_fill_reply,
 };
+
+/* DEBUG_SET */
+
+static const struct nla_policy
+debug_set_policy[ETHTOOL_A_DEBUG_MAX + 1] = {
+	[ETHTOOL_A_DEBUG_UNSPEC]	= { .type = NLA_REJECT },
+	[ETHTOOL_A_DEBUG_HEADER]	= { .type = NLA_NESTED },
+	[ETHTOOL_A_DEBUG_MSGMASK]	= { .type = NLA_NESTED },
+};
+
+int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info)
+{
+	struct nlattr *tb[ETHTOOL_A_DEBUG_MAX + 1];
+	struct ethnl_req_info req_info = {};
+	struct net_device *dev;
+	bool mod = false;
+	u32 msg_mask;
+	int ret;
+
+	ret = nlmsg_parse(info->nlhdr, GENL_HDRLEN, tb,
+			  ETHTOOL_A_DEBUG_MAX, debug_set_policy,
+			  info->extack);
+	if (ret < 0)
+		return ret;
+	ret = ethnl_parse_header(&req_info, tb[ETHTOOL_A_DEBUG_HEADER],
+				 genl_info_net(info), info->extack, true);
+	if (ret < 0)
+		return ret;
+	dev = req_info.dev;
+	if (!dev->ethtool_ops->get_msglevel || !dev->ethtool_ops->set_msglevel)
+		return -EOPNOTSUPP;
+
+	rtnl_lock();
+	ret = ethnl_ops_begin(dev);
+	if (ret < 0)
+		goto out_rtnl;
+
+	msg_mask = dev->ethtool_ops->get_msglevel(dev);
+	ret = ethnl_update_bitset32(&msg_mask, NETIF_MSG_CLASS_COUNT,
+				    tb[ETHTOOL_A_DEBUG_MSGMASK],
+				    netif_msg_class_names, info->extack, &mod);
+	if (ret < 0 || !mod)
+		goto out_ops;
+
+	dev->ethtool_ops->set_msglevel(dev, msg_mask);
+
+out_ops:
+	ethnl_ops_complete(dev);
+out_rtnl:
+	rtnl_unlock();
+	dev_put(dev);
+	return ret;
+}
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index bdaf583e392b..bcdc42e0bd14 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -672,6 +672,11 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.dumpit	= ethnl_default_dumpit,
 		.done	= ethnl_default_done,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_DEBUG_SET,
+		.flags	= GENL_UNS_ADMIN_PERM,
+		.doit	= ethnl_set_debug,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 9bd8ef671501..772723e536e8 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -338,5 +338,6 @@ extern const struct ethnl_request_ops ethnl_debug_request_ops;
 
 int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info);
+int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info);
 
 #endif /* _NET_ETHTOOL_NETLINK_H */
-- 
2.25.0

[PATCH net-next 4/7] ethtool: add DEBUG_NTF notification

[Michal Kubecek]

Send ETHTOOL_MSG_DEBUG_NTF notification message whenever debugging message
mask for a device are modified using ETHTOOL_MSG_DEBUG_SET netlink message
or ETHTOOL_SMSGLVL ioctl request.

The notification message has the same format as reply to DEBUG_GET request.
As with other ethtool notifications, netlink requests only trigger the
notification if the mask is actually changed while ioctl request trigger it
whenever the request results in calling the ethtool_ops handler.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.rst | 1 +
 include/uapi/linux/ethtool_netlink.h         | 1 +
 net/ethtool/debug.c                          | 1 +
 net/ethtool/ioctl.c                          | 2 ++
 net/ethtool/netlink.c                        | 2 ++
 5 files changed, 7 insertions(+)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 58473c05319f..2263f885d18f 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -199,6 +199,7 @@ Kernel to userspace:
   ``ETHTOOL_MSG_LINKMODES_NTF``         link modes notification
   ``ETHTOOL_MSG_LINKSTATE_GET_REPLY``   link state info
   ``ETHTOOL_MSG_DEBUG_GET_REPLY``       debugging settings
+  ``ETHTOOL_MSG_DEBUG_NTF``             debugging settings notification
   ===================================== ================================
 
 ``GET`` requests are sent by userspace applications to retrieve device
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 8f0b6fd277d5..67a06b94bf28 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -38,6 +38,7 @@ enum {
 	ETHTOOL_MSG_LINKMODES_NTF,
 	ETHTOOL_MSG_LINKSTATE_GET_REPLY,
 	ETHTOOL_MSG_DEBUG_GET_REPLY,
+	ETHTOOL_MSG_DEBUG_NTF,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
diff --git a/net/ethtool/debug.c b/net/ethtool/debug.c
index 6fc3ef8113c0..aaef4843e6ba 100644
--- a/net/ethtool/debug.c
+++ b/net/ethtool/debug.c
@@ -123,6 +123,7 @@ int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info)
 		goto out_ops;
 
 	dev->ethtool_ops->set_msglevel(dev, msg_mask);
+	ethtool_notify(dev, ETHTOOL_MSG_DEBUG_NTF, NULL);
 
 out_ops:
 	ethnl_ops_complete(dev);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 182bffbffa78..46e0b31782fc 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -2545,6 +2545,8 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SMSGLVL:
 		rc = ethtool_set_value_void(dev, useraddr,
 				       dev->ethtool_ops->set_msglevel);
+		if (!rc)
+			ethtool_notify(dev, ETHTOOL_MSG_DEBUG_NTF, NULL);
 		break;
 	case ETHTOOL_GEEE:
 		rc = ethtool_get_eee(dev, useraddr);
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index bcdc42e0bd14..9a0a6c2f8dbb 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -524,6 +524,7 @@ static const struct ethnl_request_ops *
 ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = {
 	[ETHTOOL_MSG_LINKINFO_NTF]	= &ethnl_linkinfo_request_ops,
 	[ETHTOOL_MSG_LINKMODES_NTF]	= &ethnl_linkmodes_request_ops,
+	[ETHTOOL_MSG_DEBUG_NTF]		= &ethnl_debug_request_ops,
 };
 
 /* default notification handler */
@@ -607,6 +608,7 @@ typedef void (*ethnl_notify_handler_t)(struct net_device *dev, unsigned int cmd,
 static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
 	[ETHTOOL_MSG_LINKINFO_NTF]	= ethnl_default_notify,
 	[ETHTOOL_MSG_LINKMODES_NTF]	= ethnl_default_notify,
+	[ETHTOOL_MSG_DEBUG_NTF]		= ethnl_default_notify,
 };
 
 void ethtool_notify(struct net_device *dev, unsigned int cmd, const void *data)
-- 
2.25.0

[PATCH net-next 5/7] ethtool: provide WoL settings with WOL_GET request

[Michal Kubecek]

Implement WOL_GET request to get wake-on-lan settings for a device,
traditionally available via ETHTOOL_GWOL ioctl request.

As part of the implementation, provide symbolic names for wake-on-line
modes as ETH_SS_WOL_MODES string set.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.rst | 30 +++++-
 include/uapi/linux/ethtool.h                 |  4 +
 include/uapi/linux/ethtool_netlink.h         | 15 +++
 net/ethtool/Makefile                         |  2 +-
 net/ethtool/common.c                         | 12 +++
 net/ethtool/common.h                         |  1 +
 net/ethtool/netlink.c                        |  9 ++
 net/ethtool/netlink.h                        |  1 +
 net/ethtool/strset.c                         |  5 +
 net/ethtool/wol.c                            | 99 ++++++++++++++++++++
 10 files changed, 176 insertions(+), 2 deletions(-)
 create mode 100644 net/ethtool/wol.c

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 2263f885d18f..5fd85e3ea96e 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -187,6 +187,7 @@ Userspace to kernel:
   ``ETHTOOL_MSG_LINKSTATE_GET``         get link state
   ``ETHTOOL_MSG_DEBUG_GET``             get debugging settings
   ``ETHTOOL_MSG_DEBUG_SET``             set debugging settings
+  ``ETHTOOL_MSG_WOL_GET``               get wake-on-lan settings
   ===================================== ================================
 
 Kernel to userspace:
@@ -200,6 +201,7 @@ Kernel to userspace:
   ``ETHTOOL_MSG_LINKSTATE_GET_REPLY``   link state info
   ``ETHTOOL_MSG_DEBUG_GET_REPLY``       debugging settings
   ``ETHTOOL_MSG_DEBUG_NTF``             debugging settings notification
+  ``ETHTOOL_MSG_WOL_GET_REPLY``         wake-on-lan settings
   ===================================== ================================
 
 ``GET`` requests are sent by userspace applications to retrieve device
@@ -474,6 +476,32 @@ Request contents:
 enabled debugging message types for the device.
 
 
+WOL_GET
+=======
+
+Query device wake-on-lan settings. Unlike most "GET" type requests,
+``ETHTOOL_MSG_WOL_GET`` requires (netns) ``CAP_NET_ADMIN`` privileges as it
+(potentially) provides SecureOn(tm) password which is confidential.
+
+Request contents:
+
+  ====================================  ======  ==========================
+  ``ETHTOOL_A_WOL_HEADER``              nested  request header
+  ====================================  ======  ==========================
+
+Kernel response contents:
+
+  ====================================  ======  ==========================
+  ``ETHTOOL_A_WOL_HEADER``              nested  reply header
+  ``ETHTOOL_A_WOL_MODES``               bitset  mask of enabled WoL modes
+  ``ETHTOOL_A_WOL_SOPASS``              binary  SecureOn(tm) password
+  ====================================  ======  ==========================
+
+In reply, ``ETHTOOL_A_WOL_MODES`` mask consists of modes supported by the
+device, value of modes which are enabled. ``ETHTOOL_A_WOL_SOPASS`` is only
+included in reply if ``WAKE_MAGICSECURE`` mode is supported.
+
+
 Request translation
 ===================
 
@@ -490,7 +518,7 @@ have their netlink replacement yet.
                                       ``ETHTOOL_MSG_LINKMODES_SET``
   ``ETHTOOL_GDRVINFO``                n/a
   ``ETHTOOL_GREGS``                   n/a
-  ``ETHTOOL_GWOL``                    n/a
+  ``ETHTOOL_GWOL``                    ``ETHTOOL_MSG_WOL_GET``
   ``ETHTOOL_SWOL``                    n/a
   ``ETHTOOL_GMSGLVL``                 ``ETHTOOL_MSG_DEBUG_GET``
   ``ETHTOOL_SMSGLVL``                 ``ETHTOOL_MSG_DEBUG_SET``
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 456fb2aa0fad..4295ebfa2f91 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -595,6 +595,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_PHY_TUNABLES: PHY tunable names
  * @ETH_SS_LINK_MODES: link mode names
  * @ETH_SS_MSG_CLASSES: debug message class names
+ * @ETH_SS_WOL_MODES: wake-on-lan modes
  */
 enum ethtool_stringset {
 	ETH_SS_TEST		= 0,
@@ -608,6 +609,7 @@ enum ethtool_stringset {
 	ETH_SS_PHY_TUNABLES,
 	ETH_SS_LINK_MODES,
 	ETH_SS_MSG_CLASSES,
+	ETH_SS_WOL_MODES,
 
 	/* add new constants above here */
 	ETH_SS_COUNT
@@ -1695,6 +1697,8 @@ static inline int ethtool_validate_duplex(__u8 duplex)
 #define WAKE_MAGICSECURE	(1 << 6) /* only meaningful if WAKE_MAGIC */
 #define WAKE_FILTER		(1 << 7)
 
+#define WOL_MODE_COUNT		8
+
 /* L2-L4 network traffic flow types */
 #define	TCP_V4_FLOW	0x01	/* hash or spec (tcp_ip4_spec) */
 #define	UDP_V4_FLOW	0x02	/* hash or spec (udp_ip4_spec) */
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 67a06b94bf28..dcc5c32dc018 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -22,6 +22,7 @@ enum {
 	ETHTOOL_MSG_LINKSTATE_GET,
 	ETHTOOL_MSG_DEBUG_GET,
 	ETHTOOL_MSG_DEBUG_SET,
+	ETHTOOL_MSG_WOL_GET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -39,6 +40,7 @@ enum {
 	ETHTOOL_MSG_LINKSTATE_GET_REPLY,
 	ETHTOOL_MSG_DEBUG_GET_REPLY,
 	ETHTOOL_MSG_DEBUG_NTF,
+	ETHTOOL_MSG_WOL_GET_REPLY,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
@@ -211,6 +213,19 @@ enum {
 	ETHTOOL_A_DEBUG_MAX = __ETHTOOL_A_DEBUG_CNT - 1
 };
 
+/* WOL */
+
+enum {
+	ETHTOOL_A_WOL_UNSPEC,
+	ETHTOOL_A_WOL_HEADER,			/* nest - _A_HEADER_* */
+	ETHTOOL_A_WOL_MODES,			/* bitset */
+	ETHTOOL_A_WOL_SOPASS,			/* binary */
+
+	/* add new constants above here */
+	__ETHTOOL_A_WOL_CNT,
+	ETHTOOL_A_WOL_MAX = __ETHTOOL_A_WOL_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 c120c820a4f5..424545a4aaec 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -5,4 +5,4 @@ obj-y				+= ioctl.o common.o
 obj-$(CONFIG_ETHTOOL_NETLINK)	+= ethtool_nl.o
 
 ethtool_nl-y	:= netlink.o bitset.o strset.o linkinfo.o linkmodes.o \
-		   linkstate.o debug.o
+		   linkstate.o debug.o wol.o
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 93a46c640181..fc26d912d670 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -189,6 +189,18 @@ const char netif_msg_class_names[][ETH_GSTRING_LEN] = {
 };
 static_assert(ARRAY_SIZE(netif_msg_class_names) == NETIF_MSG_CLASS_COUNT);
 
+const char wol_mode_names[][ETH_GSTRING_LEN] = {
+	[const_ilog2(WAKE_PHY)]		= "phy",
+	[const_ilog2(WAKE_UCAST)]	= "ucast",
+	[const_ilog2(WAKE_MCAST)]	= "mcast",
+	[const_ilog2(WAKE_BCAST)]	= "bcast",
+	[const_ilog2(WAKE_ARP)]		= "arp",
+	[const_ilog2(WAKE_MAGIC)]	= "magic",
+	[const_ilog2(WAKE_MAGICSECURE)]	= "magicsecure",
+	[const_ilog2(WAKE_FILTER)]	= "filter",
+};
+static_assert(ARRAY_SIZE(wol_mode_names) == WOL_MODE_COUNT);
+
 /* return false if legacy contained non-0 deprecated fields
  * maxtxpkt/maxrxpkt. rest of ksettings always updated
  */
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index 064c5c3aa990..40ba74e0b9bb 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -20,6 +20,7 @@ extern const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
 extern const char link_mode_names[][ETH_GSTRING_LEN];
 extern const char netif_msg_class_names[][ETH_GSTRING_LEN];
+extern const char wol_mode_names[][ETH_GSTRING_LEN];
 
 int __ethtool_get_link(struct net_device *dev);
 
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 9a0a6c2f8dbb..eeb6d8594e1b 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -214,6 +214,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_LINKMODES_GET]	= &ethnl_linkmodes_request_ops,
 	[ETHTOOL_MSG_LINKSTATE_GET]	= &ethnl_linkstate_request_ops,
 	[ETHTOOL_MSG_DEBUG_GET]		= &ethnl_debug_request_ops,
+	[ETHTOOL_MSG_WOL_GET]		= &ethnl_wol_request_ops,
 };
 
 static struct ethnl_dump_ctx *ethnl_dump_context(struct netlink_callback *cb)
@@ -679,6 +680,14 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= ethnl_set_debug,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_WOL_GET,
+		.flags	= GENL_UNS_ADMIN_PERM,
+		.doit	= ethnl_default_doit,
+		.start	= ethnl_default_start,
+		.dumpit	= ethnl_default_dumpit,
+		.done	= ethnl_default_done,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 772723e536e8..9fcd6f87b396 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -335,6 +335,7 @@ extern const struct ethnl_request_ops ethnl_linkinfo_request_ops;
 extern const struct ethnl_request_ops ethnl_linkmodes_request_ops;
 extern const struct ethnl_request_ops ethnl_linkstate_request_ops;
 extern const struct ethnl_request_ops ethnl_debug_request_ops;
+extern const struct ethnl_request_ops ethnl_wol_request_ops;
 
 int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info);
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index 7a45c25355b8..8e5911887b4c 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -55,6 +55,11 @@ static const struct strset_info info_template[] = {
 		.count		= NETIF_MSG_CLASS_COUNT,
 		.strings	= netif_msg_class_names,
 	},
+	[ETH_SS_WOL_MODES] = {
+		.per_dev	= false,
+		.count		= WOL_MODE_COUNT,
+		.strings	= wol_mode_names,
+	},
 };
 
 struct strset_req_info {
diff --git a/net/ethtool/wol.c b/net/ethtool/wol.c
new file mode 100644
index 000000000000..7c9a1ef622ce
--- /dev/null
+++ b/net/ethtool/wol.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "netlink.h"
+#include "common.h"
+#include "bitset.h"
+
+struct wol_req_info {
+	struct ethnl_req_info		base;
+};
+
+struct wol_reply_data {
+	struct ethnl_reply_data		base;
+	struct ethtool_wolinfo		wol;
+	bool				show_sopass;
+};
+
+#define WOL_REPDATA(__reply_base) \
+	container_of(__reply_base, struct wol_reply_data, base)
+
+static const struct nla_policy
+wol_get_policy[ETHTOOL_A_WOL_MAX + 1] = {
+	[ETHTOOL_A_WOL_UNSPEC]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_WOL_HEADER]		= { .type = NLA_NESTED },
+	[ETHTOOL_A_WOL_MODES]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_WOL_SOPASS]		= { .type = NLA_REJECT },
+};
+
+static int wol_prepare_data(const struct ethnl_req_info *req_base,
+			    struct ethnl_reply_data *reply_base,
+			    struct genl_info *info)
+{
+	struct wol_reply_data *data = WOL_REPDATA(reply_base);
+	struct net_device *dev = reply_base->dev;
+	int ret;
+
+	if (!dev->ethtool_ops->get_wol)
+		return -EOPNOTSUPP;
+
+	ret = ethnl_ops_begin(dev);
+	if (ret < 0)
+		return ret;
+	dev->ethtool_ops->get_wol(dev, &data->wol);
+	ethnl_ops_complete(dev);
+	data->show_sopass = data->wol.supported & WAKE_MAGICSECURE;
+
+	return 0;
+}
+
+static int wol_reply_size(const struct ethnl_req_info *req_base,
+			  const struct ethnl_reply_data *reply_base)
+{
+	bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+	const struct wol_reply_data *data = WOL_REPDATA(reply_base);
+	int len;
+
+	len = ethnl_bitset32_size(&data->wol.wolopts, &data->wol.supported,
+				  WOL_MODE_COUNT, wol_mode_names, compact);
+	if (len < 0)
+		return len;
+	if (data->show_sopass)
+		len += nla_total_size(sizeof(data->wol.sopass));
+
+	return len;
+}
+
+static int wol_fill_reply(struct sk_buff *skb,
+			  const struct ethnl_req_info *req_base,
+			  const struct ethnl_reply_data *reply_base)
+{
+	bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+	const struct wol_reply_data *data = WOL_REPDATA(reply_base);
+	int ret;
+
+	ret = ethnl_put_bitset32(skb, ETHTOOL_A_WOL_MODES, &data->wol.wolopts,
+				 &data->wol.supported, WOL_MODE_COUNT,
+				 wol_mode_names, compact);
+	if (ret < 0)
+		return ret;
+	if (data->show_sopass &&
+	    nla_put(skb, ETHTOOL_A_WOL_SOPASS, sizeof(data->wol.sopass),
+		    data->wol.sopass))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+const struct ethnl_request_ops ethnl_wol_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_WOL_GET,
+	.reply_cmd		= ETHTOOL_MSG_WOL_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_WOL_HEADER,
+	.max_attr		= ETHTOOL_A_WOL_MAX,
+	.req_info_size		= sizeof(struct wol_req_info),
+	.reply_data_size	= sizeof(struct wol_reply_data),
+	.request_policy		= wol_get_policy,
+
+	.prepare_data		= wol_prepare_data,
+	.reply_size		= wol_reply_size,
+	.fill_reply		= wol_fill_reply,
+};
-- 
2.25.0

[PATCH net-next 6/7] ethtool: set wake-on-lan settings with WOL_SET request

[Michal Kubecek]

Implement WOL_SET netlink request to set wake-on-lan settings. This is
equivalent to ETHTOOL_SWOL ioctl request.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.rst | 20 +++++-
 include/uapi/linux/ethtool_netlink.h         |  1 +
 net/ethtool/netlink.c                        |  5 ++
 net/ethtool/netlink.h                        |  1 +
 net/ethtool/wol.c                            | 76 ++++++++++++++++++++
 5 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 5fd85e3ea96e..f16f74bbb546 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -188,6 +188,7 @@ Userspace to kernel:
   ``ETHTOOL_MSG_DEBUG_GET``             get debugging settings
   ``ETHTOOL_MSG_DEBUG_SET``             set debugging settings
   ``ETHTOOL_MSG_WOL_GET``               get wake-on-lan settings
+  ``ETHTOOL_MSG_WOL_SET``               set wake-on-lan settings
   ===================================== ================================
 
 Kernel to userspace:
@@ -502,6 +503,23 @@ device, value of modes which are enabled. ``ETHTOOL_A_WOL_SOPASS`` is only
 included in reply if ``WAKE_MAGICSECURE`` mode is supported.
 
 
+WOL_SET
+=======
+
+Set or update wake-on-lan settings.
+
+Request contents:
+
+  ====================================  ======  ==========================
+  ``ETHTOOL_A_WOL_HEADER``              nested  request header
+  ``ETHTOOL_A_WOL_MODES``               bitset  enabled WoL modes
+  ``ETHTOOL_A_WOL_SOPASS``              binary  SecureOn(tm) password
+  ====================================  ======  ==========================
+
+``ETHTOOL_A_WOL_SOPASS`` is only allowed for devices supporting
+``WAKE_MAGICSECURE`` mode.
+
+
 Request translation
 ===================
 
@@ -519,7 +537,7 @@ have their netlink replacement yet.
   ``ETHTOOL_GDRVINFO``                n/a
   ``ETHTOOL_GREGS``                   n/a
   ``ETHTOOL_GWOL``                    ``ETHTOOL_MSG_WOL_GET``
-  ``ETHTOOL_SWOL``                    n/a
+  ``ETHTOOL_SWOL``                    ``ETHTOOL_MSG_WOL_SET``
   ``ETHTOOL_GMSGLVL``                 ``ETHTOOL_MSG_DEBUG_GET``
   ``ETHTOOL_SMSGLVL``                 ``ETHTOOL_MSG_DEBUG_SET``
   ``ETHTOOL_NWAY_RST``                n/a
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index dcc5c32dc018..59de35695521 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -23,6 +23,7 @@ enum {
 	ETHTOOL_MSG_DEBUG_GET,
 	ETHTOOL_MSG_DEBUG_SET,
 	ETHTOOL_MSG_WOL_GET,
+	ETHTOOL_MSG_WOL_SET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index eeb6d8594e1b..2c375f9095fe 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -688,6 +688,11 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.dumpit	= ethnl_default_dumpit,
 		.done	= ethnl_default_done,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_WOL_SET,
+		.flags	= GENL_UNS_ADMIN_PERM,
+		.doit	= ethnl_set_wol,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 9fcd6f87b396..60efd87686ad 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -340,5 +340,6 @@ extern const struct ethnl_request_ops ethnl_wol_request_ops;
 int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info);
+int ethnl_set_wol(struct sk_buff *skb, struct genl_info *info);
 
 #endif /* _NET_ETHTOOL_NETLINK_H */
diff --git a/net/ethtool/wol.c b/net/ethtool/wol.c
index 7c9a1ef622ce..a2724378fac4 100644
--- a/net/ethtool/wol.c
+++ b/net/ethtool/wol.c
@@ -97,3 +97,79 @@ const struct ethnl_request_ops ethnl_wol_request_ops = {
 	.reply_size		= wol_reply_size,
 	.fill_reply		= wol_fill_reply,
 };
+
+/* WOL_SET */
+
+static const struct nla_policy
+wol_set_policy[ETHTOOL_A_WOL_MAX + 1] = {
+	[ETHTOOL_A_WOL_UNSPEC]		= { .type = NLA_REJECT },
+	[ETHTOOL_A_WOL_HEADER]		= { .type = NLA_NESTED },
+	[ETHTOOL_A_WOL_MODES]		= { .type = NLA_NESTED },
+	[ETHTOOL_A_WOL_SOPASS]		= { .type = NLA_BINARY,
+					    .len = SOPASS_MAX },
+};
+
+int ethnl_set_wol(struct sk_buff *skb, struct genl_info *info)
+{
+	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
+	struct nlattr *tb[ETHTOOL_A_WOL_MAX + 1];
+	struct ethnl_req_info req_info = {};
+	struct net_device *dev;
+	bool mod = false;
+	int ret;
+
+	ret = nlmsg_parse(info->nlhdr, GENL_HDRLEN, tb, ETHTOOL_A_WOL_MAX,
+			  wol_set_policy, info->extack);
+	if (ret < 0)
+		return ret;
+	ret = ethnl_parse_header(&req_info, tb[ETHTOOL_A_WOL_HEADER],
+				 genl_info_net(info), info->extack, true);
+	if (ret < 0)
+		return ret;
+	dev = req_info.dev;
+	if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol)
+		return -EOPNOTSUPP;
+
+	rtnl_lock();
+	ret = ethnl_ops_begin(dev);
+	if (ret < 0)
+		goto out_rtnl;
+
+	dev->ethtool_ops->get_wol(dev, &wol);
+	ret = ethnl_update_bitset32(&wol.wolopts, WOL_MODE_COUNT,
+				    tb[ETHTOOL_A_WOL_MODES], wol_mode_names,
+				    info->extack, &mod);
+	if (ret < 0)
+		goto out_ops;
+	if (wol.wolopts & ~wol.supported) {
+		NL_SET_ERR_MSG_ATTR(info->extack, tb[ETHTOOL_A_WOL_MODES],
+				    "cannot enable unsupported WoL mode");
+		ret = -EINVAL;
+		goto out_ops;
+	}
+	if (tb[ETHTOOL_A_WOL_SOPASS]) {
+		if (!(wol.supported & WAKE_MAGICSECURE)) {
+			NL_SET_ERR_MSG_ATTR(info->extack,
+					    tb[ETHTOOL_A_WOL_SOPASS],
+					    "magicsecure not supported, cannot set password");
+			ret = -EINVAL;
+			goto out_ops;
+		}
+		ethnl_update_binary(wol.sopass, sizeof(wol.sopass),
+				    tb[ETHTOOL_A_WOL_SOPASS], &mod);
+	}
+
+	if (!mod)
+		goto out_ops;
+	ret = dev->ethtool_ops->set_wol(dev, &wol);
+	if (ret)
+		goto out_ops;
+	dev->wol_enabled = !!wol.wolopts;
+
+out_ops:
+	ethnl_ops_complete(dev);
+out_rtnl:
+	rtnl_unlock();
+	dev_put(dev);
+	return ret;
+}
-- 
2.25.0

[PATCH net-next 7/7] ethtool: add WOL_NTF notification

[Michal Kubecek]

Send ETHTOOL_MSG_WOL_NTF notification whenever wake-on-lan settings of
a device are modified using ETHTOOL_MSG_WOL_SET netlink message or
ETHTOOL_SWOL ioctl request.

As notifications can be received by anyone, do not include SecureOn(tm)
password in notification messages.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Documentation/networking/ethtool-netlink.rst | 5 +++--
 include/uapi/linux/ethtool_netlink.h         | 1 +
 net/ethtool/ioctl.c                          | 1 +
 net/ethtool/netlink.c                        | 2 ++
 net/ethtool/wol.c                            | 4 +++-
 5 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index f16f74bbb546..f1f868479ceb 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -193,7 +193,7 @@ Userspace to kernel:
 
 Kernel to userspace:
 
-  ===================================== ================================
+  ===================================== =================================
   ``ETHTOOL_MSG_STRSET_GET_REPLY``      string set contents
   ``ETHTOOL_MSG_LINKINFO_GET_REPLY``    link settings
   ``ETHTOOL_MSG_LINKINFO_NTF``          link settings notification
@@ -203,7 +203,8 @@ Kernel to userspace:
   ``ETHTOOL_MSG_DEBUG_GET_REPLY``       debugging settings
   ``ETHTOOL_MSG_DEBUG_NTF``             debugging settings notification
   ``ETHTOOL_MSG_WOL_GET_REPLY``         wake-on-lan settings
-  ===================================== ================================
+  ``ETHTOOL_MSG_WOL_NTF``               wake-on-lan settings notification
+  ===================================== =================================
 
 ``GET`` requests are sent by userspace applications to retrieve device
 information. They usually do not contain any message specific attributes.
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 59de35695521..7e0b460f872c 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -42,6 +42,7 @@ enum {
 	ETHTOOL_MSG_DEBUG_GET_REPLY,
 	ETHTOOL_MSG_DEBUG_NTF,
 	ETHTOOL_MSG_WOL_GET_REPLY,
+	ETHTOOL_MSG_WOL_NTF,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 46e0b31782fc..b88dd14e41c6 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1316,6 +1316,7 @@ static int ethtool_set_wol(struct net_device *dev, char __user *useraddr)
 		return ret;
 
 	dev->wol_enabled = !!wol.wolopts;
+	ethtool_notify(dev, ETHTOOL_MSG_WOL_NTF, NULL);
 
 	return 0;
 }
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 2c375f9095fe..180c194fab07 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -526,6 +526,7 @@ ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = {
 	[ETHTOOL_MSG_LINKINFO_NTF]	= &ethnl_linkinfo_request_ops,
 	[ETHTOOL_MSG_LINKMODES_NTF]	= &ethnl_linkmodes_request_ops,
 	[ETHTOOL_MSG_DEBUG_NTF]		= &ethnl_debug_request_ops,
+	[ETHTOOL_MSG_WOL_NTF]		= &ethnl_wol_request_ops,
 };
 
 /* default notification handler */
@@ -610,6 +611,7 @@ static const ethnl_notify_handler_t ethnl_notify_handlers[] = {
 	[ETHTOOL_MSG_LINKINFO_NTF]	= ethnl_default_notify,
 	[ETHTOOL_MSG_LINKMODES_NTF]	= ethnl_default_notify,
 	[ETHTOOL_MSG_DEBUG_NTF]		= ethnl_default_notify,
+	[ETHTOOL_MSG_WOL_NTF]		= ethnl_default_notify,
 };
 
 void ethtool_notify(struct net_device *dev, unsigned int cmd, const void *data)
diff --git a/net/ethtool/wol.c b/net/ethtool/wol.c
index a2724378fac4..e1b8a65b64c4 100644
--- a/net/ethtool/wol.c
+++ b/net/ethtool/wol.c
@@ -41,7 +41,8 @@ static int wol_prepare_data(const struct ethnl_req_info *req_base,
 		return ret;
 	dev->ethtool_ops->get_wol(dev, &data->wol);
 	ethnl_ops_complete(dev);
-	data->show_sopass = data->wol.supported & WAKE_MAGICSECURE;
+	/* do not include password in notifications */
+	data->show_sopass = info && (data->wol.supported & WAKE_MAGICSECURE);
 
 	return 0;
 }
@@ -165,6 +166,7 @@ int ethnl_set_wol(struct sk_buff *skb, struct genl_info *info)
 	if (ret)
 		goto out_ops;
 	dev->wol_enabled = !!wol.wolopts;
+	ethtool_notify(dev, ETHTOOL_MSG_WOL_NTF, NULL);
 
 out_ops:
 	ethnl_ops_complete(dev);
-- 
2.25.0

Re: [PATCH net-next 0/7] ethtool netlink interface, part 2

[Andrew Lunn]

On Sun, Jan 26, 2020 at 11:10:58PM +0100, Michal Kubecek wrote:
> This shorter series adds support for getting and setting of wake-on-lan
> settings and message mask (originally message level). Together with the
> code already in net-next, this will allow full implementation of
> "ethtool <dev>" and "ethtool -s <dev> ...".

Hi Michal

It is nice to see more of this work being posted. But what about the
user space side? Do you plan to post that soon?

Thanks
	Andrew

Re: [PATCH net-next 2/7] ethtool: provide message mask with DEBUG_GET request

[Andrew Lunn]

On Sun, Jan 26, 2020 at 11:11:04PM +0100, Michal Kubecek wrote:
> Implement DEBUG_GET request to get debugging settings for a device. At the
> moment, only message mask corresponding to message level as reported by
> ETHTOOL_GMSGLVL ioctl request is provided. (It is called message level in
> ioctl interface but almost all drivers interpret it as a bit mask.)
> 
> As part of the implementation, provide symbolic names for message mask bits
> as ETH_SS_MSG_CLASSES string set.
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 3/7] ethtool: set message mask with DEBUG_SET request

[Andrew Lunn]

On Sun, Jan 26, 2020 at 11:11:07PM +0100, Michal Kubecek wrote:
> Implement DEBUG_SET netlink request to set debugging settings for a device.
> At the moment, only message mask corresponding to message level as set by
> ETHTOOL_SMSGLVL ioctl request can be set. (It is called message level in
> ioctl interface but almost all drivers interpret it as a bit mask.)
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> +int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct nlattr *tb[ETHTOOL_A_DEBUG_MAX + 1];
> +	struct ethnl_req_info req_info = {};
> +	struct net_device *dev;
> +	bool mod = false;
> +	u32 msg_mask;
> +	int ret;
> +
> +	ret = nlmsg_parse(info->nlhdr, GENL_HDRLEN, tb,
> +			  ETHTOOL_A_DEBUG_MAX, debug_set_policy,
> +			  info->extack);
> +	if (ret < 0)
> +		return ret;
> +	ret = ethnl_parse_header(&req_info, tb[ETHTOOL_A_DEBUG_HEADER],
> +				 genl_info_net(info), info->extack, true);
> +	if (ret < 0)
> +		return ret;
> +	dev = req_info.dev;
> +	if (!dev->ethtool_ops->get_msglevel || !dev->ethtool_ops->set_msglevel)
> +		return -EOPNOTSUPP;

This seems like a new requirement, that both get and set callbacks are
implemented. However, A quick look thought the code suggests all
drivers already do have both get and set. So i think this is safe.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 3/7] ethtool: set message mask with DEBUG_SET request

[Andrew Lunn]

> +	ret = ethnl_parse_header(&req_info, tb[ETHTOOL_A_DEBUG_HEADER],
> +				 genl_info_net(info), info->extack, true);

> +	dev_put(dev);

Hi Michal

While reviewing this patch i noticed this dev_put() and wondered where
the dev_get() was. It is hiding inside ethnl_parse_header(). The
documentation does make it clear it takes a reference on the device,
but how many people read the documentation? I would not be too
surprised if at some point in the future we have bugs from missing
dev_put().

Could we make this a bit more explicit by calling it
ethnl_parse_header_dev_get(). It is rather long though.

     Andrew

Re: [PATCH net-next 4/7] ethtool: add DEBUG_NTF notification

[Andrew Lunn]

On Sun, Jan 26, 2020 at 11:11:10PM +0100, Michal Kubecek wrote:
> Send ETHTOOL_MSG_DEBUG_NTF notification message whenever debugging message
> mask for a device are modified using ETHTOOL_MSG_DEBUG_SET netlink message
> or ETHTOOL_SMSGLVL ioctl request.
> 
> The notification message has the same format as reply to DEBUG_GET request.
> As with other ethtool notifications, netlink requests only trigger the
> notification if the mask is actually changed while ioctl request trigger it
> whenever the request results in calling the ethtool_ops handler.
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 5/7] ethtool: provide WoL settings with WOL_GET request

[Andrew Lunn]

On Sun, Jan 26, 2020 at 11:11:13PM +0100, Michal Kubecek wrote:
> Implement WOL_GET request to get wake-on-lan settings for a device,
> traditionally available via ETHTOOL_GWOL ioctl request.
> 
> As part of the implementation, provide symbolic names for wake-on-line

Hi Michal

LAN, not line.

> modes as ETH_SS_WOL_MODES string set.

I think it would also be good to repeat here what you have in the
cover note, that unlike the ioctl version, CAP_NET_ADMIN is required.
The cover note does get committed to the git history, but it is a lot
less likely to be seen than this patches commit message.

> +WOL_GET
> +=======
> +
> +Query device wake-on-lan settings. Unlike most "GET" type requests,
> +``ETHTOOL_MSG_WOL_GET`` requires (netns) ``CAP_NET_ADMIN`` privileges as it
> +(potentially) provides SecureOn(tm) password which is confidential.

provides _the_ SecureOn(tm)

Apart from that:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 3/7] ethtool: set message mask with DEBUG_SET request

[Michal Kubecek]

On Mon, Jan 27, 2020 at 01:22:06AM +0100, Andrew Lunn wrote:
> On Sun, Jan 26, 2020 at 11:11:07PM +0100, Michal Kubecek wrote:
> > Implement DEBUG_SET netlink request to set debugging settings for a device.
> > At the moment, only message mask corresponding to message level as set by
> > ETHTOOL_SMSGLVL ioctl request can be set. (It is called message level in
> > ioctl interface but almost all drivers interpret it as a bit mask.)
> > 
> > Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> > +int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info)
> > +{
> > +	struct nlattr *tb[ETHTOOL_A_DEBUG_MAX + 1];
> > +	struct ethnl_req_info req_info = {};
> > +	struct net_device *dev;
> > +	bool mod = false;
> > +	u32 msg_mask;
> > +	int ret;
> > +
> > +	ret = nlmsg_parse(info->nlhdr, GENL_HDRLEN, tb,
> > +			  ETHTOOL_A_DEBUG_MAX, debug_set_policy,
> > +			  info->extack);
> > +	if (ret < 0)
> > +		return ret;
> > +	ret = ethnl_parse_header(&req_info, tb[ETHTOOL_A_DEBUG_HEADER],
> > +				 genl_info_net(info), info->extack, true);
> > +	if (ret < 0)
> > +		return ret;
> > +	dev = req_info.dev;
> > +	if (!dev->ethtool_ops->get_msglevel || !dev->ethtool_ops->set_msglevel)
> > +		return -EOPNOTSUPP;
> 
> This seems like a new requirement, that both get and set callbacks are
> implemented. However, A quick look thought the code suggests all
> drivers already do have both get and set. So i think this is safe.

Technically it's a new requirement but as ethtool (userspace utility)
always issues ETHTOOL_GMSGLVL before ETHTOOL_SMSGLVL and does so even
for command lines like "ethtool -s eth0 msglvl 5" where it's not really
needed, providing ->set_msglevel() without ->get_msglevel() wouldn't
be of much use even now.

Michal

> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew

Re: [PATCH net-next 3/7] ethtool: set message mask with DEBUG_SET request

[Michal Kubecek]

On Mon, Jan 27, 2020 at 02:04:22AM +0100, Andrew Lunn wrote:
> > +	ret = ethnl_parse_header(&req_info, tb[ETHTOOL_A_DEBUG_HEADER],
> > +				 genl_info_net(info), info->extack, true);
> 
> > +	dev_put(dev);
> 
> Hi Michal
> 
> While reviewing this patch i noticed this dev_put() and wondered where
> the dev_get() was. It is hiding inside ethnl_parse_header(). The
> documentation does make it clear it takes a reference on the device,
> but how many people read the documentation? I would not be too
> surprised if at some point in the future we have bugs from missing
> dev_put().
> 
> Could we make this a bit more explicit by calling it
> ethnl_parse_header_dev_get(). It is rather long though.

Good point, I'll think about the name some more and send a cleanup patch
when net-next reopens.

Michal

Re: [PATCH net-next 0/7] ethtool netlink interface, part 2

[Michal Kubecek]

On Mon, Jan 27, 2020 at 12:34:48AM +0100, Andrew Lunn wrote:
> On Sun, Jan 26, 2020 at 11:10:58PM +0100, Michal Kubecek wrote:
> > This shorter series adds support for getting and setting of wake-on-lan
> > settings and message mask (originally message level). Together with the
> > code already in net-next, this will allow full implementation of
> > "ethtool <dev>" and "ethtool -s <dev> ...".
> 
> Hi Michal
> 
> It is nice to see more of this work being posted. But what about the
> user space side? Do you plan to post that soon?

Yes, once I get it into presentable shape. I definitely want ethtool 5.6
to support what kernel 5.6 will.

Michal

Re: [PATCH net-next 0/7] ethtool netlink interface, part 2

[David Miller]

From: Michal Kubecek <mkubecek@suse.cz>
Date: Sun, 26 Jan 2020 23:10:58 +0100 (CET)

> This shorter series adds support for getting and setting of wake-on-lan
> settings and message mask (originally message level). Together with the
> code already in net-next, this will allow full implementation of
> "ethtool <dev>" and "ethtool -s <dev> ...".
> 
> Older versions of the ethtool netlink series allowed getting WoL settings
> by unprivileged users and only filtered out the password but this was
> a source of controversy so for now, ETHTOOL_MSG_WOL_GET request always
> requires CAP_NET_ADMIN as ETHTOOL_GWOL ioctl request does.

It looks like this will need to be respun at least once, and net-next
is closing today so....

Re: [PATCH net-next 0/7] ethtool netlink interface, part 2

[Michal Kubecek]

On Mon, Jan 27, 2020 at 10:40:49AM +0100, David Miller wrote:
> From: Michal Kubecek <mkubecek@suse.cz>
> Date: Sun, 26 Jan 2020 23:10:58 +0100 (CET)
> 
> > This shorter series adds support for getting and setting of wake-on-lan
> > settings and message mask (originally message level). Together with the
> > code already in net-next, this will allow full implementation of
> > "ethtool <dev>" and "ethtool -s <dev> ...".
> > 
> > Older versions of the ethtool netlink series allowed getting WoL settings
> > by unprivileged users and only filtered out the password but this was
> > a source of controversy so for now, ETHTOOL_MSG_WOL_GET request always
> > requires CAP_NET_ADMIN as ETHTOOL_GWOL ioctl request does.
> 
> It looks like this will need to be respun at least once, and net-next
> is closing today so....

The problem with ethnl_parse_header() name not making it obvious that it
takes a reference is not introduced in this series, the function is
already in net-next so that it does not matter if this series is merged
or not. Other than that, there is only the missing "the" in
documentation.

Michal

Re: [PATCH net-next 0/7] ethtool netlink interface, part 2

[David Miller]

From: Michal Kubecek <mkubecek@suse.cz>
Date: Mon, 27 Jan 2020 10:57:44 +0100

> On Mon, Jan 27, 2020 at 10:40:49AM +0100, David Miller wrote:
>> From: Michal Kubecek <mkubecek@suse.cz>
>> Date: Sun, 26 Jan 2020 23:10:58 +0100 (CET)
>> 
>> > This shorter series adds support for getting and setting of wake-on-lan
>> > settings and message mask (originally message level). Together with the
>> > code already in net-next, this will allow full implementation of
>> > "ethtool <dev>" and "ethtool -s <dev> ...".
>> > 
>> > Older versions of the ethtool netlink series allowed getting WoL settings
>> > by unprivileged users and only filtered out the password but this was
>> > a source of controversy so for now, ETHTOOL_MSG_WOL_GET request always
>> > requires CAP_NET_ADMIN as ETHTOOL_GWOL ioctl request does.
>> 
>> It looks like this will need to be respun at least once, and net-next
>> is closing today so....
> 
> The problem with ethnl_parse_header() name not making it obvious that it
> takes a reference is not introduced in this series, the function is
> already in net-next so that it does not matter if this series is merged
> or not. Other than that, there is only the missing "the" in
> documentation.

Ok, I'll reconsider, thanks.

Re: [PATCH net-next 0/7] ethtool netlink interface, part 2

[David Miller]

From: Michal Kubecek <mkubecek@suse.cz>
Date: Mon, 27 Jan 2020 10:57:44 +0100

> On Mon, Jan 27, 2020 at 10:40:49AM +0100, David Miller wrote:
>> From: Michal Kubecek <mkubecek@suse.cz>
>> Date: Sun, 26 Jan 2020 23:10:58 +0100 (CET)
>> 
>> > This shorter series adds support for getting and setting of wake-on-lan
>> > settings and message mask (originally message level). Together with the
>> > code already in net-next, this will allow full implementation of
>> > "ethtool <dev>" and "ethtool -s <dev> ...".
>> > 
>> > Older versions of the ethtool netlink series allowed getting WoL settings
>> > by unprivileged users and only filtered out the password but this was
>> > a source of controversy so for now, ETHTOOL_MSG_WOL_GET request always
>> > requires CAP_NET_ADMIN as ETHTOOL_GWOL ioctl request does.
>> 
>> It looks like this will need to be respun at least once, and net-next
>> is closing today so....
> 
> The problem with ethnl_parse_header() name not making it obvious that it
> takes a reference is not introduced in this series, the function is
> already in net-next so that it does not matter if this series is merged
> or not. Other than that, there is only the missing "the" in
> documentation.

Ok, looks good, series applied.

Thanky you.

Re: [PATCH net-next 0/7] ethtool netlink interface, part 2

[Andrew Lunn]

On Mon, Jan 27, 2020 at 11:32:39AM +0100, David Miller wrote:
> From: Michal Kubecek <mkubecek@suse.cz>
> Date: Mon, 27 Jan 2020 10:57:44 +0100
> 
> > On Mon, Jan 27, 2020 at 10:40:49AM +0100, David Miller wrote:
> >> From: Michal Kubecek <mkubecek@suse.cz>
> >> Date: Sun, 26 Jan 2020 23:10:58 +0100 (CET)
> >> 
> >> > This shorter series adds support for getting and setting of wake-on-lan
> >> > settings and message mask (originally message level). Together with the
> >> > code already in net-next, this will allow full implementation of
> >> > "ethtool <dev>" and "ethtool -s <dev> ...".
> >> > 
> >> > Older versions of the ethtool netlink series allowed getting WoL settings
> >> > by unprivileged users and only filtered out the password but this was
> >> > a source of controversy so for now, ETHTOOL_MSG_WOL_GET request always
> >> > requires CAP_NET_ADMIN as ETHTOOL_GWOL ioctl request does.
> >> 
> >> It looks like this will need to be respun at least once, and net-next
> >> is closing today so....
> > 
> > The problem with ethnl_parse_header() name not making it obvious that it
> > takes a reference is not introduced in this series, the function is
> > already in net-next so that it does not matter if this series is merged
> > or not. Other than that, there is only the missing "the" in
> > documentation.
> 
> Ok, looks good, series applied.

Yes, i don't see anything here which cannot be fixed up later. So i
agree with merging it.

      Andrew