[PATCH net-next 0/3] tools: ynl-gen: support full range of min/max checks

[PATCH net-next 0/3] tools: ynl-gen: support full range of min/max checks

[Jakub Kicinski]

YNL code gen currently supports only very simple range checks
within the range of s16. Add support for full range of u64 / s64
which is good to have, and will be even more important with uint / sint.

Jakub Kicinski (3):
  tools: ynl-gen: track attribute use
  tools: ynl-gen: support full range of min/max checks for integer
    values
  tools: ynl-gen: support limit names

 Documentation/netlink/genetlink-c.yaml      |  10 +-
 Documentation/netlink/genetlink-legacy.yaml |  10 +-
 Documentation/netlink/genetlink.yaml        |  10 +-
 tools/net/ynl/ynl-gen-c.py                  | 115 ++++++++++++++++++--
 4 files changed, 132 insertions(+), 13 deletions(-)

-- 
2.41.0

[PATCH net-next 1/3] tools: ynl-gen: track attribute use

[Jakub Kicinski]

For range validation we'll need to know if any individual
attribute is used on input (i.e. whether we will generate
a policy for it). Track this information.

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

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index f125b5f704ba..7f4ad4014d17 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -42,6 +42,9 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         self.type = attr['type']
         self.checks = attr.get('checks', {})
 
+        self.request = False
+        self.reply = False
+
         if 'len' in attr:
             self.len = attr['len']
         if 'nested-attributes' in attr:
@@ -846,6 +849,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
 
         self._load_root_sets()
         self._load_nested_sets()
+        self._load_attr_use()
         self._load_hooks()
 
         self.kernel_policy = self.yaml.get('kernel-policy', 'split')
@@ -966,6 +970,22 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
                         child.request |= struct.request
                         child.reply |= struct.reply
 
+    def _load_attr_use(self):
+        for _, struct in self.pure_nested_structs.items():
+            if struct.request:
+                for _, arg in struct.member_list():
+                    arg.request = True
+            if struct.reply:
+                for _, arg in struct.member_list():
+                    arg.reply = True
+
+        for root_set, rs_members in self.root_sets.items():
+            for attr, spec in self.attr_sets[root_set].items():
+                if attr in rs_members['request']:
+                    spec.request = True
+                if attr in rs_members['reply']:
+                    spec.reply = True
+
     def _load_global_policy(self):
         global_set = set()
         attr_set_name = None
-- 
2.41.0

[PATCH net-next 2/3] tools: ynl-gen: support full range of min/max checks for integer values

[Jakub Kicinski]

Extend the support to full range of min/max checks.
None of the existing YNL families required complex integer validation.

The support is less than trivial, because we try to keep struct nla_policy
tiny the min/max members it holds in place are s16. Meaning we can only
express checks in range of s16. For larger ranges we need to define
a structure and link it in the policy.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/genetlink-c.yaml      |  3 +
 Documentation/netlink/genetlink-legacy.yaml |  3 +
 Documentation/netlink/genetlink.yaml        |  3 +
 tools/net/ynl/ynl-gen-c.py                  | 66 ++++++++++++++++++---
 4 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/Documentation/netlink/genetlink-c.yaml b/Documentation/netlink/genetlink-c.yaml
index f9366aaddd21..75af0b51f3d7 100644
--- a/Documentation/netlink/genetlink-c.yaml
+++ b/Documentation/netlink/genetlink-c.yaml
@@ -184,6 +184,9 @@ additionalProperties: False
                   min:
                     description: Min value for an integer attribute.
                     type: integer
+                  max:
+                    description: Max value for an integer attribute.
+                    type: integer
                   min-len:
                     description: Min length for a binary attribute.
                     $ref: '#/$defs/len-or-define'
diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index a6a490333a1a..c0f17a8bfe0d 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -227,6 +227,9 @@ additionalProperties: False
                   min:
                     description: Min value for an integer attribute.
                     type: integer
+                  max:
+                    description: Max value for an integer attribute.
+                    type: integer
                   min-len:
                     description: Min length for a binary attribute.
                     $ref: '#/$defs/len-or-define'
diff --git a/Documentation/netlink/genetlink.yaml b/Documentation/netlink/genetlink.yaml
index 2b788e607a14..4fd56e3b1553 100644
--- a/Documentation/netlink/genetlink.yaml
+++ b/Documentation/netlink/genetlink.yaml
@@ -157,6 +157,9 @@ additionalProperties: False
                   min:
                     description: Min value for an integer attribute.
                     type: integer
+                  max:
+                    description: Max value for an integer attribute.
+                    type: integer
                   min-len:
                     description: Min length for a binary attribute.
                     $ref: '#/$defs/len-or-define'
diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 7f4ad4014d17..9d008346dd50 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -47,6 +47,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
 
         if 'len' in attr:
             self.len = attr['len']
+
         if 'nested-attributes' in attr:
             self.nested_attrs = attr['nested-attributes']
             if self.nested_attrs == family.name:
@@ -262,6 +263,27 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         if 'byte-order' in attr:
             self.byte_order_comment = f" /* {attr['byte-order']} */"
 
+        if 'enum' in self.attr:
+            enum = self.family.consts[self.attr['enum']]
+            low, high = enum.value_range()
+            if 'min' not in self.checks:
+                if low != 0 or self.type[0] == 's':
+                    self.checks['min'] = low
+            if 'max' not in self.checks:
+                self.checks['max'] = high
+
+        if 'min' in self.checks and 'max' in self.checks:
+            if self.checks['min'] > self.checks['max']:
+                raise Exception(f'Invalid limit for "{self.name}" min: {self.checks["min"]} max: {self.checks["max"]}')
+            self.checks['range'] = True
+
+        low = min(self.checks.get('min', 0), self.checks.get('max', 0))
+        high = max(self.checks.get('min', 0), self.checks.get('max', 0))
+        if low < 0 and self.type[0] == 'u':
+            raise Exception(f'Invalid limit for "{self.name}" negative limit for unsigned type')
+        if low < -32768 or high > 32767:
+            self.checks['full-range'] = True
+
         # Added by resolve():
         self.is_bitfield = None
         delattr(self, "is_bitfield")
@@ -301,14 +323,14 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
                 flag_cnt = len(flags['entries'])
                 mask = (1 << flag_cnt) - 1
             return f"NLA_POLICY_MASK({policy}, 0x{mask:x})"
+        elif 'full-range' in self.checks:
+            return f"NLA_POLICY_FULL_RANGE({policy}, &{c_lower(self.enum_name)}_range)"
+        elif 'range' in self.checks:
+            return f"NLA_POLICY_RANGE({policy}, {self.checks['min']}, {self.checks['max']})"
         elif 'min' in self.checks:
             return f"NLA_POLICY_MIN({policy}, {self.checks['min']})"
-        elif 'enum' in self.attr:
-            enum = self.family.consts[self.attr['enum']]
-            low, high = enum.value_range()
-            if low == 0:
-                return f"NLA_POLICY_MAX({policy}, {high})"
-            return f"NLA_POLICY_RANGE({policy}, {low}, {high})"
+        elif 'max' in self.checks:
+            return f"NLA_POLICY_MAX({policy}, {self.checks['max']})"
         return super()._attr_policy(policy)
 
     def _attr_typol(self):
