[Draft PATCH net-next 0/3] add YAML spec for team

[Draft PATCH net-next 0/3] add YAML spec for team

[Hangbin Liu]

Hi Jakub,

You suggested me to add yaml spec for bridge. Since I'm not familiar with
writing the spec file, I choose to convert team as a start.

There are still some questions I got during convertion.

1. Is there a preference to use "-" instead of "_" for the names in spec file?
   e.g. the attr-cnt-name in team.spec, should I use __team-attr-item-port-max
   or --team-attr-item-port-max, or __team_attr_item_port_max?
2. I saw ynl-gen-c.py deals with unterminated-ok. But this policy is not shown
   in the schemas. Is it a new feature that still working on?
3. Do we have to hard code the string max-len? Is there a way to use
   the name in definitions? e.g.
   name: name
   type: string
   checks:
     max-len: string-max-len
4. The doc will be generate to rst file in future, so there will not have
   other comments in the _nl.c or _nl.h files, right?
5. the genl_multicast_group is forced to use list. But the team use format
   like { .name = TEAM_GENL_CHANGE_EVENT_MC_GRP_NAME, }. Should we support
   this legacy format?
6. The _UAPI_LINUX_IF_TEAM_H_ is rename to _UAPI_LINUX_IF_TEAM_H in uapi
   header. Does that affects?
7. When build, I got error modpost: missing MODULE_LICENSE() in drivers/net/team/team_nl.o.
   Should we add the MODULE_LICENSE support in ynl-gen-c.py?
8. When build, I also got errors like
     ERROR: modpost: "team_nl_policy" [drivers/net/team/team.ko] undefined!
     ERROR: modpost: "team_nl_ops" [drivers/net/team/team.ko] undefined!
     ERROR: modpost: "team_nl_noop_doit" [drivers/net/team/team_nl.ko] undefined!
     ERROR: modpost: "team_nl_options_set_doit" [drivers/net/team/team_nl.ko] undefined!
     ERROR: modpost: "team_nl_options_get_doit" [drivers/net/team/team_nl.ko] undefined!
     ERROR: modpost: "team_nl_port_list_get_doit" [drivers/net/team/team_nl.ko] undefined!
     ERROR: modpost: "team_attr_option_nl_policy" [drivers/net/team/team.ko] undefined!
  Do you know why include "team_nl.h" doesn't help?

Thanks
Hangbin

Hangbin Liu (3):
  Documentation: netlink: add a YAML spec for team
  net: team: use policy generated by YAML spec
  uapi: team: use header file generated from YAML spec

 Documentation/netlink/specs/team.yaml | 205 ++++++++++++++++++++++++++
 MAINTAINERS                           |   1 +
 drivers/net/team/Makefile             |   2 +-
 drivers/net/team/team.c               |  59 +-------
 drivers/net/team/team_nl.c            |  59 ++++++++
 drivers/net/team/team_nl.h            |  29 ++++
 include/uapi/linux/if_team.h          | 116 ++++++---------
 7 files changed, 345 insertions(+), 126 deletions(-)
 create mode 100644 Documentation/netlink/specs/team.yaml
 create mode 100644 drivers/net/team/team_nl.c
 create mode 100644 drivers/net/team/team_nl.h

-- 
2.43.0

[Draft PATCH net-next 1/3] Documentation: netlink: add a YAML spec for team

[Hangbin Liu]

Add a YAML specification for team.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 Documentation/netlink/specs/team.yaml | 205 ++++++++++++++++++++++++++
 MAINTAINERS                           |   1 +
 2 files changed, 206 insertions(+)
 create mode 100644 Documentation/netlink/specs/team.yaml

diff --git a/Documentation/netlink/specs/team.yaml b/Documentation/netlink/specs/team.yaml
new file mode 100644
index 000000000000..5647068bf521
--- /dev/null
+++ b/Documentation/netlink/specs/team.yaml
@@ -0,0 +1,205 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: team
+
+protocol: genetlink-legacy
+
+doc: |
+  Network team device driver.
+
+c-family-name: team-genl-name
+c-version-name: team-genl-version
+kernel-policy: global
+uapi-header: linux/if_team.h
+
+definitions:
+  -
+    name: string-max-len
+    type: const
+    value: 32
+  -
+    name: genl-change-event-mc-grp-name
+    type: const
+    value: change_event
+
+attribute-sets:
+  -
+    name: team
+    doc:
+      The team nested layout of get/set msg looks like
+          [TEAM_ATTR_LIST_OPTION]
+              [TEAM_ATTR_ITEM_OPTION]
+                  [TEAM_ATTR_OPTION_*], ...
+              [TEAM_ATTR_ITEM_OPTION]
+                  [TEAM_ATTR_OPTION_*], ...
+              ...
+          [TEAM_ATTR_LIST_PORT]
+              [TEAM_ATTR_ITEM_PORT]
+                  [TEAM_ATTR_PORT_*], ...
+              [TEAM_ATTR_ITEM_PORT]
+                  [TEAM_ATTR_PORT_*], ...
+              ...
+    name-prefix: team-attr-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: team-ifindex
+        type: u32
+      -
+        name: list-option
+        type: nest
+        nested-attributes: item-option
+      -
+        name: list-port
+        type: nest
+        nested-attributes: item-port
+  -
+    name: item-option
+    name-prefix: team-attr-item-
+    attr-cnt-name: __team-attr-item-option-max
+    attr-max-name: team-attr-item-option-max
+    attributes:
+      -
+        name: option-unspec
+        type: unused
+        value: 0
+      -
+        name: option
+        type: nest
+        nested-attributes: attr-option
+  -
+    name: attr-option
+    name-prefix: team-attr-option-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: name
+        type: string
+        checks:
+          # no unterminated-ok defination?
+          # do we have to hard code this?
+          max-len: 32
+      -
+        name: changed
+        type: flag
+      -
+        name: type
+        type: u8
+      -
+        name: data
+        type: binary
+      -
+        name: removed
+        type: flag
+      -
+        name: port-ifindex
+        type: u32
+        doc: for per-port options
+      -
+        name: array-index
+        type: u32
+        doc: for for array options
+  -
+    name: item-port
+    name-prefix: team-attr-item-
+    attr-cnt-name: __team-attr-item-port-max
+    attr-max-name: team-attr-item-port-max
+    attributes:
+      -
+        name: port-unspec
+        type: unused
+        value: 0
+      -
+        name: port
+        type: nest
+        nested-attributes: attr-port
+  -
+    name: attr-port
+    name-prefix: team-attr-port-
+    attributes:
+      -
+        name: unspec
+        type: unused
+        value: 0
+      -
+        name: ifindex
+        type: u32
+      -
+        name: changed
+        type: flag
+      -
+        name: linkup
+        type: flag
+      -
+        name: speed
+        type: u32
+      -
+        name: duplex
+        type: u8
+      -
+        name: removed
+        type: flag
+
+operations:
+  list:
+    -
+      name: noop
+      doc: No operation
+      value: 0
+      attribute-set: team
+      dont-validate: [ strict, dump ]
+
+      do:
+        # Actually it only reply the team netlink family
+        reply:
+          attributes:
+            - team-ifindex
+
+    -
+      name: options-set
+      doc: Set team options
+      attribute-set: team
+      dont-validate: [ strict, dump ]
+      flags: [ admin-perm ]
+
+      do:
+        request:
+          attributes:
+            - team-ifindex
+            - list-option
+
+    -
+      name: options-get
+      doc: Get team options info
+      attribute-set: team
+      dont-validate: [ strict, dump ]
+      flags: [ admin-perm ]
+
+      do:
+        request:
+          attributes:
+            - team-ifindex
+        reply:
+          attributes:
+            - list-option
+
+    -
+      name: port-list-get
+      doc: Get team ports info
+      attribute-set: team
+      dont-validate: [ strict, dump ]
+      flags: [ admin-perm ]
+
+      do:
+        request:
+          attributes:
+            - team-ifindex
+        reply:
+          attributes:
+            - list-port
diff --git a/MAINTAINERS b/MAINTAINERS
index 7fb66210069b..b64e449f47f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21301,6 +21301,7 @@ F:	drivers/net/team/
 F:	include/linux/if_team.h
 F:	include/uapi/linux/if_team.h
 F:	tools/testing/selftests/drivers/net/team/
