[patch net-next 0/8] devlink: use spec to generate split ops

[patch net-next 0/8] devlink: use spec to generate split ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

This is an outcome of the discussion in the following thread:
https://lore.kernel.org/netdev/20230720121829.566974-1-jiri@resnulli.us/
It serves as a dependency on the linked selector patchset.

There is an existing spec for devlink used for userspace part
generation. There are two commands supported there.

This patch extends the spec so kernel split ops code could
be generated from it.

Jiri Pirko (8):
  ynl-gen-c.py: fix rendering of validate field
  ynl-gen-c.py: allow directional model for kernel mode
  devlink: rename devlink_nl_ops to devlink_nl_small_ops
  devlink: add split ops generated according to spec
  devlink: include the generated netlink header
  devlink: rename couple of doit netlink callbacks to match generated
    names
  devlink: introduce couple of dumpit callback for split ops
  devlink: use generated split ops and remove duplicated commands from
    small ops

 Documentation/netlink/specs/devlink.yaml | 14 +++++-
 net/devlink/Makefile                     |  2 +-
 net/devlink/dev.c                        | 27 ++++++-----
 net/devlink/devl_internal.h              | 20 ++++----
 net/devlink/leftover.c                   | 16 +------
 net/devlink/netlink.c                    | 35 ++++++++------
 net/devlink/netlink_gen.c                | 59 ++++++++++++++++++++++++
 net/devlink/netlink_gen.h                | 33 +++++++++++++
 tools/net/ynl/ynl-gen-c.py               | 12 ++++-
 9 files changed, 163 insertions(+), 55 deletions(-)
 create mode 100644 net/devlink/netlink_gen.c
 create mode 100644 net/devlink/netlink_gen.h

-- 
2.41.0

[patch net-next 1/8] ynl-gen-c.py: fix rendering of validate field

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

For split ops, do and dump has different value in validate field. Fix
the rendering so for do op, only "strict" is filled out and for dump op,
"strict" is prefixed by "dump-".

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 tools/net/ynl/ynl-gen-c.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 650be9b8b693..1c36d0c935da 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -1988,9 +1988,17 @@ def print_kernel_op_table(family, cw):
                 cw.block_start()
                 members = [('cmd', op.enum_name)]
                 if 'dont-validate' in op:
+                    dont_validate = []
+                    for x in op['dont-validate']:
+                        if op_mode == 'do' and x == 'dump':
+                            continue
+                        if op_mode == "dump" and x == 'strict':
+                            x = 'dump-' + x
+                        dont_validate.append(x)
+
                     members.append(('validate',
                                     ' | '.join([c_upper('genl-dont-validate-' + x)
-                                                for x in op['dont-validate']])), )
+                                                for x in dont_validate])), )
                 name = c_lower(f"{family.name}-nl-{op_name}-{op_mode}it")
                 if 'pre' in op[op_mode]:
                     members.append((cb_names[op_mode]['pre'], c_lower(op[op_mode]['pre'])))
-- 
2.41.0

[patch net-next 2/8] ynl-gen-c.py: allow directional model for kernel mode

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Directional model is only considered in uapi mode. No reason to forbid
directional model for kernel mode, so lift the limitation.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 tools/net/ynl/ynl-gen-c.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 1c36d0c935da..6f77c69fc410 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -2317,7 +2317,7 @@ def main():
         return
 
     supported_models = ['unified']
-    if args.mode == 'user':
+    if args.mode in  ['user', 'kernel']:
         supported_models += ['directional']
     if parsed.msg_id_model not in supported_models:
         print(f'Message enum-model {parsed.msg_id_model} not supported for {args.mode} generation')
-- 
2.41.0

[patch net-next 3/8] devlink: rename devlink_nl_ops to devlink_nl_small_ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

In order to avoid name collision with the generated split ops array
which is going to be introduced as a follow-up patch, rename
the existing ops array to devlink_nl_small_ops.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 net/devlink/devl_internal.h | 2 +-
 net/devlink/leftover.c      | 2 +-
 net/devlink/netlink.c       | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 62921b2eb0d3..c67f074641d4 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -119,7 +119,7 @@ struct devlink_cmd {
 			struct netlink_callback *cb);
 };
 
