[PATCH net-next 1/2] ethtool: netlink: handle SET intro/outro in the common code

[PATCH net-next 1/2] ethtool: netlink: handle SET intro/outro in the common code

[Jakub Kicinski]

Most ethtool SET callbacks follow the same general structure.

  ethnl_parse_header_dev_get()
  rtnl_lock()
  ethnl_ops_begin()

  ... do stuff ...

  ethtool_notify()
  ethnl_ops_complete()
  rtnl_unlock()
  ethnl_parse_header_dev_put()

This leads to a lot of copy / pasted code an bugs when people
mis-handle the error path.

Add a generic implementation of this pattern with a .set callback
in struct ethnl_request_ops called to "do stuff".

Also add an optional .set_validate which is called before
ethnl_ops_begin() -- a lot of implementations do basic request
capability / sanity checking at that point.

Because we want to avoid generating the notification when
no change happened - adopt a slightly hairy return values:
 - 0 means nothing to do (no notification)
 - 1 means done / continue
 - negative error codes on error

Reuse .hdr_attr from struct ethnl_request_ops, GET and SET
use the same attr spaces in all cases.

Convert pause as an example (and to avoid unused function warnings).

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: mkubecek@suse.cz
---
 net/ethtool/netlink.c | 49 ++++++++++++++++++++++++++-
 net/ethtool/netlink.h | 22 +++++++++++--
 net/ethtool/pause.c   | 77 +++++++++++++++++--------------------------
 3 files changed, 99 insertions(+), 49 deletions(-)

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 9f924875bba9..31eaff9eb90b 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -279,6 +279,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_CHANNELS_GET]	= &ethnl_channels_request_ops,
 	[ETHTOOL_MSG_COALESCE_GET]	= &ethnl_coalesce_request_ops,
 	[ETHTOOL_MSG_PAUSE_GET]		= &ethnl_pause_request_ops,
+	[ETHTOOL_MSG_PAUSE_SET]		= &ethnl_pause_request_ops,
 	[ETHTOOL_MSG_EEE_GET]		= &ethnl_eee_request_ops,
 	[ETHTOOL_MSG_FEC_GET]		= &ethnl_fec_request_ops,
 	[ETHTOOL_MSG_TSINFO_GET]	= &ethnl_tsinfo_request_ops,
@@ -590,6 +591,52 @@ static int ethnl_default_done(struct netlink_callback *cb)
 	return 0;
 }
 
+static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	const struct ethnl_request_ops *ops;
+	struct ethnl_req_info req_info = {};
+	const u8 cmd = info->genlhdr->cmd;
+	int ret;
+
+	ops = ethnl_default_requests[cmd];
+	if (WARN_ONCE(!ops, "cmd %u has no ethnl_request_ops\n", cmd))
+		return -EOPNOTSUPP;
+	if (GENL_REQ_ATTR_CHECK(info, ops->hdr_attr))
+		return -EINVAL;
+
+	ret = ethnl_parse_header_dev_get(&req_info, info->attrs[ops->hdr_attr],
+					 genl_info_net(info), info->extack,
+					 true);
+	if (ret < 0)
+		return ret;
+
+	if (ops->set_validate) {
+		ret = ops->set_validate(&req_info, info);
+		/* 0 means nothing to do */
+		if (ret <= 0)
+			goto out_dev;
+	}
+
+	rtnl_lock();
+	ret = ethnl_ops_begin(req_info.dev);
+	if (ret < 0)
+		goto out_rtnl;
+
+	ret = ops->set(&req_info, info);
+	if (ret <= 0)
+		goto out_ops;
+	ethtool_notify(req_info.dev, ops->set_ntf_cmd, NULL);
+
+	ret = 0;
+out_ops:
+	ethnl_ops_complete(req_info.dev);
+out_rtnl:
+	rtnl_unlock();
+out_dev:
+	ethnl_parse_header_dev_put(&req_info);
+	return ret;
+}
+
 static const struct ethnl_request_ops *
 ethnl_default_notify_ops[ETHTOOL_MSG_KERNEL_MAX + 1] = {
 	[ETHTOOL_MSG_LINKINFO_NTF]	= &ethnl_linkinfo_request_ops,
@@ -918,7 +965,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_PAUSE_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_pause,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_pause_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_pause_set_policy) - 1,
 	},
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index f271266f6e28..e6cd1e5b141e 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -258,13 +258,14 @@ int ethnl_ops_begin(struct net_device *dev);
 void ethnl_ops_complete(struct net_device *dev);
 
 /**
- * struct ethnl_request_ops - unified handling of GET requests
+ * struct ethnl_request_ops - unified handling of GET and SET requests
  * @request_cmd:      command id for request (GET)
  * @reply_cmd:        command id for reply (GET_REPLY)
  * @hdr_attr:         attribute type for request header
  * @req_info_size:    size of request info
  * @reply_data_size:  size of reply data
  * @allow_nodev_do:   allow non-dump request with no device identification
+ * @set_ntf_cmd:      notification to generate on changes (SET)
  * @parse_request:
  *	Parse request except common header (struct ethnl_req_info). Common
  *	header is already filled on entry, the rest up to @repdata_offset
@@ -293,6 +294,18 @@ void ethnl_ops_complete(struct net_device *dev);
  *	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.
+ * @set_validate:
+ *	Check if set operation is supported for a given device, and perform
+ *	extra input checks. Expected return values:
+ *	 - 0 if the operation is a noop for the device (rare)
+ *	 - 1 if operation should proceed to calling @set
+ *	 - negative errno on errors
+ *	Called without any locks, just a reference on the netdev.
+ * @set:
+ *	Execute the set operation. The implementation should return
+ *	 - 0 if no configuration has changed
+ *	 - 1 if configuration changed and notification should be generated
+ *	 - negative errno on errors
  *
  * Description of variable parts of GET request handling when using the
  * unified infrastructure. When used, a pointer to an instance of this
@@ -309,6 +322,7 @@ struct ethnl_request_ops {
 	unsigned int		req_info_size;
 	unsigned int		reply_data_size;
 	bool			allow_nodev_do;
+	u8			set_ntf_cmd;
 
 	int (*parse_request)(struct ethnl_req_info *req_info,
 			     struct nlattr **tb,
@@ -322,6 +336,11 @@ struct ethnl_request_ops {
 			  const struct ethnl_req_info *req_info,
 			  const struct ethnl_reply_data *reply_data);
 	void (*cleanup_data)(struct ethnl_reply_data *reply_data);
+
+	int (*set_validate)(struct ethnl_req_info *req_info,
+			    struct genl_info *info);
+	int (*set)(struct ethnl_req_info *req_info,
+		   struct genl_info *info);
 };
 
 /* request handlers */