+F:	Documentation/netlink/specs/team.yaml
 
 TECHNICAL ADVISORY BOARD PROCESS DOCS
 M:	"Theodore Ts'o" <tytso@mit.edu>
-- 
2.43.0

[Draft PATCH net-next 2/3] net: team: use policy generated by YAML spec

[Hangbin Liu]

generated with:

 $ ./tools/net/ynl/ynl-gen-c.py --mode kernel \
 > --spec Documentation/netlink/specs/team.yaml --source \
 > -o drivers/net/team/team_nl.c
 $ ./tools/net/ynl/ynl-gen-c.py --mode kernel \
 > --spec Documentation/netlink/specs/team.yaml --header \
 > -o drivers/net/team/team_nl.h

The TEAM_ATTR_LIST_PORT in team_nl_policy is removed as it only in the
port list reply attributes.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 drivers/net/team/Makefile  |  2 +-
 drivers/net/team/team.c    | 59 +++++---------------------------------
 drivers/net/team/team_nl.c | 59 ++++++++++++++++++++++++++++++++++++++
 drivers/net/team/team_nl.h | 29 +++++++++++++++++++
 4 files changed, 96 insertions(+), 53 deletions(-)
 create mode 100644 drivers/net/team/team_nl.c
 create mode 100644 drivers/net/team/team_nl.h

diff --git a/drivers/net/team/Makefile b/drivers/net/team/Makefile
index f582d81a5091..43ee154db26e 100644
--- a/drivers/net/team/Makefile
+++ b/drivers/net/team/Makefile
@@ -3,7 +3,7 @@
 # Makefile for the network team driver
 #
 
-obj-$(CONFIG_NET_TEAM) += team.o
+obj-$(CONFIG_NET_TEAM) += team.o team_nl.o
 obj-$(CONFIG_NET_TEAM_MODE_BROADCAST) += team_mode_broadcast.o
 obj-$(CONFIG_NET_TEAM_MODE_ROUNDROBIN) += team_mode_roundrobin.o
 obj-$(CONFIG_NET_TEAM_MODE_RANDOM) += team_mode_random.o
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 508d9a392ab1..90e73665978a 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -28,6 +28,8 @@
 #include <generated/utsrelease.h>
 #include <linux/if_team.h>
 
+#include "team_nl.h"
+
 #define DRV_NAME "team"
 
 
@@ -2254,28 +2256,7 @@ static struct rtnl_link_ops team_link_ops __read_mostly = {
 
 static struct genl_family team_nl_family;
 
-static const struct nla_policy team_nl_policy[TEAM_ATTR_MAX + 1] = {
-	[TEAM_ATTR_UNSPEC]			= { .type = NLA_UNSPEC, },
-	[TEAM_ATTR_TEAM_IFINDEX]		= { .type = NLA_U32 },
-	[TEAM_ATTR_LIST_OPTION]			= { .type = NLA_NESTED },
-	[TEAM_ATTR_LIST_PORT]			= { .type = NLA_NESTED },
-};
-
-static const struct nla_policy
-team_nl_option_policy[TEAM_ATTR_OPTION_MAX + 1] = {
-	[TEAM_ATTR_OPTION_UNSPEC]		= { .type = NLA_UNSPEC, },
-	[TEAM_ATTR_OPTION_NAME] = {
-		.type = NLA_STRING,
-		.len = TEAM_STRING_MAX_LEN,
-	},
-	[TEAM_ATTR_OPTION_CHANGED]		= { .type = NLA_FLAG },
-	[TEAM_ATTR_OPTION_TYPE]			= { .type = NLA_U8 },
-	[TEAM_ATTR_OPTION_DATA]			= { .type = NLA_BINARY },
-	[TEAM_ATTR_OPTION_PORT_IFINDEX]		= { .type = NLA_U32 },
-	[TEAM_ATTR_OPTION_ARRAY_INDEX]		= { .type = NLA_U32 },
-};
-
-static int team_nl_cmd_noop(struct sk_buff *skb, struct genl_info *info)
+int team_nl_noop_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct sk_buff *msg;
 	void *hdr;
@@ -2513,7 +2494,7 @@ static int team_nl_send_options_get(struct team *team, u32 portid, u32 seq,
 	return err;
 }
 
-static int team_nl_cmd_options_get(struct sk_buff *skb, struct genl_info *info)
+int team_nl_options_get_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct team *team;
 	struct team_option_inst *opt_inst;
@@ -2538,7 +2519,7 @@ static int team_nl_cmd_options_get(struct sk_buff *skb, struct genl_info *info)
 static int team_nl_send_event_options_get(struct team *team,
 					  struct list_head *sel_opt_inst_list);
 
-static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
+int team_nl_options_set_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct team *team;
 	int err = 0;
@@ -2579,7 +2560,7 @@ static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
 		err = nla_parse_nested_deprecated(opt_attrs,
 						  TEAM_ATTR_OPTION_MAX,
 						  nl_option,
-						  team_nl_option_policy,
+						  team_attr_option_nl_policy,
 						  info->extack);
 		if (err)
 			goto team_put;
@@ -2802,7 +2783,7 @@ static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
 	return err;
 }
 