-extern const struct genl_small_ops devlink_nl_ops[56];
+extern const struct genl_small_ops devlink_nl_small_ops[56];
 
 struct devlink *
 devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs);
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 5128b9c7eea8..8f42f1f45705 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -6278,7 +6278,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 	return devlink_trap_policer_set(devlink, policer_item, info);
 }
 
-const struct genl_small_ops devlink_nl_ops[56] = {
+const struct genl_small_ops devlink_nl_small_ops[56] = {
 	{
 		.cmd = DEVLINK_CMD_GET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c
index 7a332eb70f70..82a3238d5344 100644
--- a/net/devlink/netlink.c
+++ b/net/devlink/netlink.c
@@ -243,8 +243,8 @@ struct genl_family devlink_nl_family __ro_after_init = {
 	.pre_doit	= devlink_nl_pre_doit,
 	.post_doit	= devlink_nl_post_doit,
 	.module		= THIS_MODULE,
-	.small_ops	= devlink_nl_ops,
-	.n_small_ops	= ARRAY_SIZE(devlink_nl_ops),
+	.small_ops	= devlink_nl_small_ops,
+	.n_small_ops	= ARRAY_SIZE(devlink_nl_small_ops),
 	.resv_start_op	= DEVLINK_CMD_SELFTESTS_RUN + 1,
 	.mcgrps		= devlink_nl_mcgrps,
 	.n_mcgrps	= ARRAY_SIZE(devlink_nl_mcgrps),
-- 
2.41.0

[patch net-next 4/8] devlink: add split ops generated according to spec

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Improve the existing devlink spec in order to serve as a source fot
generation of valid devlink split ops for the existing commands.
Add the generated sources.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/specs/devlink.yaml | 14 +++++-
 net/devlink/Makefile                     |  2 +-
 net/devlink/netlink_gen.c                | 59 ++++++++++++++++++++++++
 net/devlink/netlink_gen.h                | 33 +++++++++++++
 4 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 net/devlink/netlink_gen.c
 create mode 100644 net/devlink/netlink_gen.h

diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index 5d46ca966979..f6df0b3fd502 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -165,8 +165,13 @@ operations:
       name: get
       doc: Get devlink instances.
       attribute-set: devlink
+      dont-validate:
+        - strict
+        - dump
 
       do:
+        pre: devlink-nl-pre-doit
+        post: devlink-nl-post-doit
         request:
           value: 1
           attributes: &dev-id-attrs
@@ -189,12 +194,17 @@ operations:
       name: info-get
       doc: Get device information, like driver name, hardware and firmware versions etc.
       attribute-set: devlink
+      dont-validate:
+        - strict
+        - dump
 
       do:
+        pre: devlink-nl-pre-doit
+        post: devlink-nl-post-doit
         request:
           value: 51
           attributes: *dev-id-attrs
-        reply:
+        reply: &info-get-reply
           value: 51
           attributes:
             - bus-name
@@ -204,3 +214,5 @@ operations:
             - info-version-fixed
             - info-version-running
             - info-version-stored
+      dump:
+        reply: *info-get-reply
diff --git a/net/devlink/Makefile b/net/devlink/Makefile
index ef91a76646a3..a087af581847 100644
--- a/net/devlink/Makefile
+++ b/net/devlink/Makefile
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 
-obj-y := leftover.o core.o netlink.o dev.o health.o
+obj-y := leftover.o core.o netlink.o netlink_gen.o dev.o health.o
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
new file mode 100644
index 000000000000..0091edc73a7a
--- /dev/null
+++ b/net/devlink/netlink_gen.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/devlink.yaml */
+/* YNL-GEN kernel source */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "netlink_gen.h"
+
+#include <uapi/linux/devlink.h>
+
+/* DEVLINK_CMD_GET - do */
+const struct nla_policy devlink_get_nl_policy[DEVLINK_ATTR_DEV_NAME + 1] = {
+	[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
+	[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
+};
+
+/* DEVLINK_CMD_INFO_GET - do */
+const struct nla_policy devlink_info_get_nl_policy[DEVLINK_ATTR_DEV_NAME + 1] = {
+	[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
+	[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
+};
+
+/* Ops table for devlink */
+const struct genl_split_ops devlink_nl_ops[4] = {
+	{
+		.cmd		= DEVLINK_CMD_GET,
+		.validate	= GENL_DONT_VALIDATE_STRICT,
+		.pre_doit	= devlink_nl_pre_doit,
+		.doit		= devlink_nl_get_doit,
+		.post_doit	= devlink_nl_post_doit,
+		.policy		= devlink_get_nl_policy,
+		.maxattr	= DEVLINK_ATTR_DEV_NAME,
+		.flags		= GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd		= DEVLINK_CMD_GET,
+		.validate	= GENL_DONT_VALIDATE_DUMP_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.dumpit		= devlink_nl_get_dumpit,
+		.flags		= GENL_CMD_CAP_DUMP,
+	},
+	{
+		.cmd		= DEVLINK_CMD_INFO_GET,
+		.validate	= GENL_DONT_VALIDATE_STRICT,
+		.pre_doit	= devlink_nl_pre_doit,
+		.doit		= devlink_nl_info_get_doit,
+		.post_doit	= devlink_nl_post_doit,
+		.policy		= devlink_info_get_nl_policy,
+		.maxattr	= DEVLINK_ATTR_DEV_NAME,
+		.flags		= GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd		= DEVLINK_CMD_INFO_GET,
+		.validate	= GENL_DONT_VALIDATE_DUMP_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.dumpit		= devlink_nl_info_get_dumpit,
+		.flags		= GENL_CMD_CAP_DUMP,
+	},
+};
diff --git a/net/devlink/netlink_gen.h b/net/devlink/netlink_gen.h
new file mode 100644
index 000000000000..b209869de83f
--- /dev/null
+++ b/net/devlink/netlink_gen.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/devlink.yaml */
+/* YNL-GEN kernel header */
+
+#ifndef _LINUX_DEVLINK_GEN_H
+#define _LINUX_DEVLINK_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/devlink.h>
+
+extern const struct nla_policy devlink_get_nl_policy[DEVLINK_ATTR_DEV_NAME + 1];
+
+extern const struct nla_policy devlink_info_get_nl_policy[DEVLINK_ATTR_DEV_NAME + 1];
+
+/* Ops table for devlink */
+extern const struct genl_split_ops devlink_nl_ops[4];
+
+int devlink_nl_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
+			struct genl_info *info);
+void
+devlink_nl_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
+		     struct genl_info *info);
+
+int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info);
+int devlink_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
+int devlink_nl_info_get_doit(struct sk_buff *skb, struct genl_info *info);
+int devlink_nl_info_get_dumpit(struct sk_buff *skb,
+			       struct netlink_callback *cb);
+
+#endif /* _LINUX_DEVLINK_GEN_H */
-- 
2.41.0

[patch net-next 5/8] devlink: include the generated netlink header

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Put the newly added generated header to the include list. Un-static the
pre-doit and post-doit functions as they are used in the generated
files.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 net/devlink/devl_internal.h | 2 ++
 net/devlink/netlink.c       | 8 ++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index c67f074641d4..f5ad66d5773c 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -12,6 +12,8 @@
 #include <net/devlink.h>
 #include <net/net_namespace.h>
 
+#include "netlink_gen.h"
+
 #define DEVLINK_REGISTERED XA_MARK_1
 
 #define DEVLINK_RELOAD_STATS_ARRAY_SIZE \
diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c
index 82a3238d5344..39e07a5a69af 100644
--- a/net/devlink/netlink.c
+++ b/net/devlink/netlink.c
@@ -109,8 +109,8 @@ devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs)
 	return ERR_PTR(-ENODEV);
 }
 
-static int devlink_nl_pre_doit(const struct genl_split_ops *ops,
-			       struct sk_buff *skb, struct genl_info *info)
+int devlink_nl_pre_doit(const struct genl_split_ops *ops,
+			struct sk_buff *skb, struct genl_info *info)
 {
 	struct devlink_linecard *linecard;
 	struct devlink_port *devlink_port;
@@ -167,8 +167,8 @@ static int devlink_nl_pre_doit(const struct genl_split_ops *ops,
 	return err;
 }
 
-static void devlink_nl_post_doit(const struct genl_split_ops *ops,
-				 struct sk_buff *skb, struct genl_info *info)
+void devlink_nl_post_doit(const struct genl_split_ops *ops,
+			  struct sk_buff *skb, struct genl_info *info)
 {
 	struct devlink *devlink;
 
-- 
2.41.0

[patch net-next 6/8] devlink: rename couple of doit netlink callbacks to match generated names

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

The generated names of the doit netlink callback are missing "cmd" in
their names. Fix the names and remove the original prototypes.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 net/devlink/dev.c           | 4 ++--
 net/devlink/devl_internal.h | 2 --
 net/devlink/leftover.c      | 4 ++--
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index bf1d6f1bcfc7..2e120b3da220 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -196,7 +196,7 @@ void devlink_notify(struct devlink *devlink, enum devlink_command cmd)
 				msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
 }
 
-int devlink_nl_cmd_get_doit(struct sk_buff *skb, struct genl_info *info)
+int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct devlink *devlink = info->user_ptr[0];
 	struct sk_buff *msg;
@@ -804,7 +804,7 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 	return err;
 }
 
-int devlink_nl_cmd_info_get_doit(struct sk_buff *skb, struct genl_info *info)
+int devlink_nl_info_get_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct devlink *devlink = info->user_ptr[0];
 	struct sk_buff *msg;
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index f5ad66d5773c..419bc92da503 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -216,11 +216,9 @@ struct devlink_rate *
 devlink_rate_node_get_from_info(struct devlink *devlink,
 				struct genl_info *info);
 /* Devlink nl cmds */
-int devlink_nl_cmd_get_doit(struct sk_buff *skb, struct genl_info *info);
 int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info);
 int devlink_nl_cmd_eswitch_get_doit(struct sk_buff *skb, struct genl_info *info);
 int devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb, struct genl_info *info);
-int devlink_nl_cmd_info_get_doit(struct sk_buff *skb, struct genl_info *info);
 int devlink_nl_cmd_flash_update(struct sk_buff *skb, struct genl_info *info);
 int devlink_nl_cmd_selftests_get_doit(struct sk_buff *skb, struct genl_info *info);
 int devlink_nl_cmd_selftests_run(struct sk_buff *skb, struct genl_info *info);
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 8f42f1f45705..094cd0e8224e 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -6282,7 +6282,7 @@ const struct genl_small_ops devlink_nl_small_ops[56] = {
 	{
 		.cmd = DEVLINK_CMD_GET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
-		.doit = devlink_nl_cmd_get_doit,
+		.doit = devlink_nl_get_doit,
 		.dumpit = devlink_nl_instance_iter_dumpit,
 		/* can be retrieved by unprivileged users */
 	},
@@ -6536,7 +6536,7 @@ const struct genl_small_ops devlink_nl_small_ops[56] = {
 	{
 		.cmd = DEVLINK_CMD_INFO_GET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
-		.doit = devlink_nl_cmd_info_get_doit,
+		.doit = devlink_nl_info_get_doit,
 		.dumpit = devlink_nl_instance_iter_dumpit,
 		/* can be retrieved by unprivileged users */
 	},
-- 
2.41.0

[patch net-next 7/8] devlink: introduce couple of dumpit callback for split ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Introduce couple of dumpit callbacks for generated split ops. Have them
as a thin wrapper around iteration function and allow to pass dump_one()
function pointer directly without need to store in devlink_cmd structs.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 net/devlink/dev.c           | 23 +++++++++++++----------
 net/devlink/devl_internal.h | 14 ++++++++------
 net/devlink/leftover.c      |  4 ++--
 net/devlink/netlink.c       | 21 ++++++++++++---------
 4 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 2e120b3da220..1782157351ed 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -217,17 +217,19 @@ int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info)
 }
 
 static int
-devlink_nl_cmd_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
-			    struct netlink_callback *cb)
+devlink_nl_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
+			struct netlink_callback *cb)
 {
 	return devlink_nl_fill(msg, devlink, DEVLINK_CMD_NEW,
 			       NETLINK_CB(cb->skb).portid,
 			       cb->nlh->nlmsg_seq, NLM_F_MULTI);
 }
 
