[PATCH net-next 0/2] tools: ynl-gen: improvements for DPLL

[PATCH net-next 0/2] tools: ynl-gen: improvements for DPLL

[Jakub Kicinski]

A set of improvements needed to generate the kernel code for DPLL.

Jakub Kicinski (2):
  tools: ynl-gen: correct enum policies
  tools: ynl-gen: inherit policy in multi-attr

 tools/net/ynl/ynl-gen-c.py | 57 +++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 20 deletions(-)

-- 
2.40.1

[PATCH net-next 1/2] tools: ynl-gen: correct enum policies

[Jakub Kicinski]

Scalar range validation assumes enums start at 0.
Teach it to properly calculate the value range.

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

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 870f98d0e12c..54777d529f5e 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -300,8 +300,10 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
             return f"NLA_POLICY_MIN({policy}, {self.checks['min']})"
         elif 'enum' in self.attr:
             enum = self.family.consts[self.attr['enum']]
-            cnt = len(enum['entries'])
-            return f"NLA_POLICY_MAX({policy}, {cnt - 1})"
+            low, high = enum.value_range()
+            if low == 0:
+                return f"NLA_POLICY_MAX({policy}, {high})"
+            return f"NLA_POLICY_RANGE({policy}, {low}, {high})"
         return super()._attr_policy(policy)
 
     def _attr_typol(self):
@@ -676,6 +678,15 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
     def new_entry(self, entry, prev_entry, value_start):
         return EnumEntry(self, entry, prev_entry, value_start)
 
+    def value_range(self):
+        low = min([x.value for x in self.entries.values()])
+        high = max([x.value for x in self.entries.values()])
+
+        if high - low + 1 != len(self.entries):
+            raise Exception("Can't get value range for a noncontiguous enum")
+
+        return low, high
+
 
 class AttrSet(SpecAttrSet):
     def __init__(self, family, yaml):
-- 
2.40.1

RE: [PATCH net-next 1/2] tools: ynl-gen: correct enum policies

[Keller, Jacob E]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Monday, June 12, 2023 8:59 AM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; edumazet@google.com; pabeni@redhat.com;
> Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Jakub Kicinski
> <kuba@kernel.org>
> Subject: [PATCH net-next 1/2] tools: ynl-gen: correct enum policies
> 
> Scalar range validation assumes enums start at 0.
> Teach it to properly calculate the value range.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  tools/net/ynl/ynl-gen-c.py | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
> index 870f98d0e12c..54777d529f5e 100755
> --- a/tools/net/ynl/ynl-gen-c.py
> +++ b/tools/net/ynl/ynl-gen-c.py
> @@ -300,8 +300,10 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr,
> SpecOperation, SpecEnumSet, S
>              return f"NLA_POLICY_MIN({policy}, {self.checks['min']})"
>          elif 'enum' in self.attr:
>              enum = self.family.consts[self.attr['enum']]
> -            cnt = len(enum['entries'])
> -            return f"NLA_POLICY_MAX({policy}, {cnt - 1})"
> +            low, high = enum.value_range()
> +            if low == 0:
> +                return f"NLA_POLICY_MAX({policy}, {high})"
> +            return f"NLA_POLICY_RANGE({policy}, {low}, {high})"
>          return super()._attr_policy(policy)
> 
>      def _attr_typol(self):
> @@ -676,6 +678,15 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr,
> SpecOperation, SpecEnumSet, S
>      def new_entry(self, entry, prev_entry, value_start):
>          return EnumEntry(self, entry, prev_entry, value_start)
> 
> +    def value_range(self):
> +        low = min([x.value for x in self.entries.values()])
> +        high = max([x.value for x in self.entries.values()])
> +
> +        if high - low + 1 != len(self.entries):
> +            raise Exception("Can't get value range for a noncontiguous enum")
> +

Guessing we don't anticipate any non-contiguous enums so an exception here is fine

> +        return low, high
> +
> 
>  class AttrSet(SpecAttrSet):
>      def __init__(self, family, yaml):
> --
> 2.40.1
> 

[PATCH net-next 2/2] tools: ynl-gen: inherit policy in multi-attr

[Jakub Kicinski]

