netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code
@ 2023-06-02  2:35 Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 01/10] tools: ynl-gen: add extra headers for user space Jakub Kicinski
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

Every now and then I wish I finished the user space part of
the netlink specs, Python scripts kind of stole the show but
C is useful for selftests and stuff which needs to be fast.
Recently someone asked me how to access devlink and ethtool
from C++ which pushed me over the edge.

Fix things which bit rotted and finish notification handling.
This series contains code gen changes only. I'll follow up
with the fixed component, samples and docs as soon as it's
merged.

Jakub Kicinski (10):
  tools: ynl-gen: add extra headers for user space
  tools: ynl-gen: fix unused / pad attribute handling
  tools: ynl-gen: don't override pure nested struct
  tools: ynl-gen: loosen type consistency check for events
  tools: ynl-gen: add error checking for nested structs
  tools: ynl-gen: generate enum-to-string helpers
  tools: ynl-gen: move the response reading logic into YNL
  tools: ynl-gen: generate alloc and free helpers for req
  tools: ynl-gen: switch to family struct
  tools: ynl-gen: generate static descriptions of notifications

 tools/net/ynl/ynl-gen-c.py | 253 +++++++++++++++++++++++++++++--------
 1 file changed, 199 insertions(+), 54 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next 01/10] tools: ynl-gen: add extra headers for user space
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 02/10] tools: ynl-gen: fix unused / pad attribute handling Jakub Kicinski
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

Make sure all relevant headers are included, we allocate memory,
use memcpy() and Linux types without including the headers.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index be664510f484..5823ddf912f6 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -2103,6 +2103,13 @@ _C_KW = {
             cw.nl()
         headers = ['uapi/' + parsed.uapi_header]
     else:
+        cw.p('#include <stdlib.h>')
+        if args.header:
+            cw.p('#include <string.h>')
+            cw.p('#include <linux/types.h>')
+        else:
+            cw.p(f'#include "{parsed.name}-user.h"')
+            cw.p('#include "ynl.h"')
         headers = [parsed.uapi_header]
     for definition in parsed['definitions']:
         if 'header' in definition:
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 02/10] tools: ynl-gen: fix unused / pad attribute handling
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 01/10] tools: ynl-gen: add extra headers for user space Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 03/10] tools: ynl-gen: don't override pure nested struct Jakub Kicinski
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

Unused and Pad attributes don't carry information.
Unused should never exist, and be rejected.
Pad should be silently skipped.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 5823ddf912f6..11dcbfc21ecc 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -170,6 +170,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         for line in lines:
             ri.cw.p(line)
         ri.cw.block_end()
+        return True
 
     def _setter_lines(self, ri, member, presence):
         raise Exception(f"Setter not implemented for class type {self.type}")
@@ -197,6 +198,12 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
     def presence_type(self):
         return ''
 
+    def arg_member(self, ri):
+        return []
+
+    def _attr_get(self, ri, var):
+        return ['return MNL_CB_ERROR;'], None, None
+
     def _attr_typol(self):
         return '.type = YNL_PT_REJECT, '
 
@@ -208,8 +215,14 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
     def presence_type(self):
         return ''
 
+    def arg_member(self, ri):
+        return []
+
     def _attr_typol(self):
-        return '.type = YNL_PT_REJECT, '
+        return '.type = YNL_PT_IGNORE, '
+
+    def attr_get(self, ri, var, first):
+        pass
 
     def attr_policy(self, cw):
         pass
@@ -1211,8 +1224,9 @@ _C_KW = {
 
     first = True
     for _, arg in struct.member_list():
-        arg.attr_get(ri, 'dst', first=first)
-        first = False
+        good = arg.attr_get(ri, 'dst', first=first)
+        # First may be 'unused' or 'pad', ignore those
+        first &= not good
 
     ri.cw.block_end()
     ri.cw.nl()
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 03/10] tools: ynl-gen: don't override pure nested struct
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 01/10] tools: ynl-gen: add extra headers for user space Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 02/10] tools: ynl-gen: fix unused / pad attribute handling Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 04/10] tools: ynl-gen: loosen type consistency check for events Jakub Kicinski
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

