[PATCH net-next v2 0/3] tools: ynl-gen: support sub-types for binary attributes

[PATCH net-next v2 0/3] tools: ynl-gen: support sub-types for binary attributes

[Jakub Kicinski]

Binary attributes have sub-type annotations which either indicate
that the binary object should be interpreted as a raw / C array of
a simple type (e.g. u32), or that it's a struct.

Use this information in the C codegen instead of outputting void *
for all binary attrs. It doesn't make a huge difference in the genl
families, but in classic Netlink there is a lot more structs.

v2:
 - use sub-classes
v1: https://lore.kernel.org/20250508022839.1256059-1-kuba@kernel.org

Jakub Kicinski (3):
  tools: ynl-gen: support sub-type for binary attributes
  tools: ynl-gen: auto-indent else
  tools: ynl-gen: support struct for binary attributes

 tools/net/ynl/pyynl/ynl_gen_c.py | 63 ++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 3 deletions(-)

-- 
2.49.0

[PATCH net-next v2 1/3] tools: ynl-gen: support sub-type for binary attributes

[Jakub Kicinski]

Sub-type annotation on binary attributes may indicate that the attribute
carries an array of simple types (also referred to as "C array" in docs).
Support rendering them as such in the C user code. For example for u32,
instead of:

  struct {
    u32 arr;
  } _len;

  void *arr;

render:

  struct {
    u32 arr;
  } _count;

  __u32 *arr;

Note that count is the number of elements while len was the length in bytes.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - create a separate class
v1: https://lore.kernel.org/20250508022839.1256059-2-kuba@kernel.org/
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 43 +++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 4a2d3cc07e14..4e2ae738c0aa 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -163,7 +163,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         return False
 
     def _free_lines(self, ri, var, ref):
-        if self.is_multi_val() or self.presence_type() == 'len':
+        if self.is_multi_val() or self.presence_type() in {'count', 'len'}:
             return [f'free({var}->{ref}{self.c_name});']
         return []
 
@@ -566,6 +566,40 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
                 f'memcpy({member}, {self.c_name}, {presence});']
 
 
+class TypeBinaryScalarArray(TypeBinary):
+    def arg_member(self, ri):
+        return [f'__{self.get("sub-type")} *{self.c_name}', 'size_t count']
+
+    def presence_type(self):
+        return 'count'
+
+    def struct_member(self, ri):
+        ri.cw.p(f'__{self.get("sub-type")} *{self.c_name};')
+
+    def attr_put(self, ri, var):
+        presence = self.presence_type()
+        ri.cw.block_start(line=f"if ({var}->_{presence}.{self.c_name})")
+        ri.cw.p(f"i = {var}->_{presence}.{self.c_name} * sizeof(__{self.get('sub-type')});")
+        ri.cw.p(f"ynl_attr_put(nlh, {self.enum_name}, " +
+                f"{var}->{self.c_name}, i);")
+        ri.cw.block_end()
+
+    def _attr_get(self, ri, var):
+        len_mem = var + '->_count.' + self.c_name
+        return [f"{len_mem} = len / sizeof(__{self.get('sub-type')});",
+                f"len = {len_mem} * sizeof(__{self.get('sub-type')});",
+                f"{var}->{self.c_name} = malloc(len);",
+                f"memcpy({var}->{self.c_name}, ynl_attr_data(attr), len);"], \
+               ['len = ynl_attr_data_len(attr);'], \
+               ['unsigned int len;']
+
+    def _setter_lines(self, ri, member, presence):
+        return [f"{presence} = count;",
+                f"count *= sizeof(__{self.get('sub-type')});",
+                f"{member} = malloc(count);",
+                f'memcpy({member}, {self.c_name}, count);']
+
+
 class TypeBitfield32(Type):
     def _complex_member_type(self, ri):
         return "struct nla_bitfield32"
@@ -672,7 +706,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         lines = []
         if self.attr['type'] in scalars:
             lines += [f"free({var}->{ref}{self.c_name});"]