@@ -1241,7 +1263,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         for one in members:
             line = '.' + one[0]
             line += '\t' * ((longest - len(one[0]) - 1 + 7) // 8)
-            line += '= ' + one[1] + ','
+            line += '= ' + str(one[1]) + ','
             self.p(line)
 
 
@@ -1940,6 +1962,34 @@ _C_KW = {
     return family.kernel_policy == 'split' or kernel_can_gen_family_struct(family)
 
 
+def print_kernel_policy_ranges(family, cw):
+    first = True
+    for _, attr_set in family.attr_sets.items():
+        if attr_set.subset_of:
+            continue
+
+        for _, attr in attr_set.items():
+            if not attr.request:
+                continue
+            if 'full-range' not in attr.checks:
+                continue
+
+            if first:
+                cw.p('/* Integer value ranges */')
+                first = False
+
+            sign = '' if attr.type[0] == 'u' else '_signed'
+            cw.block_start(line=f'struct netlink_range_validation{sign} {c_lower(attr.enum_name)}_range =')
+            members = []
+            if 'min' in attr.checks:
+                members.append(('min', attr.checks['min']))
+            if 'max' in attr.checks:
+                members.append(('max', attr.checks['max']))
+            cw.write_struct_init(members)
+            cw.block_end(line=';')
+            cw.nl()
+
+
 def print_kernel_op_table_fwd(family, cw, terminate):
     exported = not kernel_can_gen_family_struct(family)
 
@@ -2479,6 +2529,8 @@ _C_KW = {
             print_kernel_mcgrp_hdr(parsed, cw)
             print_kernel_family_struct_hdr(parsed, cw)
         else:
+            print_kernel_policy_ranges(parsed, cw)
+
             for _, struct in sorted(parsed.pure_nested_structs.items()):
                 if struct.request:
                     cw.p('/* Common nested types */')
-- 
2.41.0

[PATCH net-next 3/3] tools: ynl-gen: support limit names

[Jakub Kicinski]

Support the use of symbolic names like s8-min or u32-max in checks
to make writing specs less painful.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/genetlink-c.yaml      |  9 ++++-
 Documentation/netlink/genetlink-legacy.yaml |  9 ++++-
 Documentation/netlink/genetlink.yaml        |  9 ++++-
 tools/net/ynl/ynl-gen-c.py                  | 45 ++++++++++++++++-----
 4 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/Documentation/netlink/genetlink-c.yaml b/Documentation/netlink/genetlink-c.yaml
index 75af0b51f3d7..dee11c514896 100644
--- a/Documentation/netlink/genetlink-c.yaml
+++ b/Documentation/netlink/genetlink-c.yaml
@@ -13,6 +13,11 @@ $schema: https://json-schema.org/draft-07/schema
     type: [ string, integer ]
     pattern: ^[0-9A-Za-z_]+( - 1)?$
     minimum: 0
+  len-or-limit:
+    # literal int or limit based on fixed-width type e.g. u8-min, u16-max, etc.
+    type: [ string, integer ]
+    pattern: ^[su](8|16|32|64)-(min|max)$
+    minimum: 0
 
 # Schema for specs
 title: Protocol
@@ -183,10 +188,10 @@ additionalProperties: False
                     type: string
                   min:
                     description: Min value for an integer attribute.
-                    type: integer
+                    $ref: '#/$defs/len-or-limit'
                   max:
                     description: Max value for an integer attribute.
-                    type: integer
+                    $ref: '#/$defs/len-or-limit'
                   min-len:
                     description: Min length for a binary attribute.
                     $ref: '#/$defs/len-or-define'
diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index c0f17a8bfe0d..9194f3e223ef 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -13,6 +13,11 @@ $schema: https://json-schema.org/draft-07/schema
     type: [ string, integer ]
     pattern: ^[0-9A-Za-z_]+( - 1)?$
     minimum: 0
+  len-or-limit:
+    # literal int or limit based on fixed-width type e.g. u8-min, u16-max, etc.
+    type: [ string, integer ]
+    pattern: ^[su](8|16|32|64)-(min|max)$
+    minimum: 0
 
 # Schema for specs
 title: Protocol
@@ -226,10 +231,10 @@ additionalProperties: False
                     type: string
                   min:
                     description: Min value for an integer attribute.
-                    type: integer
+                    $ref: '#/$defs/len-or-limit'
                   max:
                     description: Max value for an integer attribute.
-                    type: integer
+                    $ref: '#/$defs/len-or-limit'
                   min-len:
                     description: Min length for a binary attribute.
                     $ref: '#/$defs/len-or-define'
diff --git a/Documentation/netlink/genetlink.yaml b/Documentation/netlink/genetlink.yaml
index 4fd56e3b1553..0a4ae861d011 100644
--- a/Documentation/netlink/genetlink.yaml
+++ b/Documentation/netlink/genetlink.yaml
@@ -13,6 +13,11 @@ $schema: https://json-schema.org/draft-07/schema
     type: [ string, integer ]
     pattern: ^[0-9A-Za-z_]+( - 1)?$
     minimum: 0
+  len-or-limit:
+    # literal int or limit based on fixed-width type e.g. u8-min, u16-max, etc.
+    type: [ string, integer ]
+    pattern: ^[su](8|16|32|64)-(min|max)$
+    minimum: 0
 
 # Schema for specs
 title: Protocol
@@ -156,10 +161,10 @@ additionalProperties: False
                     type: string
                   min:
                     description: Min value for an integer attribute.
-                    type: integer
+                    $ref: '#/$defs/len-or-limit'
                   max:
                     description: Max value for an integer attribute.
-                    type: integer
+                    $ref: '#/$defs/len-or-limit'
                   min-len:
                     description: Min length for a binary attribute.
                     $ref: '#/$defs/len-or-define'
diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 9d008346dd50..552ba49a444c 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -20,6 +20,21 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
     return name.lower().replace('-', '_')
 
 
+def limit_to_number(name):
+    """
+    Turn a string limit like u32-max or s64-min into its numerical value
+    """
+    if name[0] == 'u' and name.endswith('-min'):
+        return 0
+    width = int(name[1:-4])
+    if name[0] == 's':
+        width -= 1
+    value = (1 << width) - 1
+    if name[0] == 's' and name.endswith('-min'):
+        value = -value - 1
+    return value
+
+
 class BaseNlLib:
     def get_family_id(self):
         return 'ys->family_id'
@@ -68,6 +83,14 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         self.enum_name = None
         delattr(self, "enum_name")
 
+    def get_limit(self, limit, default=None):
+        value = self.checks.get(limit, default)
+        if value is None:
+            return value
+        if not isinstance(value, int):
+            value = limit_to_number(value)
+        return value
+
     def resolve(self):
         if 'name-prefix' in self.attr:
             enum_name = f"{self.attr['name-prefix']}{self.name}"
@@ -273,12 +296,12 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
                 self.checks['max'] = high
 
         if 'min' in self.checks and 'max' in self.checks:
-            if self.checks['min'] > self.checks['max']:
-                raise Exception(f'Invalid limit for "{self.name}" min: {self.checks["min"]} max: {self.checks["max"]}')
+            if self.get_limit('min') > self.get_limit('max'):
+                raise Exception(f'Invalid limit for "{self.name}" min: {self.get_limit("min")} max: {self.get_limit("max")}')
             self.checks['range'] = True
 
-        low = min(self.checks.get('min', 0), self.checks.get('max', 0))
-        high = max(self.checks.get('min', 0), self.checks.get('max', 0))
+        low = min(self.get_limit('min', 0), self.get_limit('max', 0))
+        high = max(self.get_limit('min', 0), self.get_limit('max', 0))
         if low < 0 and self.type[0] == 'u':
             raise Exception(f'Invalid limit for "{self.name}" negative limit for unsigned type')
         if low < -32768 or high > 32767:
@@ -326,11 +349,11 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         elif 'full-range' in self.checks:
             return f"NLA_POLICY_FULL_RANGE({policy}, &{c_lower(self.enum_name)}_range)"
         elif 'range' in self.checks:
-            return f"NLA_POLICY_RANGE({policy}, {self.checks['min']}, {self.checks['max']})"
+            return f"NLA_POLICY_RANGE({policy}, {self.get_limit('min')}, {self.get_limit('max')})"
         elif 'min' in self.checks:
-            return f"NLA_POLICY_MIN({policy}, {self.checks['min']})"
+            return f"NLA_POLICY_MIN({policy}, {self.get_limit('min')})"
         elif 'max' in self.checks:
-            return f"NLA_POLICY_MAX({policy}, {self.checks['max']})"
+            return f"NLA_POLICY_MAX({policy}, {self.get_limit('max')})"
         return super()._attr_policy(policy)
 
     def _attr_typol(self):
@@ -382,7 +405,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
     def _attr_policy(self, policy):
         mem = '{ .type = ' + policy
         if 'max-len' in self.checks:
-            mem += ', .len = ' + str(self.checks['max-len'])
+            mem += ', .len = ' + str(self.get_limit('max-len'))
         mem += ', }'
         return mem
 
@@ -431,7 +454,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
     def _attr_policy(self, policy):
         mem = '{ '
         if len(self.checks) == 1 and 'min-len' in self.checks:
-            mem += '.len = ' + str(self.checks['min-len'])
+            mem += '.len = ' + str(self.get_limit('min-len'))
         elif len(self.checks) == 0:
             mem += '.type = NLA_BINARY'
         else:
@@ -1982,9 +2005,9 @@ _C_KW = {
             cw.block_start(line=f'struct netlink_range_validation{sign} {c_lower(attr.enum_name)}_range =')
             members = []
             if 'min' in attr.checks:
-                members.append(('min', attr.checks['min']))
+                members.append(('min', attr.get_limit('min')))
             if 'max' in attr.checks:
-                members.append(('max', attr.checks['max']))
+                members.append(('max', attr.get_limit('max')))
             cw.write_struct_init(members)
             cw.block_end(line=';')
             cw.nl()
-- 
2.41.0

Re: [PATCH net-next 0/3] tools: ynl-gen: support full range of min/max checks

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 18 Oct 2023 09:39:14 -0700 you wrote:
> YNL code gen currently supports only very simple range checks
> within the range of s16. Add support for full range of u64 / s64
> which is good to have, and will be even more important with uint / sint.
> 
> Jakub Kicinski (3):
>   tools: ynl-gen: track attribute use
>   tools: ynl-gen: support full range of min/max checks for integer
>     values
>   tools: ynl-gen: support limit names
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] tools: ynl-gen: track attribute use
    https://git.kernel.org/netdev/net-next/c/ee0a4cfcbdcc
  - [net-next,2/3] tools: ynl-gen: support full range of min/max checks for integer values
    https://git.kernel.org/netdev/net-next/c/668c1ac828fb
  - [net-next,3/3] tools: ynl-gen: support limit names
    https://git.kernel.org/netdev/net-next/c/f9bc3cbc20d0

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