diff mbox series

[net-next,v5,10/22] ethtool: generic handlers for GET requests

Message ID bda9c044a590ed61caa43c37d32b3b7f2b6326c5.1553532199.git.mkubecek@suse.cz
State Changes Requested
Delegated to: David Miller
Headers show
Series ethtool netlink interface, part 1 | expand

Commit Message

Michal Kubecek March 25, 2019, 5:08 p.m. UTC
Some parts of processing GET type requests and related notifications are
independent of a command. Provide universal functions so that only four
callbacks need to be defined for each command type:

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

These callback are defined in an instance of struct get_request_ops.

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

Comments

Jiri Pirko March 27, 2019, 4:35 p.m. UTC | #1
Mon, Mar 25, 2019 at 06:08:24PM CET, mkubecek@suse.cz wrote:
>Some parts of processing GET type requests and related notifications are
>independent of a command. Provide universal functions so that only four
>callbacks need to be defined for each command type:
>
>  parse_request() - parse incoming message
>  prepare_data()  - retrieve data from driver or NIC
>  reply_size()    - estimate reply message size
>  fill_reply()    - compose reply message
>
>These callback are defined in an instance of struct get_request_ops.
>
>Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

[...]

>+/**
>+ * struct get_request_ops - unified handling of GET requests
>+ * @request_cmd:    command id for request (GET)
>+ * @reply_cmd:      command id for reply (SET)
>+ * @dev_attr:       attribute type for device specification
>+ * @data_size:      total length of data structure
>+ * @repdata_offset: offset of "reply data" part (struct common_reply_data)
>+ * @allow_nodev_do: do not fail if device is not specified for non-dump request
>+ * @parse_request:
>+ *	parse request message and fill request info; request info is zero
>+ *	initialized on entry except reply_data pointer (which is initialized)
>+ * @prepare_data:
>+ *	retrieve data needed to compose a reply message; reply data are zero
>+ *	initialized on entry except for @dev
>+ * @reply_size:
>+ *	return size of reply message payload without device specification;
>+ *	returned size may be bigger than actual reply size but it must suffice
>+ *	to hold the reply
>+ * @fill_reply:
>+ *	fill reply message payload using the data prepared by @prepare_data()
>+ * @cleanup
>+ *	(optional) called when data are no longer needed; use e.g. to free
>+ *	any additional data structures allocated in prepare_data() which are
>+ *	not part of the main structure
>+ *
>+ * Description of variable parts of GET request handling when using the unified
>+ * infrastructure. When used, a pointer to an instance of this structure is to
>+ * be added to &get_requests array, generic handlers ethnl_get_doit(),
>+ * ethnl_get_dumpit(), ethnl_get_start() and ethnl_get_done() used in
>+ * @ethnl_genl_ops and (optionally) ethnl_std_notify() as notification handler
>+ * in &ethnl_notify_handlers.
>+ */
>+struct get_request_ops {

First of all, please maintain the ethnl prefix. Not only here, but also
for other structs and functions (common_req_info, parse_*, etc).

But I must ask, why do we need this wrappers of ops around ops?
I believe it would be much nicer to use genl ops directly and do the
common work in pre_doit and and post_doit. Please see devlink.c for
examples.

Wrappers are unfortunately quite common in this patchset. Most of the
time, they do things much harder to follow and understand :(


>+	u8			request_cmd;
>+	u8			reply_cmd;
>+	u16			dev_attrtype;
>+	unsigned int		data_size;
>+	unsigned int		repdata_offset;
>+	bool			allow_nodev_do;
>+
>+	int (*parse_request)(struct common_req_info *req_info,
>+			     struct sk_buff *skb, struct genl_info *info,
>+			     const struct nlmsghdr *nlhdr);
>+	int (*prepare_data)(struct common_req_info *req_info,
>+			    struct genl_info *info);
>+	int (*reply_size)(const struct common_req_info *req_info);
>+	int (*fill_reply)(struct sk_buff *skb,
>+			  const struct common_req_info *req_info);
>+	void (*cleanup)(struct common_req_info *req_info);
>+};
>+
> #endif /* _NET_ETHTOOL_NETLINK_H */
>-- 
>2.21.0
>
Michal Kubecek March 27, 2019, 9:53 p.m. UTC | #2
On Wed, Mar 27, 2019 at 05:35:07PM +0100, Jiri Pirko wrote:
> Mon, Mar 25, 2019 at 06:08:24PM CET, mkubecek@suse.cz wrote:
> >Some parts of processing GET type requests and related notifications are
> >independent of a command. Provide universal functions so that only four
> >callbacks need to be defined for each command type:
> >
> >  parse_request() - parse incoming message
> >  prepare_data()  - retrieve data from driver or NIC
> >  reply_size()    - estimate reply message size
> >  fill_reply()    - compose reply message
> >
> >These callback are defined in an instance of struct get_request_ops.
> >
> >Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> 
> [...]
> 
> >+/**
> >+ * struct get_request_ops - unified handling of GET requests
> >+ * @request_cmd:    command id for request (GET)
> >+ * @reply_cmd:      command id for reply (SET)
> >+ * @dev_attr:       attribute type for device specification
> >+ * @data_size:      total length of data structure
> >+ * @repdata_offset: offset of "reply data" part (struct common_reply_data)
> >+ * @allow_nodev_do: do not fail if device is not specified for non-dump request
> >+ * @parse_request:
> >+ *	parse request message and fill request info; request info is zero
> >+ *	initialized on entry except reply_data pointer (which is initialized)
> >+ * @prepare_data:
> >+ *	retrieve data needed to compose a reply message; reply data are zero
> >+ *	initialized on entry except for @dev
> >+ * @reply_size:
> >+ *	return size of reply message payload without device specification;
> >+ *	returned size may be bigger than actual reply size but it must suffice
> >+ *	to hold the reply
> >+ * @fill_reply:
> >+ *	fill reply message payload using the data prepared by @prepare_data()
> >+ * @cleanup
> >+ *	(optional) called when data are no longer needed; use e.g. to free
> >+ *	any additional data structures allocated in prepare_data() which are
> >+ *	not part of the main structure
> >+ *
> >+ * Description of variable parts of GET request handling when using the unified
> >+ * infrastructure. When used, a pointer to an instance of this structure is to
> >+ * be added to &get_requests array, generic handlers ethnl_get_doit(),
> >+ * ethnl_get_dumpit(), ethnl_get_start() and ethnl_get_done() used in
> >+ * @ethnl_genl_ops and (optionally) ethnl_std_notify() as notification handler
> >+ * in &ethnl_notify_handlers.
> >+ */
> >+struct get_request_ops {
> 
> First of all, please maintain the ethnl prefix. Not only here, but also
> for other structs and functions (common_req_info, parse_*, etc).
> 
> But I must ask, why do we need this wrappers of ops around ops?
> I believe it would be much nicer to use genl ops directly and do the
> common work in pre_doit and and post_doit. Please see devlink.c for
> examples.
> 
> Wrappers are unfortunately quite common in this patchset. Most of the
> time, they do things much harder to follow and understand :(

I'm a bit surprised by this position because so far my experience with
linux networking code seemed to suggest that using simple wrappers and
helpers is how things are supposed to be done. And while following a
chain of such wrappers (often each in a different file) in a code I was
reading for the first time could be frustrating at times, mostly I had
to admit that this style has its merits. After all, genetlink itself is
full of simple wrappers around netlink functions.

Let me point out one thing: most of these helpers and wrappers are not
artificial, they haven't been written in advance with an idea that they
might be useful (the patch series does not, of course, reflect the
development history); most of them were written when I realized I'm
writing the same or almost the same code again and again.

So when I caught myself writing

  ... = nla_nest_start(skb, ... | NLA_F_NESTED);

for the third or fourth time and I realized that every nla_nest_start()
call in the code will have this bitwise or, I felt it would deserve
a helper. (If I expected some objection, it was rather the optical
asymmetry of ethnl_nest_start() being closed with nla_nest_end().)
It would be much nicer to have it in nla_nest_start() but unfortunately
it's too late for that.

And it's exactly the same case with get_request_ops. For quite long
(until after RFC v2), this framework didn't exist and code for get
request processing (both doit and dumpit) and notifications was written
separately for each message type. Realizing that big part of each new
file is in fact an exact copy of the previous one with some string
replacements and that it's going to be like that for most of the future
files, that led me to identifying which parts are specific to message
type and which are generic.

If I have to get rid of get_request_ops, it will only result in having
multiple copies of functions which would replace ethnl_get_doit(),
ethnl_get_dumpit() and ethnl_std_notify(). They would be slightly
simpler but would look the same except for "info" in one being replaced
by "strset" in second, "settings" in third etc. Later, there would be
one more copy for stats, one for tunables etc.

I don't think the generic code can be handled just by pre_doit and
post_doit as the generic and message specific part are interleaved and
the generic parts are also different for do requests, dump requests and
notifications.

Michal
Jiri Pirko March 28, 2019, 1:32 p.m. UTC | #3
Wed, Mar 27, 2019 at 10:53:42PM CET, mkubecek@suse.cz wrote:
>On Wed, Mar 27, 2019 at 05:35:07PM +0100, Jiri Pirko wrote:
>> Mon, Mar 25, 2019 at 06:08:24PM CET, mkubecek@suse.cz wrote:
>> >Some parts of processing GET type requests and related notifications are
>> >independent of a command. Provide universal functions so that only four
>> >callbacks need to be defined for each command type:
>> >
>> >  parse_request() - parse incoming message
>> >  prepare_data()  - retrieve data from driver or NIC
>> >  reply_size()    - estimate reply message size
>> >  fill_reply()    - compose reply message
>> >
>> >These callback are defined in an instance of struct get_request_ops.
>> >
>> >Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
>> 
>> [...]
>> 
>> >+/**
>> >+ * struct get_request_ops - unified handling of GET requests
>> >+ * @request_cmd:    command id for request (GET)
>> >+ * @reply_cmd:      command id for reply (SET)
>> >+ * @dev_attr:       attribute type for device specification
>> >+ * @data_size:      total length of data structure
>> >+ * @repdata_offset: offset of "reply data" part (struct common_reply_data)
>> >+ * @allow_nodev_do: do not fail if device is not specified for non-dump request
>> >+ * @parse_request:
>> >+ *	parse request message and fill request info; request info is zero
>> >+ *	initialized on entry except reply_data pointer (which is initialized)
>> >+ * @prepare_data:
>> >+ *	retrieve data needed to compose a reply message; reply data are zero
>> >+ *	initialized on entry except for @dev
>> >+ * @reply_size:
>> >+ *	return size of reply message payload without device specification;
>> >+ *	returned size may be bigger than actual reply size but it must suffice
>> >+ *	to hold the reply
>> >+ * @fill_reply:
>> >+ *	fill reply message payload using the data prepared by @prepare_data()
>> >+ * @cleanup
>> >+ *	(optional) called when data are no longer needed; use e.g. to free
>> >+ *	any additional data structures allocated in prepare_data() which are
>> >+ *	not part of the main structure
>> >+ *
>> >+ * Description of variable parts of GET request handling when using the unified
>> >+ * infrastructure. When used, a pointer to an instance of this structure is to
>> >+ * be added to &get_requests array, generic handlers ethnl_get_doit(),
>> >+ * ethnl_get_dumpit(), ethnl_get_start() and ethnl_get_done() used in
>> >+ * @ethnl_genl_ops and (optionally) ethnl_std_notify() as notification handler
>> >+ * in &ethnl_notify_handlers.
>> >+ */
>> >+struct get_request_ops {
>> 
>> First of all, please maintain the ethnl prefix. Not only here, but also
>> for other structs and functions (common_req_info, parse_*, etc).
>> 
>> But I must ask, why do we need this wrappers of ops around ops?
>> I believe it would be much nicer to use genl ops directly and do the
>> common work in pre_doit and and post_doit. Please see devlink.c for
>> examples.
>> 
>> Wrappers are unfortunately quite common in this patchset. Most of the
>> time, they do things much harder to follow and understand :(
>
>I'm a bit surprised by this position because so far my experience with
>linux networking code seemed to suggest that using simple wrappers and
>helpers is how things are supposed to be done. And while following a
>chain of such wrappers (often each in a different file) in a code I was
>reading for the first time could be frustrating at times, mostly I had
>to admit that this style has its merits. After all, genetlink itself is
>full of simple wrappers around netlink functions.
>
>Let me point out one thing: most of these helpers and wrappers are not
>artificial, they haven't been written in advance with an idea that they
>might be useful (the patch series does not, of course, reflect the
>development history); most of them were written when I realized I'm
>writing the same or almost the same code again and again.
>
>So when I caught myself writing
>
>  ... = nla_nest_start(skb, ... | NLA_F_NESTED);
>
>for the third or fourth time and I realized that every nla_nest_start()
>call in the code will have this bitwise or, I felt it would deserve
>a helper. (If I expected some objection, it was rather the optical
>asymmetry of ethnl_nest_start() being closed with nla_nest_end().)
>It would be much nicer to have it in nla_nest_start() but unfortunately
>it's too late for that.

Okay, this wrapper I can understand, but it certainly isn't ethnl
specific wrapper. It is generic, many others might use it:
$ git grep NLA_F_NESTED |grep nla_nest_start


>
>And it's exactly the same case with get_request_ops. For quite long
>(until after RFC v2), this framework didn't exist and code for get
>request processing (both doit and dumpit) and notifications was written
>separately for each message type. Realizing that big part of each new
>file is in fact an exact copy of the previous one with some string
>replacements and that it's going to be like that for most of the future
>files, that led me to identifying which parts are specific to message
>type and which are generic.
>
>If I have to get rid of get_request_ops, it will only result in having
>multiple copies of functions which would replace ethnl_get_doit(),
>ethnl_get_dumpit() and ethnl_std_notify(). They would be slightly
>simpler but would look the same except for "info" in one being replaced
>by "strset" in second, "settings" in third etc. Later, there would be
>one more copy for stats, one for tunables etc.
>
>I don't think the generic code can be handled just by pre_doit and
>post_doit as the generic and message specific part are interleaved and
>the generic parts are also different for do requests, dump requests and
>notifications.

What do you mean by "interleaved"? I guess you can make the attrs to be
formatted in the way it is not interleaved. Like what I suggested, to
have top-level generic attr enum and one of the generic attrs would nest
the cmd-specific attrs. That way, pre_doit can work with the generic
attrs, doit op can work with cmd-specific attrs. Similar to devlink
code.

>
>Michal
diff mbox series

Patch

diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index d84b9780dfa7..7371cdf98ce9 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -151,6 +151,325 @@  struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
 	return NULL;
 }
 
+/* GET request helpers */
+
+const struct get_request_ops *get_requests[__ETHNL_CMD_CNT] = {
+};
+
+/**
+ * ethnl_alloc_get_data() - Allocate and initialize data for a GET request
+ * @ops: instance of struct get_request_ops describing size and layout
+ *
+ * This initializes only the first part (req_info), second part (reply_data)
+ * is initialized before filling the reply data into it (which is done for
+ * each iteration in dump requests).
+ *
+ * Return: pointer to allocated and initialized data, NULL on error
+ */
+static struct common_req_info *
+ethnl_alloc_get_data(const struct get_request_ops *ops)
+{
+	struct common_req_info *req_info;
+
+	req_info = kmalloc(ops->data_size, GFP_KERNEL);
+	if (!req_info)
+		return NULL;
+
+	memset(req_info, '\0', ops->repdata_offset);
+	req_info->reply_data =
+		(struct common_reply_data *)((char *)req_info +
+					     ops->repdata_offset);
+
+	return req_info;
+}
+
+/**
+ * ethnl_free_get_data() - free GET request data
+ * @ops: instance of struct get_request_ops describing the layout
+ * @req_info: pointer to embedded struct common_req_info (at offset 0)
+ *
+ * Calls ->cleanup() handler if defined and frees the data block.
+ */
+static void ethnl_free_get_data(const struct get_request_ops *ops,
+				struct common_req_info *req_info)
+{
+	if (ops->cleanup)
+		ops->cleanup(req_info);
+	kfree(req_info);
+}
+
+/**
+ * ethnl_init_reply_data() - Initialize reply data for GET request
+ * @req_info: pointer to embedded struct common_req_info
+ * @ops:      instance of struct get_request_ops describing the layout
+ * @dev:      network device to initialize the reply for
+ *
+ * Fills the reply data part with zeros and sets the dev member. Must be called
+ * before calling the ->fill_reply() callback (for each iteration when handling
+ * dump requests).
+ */
+static void ethnl_init_reply_data(const struct common_req_info *req_info,
+				  const struct get_request_ops *ops,
+				  struct net_device *dev)
+{
+	memset(req_info->reply_data, '\0',
+	       ops->data_size - ops->repdata_offset);
+	req_info->reply_data->dev = dev;
+}
+
+/* generic ->doit() handler for GET type requests */
+int ethnl_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	const u8 cmd = info->genlhdr->cmd;
+	struct common_req_info *req_info;
+	const struct get_request_ops *ops;
+	struct sk_buff *rskb;
+	void *reply_payload;
+	int reply_len;
+	int ret;
+
+	ops = get_requests[cmd];
+	if (WARN_ONCE(!ops, "cmd %u has no get_request_ops\n", cmd))
+		return -EOPNOTSUPP;
+	req_info = ethnl_alloc_get_data(ops);
+	if (!req_info)
+		return -ENOMEM;
+	ret = ops->parse_request(req_info, skb, info, info->nlhdr);
+	if (!ops->allow_nodev_do && !req_info->dev) {
+		ETHNL_SET_ERRMSG(info, "device not specified in do request");
+		ret = -EINVAL;
+	}
+	if (ret < 0)
+		goto err_dev;
+	ethnl_init_reply_data(req_info, ops, req_info->dev);
+
+	rtnl_lock();
+	ret = ops->prepare_data(req_info, info);
+	if (ret < 0)
+		goto err_rtnl;
+	reply_len = ops->reply_size(req_info);
+	if (ret < 0)
+		goto err_rtnl;
+	ret = -ENOMEM;
+	rskb = ethnl_reply_init(reply_len, req_info->dev, ops->reply_cmd,
+				ops->dev_attrtype, info, &reply_payload);
+	if (!rskb)
+		goto err_rtnl;
+	ret = ops->fill_reply(rskb, req_info);
+	if (ret < 0)
+		goto err;
+	rtnl_unlock();
+
+	genlmsg_end(rskb, reply_payload);
+	if (req_info->dev)
+		dev_put(req_info->dev);
+	ethnl_free_get_data(ops, req_info);
+	return genlmsg_reply(rskb, info);
+
+err:
+	WARN_ONCE(ret == -EMSGSIZE,
+		  "calculated message payload length (%d) not sufficient\n",
+		  reply_len);
+	nlmsg_free(rskb);
+	ethnl_free_get_data(ops, req_info);
+err_rtnl:
+	rtnl_unlock();
+err_dev:
+	if (req_info->dev)
+		dev_put(req_info->dev);
+	return ret;
+}
+
+static int ethnl_get_dump_one(struct sk_buff *skb,
+			      struct net_device *dev,
+			      const struct get_request_ops *ops,
+			      struct common_req_info *req_info)
+{
+	int ret;
+
+	ethnl_init_reply_data(req_info, ops, dev);
+	rtnl_lock();
+	ret = ops->prepare_data(req_info, NULL);
+	if (ret < 0)
+		return ret;
+	ret = ethnl_fill_dev(skb, dev, ops->dev_attrtype);
+	if (ret < 0)
+		return ret;
+	ret = ops->fill_reply(skb, req_info);
+	rtnl_unlock();
+
+	req_info->reply_data->dev = NULL;
+	return ret;
+}
+
+/* generic ->dumpit() handler for GET requests; device iteration copied from
+ * rtnl_dump_ifinfo()
+ * cb->args[0]: pointer to struct get_request_ops
+ * cb->args[1]: pointer to request data
+ * cb->args[2]: iteration position - hashbucket
+ * cb->args[3]: iteration position - ifindex
+ */
+int ethnl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	struct net *net = sock_net(skb->sk);
+	struct common_req_info *req_info;
+	const struct get_request_ops *ops;
+	int h, s_h, idx = 0, s_idx;
+	struct hlist_head *head;
+	struct net_device *dev;
+	int ret = 0;
+	void *ehdr;
+
+	ops = (const struct get_request_ops *)cb->args[0];
+	req_info = (struct common_req_info *)cb->args[1];
+	s_h = cb->args[2];
+	s_idx = cb->args[3];
+
+	for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
+		idx = 0;
+		head = &net->dev_index_head[h];
+		hlist_for_each_entry(dev, head, index_hlist) {
+			if (idx < s_idx)
+				goto cont;
+			ehdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
+					   cb->nlh->nlmsg_seq,
+					   &ethtool_genl_family, 0,
+					   ops->reply_cmd);
+			ret = ethnl_get_dump_one(skb, dev, ops, req_info);
+			if (ret < 0) {
+				genlmsg_cancel(skb, ehdr);
+				if (ret == -EOPNOTSUPP)
+					goto cont;
+				if (likely(skb->len))
+					goto out;
+				goto out_err;
+			}
+			genlmsg_end(skb, ehdr);
+cont:
+			idx++;
+		}
+	}
+out:
+	ret = skb->len;
+out_err:
+	cb->args[2] = h;
+	cb->args[3] = idx;
+	cb->seq = net->dev_base_seq;
+	nl_dump_check_consistent(cb, nlmsg_hdr(skb));
+
+	return ret;
+}
+
+/* generic ->start() handler for GET requests */
+static int ethnl_get_start(struct netlink_callback *cb)
+{
+	struct common_req_info *req_info;
+	const struct get_request_ops *ops;
+	struct genlmsghdr *ghdr;
+	int ret;
+
+	ghdr = nlmsg_data(cb->nlh);
+	ops = get_requests[ghdr->cmd];
+	if (WARN_ONCE(!ops, "cmd %u has no get_request_ops\n", ghdr->cmd))
+		return -EOPNOTSUPP;
+	req_info = ethnl_alloc_get_data(ops);
+	if (!req_info)
+		return -ENOMEM;
+
+	ret = ops->parse_request(req_info, cb->skb, NULL, cb->nlh);
+	if (req_info->dev) {
+		/* We ignore device specification in dump requests but as the
+		 * same parser as for non-dump (doit) requests is used, it
+		 * would take reference to the device if it finds one
+		 */
+		dev_put(req_info->dev);
+		req_info->dev = NULL;
+	}
+	if (ret < 0)
+		return ret;
+
+	cb->args[0] = (long)ops;
+	cb->args[1] = (long)req_info;
+	cb->args[2] = 0;
+	cb->args[3] = 0;
+
+	return 0;
+}
+
+/* generic ->done() handler for GET requests */
+static int ethnl_get_done(struct netlink_callback *cb)
+{
+	ethnl_free_get_data((const struct get_request_ops *)cb->args[0],
+			    (struct common_req_info *)cb->args[1]);
+
+	return 0;
+}
+
+/* generic notification handler */
+static void ethnl_std_notify(struct net_device *dev,
+			     struct netlink_ext_ack *extack, unsigned int cmd,
+			     u32 req_mask, const void *data)
+{
+	struct common_req_info *req_info;
+	const struct get_request_ops *ops;
+	struct sk_buff *skb;
+	void *reply_payload;
+	int reply_len;
+	int ret;
+
+	ops = get_requests[cmd - 1];
+	if (WARN_ONCE(!ops, "cmd %u has no get_request_ops\n", cmd - 1))
+		return;
+	/* when ethnl_std_notify() is used as notify handler, command id of
+	 * corresponding GET request must be one less than cmd argument passed
+	 * to ethnl_std_notify()
+	 */
+	if (WARN_ONCE(ops->reply_cmd != cmd,
+		      "reply_cmd for %u is %u, expected %u\n", cmd - 1,
+		      ops->reply_cmd, cmd))
+		return;
+
+	req_info = ethnl_alloc_get_data(ops);
+	if (!req_info)
+		return;
+	req_info->dev = dev;
+	req_info->req_mask = req_mask;
+	req_info->compact = true;
+
+	ethnl_init_reply_data(req_info, ops, dev);
+	ret = ops->prepare_data(req_info, NULL);
+	if (ret < 0)
+		goto err_data;
+	reply_len = ops->reply_size(req_info);
+	if (reply_len < 0)
+		goto err_data;
+	skb = genlmsg_new(reply_len, GFP_KERNEL);
+	if (!skb)
+		goto err_data;
+	reply_payload = genlmsg_put(skb, 0, ++ethnl_bcast_seq,
+				    &ethtool_genl_family, 0, ops->reply_cmd);
+	if (!reply_payload)
+		goto err_skb;
+
+	ret = ethnl_fill_dev(skb, dev, ops->dev_attrtype);
+	if (ret < 0)
+		goto err_skb;
+	ret = ops->fill_reply(skb, req_info);
+	if (ret < 0)
+		goto err_skb;
+	ethnl_free_get_data(ops, req_info);
+	genlmsg_end(skb, reply_payload);
+
+	genlmsg_multicast(&ethtool_genl_family, skb, 0, ETHNL_MCGRP_MONITOR,
+			  GFP_KERNEL);
+	return;
+
+err_skb:
+	nlmsg_free(skb);
+err_data:
+	ethnl_free_get_data(ops, req_info);
+}
+
 /* notifications */
 
 typedef void (*ethnl_notify_handler_t)(struct net_device *dev,
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 5f2299548915..625f912144b1 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -164,4 +164,115 @@  static inline unsigned int dev_ident_size(void)
 			      nla_total_size(IFNAMSIZ));
 }
 
