Merge branch 'ynl-ethtool'

Jakub Kicinski says:

====================
tools: ynl: generate code for the ethtool family

And finally ethtool support. Thanks to Stan's work the ethtool family
spec is quite complete, so there is a lot of operations to support.

I chickened out of stats-get support, they require at the very least
type-value support on a u64 scalar. Type-value is an arrangement where
a u16 attribute is encoded directly in attribute type. Code gen can
support this if the inside is a nest, we just throw in an extra
field into that nest to carry the attr type. But a little more coding
is needed to for a scalar, because first we need to turn the scalar
into a struct with one member, then we can add the attr type.

Other than that ethtool required event support (notification which
does not share contents with any GET), but the previous series
already added that to the codegen.

I haven't tested all the ops here, and a few I tried seem to work.
====================
Acked-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

Documentation/netlink/genetlink-c.yaml

@@ -195,6 +195,10 @@ properties:
                     description: Max length for a string or a binary attribute.
                     $ref: '#/$defs/len-or-define'
               sub-type: *attr-type
+              # Start genetlink-c
+              name-prefix:
+                type: string
+              # End genetlink-c
 
       # Make sure name-prefix does not appear in subsets (subsets inherit naming)
       dependencies:

Documentation/netlink/genetlink-legacy.yaml

@@ -226,6 +226,10 @@ properties:
                     description: Max length for a string or a binary attribute.
                     $ref: '#/$defs/len-or-define'
               sub-type: *attr-type
+              # Start genetlink-c
+              name-prefix:
+                type: string
+              # End genetlink-c
               # Start genetlink-legacy
               struct:
                 description: Name of the struct type used for the attribute.

Documentation/netlink/specs/ethtool.yaml

@@ -9,8 +9,13 @@ doc: Partial family for Ethtool Netlink.
 definitions:
   -
     name: udp-tunnel-type
+    enum-name:
     type: enum
     entries: [ vxlan, geneve, vxlan-gpe ]
+  -
+    name: stringset
+    type: enum
+    entries: []
 
 attribute-sets:
   -
@@ -497,7 +502,7 @@ attribute-sets:
     attributes:
       -
         name: pad
-        type: u32
+        type: pad
       -
         name: tx-frames
         type: u64
@@ -577,7 +582,7 @@ attribute-sets:
         name: phc-index
         type: u32
   -
-    name: cable-test-ntf-nest-result
+    name: cable-result
     attributes:
       -
         name: pair
@@ -586,7 +591,7 @@ attribute-sets:
         name: code
         type: u8
   -
-    name: cable-test-ntf-nest-fault-length
+    name: cable-fault-length
     attributes:
       -
         name: pair
@@ -595,18 +600,25 @@ attribute-sets:
         name: cm
         type: u32
   -
-    name: cable-test-ntf-nest
+    name: cable-nest
     attributes:
       -
         name: result
         type: nest
-        nested-attributes: cable-test-ntf-nest-result
+        nested-attributes: cable-result
       -
         name: fault-length
         type: nest
-        nested-attributes: cable-test-ntf-nest-fault-length
+        nested-attributes: cable-fault-length
   -
     name: cable-test
+    attributes:
+      -
+        name: header
+        type: nest
+        nested-attributes: header
+  -
+    name: cable-test-ntf
     attributes:
       -
         name: header
@@ -618,7 +630,7 @@ attribute-sets:
       -
         name: nest
         type: nest
-        nested-attributes: cable-test-ntf-nest
+        nested-attributes: cable-nest
   -
     name: cable-test-tdr-cfg
     attributes:
@@ -632,8 +644,22 @@ attribute-sets:
         name: step
         type: u32
       -
-        name: pari
+        name: pair
         type: u8
+  -
+    name: cable-test-tdr-ntf
+    attributes:
+      -
+        name: header
+        type: nest
+        nested-attributes: header
+      -
+        name: status
+        type: u8
+      -
+        name: nest
+        type: nest
+        nested-attributes: cable-nest
   -
     name: cable-test-tdr
     attributes:
@@ -646,7 +672,7 @@ attribute-sets:
         type: nest
         nested-attributes: cable-test-tdr-cfg
   -
-    name: tunnel-info-udp-entry
+    name: tunnel-udp-entry
     attributes:
       -
         name: port
@@ -657,7 +683,7 @@ attribute-sets:
         type: u32
         enum: udp-tunnel-type
   -
