[PATCH net-next] tools: ynl-gen: use names of constants in generated limits

[PATCH net-next] tools: ynl-gen: use names of constants in generated limits

[Jakub Kicinski]

YNL specs can use string expressions for limits, like s32-min
or u16-max. We convert all of those into their numeric values
when generating the code, which isn't always helpful. Try to
retain the string representations in the output. Any sort of
calculations still need the integers.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/core/netdev-genl-gen.c |  4 ++--
 tools/net/ynl/ynl-gen-c.py | 36 +++++++++++++++++++++++-------------
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c
index b28424ae06d5..226a7e2c023c 100644
--- a/net/core/netdev-genl-gen.c
+++ b/net/core/netdev-genl-gen.c
@@ -14,12 +14,12 @@
 /* Integer value ranges */
 static const struct netlink_range_validation netdev_a_page_pool_id_range = {
 	.min	= 1ULL,
-	.max	= 4294967295ULL,
+	.max	= U32_MAX,
 };
 
 static const struct netlink_range_validation netdev_a_page_pool_ifindex_range = {
 	.min	= 1ULL,
-	.max	= 2147483647ULL,
+	.max	= S32_MAX,
 };
 
 /* Common nested types */
diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 9e8254aad578..d64cb2b49c44 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -80,11 +80,21 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         value = self.checks.get(limit, default)
         if value is None:
             return value
-        elif value in self.family.consts:
+        if isinstance(value, int):
+            return value
+        if value in self.family.consts:
+            raise Exception("Resolving family constants not implemented, yet")
+        return limit_to_number(value)
+
+    def get_limit_str(self, limit, default=None, suffix=''):
+        value = self.checks.get(limit, default)
+        if value is None:
+            return ''
+        if isinstance(value, int):
+            return str(value) + suffix
+        if value in self.family.consts:
             return c_upper(f"{self.family['name']}-{value}")
-        if not isinstance(value, int):
-            value = limit_to_number(value)
-        return value
+        return c_upper(value)
 
     def resolve(self):
         if 'name-prefix' in self.attr:
@@ -358,11 +368,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.get_limit('min')}, {self.get_limit('max')})"
+            return f"NLA_POLICY_RANGE({policy}, {self.get_limit_str('min')}, {self.get_limit_str('max')})"
         elif 'min' in self.checks:
-            return f"NLA_POLICY_MIN({policy}, {self.get_limit('min')})"
+            return f"NLA_POLICY_MIN({policy}, {self.get_limit_str('min')})"
         elif 'max' in self.checks:
-            return f"NLA_POLICY_MAX({policy}, {self.get_limit('max')})"
+            return f"NLA_POLICY_MAX({policy}, {self.get_limit_str('max')})"
         return super()._attr_policy(policy)
 
     def _attr_typol(self):
@@ -413,11 +423,11 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
 
     def _attr_policy(self, policy):
         if 'exact-len' in self.checks:
-            mem = 'NLA_POLICY_EXACT_LEN(' + str(self.get_limit('exact-len')) + ')'
+            mem = 'NLA_POLICY_EXACT_LEN(' + self.get_limit_str('exact-len') + ')'
         else:
             mem = '{ .type = ' + policy
             if 'max-len' in self.checks:
-                mem += ', .len = ' + str(self.get_limit('max-len'))
+                mem += ', .len = ' + self.get_limit_str('max-len')
             mem += ', }'
         return mem
 
@@ -476,9 +486,9 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
         if len(self.checks) == 0:
             mem = '{ .type = NLA_BINARY, }'
         elif 'exact-len' in self.checks:
-            mem = 'NLA_POLICY_EXACT_LEN(' + str(self.get_limit('exact-len')) + ')'
+            mem = 'NLA_POLICY_EXACT_LEN(' + self.get_limit_str('exact-len') + ')'
         elif 'min-len' in self.checks:
-            mem = '{ .len = ' + str(self.get_limit('min-len')) + ', }'
+            mem = '{ .len = ' + self.get_limit_str('min-len') + ', }'
 
         return mem
 
@@ -2166,9 +2176,9 @@ _C_KW = {
             cw.block_start(line=f'static const struct netlink_range_validation{sign} {c_lower(attr.enum_name)}_range =')
             members = []
             if 'min' in attr.checks:
-                members.append(('min', str(attr.get_limit('min')) + suffix))
+                members.append(('min', attr.get_limit_str('min', suffix=suffix)))
             if 'max' in attr.checks:
-                members.append(('max', str(attr.get_limit('max')) + suffix))
+                members.append(('max', attr.get_limit_str('max', suffix=suffix)))
             cw.write_struct_init(members)
             cw.block_end(line=';')
             cw.nl()
-- 
2.46.2

Re: [PATCH net-next] tools: ynl-gen: use names of constants in generated limits

[Joe Damato]

On Thu, Oct 10, 2024 at 08:12:48AM -0700, Jakub Kicinski wrote:
> YNL specs can use string expressions for limits, like s32-min
> or u16-max. We convert all of those into their numeric values
> when generating the code, which isn't always helpful. Try to
> retain the string representations in the output. Any sort of
> calculations still need the integers.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/core/netdev-genl-gen.c |  4 ++--
>  tools/net/ynl/ynl-gen-c.py | 36 +++++++++++++++++++++++-------------
>  2 files changed, 25 insertions(+), 15 deletions(-)

I rebased my per-NAPI changes on top of this commit and gave it a test
run and it generated this diff:

diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c
index 3692c8270c6b..21de7e10be16 100644
--- a/net/core/netdev-genl-gen.c
+++ b/net/core/netdev-genl-gen.c
@@ -23,7 +23,7 @@ static const struct netlink_range_validation netdev_a_page_pool_ifindex_range =
 };

 static const struct netlink_range_validation netdev_a_napi_defer_hard_irqs_range = {
-       .max    = 2147483647ULL,
+       .max    = S32_MAX,
 };

 /* Common nested types */
---

which is much nicer! I am not a python expert, but the code seems
reasonable to me and the generated output is a huge improvement (IMO).

Thanks for doing this.

Reviewed-by: Joe Damato <jdamato@fastly.com>

Re: [PATCH net-next] tools: ynl-gen: use names of constants in generated limits

[Jakub Kicinski]

On Thu, 10 Oct 2024 08:12:48 -0700 Jakub Kicinski wrote:
> YNL specs can use string expressions for limits, like s32-min
> or u16-max. We convert all of those into their numeric values
> when generating the code, which isn't always helpful. Try to
> retain the string representations in the output. Any sort of
> calculations still need the integers.

Missed CCing Donald, sorry! Link to full patch:
https://lore.kernel.org/all/20241010151248.2049755-1-kuba@kernel.org/