[PATCH net-next 0/3] tools: ynl: prevent reorder and fix flags

[PATCH net-next 0/3] tools: ynl: prevent reorder and fix flags

[Jakub Kicinski]

Some codegen improvements for YAML specs.

First, Lorenzon discovered when switching the XDP feature family
to use flags instead of pure enum that the kdoc got garbled.
The support for enum and flags is therefore unified.

Second when regenerating all families we discussed so far I noticed
that some netlink policies jumped around. We need to ensure we don't
render code based on their ordering in a hash.

Jakub Kicinski (3):
  tools: ynl: support kdocs for flags in code generation
  tools: ynl: rename ops_list -> msg_list
  tools: ynl: store ops in ordered dict to avoid random ordering

 tools/net/ynl/ynl-gen-c.py | 50 +++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 22 deletions(-)

-- 
2.39.1

[PATCH net-next 1/3] tools: ynl: support kdocs for flags in code generation

[Jakub Kicinski]

Lorenzo reports that after switching from enum to flags netdev
family lost ability to render kdoc (and the enum contents got
generally garbled).

Combine the flags and enum handling in uAPI handling.

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

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 0c0f18540b7f..91df8eec86f9 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -565,6 +565,7 @@ import yaml
             self.doc = yaml.get('doc', '')
 
         self.yaml = yaml
+        self.enum_set = enum_set
         self.c_name = c_upper(enum_set.value_pfx + self.name)
 
         if 'value' in yaml:
@@ -572,11 +573,14 @@ import yaml
             if prev:
                 self.value_change = (self.value != prev.value + 1)
         elif prev:
+            self.value_change = False
             self.value = prev.value + 1
         else:
             self.value = value_start
             self.value_change = (self.value != 0)
 
+        self.value_change = self.value_change or self.enum_set['type'] == 'flags'
+
     def __getitem__(self, key):
         return self.yaml[key]
 
@@ -586,6 +590,17 @@ import yaml
     def has_doc(self):
         return bool(self.doc)
 
+    # raw value, i.e. the id in the enum, unlike user value which is a mask for flags
+    def raw_value(self):
+        return self.value
+
+    # user value, same as raw value for enums, for flags it's the mask
+    def user_value(self):
+        if self.enum_set['type'] == 'flags':
+            return 1 << self.value
+        else:
+            return self.value
+
 
 class EnumSet:
     def __init__(self, family, yaml):
@@ -824,7 +839,7 @@ import yaml
 
     def _dictify(self):
         for elem in self.yaml['definitions']:
-            if elem['type'] == 'enum':
+            if elem['type'] == 'enum' or elem['type'] == 'flags':
                 self.consts[elem['name']] = EnumSet(self, elem)
             else:
                 self.consts[elem['name']] = elem
