diff mbox series

[net-next,3/7] ynl: support directional specs in ynl-gen-c.py

Message ID 20241113181023.2030098-4-sdf@fomichev.me (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ethtool: generate uapi header from the spec | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl fail Generated files up to date; build failed; build has 3 warnings/errors; GEN HAS DIFF 2 files changed, 75 insertions(+), 10 deletions(-);
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3 this patch: 3
netdev/build_tools success Errors and warnings before: 0 (+0) this patch: 0 (+0)
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 3 this patch: 3
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 134 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Stanislav Fomichev Nov. 13, 2024, 6:10 p.m. UTC 
The intent is to generate ethtool uapi headers. For now, some of the
things are hard-coded:
- <FAMILY>_MSG_{USER,KERNEL}_MAX
- the split between USER and KERNEL messages

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/net/ynl/ynl-gen-c.py | 116 +++++++++++++++++++++++++++----------
 1 file changed, 85 insertions(+), 31 deletions(-)

Comments

Jakub Kicinski Nov. 13, 2024, 8:12 p.m. UTC | #1 
On Wed, 13 Nov 2024 10:10:19 -0800 Stanislav Fomichev wrote:
> -    supported_models = ['unified']
> -    if args.mode in ['user', 'kernel']:
> -        supported_models += ['directional']
> -    if parsed.msg_id_model not in supported_models:
> -        print(f'Message enum-model {parsed.msg_id_model} not supported for {args.mode} generation')
> -        os.sys.exit(1)

Don't we still need to validate that it's one of the two options?
Stanislav Fomichev Nov. 13, 2024, 11:45 p.m. UTC | #2 
On 11/13, Jakub Kicinski wrote:
> On Wed, 13 Nov 2024 10:10:19 -0800 Stanislav Fomichev wrote:
> > -    supported_models = ['unified']
> > -    if args.mode in ['user', 'kernel']:
> > -        supported_models += ['directional']
> > -    if parsed.msg_id_model not in supported_models:
> > -        print(f'Message enum-model {parsed.msg_id_model} not supported for {args.mode} generation')
> > -        os.sys.exit(1)
> 
> Don't we still need to validate that it's one of the two options?

I removed it because I'm assuming only two modes exist (and we support
them both now). Are you suggesting it's better to future-proof it and
still keep the check in case we add some new modes in the future? (or
running against some rogue specs?)
Jakub Kicinski Nov. 14, 2024, 2:32 a.m. UTC | #3 
On Wed, 13 Nov 2024 15:45:21 -0800 Stanislav Fomichev wrote:
> On 11/13, Jakub Kicinski wrote:
> > On Wed, 13 Nov 2024 10:10:19 -0800 Stanislav Fomichev wrote:  
> > > -    supported_models = ['unified']
> > > -    if args.mode in ['user', 'kernel']:
> > > -        supported_models += ['directional']
> > > -    if parsed.msg_id_model not in supported_models:
> > > -        print(f'Message enum-model {parsed.msg_id_model} not supported for {args.mode} generation')
> > > -        os.sys.exit(1)  
> > 
> > Don't we still need to validate that it's one of the two options?  
> 
> I removed it because I'm assuming only two modes exist (and we support
> them both now). Are you suggesting it's better to future-proof it and
> still keep the check in case we add some new modes in the future? (or
> running against some rogue specs?)

TBH I don't remember how much precedent there is for C codegen
depending on jsonschema for spec input validation. My gut tells
me to do:

+    if family.msg_id_model == 'unified':
+        render_uapi_unified(family, cw, max_by_define, separate_ntf)
+    elif family.msg_id_model == 'directional':
+        render_uapi_directional(family, cw, max_by_define)
+    else:
+        raise ..

and then we can indeed drop the validation of the arg directly
diff mbox series

Patch

diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 0de918c7f18d..a7b7aea1159b 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -2416,6 +2416,87 @@  _C_KW = {
     cw.block_start(line=start_line)
 
 
+def render_uapi_unified(family, cw, max_by_define, separate_ntf):
+    max_name = c_upper(family.get('cmd-max-name', f"{family.op_prefix}MAX"))
+    cnt_name = c_upper(family.get('cmd-cnt-name', f"__{family.op_prefix}MAX"))
+    max_value = f"({cnt_name} - 1)"
+
+    uapi_enum_start(family, cw, family['operations'], 'enum-name')
+    val = 0
+    for op in family.msgs.values():
+        if separate_ntf and ('notify' in op or 'event' in op):
+            continue
+
+        suffix = ','
+        if op.value != val:
+            suffix = f" = {op.value},"
+            val = op.value
+        cw.p(op.enum_name + suffix)
+        val += 1
+    cw.nl()
+    cw.p(cnt_name + ('' if max_by_define else ','))
+    if not max_by_define:
+        cw.p(f"{max_name} = {max_value}")
+    cw.block_end(line=';')
+    if max_by_define:
+        cw.p(f"#define {max_name} {max_value}")
+    cw.nl()
+
+
+def render_uapi_directional(family, cw, max_by_define):
+    max_name = f"{family.op_prefix}USER_MAX"
+    cnt_name = f"__{family.op_prefix}USER_CNT"
+    max_value = f"({cnt_name} - 1)"
+
+    cw.block_start(line='enum')
+    cw.p(c_upper(f'{family.name}_MSG_USER_NONE = 0,'))
+    val = 0
+    for op in family.msgs.values():
+        if 'do' in op and 'event' not in op:
+            suffix = ','
+            if op.value and op.value != val:
+                suffix = f" = {op.value},"
+                val = op.value
+            cw.p(op.enum_name + suffix)
+            val += 1
+    cw.nl()
+    cw.p(cnt_name + ('' if max_by_define else ','))
+    if not max_by_define:
+        cw.p(f"{max_name} = {max_value}")
+    cw.block_end(line=';')
+    if max_by_define:
+        cw.p(f"#define {max_name} {max_value}")
+    cw.nl()
+
+    max_name = f"{family.op_prefix}KERNEL_MAX"
+    cnt_name = f"__{family.op_prefix}KERNEL_CNT"
+    max_value = f"({cnt_name} - 1)"
+
+    cw.block_start(line='enum')
+    cw.p(c_upper(f'{family.name}_MSG_KERNEL_NONE = 0,'))
+    val = 0
+    for op in family.msgs.values():
+        if ('do' in op and 'reply' in op['do']) or 'notify' in op or 'event' in op:
+            enum_name = op.enum_name
+            if 'event' not in op and 'notify' not in op:
+                enum_name = f'{enum_name}_REPLY'
+
+            suffix = ','
+            if op.value and op.value != val:
+                suffix = f" = {op.value},"
+                val = op.value
+            cw.p(enum_name + suffix)
+            val += 1
+    cw.nl()
+    cw.p(cnt_name + ('' if max_by_define else ','))
+    if not max_by_define:
+        cw.p(f"{max_name} = {max_value}")
+    cw.block_end(line=';')
+    if max_by_define:
+        cw.p(f"#define {max_name} {max_value}")
+    cw.nl()
+
+
 def render_uapi(family, cw):
     hdr_prot = f"_UAPI_LINUX_{c_upper(family.uapi_header_name)}_H"
     cw.p('#ifndef ' + hdr_prot)
@@ -2519,30 +2600,10 @@  _C_KW = {
     # Commands
     separate_ntf = 'async-prefix' in family['operations']
 
-    max_name = c_upper(family.get('cmd-max-name', f"{family.op_prefix}MAX"))
-    cnt_name = c_upper(family.get('cmd-cnt-name', f"__{family.op_prefix}MAX"))
-    max_value = f"({cnt_name} - 1)"
-
-    uapi_enum_start(family, cw, family['operations'], 'enum-name')
-    val = 0
-    for op in family.msgs.values():
-        if separate_ntf and ('notify' in op or 'event' in op):
-            continue
-
-        suffix = ','
-        if op.value != val:
-            suffix = f" = {op.value},"
-            val = op.value
-        cw.p(op.enum_name + suffix)
-        val += 1
-    cw.nl()
-    cw.p(cnt_name + ('' if max_by_define else ','))
-    if not max_by_define:
-        cw.p(f"{max_name} = {max_value}")
-    cw.block_end(line=';')
-    if max_by_define:
-        cw.p(f"#define {max_name} {max_value}")
-    cw.nl()
+    if family.msg_id_model == 'unified':
+        render_uapi_unified(family, cw, max_by_define, separate_ntf)
+    else:
+        render_uapi_directional(family, cw, max_by_define)
 
     if separate_ntf:
         uapi_enum_start(family, cw, family['operations'], enum_name='async-enum')
@@ -2666,13 +2727,6 @@  _C_KW = {
         os.sys.exit(1)
         return
 
-    supported_models = ['unified']
-    if args.mode in ['user', 'kernel']:
-        supported_models += ['directional']
-    if parsed.msg_id_model not in supported_models:
-        print(f'Message enum-model {parsed.msg_id_model} not supported for {args.mode} generation')
-        os.sys.exit(1)
-
     cw = CodeWriter(BaseNlLib(), args.out_file, overwrite=(not args.cmp_out))
 
     _, spec_kernel = find_kernel_root(args.spec)