Instead of reimplementing policies in MutliAttr for every
underlying type forward the calls to the base type.
This will be needed for DPLL which uses a multi-attr nest,
and currently gets an invalid NLA_NEST policy generated.

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

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 54777d529f5e..71c5e79e877f 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -462,6 +462,11 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
 
 
 class TypeMultiAttr(Type):
+    def __init__(self, family, attr_set, attr, value, base_type):
+        super().__init__(family, attr_set, attr, value)
+
+        self.base_type = base_type
+
     def is_multi_val(self):
         return True
 
@@ -497,13 +502,11 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         else:
             raise Exception(f"Free of MultiAttr sub-type {self.attr['type']} not supported yet")
 
+    def _attr_policy(self, policy):
+        return self.base_type._attr_policy(policy)
+
     def _attr_typol(self):
-        if 'type' not in self.attr or self.attr['type'] == 'nest':
-            return f'.type = YNL_PT_NEST, .nest = &{self.nested_render_name}_nest, '
-        elif self.attr['type'] in scalars:
-            return f".type = YNL_PT_U{self.attr['type'][1:]}, "
-        else:
-            raise Exception(f"Sub-type {self.attr['type']} not supported yet")
+        return self.base_type._attr_typol()
 
     def _attr_get(self, ri, var):
         return f'n_{self.c_name}++;', None, None
@@ -717,29 +720,32 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
             self.c_name = ''
 
     def new_attr(self, elem, value):
-        if 'multi-attr' in elem and elem['multi-attr']:
-            return TypeMultiAttr(self.family, self, elem, value)
-        elif elem['type'] in scalars:
-            return TypeScalar(self.family, self, elem, value)
+        if elem['type'] in scalars:
+            t = TypeScalar(self.family, self, elem, value)
         elif elem['type'] == 'unused':
-            return TypeUnused(self.family, self, elem, value)
+            t = TypeUnused(self.family, self, elem, value)
         elif elem['type'] == 'pad':
-            return TypePad(self.family, self, elem, value)
+            t = TypePad(self.family, self, elem, value)
         elif elem['type'] == 'flag':
-            return TypeFlag(self.family, self, elem, value)
+            t = TypeFlag(self.family, self, elem, value)
         elif elem['type'] == 'string':
-            return TypeString(self.family, self, elem, value)
+            t = TypeString(self.family, self, elem, value)
         elif elem['type'] == 'binary':
-            return TypeBinary(self.family, self, elem, value)
+            t = TypeBinary(self.family, self, elem, value)
         elif elem['type'] == 'nest':
-            return TypeNest(self.family, self, elem, value)
+            t = TypeNest(self.family, self, elem, value)
         elif elem['type'] == 'array-nest':
-            return TypeArrayNest(self.family, self, elem, value)
+            t = TypeArrayNest(self.family, self, elem, value)
         elif elem['type'] == 'nest-type-value':
-            return TypeNestTypeValue(self.family, self, elem, value)
+            t = TypeNestTypeValue(self.family, self, elem, value)
         else:
             raise Exception(f"No typed class for type {elem['type']}")
 
+        if 'multi-attr' in elem and elem['multi-attr']:
+            t = TypeMultiAttr(self.family, self, elem, value, t)
+
+        return t
+
 
 class Operation(SpecOperation):
     def __init__(self, family, yaml, req_value, rsp_value):
-- 
2.40.1

RE: [PATCH net-next 2/2] tools: ynl-gen: inherit policy in multi-attr

[Keller, Jacob E]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Monday, June 12, 2023 8:59 AM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; edumazet@google.com; pabeni@redhat.com;
> Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Jakub Kicinski
> <kuba@kernel.org>
> Subject: [PATCH net-next 2/2] tools: ynl-gen: inherit policy in multi-attr
> 
> Instead of reimplementing policies in MutliAttr for every
> underlying type forward the calls to the base type.
> This will be needed for DPLL which uses a multi-attr nest,
> and currently gets an invalid NLA_NEST policy generated.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Jacob Keller <Jacob.e.keller@intel.com>