@@ -403,7 +422,6 @@ int ethnl_set_privflags(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_channels(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_coalesce(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_pause(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_eee(struct sk_buff *skb, struct genl_info *info);
 int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);
 int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info);
diff --git a/net/ethtool/pause.c b/net/ethtool/pause.c
index a8c113d244db..8e9aced3eeec 100644
--- a/net/ethtool/pause.c
+++ b/net/ethtool/pause.c
@@ -114,18 +114,6 @@ static int pause_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_pause_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_PAUSE_GET,
-	.reply_cmd		= ETHTOOL_MSG_PAUSE_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_PAUSE_HEADER,
-	.req_info_size		= sizeof(struct pause_req_info),
-	.reply_data_size	= sizeof(struct pause_reply_data),
-
-	.prepare_data		= pause_prepare_data,
-	.reply_size		= pause_reply_size,
-	.fill_reply		= pause_fill_reply,
-};
-
 /* PAUSE_SET */
 
 const struct nla_policy ethnl_pause_set_policy[] = {
@@ -136,51 +124,48 @@ const struct nla_policy ethnl_pause_set_policy[] = {
 	[ETHTOOL_A_PAUSE_TX]			= { .type = NLA_U8 },
 };
 
-int ethnl_set_pause(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_pause_validate(struct ethnl_req_info *req_info,
+			 struct genl_info *info)
 {
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+
+	return ops->get_pauseparam && ops->set_pauseparam ? 1 : -EOPNOTSUPP;
+}
+
+static int
+ethnl_set_pause(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	struct net_device *dev = req_info->dev;
 	struct ethtool_pauseparam params = {};
-	struct ethnl_req_info req_info = {};
 	struct nlattr **tb = info->attrs;
-	const struct ethtool_ops *ops;
-	struct net_device *dev;
 	bool mod = false;
 	int ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_PAUSE_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-	dev = req_info.dev;
-	ops = dev->ethtool_ops;
-	ret = -EOPNOTSUPP;
-	if (!ops->get_pauseparam || !ops->set_pauseparam)
-		goto out_dev;
-
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
-	ops->get_pauseparam(dev, &params);
+	dev->ethtool_ops->get_pauseparam(dev, &params);
 
 	ethnl_update_bool32(&params.autoneg, tb[ETHTOOL_A_PAUSE_AUTONEG], &mod);
 	ethnl_update_bool32(&params.rx_pause, tb[ETHTOOL_A_PAUSE_RX], &mod);
 	ethnl_update_bool32(&params.tx_pause, tb[ETHTOOL_A_PAUSE_TX], &mod);
-	ret = 0;
 	if (!mod)
-		goto out_ops;
+		return 0;
 
 	ret = dev->ethtool_ops->set_pauseparam(dev, &params);
-	if (ret < 0)
-		goto out_ops;
-	ethtool_notify(dev, ETHTOOL_MSG_PAUSE_NTF, NULL);
-
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return ret < 0 ? ret : 1;
 }
+
+const struct ethnl_request_ops ethnl_pause_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_PAUSE_GET,
+	.reply_cmd		= ETHTOOL_MSG_PAUSE_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_PAUSE_HEADER,
+	.req_info_size		= sizeof(struct pause_req_info),
+	.reply_data_size	= sizeof(struct pause_reply_data),
+
+	.prepare_data		= pause_prepare_data,
+	.reply_size		= pause_reply_size,
+	.fill_reply		= pause_fill_reply,
+
+	.set_validate		= ethnl_set_pause_validate,
+	.set			= ethnl_set_pause,
+	.set_ntf_cmd		= ETHTOOL_MSG_PAUSE_NTF,
+};
-- 
2.39.0

[PATCH net-next 2/2] ethtool: netlink: convert commands to common SET

[Jakub Kicinski]

Convert all SET commands where new common code is applicable.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: piergiorgio.beruto@gmail.com
CC: gal@nvidia.com
CC: tariqt@nvidia.com
CC: dnlplm@gmail.com
CC: sean.anderson@seco.com
CC: linux@rempel-privat.de
CC: lkp@intel.com
CC: bagasdotme@gmail.com
CC: wangjie125@huawei.com
CC: huangguangbin2@huawei.com
---
 net/ethtool/channels.c  |  92 ++++++++++++++----------------------
 net/ethtool/coalesce.c  |  93 ++++++++++++++++--------------------
 net/ethtool/debug.c     |  71 ++++++++++++----------------
 net/ethtool/eee.c       |  78 ++++++++++++-------------------
 net/ethtool/fec.c       |  83 +++++++++++++--------------------
 net/ethtool/linkinfo.c  |  81 ++++++++++++++------------------
 net/ethtool/linkmodes.c |  91 +++++++++++++++++-------------------
 net/ethtool/module.c    |  89 ++++++++++++++---------------------
 net/ethtool/netlink.c   |  39 ++++++++++------
 net/ethtool/netlink.h   |  13 ------
 net/ethtool/plca.c      |  75 +++++++++--------------------
 net/ethtool/privflags.c |  84 ++++++++++++++++-----------------
 net/ethtool/pse-pd.c    |  79 ++++++++++++-------------------
 net/ethtool/rings.c     | 101 +++++++++++++++++-----------------------
 net/ethtool/wol.c       |  79 +++++++++++++------------------
 15 files changed, 474 insertions(+), 674 deletions(-)

diff --git a/net/ethtool/channels.c b/net/ethtool/channels.c
index c7e37130647e..61c40e889a4d 100644
--- a/net/ethtool/channels.c
+++ b/net/ethtool/channels.c
@@ -86,18 +86,6 @@ static int channels_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_channels_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_CHANNELS_GET,
-	.reply_cmd		= ETHTOOL_MSG_CHANNELS_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_CHANNELS_HEADER,
-	.req_info_size		= sizeof(struct channels_req_info),
-	.reply_data_size	= sizeof(struct channels_reply_data),
-
-	.prepare_data		= channels_prepare_data,
-	.reply_size		= channels_reply_size,
-	.fill_reply		= channels_fill_reply,
-};
-
 /* CHANNELS_SET */
 
 const struct nla_policy ethnl_channels_set_policy[] = {
@@ -109,36 +97,28 @@ const struct nla_policy ethnl_channels_set_policy[] = {
 	[ETHTOOL_A_CHANNELS_COMBINED_COUNT]	= { .type = NLA_U32 },
 };
 
-int ethnl_set_channels(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_channels_validate(struct ethnl_req_info *req_info,
+			    struct genl_info *info)
+{
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+
+	return ops->get_channels && ops->set_channels ? 1 : -EOPNOTSUPP;
+}
+
+static int
+ethnl_set_channels(struct ethnl_req_info *req_info, struct genl_info *info)
 {
 	unsigned int from_channel, old_total, i;
 	bool mod = false, mod_combined = false;
+	struct net_device *dev = req_info->dev;
 	struct ethtool_channels channels = {};
-	struct ethnl_req_info req_info = {};
 	struct nlattr **tb = info->attrs;
 	u32 err_attr, max_rxfh_in_use;
-	const struct ethtool_ops *ops;
-	struct net_device *dev;
 	u64 max_rxnfc_in_use;
 	int ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_CHANNELS_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-	dev = req_info.dev;
-	ops = dev->ethtool_ops;
-	ret = -EOPNOTSUPP;
-	if (!ops->get_channels || !ops->set_channels)
-		goto out_dev;
-
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
-	ops->get_channels(dev, &channels);
+	dev->ethtool_ops->get_channels(dev, &channels);
 	old_total = channels.combined_count +
 		    max(channels.rx_count, channels.tx_count);
 
@@ -151,9 +131,8 @@ int ethnl_set_channels(struct sk_buff *skb, struct genl_info *info)
 	ethnl_update_u32(&channels.combined_count,
 			 tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT], &mod_combined);
 	mod |= mod_combined;
-	ret = 0;
 	if (!mod)
-		goto out_ops;
+		return 0;
 
 	/* ensure new channel counts are within limits */
 	if (channels.rx_count > channels.max_rx)
@@ -167,10 +146,9 @@ int ethnl_set_channels(struct sk_buff *skb, struct genl_info *info)
 	else
 		err_attr = 0;
 	if (err_attr) {
-		ret = -EINVAL;
 		NL_SET_ERR_MSG_ATTR(info->extack, tb[err_attr],
 				    "requested channel count exceeds maximum");
-		goto out_ops;
+		return -EINVAL;
 	}
 
 	/* ensure there is at least one RX and one TX channel */
@@ -183,10 +161,9 @@ int ethnl_set_channels(struct sk_buff *skb, struct genl_info *info)
 	if (err_attr) {
 		if (mod_combined)
 			err_attr = ETHTOOL_A_CHANNELS_COMBINED_COUNT;
-		ret = -EINVAL;
 		NL_SET_ERR_MSG_ATTR(info->extack, tb[err_attr],
 				    "requested channel counts would result in no RX or TX channel being configured");
-		goto out_ops;
+		return -EINVAL;
 	}
 
 	/* ensure the new Rx count fits within the configured Rx flow
@@ -198,14 +175,12 @@ int ethnl_set_channels(struct sk_buff *skb, struct genl_info *info)
 	    ethtool_get_max_rxfh_channel(dev, &max_rxfh_in_use))
 		max_rxfh_in_use = 0;
 	if (channels.combined_count + channels.rx_count <= max_rxfh_in_use) {
-		ret = -EINVAL;
 		GENL_SET_ERR_MSG(info, "requested channel counts are too low for existing indirection table settings");
-		goto out_ops;
+		return -EINVAL;
 	}
 	if (channels.combined_count + channels.rx_count <= max_rxnfc_in_use) {
-		ret = -EINVAL;
 		GENL_SET_ERR_MSG(info, "requested channel counts are too low for existing ntuple filter settings");
-		goto out_ops;
+		return -EINVAL;
 	}
 
 	/* Disabling channels, query zero-copy AF_XDP sockets */
@@ -213,21 +188,26 @@ int ethnl_set_channels(struct sk_buff *skb, struct genl_info *info)
 		       min(channels.rx_count, channels.tx_count);
 	for (i = from_channel; i < old_total; i++)
 		if (xsk_get_pool_from_qid(dev, i)) {
-			ret = -EINVAL;
 			GENL_SET_ERR_MSG(info, "requested channel counts are too low for existing zerocopy AF_XDP sockets");
-			goto out_ops;
+			return -EINVAL;
 		}
 
 	ret = dev->ethtool_ops->set_channels(dev, &channels);
-	if (ret < 0)
-		goto out_ops;
-	ethtool_notify(dev, ETHTOOL_MSG_CHANNELS_NTF, NULL);
-
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return ret < 0 ? ret : 1;
 }
+
+const struct ethnl_request_ops ethnl_channels_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_CHANNELS_GET,
+	.reply_cmd		= ETHTOOL_MSG_CHANNELS_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_CHANNELS_HEADER,
+	.req_info_size		= sizeof(struct channels_req_info),
+	.reply_data_size	= sizeof(struct channels_reply_data),
+
+	.prepare_data		= channels_prepare_data,
+	.reply_size		= channels_reply_size,
+	.fill_reply		= channels_fill_reply,
+
+	.set_validate		= ethnl_set_channels_validate,
+	.set			= ethnl_set_channels,
+	.set_ntf_cmd		= ETHTOOL_MSG_CHANNELS_NTF,
+};
diff --git a/net/ethtool/coalesce.c b/net/ethtool/coalesce.c
index e405b47f7eed..564804a0759c 100644
--- a/net/ethtool/coalesce.c
+++ b/net/ethtool/coalesce.c
@@ -195,18 +195,6 @@ static int coalesce_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_coalesce_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_COALESCE_GET,
-	.reply_cmd		= ETHTOOL_MSG_COALESCE_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_COALESCE_HEADER,
-	.req_info_size		= sizeof(struct coalesce_req_info),
-	.reply_data_size	= sizeof(struct coalesce_reply_data),
-
-	.prepare_data		= coalesce_prepare_data,
-	.reply_size		= coalesce_reply_size,
-	.fill_reply		= coalesce_fill_reply,
-};
-
 /* COALESCE_SET */
 
 const struct nla_policy ethnl_coalesce_set_policy[] = {
@@ -241,49 +229,41 @@ const struct nla_policy ethnl_coalesce_set_policy[] = {
 	[ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS] = { .type = NLA_U32 },
 };
 
-int ethnl_set_coalesce(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_coalesce_validate(struct ethnl_req_info *req_info,
+			    struct genl_info *info)
 {
-	struct kernel_ethtool_coalesce kernel_coalesce = {};
-	struct ethtool_coalesce coalesce = {};
-	struct ethnl_req_info req_info = {};
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
 	struct nlattr **tb = info->attrs;
-	const struct ethtool_ops *ops;
-	struct net_device *dev;
 	u32 supported_params;
-	bool mod = false;
-	int ret;
 	u16 a;
 
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_COALESCE_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-	dev = req_info.dev;
-	ops = dev->ethtool_ops;
-	ret = -EOPNOTSUPP;
-	if (!ops->get_coalesce || !ops->set_coalesce)
-		goto out_dev;
-
 	/* make sure that only supported parameters are present */
 	supported_params = ops->supported_coalesce_params;
 	for (a = ETHTOOL_A_COALESCE_RX_USECS; a < __ETHTOOL_A_COALESCE_CNT; a++)
 		if (tb[a] && !(supported_params & attr_to_mask(a))) {
-			ret = -EINVAL;
 			NL_SET_ERR_MSG_ATTR(info->extack, tb[a],
 					    "cannot modify an unsupported parameter");
-			goto out_dev;
+			return -EINVAL;
 		}
 
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
-	ret = ops->get_coalesce(dev, &coalesce, &kernel_coalesce,
-				info->extack);
+	return ops->get_coalesce && ops->set_coalesce ? 1 : -EOPNOTSUPP;
+}
+
+static int
+ethnl_set_coalesce(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	struct kernel_ethtool_coalesce kernel_coalesce = {};
+	struct net_device *dev = req_info->dev;
+	struct ethtool_coalesce coalesce = {};
+	struct nlattr **tb = info->attrs;
+	bool mod = false;
+	int ret;
+
+	ret = dev->ethtool_ops->get_coalesce(dev, &coalesce, &kernel_coalesce,
+					     info->extack);
 	if (ret < 0)
-		goto out_ops;
+		return ret;
 
 	ethnl_update_u32(&coalesce.rx_coalesce_usecs,
 			 tb[ETHTOOL_A_COALESCE_RX_USECS], &mod);
@@ -339,21 +319,26 @@ int ethnl_set_coalesce(struct sk_buff *skb, struct genl_info *info)
 			 tb[ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES], &mod);
 	ethnl_update_u32(&kernel_coalesce.tx_aggr_time_usecs,
 			 tb[ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS], &mod);
-	ret = 0;
 	if (!mod)
-		goto out_ops;
+		return 0;
 
 	ret = dev->ethtool_ops->set_coalesce(dev, &coalesce, &kernel_coalesce,
 					     info->extack);
-	if (ret < 0)
-		goto out_ops;
-	ethtool_notify(dev, ETHTOOL_MSG_COALESCE_NTF, NULL);
-
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return ret < 0 ? ret : 1;
 }
+
+const struct ethnl_request_ops ethnl_coalesce_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_COALESCE_GET,
+	.reply_cmd		= ETHTOOL_MSG_COALESCE_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_COALESCE_HEADER,
+	.req_info_size		= sizeof(struct coalesce_req_info),
+	.reply_data_size	= sizeof(struct coalesce_reply_data),
+
+	.prepare_data		= coalesce_prepare_data,
+	.reply_size		= coalesce_reply_size,
+	.fill_reply		= coalesce_fill_reply,
+
+	.set_validate		= ethnl_set_coalesce_validate,
+	.set			= ethnl_set_coalesce,
+	.set_ntf_cmd		= ETHTOOL_MSG_COALESCE_NTF,
+};
diff --git a/net/ethtool/debug.c b/net/ethtool/debug.c
index d73888c7d19c..e4369769817e 100644
--- a/net/ethtool/debug.c
+++ b/net/ethtool/debug.c
@@ -63,18 +63,6 @@ static int debug_fill_reply(struct sk_buff *skb,
 				  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,
-	.req_info_size		= sizeof(struct debug_req_info),
-	.reply_data_size	= sizeof(struct debug_reply_data),
-
-	.prepare_data		= debug_prepare_data,
-	.reply_size		= debug_reply_size,
-	.fill_reply		= debug_fill_reply,
-};
-
 /* DEBUG_SET */
 
 const struct nla_policy ethnl_debug_set_policy[] = {
@@ -83,46 +71,47 @@ const struct nla_policy ethnl_debug_set_policy[] = {
 	[ETHTOOL_A_DEBUG_MSGMASK]	= { .type = NLA_NESTED },
 };
 
-int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_debug_validate(struct ethnl_req_info *req_info,
+			 struct genl_info *info)
 {
-	struct ethnl_req_info req_info = {};
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+
+	return ops->get_msglevel && ops->set_msglevel ? 1 : -EOPNOTSUPP;
+}
+
+static int
+ethnl_set_debug(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	struct net_device *dev = req_info->dev;
 	struct nlattr **tb = info->attrs;
-	struct net_device *dev;
 	bool mod = false;
 	u32 msg_mask;
 	int ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_DEBUG_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-	dev = req_info.dev;
-	ret = -EOPNOTSUPP;
-	if (!dev->ethtool_ops->get_msglevel || !dev->ethtool_ops->set_msglevel)
-		goto out_dev;
-
-	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;
+		return ret;
 
 	dev->ethtool_ops->set_msglevel(dev, msg_mask);
-	ethtool_notify(dev, ETHTOOL_MSG_DEBUG_NTF, NULL);
-
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return 1;
 }
+
+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,
+	.req_info_size		= sizeof(struct debug_req_info),
+	.reply_data_size	= sizeof(struct debug_reply_data),
+
+	.prepare_data		= debug_prepare_data,
+	.reply_size		= debug_reply_size,
+	.fill_reply		= debug_fill_reply,
+
+	.set_validate		= ethnl_set_debug_validate,
+	.set			= ethnl_set_debug,
+	.set_ntf_cmd		= ETHTOOL_MSG_DEBUG_NTF,
+};
diff --git a/net/ethtool/eee.c b/net/ethtool/eee.c
index 45c42b2d5f17..42104bcb0e47 100644
--- a/net/ethtool/eee.c
+++ b/net/ethtool/eee.c
@@ -108,18 +108,6 @@ static int eee_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_eee_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_EEE_GET,
-	.reply_cmd		= ETHTOOL_MSG_EEE_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_EEE_HEADER,
-	.req_info_size		= sizeof(struct eee_req_info),
-	.reply_data_size	= sizeof(struct eee_reply_data),
-
-	.prepare_data		= eee_prepare_data,
-	.reply_size		= eee_reply_size,
-	.fill_reply		= eee_fill_reply,
-};
-
 /* EEE_SET */
 
 const struct nla_policy ethnl_eee_set_policy[] = {
@@ -131,60 +119,56 @@ const struct nla_policy ethnl_eee_set_policy[] = {
 	[ETHTOOL_A_EEE_TX_LPI_TIMER]	= { .type = NLA_U32 },
 };
 
-int ethnl_set_eee(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_eee_validate(struct ethnl_req_info *req_info, struct genl_info *info)
 {
-	struct ethnl_req_info req_info = {};
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+
+	return ops->get_eee && ops->set_eee ? 1 : -EOPNOTSUPP;
+}
+
+static int
+ethnl_set_eee(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	struct net_device *dev = req_info->dev;
 	struct nlattr **tb = info->attrs;
-	const struct ethtool_ops *ops;
 	struct ethtool_eee eee = {};
-	struct net_device *dev;
 	bool mod = false;
 	int ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_EEE_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
+	ret = dev->ethtool_ops->get_eee(dev, &eee);
 	if (ret < 0)
 		return ret;
-	dev = req_info.dev;
-	ops = dev->ethtool_ops;
-	ret = -EOPNOTSUPP;
-	if (!ops->get_eee || !ops->set_eee)
-		goto out_dev;
-
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
-	ret = ops->get_eee(dev, &eee);
-	if (ret < 0)
-		goto out_ops;
 
 	ret = ethnl_update_bitset32(&eee.advertised, EEE_MODES_COUNT,
 				    tb[ETHTOOL_A_EEE_MODES_OURS],
 				    link_mode_names, info->extack, &mod);
 	if (ret < 0)
-		goto out_ops;
+		return ret;
 	ethnl_update_bool32(&eee.eee_enabled, tb[ETHTOOL_A_EEE_ENABLED], &mod);
 	ethnl_update_bool32(&eee.tx_lpi_enabled,
 			    tb[ETHTOOL_A_EEE_TX_LPI_ENABLED], &mod);
 	ethnl_update_u32(&eee.tx_lpi_timer, tb[ETHTOOL_A_EEE_TX_LPI_TIMER],
 			 &mod);
-	ret = 0;
 	if (!mod)
-		goto out_ops;
+		return 0;
 
 	ret = dev->ethtool_ops->set_eee(dev, &eee);
-	if (ret < 0)
-		goto out_ops;
-	ethtool_notify(dev, ETHTOOL_MSG_EEE_NTF, NULL);
-
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return ret < 0 ? ret : 1;
 }
+
+const struct ethnl_request_ops ethnl_eee_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_EEE_GET,
+	.reply_cmd		= ETHTOOL_MSG_EEE_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_EEE_HEADER,
+	.req_info_size		= sizeof(struct eee_req_info),
+	.reply_data_size	= sizeof(struct eee_reply_data),
+
+	.prepare_data		= eee_prepare_data,
+	.reply_size		= eee_reply_size,
+	.fill_reply		= eee_fill_reply,
+
+	.set_validate		= ethnl_set_eee_validate,
+	.set			= ethnl_set_eee,
+	.set_ntf_cmd		= ETHTOOL_MSG_EEE_NTF,
+};
diff --git a/net/ethtool/fec.c b/net/ethtool/fec.c
index 9f5a134e2e01..0d9a3d153170 100644
--- a/net/ethtool/fec.c
+++ b/net/ethtool/fec.c
@@ -217,18 +217,6 @@ static int fec_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_fec_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_FEC_GET,
-	.reply_cmd		= ETHTOOL_MSG_FEC_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_FEC_HEADER,
-	.req_info_size		= sizeof(struct fec_req_info),
-	.reply_data_size	= sizeof(struct fec_reply_data),
-
-	.prepare_data		= fec_prepare_data,
-	.reply_size		= fec_reply_size,
-	.fill_reply		= fec_fill_reply,
-};
-
 /* FEC_SET */
 
 const struct nla_policy ethnl_fec_set_policy[ETHTOOL_A_FEC_AUTO + 1] = {
@@ -237,36 +225,28 @@ const struct nla_policy ethnl_fec_set_policy[ETHTOOL_A_FEC_AUTO + 1] = {
 	[ETHTOOL_A_FEC_AUTO]	= NLA_POLICY_MAX(NLA_U8, 1),
 };
 
-int ethnl_set_fec(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_fec_validate(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+
+	return ops->get_fecparam && ops->set_fecparam ? 1 : -EOPNOTSUPP;
+}
+
+static int
+ethnl_set_fec(struct ethnl_req_info *req_info, struct genl_info *info)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(fec_link_modes) = {};
-	struct ethnl_req_info req_info = {};
+	struct net_device *dev = req_info->dev;
 	struct nlattr **tb = info->attrs;
 	struct ethtool_fecparam fec = {};
-	const struct ethtool_ops *ops;
-	struct net_device *dev;
 	bool mod = false;
 	u8 fec_auto;
 	int ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info, tb[ETHTOOL_A_FEC_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
+	ret = dev->ethtool_ops->get_fecparam(dev, &fec);
 	if (ret < 0)
 		return ret;
-	dev = req_info.dev;
-	ops = dev->ethtool_ops;
-	ret = -EOPNOTSUPP;
-	if (!ops->get_fecparam || !ops->set_fecparam)
-		goto out_dev;
-
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
-	ret = ops->get_fecparam(dev, &fec);
-	if (ret < 0)
-		goto out_ops;
 
 	ethtool_fec_to_link_modes(fec.fec, fec_link_modes, &fec_auto);
 
@@ -275,36 +255,39 @@ int ethnl_set_fec(struct sk_buff *skb, struct genl_info *info)
 				  tb[ETHTOOL_A_FEC_MODES],
 				  link_mode_names, info->extack, &mod);
 	if (ret < 0)
-		goto out_ops;
+		return ret;
 	ethnl_update_u8(&fec_auto, tb[ETHTOOL_A_FEC_AUTO], &mod);
-
-	ret = 0;
 	if (!mod)
-		goto out_ops;
+		return 0;
 
 	ret = ethtool_link_modes_to_fecparam(&fec, fec_link_modes, fec_auto);
 	if (ret) {
 		NL_SET_ERR_MSG_ATTR(info->extack, tb[ETHTOOL_A_FEC_MODES],
 				    "invalid FEC modes requested");
-		goto out_ops;
+		return ret;
 	}
 	if (!fec.fec) {
-		ret = -EINVAL;
 		NL_SET_ERR_MSG_ATTR(info->extack, tb[ETHTOOL_A_FEC_MODES],
 				    "no FEC modes set");
-		goto out_ops;
+		return -EINVAL;
 	}
 
 	ret = dev->ethtool_ops->set_fecparam(dev, &fec);
-	if (ret < 0)
-		goto out_ops;
-	ethtool_notify(dev, ETHTOOL_MSG_FEC_NTF, NULL);
-
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return ret < 0 ? ret : 1;
 }
+
+const struct ethnl_request_ops ethnl_fec_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_FEC_GET,
+	.reply_cmd		= ETHTOOL_MSG_FEC_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_FEC_HEADER,
+	.req_info_size		= sizeof(struct fec_req_info),
+	.reply_data_size	= sizeof(struct fec_reply_data),
+
+	.prepare_data		= fec_prepare_data,
+	.reply_size		= fec_reply_size,
+	.fill_reply		= fec_fill_reply,
+
+	.set_validate		= ethnl_set_fec_validate,
+	.set			= ethnl_set_fec,
+	.set_ntf_cmd		= ETHTOOL_MSG_FEC_NTF,
+};
diff --git a/net/ethtool/linkinfo.c b/net/ethtool/linkinfo.c
index efa0f7f48836..310dfe63292a 100644
--- a/net/ethtool/linkinfo.c
+++ b/net/ethtool/linkinfo.c
@@ -73,18 +73,6 @@ static int linkinfo_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_linkinfo_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_LINKINFO_GET,
-	.reply_cmd		= ETHTOOL_MSG_LINKINFO_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_LINKINFO_HEADER,
-	.req_info_size		= sizeof(struct linkinfo_req_info),
-	.reply_data_size	= sizeof(struct linkinfo_reply_data),
-
-	.prepare_data		= linkinfo_prepare_data,
-	.reply_size		= linkinfo_reply_size,
-	.fill_reply		= linkinfo_fill_reply,
-};
-
 /* LINKINFO_SET */
 
 const struct nla_policy ethnl_linkinfo_set_policy[] = {
@@ -95,37 +83,31 @@ const struct nla_policy ethnl_linkinfo_set_policy[] = {
 	[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL]	= { .type = NLA_U8 },
 };
 
-int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_linkinfo_validate(struct ethnl_req_info *req_info,
+			    struct genl_info *info)
+{
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+
+	if (!ops->get_link_ksettings || !ops->set_link_ksettings)
+		return -EOPNOTSUPP;
+	return 1;
+}
+
+static int
+ethnl_set_linkinfo(struct ethnl_req_info *req_info, struct genl_info *info)
 {
 	struct ethtool_link_ksettings ksettings = {};
 	struct ethtool_link_settings *lsettings;
-	struct ethnl_req_info req_info = {};
+	struct net_device *dev = req_info->dev;
 	struct nlattr **tb = info->attrs;
-	struct net_device *dev;
 	bool mod = false;
 	int ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_LINKINFO_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-	dev = req_info.dev;
-	ret = -EOPNOTSUPP;
-	if (!dev->ethtool_ops->get_link_ksettings ||
-	    !dev->ethtool_ops->set_link_ksettings)
-		goto out_dev;
-
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
-
 	ret = __ethtool_get_link_ksettings(dev, &ksettings);
 	if (ret < 0) {
 		GENL_SET_ERR_MSG(info, "failed to retrieve link settings");
-		goto out_ops;
+		return ret;
 	}
 	lsettings = &ksettings.base;
 
@@ -134,21 +116,30 @@ int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info)
 			&mod);
 	ethnl_update_u8(&lsettings->eth_tp_mdix_ctrl,
 			tb[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL], &mod);
