diff mbox series

[net-next,v2,5/6] tools: ynl: Add fixed-header support to ynl

Message ID 20230319193803.97453-6-donald.hunter@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ynl: add support for user headers and struct attrs | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
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: 18 this patch: 18
netdev/cc_maintainers warning 2 maintainers not CCed: corbet@lwn.net linux-doc@vger.kernel.org
netdev/build_clang success Errors and warnings before: 18 this patch: 18
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: 18 this patch: 18
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 147 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Donald Hunter March 19, 2023, 7:38 p.m. UTC
Add support for netlink families that add an optional fixed header structure
after the genetlink header and before any attributes. The fixed-header can be
specified on a per op basis, or once for all operations.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
---
 Documentation/netlink/genetlink-legacy.yaml | 10 +++++++
 tools/net/ynl/lib/nlspec.py                 | 30 ++++++++++++---------
 tools/net/ynl/lib/ynl.py                    | 23 +++++++++++++---
 3 files changed, 46 insertions(+), 17 deletions(-)

Comments

Jakub Kicinski March 22, 2023, 5:34 a.m. UTC | #1
On Sun, 19 Mar 2023 19:38:02 +0000 Donald Hunter wrote:
> -    def __init__(self, family, yaml, req_value, rsp_value):
> +    def __init__(self, family, yaml, req_value, rsp_value, default_fixed_header):
>          super().__init__(family, yaml)
>  
>          self.value = req_value if req_value == rsp_value else None
> @@ -278,6 +279,7 @@ class SpecOperation(SpecElement):
>          self.is_call = 'do' in yaml or 'dump' in yaml
>          self.is_async = 'notify' in yaml or 'event' in yaml
>          self.is_resv = not self.is_async and not self.is_call
> +        self.fixed_header = self.yaml.get('fixed-header', default_fixed_header)
>  
>          # Added by resolve:
>          self.attr_set = None
> @@ -384,24 +386,26 @@ class SpecFamily(SpecElement):
>      def new_struct(self, elem):
>          return SpecStruct(self, elem)
>  
> -    def new_operation(self, elem, req_val, rsp_val):
> -        return SpecOperation(self, elem, req_val, rsp_val)
> +    def new_operation(self, elem, req_val, rsp_val, default_fixed_header):
> +        return SpecOperation(self, elem, req_val, rsp_val, default_fixed_header)
>  
>      def add_unresolved(self, elem):
>          self._resolution_list.append(elem)
>  
>      def _dictify_ops_unified(self):
> +        default_fixed_header = self.yaml['operations'].get('fixed-header')
>          val = 1
>          for elem in self.yaml['operations']['list']:
>              if 'value' in elem:
>                  val = elem['value']
>  
> -            op = self.new_operation(elem, val, val)
> +            op = self.new_operation(elem, val, val, default_fixed_header)
>              val += 1
>  
>              self.msgs[op.name] = op
>  
>      def _dictify_ops_directional(self):
> +        default_fixed_header = self.yaml['operations'].get('fixed-header')
>          req_val = rsp_val = 1
>          for elem in self.yaml['operations']['list']:
>              if 'notify' in elem:
> @@ -426,7 +430,7 @@ class SpecFamily(SpecElement):
>              else:
>                  raise Exception("Can't parse directional ops")
>  
> -            op = self.new_operation(elem, req_val, rsp_val)
> +            op = self.new_operation(elem, req_val, rsp_val, default_fixed_header)

Can we record the "family-default" fixed header in the family and read
from there rather than passing the arg around?

Also - doc - to be clear - by documentation I mean in the right places
under Documentation/user-api/netlink/
Donald Hunter March 22, 2023, 11:54 a.m. UTC | #2
Jakub Kicinski <kuba@kernel.org> writes:

> On Sun, 19 Mar 2023 19:38:02 +0000 Donald Hunter wrote:
>>
>>      def _dictify_ops_directional(self):
>> +        default_fixed_header = self.yaml['operations'].get('fixed-header')
>>          req_val = rsp_val = 1
>>          for elem in self.yaml['operations']['list']:
>>              if 'notify' in elem:
>> @@ -426,7 +430,7 @@ class SpecFamily(SpecElement):
>>              else:
>>                  raise Exception("Can't parse directional ops")
>>  
>> -            op = self.new_operation(elem, req_val, rsp_val)
>> +            op = self.new_operation(elem, req_val, rsp_val, default_fixed_header)
>
> Can we record the "family-default" fixed header in the family and read
> from there rather than passing the arg around?

Yes, will do.

> Also - doc - to be clear - by documentation I mean in the right places
> under Documentation/user-api/netlink/

Ack.
diff mbox series

Patch

diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index 654f40b26beb..30a8051c1d8f 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -261,6 +261,13 @@  properties:
       async-enum:
         description: Name for the enum type with notifications/events.
         type: string
+      # Start genetlink-legacy
+      fixed-header: &fixed-header
+        description: |
+          Name of the structure defininig the optional fixed-length protocol header. This header is
+          placed in a message after the netlink and genetlink headers and before any attributes.
+        type: string
+      # End genetlink-legacy
       list:
         description: List of commands
         type: array
@@ -293,6 +300,9 @@  properties:
               type: array
               items:
                 enum: [ strict, dump ]
+            # Start genetlink-legacy
+            fixed-header: *fixed-header
+            # End genetlink-legacy
             do: &subop-type
               description: Main command handler.
               type: object
diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py
index 5ac2dfd415c5..69ee9f940e0e 100644
--- a/tools/net/ynl/lib/nlspec.py
+++ b/tools/net/ynl/lib/nlspec.py
@@ -257,18 +257,19 @@  class SpecOperation(SpecElement):
     Information about a single Netlink operation.
 
     Attributes:
