Thread (26 messages) flat view 26 messages, 7 authors, 2017-12-18

Re: [RFC PATCH 2/9] ethtool: introduce ethtool netlink interface

From: Jiri Pirko <jiri@resnulli.us>
Date: 2017-12-11 16:02:29
Also in: lkml

Mon, Dec 11, 2017 at 02:53:31PM CET, mkubecek@suse.cz wrote:
No function implemented yet, only genetlink and module infrastructure.
Register/unregister genetlink family "ethtool" and allow the module to be
autoloaded by genetlink code (if built as a module, distributions would
probably prefer "y").

Signed-off-by: Michal Kubecek <redacted>
[...]

+
+/* identifies the device to query/set
+ * - use either ifindex or ifname, not both
+ * - for dumps and messages not related to a particular devices, fill neither
+ * - info_mask is a bitfield, interpretation depends on the command
+ */
+struct ethnlmsghdr {
+	__u32	ifindex;		/* device ifindex */
+	__u16	flags;			/* request/response flags */
+	__u16	info_mask;		/* request/response info mask */
+	char	ifname[IFNAMSIZ];	/* device name */
Why do you need this header? You can have 2 attrs:
ETHTOOL_ATTR_IFINDEX and
ETHTOOL_ATTR_IFNAME

Why do you need per-command flags and info_mask? Could be bitfield
attr if needed by specific command.


quoted hunk ↗ jump to hunk
+};
+
+#define ETHNL_HDRLEN NLMSG_ALIGN(sizeof(struct ethnlmsghdr))
+
+enum {
+	ETHTOOL_CMD_NOOP,
+
+	__ETHTOOL_CMD_MAX,
+	ETHTOOL_CMD_MAX = (__ETHTOOL_CMD_MAX - 1),
+};
+
+/* generic netlink info */
+#define ETHTOOL_GENL_NAME "ethtool"
+#define ETHTOOL_GENL_VERSION 1
+
+#endif /* _UAPI_LINUX_ETHTOOL_NETLINK_H_ */
diff --git a/net/Kconfig b/net/Kconfig
index 9dba2715919d..a5e3c89a2495 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -440,6 +440,13 @@ config MAY_USE_DEVLINK
	  on MAY_USE_DEVLINK to ensure they do not cause link errors when
	  devlink is a loadable module and the driver using it is built-in.

+config ETHTOOL_NETLINK
+	tristate "Netlink interface for ethtool"
+	default m
+	help
+	   New netlink based interface for ethtool which is going to obsolete
+	   the old ioctl based one once it provides all features.
+
endif   # if NET

# Used by archs to tell that they support BPF JIT compiler plus which flavour.
diff --git a/net/core/Makefile b/net/core/Makefile
index 1fd0a9c88b1b..617ab2abecdf 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_DST_CACHE) += dst_cache.o
obj-$(CONFIG_HWBM) += hwbm.o
obj-$(CONFIG_NET_DEVLINK) += devlink.o
obj-$(CONFIG_GRO_CELLS) += gro_cells.o
+obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_netlink.o
diff --git a/net/core/ethtool_netlink.c b/net/core/ethtool_netlink.c
new file mode 100644
index 000000000000..46a226bb9a2c
--- /dev/null
+++ b/net/core/ethtool_netlink.c
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#include <linux/module.h>
+#include <linux/ethtool_netlink.h>
+#include <linux/netdevice.h>
+#include <net/genetlink.h>
+#include "ethtool_common.h"
+
+static struct genl_family ethtool_genl_family;
+
+/* genetlink paperwork */
What "paperwork"? :)

+
+static const struct genl_ops ethtool_genl_ops[] = {
+};
+
+static struct genl_family ethtool_genl_family = {
+	.hdrsize	= ETHNL_HDRLEN,
+	.name		= ETHTOOL_GENL_NAME,
+	.version	= ETHTOOL_GENL_VERSION,
+	.netnsok	= true,
+	.ops		= ethtool_genl_ops,
+	.n_ops		= ARRAY_SIZE(ethtool_genl_ops),
+};
+
+/* module paperwork */
+
+static int __init ethtool_nl_init(void)
+{
+	return genl_register_family(&ethtool_genl_family);
+}
+
+static void __exit ethtool_nl_exit(void)
+{
+	genl_unregister_family(&ethtool_genl_family);
+}
+
+module_init(ethtool_nl_init);
+module_exit(ethtool_nl_exit);
+
+/* this alias is for autoload */
+MODULE_ALIAS("net-pf-" __stringify(PF_NETLINK)
+	     "-proto-" __stringify(NETLINK_GENERIC)
+	     "-family-" ETHTOOL_GENL_NAME);
You can use MODULE_ALIAS_GENL_FAMILY instead.

+MODULE_AUTHOR("Michal Kubecek [off-list ref]");
+MODULE_DESCRIPTION("Netlink interface for ethtool");
+MODULE_LICENSE("GPL");
"GPL v2"?

-- 
2.15.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help