-	ret = 0;
 	if (!mod)
-		goto out_ops;
+		return 0;
 
 	ret = dev->ethtool_ops->set_link_ksettings(dev, &ksettings);
-	if (ret < 0)
+	if (ret < 0) {
 		GENL_SET_ERR_MSG(info, "link settings update failed");
-	else
-		ethtool_notify(dev, ETHTOOL_MSG_LINKINFO_NTF, NULL);
+		return ret;
+	}
 
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return 1;
 }
+
+const struct ethnl_request_ops ethnl_linkinfo_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_LINKINFO_GET,
+	.reply_cmd		= ETHTOOL_MSG_LINKINFO_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_LINKINFO_HEADER,
+	.req_info_size		= sizeof(struct linkinfo_req_info),
+	.reply_data_size	= sizeof(struct linkinfo_reply_data),
+
+	.prepare_data		= linkinfo_prepare_data,
+	.reply_size		= linkinfo_reply_size,
+	.fill_reply		= linkinfo_fill_reply,
+
+	.set_validate		= ethnl_set_linkinfo_validate,
+	.set			= ethnl_set_linkinfo,
+	.set_ntf_cmd		= ETHTOOL_MSG_LINKINFO_NTF,
+};
diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
index 126e06c713a3..fab66c169b9f 100644
--- a/net/ethtool/linkmodes.c
+++ b/net/ethtool/linkmodes.c
@@ -151,18 +151,6 @@ static int linkmodes_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_linkmodes_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_LINKMODES_GET,
-	.reply_cmd		= ETHTOOL_MSG_LINKMODES_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_LINKMODES_HEADER,
-	.req_info_size		= sizeof(struct linkmodes_req_info),
-	.reply_data_size	= sizeof(struct linkmodes_reply_data),
-
-	.prepare_data		= linkmodes_prepare_data,
-	.reply_size		= linkmodes_reply_size,
-	.fill_reply		= linkmodes_fill_reply,
-};
-
 /* LINKMODES_SET */
 
 const struct nla_policy ethnl_linkmodes_set_policy[] = {
@@ -310,59 +298,64 @@ static int ethnl_update_linkmodes(struct genl_info *info, struct nlattr **tb,
 	return 0;
 }
 
-int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_linkmodes_validate(struct ethnl_req_info *req_info,
+			     struct genl_info *info)
 {
-	struct ethtool_link_ksettings ksettings = {};
-	struct ethnl_req_info req_info = {};
-	struct nlattr **tb = info->attrs;
-	struct net_device *dev;
-	bool mod = false;
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
 	int ret;
 
-	ret = ethnl_check_linkmodes(info, tb);
+	ret = ethnl_check_linkmodes(info, info->attrs);
 	if (ret < 0)
 		return ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_LINKMODES_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-	dev = req_info.dev;
-	ret = -EOPNOTSUPP;
-	if (!dev->ethtool_ops->get_link_ksettings ||
-	    !dev->ethtool_ops->set_link_ksettings)
-		goto out_dev;
+	if (!ops->get_link_ksettings || !ops->set_link_ksettings)
+		return -EOPNOTSUPP;
+	return 1;
+}
 
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
+static int
+ethnl_set_linkmodes(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	struct ethtool_link_ksettings ksettings = {};
+	struct net_device *dev = req_info->dev;
+	struct nlattr **tb = info->attrs;
+	bool mod = false;
+	int ret;
 
 	ret = __ethtool_get_link_ksettings(dev, &ksettings);
 	if (ret < 0) {
 		GENL_SET_ERR_MSG(info, "failed to retrieve link settings");
-		goto out_ops;
+		return ret;
 	}
 
 	ret = ethnl_update_linkmodes(info, tb, &ksettings, &mod, dev);
 	if (ret < 0)
-		goto out_ops;
+		return ret;
+	if (!mod)
+		return 0;
 
-	if (mod) {
-		ret = dev->ethtool_ops->set_link_ksettings(dev, &ksettings);
-		if (ret < 0)
-			GENL_SET_ERR_MSG(info, "link settings update failed");
-		else
-			ethtool_notify(dev, ETHTOOL_MSG_LINKMODES_NTF, NULL);
+	ret = dev->ethtool_ops->set_link_ksettings(dev, &ksettings);
+	if (ret < 0) {
+		GENL_SET_ERR_MSG(info, "link settings update failed");
+		return ret;
 	}
 
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return 1;
 }
+
+const struct ethnl_request_ops ethnl_linkmodes_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_LINKMODES_GET,
+	.reply_cmd		= ETHTOOL_MSG_LINKMODES_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_LINKMODES_HEADER,
+	.req_info_size		= sizeof(struct linkmodes_req_info),
+	.reply_data_size	= sizeof(struct linkmodes_reply_data),
+
+	.prepare_data		= linkmodes_prepare_data,
+	.reply_size		= linkmodes_reply_size,
+	.fill_reply		= linkmodes_fill_reply,
+
+	.set_validate		= ethnl_set_linkmodes_validate,
+	.set			= ethnl_set_linkmodes,
+	.set_ntf_cmd		= ETHTOOL_MSG_LINKMODES_NTF,
+};
diff --git a/net/ethtool/module.c b/net/ethtool/module.c
index 898ed436b9e4..e0d539b21423 100644
--- a/net/ethtool/module.c
+++ b/net/ethtool/module.c
@@ -91,18 +91,6 @@ static int module_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_module_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_MODULE_GET,
-	.reply_cmd		= ETHTOOL_MSG_MODULE_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_MODULE_HEADER,
-	.req_info_size		= sizeof(struct module_req_info),
-	.reply_data_size	= sizeof(struct module_reply_data),
-
-	.prepare_data		= module_prepare_data,
-	.reply_size		= module_reply_size,
-	.fill_reply		= module_fill_reply,
-};
-
 /* MODULE_SET */
 
 const struct nla_policy ethnl_module_set_policy[ETHTOOL_A_MODULE_POWER_MODE_POLICY + 1] = {
@@ -112,69 +100,62 @@ const struct nla_policy ethnl_module_set_policy[ETHTOOL_A_MODULE_POWER_MODE_POLI
 				 ETHTOOL_MODULE_POWER_MODE_POLICY_AUTO),
 };
 