-        value       numerical ID when serialized, None if req/rsp values differ
+        value           numerical ID when serialized, None if req/rsp values differ
 
-        req_value   numerical ID when serialized, user -> kernel
-        rsp_value   numerical ID when serialized, user <- kernel
-        is_call     bool, whether the operation is a call
-        is_async    bool, whether the operation is a notification
-        is_resv     bool, whether the operation does not exist (it's just a reserved ID)
-        attr_set    attribute set name
+        req_value       numerical ID when serialized, user -> kernel
+        rsp_value       numerical ID when serialized, user <- kernel
+        is_call         bool, whether the operation is a call
+        is_async        bool, whether the operation is a notification
+        is_resv         bool, whether the operation does not exist (it's just a reserved ID)
+        attr_set        attribute set name
+        fixed_header    string, optional fixed header structure name
 
-        yaml        raw spec as loaded from the spec file
+        yaml            raw spec as loaded from the spec file
     """
-    def __init__(self, family, yaml, req_value, rsp_value):
+    def __init__(self, family, yaml, req_value, rsp_value, default_fixed_header):
         super().__init__(family, yaml)
 
         self.value = req_value if req_value == rsp_value else None
@@ -278,6 +279,7 @@  class SpecOperation(SpecElement):
         self.is_call = 'do' in yaml or 'dump' in yaml
         self.is_async = 'notify' in yaml or 'event' in yaml
         self.is_resv = not self.is_async and not self.is_call
+        self.fixed_header = self.yaml.get('fixed-header', default_fixed_header)
 
         # Added by resolve:
         self.attr_set = None
@@ -384,24 +386,26 @@  class SpecFamily(SpecElement):
     def new_struct(self, elem):
         return SpecStruct(self, elem)
 
-    def new_operation(self, elem, req_val, rsp_val):
-        return SpecOperation(self, elem, req_val, rsp_val)
+    def new_operation(self, elem, req_val, rsp_val, default_fixed_header):
+        return SpecOperation(self, elem, req_val, rsp_val, default_fixed_header)
 
     def add_unresolved(self, elem):
         self._resolution_list.append(elem)
 
     def _dictify_ops_unified(self):
+        default_fixed_header = self.yaml['operations'].get('fixed-header')
         val = 1
         for elem in self.yaml['operations']['list']:
             if 'value' in elem:
                 val = elem['value']
 
-            op = self.new_operation(elem, val, val)
+            op = self.new_operation(elem, val, val, default_fixed_header)
             val += 1
 
             self.msgs[op.name] = op
 
     def _dictify_ops_directional(self):
+        default_fixed_header = self.yaml['operations'].get('fixed-header')
         req_val = rsp_val = 1
         for elem in self.yaml['operations']['list']:
             if 'notify' in elem:
@@ -426,7 +430,7 @@  class SpecFamily(SpecElement):
             else:
                 raise Exception("Can't parse directional ops")
 
-            op = self.new_operation(elem, req_val, rsp_val)
+            op = self.new_operation(elem, req_val, rsp_val, default_fixed_header)
             req_val = req_val_next
             rsp_val = rsp_val_next
 
diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
index 24f8af3c2b38..736a637ecb2b 100644
--- a/tools/net/ynl/lib/ynl.py
+++ b/tools/net/ynl/lib/ynl.py
@@ -277,14 +277,22 @@  def _genl_load_families():
 
 
 class GenlMsg:
-    def __init__(self, nl_msg):
+    def __init__(self, nl_msg, fixed_header_members = []):
         self.nl = nl_msg
 
         self.hdr = nl_msg.raw[0:4]
-        self.raw = nl_msg.raw[4:]
+        offset = 4
 
         self.genl_cmd, self.genl_version, _ = struct.unpack("BBH", self.hdr)
 
+        self.fixed_header_attrs = dict()
+        for m in fixed_header_members:
+            format, size = NlAttr.type_formats[m.type]
+            decoded = struct.unpack_from(format, nl_msg.raw, offset)
+            offset += size
+            self.fixed_header_attrs[m.name] = decoded[0]
+
+        self.raw = nl_msg.raw[offset:]
         self.raw_attrs = NlAttrs(self.raw)
 
     def __repr__(self):
@@ -496,6 +504,13 @@  class YnlFamily(SpecFamily):
 
         req_seq = random.randint(1024, 65535)
         msg = _genl_msg(self.family.family_id, nl_flags, op.req_value, 1, req_seq)
+        fixed_header_members = []
+        if op.fixed_header:
+            fixed_header_members = self.consts[op.fixed_header].members
+            for m in fixed_header_members:
+                value = vals.pop(m.name)
+                format, _ = NlAttr.type_formats[m.type]
+                msg += struct.pack(format, value)
         for name, value in vals.items():
             msg += self._add_attr(op.attr_set.name, name, value)
         msg = _genl_msg_finalize(msg)
@@ -522,7 +537,7 @@  class YnlFamily(SpecFamily):
                     done = True
                     break
 
-                gm = GenlMsg(nl_msg)
+                gm = GenlMsg(nl_msg, fixed_header_members)
                 # Check if this is a reply to our request
                 if nl_msg.nl_seq != req_seq or gm.genl_cmd != op.rsp_value:
                     if gm.genl_cmd in self.async_msg_ids:
@@ -532,7 +547,7 @@  class YnlFamily(SpecFamily):
                         print('Unexpected message: ' + repr(gm))
                         continue
 
-                rsp.append(self._decode(gm.raw_attrs, op.attr_set.name))
+                rsp.append(self._decode(gm.raw_attrs, op.attr_set.name) | gm.fixed_header_attrs)
 
         if not rsp:
             return None