-        elif self.attr['type'] == 'binary' and 'struct' in self.attr:
+        elif self.attr['type'] == 'binary':
             lines += [f"free({var}->{ref}{self.c_name});"]
         elif self.attr['type'] == 'string':
             lines += [
@@ -976,7 +1010,10 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         elif elem['type'] == 'string':
             t = TypeString(self.family, self, elem, value)
         elif elem['type'] == 'binary':
-            t = TypeBinary(self.family, self, elem, value)
+            if elem.get('sub-type') in scalars:
+                t = TypeBinaryScalarArray(self.family, self, elem, value)
+            else:
+                t = TypeBinary(self.family, self, elem, value)
         elif elem['type'] == 'bitfield32':
             t = TypeBitfield32(self.family, self, elem, value)
         elif elem['type'] == 'nest':
-- 
2.49.0

Re: [PATCH net-next v2 1/3] tools: ynl-gen: support sub-type for binary attributes

[Donald Hunter]

Jakub Kicinski <kuba@kernel.org> writes:

> Sub-type annotation on binary attributes may indicate that the attribute
> carries an array of simple types (also referred to as "C array" in docs).
> Support rendering them as such in the C user code. For example for u32,
> instead of:
>
>   struct {
>     u32 arr;
>   } _len;
>
>   void *arr;
>
> render:
>
>   struct {
>     u32 arr;
>   } _count;
>
>   __u32 *arr;
>
> Note that count is the number of elements while len was the length in bytes.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

[PATCH net-next v2 2/3] tools: ynl-gen: auto-indent else

[Jakub Kicinski]

We auto-indent if statements (increase the indent of the subsequent
line by 1), do the same thing for else branches without a block.
There hasn't been any else branches before but we're about to add one.

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 4e2ae738c0aa..9a5c65966e9d 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -1458,6 +1458,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         if self._silent_block:
             ind += 1
         self._silent_block = line.endswith(')') and CodeWriter._is_cond(line)
+        self._silent_block |= line.strip() == 'else'
         if line[0] == '#':
             ind = 0
         if add_ind:
-- 
2.49.0

[PATCH net-next v2 3/3] tools: ynl-gen: support struct for binary attributes

[Jakub Kicinski]

Support using a struct pointer for binary attrs. Len field is maintained
because the structs may grow with newer kernel versions. Or, which matters
more, be shorter if the binary is built against newer uAPI than kernel
against which it's executed. Since we are storing a pointer to a struct
type - always allocate at least the amount of memory needed by the struct
per current uAPI headers (unused mem is zeroed). Technically users should
check the length field but per modern ASAN checks storing a short object
under a pointer seems like a bad idea.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
  - create a separate class
v1: https://lore.kernel.org/20250508022839.1256059-4-kuba@kernel.org
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 9a5c65966e9d..3b064c61a374 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -566,6 +566,23 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
                 f'memcpy({member}, {self.c_name}, {presence});']
 
 
+class TypeBinaryStruct(TypeBinary):
+    def struct_member(self, ri):
+        ri.cw.p(f'struct {c_lower(self.get("struct"))} *{self.c_name};')
+
+    def _attr_get(self, ri, var):
+        struct_sz = 'sizeof(struct ' + c_lower(self.get("struct")) + ')'
+        len_mem = var + '->_' + self.presence_type() + '.' + self.c_name
+        return [f"{len_mem} = len;",
+                f"if (len < {struct_sz})",
+                f"{var}->{self.c_name} = calloc(1, {struct_sz});",
+                "else",
+                f"{var}->{self.c_name} = malloc(len);",
+                f"memcpy({var}->{self.c_name}, ynl_attr_data(attr), len);"], \
+               ['len = ynl_attr_data_len(attr);'], \
+               ['unsigned int len;']
+
+
 class TypeBinaryScalarArray(TypeBinary):
     def arg_member(self, ri):
         return [f'__{self.get("sub-type")} *{self.c_name}', 'size_t count']
@@ -1010,7 +1027,9 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         elif elem['type'] == 'string':
             t = TypeString(self.family, self, elem, value)
         elif elem['type'] == 'binary':
-            if elem.get('sub-type') in scalars:
+            if 'struct' in elem:
+                t = TypeBinaryStruct(self.family, self, elem, value)
+            elif elem.get('sub-type') in scalars:
                 t = TypeBinaryScalarArray(self.family, self, elem, value)
             else:
                 t = TypeBinary(self.family, self, elem, value)
-- 
2.49.0

Re: [PATCH net-next v2 3/3] tools: ynl-gen: support struct for binary attributes

[Donald Hunter]

Jakub Kicinski <kuba@kernel.org> writes:

> Support using a struct pointer for binary attrs. Len field is maintained
> because the structs may grow with newer kernel versions. Or, which matters
> more, be shorter if the binary is built against newer uAPI than kernel
> against which it's executed. Since we are storing a pointer to a struct
> type - always allocate at least the amount of memory needed by the struct
> per current uAPI headers (unused mem is zeroed). Technically users should
> check the length field but per modern ASAN checks storing a short object
> under a pointer seems like a bad idea.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Donald Hunter <donald.hunter@gmail.com>

Re: [PATCH net-next v2 0/3] tools: ynl-gen: support sub-types for binary attributes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri,  9 May 2025 08:42:10 -0700 you wrote:
> Binary attributes have sub-type annotations which either indicate
> that the binary object should be interpreted as a raw / C array of
> a simple type (e.g. u32), or that it's a struct.
> 
> Use this information in the C codegen instead of outputting void *
> for all binary attrs. It doesn't make a huge difference in the genl
> families, but in classic Netlink there is a lot more structs.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/3] tools: ynl-gen: support sub-type for binary attributes
    https://git.kernel.org/netdev/net-next/c/02a562bb2b08
  - [net-next,v2,2/3] tools: ynl-gen: auto-indent else
    https://git.kernel.org/netdev/net-next/c/9ba8e351efd4
  - [net-next,v2,3/3] tools: ynl-gen: support struct for binary attributes
    https://git.kernel.org/netdev/net-next/c/25e37418c872

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