-static int module_set_power_mode(struct net_device *dev, struct nlattr **tb,
-				 bool *p_mod, struct netlink_ext_ack *extack)
+static int
+ethnl_set_module_validate(struct ethnl_req_info *req_info,
+			  struct genl_info *info)
 {
-	struct ethtool_module_power_mode_params power = {};
-	struct ethtool_module_power_mode_params power_new;
-	const struct ethtool_ops *ops = dev->ethtool_ops;
-	int ret;
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+	struct nlattr **tb = info->attrs;
 
 	if (!tb[ETHTOOL_A_MODULE_POWER_MODE_POLICY])
 		return 0;
 
 	if (!ops->get_module_power_mode || !ops->set_module_power_mode) {
-		NL_SET_ERR_MSG_ATTR(extack,
+		NL_SET_ERR_MSG_ATTR(info->extack,
 				    tb[ETHTOOL_A_MODULE_POWER_MODE_POLICY],
 				    "Setting power mode policy is not supported by this device");
 		return -EOPNOTSUPP;
 	}
 
-	power_new.policy = nla_get_u8(tb[ETHTOOL_A_MODULE_POWER_MODE_POLICY]);
-	ret = ops->get_module_power_mode(dev, &power, extack);
-	if (ret < 0)
-		return ret;
-
-	if (power_new.policy == power.policy)
-		return 0;
-	*p_mod = true;
-
-	return ops->set_module_power_mode(dev, &power_new, extack);
+	return 1;
 }
 
-int ethnl_set_module(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_module(struct ethnl_req_info *req_info, struct genl_info *info)
 {
-	struct ethnl_req_info req_info = {};
+	struct ethtool_module_power_mode_params power = {};
+	struct ethtool_module_power_mode_params power_new;
+	const struct ethtool_ops *ops;
+	struct net_device *dev = req_info->dev;
 	struct nlattr **tb = info->attrs;
-	struct net_device *dev;
-	bool mod = false;
 	int ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info, tb[ETHTOOL_A_MODULE_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
+	ops = dev->ethtool_ops;
+
+	power_new.policy = nla_get_u8(tb[ETHTOOL_A_MODULE_POWER_MODE_POLICY]);
+	ret = ops->get_module_power_mode(dev, &power, info->extack);
 	if (ret < 0)
 		return ret;
-	dev = req_info.dev;
 
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
+	if (power_new.policy == power.policy)
+		return 0;
 
-	ret = module_set_power_mode(dev, tb, &mod, info->extack);
-	if (ret < 0)
-		goto out_ops;
+	ret = ops->set_module_power_mode(dev, &power_new, info->extack);
+	return ret < 0 ? ret : 1;
+}
 
-	if (!mod)
-		goto out_ops;
+const struct ethnl_request_ops ethnl_module_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_MODULE_GET,
+	.reply_cmd		= ETHTOOL_MSG_MODULE_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_MODULE_HEADER,
+	.req_info_size		= sizeof(struct module_req_info),
+	.reply_data_size	= sizeof(struct module_reply_data),
 
-	ethtool_notify(dev, ETHTOOL_MSG_MODULE_NTF, NULL);
+	.prepare_data		= module_prepare_data,
+	.reply_size		= module_reply_size,
+	.fill_reply		= module_fill_reply,
 
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
-}
+	.set_validate		= ethnl_set_module_validate,
+	.set			= ethnl_set_module,
+	.set_ntf_cmd		= ETHTOOL_MSG_MODULE_NTF,
+};
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 31eaff9eb90b..a2761e1ea653 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -269,27 +269,40 @@ static const struct ethnl_request_ops *
 ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_STRSET_GET]	= &ethnl_strset_request_ops,
 	[ETHTOOL_MSG_LINKINFO_GET]	= &ethnl_linkinfo_request_ops,