-const struct devlink_cmd devl_cmd_get = {
-	.dump_one		= devlink_nl_cmd_get_dump_one,
-};
+int devlink_nl_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb)
+{
+	return devlink_nl_dumpit(msg, cb, devlink_nl_get_dump_one);
+}
+
 
 static void devlink_reload_failed_set(struct devlink *devlink,
 				      bool reload_failed)
@@ -826,8 +828,8 @@ int devlink_nl_info_get_doit(struct sk_buff *skb, struct genl_info *info)
 }
 
 static int
-devlink_nl_cmd_info_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
-				 struct netlink_callback *cb)
+devlink_nl_info_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
+			     struct netlink_callback *cb)
 {
 	int err;
 
@@ -840,9 +842,10 @@ devlink_nl_cmd_info_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
 	return err;
 }
 
-const struct devlink_cmd devl_cmd_info_get = {
-	.dump_one		= devlink_nl_cmd_info_get_dump_one,
-};
+int devlink_nl_info_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb)
+{
+	return devlink_nl_dumpit(msg, cb, devlink_nl_info_get_dump_one);
+}
 
 static int devlink_nl_flash_update_fill(struct sk_buff *msg,
 					struct devlink *devlink,
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 419bc92da503..51de0e1fc769 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -116,9 +116,12 @@ struct devlink_nl_dump_state {
 	};
 };
 