For pure structs (parsed nested attributes) we track what
forms of the struct exist in request and reply directions.
Make sure we don't overwrite the recorded struct each time,
otherwise the information is lost.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 11dcbfc21ecc..40f7c47407c8 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -825,7 +825,8 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
                     inherit = set()
                     nested = spec['nested-attributes']
                     if nested not in self.root_sets:
-                        self.pure_nested_structs[nested] = Struct(self, nested, inherited=inherit)
+                        if nested not in self.pure_nested_structs:
+                            self.pure_nested_structs[nested] = Struct(self, nested, inherited=inherit)
                     if attr in rs_members['request']:
                         self.pure_nested_structs[nested].request = True
                     if attr in rs_members['reply']:
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 04/10] tools: ynl-gen: loosen type consistency check for events
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
                   ` (2 preceding siblings ...)
  2023-06-02  2:35 ` [PATCH net-next 03/10] tools: ynl-gen: don't override pure nested struct Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 05/10] tools: ynl-gen: add error checking for nested structs Jakub Kicinski
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

Both event and notify types are always consistent. Rewrite
the condition checking if we can reuse reply types to be
less picky and let notify thru.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 40f7c47407c8..2ceb4ce1423f 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -897,11 +897,12 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         self.op_mode = op_mode
 
         # 'do' and 'dump' response parsing is identical
-        if op_mode != 'do' and 'dump' in op and 'do' in op and 'reply' in op['do'] and \
-           op["do"]["reply"] == op["dump"]["reply"]:
-            self.type_consistent = True
-        else:
-            self.type_consistent = op_mode == 'event'
+        self.type_consistent = True
+        if op_mode != 'do' and 'dump' in op and 'do' in op:
+            if ('reply' in op['do']) != ('reply' in op["dump"]):
+                self.type_consistent = False
+            elif 'reply' in op['do'] and op["do"]["reply"] != op["dump"]["reply"]:
+                self.type_consistent = False
 
         self.attr_set = attr_set
         if not self.attr_set:
@@ -2245,7 +2246,7 @@ _C_KW = {
                     ri = RenderInfo(cw, parsed, args.mode, op, op_name, 'notify')
                     has_ntf = True
                     if not ri.type_consistent:
-                        raise Exception('Only notifications with consistent types supported')
+                        raise Exception(f'Only notifications with consistent types supported ({op.name})')
                     print_wrapped_type(ri)
 
                 if 'event' in op:
@@ -2304,7 +2305,7 @@ _C_KW = {
                     ri = RenderInfo(cw, parsed, args.mode, op, op_name, 'notify')
                     has_ntf = True
                     if not ri.type_consistent:
-                        raise Exception('Only notifications with consistent types supported')
+                        raise Exception(f'Only notifications with consistent types supported ({op.name})')
                     print_ntf_type_free(ri)
 
                 if 'event' in op:
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 05/10] tools: ynl-gen: add error checking for nested structs
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
                   ` (3 preceding siblings ...)
  2023-06-02  2:35 ` [PATCH net-next 04/10] tools: ynl-gen: loosen type consistency check for events Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 06/10] tools: ynl-gen: generate enum-to-string helpers Jakub Kicinski
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

Parsing nested types may return an error, propagate it.
Not marking as a fix, because nothing uses YNL upstream.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 2ceb4ce1423f..8bf4b70216d7 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -424,7 +424,8 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
                             f"{self.enum_name}, &{var}->{self.c_name})")
 
     def _attr_get(self, ri, var):
-        get_lines = [f"{self.nested_render_name}_parse(&parg, attr);"]
+        get_lines = [f"if ({self.nested_render_name}_parse(&parg, attr))",
+                     "return MNL_CB_ERROR;"]
         init_lines = [f"parg.rsp_policy = &{self.nested_render_name}_nest;",
                       f"parg.data = &{var}->{self.c_name};"]
         return get_lines, init_lines, None
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 06/10] tools: ynl-gen: generate enum-to-string helpers
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
                   ` (4 preceding siblings ...)
  2023-06-02  2:35 ` [PATCH net-next 05/10] tools: ynl-gen: add error checking for nested structs Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 07/10] tools: ynl-gen: move the response reading logic into YNL Jakub Kicinski
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