+	[ETHTOOL_MSG_LINKINFO_SET]	= &ethnl_linkinfo_request_ops,
 	[ETHTOOL_MSG_LINKMODES_GET]	= &ethnl_linkmodes_request_ops,
+	[ETHTOOL_MSG_LINKMODES_SET]	= &ethnl_linkmodes_request_ops,
 	[ETHTOOL_MSG_LINKSTATE_GET]	= &ethnl_linkstate_request_ops,
 	[ETHTOOL_MSG_DEBUG_GET]		= &ethnl_debug_request_ops,
+	[ETHTOOL_MSG_DEBUG_SET]		= &ethnl_debug_request_ops,
 	[ETHTOOL_MSG_WOL_GET]		= &ethnl_wol_request_ops,
+	[ETHTOOL_MSG_WOL_SET]		= &ethnl_wol_request_ops,
 	[ETHTOOL_MSG_FEATURES_GET]	= &ethnl_features_request_ops,
 	[ETHTOOL_MSG_PRIVFLAGS_GET]	= &ethnl_privflags_request_ops,
+	[ETHTOOL_MSG_PRIVFLAGS_SET]	= &ethnl_privflags_request_ops,
 	[ETHTOOL_MSG_RINGS_GET]		= &ethnl_rings_request_ops,
+	[ETHTOOL_MSG_RINGS_SET]		= &ethnl_rings_request_ops,
 	[ETHTOOL_MSG_CHANNELS_GET]	= &ethnl_channels_request_ops,
+	[ETHTOOL_MSG_CHANNELS_SET]	= &ethnl_channels_request_ops,
 	[ETHTOOL_MSG_COALESCE_GET]	= &ethnl_coalesce_request_ops,
+	[ETHTOOL_MSG_COALESCE_SET]	= &ethnl_coalesce_request_ops,
 	[ETHTOOL_MSG_PAUSE_GET]		= &ethnl_pause_request_ops,
 	[ETHTOOL_MSG_PAUSE_SET]		= &ethnl_pause_request_ops,
 	[ETHTOOL_MSG_EEE_GET]		= &ethnl_eee_request_ops,
+	[ETHTOOL_MSG_EEE_SET]		= &ethnl_eee_request_ops,
 	[ETHTOOL_MSG_FEC_GET]		= &ethnl_fec_request_ops,
+	[ETHTOOL_MSG_FEC_SET]		= &ethnl_fec_request_ops,
 	[ETHTOOL_MSG_TSINFO_GET]	= &ethnl_tsinfo_request_ops,
 	[ETHTOOL_MSG_MODULE_EEPROM_GET]	= &ethnl_module_eeprom_request_ops,
 	[ETHTOOL_MSG_STATS_GET]		= &ethnl_stats_request_ops,
 	[ETHTOOL_MSG_PHC_VCLOCKS_GET]	= &ethnl_phc_vclocks_request_ops,
 	[ETHTOOL_MSG_MODULE_GET]	= &ethnl_module_request_ops,
+	[ETHTOOL_MSG_MODULE_SET]	= &ethnl_module_request_ops,
 	[ETHTOOL_MSG_PSE_GET]		= &ethnl_pse_request_ops,
+	[ETHTOOL_MSG_PSE_SET]		= &ethnl_pse_request_ops,
 	[ETHTOOL_MSG_RSS_GET]		= &ethnl_rss_request_ops,
 	[ETHTOOL_MSG_PLCA_GET_CFG]	= &ethnl_plca_cfg_request_ops,
+	[ETHTOOL_MSG_PLCA_SET_CFG]	= &ethnl_plca_cfg_request_ops,
 	[ETHTOOL_MSG_PLCA_GET_STATUS]	= &ethnl_plca_status_request_ops,
 };
 
@@ -811,7 +824,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_LINKINFO_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_linkinfo,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_linkinfo_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_linkinfo_set_policy) - 1,
 	},
@@ -827,7 +840,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_LINKMODES_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_linkmodes,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_linkmodes_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_linkmodes_set_policy) - 1,
 	},
@@ -852,7 +865,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_DEBUG_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_debug,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_debug_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_debug_set_policy) - 1,
 	},
@@ -869,7 +882,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_WOL_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_wol,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_wol_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_wol_set_policy) - 1,
 	},
@@ -901,7 +914,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_PRIVFLAGS_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_privflags,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_privflags_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_privflags_set_policy) - 1,
 	},
@@ -917,7 +930,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_RINGS_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_rings,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_rings_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_rings_set_policy) - 1,
 	},
@@ -933,7 +946,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_CHANNELS_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_channels,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_channels_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_channels_set_policy) - 1,
 	},
@@ -949,7 +962,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_COALESCE_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_coalesce,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_coalesce_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_coalesce_set_policy) - 1,
 	},
@@ -981,7 +994,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_EEE_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_eee,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_eee_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_eee_set_policy) - 1,
 	},
@@ -1028,7 +1041,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_FEC_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_fec,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_fec_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_fec_set_policy) - 1,
 	},
@@ -1072,7 +1085,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_MODULE_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_module,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_module_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_module_set_policy) - 1,
 	},
@@ -1088,7 +1101,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_PSE_SET,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_pse,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_pse_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_pse_set_policy) - 1,
 	},