+typedef int devlink_nl_dump_one_func_t(struct sk_buff *msg,
+				       struct devlink *devlink,
+				       struct netlink_callback *cb);
+
 struct devlink_cmd {
-	int (*dump_one)(struct sk_buff *msg, struct devlink *devlink,
-			struct netlink_callback *cb);
+	devlink_nl_dump_one_func_t *dump_one;
 };
 
 extern const struct genl_small_ops devlink_nl_small_ops[56];
@@ -129,8 +132,9 @@ devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs);
 void devlink_notify_unregister(struct devlink *devlink);
 void devlink_notify_register(struct devlink *devlink);
 
-int devlink_nl_instance_iter_dumpit(struct sk_buff *msg,
-				    struct netlink_callback *cb);
+int devlink_nl_dumpit(struct sk_buff *msg, struct netlink_callback *cb,
+		      devlink_nl_dump_one_func_t *dump_one);
+int devlink_nl_instance_iter_dumpit(struct sk_buff *msg, struct netlink_callback *cb);
 
 static inline struct devlink_nl_dump_state *
 devlink_dump_state(struct netlink_callback *cb)
@@ -151,7 +155,6 @@ devlink_nl_put_handle(struct sk_buff *msg, struct devlink *devlink)
 }
 
 /* Commands */