-    name: tunnel-info-udp-table
+    name: tunnel-udp-table
     attributes:
       -
         name: size
@@ -667,9 +693,17 @@ attribute-sets:
         type: nest
         nested-attributes: bitset
       -
-        name: udp-ports
+        name: entry
         type: nest
-        nested-attributes: tunnel-info-udp-entry
+        multi-attr: true
+        nested-attributes: tunnel-udp-entry
+  -
+    name: tunnel-udp
+    attributes:
+      -
+        name: table
+        type: nest
+        nested-attributes: tunnel-udp-table
   -
     name: tunnel-info
     attributes:
@@ -680,13 +714,13 @@ attribute-sets:
       -
         name: udp-ports
         type: nest
-        nested-attributes: tunnel-info-udp-table
+        nested-attributes: tunnel-udp
   -
     name: fec-stat
     attributes:
       -
         name: pad
-        type: u8
+        type: pad
       -
         name: corrected
         type: binary
@@ -750,7 +784,7 @@ attribute-sets:
     attributes:
       -
         name: pad
-        type: u32
+        type: pad
       -
         name: id
         type: u32
@@ -759,16 +793,29 @@ attribute-sets:
         type: u32
       -
         name: stat
-        type: nest
-        nested-attributes: u64
+        type: u64
+        type-value: [ id ]
       -
         name: hist-rx
         type: nest
-        nested-attributes: u64
+        nested-attributes: stats-grp-hist
       -
         name: hist-tx
         type: nest
-        nested-attributes: u64
+        nested-attributes: stats-grp-hist
+      -
+        name: hist-bkt-low
+        type: u32
+      -
+        name: hist-bkt-hi
+        type: u32
+      -
+        name: hist-val
+        type: u64
+  -
+    name: stats-grp-hist
+    subset-of: stats-grp
+    attributes:
       -
         name: hist-bkt-low
         type: u32
@@ -783,7 +830,7 @@ attribute-sets:
     attributes:
       -
         name: pad
-        type: u32
+        type: pad
       -
         name: header
         type: nest
@@ -836,12 +883,15 @@ attribute-sets:
       -
         name: admin-state
         type: u32
+        name-prefix: ethtool-a-podl-pse-
       -
         name: admin-control
         type: u32
+        name-prefix: ethtool-a-podl-pse-
       -
         name: pw-d-status
         type: u32
+        name-prefix: ethtool-a-podl-pse-
   -
     name: rss
     attributes:
@@ -895,6 +945,7 @@ attribute-sets:
 
 operations:
   enum-model: directional
+  name-prefix: ethtool-msg-
   list:
     -
       name: strset-get
@@ -1348,10 +1399,16 @@ operations:
         request:
           attributes:
             - header
-        reply:
-          attributes:
-            - header
-            - cable-test-ntf-nest
+    -
+      name: cable-test-ntf
+      doc: Cable test notification.
+
+      attribute-set: cable-test-ntf
+
+      event:
+        attributes:
+          - header
+          - status
     -
       name: cable-test-tdr-act
       doc: Cable test TDR.
@@ -1362,10 +1419,17 @@ operations:
         request:
           attributes:
             - header
-        reply:
-          attributes:
-            - header
-            - cable-test-tdr-cfg
+    -
+      name: cable-test-tdr-ntf
+      doc: Cable test TDR notification.
+
+      attribute-set: cable-test-tdr-ntf
+
+      event:
+        attributes:
+          - header
+          - status
+          - nest
     -
       name: tunnel-info-get
       doc: Get tsinfo params.

tools/net/ynl/generated/Makefile

@@ -7,9 +7,12 @@ ifeq ("$(DEBUG)","1")
   CFLAGS += -g -fsanitize=address -fsanitize=leak -static-libasan
 endif
 
+YNL_GEN_ARG_ethtool:=--user-header linux/ethtool_netlink.h \
+	--exclude-op stats-get
+
 TOOL:=../ynl-gen-c.py
 
-GENS:=devlink handshake fou netdev
+GENS:=ethtool devlink handshake fou netdev
 SRCS=$(patsubst %,%-user.c,${GENS})
 HDRS=$(patsubst %,%-user.h,${GENS})
 OBJS=$(patsubst %,%-user.o,${GENS})
@@ -22,11 +25,11 @@ protos.a: $(OBJS)
 
 %-user.h: ../../../../Documentation/netlink/specs/%.yaml $(TOOL)
 	@echo -e "\tGEN $@"