@@ -1110,7 +1123,7 @@ static const struct genl_ops ethtool_genl_ops[] = {
 	{
 		.cmd	= ETHTOOL_MSG_PLCA_SET_CFG,
 		.flags	= GENL_UNS_ADMIN_PERM,
-		.doit	= ethnl_set_plca_cfg,
+		.doit	= ethnl_default_set_doit,
 		.policy = ethnl_plca_set_cfg_policy,
 		.maxattr = ARRAY_SIZE(ethnl_plca_set_cfg_policy) - 1,
 	},
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index e6cd1e5b141e..204616e8ec86 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -413,25 +413,12 @@ extern const struct nla_policy ethnl_plca_get_cfg_policy[ETHTOOL_A_PLCA_HEADER +
 extern const struct nla_policy ethnl_plca_set_cfg_policy[ETHTOOL_A_PLCA_MAX + 1];
 extern const struct nla_policy ethnl_plca_get_status_policy[ETHTOOL_A_PLCA_HEADER + 1];
 
-int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_wol(struct sk_buff *skb, struct genl_info *info);
 int ethnl_set_features(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_privflags(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_channels(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_coalesce(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_eee(struct sk_buff *skb, struct genl_info *info);
 int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);
 int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info);
 int ethnl_tunnel_info_doit(struct sk_buff *skb, struct genl_info *info);
 int ethnl_tunnel_info_start(struct netlink_callback *cb);
 int ethnl_tunnel_info_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
-int ethnl_set_fec(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_module(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_pse(struct sk_buff *skb, struct genl_info *info);
-int ethnl_set_plca_cfg(struct sk_buff *skb, struct genl_info *info);
 
 extern const char stats_std_names[__ETHTOOL_STATS_CNT][ETH_GSTRING_LEN];
 extern const char stats_eth_phy_names[__ETHTOOL_A_STATS_ETH_PHY_CNT][ETH_GSTRING_LEN];
diff --git a/net/ethtool/plca.c b/net/ethtool/plca.c
index be7404dc9ef2..5a8cab4df0c9 100644
--- a/net/ethtool/plca.c
+++ b/net/ethtool/plca.c
@@ -112,18 +112,6 @@ static int plca_get_cfg_fill_reply(struct sk_buff *skb,
 	return 0;
 };
 
-const struct ethnl_request_ops ethnl_plca_cfg_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_PLCA_GET_CFG,
-	.reply_cmd		= ETHTOOL_MSG_PLCA_GET_CFG_REPLY,
-	.hdr_attr		= ETHTOOL_A_PLCA_HEADER,
-	.req_info_size		= sizeof(struct plca_req_info),
-	.reply_data_size	= sizeof(struct plca_reply_data),
-
-	.prepare_data		= plca_get_cfg_prepare_data,
-	.reply_size		= plca_get_cfg_reply_size,
-	.fill_reply		= plca_get_cfg_fill_reply,
-};
-
 // PLCA set configuration message ------------------------------------------- //
 
 const struct nla_policy ethnl_plca_set_cfg_policy[] = {
@@ -137,42 +125,23 @@ const struct nla_policy ethnl_plca_set_cfg_policy[] = {
 	[ETHTOOL_A_PLCA_BURST_TMR]	= NLA_POLICY_MAX(NLA_U32, 255),
 };
 
-int ethnl_set_plca_cfg(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_plca(struct ethnl_req_info *req_info, struct genl_info *info)
 {
-	struct ethnl_req_info req_info = {};
-	struct nlattr **tb = info->attrs;
+	struct net_device *dev = req_info->dev;
 	const struct ethtool_phy_ops *ops;
+	struct nlattr **tb = info->attrs;
 	struct phy_plca_cfg plca_cfg;
-	struct net_device *dev;
 	bool mod = false;
 	int ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_PLCA_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-
-	dev = req_info.dev;
-
-	rtnl_lock();
-
 	// check that the PHY device is available and connected
-	if (!dev->phydev) {
-		ret = -EOPNOTSUPP;
-		goto out_rtnl;
-	}
+	if (!dev->phydev)
+		return -EOPNOTSUPP;
 
 	ops = ethtool_phy_ops;
-	if (!ops || !ops->set_plca_cfg) {
-		ret = -EOPNOTSUPP;
-		goto out_rtnl;
-	}
-
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
+	if (!ops || !ops->set_plca_cfg)
+		return -EOPNOTSUPP;
 
 	memset(&plca_cfg, 0xff, sizeof(plca_cfg));
 	plca_update_sint(&plca_cfg.enabled, tb[ETHTOOL_A_PLCA_ENABLED], &mod);
@@ -183,25 +152,27 @@ int ethnl_set_plca_cfg(struct sk_buff *skb, struct genl_info *info)
 			 &mod);
 	plca_update_sint(&plca_cfg.burst_tmr, tb[ETHTOOL_A_PLCA_BURST_TMR],
 			 &mod);
-
-	ret = 0;
 	if (!mod)
-		goto out_ops;
+		return 0;
 
 	ret = ops->set_plca_cfg(dev->phydev, &plca_cfg, info->extack);
-	if (ret < 0)
-		goto out_ops;
+	return ret < 0 ? ret : 1;
+}
 
-	ethtool_notify(dev, ETHTOOL_MSG_PLCA_NTF, NULL);
+const struct ethnl_request_ops ethnl_plca_cfg_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_PLCA_GET_CFG,
+	.reply_cmd		= ETHTOOL_MSG_PLCA_GET_CFG_REPLY,
+	.hdr_attr		= ETHTOOL_A_PLCA_HEADER,
+	.req_info_size		= sizeof(struct plca_req_info),
+	.reply_data_size	= sizeof(struct plca_reply_data),
 
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-	ethnl_parse_header_dev_put(&req_info);
+	.prepare_data		= plca_get_cfg_prepare_data,
+	.reply_size		= plca_get_cfg_reply_size,
+	.fill_reply		= plca_get_cfg_fill_reply,
 
-	return ret;
-}
+	.set			= ethnl_set_plca,
+	.set_ntf_cmd		= ETHTOOL_MSG_PLCA_NTF,
+};
 
 // PLCA get status message -------------------------------------------------- //
 
diff --git a/net/ethtool/privflags.c b/net/ethtool/privflags.c
index 4c7bfa81e4ab..23264a1ebf12 100644
--- a/net/ethtool/privflags.c
+++ b/net/ethtool/privflags.c
@@ -118,19 +118,6 @@ static void privflags_cleanup_data(struct ethnl_reply_data *reply_data)
 	kfree(data->priv_flag_names);
 }
 
-const struct ethnl_request_ops ethnl_privflags_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_PRIVFLAGS_GET,
-	.reply_cmd		= ETHTOOL_MSG_PRIVFLAGS_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_PRIVFLAGS_HEADER,
-	.req_info_size		= sizeof(struct privflags_req_info),
-	.reply_data_size	= sizeof(struct privflags_reply_data),
-
-	.prepare_data		= privflags_prepare_data,
-	.reply_size		= privflags_reply_size,
-	.fill_reply		= privflags_fill_reply,
-	.cleanup_data		= privflags_cleanup_data,
-};
-
 /* PRIVFLAGS_SET */
 
 const struct nla_policy ethnl_privflags_set_policy[] = {
@@ -139,63 +126,70 @@ const struct nla_policy ethnl_privflags_set_policy[] = {
 	[ETHTOOL_A_PRIVFLAGS_FLAGS]		= { .type = NLA_NESTED },
 };
 
-int ethnl_set_privflags(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_privflags_validate(struct ethnl_req_info *req_info,
+			     struct genl_info *info)
+{
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+
+	if (!info->attrs[ETHTOOL_A_PRIVFLAGS_FLAGS])
+		return -EINVAL;
+
+	if (!ops->get_priv_flags || !ops->set_priv_flags ||
+	    !ops->get_sset_count || !ops->get_strings)
+		return -EOPNOTSUPP;
+	return 1;
+}
+
+static int
+ethnl_set_privflags(struct ethnl_req_info *req_info, struct genl_info *info)
 {
 	const char (*names)[ETH_GSTRING_LEN] = NULL;
-	struct ethnl_req_info req_info = {};
+	struct net_device *dev = req_info->dev;
 	struct nlattr **tb = info->attrs;
-	const struct ethtool_ops *ops;
-	struct net_device *dev;
 	unsigned int nflags;
 	bool mod = false;
 	bool compact;
 	u32 flags;
 	int ret;
 
-	if (!tb[ETHTOOL_A_PRIVFLAGS_FLAGS])
-		return -EINVAL;
 	ret = ethnl_bitset_is_compact(tb[ETHTOOL_A_PRIVFLAGS_FLAGS], &compact);
 	if (ret < 0)
 		return ret;
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_PRIVFLAGS_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-	dev = req_info.dev;
-	ops = dev->ethtool_ops;
-	ret = -EOPNOTSUPP;
-	if (!ops->get_priv_flags || !ops->set_priv_flags ||
-	    !ops->get_sset_count || !ops->get_strings)
-		goto out_dev;
 
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
 	ret = ethnl_get_priv_flags_info(dev, &nflags, compact ? NULL : &names);
 	if (ret < 0)
-		goto out_ops;
-	flags = ops->get_priv_flags(dev);
+		return ret;
+	flags = dev->ethtool_ops->get_priv_flags(dev);
 
 	ret = ethnl_update_bitset32(&flags, nflags,
 				    tb[ETHTOOL_A_PRIVFLAGS_FLAGS], names,
 				    info->extack, &mod);
 	if (ret < 0 || !mod)
 		goto out_free;
-	ret = ops->set_priv_flags(dev, flags);
+	ret = dev->ethtool_ops->set_priv_flags(dev, flags);
 	if (ret < 0)
 		goto out_free;
-	ethtool_notify(dev, ETHTOOL_MSG_PRIVFLAGS_NTF, NULL);
+	ret = 1;
 
 out_free:
 	kfree(names);
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
 	return ret;
 }
+
+const struct ethnl_request_ops ethnl_privflags_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_PRIVFLAGS_GET,
+	.reply_cmd		= ETHTOOL_MSG_PRIVFLAGS_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_PRIVFLAGS_HEADER,
+	.req_info_size		= sizeof(struct privflags_req_info),
+	.reply_data_size	= sizeof(struct privflags_reply_data),
+
+	.prepare_data		= privflags_prepare_data,
+	.reply_size		= privflags_reply_size,
+	.fill_reply		= privflags_fill_reply,
+	.cleanup_data		= privflags_cleanup_data,
+
+	.set_validate		= ethnl_set_privflags_validate,
+	.set			= ethnl_set_privflags,
+	.set_ntf_cmd		= ETHTOOL_MSG_PRIVFLAGS_NTF,
+};
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index e8683e485dc9..a5b607b0a652 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -106,18 +106,6 @@ static int pse_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_pse_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_PSE_GET,
-	.reply_cmd		= ETHTOOL_MSG_PSE_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_PSE_HEADER,
-	.req_info_size		= sizeof(struct pse_req_info),
-	.reply_data_size	= sizeof(struct pse_reply_data),
-
-	.prepare_data		= pse_prepare_data,
-	.reply_size		= pse_reply_size,
-	.fill_reply		= pse_fill_reply,
-};
-
 /* PSE_SET */
 
 const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = {
@@ -127,59 +115,50 @@ const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = {
 				 ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED),
 };
 
-static int pse_set_pse_config(struct net_device *dev,
-			      struct netlink_ext_ack *extack,
-			      struct nlattr **tb)
+static int
+ethnl_set_pse_validate(struct ethnl_req_info *req_info, struct genl_info *info)
 {
-	struct phy_device *phydev = dev->phydev;
-	struct pse_control_config config = {};
+	return !!info->attrs[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL];
+}
 
-	/* Optional attribute. Do not return error if not set. */
-	if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL])
-		return 0;
+static int
+ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	struct net_device *dev = req_info->dev;
+	struct pse_control_config config = {};
+	struct nlattr **tb = info->attrs;
+	struct phy_device *phydev;
 
 	/* this values are already validated by the ethnl_pse_set_policy */
 	config.admin_cotrol = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
 