-static int team_nl_cmd_port_list_get(struct sk_buff *skb,
+int team_nl_port_list_get_doit(struct sk_buff *skb,
 				     struct genl_info *info)
 {
 	struct team *team;
@@ -2820,32 +2801,6 @@ static int team_nl_cmd_port_list_get(struct sk_buff *skb,
 	return err;
 }
 
-static const struct genl_small_ops team_nl_ops[] = {
-	{
-		.cmd = TEAM_CMD_NOOP,
-		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
-		.doit = team_nl_cmd_noop,
-	},
-	{
-		.cmd = TEAM_CMD_OPTIONS_SET,
-		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
-		.doit = team_nl_cmd_options_set,
-		.flags = GENL_ADMIN_PERM,
-	},
-	{
-		.cmd = TEAM_CMD_OPTIONS_GET,
-		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
-		.doit = team_nl_cmd_options_get,
-		.flags = GENL_ADMIN_PERM,
-	},
-	{
-		.cmd = TEAM_CMD_PORT_LIST_GET,
-		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
-		.doit = team_nl_cmd_port_list_get,
-		.flags = GENL_ADMIN_PERM,
-	},
-};
-
 static const struct genl_multicast_group team_nl_mcgrps[] = {
 	{ .name = TEAM_GENL_CHANGE_EVENT_MC_GRP_NAME, },
 };
diff --git a/drivers/net/team/team_nl.c b/drivers/net/team/team_nl.c
new file mode 100644
index 000000000000..c57bb3d4d7d5
--- /dev/null
+++ b/drivers/net/team/team_nl.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/team.yaml */
+/* YNL-GEN kernel source */
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "team_nl.h"
+
+#include <uapi/linux/if_team.h>
+
+/* Common nested types */
+const struct nla_policy team_attr_option_nl_policy[TEAM_ATTR_OPTION_ARRAY_INDEX + 1] = {
+	[TEAM_ATTR_OPTION_NAME] = { .type = NLA_NUL_STRING, .len = 32, },
+	[TEAM_ATTR_OPTION_CHANGED] = { .type = NLA_FLAG, },
+	[TEAM_ATTR_OPTION_TYPE] = { .type = NLA_U8, },
+	[TEAM_ATTR_OPTION_DATA] = { .type = NLA_BINARY, },
+	[TEAM_ATTR_OPTION_REMOVED] = { .type = NLA_FLAG, },
+	[TEAM_ATTR_OPTION_PORT_IFINDEX] = { .type = NLA_U32, },
+	[TEAM_ATTR_OPTION_ARRAY_INDEX] = { .type = NLA_U32, },
+};
+
+const struct nla_policy team_item_option_nl_policy[TEAM_ATTR_ITEM_OPTION + 1] = {
+	[TEAM_ATTR_ITEM_OPTION] = NLA_POLICY_NESTED(team_attr_option_nl_policy),
+};
+
+/* Global operation policy for team */
+const struct nla_policy team_nl_policy[TEAM_ATTR_LIST_OPTION + 1] = {
+	[TEAM_ATTR_TEAM_IFINDEX] = { .type = NLA_U32, },
+	[TEAM_ATTR_LIST_OPTION] = NLA_POLICY_NESTED(team_item_option_nl_policy),
+};
+
+/* Ops table for team */
+const struct genl_small_ops team_nl_ops[4] = {
+	{
+		.cmd		= TEAM_CMD_NOOP,
+		.validate	= GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit		= team_nl_noop_doit,
+	},
+	{
+		.cmd		= TEAM_CMD_OPTIONS_SET,
+		.validate	= GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit		= team_nl_options_set_doit,
+		.flags		= GENL_ADMIN_PERM,
+	},
+	{
+		.cmd		= TEAM_CMD_OPTIONS_GET,
+		.validate	= GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit		= team_nl_options_get_doit,
+		.flags		= GENL_ADMIN_PERM,
+	},
+	{
+		.cmd		= TEAM_CMD_PORT_LIST_GET,
+		.validate	= GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit		= team_nl_port_list_get_doit,
+		.flags		= GENL_ADMIN_PERM,
+	},
+};
diff --git a/drivers/net/team/team_nl.h b/drivers/net/team/team_nl.h
new file mode 100644
index 000000000000..c9ec1b22ac4d
--- /dev/null
+++ b/drivers/net/team/team_nl.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/team.yaml */
+/* YNL-GEN kernel header */
+
+#ifndef _LINUX_TEAM_GEN_H
+#define _LINUX_TEAM_GEN_H
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include <uapi/linux/if_team.h>
+
+/* Common nested types */
+extern const struct nla_policy team_attr_option_nl_policy[TEAM_ATTR_OPTION_ARRAY_INDEX + 1];
+extern const struct nla_policy team_item_option_nl_policy[TEAM_ATTR_ITEM_OPTION + 1];
+
+/* Global operation policy for team */
+extern const struct nla_policy team_nl_policy[TEAM_ATTR_LIST_OPTION + 1];
+
+/* Ops table for team */
+extern const struct genl_small_ops team_nl_ops[4];
+
+int team_nl_noop_doit(struct sk_buff *skb, struct genl_info *info);
+int team_nl_options_set_doit(struct sk_buff *skb, struct genl_info *info);
+int team_nl_options_get_doit(struct sk_buff *skb, struct genl_info *info);
+int team_nl_port_list_get_doit(struct sk_buff *skb, struct genl_info *info);
+
+#endif /* _LINUX_TEAM_GEN_H */
-- 
2.43.0

[Draft PATCH net-next 3/3] uapi: team: use header file generated from YAML spec

[Hangbin Liu]

generated with:

 $ ./tools/net/ynl/ynl-gen-c.py --mode uapi \
 > --spec Documentation/netlink/specs/team.yaml \
 > --header -o include/uapi/linux/if_team.h

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 include/uapi/linux/if_team.h | 116 +++++++++++++----------------------
 1 file changed, 43 insertions(+), 73 deletions(-)

diff --git a/include/uapi/linux/if_team.h b/include/uapi/linux/if_team.h
index 13c61fecb78b..a5c06243a435 100644
--- a/include/uapi/linux/if_team.h
+++ b/include/uapi/linux/if_team.h
@@ -1,108 +1,78 @@
-/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
-/*
- * include/linux/if_team.h - Network team device driver header
- * Copyright (c) 2011 Jiri Pirko <jpirko@redhat.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/team.yaml */
+/* YNL-GEN uapi header */
 
-#ifndef _UAPI_LINUX_IF_TEAM_H_
-#define _UAPI_LINUX_IF_TEAM_H_
+#ifndef _UAPI_LINUX_IF_TEAM_H
+#define _UAPI_LINUX_IF_TEAM_H
 
+#define TEAM_GENL_NAME		"team"
+#define TEAM_GENL_VERSION	1
 
-#define TEAM_STRING_MAX_LEN 32
-
-/**********************************
- * NETLINK_GENERIC netlink family.
- **********************************/
-
-enum {
-	TEAM_CMD_NOOP,
-	TEAM_CMD_OPTIONS_SET,
-	TEAM_CMD_OPTIONS_GET,
-	TEAM_CMD_PORT_LIST_GET,
-
-	__TEAM_CMD_MAX,
-	TEAM_CMD_MAX = (__TEAM_CMD_MAX - 1),
-};
+#define TEAM_STRING_MAX_LEN			32
+#define TEAM_GENL_CHANGE_EVENT_MC_GRP_NAME	"change_event"
 
 enum {
 	TEAM_ATTR_UNSPEC,
-	TEAM_ATTR_TEAM_IFINDEX,		/* u32 */
-	TEAM_ATTR_LIST_OPTION,		/* nest */
-	TEAM_ATTR_LIST_PORT,		/* nest */
+	TEAM_ATTR_TEAM_IFINDEX,
+	TEAM_ATTR_LIST_OPTION,
+	TEAM_ATTR_LIST_PORT,
 
 	__TEAM_ATTR_MAX,
-	TEAM_ATTR_MAX = __TEAM_ATTR_MAX - 1,
+	TEAM_ATTR_MAX = (__TEAM_ATTR_MAX - 1)
 };
 
-/* Nested layout of get/set msg:
- *
- *	[TEAM_ATTR_LIST_OPTION]
- *		[TEAM_ATTR_ITEM_OPTION]
- *			[TEAM_ATTR_OPTION_*], ...
- *		[TEAM_ATTR_ITEM_OPTION]
- *			[TEAM_ATTR_OPTION_*], ...
- *		...
- *	[TEAM_ATTR_LIST_PORT]
- *		[TEAM_ATTR_ITEM_PORT]
- *			[TEAM_ATTR_PORT_*], ...
- *		[TEAM_ATTR_ITEM_PORT]
- *			[TEAM_ATTR_PORT_*], ...
- *		...
- */
-
 enum {
 	TEAM_ATTR_ITEM_OPTION_UNSPEC,
-	TEAM_ATTR_ITEM_OPTION,		/* nest */
+	TEAM_ATTR_ITEM_OPTION,
 
 	__TEAM_ATTR_ITEM_OPTION_MAX,
-	TEAM_ATTR_ITEM_OPTION_MAX = __TEAM_ATTR_ITEM_OPTION_MAX - 1,
+	TEAM_ATTR_ITEM_OPTION_MAX = (__TEAM_ATTR_ITEM_OPTION_MAX - 1)
 };
 
 enum {
 	TEAM_ATTR_OPTION_UNSPEC,
-	TEAM_ATTR_OPTION_NAME,		/* string */
-	TEAM_ATTR_OPTION_CHANGED,	/* flag */
-	TEAM_ATTR_OPTION_TYPE,		/* u8 */
-	TEAM_ATTR_OPTION_DATA,		/* dynamic */
-	TEAM_ATTR_OPTION_REMOVED,	/* flag */
-	TEAM_ATTR_OPTION_PORT_IFINDEX,	/* u32 */ /* for per-port options */
-	TEAM_ATTR_OPTION_ARRAY_INDEX,	/* u32 */ /* for array options */
+	TEAM_ATTR_OPTION_NAME,
+	TEAM_ATTR_OPTION_CHANGED,
+	TEAM_ATTR_OPTION_TYPE,
+	TEAM_ATTR_OPTION_DATA,
+	TEAM_ATTR_OPTION_REMOVED,
+	TEAM_ATTR_OPTION_PORT_IFINDEX,
+	TEAM_ATTR_OPTION_ARRAY_INDEX,
 
 	__TEAM_ATTR_OPTION_MAX,
-	TEAM_ATTR_OPTION_MAX = __TEAM_ATTR_OPTION_MAX - 1,
+	TEAM_ATTR_OPTION_MAX = (__TEAM_ATTR_OPTION_MAX - 1)
 };
 
 enum {
 	TEAM_ATTR_ITEM_PORT_UNSPEC,
-	TEAM_ATTR_ITEM_PORT,		/* nest */
+	TEAM_ATTR_ITEM_PORT,
 
 	__TEAM_ATTR_ITEM_PORT_MAX,
-	TEAM_ATTR_ITEM_PORT_MAX = __TEAM_ATTR_ITEM_PORT_MAX - 1,
+	TEAM_ATTR_ITEM_PORT_MAX = (__TEAM_ATTR_ITEM_PORT_MAX - 1)
 };
 
 enum {
 	TEAM_ATTR_PORT_UNSPEC,
-	TEAM_ATTR_PORT_IFINDEX,		/* u32 */
-	TEAM_ATTR_PORT_CHANGED,		/* flag */
-	TEAM_ATTR_PORT_LINKUP,		/* flag */
-	TEAM_ATTR_PORT_SPEED,		/* u32 */
-	TEAM_ATTR_PORT_DUPLEX,		/* u8 */
-	TEAM_ATTR_PORT_REMOVED,		/* flag */
+	TEAM_ATTR_PORT_IFINDEX,
+	TEAM_ATTR_PORT_CHANGED,
+	TEAM_ATTR_PORT_LINKUP,
+	TEAM_ATTR_PORT_SPEED,
+	TEAM_ATTR_PORT_DUPLEX,
+	TEAM_ATTR_PORT_REMOVED,
 
 	__TEAM_ATTR_PORT_MAX,
-	TEAM_ATTR_PORT_MAX = __TEAM_ATTR_PORT_MAX - 1,
+	TEAM_ATTR_PORT_MAX = (__TEAM_ATTR_PORT_MAX - 1)
 };
 
-/*
- * NETLINK_GENERIC related info
- */
-#define TEAM_GENL_NAME "team"
-#define TEAM_GENL_VERSION 0x1
-#define TEAM_GENL_CHANGE_EVENT_MC_GRP_NAME "change_event"
+enum {
+	TEAM_CMD_NOOP,
+	TEAM_CMD_OPTIONS_SET,
+	TEAM_CMD_OPTIONS_GET,
+	TEAM_CMD_PORT_LIST_GET,
+
+	__TEAM_CMD_MAX,
+	TEAM_CMD_MAX = (__TEAM_CMD_MAX - 1)
+};
 
-#endif /* _UAPI_LINUX_IF_TEAM_H_ */
+#endif /* _UAPI_LINUX_IF_TEAM_H */
-- 
2.43.0

Re: [Draft PATCH net-next 1/3] Documentation: netlink: add a YAML spec for team

[Jiri Pirko]

Wed, Dec 13, 2023 at 09:45:00AM CET, liuhangbin@gmail.com wrote:
>Add a YAML specification for team.
>
>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>---
> Documentation/netlink/specs/team.yaml | 205 ++++++++++++++++++++++++++
> MAINTAINERS                           |   1 +
> 2 files changed, 206 insertions(+)
> create mode 100644 Documentation/netlink/specs/team.yaml
>
>diff --git a/Documentation/netlink/specs/team.yaml b/Documentation/netlink/specs/team.yaml
>new file mode 100644
>index 000000000000..5647068bf521
>--- /dev/null
>+++ b/Documentation/netlink/specs/team.yaml
>@@ -0,0 +1,205 @@
>+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
>+
>+name: team
>+
>+protocol: genetlink-legacy
>+
>+doc: |
>+  Network team device driver.
>+
>+c-family-name: team-genl-name
>+c-version-name: team-genl-version
>+kernel-policy: global
>+uapi-header: linux/if_team.h
>+
>+definitions:
>+  -
>+    name: string-max-len
>+    type: const
>+    value: 32
>+  -
>+    name: genl-change-event-mc-grp-name
>+    type: const
>+    value: change_event
>+
>+attribute-sets:
>+  -
>+    name: team
>+    doc:
>+      The team nested layout of get/set msg looks like
>+          [TEAM_ATTR_LIST_OPTION]
>+              [TEAM_ATTR_ITEM_OPTION]
>+                  [TEAM_ATTR_OPTION_*], ...
>+              [TEAM_ATTR_ITEM_OPTION]
>+                  [TEAM_ATTR_OPTION_*], ...
>+              ...
>+          [TEAM_ATTR_LIST_PORT]
>+              [TEAM_ATTR_ITEM_PORT]
>+                  [TEAM_ATTR_PORT_*], ...
>+              [TEAM_ATTR_ITEM_PORT]
>+                  [TEAM_ATTR_PORT_*], ...
>+              ...
>+    name-prefix: team-attr-
>+    attributes:
>+      -
>+        name: unspec
>+        type: unused
>+        value: 0
>+      -
>+        name: team-ifindex
>+        type: u32
>+      -
>+        name: list-option
>+        type: nest
>+        nested-attributes: item-option
>+      -
>+        name: list-port
>+        type: nest
>+        nested-attributes: item-port
>+  -
>+    name: item-option
>+    name-prefix: team-attr-item-
>+    attr-cnt-name: __team-attr-item-option-max
>+    attr-max-name: team-attr-item-option-max
>+    attributes:
>+      -
>+        name: option-unspec
>+        type: unused
>+        value: 0
>+      -
>+        name: option
>+        type: nest
>+        nested-attributes: attr-option
>+  -
>+    name: attr-option
>+    name-prefix: team-attr-option-
>+    attributes:
>+      -
>+        name: unspec
>+        type: unused
>+        value: 0
>+      -
>+        name: name
>+        type: string
>+        checks:
>+          # no unterminated-ok defination?
>+          # do we have to hard code this?
>+          max-len: 32
>+      -
>+        name: changed
>+        type: flag
>+      -
>+        name: type
>+        type: u8
>+      -
>+        name: data
>+        type: binary
>+      -
>+        name: removed
>+        type: flag
>+      -
>+        name: port-ifindex
>+        type: u32
>+        doc: for per-port options
>+      -
>+        name: array-index
>+        type: u32
>+        doc: for for array options
>+  -
>+    name: item-port
>+    name-prefix: team-attr-item-
>+    attr-cnt-name: __team-attr-item-port-max
>+    attr-max-name: team-attr-item-port-max
>+    attributes:
>+      -
>+        name: port-unspec
>+        type: unused
>+        value: 0
>+      -
>+        name: port
>+        type: nest
>+        nested-attributes: attr-port
>+  -
>+    name: attr-port
>+    name-prefix: team-attr-port-
>+    attributes:
>+      -
>+        name: unspec
>+        type: unused
>+        value: 0
>+      -
>+        name: ifindex
>+        type: u32
>+      -
>+        name: changed
>+        type: flag
>+      -
>+        name: linkup
>+        type: flag
>+      -
>+        name: speed
>+        type: u32
>+      -
>+        name: duplex
>+        type: u8
>+      -
>+        name: removed
>+        type: flag
>+
>+operations:
>+  list:
>+    -
>+      name: noop
>+      doc: No operation
>+      value: 0
>+      attribute-set: team
>+      dont-validate: [ strict, dump ]

What is this good for?


>+
>+      do:
>+        # Actually it only reply the team netlink family
>+        reply:
>+          attributes:
>+            - team-ifindex
>+
>+    -
>+      name: options-set
>+      doc: Set team options
>+      attribute-set: team
>+      dont-validate: [ strict, dump ]

There is no dump op. Same below.


>+      flags: [ admin-perm ]
>+
>+      do:
>+        request:
>+          attributes:
>+            - team-ifindex
>+            - list-option
>+
>+    -
>+      name: options-get
>+      doc: Get team options info
>+      attribute-set: team
>+      dont-validate: [ strict, dump ]
>+      flags: [ admin-perm ]
>+
>+      do:
>+        request:
>+          attributes:
>+            - team-ifindex
>+        reply:
>+          attributes:
>+            - list-option
>+
>+    -
>+      name: port-list-get
>+      doc: Get team ports info
>+      attribute-set: team
>+      dont-validate: [ strict, dump ]
>+      flags: [ admin-perm ]
>+
>+      do:
>+        request:
>+          attributes:
>+            - team-ifindex
>+        reply:
>+          attributes:
>+            - list-port
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 7fb66210069b..b64e449f47f9 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -21301,6 +21301,7 @@ F:	drivers/net/team/
> F:	include/linux/if_team.h
> F:	include/uapi/linux/if_team.h
> F:	tools/testing/selftests/drivers/net/team/
>+F:	Documentation/netlink/specs/team.yaml
> 
> TECHNICAL ADVISORY BOARD PROCESS DOCS
> M:	"Theodore Ts'o" <tytso@mit.edu>
>-- 
>2.43.0
>

Re: [Draft PATCH net-next 3/3] uapi: team: use header file generated from YAML spec

[Jiri Pirko]

Wed, Dec 13, 2023 at 09:45:02AM CET, liuhangbin@gmail.com wrote:
>generated with:
>
> $ ./tools/net/ynl/ynl-gen-c.py --mode uapi \
> > --spec Documentation/netlink/specs/team.yaml \
> > --header -o include/uapi/linux/if_team.h
>
>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>


This looks fine to me. I don't see why
s/_UAPI_LINUX_IF_TEAM_H_/_UAPI_LINUX_IF_TEAM_H/ would cause issues...

Re: [Draft PATCH net-next 2/3] net: team: use policy generated by YAML spec

[Jiri Pirko]

Wed, Dec 13, 2023 at 09:45:01AM CET, liuhangbin@gmail.com wrote:
>generated with:
>
> $ ./tools/net/ynl/ynl-gen-c.py --mode kernel \
> > --spec Documentation/netlink/specs/team.yaml --source \
> > -o drivers/net/team/team_nl.c
> $ ./tools/net/ynl/ynl-gen-c.py --mode kernel \
> > --spec Documentation/netlink/specs/team.yaml --header \
> > -o drivers/net/team/team_nl.h
>
>The TEAM_ATTR_LIST_PORT in team_nl_policy is removed as it only in the

"only is in the" ? Looks like you are missing verb somewhere anyway.

Overall looks fine.


>port list reply attributes.
>

Re: [Draft PATCH net-next 1/3] Documentation: netlink: add a YAML spec for team

[Jakub Kicinski]

On Wed, 13 Dec 2023 16:45:00 +0800 Hangbin Liu wrote:
> +    -
> +      name: noop
> +      doc: No operation
> +      value: 0
> +      attribute-set: team
> +      dont-validate: [ strict, dump ]
> +
> +      do:
> +        # Actually it only reply the team netlink family
> +        reply:
> +          attributes:
> +            - team-ifindex

Oh my. Does it actually take team-ifindex or its an op with no input
and no output?

Re: [Draft PATCH net-next 0/3] add YAML spec for team

[Jakub Kicinski]

On Wed, 13 Dec 2023 16:44:59 +0800 Hangbin Liu wrote:
> You suggested me to add yaml spec for bridge. Since I'm not familiar with
> writing the spec file, I choose to convert team as a start.

Nice work! If you write a spec you don't necessarily have to use
the spec for C code gen, but I will obviously not stop you from
going the extra mile :)

> There are still some questions I got during convertion.
> 
> 1. Is there a preference to use "-" instead of "_" for the names in spec file?
>    e.g. the attr-cnt-name in team.spec, should I use __team-attr-item-port-max
>    or --team-attr-item-port-max, or __team_attr_item_port_max?

Minor preference for using -, but it mostly matters for things which
will be visible outside of C. For instance in attr names when they are
used in python: 
  msg['port-index']
looks nicer to me than
  msg['port_index']
and is marginally easier to type. But cnt-name is a C thing, so up to
you. If I was writing it myself I'd probably go with
--team-attr-item-port-max, that's what MPTCP did.

> 2. I saw ynl-gen-c.py deals with unterminated-ok. But this policy is not shown
>    in the schemas. Is it a new feature that still working on?

I must have added it to the code gen when experimenting with a family 
I didn't end up supporting. I'm not actively working on that one, feel
free to take a stab at finishing it or LMK if you need help.

> 3. Do we have to hard code the string max-len? Is there a way to use
>    the name in definitions? e.g.
>    name: name
>    type: string
>    checks:
>      max-len: string-max-len

Yes, that's the intention, if codegen doesn't support that today it
should be improved.

> 4. The doc will be generate to rst file in future, so there will not have
>    other comments in the _nl.c or _nl.h files, right?

It already generates ReST:
https://docs.kernel.org/next/networking/netlink_spec/
We do still generate kdoc in the uAPI header, tho.

> 5. the genl_multicast_group is forced to use list. But the team use format
>    like { .name = TEAM_GENL_CHANGE_EVENT_MC_GRP_NAME, }. Should we support
>    this legacy format?

Do you mean that we generate:

	[ID] = { "name", }

rather than:

	[ID] = { .name = "name", }

? I think the struct had only one member at the time, so I didn't
bother adding the .name, but you can change the code-gen.

> 6. The _UAPI_LINUX_IF_TEAM_H_ is rename to _UAPI_LINUX_IF_TEAM_H in uapi
>    header. Does that affects?

Hopefully it's fine. Let's try to make the change and deal with
problems if any get reported. Having the standardized guards
helps a little bit in our Makefile magic...

> 7. When build, I got error modpost: missing MODULE_LICENSE() in drivers/net/team/team_nl.o.
>    Should we add the MODULE_LICENSE support in ynl-gen-c.py?

Not sure if we can, the generated code should be linked with 
the implementation to form a full module. The manually written
part of the implementation should define the license. YAML specs
have a fairly odd / broadly open license because they are uAPI.
We don't want to start getting into licensing business.

> 8. When build, I also got errors like
>      ERROR: modpost: "team_nl_policy" [drivers/net/team/team.ko] undefined!
>      ERROR: modpost: "team_nl_ops" [drivers/net/team/team.ko] undefined!
>      ERROR: modpost: "team_nl_noop_doit" [drivers/net/team/team_nl.ko] undefined!
>      ERROR: modpost: "team_nl_options_set_doit" [drivers/net/team/team_nl.ko] undefined!
>      ERROR: modpost: "team_nl_options_get_doit" [drivers/net/team/team_nl.ko] undefined!
>      ERROR: modpost: "team_nl_port_list_get_doit" [drivers/net/team/team_nl.ko] undefined!
>      ERROR: modpost: "team_attr_option_nl_policy" [drivers/net/team/team.ko] undefined!
>   Do you know why include "team_nl.h" doesn't help?

Same reason as the reason you're getting the LICENSE warning.
kbuild is probably trying to build team_nl and team as separate modules.

I think you'll have to rename team.c, take a look at what I did around
commit 08d323234d10. I don't know a better way...

Re: [Draft PATCH net-next 1/3] Documentation: netlink: add a YAML spec for team

[Hangbin Liu]

On Wed, Dec 13, 2023 at 04:36:32PM +0100, Jiri Pirko wrote:
> >+operations:
> >+  list:
> >+    -
> >+      name: noop
> >+      doc: No operation
> >+      value: 0
> >+      attribute-set: team
> >+      dont-validate: [ strict, dump ]
> 
> What is this good for?
> 
> 
> >+
> >+      do:
> >+        # Actually it only reply the team netlink family
> >+        reply:
> >+          attributes:
> >+            - team-ifindex
> >+
> >+    -
> >+      name: options-set
> >+      doc: Set team options
> >+      attribute-set: team
> >+      dont-validate: [ strict, dump ]
> 
> There is no dump op. Same below.
> 
Hi Jiri,

I just copied this from the current team.c code. e.g.

static const struct genl_small_ops team_nl_ops[] = {
        {
                .cmd = TEAM_CMD_NOOP,
                .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
                .doit = team_nl_cmd_noop,
        },
        {
                .cmd = TEAM_CMD_OPTIONS_SET,
                .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
                .doit = team_nl_cmd_options_set,
                .flags = GENL_ADMIN_PERM,
        },

Do you want to remove all the GENL_DONT_VALIDATE_DUMP flags from team_nl_ops?

Thanks
Hangbin

Re: [Draft PATCH net-next 1/3] Documentation: netlink: add a YAML spec for team

[Hangbin Liu]

On Wed, Dec 13, 2023 at 08:18:18AM -0800, Jakub Kicinski wrote:
> On Wed, 13 Dec 2023 16:45:00 +0800 Hangbin Liu wrote:
> > +    -
> > +      name: noop
> > +      doc: No operation
> > +      value: 0
> > +      attribute-set: team
> > +      dont-validate: [ strict, dump ]
> > +
> > +      do:
> > +        # Actually it only reply the team netlink family
> > +        reply:
> > +          attributes:
> > +            - team-ifindex
> 
> Oh my. Does it actually take team-ifindex or its an op with no input
> and no output?

No, it doesn't take team-ifindex. It's an option with no input
and just reply the team_nl_family.

I added this reply attribute just to make sure the TEAM_CMD_NOOP show in the
cmd enum to not break uAPI.

Thanks
Hangbin

Re: [Draft PATCH net-next 0/3] add YAML spec for team

[Hangbin Liu]

Hi Jakub,
On Wed, Dec 13, 2023 at 08:36:42AM -0800, Jakub Kicinski wrote:
> > There are still some questions I got during convertion.
> > 
> > 1. Is there a preference to use "-" instead of "_" for the names in spec file?
> >    e.g. the attr-cnt-name in team.spec, should I use __team-attr-item-port-max
> >    or --team-attr-item-port-max, or __team_attr_item_port_max?
> 
> Minor preference for using -, but it mostly matters for things which
> will be visible outside of C. For instance in attr names when they are
> used in python: 
>   msg['port-index']
> looks nicer to me than
>   msg['port_index']
> and is marginally easier to type. But cnt-name is a C thing, so up to
> you. If I was writing it myself I'd probably go with
> --team-attr-item-port-max, that's what MPTCP did.

Thanks, I will do as what else do

> 
> > 2. I saw ynl-gen-c.py deals with unterminated-ok. But this policy is not shown
> >    in the schemas. Is it a new feature that still working on?
> 
> I must have added it to the code gen when experimenting with a family 
> I didn't end up supporting. I'm not actively working on that one, feel
> free to take a stab at finishing it or LMK if you need help.

OK, I will do it.

> 
> > 3. Do we have to hard code the string max-len? Is there a way to use
> >    the name in definitions? e.g.
> >    name: name
> >    type: string
> >    checks:
> >      max-len: string-max-len
> 
> Yes, that's the intention, if codegen doesn't support that today it
> should be improved.

I can try improve this. But may a little late (should go next year).
If you have time you can improve this directly.

> 
> > 4. The doc will be generate to rst file in future, so there will not have
> >    other comments in the _nl.c or _nl.h files, right?
> 
> It already generates ReST:
> https://docs.kernel.org/next/networking/netlink_spec/
> We do still generate kdoc in the uAPI header, tho.

How to generate the doc in uAPI header?

> 
> > 5. the genl_multicast_group is forced to use list. But the team use format
> >    like { .name = TEAM_GENL_CHANGE_EVENT_MC_GRP_NAME, }. Should we support
> >    this legacy format?
> 
> Do you mean that we generate:
> 
> 	[ID] = { "name", }
> 
> rather than:
> 
> 	[ID] = { .name = "name", }
> 
> ? I think the struct had only one member at the time, so I didn't
> bother adding the .name, but you can change the code-gen.

No, the current team just do like

static const struct genl_multicast_group team_nl_mcgrps[] = {
        { .name = TEAM_GENL_CHANGE_EVENT_MC_GRP_NAME, },
};

So there is new ID was defined. Looks the group id is not a must.

> > 7. When build, I got error modpost: missing MODULE_LICENSE() in drivers/net/team/team_nl.o.
> >    Should we add the MODULE_LICENSE support in ynl-gen-c.py?
> 
> Not sure if we can, the generated code should be linked with 
> the implementation to form a full module. The manually written
> part of the implementation should define the license. YAML specs
> have a fairly odd / broadly open license because they are uAPI.
> We don't want to start getting into licensing business.
> 
> > 8. When build, I also got errors like
> >      ERROR: modpost: "team_nl_policy" [drivers/net/team/team.ko] undefined!
> >      ERROR: modpost: "team_nl_ops" [drivers/net/team/team.ko] undefined!
> >      ERROR: modpost: "team_nl_noop_doit" [drivers/net/team/team_nl.ko] undefined!
> >      ERROR: modpost: "team_nl_options_set_doit" [drivers/net/team/team_nl.ko] undefined!
> >      ERROR: modpost: "team_nl_options_get_doit" [drivers/net/team/team_nl.ko] undefined!
> >      ERROR: modpost: "team_nl_port_list_get_doit" [drivers/net/team/team_nl.ko] undefined!
> >      ERROR: modpost: "team_attr_option_nl_policy" [drivers/net/team/team.ko] undefined!
> >   Do you know why include "team_nl.h" doesn't help?
> 
> Same reason as the reason you're getting the LICENSE warning.
> kbuild is probably trying to build team_nl and team as separate modules.
> 
> I think you'll have to rename team.c, take a look at what I did around
> commit 08d323234d10. I don't know a better way...

Thanks, this works for me.

Cheers
Hangbin

Re: [Draft PATCH net-next 1/3] Documentation: netlink: add a YAML spec for team

[Jiri Pirko]

Thu, Dec 14, 2023 at 04:44:10AM CET, liuhangbin@gmail.com wrote:
>On Wed, Dec 13, 2023 at 04:36:32PM +0100, Jiri Pirko wrote:
>> >+operations:
>> >+  list:
>> >+    -
>> >+      name: noop
>> >+      doc: No operation
>> >+      value: 0
>> >+      attribute-set: team
>> >+      dont-validate: [ strict, dump ]
>> 
>> What is this good for?
>> 
>> 
>> >+
>> >+      do:
>> >+        # Actually it only reply the team netlink family
>> >+        reply:
>> >+          attributes:
>> >+            - team-ifindex
>> >+
>> >+    -
>> >+      name: options-set
>> >+      doc: Set team options
>> >+      attribute-set: team
>> >+      dont-validate: [ strict, dump ]
>> 
>> There is no dump op. Same below.
>> 
>Hi Jiri,
>
>I just copied this from the current team.c code. e.g.

Right, it is something which is not looked at. Remove it during the
conversion.

>
>static const struct genl_small_ops team_nl_ops[] = {
>        {
>                .cmd = TEAM_CMD_NOOP,
>                .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
>                .doit = team_nl_cmd_noop,
>        },
>        {
>                .cmd = TEAM_CMD_OPTIONS_SET,
>                .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
>                .doit = team_nl_cmd_options_set,
>                .flags = GENL_ADMIN_PERM,
>        },
>
>Do you want to remove all the GENL_DONT_VALIDATE_DUMP flags from team_nl_ops?
>
>Thanks
>Hangbin

Re: [Draft PATCH net-next 1/3] Documentation: netlink: add a YAML spec for team

[Jakub Kicinski]

On Thu, 14 Dec 2023 11:52:50 +0800 Hangbin Liu wrote:
> > Oh my. Does it actually take team-ifindex or its an op with no input
> > and no output?  
> 
> No, it doesn't take team-ifindex. It's an option with no input
> and just reply the team_nl_family.
> 
> I added this reply attribute just to make sure the TEAM_CMD_NOOP show in the
> cmd enum to not break uAPI.

Another todo for the codegen, I guess.

Re: [Draft PATCH net-next 0/3] add YAML spec for team

[Jakub Kicinski]

On Thu, 14 Dec 2023 12:15:07 +0800 Hangbin Liu wrote:
> > > 3. Do we have to hard code the string max-len? Is there a way to use
> > >    the name in definitions? e.g.
> > >    name: name
> > >    type: string
> > >    checks:
> > >      max-len: string-max-len  
> > 
> > Yes, that's the intention, if codegen doesn't support that today it
> > should be improved.  
> 
> I can try improve this. But may a little late (should go next year).
> If you have time you can improve this directly.

Noted on my todo list but no ETA, let's see who gets to it first.. :)

> > > 4. The doc will be generate to rst file in future, so there will not have
> > >    other comments in the _nl.c or _nl.h files, right?  
> > 
> > It already generates ReST:
> > https://docs.kernel.org/next/networking/netlink_spec/
> > We do still generate kdoc in the uAPI header, tho.  
> 
> How to generate the doc in uAPI header?

The doc strings for enum types should appear in uAPI.
Other docs in uAPI usually describe nesting.. which the YAML spec
makes a bit obsolete / possible to generate automatically.
If there's more that we need we can extend the codegen..