Re: [PATCH net-next v2 11/12] tools: ynl-gen: don't init enum checks for classic netlink

[Donald Hunter <donald.hunter@gmail.com>]

Jakub Kicinski <kuba@kernel.org> writes:

> rt-link has a vlan-protocols enum with:
>
>    name: 8021q     value: 33024
>    name: 8021ad    value: 34984
>
> It's nice to have, since it converts the values to strings in Python.
> For C, however, the codegen is trying to use enums to generate strict
> policy checks. Parsing such sparse enums is not possible via policies.
>
> Since for classic netlink we don't support kernel codegen and policy
> generation - skip the auto-generation of checks from enums.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2:
>  - move the comment about the skip before the if
> v1: https://lore.kernel.org/4b8339b7-9dc6-4231-a60f-0c9f6296358a@intel.com
> ---
>  tools/net/ynl/pyynl/ynl_gen_c.py | 46 ++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
> index 2d185c7ea16c..eda9109243e2 100755
> --- a/tools/net/ynl/pyynl/ynl_gen_c.py
> +++ b/tools/net/ynl/pyynl/ynl_gen_c.py
> @@ -357,26 +357,10 @@ 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.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.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:
> -            self.checks['full-range'] = True
> +        # Classic families have some funny enums, don't bother
> +        # computing checks we only need them for policy

grammar nit: computing checks, since we only need ...

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