+	phydev = dev->phydev;
 	if (!phydev) {
-		NL_SET_ERR_MSG(extack, "No PHY is attached");
+		NL_SET_ERR_MSG(info->extack, "No PHY is attached");
 		return -EOPNOTSUPP;
 	}
 
 	if (!phydev->psec) {
-		NL_SET_ERR_MSG(extack, "No PSE is attached");
+		NL_SET_ERR_MSG(info->extack, "No PSE is attached");
 		return -EOPNOTSUPP;
 	}
 
-	return pse_ethtool_set_config(phydev->psec, extack, &config);
+	/* Return errno directly - PSE has no notification */
+	return pse_ethtool_set_config(phydev->psec, info->extack, &config);
 }
 
-int ethnl_set_pse(struct sk_buff *skb, struct genl_info *info)
-{
-	struct ethnl_req_info req_info = {};
-	struct nlattr **tb = info->attrs;
-	struct net_device *dev;
-	int ret;
-
-	ret = ethnl_parse_header_dev_get(&req_info, tb[ETHTOOL_A_PSE_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-
-	dev = req_info.dev;
-
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
-
-	ret = pse_set_pse_config(dev, info->extack, tb);
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
+const struct ethnl_request_ops ethnl_pse_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_PSE_GET,
+	.reply_cmd		= ETHTOOL_MSG_PSE_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_PSE_HEADER,
+	.req_info_size		= sizeof(struct pse_req_info),
+	.reply_data_size	= sizeof(struct pse_reply_data),
 
-	ethnl_parse_header_dev_put(&req_info);
+	.prepare_data		= pse_prepare_data,
+	.reply_size		= pse_reply_size,
+	.fill_reply		= pse_fill_reply,
 
-	return ret;
-}
+	.set_validate		= ethnl_set_pse_validate,
+	.set			= ethnl_set_pse,
+	/* PSE has no notification */
+};
diff --git a/net/ethtool/rings.c b/net/ethtool/rings.c
index fa3ec8d438f7..2a2d3539630c 100644
--- a/net/ethtool/rings.c
+++ b/net/ethtool/rings.c
@@ -102,18 +102,6 @@ static int rings_fill_reply(struct sk_buff *skb,
 	return 0;
 }
 
-const struct ethnl_request_ops ethnl_rings_request_ops = {
-	.request_cmd		= ETHTOOL_MSG_RINGS_GET,
-	.reply_cmd		= ETHTOOL_MSG_RINGS_GET_REPLY,
-	.hdr_attr		= ETHTOOL_A_RINGS_HEADER,
-	.req_info_size		= sizeof(struct rings_req_info),
-	.reply_data_size	= sizeof(struct rings_reply_data),
-
-	.prepare_data		= rings_prepare_data,
-	.reply_size		= rings_reply_size,
-	.fill_reply		= rings_fill_reply,
-};
-
 /* RINGS_SET */
 
 const struct nla_policy ethnl_rings_set_policy[] = {
@@ -128,62 +116,53 @@ const struct nla_policy ethnl_rings_set_policy[] = {
 	[ETHTOOL_A_RINGS_TX_PUSH]		= NLA_POLICY_MAX(NLA_U8, 1),
 };
 
-int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_rings_validate(struct ethnl_req_info *req_info,
+			 struct genl_info *info)
 {
-	struct kernel_ethtool_ringparam kernel_ringparam = {};
-	struct ethtool_ringparam ringparam = {};
-	struct ethnl_req_info req_info = {};
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
 	struct nlattr **tb = info->attrs;
-	const struct nlattr *err_attr;
-	const struct ethtool_ops *ops;
-	struct net_device *dev;
-	bool mod = false;
-	int ret;
-
-	ret = ethnl_parse_header_dev_get(&req_info,
-					 tb[ETHTOOL_A_RINGS_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-	dev = req_info.dev;
-	ops = dev->ethtool_ops;
-	ret = -EOPNOTSUPP;
-	if (!ops->get_ringparam || !ops->set_ringparam)
-		goto out_dev;
 
 	if (tb[ETHTOOL_A_RINGS_RX_BUF_LEN] &&
 	    !(ops->supported_ring_params & ETHTOOL_RING_USE_RX_BUF_LEN)) {
-		ret = -EOPNOTSUPP;
 		NL_SET_ERR_MSG_ATTR(info->extack,
 				    tb[ETHTOOL_A_RINGS_RX_BUF_LEN],
 				    "setting rx buf len not supported");
-		goto out_dev;
+		return -EOPNOTSUPP;
 	}
 
 	if (tb[ETHTOOL_A_RINGS_CQE_SIZE] &&
 	    !(ops->supported_ring_params & ETHTOOL_RING_USE_CQE_SIZE)) {
-		ret = -EOPNOTSUPP;
 		NL_SET_ERR_MSG_ATTR(info->extack,
 				    tb[ETHTOOL_A_RINGS_CQE_SIZE],
 				    "setting cqe size not supported");
-		goto out_dev;
+		return -EOPNOTSUPP;
 	}
 
 	if (tb[ETHTOOL_A_RINGS_TX_PUSH] &&
 	    !(ops->supported_ring_params & ETHTOOL_RING_USE_TX_PUSH)) {
-		ret = -EOPNOTSUPP;
 		NL_SET_ERR_MSG_ATTR(info->extack,
 				    tb[ETHTOOL_A_RINGS_TX_PUSH],
 				    "setting tx push not supported");
-		goto out_dev;
+		return -EOPNOTSUPP;
 	}
 
-	rtnl_lock();
-	ret = ethnl_ops_begin(dev);
-	if (ret < 0)
-		goto out_rtnl;
-	ops->get_ringparam(dev, &ringparam, &kernel_ringparam, info->extack);
+	return ops->get_ringparam && ops->set_ringparam ? 1 : -EOPNOTSUPP;
+}
+
+static int
+ethnl_set_rings(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	struct kernel_ethtool_ringparam kernel_ringparam = {};
+	struct ethtool_ringparam ringparam = {};
+	struct net_device *dev = req_info->dev;
+	struct nlattr **tb = info->attrs;
+	const struct nlattr *err_attr;
+	bool mod = false;
+	int ret;
+
+	dev->ethtool_ops->get_ringparam(dev, &ringparam,
+					&kernel_ringparam, info->extack);
 
 	ethnl_update_u32(&ringparam.rx_pending, tb[ETHTOOL_A_RINGS_RX], &mod);
 	ethnl_update_u32(&ringparam.rx_mini_pending,
@@ -197,9 +176,8 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
 			 tb[ETHTOOL_A_RINGS_CQE_SIZE], &mod);
 	ethnl_update_u8(&kernel_ringparam.tx_push,
 			tb[ETHTOOL_A_RINGS_TX_PUSH], &mod);
-	ret = 0;
 	if (!mod)
-		goto out_ops;
+		return 0;
 
 	/* ensure new ring parameters are within limits */
 	if (ringparam.rx_pending > ringparam.rx_max_pending)
@@ -213,23 +191,28 @@ int ethnl_set_rings(struct sk_buff *skb, struct genl_info *info)
 	else
 		err_attr = NULL;
 	if (err_attr) {
-		ret = -EINVAL;
 		NL_SET_ERR_MSG_ATTR(info->extack, err_attr,
 				    "requested ring size exceeds maximum");
-		goto out_ops;
+		return -EINVAL;
 	}
 
 	ret = dev->ethtool_ops->set_ringparam(dev, &ringparam,
 					      &kernel_ringparam, info->extack);
-	if (ret < 0)
-		goto out_ops;
-	ethtool_notify(dev, ETHTOOL_MSG_RINGS_NTF, NULL);
-
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return ret < 0 ? ret : 1;
 }
+
+const struct ethnl_request_ops ethnl_rings_request_ops = {
+	.request_cmd		= ETHTOOL_MSG_RINGS_GET,
+	.reply_cmd		= ETHTOOL_MSG_RINGS_GET_REPLY,
+	.hdr_attr		= ETHTOOL_A_RINGS_HEADER,
+	.req_info_size		= sizeof(struct rings_req_info),
+	.reply_data_size	= sizeof(struct rings_reply_data),
+
+	.prepare_data		= rings_prepare_data,
+	.reply_size		= rings_reply_size,
+	.fill_reply		= rings_fill_reply,
+
+	.set_validate		= ethnl_set_rings_validate,
+	.set			= ethnl_set_rings,
+	.set_ntf_cmd		= ETHTOOL_MSG_RINGS_NTF,
+};
diff --git a/net/ethtool/wol.c b/net/ethtool/wol.c
index 88f435e76481..a4a43d9e6e9d 100644
--- a/net/ethtool/wol.c
+++ b/net/ethtool/wol.c
@@ -82,18 +82,6 @@ static int wol_fill_reply(struct sk_buff *skb,
 	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,
-	.req_info_size		= sizeof(struct wol_req_info),
-	.reply_data_size	= sizeof(struct wol_reply_data),
-
-	.prepare_data		= wol_prepare_data,
-	.reply_size		= wol_reply_size,
-	.fill_reply		= wol_fill_reply,
-};
-
 /* WOL_SET */
 
 const struct nla_policy ethnl_wol_set_policy[] = {
@@ -104,67 +92,66 @@ const struct nla_policy ethnl_wol_set_policy[] = {
 					    .len = SOPASS_MAX },
 };
 
-int ethnl_set_wol(struct sk_buff *skb, struct genl_info *info)
+static int
+ethnl_set_wol_validate(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+
+	return ops->get_wol && ops->set_wol ? 1 : -EOPNOTSUPP;
+}
+
+static int
+ethnl_set_wol(struct ethnl_req_info *req_info, struct genl_info *info)
 {
 	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
-	struct ethnl_req_info req_info = {};
+	struct net_device *dev = req_info->dev;
 	struct nlattr **tb = info->attrs;
-	struct net_device *dev;
 	bool mod = false;
 	int ret;
 
-	ret = ethnl_parse_header_dev_get(&req_info, tb[ETHTOOL_A_WOL_HEADER],
-					 genl_info_net(info), info->extack,
-					 true);
-	if (ret < 0)
-		return ret;
-	dev = req_info.dev;
-	ret = -EOPNOTSUPP;
-	if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol)
-		goto out_dev;
-
-	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;
+		return ret;
 	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;
+		return -EINVAL;
 	}
 	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;
+			return -EINVAL;
 		}
 		ethnl_update_binary(wol.sopass, sizeof(wol.sopass),
 				    tb[ETHTOOL_A_WOL_SOPASS], &mod);
 	}
 
 	if (!mod)
-		goto out_ops;
+		return 0;
 	ret = dev->ethtool_ops->set_wol(dev, &wol);
 	if (ret)
-		goto out_ops;
+		return ret;
 	dev->wol_enabled = !!wol.wolopts;
-	ethtool_notify(dev, ETHTOOL_MSG_WOL_NTF, NULL);
-
-out_ops:
-	ethnl_ops_complete(dev);
-out_rtnl:
-	rtnl_unlock();
-out_dev:
-	ethnl_parse_header_dev_put(&req_info);
-	return ret;
+	return 1;
 }