-	@$(TOOL) --mode user --header --spec $< > $@
+	@$(TOOL) --mode user --header --spec $< $(YNL_GEN_ARG_$*) > $@
 
 %-user.c: ../../../../Documentation/netlink/specs/%.yaml $(TOOL)
 	@echo -e "\tGEN $@"
-	@$(TOOL) --mode user --source --spec $< > $@
+	@$(TOOL) --mode user --source --spec $< $(YNL_GEN_ARG_$*) > $@
 
 %-user.o: %-user.c %-user.h
 	@echo -e "\tCC $@"

tools/net/ynl/lib/nlspec.py

@@ -334,7 +334,7 @@ class SpecFamily(SpecElement):
         consts     dict of all constants/enums
         fixed_header  string, optional name of family default fixed header struct
     """
-    def __init__(self, spec_path, schema_path=None):
+    def __init__(self, spec_path, schema_path=None, exclude_ops=None):
         with open(spec_path, "r") as stream:
             prefix = '# SPDX-License-Identifier: '
             first = stream.readline().strip()
@@ -349,6 +349,8 @@ class SpecFamily(SpecElement):
 
         super().__init__(self, spec)
 
+        self._exclude_ops = exclude_ops if exclude_ops else []
+
         self.proto = self.yaml.get('protocol', 'genetlink')
         self.msg_id_model = self.yaml['operations'].get('enum-model', 'unified')
 
@@ -449,7 +451,13 @@ class SpecFamily(SpecElement):
                 req_val = None
             if rsp_val == rsp_val_next:
                 rsp_val = None
-            op = self.new_operation(elem, req_val, rsp_val)
+
+            skip = False
+            for exclude in self._exclude_ops:
+                skip |= bool(exclude.match(elem['name']))
+            if not skip:
+                op = self.new_operation(elem, req_val, rsp_val)
+
             req_val = req_val_next
             rsp_val = rsp_val_next
 

tools/net/ynl/samples/.gitignore

+ethtool
 devlink
 netdev

tools/net/ynl/samples/ethtool.c

+// SPDX-License-Identifier: GPL-2.0
+#include <stdio.h>
+#include <string.h>
+
+#include <ynl.h>
+
+#include <net/if.h>
+
+#include "ethtool-user.h"
+
+int main(int argc, char **argv)
+{
+	struct ethtool_channels_get_req_dump creq = {};
+	struct ethtool_rings_get_req_dump rreq = {};
+	struct ethtool_channels_get_list *channels;
+	struct ethtool_rings_get_list *rings;
+	struct ynl_sock *ys;
+
+	ys = ynl_sock_create(&ynl_ethtool_family, NULL);
+	if (!ys)
+		return 1;
+
+	creq._present.header = 1; /* ethtool needs an empty nest, sigh */
+	channels = ethtool_channels_get_dump(ys, &creq);
+	if (!channels)
+		goto err_close;
+
+	printf("Channels:\n");
+	ynl_dump_foreach(channels, dev) {
+		printf("  %8s: ", dev->header.dev_name);
+		if (dev->_present.rx_count)
+			printf("rx %d ", dev->rx_count);
+		if (dev->_present.tx_count)
+			printf("tx %d ", dev->tx_count);
+		if (dev->_present.combined_count)
+			printf("combined %d ", dev->combined_count);
+		printf("\n");
+	}
+	ethtool_channels_get_list_free(channels);
+
+	rreq._present.header = 1; /* ethtool needs an empty nest.. */
+	rings = ethtool_rings_get_dump(ys, &rreq);
+	if (!rings)
+		goto err_close;
+
+	printf("Rings:\n");
+	ynl_dump_foreach(rings, dev) {
+		printf("  %8s: ", dev->header.dev_name);
+		if (dev->_present.rx)
+			printf("rx %d ", dev->rx);
+		if (dev->_present.tx)
+			printf("tx %d ", dev->tx);
+		printf("\n");
+	}
+	ethtool_rings_get_list_free(rings);
+
+	ynl_sock_destroy(ys);
+
+	return 0;
+
+err_close:
+	fprintf(stderr, "YNL (%d): %s\n", ys->err.code, ys->err.msg);
+	ynl_sock_destroy(ys);
+	return 2;
+}

tools/net/ynl/ynl-gen-c.py

@@ -4,6 +4,7 @@
 import argparse
 import collections
 import os
+import re
 import yaml
 
 from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, SpecEnumEntry
@@ -48,6 +49,11 @@ class Type(SpecAttr):
             else:
                 self.nested_render_name = f"{family.name}_{c_lower(self.nested_attrs)}"
 
+            if self.nested_attrs in self.family.consts:
+                self.nested_struct_type = 'struct ' + self.nested_render_name + '_'
+            else:
+                self.nested_struct_type = 'struct ' + self.nested_render_name
+
         self.c_name = c_lower(self.name)
         if self.c_name in _C_KW:
             self.c_name += '_'
@@ -57,8 +63,11 @@ class Type(SpecAttr):
         delattr(self, "enum_name")
 
     def resolve(self):
-        self.enum_name = f"{self.attr_set.name_prefix}{self.name}"
-        self.enum_name = c_upper(self.enum_name)
+        if 'name-prefix' in self.attr:
+            enum_name = f"{self.attr['name-prefix']}{self.name}"
+        else:
+            enum_name = f"{self.attr_set.name_prefix}{self.name}"
+        self.enum_name = c_upper(enum_name)
 
     def is_multi_val(self):
         return None
@@ -264,7 +273,8 @@ class TypeScalar(Type):
         else:
             self.is_bitfield = False
 
-        if 'enum' in self.attr and not self.is_bitfield:
+        maybe_enum = not self.is_bitfield and 'enum' in self.attr
+        if maybe_enum and self.family.consts[self.attr['enum']].enum_name:
             self.type_name = f"enum {self.family.name}_{c_lower(self.attr['enum'])}"
         else:
             self.type_name = '__' + self.type
@@ -420,7 +430,7 @@ class TypeBinary(Type):
 
 class TypeNest(Type):
     def _complex_member_type(self, ri):
-        return f"struct {self.nested_render_name}"
+        return self.nested_struct_type
 
     def free(self, ri, var, ref):
         ri.cw.p(f'{self.nested_render_name}_free(&{var}->{ref}{self.c_name});')
@@ -465,7 +475,7 @@ class TypeMultiAttr(Type):
 
     def _complex_member_type(self, ri):
         if 'type' not in self.attr or self.attr['type'] == 'nest':
-            return f"struct {self.nested_render_name}"
+            return self.nested_struct_type
         elif self.attr['type'] in scalars:
             scalar_pfx = '__' if ri.ku_space == 'user' else ''
             return scalar_pfx + self.attr['type']
@@ -525,7 +535,7 @@ class TypeArrayNest(Type):
 
     def _complex_member_type(self, ri):
         if 'sub-type' not in self.attr or self.attr['sub-type'] == 'nest':
-            return f"struct {self.nested_render_name}"
+            return self.nested_struct_type
         elif self.attr['sub-type'] in scalars:
             scalar_pfx = '__' if ri.ku_space == 'user' else ''
             return scalar_pfx + self.attr['sub-type']
@@ -545,7 +555,7 @@ class TypeArrayNest(Type):
 
 class TypeNestTypeValue(Type):
     def _complex_member_type(self, ri):
-        return f"struct {self.nested_render_name}"
+        return self.nested_struct_type
 
     def _attr_typol(self):
         return f'.type = YNL_PT_NEST, .nest = &{self.nested_render_name}_nest, '
@@ -588,6 +598,8 @@ class Struct:
         else:
             self.render_name = f"{family.name}_{c_lower(space_name)}"
         self.struct_name = 'struct ' + self.render_name
+        if self.nested and space_name in family.consts:
+            self.struct_name += '_'
         self.ptr_name = self.struct_name + ' *'
 
         self.request = False
@@ -648,7 +660,14 @@ class EnumEntry(SpecEnumEntry):
 class EnumSet(SpecEnumSet):
     def __init__(self, family, yaml):
         self.render_name = c_lower(family.name + '-' + yaml['name'])
-        self.enum_name = 'enum ' + self.render_name
+
+        if 'enum-name' in yaml:
+            if yaml['enum-name']:
+                self.enum_name = 'enum ' + c_lower(yaml['enum-name'])
+            else:
+                self.enum_name = None
+        else:
+            self.enum_name = 'enum ' + self.render_name
 
         self.value_pfx = yaml.get('name-prefix', f"{family.name}-{yaml['name']}-")
 
@@ -739,7 +758,7 @@ class Operation(SpecOperation):
 
 
 class Family(SpecFamily):
-    def __init__(self, file_name):
+    def __init__(self, file_name, exclude_ops):
         # Added by resolve:
         self.c_name = None
         delattr(self, "c_name")
@@ -754,7 +773,7 @@ class Family(SpecFamily):
         self.hooks = None
         delattr(self, "hooks")
 
-        super().__init__(file_name)
+        super().__init__(file_name, exclude_ops=exclude_ops)
 
         self.fam_key = c_upper(self.yaml.get('c-family-name', self.yaml["name"] + '_FAMILY_NAME'))
         self.ver_key = c_upper(self.yaml.get('c-version-name', self.yaml["name"] + '_FAMILY_VERSION'))
@@ -982,10 +1001,13 @@ class RenderInfo:
         if not self.attr_set:
             self.attr_set = op['attribute-set']
 
+        self.type_name_conflict = False
         if op:
             self.type_name = c_lower(op.name)
         else:
             self.type_name = c_lower(attr_set)
+            if attr_set in family.consts:
+                self.type_name_conflict = True
 
         self.cw = cw
 
@@ -1622,12 +1644,17 @@ def print_alloc_wrapper(ri, direction):
 
 def print_free_prototype(ri, direction, suffix=';'):
     name = op_prefix(ri, direction)
+    struct_name = name
+    if ri.type_name_conflict:
+        struct_name += '_'
     arg = free_arg_name(direction)
-    ri.cw.write_func_prot('void', f"{name}_free", [f"struct {name} *{arg}"], suffix=suffix)
+    ri.cw.write_func_prot('void', f"{name}_free", [f"struct {struct_name} *{arg}"], suffix=suffix)
 
 
 def _print_type(ri, direction, struct):
     suffix = f'_{ri.type_name}{direction_to_suffix[direction]}'
+    if not direction and ri.type_name_conflict:
+        suffix += '_'
 
     if ri.op_mode == 'dump':
         suffix += '_dump'
@@ -2241,6 +2268,7 @@ def main():
     parser.add_argument('--header', dest='header', action='store_true', default=None)
     parser.add_argument('--source', dest='header', action='store_false')
     parser.add_argument('--user-header', nargs='+', default=[])
+    parser.add_argument('--exclude-op', action='append', default=[])
     parser.add_argument('-o', dest='out_file', type=str)
     args = parser.parse_args()
 
@@ -2249,8 +2277,10 @@ def main():
     if args.header is None:
         parser.error("--header or --source is required")
 
+    exclude_ops = [re.compile(expr) for expr in args.exclude_op]
+
     try:
-        parsed = Family(args.spec)
+        parsed = Family(args.spec, exclude_ops)
         if parsed.license != '((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)':
             print('Spec license:', parsed.license)
             print('License must be: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)')
@@ -2277,6 +2307,11 @@ def main():
     cw.p("/* Do not edit directly, auto-generated from: */")
     cw.p(f"/*\t{spec_kernel} */")
     cw.p(f"/* YNL-GEN {args.mode} {'header' if args.header else 'source'} */")
+    if args.exclude_op or args.user_header:
+        line = ''
+        line += ' --user-header '.join([''] + args.user_header)
+        line += ' --exclude-op '.join([''] + args.exclude_op)
+        cw.p(f'/* YNL-ARG{line} */')
     cw.nl()
 
     if args.mode == 'uapi':

tools/net/ynl/ynl-regen.sh

@@ -19,6 +19,7 @@ for f in $files; do
     # params:     0       1      2     3
     #         $YAML YNL-GEN kernel $mode
     params=( $(git grep -B1 -h '/\* YNL-GEN' $f | sed 's@/\*\(.*\)\*/@\1@') )
+    args=$(sed -n 's@/\* YNL-ARG \(.*\) \*/@\1@p' $f)
 
     if [ $f -nt ${params[0]} -a -z "$force" ]; then
 	echo -e "\tSKIP $f"
@@ -26,5 +27,6 @@ for f in $files; do
     fi
 
     echo -e "\tGEN ${params[2]}\t$f"
-    $TOOL --mode ${params[2]} --${params[3]} --spec $KDIR/${params[0]} -o $f
+    $TOOL --mode ${params[2]} --${params[3]} --spec $KDIR/${params[0]} \
+	  $args -o $f
 done