It's sometimes useful to print the name of an enum value,
flag or name of the op. Python can do it, add C helper
code gen for getting names of things.

Example:

  static const char * const netdev_xdp_act_strmap[] = {
	[0] = "basic",
	[1] = "redirect",
	[2] = "ndo-xmit",
	[3] = "xsk-zerocopy",
	[4] = "hw-offload",
	[5] = "rx-sg",
	[6] = "ndo-xmit-sg",
  };

  const char *netdev_xdp_act_str(enum netdev_xdp_act value)
  {
	value = ffs(value) - 1;
	if (value < 0 || value >= (int)MNL_ARRAY_SIZE(netdev_xdp_act_strmap))
		return NULL;
	return netdev_xdp_act_strmap[value];
  }

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 66 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 8bf4b70216d7..5318edfdb874 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -1168,6 +1168,56 @@ _C_KW = {
     cw.nl()
 
 
+def put_op_name_fwd(family, cw):
+    cw.write_func_prot('const char *', f'{family.name}_op_str', ['int op'], suffix=';')
+
+
+def put_op_name(family, cw):
+    map_name = f'{family.name}_op_strmap'
+    cw.block_start(line=f"static const char * const {map_name}[] =")
+    for op_name, op in family.msgs.items():
+        cw.p(f'[{op.enum_name}] = "{op_name}",')
+    cw.block_end(line=';')
+    cw.nl()
+
+    cw.write_func_prot('const char *', f'{family.name}_op_str', ['int op'])
+    cw.block_start()
+    cw.p(f'if (op < 0 || op >= (int)MNL_ARRAY_SIZE({map_name}))')
+    cw.p('return NULL;')
+    cw.p(f'return {map_name}[op];')
+    cw.block_end()
+    cw.nl()
+
+
+def put_enum_to_str_fwd(family, cw, enum):
+    args = [f'enum {enum.render_name} value']
+    if 'enum-name' in enum and not enum['enum-name']:
+        args = ['int value']
+    cw.write_func_prot('const char *', f'{enum.render_name}_str', args, suffix=';')
+
+
+def put_enum_to_str(family, cw, enum):
+    map_name = f'{enum.render_name}_strmap'
+    cw.block_start(line=f"static const char * const {map_name}[] =")
+    for entry in enum.entries.values():
+        cw.p(f'[{entry.value}] = "{entry.name}",')
+    cw.block_end(line=';')
+    cw.nl()
+
+    args = [f'enum {enum.render_name} value']
+    if 'enum-name' in enum and not enum['enum-name']:
+        args = ['int value']
+    cw.write_func_prot('const char *', f'{enum.render_name}_str', args)
+    cw.block_start()
+    if enum.type == 'flags':
+        cw.p('value = ffs(value) - 1;')
+    cw.p(f'if (value < 0 || value >= (int)MNL_ARRAY_SIZE({map_name}))')
+    cw.p('return NULL;')
+    cw.p(f'return {map_name}[value];')
+    cw.block_end()
+    cw.nl()
+
+
 def put_req_nested(ri, struct):
     func_args = ['struct nlmsghdr *nlh',
                  'unsigned int attr_type',
@@ -2210,6 +2260,14 @@ _C_KW = {
     if args.mode == "user":
         has_ntf = False
         if args.header:
+            cw.p('/* Enums */')
+            put_op_name_fwd(parsed, cw)
+
+            for name, const in parsed.consts.items():
+                if isinstance(const, EnumSet):
+                    put_enum_to_str_fwd(parsed, cw, const)
+            cw.nl()
+
             cw.p('/* Common nested types */')
             for attr_set, struct in sorted(parsed.pure_nested_structs.items()):
                 ri = RenderInfo(cw, parsed, args.mode, "", "", "", attr_set)
@@ -2262,6 +2320,14 @@ _C_KW = {
                 print_ntf_parse_prototype(parsed, cw)
             cw.nl()
         else:
+            cw.p('/* Enums */')
+            put_op_name(parsed, cw)
+
+            for name, const in parsed.consts.items():
+                if isinstance(const, EnumSet):
+                    put_enum_to_str(parsed, cw, const)
+            cw.nl()
+
             cw.p('/* Policies */')
             for name, _ in parsed.attr_sets.items():
                 struct = Struct(parsed, name)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 07/10] tools: ynl-gen: move the response reading logic into YNL
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
                   ` (5 preceding siblings ...)
  2023-06-02  2:35 ` [PATCH net-next 06/10] tools: ynl-gen: generate enum-to-string helpers Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 08/10] tools: ynl-gen: generate alloc and free helpers for req Jakub Kicinski
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

We generate send() and recv() calls and all msg handling for
each operation. It's a lot of repeated code and will only grow
with notification handling. Call back to a helper YNL lib instead.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 63 ++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 36 deletions(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 5318edfdb874..7d833a42e060 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -1371,13 +1371,13 @@ _C_KW = {
     ret_err = '-1'
     direction = "request"
     local_vars = ['struct nlmsghdr *nlh;',
-                  'int len, err;']
+                  'int err;']
 
     if 'reply' in ri.op[ri.op_mode]:
         ret_ok = 'rsp'
         ret_err = 'NULL'
         local_vars += [f'{type_name(ri, rdir(direction))} *rsp;',
-                       'struct ynl_parse_arg yarg = { .ys = ys, };']
+                       'struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };']
 
     print_prototype(ri, direction, terminate=False)
     ri.cw.block_start()
@@ -1387,41 +1387,39 @@ _C_KW = {
 
     ri.cw.p(f"ys->req_policy = &{ri.struct['request'].render_name}_nest;")
     if 'reply' in ri.op[ri.op_mode]:
-        ri.cw.p(f"yarg.rsp_policy = &{ri.struct['reply'].render_name}_nest;")
+        ri.cw.p(f"yrs.yarg.rsp_policy = &{ri.struct['reply'].render_name}_nest;")
     ri.cw.nl()
     for _, attr in ri.struct["request"].member_list():
         attr.attr_put(ri, "req")
     ri.cw.nl()
 
-    ri.cw.p('err = mnl_socket_sendto(ys->sock, nlh, nlh->nlmsg_len);')
-    ri.cw.p('if (err < 0)')
-    ri.cw.p(f"return {ret_err};")
-    ri.cw.nl()
-    ri.cw.p('len = mnl_socket_recvfrom(ys->sock, ys->rx_buf, MNL_SOCKET_BUFFER_SIZE);')
-    ri.cw.p('if (len < 0)')
-    ri.cw.p(f"return {ret_err};")
-    ri.cw.nl()
-
+    parse_arg = "NULL"
     if 'reply' in ri.op[ri.op_mode]:
         ri.cw.p('rsp = calloc(1, sizeof(*rsp));')
-        ri.cw.p('yarg.data = rsp;')
+        ri.cw.p('yrs.yarg.data = rsp;')
+        ri.cw.p(f"yrs.cb = {op_prefix(ri, 'reply')}_parse;")
+        if ri.op.value is not None:
+            ri.cw.p(f'yrs.rsp_cmd = {ri.op.enum_name};')
+        else:
+            ri.cw.p(f'yrs.rsp_cmd = {ri.op.rsp_value};')
         ri.cw.nl()
-        ri.cw.p(f"err = {ri.nl.parse_cb_run(op_prefix(ri, 'reply') + '_parse', '&yarg', False)};")
-        ri.cw.p('if (err < 0)')
+        parse_arg = '&yrs'
+    ri.cw.p(f"err = ynl_exec(ys, nlh, {parse_arg});")
+    ri.cw.p('if (err < 0)')
+    if 'reply' in ri.op[ri.op_mode]:
         ri.cw.p('goto err_free;')
-        ri.cw.nl()
-
-    ri.cw.p('err = ynl_recv_ack(ys, err);')
-    ri.cw.p('if (err)')
-    ri.cw.p('goto err_free;')
+    else:
+        ri.cw.p('return -1;')
     ri.cw.nl()
+
     ri.cw.p(f"return {ret_ok};")
     ri.cw.nl()
-    ri.cw.p('err_free:')
 
     if 'reply' in ri.op[ri.op_mode]:
+        ri.cw.p('err_free:')
         ri.cw.p(f"{call_free(ri, rdir(direction), 'rsp')}")
-    ri.cw.p(f"return {ret_err};")
+        ri.cw.p(f"return {ret_err};")
+
     ri.cw.block_end()
 
 
@@ -1431,7 +1429,7 @@ _C_KW = {
     ri.cw.block_start()
     local_vars = ['struct ynl_dump_state yds = {};',
                   'struct nlmsghdr *nlh;',
-                  'int len, err;']
+                  'int err;']
 
     for var in local_vars:
         ri.cw.p(f'{var}')
@@ -1440,6 +1438,10 @@ _C_KW = {
     ri.cw.p('yds.ys = ys;')
     ri.cw.p(f"yds.alloc_sz = sizeof({type_name(ri, rdir(direction))});")
     ri.cw.p(f"yds.cb = {op_prefix(ri, 'reply', deref=True)}_parse;")
+    if ri.op.value is not None:
+        ri.cw.p(f'yds.rsp_cmd = {ri.op.enum_name};')
+    else:
+        ri.cw.p(f'yds.rsp_cmd = {ri.op.rsp_value};')
     ri.cw.p(f"yds.rsp_policy = &{ri.struct['reply'].render_name}_nest;")
     ri.cw.nl()
     ri.cw.p(f"nlh = ynl_gemsg_start_dump(ys, {ri.nl.get_family_id()}, {ri.op.enum_name}, 1);")
@@ -1451,20 +1453,9 @@ _C_KW = {
             attr.attr_put(ri, "req")
     ri.cw.nl()
 
-    ri.cw.p('err = mnl_socket_sendto(ys->sock, nlh, nlh->nlmsg_len);')
-    ri.cw.p('if (err < 0)')
-    ri.cw.p('return NULL;')
-    ri.cw.nl()
-
-    ri.cw.block_start(line='do')
-    ri.cw.p('len = mnl_socket_recvfrom(ys->sock, ys->rx_buf, MNL_SOCKET_BUFFER_SIZE);')
-    ri.cw.p('if (len < 0)')
-    ri.cw.p('goto free_list;')
-    ri.cw.nl()
-    ri.cw.p(f"err = {ri.nl.parse_cb_run('ynl_dump_trampoline', '&yds', False, indent=2)};")
+    ri.cw.p('err = ynl_exec_dump(ys, nlh, &yds);')
     ri.cw.p('if (err < 0)')
     ri.cw.p('goto free_list;')
-    ri.cw.block_end(line='while (err > 0);')
     ri.cw.nl()
 
     ri.cw.p('return yds.first;')
@@ -1631,7 +1622,7 @@ _C_KW = {
     ri.cw.block_start()
     ri.cw.p(f"{sub_type} *next = rsp;")
     ri.cw.nl()
-    ri.cw.block_start(line='while (next)')
+    ri.cw.block_start(line='while ((void *)next != YNL_LIST_END)')
     _free_type_members_iter(ri, ri.struct['reply'])
     ri.cw.p('rsp = next;')
     ri.cw.p('next = rsp->next;')
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 08/10] tools: ynl-gen: generate alloc and free helpers for req
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
                   ` (6 preceding siblings ...)
  2023-06-02  2:35 ` [PATCH net-next 07/10] tools: ynl-gen: move the response reading logic into YNL Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 09/10] tools: ynl-gen: switch to family struct Jakub Kicinski
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

We expect user to allocate requests with calloc(),
make things a bit more consistent and provide helpers.
Generate free calls, too.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 7d833a42e060..4a7ca2823270 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -1476,6 +1476,14 @@ _C_KW = {
     return 'obj'
 
 
+def print_alloc_wrapper(ri, direction):
+    name = op_prefix(ri, direction)
+    ri.cw.write_func_prot(f'static inline struct {name} *', f"{name}_alloc", [f"void"])
+    ri.cw.block_start()
+    ri.cw.p(f'return calloc(1, sizeof(struct {name}));')
+    ri.cw.block_end()
+
+
 def print_free_prototype(ri, direction, suffix=';'):
     name = op_prefix(ri, direction)
     arg = free_arg_name(direction)
@@ -1523,6 +1531,7 @@ _C_KW = {
 
 def print_type_helpers(ri, direction, deref=False):
     print_free_prototype(ri, direction)
+    ri.cw.nl()
 
     if ri.ku_space == 'user' and direction == 'request':
         for _, attr in ri.struct[direction].member_list():
@@ -1531,6 +1540,7 @@ _C_KW = {
 
 
 def print_req_type_helpers(ri):
+    print_alloc_wrapper(ri, "request")
     print_type_helpers(ri, "request")
 
 
@@ -1554,6 +1564,12 @@ _C_KW = {
     print_type(ri, "request")
 
 
+def print_req_free(ri):
+    if 'request' not in ri.op[ri.op_mode]:
+        return
+    _free_type(ri, 'request', ri.struct['request'])
+
+
 def print_rsp_type(ri):
     if (ri.op_mode == 'do' or ri.op_mode == 'dump') and 'reply' in ri.op[ri.op_mode]:
         direction = 'reply'
@@ -2344,6 +2360,7 @@ _C_KW = {
                 if 'do' in op and 'event' not in op:
                     cw.p(f"/* {op.enum_name} - do */")
                     ri = RenderInfo(cw, parsed, args.mode, op, op_name, "do")
+                    print_req_free(ri)
                     print_rsp_free(ri)
                     parse_rsp_msg(ri)
                     print_req(ri)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 09/10] tools: ynl-gen: switch to family struct
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
                   ` (7 preceding siblings ...)
  2023-06-02  2:35 ` [PATCH net-next 08/10] tools: ynl-gen: generate alloc and free helpers for req Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-02  2:35 ` [PATCH net-next 10/10] tools: ynl-gen: generate static descriptions of notifications Jakub Kicinski
  2023-06-03  6:30 ` [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code patchwork-bot+netdevbpf
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

We'll want to store static info about the family soon.
Generate a struct. This changes creation from, e.g.:

	 ys = ynl_sock_create("netdev", &yerr);
to:
	 ys = ynl_sock_create(&ynl_netdev_family, &yerr);

on user's side.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 4a7ca2823270..320e5e90920a 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -2109,6 +2109,16 @@ _C_KW = {
     cw.p(f'#endif /* {hdr_prot} */')
 
 
+def render_user_family(family, cw, prototype):
+    symbol = f'const struct ynl_family ynl_{family.c_name}_family'
+    if prototype:
+        cw.p(f'extern {symbol};')
+    else:
+        cw.block_start(f'{symbol} = ')
+        cw.p(f'.name = "{family.name}",')
+        cw.block_end(line=';')
+
+
 def find_kernel_root(full_path):
     sub_path = ''
     while True:
@@ -2204,6 +2214,8 @@ _C_KW = {
                 cw.p(f'#include "{one}"')
         else:
             cw.p('struct ynl_sock;')
+            cw.nl()
+            render_user_family(parsed, cw, True)
         cw.nl()
 
     if args.mode == "kernel":
@@ -2397,6 +2409,9 @@ _C_KW = {
                 cw.p('/* --------------- Common notification parsing --------------- */')
                 print_ntf_type_parse(parsed, cw, args.mode)
 
+            cw.nl()
+            render_user_family(parsed, cw, False)
+
     if args.header:
         cw.p(f'#endif /* {hdr_prot} */')
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 10/10] tools: ynl-gen: generate static descriptions of notifications
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
                   ` (8 preceding siblings ...)
  2023-06-02  2:35 ` [PATCH net-next 09/10] tools: ynl-gen: switch to family struct Jakub Kicinski
@ 2023-06-02  2:35 ` Jakub Kicinski
  2023-06-03  6:30 ` [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code patchwork-bot+netdevbpf
  10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-06-02  2:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski

Notifications may come in at any time. The family must be always
ready to parse a random incoming notification. Generate notification
table for parsing and tell YNL which request we're processing
to distinguish responses from notifications.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/ynl-gen-c.py | 52 ++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 320e5e90920a..4c12c6f8968e 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -887,6 +887,12 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
                     self.hooks[when][op_mode]['set'].add(name)
                     self.hooks[when][op_mode]['list'].append(name)
 
+    def has_notifications(self):
+        for op in self.ops.values():
+            if 'notify' in op or 'event' in op:
+                return True
+        return False
+
 
 class RenderInfo:
     def __init__(self, cw, family, ku_space, op, op_name, op_mode, attr_set=None):
@@ -1587,6 +1593,7 @@ _C_KW = {
     elif ri.op_mode == 'notify' or ri.op_mode == 'event':
         ri.cw.p('__u16 family;')
         ri.cw.p('__u8 cmd;')
+        ri.cw.p('struct ynl_ntf_base_type *next;')
         ri.cw.p(f"void (*free)({type_name(ri, 'reply')} *ntf);")
     ri.cw.p(f"{type_name(ri, 'reply', deref=True)} obj __attribute__ ((aligned (8)));")
     ri.cw.block_end(line=';')
@@ -2109,14 +2116,43 @@ _C_KW = {
     cw.p(f'#endif /* {hdr_prot} */')
 
 
+def _render_user_ntf_entry(ri, op):
+    ri.cw.block_start(line=f"[{op.enum_name}] = ")
+    ri.cw.p(f".alloc_sz\t= sizeof({type_name(ri, 'event')}),")
+    ri.cw.p(f".cb\t\t= {op_prefix(ri, 'reply', deref=True)}_parse,")
+    ri.cw.p(f".policy\t\t= &{ri.struct['reply'].render_name}_nest,")
+    ri.cw.p(f".free\t\t= (void *){op_prefix(ri, 'notify')}_free,")
+    ri.cw.block_end(line=',')
+
+
 def render_user_family(family, cw, prototype):
     symbol = f'const struct ynl_family ynl_{family.c_name}_family'
     if prototype:
         cw.p(f'extern {symbol};')
-    else:
-        cw.block_start(f'{symbol} = ')
-        cw.p(f'.name = "{family.name}",')
-        cw.block_end(line=';')
+        return
+
+    ntf = family.has_notifications()
+    if ntf:
+        cw.block_start(line=f"static const struct ynl_ntf_info {family['name']}_ntf_info[] = ")
+        for ntf_op in sorted(family.all_notify.keys()):
+            op = family.ops[ntf_op]
+            ri = RenderInfo(cw, family, "user", op, ntf_op, "notify")
+            for ntf in op['notify']['cmds']:
+                _render_user_ntf_entry(ri, ntf)
+        for op_name, op in family.ops.items():
+            if 'event' not in op:
+                continue
+            ri = RenderInfo(cw, family, "user", op, op_name, "event")
+            _render_user_ntf_entry(ri, op)
+        cw.block_end(line=";")
+        cw.nl()
+
+    cw.block_start(f'{symbol} = ')
+    cw.p(f'.name\t\t= "{family.name}",')
+    if ntf:
+        cw.p(f".ntf_info\t= {family['name']}_ntf_info,")
+        cw.p(f".ntf_info_size\t= MNL_ARRAY_SIZE({family['name']}_ntf_info),")
+    cw.block_end(line=';')
 
 
 def find_kernel_root(full_path):
@@ -2277,7 +2313,6 @@ _C_KW = {
             print_kernel_family_struct_src(parsed, cw)
 
     if args.mode == "user":
-        has_ntf = False
         if args.header:
             cw.p('/* Enums */')
             put_op_name_fwd(parsed, cw)
@@ -2322,7 +2357,6 @@ _C_KW = {
                 if 'notify' in op:
                     cw.p(f"/* {op.enum_name} - notify */")
                     ri = RenderInfo(cw, parsed, args.mode, op, op_name, 'notify')
-                    has_ntf = True
                     if not ri.type_consistent:
                         raise Exception(f'Only notifications with consistent types supported ({op.name})')
                     print_wrapped_type(ri)
@@ -2334,7 +2368,7 @@ _C_KW = {
                     cw.nl()
                     print_wrapped_type(ri)
 
-            if has_ntf:
+            if parsed.has_notifications():
                 cw.p('/* --------------- Common notification parsing --------------- */')
                 print_ntf_parse_prototype(parsed, cw)
             cw.nl()
@@ -2390,14 +2424,12 @@ _C_KW = {
                 if 'notify' in op:
                     cw.p(f"/* {op.enum_name} - notify */")
                     ri = RenderInfo(cw, parsed, args.mode, op, op_name, 'notify')
-                    has_ntf = True
                     if not ri.type_consistent:
                         raise Exception(f'Only notifications with consistent types supported ({op.name})')
                     print_ntf_type_free(ri)
 
                 if 'event' in op:
                     cw.p(f"/* {op.enum_name} - event */")
-                    has_ntf = True
 
                     ri = RenderInfo(cw, parsed, args.mode, op, op_name, "do")
                     parse_rsp_msg(ri)
@@ -2405,7 +2437,7 @@ _C_KW = {
                     ri = RenderInfo(cw, parsed, args.mode, op, op_name, "event")
                     print_ntf_type_free(ri)
 
-            if has_ntf:
+            if parsed.has_notifications():
                 cw.p('/* --------------- Common notification parsing --------------- */')
                 print_ntf_type_parse(parsed, cw, args.mode)
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code
  2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
                   ` (9 preceding siblings ...)
  2023-06-02  2:35 ` [PATCH net-next 10/10] tools: ynl-gen: generate static descriptions of notifications Jakub Kicinski
@ 2023-06-03  6:30 ` patchwork-bot+netdevbpf
  10 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-06-03  6:30 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  1 Jun 2023 19:35:38 -0700 you wrote:
> Every now and then I wish I finished the user space part of
> the netlink specs, Python scripts kind of stole the show but
> C is useful for selftests and stuff which needs to be fast.
> Recently someone asked me how to access devlink and ethtool
> from C++ which pushed me over the edge.
> 
> Fix things which bit rotted and finish notification handling.
> This series contains code gen changes only. I'll follow up
> with the fixed component, samples and docs as soon as it's
> merged.
> 
> [...]

Here is the summary with links:
  - [net-next,01/10] tools: ynl-gen: add extra headers for user space
    https://git.kernel.org/netdev/net-next/c/91dfaef243cd
  - [net-next,02/10] tools: ynl-gen: fix unused / pad attribute handling
    https://git.kernel.org/netdev/net-next/c/6ad49839ba9b
  - [net-next,03/10] tools: ynl-gen: don't override pure nested struct
    https://git.kernel.org/netdev/net-next/c/67c65ce762ad
  - [net-next,04/10] tools: ynl-gen: loosen type consistency check for events
    https://git.kernel.org/netdev/net-next/c/5605f102378f
  - [net-next,05/10] tools: ynl-gen: add error checking for nested structs
    https://git.kernel.org/netdev/net-next/c/eef9b794eac8
  - [net-next,06/10] tools: ynl-gen: generate enum-to-string helpers
    https://git.kernel.org/netdev/net-next/c/21b6e302789c
  - [net-next,07/10] tools: ynl-gen: move the response reading logic into YNL
    https://git.kernel.org/netdev/net-next/c/dc0956c98f11
  - [net-next,08/10] tools: ynl-gen: generate alloc and free helpers for req
    https://git.kernel.org/netdev/net-next/c/5d58f911c755
  - [net-next,09/10] tools: ynl-gen: switch to family struct
    https://git.kernel.org/netdev/net-next/c/8cb6afb33541
  - [net-next,10/10] tools: ynl-gen: generate static descriptions of notifications
    https://git.kernel.org/netdev/net-next/c/59d814f0f285

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-06-03  6:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-02  2:35 [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 01/10] tools: ynl-gen: add extra headers for user space Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 02/10] tools: ynl-gen: fix unused / pad attribute handling Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 03/10] tools: ynl-gen: don't override pure nested struct Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 04/10] tools: ynl-gen: loosen type consistency check for events Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 05/10] tools: ynl-gen: add error checking for nested structs Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 06/10] tools: ynl-gen: generate enum-to-string helpers Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 07/10] tools: ynl-gen: move the response reading logic into YNL Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 08/10] tools: ynl-gen: generate alloc and free helpers for req Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 09/10] tools: ynl-gen: switch to family struct Jakub Kicinski
2023-06-02  2:35 ` [PATCH net-next 10/10] tools: ynl-gen: generate static descriptions of notifications Jakub Kicinski
2023-06-03  6:30 ` [PATCH net-next 00/10] tools: ynl-gen: dust off the user space code patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).