+/* GET request handling */
+
+struct common_reply_data;
+
+/* The structure holding data for unified processing a GET request consists of
+ * two parts: request info and reply data. Request info starts at offset 0 with
+ * embedded struct common_req_info, is usually filled by ->parse_request() and
+ * is common for all reply messages to one request. Reply data start with
+ * embedded struct common_reply_data and contain data specific to a reply
+ * message (usually one per device for dump requests); this part is filled by
+ * ->prepare_data()
+ */
+
+/**
+ * struct common_req_info - base type of request information for GET requests
+ * @reply_data: pointer to reply data within the same block
+ * @dev:        network device the request is for (may be null)
+ * @req_mask:   request mask, bitmap of requested information
+ * @compact:    true if compact format of bitsets in reply is requested
+ *
+ * This is a common base, additional members may follow after this structure.
+ */
+struct common_req_info {
+	struct common_reply_data	*reply_data;
+	struct net_device		*dev;
+	u32				req_mask;
+	bool				compact;
+};
+
+/**
+ * struct common_reply_data - base type of reply data for GET requests
+ * @dev:       device for current reply message; in single shot requests it is
+ *             equal to &common_req_info.dev; in dumps it's different for each
+ *             reply message
+ * @info_mask: bitmap of information actually provided in reply; it is a subset
+ *             of &common_req_info.req_mask with cleared bits corresponding to
+ *             information which cannot be provided
+ *
+ * This structure is usually followed by additional members filled by
+ * ->prepare_data() and used by ->cleanup().
+ */
+struct common_reply_data {
+	struct net_device		*dev;
+	u32				info_mask;
+};
+
+static inline int ethnl_before_ops(struct net_device *dev)
+{
+	if (dev && dev->ethtool_ops->begin)
+		return dev->ethtool_ops->begin(dev);
+	else
+		return 0;
+}
+
+static inline void ethnl_after_ops(struct net_device *dev)
+{
+	if (dev && dev->ethtool_ops->complete)
+		dev->ethtool_ops->complete(dev);
+}
+
+/**
+ * struct get_request_ops - unified handling of GET requests
+ * @request_cmd:    command id for request (GET)
+ * @reply_cmd:      command id for reply (SET)
+ * @dev_attr:       attribute type for device specification
+ * @data_size:      total length of data structure
+ * @repdata_offset: offset of "reply data" part (struct common_reply_data)
+ * @allow_nodev_do: do not fail if device is not specified for non-dump request
+ * @parse_request:
+ *	parse request message and fill request info; request info is zero
+ *	initialized on entry except reply_data pointer (which is initialized)
+ * @prepare_data:
+ *	retrieve data needed to compose a reply message; reply data are zero
+ *	initialized on entry except for @dev
+ * @reply_size:
+ *	return size of reply message payload without device specification;
+ *	returned size may be bigger than actual reply size but it must suffice
+ *	to hold the reply
+ * @fill_reply:
+ *	fill reply message payload using the data prepared by @prepare_data()
+ * @cleanup
+ *	(optional) called when data are no longer needed; use e.g. to free
+ *	any additional data structures allocated in prepare_data() which are
+ *	not part of the main structure
+ *
+ * Description of variable parts of GET request handling when using the unified
+ * infrastructure. When used, a pointer to an instance of this structure is to
+ * be added to &get_requests array, generic handlers ethnl_get_doit(),
+ * ethnl_get_dumpit(), ethnl_get_start() and ethnl_get_done() used in
+ * @ethnl_genl_ops and (optionally) ethnl_std_notify() as notification handler
+ * in &ethnl_notify_handlers.
+ */
+struct get_request_ops {
+	u8			request_cmd;
+	u8			reply_cmd;
+	u16			dev_attrtype;
+	unsigned int		data_size;
+	unsigned int		repdata_offset;
+	bool			allow_nodev_do;
+
+	int (*parse_request)(struct common_req_info *req_info,
+			     struct sk_buff *skb, struct genl_info *info,
+			     const struct nlmsghdr *nlhdr);
+	int (*prepare_data)(struct common_req_info *req_info,
+			    struct genl_info *info);
+	int (*reply_size)(const struct common_req_info *req_info);
+	int (*fill_reply)(struct sk_buff *skb,
+			  const struct common_req_info *req_info);
+	void (*cleanup)(struct common_req_info *req_info);
+};
+
 #endif /* _NET_ETHTOOL_NETLINK_H */