> ---
>  tools/net/ynl/ynl-gen-c.py | 42 ++++++++++++++++++++++----------------
>  1 file changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
> index 54777d529f5e..71c5e79e877f 100755
> --- a/tools/net/ynl/ynl-gen-c.py
> +++ b/tools/net/ynl/ynl-gen-c.py
> @@ -462,6 +462,11 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr,
> SpecOperation, SpecEnumSet, S
> 
> 
>  class TypeMultiAttr(Type):
> +    def __init__(self, family, attr_set, attr, value, base_type):
> +        super().__init__(family, attr_set, attr, value)
> +
> +        self.base_type = base_type
> +
>      def is_multi_val(self):
>          return True
> 
> @@ -497,13 +502,11 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr,
> SpecOperation, SpecEnumSet, S
>          else:
>              raise Exception(f"Free of MultiAttr sub-type {self.attr['type']} not
> supported yet")
> 
> +    def _attr_policy(self, policy):
> +        return self.base_type._attr_policy(policy)
> +
>      def _attr_typol(self):
> -        if 'type' not in self.attr or self.attr['type'] == 'nest':
> -            return f'.type = YNL_PT_NEST, .nest = &{self.nested_render_name}_nest, '
> -        elif self.attr['type'] in scalars:
> -            return f".type = YNL_PT_U{self.attr['type'][1:]}, "
> -        else:
> -            raise Exception(f"Sub-type {self.attr['type']} not supported yet")
> +        return self.base_type._attr_typol()
> 
>      def _attr_get(self, ri, var):
>          return f'n_{self.c_name}++;', None, None
> @@ -717,29 +720,32 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr,
> SpecOperation, SpecEnumSet, S
>              self.c_name = ''
> 
>      def new_attr(self, elem, value):
> -        if 'multi-attr' in elem and elem['multi-attr']:
> -            return TypeMultiAttr(self.family, self, elem, value)
> -        elif elem['type'] in scalars:
> -            return TypeScalar(self.family, self, elem, value)
> +        if elem['type'] in scalars:
> +            t = TypeScalar(self.family, self, elem, value)
>          elif elem['type'] == 'unused':
> -            return TypeUnused(self.family, self, elem, value)
> +            t = TypeUnused(self.family, self, elem, value)
>          elif elem['type'] == 'pad':
> -            return TypePad(self.family, self, elem, value)
> +            t = TypePad(self.family, self, elem, value)
>          elif elem['type'] == 'flag':
> -            return TypeFlag(self.family, self, elem, value)
> +            t = TypeFlag(self.family, self, elem, value)
>          elif elem['type'] == 'string':
> -            return TypeString(self.family, self, elem, value)
> +            t = TypeString(self.family, self, elem, value)
>          elif elem['type'] == 'binary':
> -            return TypeBinary(self.family, self, elem, value)
> +            t = TypeBinary(self.family, self, elem, value)
>          elif elem['type'] == 'nest':
> -            return TypeNest(self.family, self, elem, value)
> +            t = TypeNest(self.family, self, elem, value)
>          elif elem['type'] == 'array-nest':
> -            return TypeArrayNest(self.family, self, elem, value)
> +            t = TypeArrayNest(self.family, self, elem, value)
>          elif elem['type'] == 'nest-type-value':
> -            return TypeNestTypeValue(self.family, self, elem, value)
> +            t = TypeNestTypeValue(self.family, self, elem, value)
>          else:
>              raise Exception(f"No typed class for type {elem['type']}")
> 
> +        if 'multi-attr' in elem and elem['multi-attr']:
> +            t = TypeMultiAttr(self.family, self, elem, value, t)
> +
> +        return t
> +
> 
>  class Operation(SpecOperation):
>      def __init__(self, family, yaml, req_value, rsp_value):
> --
> 2.40.1
> 

Re: [PATCH net-next 0/2] tools: ynl-gen: improvements for DPLL

[patchwork-bot+netdevbpf]

Hello:

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

On Mon, 12 Jun 2023 08:59:18 -0700 you wrote:
> A set of improvements needed to generate the kernel code for DPLL.
> 
> Jakub Kicinski (2):
>   tools: ynl-gen: correct enum policies
>   tools: ynl-gen: inherit policy in multi-attr
> 
>  tools/net/ynl/ynl-gen-c.py | 57 +++++++++++++++++++++++++-------------
>  1 file changed, 37 insertions(+), 20 deletions(-)

Here is the summary with links:
  - [net-next,1/2] tools: ynl-gen: correct enum policies
    https://git.kernel.org/netdev/net-next/c/10c4d2a7b88d
  - [net-next,2/2] tools: ynl-gen: inherit policy in multi-attr
    https://git.kernel.org/netdev/net-next/c/be093a80dff0

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