From ee0a4cfcbdcc6a7b2b35dba475e68187ebdafbf1 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 18 Oct 2023 09:39:15 -0700 Subject: [PATCH 1/3] tools: ynl-gen: track attribute use 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. Link: https://lore.kernel.org/r/20231018163917.2514503-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- 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 @@ class Type(SpecAttr): 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 @@ class Family(SpecFamily): 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 @@ class Family(SpecFamily): 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 From 668c1ac828fb546b81da6b36dae09d754bd0f59e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 18 Oct 2023 09:39:16 -0700 Subject: [PATCH 2/3] tools: ynl-gen: support full range of min/max checks for integer values 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. Link: https://lore.kernel.org/r/20231018163917.2514503-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- 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 @@ properties: 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 @@ properties: 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 @@ properties: 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 @@ class Type(SpecAttr): 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 @@ class TypeScalar(Type): 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 @@ class TypeScalar(Type): 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 @@ class CodeWriter: 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 @@ def policy_should_be_static(family): 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 @@ def main(): 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 */') From f9bc3cbc20d08c5c7b89a91e07ba46ab8042561e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 18 Oct 2023 09:39:17 -0700 Subject: [PATCH 3/3] tools: ynl-gen: support limit names Support the use of symbolic names like s8-min or u32-max in checks to make writing specs less painful. Link: https://lore.kernel.org/r/20231018163917.2514503-4-kuba@kernel.org Signed-off-by: Jakub Kicinski --- 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 @@ $defs: 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 @@ properties: 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 @@ $defs: 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 @@ properties: 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 @@ $defs: 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 @@ properties: 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 @@ def c_lower(name): 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 @@ class Type(SpecAttr): 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 @@ class TypeScalar(Type): 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 @@ class TypeScalar(Type): 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 @@ class TypeString(Type): 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 @@ class TypeBinary(Type): 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 @@ def print_kernel_policy_ranges(family, cw): 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()