+
+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,
+	.req_info_size		= sizeof(struct wol_req_info),
+	.reply_data_size	= sizeof(struct wol_reply_data),
+
+	.prepare_data		= wol_prepare_data,
+	.reply_size		= wol_reply_size,
+	.fill_reply		= wol_fill_reply,
+
+	.set_validate		= ethnl_set_wol_validate,
+	.set			= ethnl_set_wol,
+	.set_ntf_cmd		= ETHTOOL_MSG_WOL_NTF,
+};
-- 
2.39.0

Re: [PATCH net-next 2/2] ethtool: netlink: convert commands to common SET

[Gal Pressman]

On 21/01/2023 7:44, Jakub Kicinski wrote:
> Convert all SET commands where new common code is applicable.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Tested-by: Gal Pressman <gal@nvidia.com>

Re: [PATCH net-next 2/2] ethtool: netlink: convert commands to common SET

[Tariq Toukan]

On 21/01/2023 7:44, Jakub Kicinski wrote:
> Convert all SET commands where new common code is applicable.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: piergiorgio.beruto@gmail.com
> CC: gal@nvidia.com
> CC: tariqt@nvidia.com
> CC: dnlplm@gmail.com
> CC: sean.anderson@seco.com
> CC: linux@rempel-privat.de
> CC: lkp@intel.com
> CC: bagasdotme@gmail.com
> CC: wangjie125@huawei.com
> CC: huangguangbin2@huawei.com
> ---
>   net/ethtool/channels.c  |  92 ++++++++++++++----------------------
>   net/ethtool/coalesce.c  |  93 ++++++++++++++++--------------------
>   net/ethtool/debug.c     |  71 ++++++++++++----------------
>   net/ethtool/eee.c       |  78 ++++++++++++-------------------
>   net/ethtool/fec.c       |  83 +++++++++++++--------------------
>   net/ethtool/linkinfo.c  |  81 ++++++++++++++------------------
>   net/ethtool/linkmodes.c |  91 +++++++++++++++++-------------------
>   net/ethtool/module.c    |  89 ++++++++++++++---------------------
>   net/ethtool/netlink.c   |  39 ++++++++++------
>   net/ethtool/netlink.h   |  13 ------
>   net/ethtool/plca.c      |  75 +++++++++--------------------
>   net/ethtool/privflags.c |  84 ++++++++++++++++-----------------
>   net/ethtool/pse-pd.c    |  79 ++++++++++++-------------------
>   net/ethtool/rings.c     | 101 +++++++++++++++++-----------------------
>   net/ethtool/wol.c       |  79 +++++++++++++------------------
>   15 files changed, 474 insertions(+), 674 deletions(-)
> 


Acked-by: Tariq Toukan <tariqt@nvidia.com>

Re: [PATCH net-next 2/2] ethtool: netlink: convert commands to common SET

[Daniele Palmas]

Il giorno sab 21 gen 2023 alle ore 06:44 Jakub Kicinski
<kuba@kernel.org> ha scritto:
>
> Convert all SET commands where new common code is applicable.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---

Tested just the coalesce aggr settings and looks good.

Thanks,
Daniele

Re: [PATCH net-next 1/2] ethtool: netlink: handle SET intro/outro in the common code

[Paolo Abeni]

Hi,

On Fri, 2023-01-20 at 21:44 -0800, Jakub Kicinski wrote:
> diff --git a/net/ethtool/pause.c b/net/ethtool/pause.c
> index a8c113d244db..8e9aced3eeec 100644
> --- a/net/ethtool/pause.c
> +++ b/net/ethtool/pause.c
> @@ -114,18 +114,6 @@ static int pause_fill_reply(struct sk_buff *skb,
>  	return 0;
>  }
>  
> -const struct ethnl_request_ops ethnl_pause_request_ops = {
> -	.request_cmd		= ETHTOOL_MSG_PAUSE_GET,
> -	.reply_cmd		= ETHTOOL_MSG_PAUSE_GET_REPLY,
> -	.hdr_attr		= ETHTOOL_A_PAUSE_HEADER,
> -	.req_info_size		= sizeof(struct pause_req_info),
> -	.reply_data_size	= sizeof(struct pause_reply_data),
> -
> -	.prepare_data		= pause_prepare_data,
> -	.reply_size		= pause_reply_size,
> -	.fill_reply		= pause_fill_reply,
> -};
> -
>  /* PAUSE_SET */
>  
>  const struct nla_policy ethnl_pause_set_policy[] = {

This chunk does not apply cleanly due to commit 04692c9020b7 ("net:
ethtool: netlink: retrieve stats from multiple sources (eMAC, pMAC)")

Could you please rebase?

Thanks!

Paolo

Re: [PATCH net-next 2/2] ethtool: netlink: convert commands to common SET

[Paolo Abeni]

Hi,

On Fri, 2023-01-20 at 21:44 -0800, Jakub Kicinski wrote:
> @@ -241,49 +229,41 @@ const struct nla_policy ethnl_coalesce_set_policy[] = {
>  	[ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS] = { .type = NLA_U32 },
>  };
>  
> -int ethnl_set_coalesce(struct sk_buff *skb, struct genl_info *info)
> +static int
> +ethnl_set_coalesce_validate(struct ethnl_req_info *req_info,
> +			    struct genl_info *info)
>  {
> -	struct kernel_ethtool_coalesce kernel_coalesce = {};
> -	struct ethtool_coalesce coalesce = {};
> -	struct ethnl_req_info req_info = {};
> +	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
>  	struct nlattr **tb = info->attrs;
> -	const struct ethtool_ops *ops;
> -	struct net_device *dev;
>  	u32 supported_params;
> -	bool mod = false;
> -	int ret;
>  	u16 a;
>  
> -	ret = ethnl_parse_header_dev_get(&req_info,
> -					 tb[ETHTOOL_A_COALESCE_HEADER],
> -					 genl_info_net(info), info->extack,
> -					 true);
> -	if (ret < 0)
> -		return ret;
> -	dev = req_info.dev;
> -	ops = dev->ethtool_ops;
> -	ret = -EOPNOTSUPP;
> -	if (!ops->get_coalesce || !ops->set_coalesce)
> -		goto out_dev;
> -
>  	/* make sure that only supported parameters are present */
>  	supported_params = ops->supported_coalesce_params;
>  	for (a = ETHTOOL_A_COALESCE_RX_USECS; a < __ETHTOOL_A_COALESCE_CNT; a++)
>  		if (tb[a] && !(supported_params & attr_to_mask(a))) {
> -			ret = -EINVAL;
>  			NL_SET_ERR_MSG_ATTR(info->extack, tb[a],
>  					    "cannot modify an unsupported parameter");
> -			goto out_dev;
> +			return -EINVAL;
>  		}
>  
> -	rtnl_lock();
> -	ret = ethnl_ops_begin(dev);
> -	if (ret < 0)
> -		goto out_rtnl;
> -	ret = ops->get_coalesce(dev, &coalesce, &kernel_coalesce,
> -				info->extack);
> +	return ops->get_coalesce && ops->set_coalesce ? 1 : -EOPNOTSUPP;

The above changes the error code in case a coalesce op is missing (and
likely supported_coalesce_params is zero) from EOPNOTSUPP to EINVAL.

I guess it's not a big deal but perhaps we can preserve the same error
code as prior to this patch? 

Not a big deal, I see other commands check the oops last.

Thanks,

Paolo

Re: [PATCH net-next 1/2] ethtool: netlink: handle SET intro/outro in the common code

[Jakub Kicinski]

On Tue, 24 Jan 2023 12:34:23 +0100 Paolo Abeni wrote:
> On Fri, 2023-01-20 at 21:44 -0800, Jakub Kicinski wrote:
> > diff --git a/net/ethtool/pause.c b/net/ethtool/pause.c
> > index a8c113d244db..8e9aced3eeec 100644
> > --- a/net/ethtool/pause.c
> > +++ b/net/ethtool/pause.c
> > @@ -114,18 +114,6 @@ static int pause_fill_reply(struct sk_buff *skb,
> >  	return 0;
> >  }
> >  
> > -const struct ethnl_request_ops ethnl_pause_request_ops = {
> > -	.request_cmd		= ETHTOOL_MSG_PAUSE_GET,
> > -	.reply_cmd		= ETHTOOL_MSG_PAUSE_GET_REPLY,
> > -	.hdr_attr		= ETHTOOL_A_PAUSE_HEADER,
> > -	.req_info_size		= sizeof(struct pause_req_info),
> > -	.reply_data_size	= sizeof(struct pause_reply_data),
> > -
> > -	.prepare_data		= pause_prepare_data,
> > -	.reply_size		= pause_reply_size,
> > -	.fill_reply		= pause_fill_reply,
> > -};
> > -
> >  /* PAUSE_SET */
> >  
> >  const struct nla_policy ethnl_pause_set_policy[] = {  
> 
> This chunk does not apply cleanly due to commit 04692c9020b7 ("net:
> ethtool: netlink: retrieve stats from multiple sources (eMAC, pMAC)")
> 
> Could you please rebase?

Ah, sorry, didn't realize that series was changing request_ops.

I'll repost once Vladimir's fixes are in, because I think we should
add attrs and extack to req_info. That way we avoid all the bugs
with people expecting info to not be NULL and also one fewer param
for all the functions.