@@ -1973,7 +1988,8 @@ _C_KW = {
             defines = []
             cw.nl()
 
-        if const['type'] == 'enum':
+        # Write kdoc for enum and flags (one day maybe also structs)
+        if const['type'] == 'enum' or const['type'] == 'flags':
             enum = family.consts[const['name']]
 
             if enum.has_doc():
@@ -1989,13 +2005,11 @@ _C_KW = {
                 cw.p(' */')
 
             uapi_enum_start(family, cw, const, 'name')
-            first = True
             name_pfx = const.get('name-prefix', f"{family.name}-{const['name']}-")
             for entry in enum.entry_list:
                 suffix = ','
-                if first and 'value-start' in const:
-                    suffix = f" = {const['value-start']}" + suffix
-                first = False
+                if entry.value_change:
+                    suffix = f" = {entry.user_value()}" + suffix
                 cw.p(entry.c_name + suffix)
 
             if const.get('render-max', False):
@@ -2005,17 +2019,6 @@ _C_KW = {
                 cw.p(max_name + ' = (__' + max_name + ' - 1)')
             cw.block_end(line=';')
             cw.nl()
-        elif const['type'] == 'flags':
-            uapi_enum_start(family, cw, const, 'name')
-            i = const.get('value-start', 0)
-            for item in const['entries']:
-                item_name = item
-                if 'name-prefix' in const:
-                    item_name = c_upper(const['name-prefix'] + item)
-                cw.p(f'{item_name} = {1 << i},')
-                i += 1
-            cw.block_end(line=';')
-            cw.nl()
         elif const['type'] == 'const':
             defines.append([c_upper(family.get('c-define-name',
                                                f"{family.name}-{const['name']}")),
-- 
2.39.1

[PATCH net-next 2/3] tools: ynl: rename ops_list -> msg_list

[Jakub Kicinski]

ops_list contains all the operations, but the main iteration use
case is to walk only ops which define attrs. Rename ops_list to
msg_list, because now it looks like the contents are the same,
just the format is different. While at it convert from tuple
to just keys, none of the users care about the name of the op.

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

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 91df8eec86f9..9297cfacbe06 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -790,8 +790,10 @@ import yaml
         self.mcgrps = self.yaml.get('mcast-groups', {'list': []})
 
         self.consts = dict()
+        # list of all operations
+        self.msg_list = []
+        # dict of operations which have their own message type (have attributes)
         self.ops = dict()
-        self.ops_list = []
         self.attr_sets = dict()
         self.attr_sets_list = []
 
@@ -858,7 +860,7 @@ import yaml
             op = Operation(self, elem, val)
             val += 1
 
-            self.ops_list.append((elem['name'], op),)
+            self.msg_list.append(op)
             if 'notify' in elem:
                 ntf.append(op)
                 continue
@@ -2063,7 +2065,7 @@ _C_KW = {
     max_value = f"({cnt_name} - 1)"
 
     uapi_enum_start(family, cw, family['operations'], 'enum-name')
-    for _, op in family.ops_list:
+    for op in family.msg_list:
         if separate_ntf and ('notify' in op or 'event' in op):
             continue
 
@@ -2082,7 +2084,7 @@ _C_KW = {
 
     if separate_ntf:
         uapi_enum_start(family, cw, family['operations'], enum_name='async-enum')
-        for _, op in family.ops_list:
+        for op in family.msg_list:
             if separate_ntf and not ('notify' in op or 'event' in op):
                 continue
 
-- 
2.39.1

[PATCH net-next 3/3] tools: ynl: store ops in ordered dict to avoid random ordering

[Jakub Kicinski]

When rendering code we should walk the ops in the order in which
they are declared in the spec. This is both more intuitive and
prevents code from jumping around when hashing in the dict changes.

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 9297cfacbe06..1aa872e582ab 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 
 import argparse
+import collections
 import jsonschema
 import os
 import yaml
@@ -793,7 +794,7 @@ import yaml
         # list of all operations
         self.msg_list = []
         # dict of operations which have their own message type (have attributes)
-        self.ops = dict()
+        self.ops = collections.OrderedDict()
         self.attr_sets = dict()
         self.attr_sets_list = []
 
-- 
2.39.1

Re: [PATCH net-next 0/3] tools: ynl: prevent reorder and fix flags

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 25 Jan 2023 16:02:32 -0800 you wrote:
> Some codegen improvements for YAML specs.
> 
> First, Lorenzon discovered when switching the XDP feature family
> to use flags instead of pure enum that the kdoc got garbled.
> The support for enum and flags is therefore unified.
> 
> Second when regenerating all families we discussed so far I noticed
> that some netlink policies jumped around. We need to ensure we don't
> render code based on their ordering in a hash.
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] tools: ynl: support kdocs for flags in code generation
    https://git.kernel.org/netdev/net-next/c/66fa34b9c2a5
  - [net-next,2/3] tools: ynl: rename ops_list -> msg_list
    https://git.kernel.org/netdev/net-next/c/b49c34e217c6
  - [net-next,3/3] tools: ynl: store ops in ordered dict to avoid random ordering
    https://git.kernel.org/netdev/net-next/c/3a43ded081f8

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