-extern const struct devlink_cmd devl_cmd_get;
 extern const struct devlink_cmd devl_cmd_port_get;
 extern const struct devlink_cmd devl_cmd_sb_get;
 extern const struct devlink_cmd devl_cmd_sb_pool_get;
@@ -159,7 +162,6 @@ extern const struct devlink_cmd devl_cmd_sb_port_pool_get;
 extern const struct devlink_cmd devl_cmd_sb_tc_pool_bind_get;
 extern const struct devlink_cmd devl_cmd_param_get;
 extern const struct devlink_cmd devl_cmd_region_get;
-extern const struct devlink_cmd devl_cmd_info_get;
 extern const struct devlink_cmd devl_cmd_health_reporter_get;
 extern const struct devlink_cmd devl_cmd_trap_get;
 extern const struct devlink_cmd devl_cmd_trap_group_get;
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 094cd0e8224e..895b732bed8e 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -6283,7 +6283,7 @@ const struct genl_small_ops devlink_nl_small_ops[56] = {
 		.cmd = DEVLINK_CMD_GET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = devlink_nl_get_doit,
-		.dumpit = devlink_nl_instance_iter_dumpit,
+		.dumpit = devlink_nl_get_dumpit,
 		/* can be retrieved by unprivileged users */
 	},
 	{
@@ -6537,7 +6537,7 @@ const struct genl_small_ops devlink_nl_small_ops[56] = {
 		.cmd = DEVLINK_CMD_INFO_GET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = devlink_nl_info_get_doit,
-		.dumpit = devlink_nl_instance_iter_dumpit,
+		.dumpit = devlink_nl_info_get_dumpit,
 		/* can be retrieved by unprivileged users */
 	},
 	{
diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c
index 39e07a5a69af..98d5c6b0acd8 100644
--- a/net/devlink/netlink.c
+++ b/net/devlink/netlink.c
@@ -178,7 +178,6 @@ void devlink_nl_post_doit(const struct genl_split_ops *ops,
 }
 
 static const struct devlink_cmd *devl_cmds[] = {
-	[DEVLINK_CMD_GET]		= &devl_cmd_get,
 	[DEVLINK_CMD_PORT_GET]		= &devl_cmd_port_get,
 	[DEVLINK_CMD_SB_GET]		= &devl_cmd_sb_get,
 	[DEVLINK_CMD_SB_POOL_GET]	= &devl_cmd_sb_pool_get,
@@ -186,7 +185,6 @@ static const struct devlink_cmd *devl_cmds[] = {
 	[DEVLINK_CMD_SB_TC_POOL_BIND_GET] = &devl_cmd_sb_tc_pool_bind_get,
 	[DEVLINK_CMD_PARAM_GET]		= &devl_cmd_param_get,
 	[DEVLINK_CMD_REGION_GET]	= &devl_cmd_region_get,
-	[DEVLINK_CMD_INFO_GET]		= &devl_cmd_info_get,
 	[DEVLINK_CMD_HEALTH_REPORTER_GET] = &devl_cmd_health_reporter_get,
 	[DEVLINK_CMD_TRAP_GET]		= &devl_cmd_trap_get,
 	[DEVLINK_CMD_TRAP_GROUP_GET]	= &devl_cmd_trap_group_get,
@@ -196,23 +194,19 @@ static const struct devlink_cmd *devl_cmds[] = {
 	[DEVLINK_CMD_SELFTESTS_GET]	= &devl_cmd_selftests_get,
 };
 
-int devlink_nl_instance_iter_dumpit(struct sk_buff *msg,
-				    struct netlink_callback *cb)
+int devlink_nl_dumpit(struct sk_buff *msg, struct netlink_callback *cb,
+		      devlink_nl_dump_one_func_t *dump_one)
 {
-	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
 	struct devlink_nl_dump_state *state = devlink_dump_state(cb);
-	const struct devlink_cmd *cmd;
 	struct devlink *devlink;
 	int err = 0;
 
-	cmd = devl_cmds[info->op.cmd];
-
 	while ((devlink = devlinks_xa_find_get(sock_net(msg->sk),
 					       &state->instance))) {
 		devl_lock(devlink);
 
 		if (devl_is_registered(devlink))
-			err = cmd->dump_one(msg, devlink, cb);
+			err = dump_one(msg, devlink, cb);
 		else
 			err = 0;
 
@@ -233,6 +227,15 @@ int devlink_nl_instance_iter_dumpit(struct sk_buff *msg,
 	return msg->len;
 }
 
+int devlink_nl_instance_iter_dumpit(struct sk_buff *msg,
+				    struct netlink_callback *cb)
+{
+	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
+	const struct devlink_cmd *cmd = devl_cmds[info->op.cmd];
+
+	return devlink_nl_dumpit(msg, cb, cmd->dump_one);
+}
+
 struct genl_family devlink_nl_family __ro_after_init = {
 	.name		= DEVLINK_GENL_NAME,
 	.version	= DEVLINK_GENL_VERSION,
-- 
2.41.0

[patch net-next 8/8] devlink: use generated split ops and remove duplicated commands from small ops

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Do the switch and use generated split ops for get and info_get commands.
Remove those from small ops array.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 net/devlink/devl_internal.h |  2 +-
 net/devlink/leftover.c      | 16 +---------------
 net/devlink/netlink.c       |  2 ++
 3 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 51de0e1fc769..7fdd956ff992 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -124,7 +124,7 @@ struct devlink_cmd {
 	devlink_nl_dump_one_func_t *dump_one;
 };
 
-extern const struct genl_small_ops devlink_nl_small_ops[56];
+extern const struct genl_small_ops devlink_nl_small_ops[54];
 
 struct devlink *
 devlink_get_from_attrs_lock(struct net *net, struct nlattr **attrs);
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 895b732bed8e..3bf42f5335ed 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -6278,14 +6278,7 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
 	return devlink_trap_policer_set(devlink, policer_item, info);
 }
 
-const struct genl_small_ops devlink_nl_small_ops[56] = {
-	{
-		.cmd = DEVLINK_CMD_GET,
-		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
-		.doit = devlink_nl_get_doit,
-		.dumpit = devlink_nl_get_dumpit,
-		/* can be retrieved by unprivileged users */
-	},
+const struct genl_small_ops devlink_nl_small_ops[54] = {
 	{
 		.cmd = DEVLINK_CMD_PORT_GET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
@@ -6533,13 +6526,6 @@ const struct genl_small_ops devlink_nl_small_ops[56] = {
 		.dumpit = devlink_nl_cmd_region_read_dumpit,
 		.flags = GENL_ADMIN_PERM,
 	},
-	{
-		.cmd = DEVLINK_CMD_INFO_GET,
-		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
-		.doit = devlink_nl_info_get_doit,
-		.dumpit = devlink_nl_info_get_dumpit,
-		/* can be retrieved by unprivileged users */
-	},
 	{
 		.cmd = DEVLINK_CMD_HEALTH_REPORTER_GET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c
index 98d5c6b0acd8..bada2819827b 100644
--- a/net/devlink/netlink.c
+++ b/net/devlink/netlink.c
@@ -248,6 +248,8 @@ struct genl_family devlink_nl_family __ro_after_init = {
 	.module		= THIS_MODULE,
 	.small_ops	= devlink_nl_small_ops,
 	.n_small_ops	= ARRAY_SIZE(devlink_nl_small_ops),
+	.split_ops	= devlink_nl_ops,
+	.n_split_ops	= ARRAY_SIZE(devlink_nl_ops),
 	.resv_start_op	= DEVLINK_CMD_SELFTESTS_RUN + 1,
 	.mcgrps		= devlink_nl_mcgrps,
 	.n_mcgrps	= ARRAY_SIZE(devlink_nl_mcgrps),
-- 
2.41.0

Re: [patch net-next 1/8] ynl-gen-c.py: fix rendering of validate field

[Jakub Kicinski]

On Tue,  1 Aug 2023 16:19:00 +0200 Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> For split ops, do and dump has different value in validate field. Fix
> the rendering so for do op, only "strict" is filled out and for dump op,
> "strict" is prefixed by "dump-".
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  tools/net/ynl/ynl-gen-c.py | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
> index 650be9b8b693..1c36d0c935da 100755
> --- a/tools/net/ynl/ynl-gen-c.py
> +++ b/tools/net/ynl/ynl-gen-c.py
> @@ -1988,9 +1988,17 @@ def print_kernel_op_table(family, cw):
>                  cw.block_start()
>                  members = [('cmd', op.enum_name)]
>                  if 'dont-validate' in op:
> +                    dont_validate = []
> +                    for x in op['dont-validate']:
> +                        if op_mode == 'do' and x == 'dump':
> +                            continue
> +                        if op_mode == "dump" and x == 'strict':
> +                            x = 'dump-' + x
> +                        dont_validate.append(x)
> +
>                      members.append(('validate',
>                                      ' | '.join([c_upper('genl-dont-validate-' + x)
> -                                                for x in op['dont-validate']])), )
> +                                                for x in dont_validate])), )
>                  name = c_lower(f"{family.name}-nl-{op_name}-{op_mode}it")
>                  if 'pre' in op[op_mode]:
>                      members.append((cb_names[op_mode]['pre'], c_lower(op[op_mode]['pre'])))

I was hoping we can delete GENL_DONT_VALIDATE_DUMP_STRICT
but there is one cmd (TIPC_NL_LINK_GET) which
sets GENL_DONT_VALIDATE_STRICT and nothing about the dump.

To express something like that we should add dump-strict as
an allowed flag explicitly rather than doing the auto-prepending
-- 
pw-bot: cr

Re: [patch net-next 2/8] ynl-gen-c.py: allow directional model for kernel mode

[Jakub Kicinski]

On Tue,  1 Aug 2023 16:19:01 +0200 Jiri Pirko wrote:
> Directional model is only considered in uapi mode. No reason to forbid
> directional model for kernel mode, so lift the limitation.

I mean, the reason is that it's not tested so this way
I don't get people sending "fixes" claiming stuff that's
not supported is broken :)

Re: [patch net-next 4/8] devlink: add split ops generated according to spec

[Jakub Kicinski]

On Tue,  1 Aug 2023 16:19:03 +0200 Jiri Pirko wrote:
> Improve the existing devlink spec in order to serve as a source fot

s/fot/for/

> generation of valid devlink split ops for the existing commands.
> Add the generated sources.

> +/* DEVLINK_CMD_GET - do */
> +const struct nla_policy devlink_get_nl_policy[DEVLINK_ATTR_DEV_NAME + 1] = {
> +	[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
> +	[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
> +};

What's the impact of narrowing down the policies? Could you describe it
in the commit message?

Re: [patch net-next 1/8] ynl-gen-c.py: fix rendering of validate field

[Jiri Pirko]

Tue, Aug 01, 2023 at 08:25:30PM CEST, kuba@kernel.org wrote:
>On Tue,  1 Aug 2023 16:19:00 +0200 Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> For split ops, do and dump has different value in validate field. Fix
>> the rendering so for do op, only "strict" is filled out and for dump op,
>> "strict" is prefixed by "dump-".
>> 
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>>  tools/net/ynl/ynl-gen-c.py | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
>> index 650be9b8b693..1c36d0c935da 100755
>> --- a/tools/net/ynl/ynl-gen-c.py
>> +++ b/tools/net/ynl/ynl-gen-c.py
>> @@ -1988,9 +1988,17 @@ def print_kernel_op_table(family, cw):
>>                  cw.block_start()
>>                  members = [('cmd', op.enum_name)]
>>                  if 'dont-validate' in op:
>> +                    dont_validate = []
>> +                    for x in op['dont-validate']:
>> +                        if op_mode == 'do' and x == 'dump':
>> +                            continue
>> +                        if op_mode == "dump" and x == 'strict':
>> +                            x = 'dump-' + x
>> +                        dont_validate.append(x)
>> +
>>                      members.append(('validate',
>>                                      ' | '.join([c_upper('genl-dont-validate-' + x)
>> -                                                for x in op['dont-validate']])), )
>> +                                                for x in dont_validate])), )
>>                  name = c_lower(f"{family.name}-nl-{op_name}-{op_mode}it")
>>                  if 'pre' in op[op_mode]:
>>                      members.append((cb_names[op_mode]['pre'], c_lower(op[op_mode]['pre'])))
>
>I was hoping we can delete GENL_DONT_VALIDATE_DUMP_STRICT
>but there is one cmd (TIPC_NL_LINK_GET) which
>sets GENL_DONT_VALIDATE_STRICT and nothing about the dump.

I need GENL_DONT_VALIDATE_STRICT for devlink dump selectors as well. I
don't want to break existing user that may pass garbage attributes.

>
>To express something like that we should add dump-strict as
>an allowed flag explicitly rather than doing the auto-prepending

Yeah, that was an option. But strict means GENL_DONT_VALIDATE_STRICT
for do and GENL_DONT_VALIDATE_DUMP_STRICT for dump. That is why I
decided to go this way.

I will add dump-strict if you prefer it, I don't care.

>-- 
>pw-bot: cr

Re: [patch net-next 2/8] ynl-gen-c.py: allow directional model for kernel mode

[Jiri Pirko]

Tue, Aug 01, 2023 at 08:27:03PM CEST, kuba@kernel.org wrote:
>On Tue,  1 Aug 2023 16:19:01 +0200 Jiri Pirko wrote:
>> Directional model is only considered in uapi mode. No reason to forbid
>> directional model for kernel mode, so lift the limitation.
>
>I mean, the reason is that it's not tested so this way
>I don't get people sending "fixes" claiming stuff that's
>not supported is broken :)

Got it, but I need it for devlink. Works fine as far as I can tell.

Re: [patch net-next 4/8] devlink: add split ops generated according to spec

[Jiri Pirko]

Tue, Aug 01, 2023 at 08:56:55PM CEST, kuba@kernel.org wrote:
>On Tue,  1 Aug 2023 16:19:03 +0200 Jiri Pirko wrote:
>> Improve the existing devlink spec in order to serve as a source fot
>
>s/fot/for/
>
>> generation of valid devlink split ops for the existing commands.
>> Add the generated sources.
>
>> +/* DEVLINK_CMD_GET - do */
>> +const struct nla_policy devlink_get_nl_policy[DEVLINK_ATTR_DEV_NAME + 1] = {
>> +	[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
>> +	[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
>> +};
>
>What's the impact of narrowing down the policies? Could you describe it
>in the commit message?

Should be no impact afaik. The code does not care about the rest of the
attributes and dont-validate-strict will allow any garbage to be passed
by the user. I will put a note in the commit message as you asked.

Re: [patch net-next 2/8] ynl-gen-c.py: allow directional model for kernel mode

[Jiri Pirko]

Tue, Aug 01, 2023 at 08:27:03PM CEST, kuba@kernel.org wrote:
>On Tue,  1 Aug 2023 16:19:01 +0200 Jiri Pirko wrote:
>> Directional model is only considered in uapi mode. No reason to forbid
>> directional model for kernel mode, so lift the limitation.
>
>I mean, the reason is that it's not tested so this way
>I don't get people sending "fixes" claiming stuff that's
>not supported is broken :)

I re-phrased the patch description accordingly, hopefully to your
satisfaction.