Date: Wed, 29 Nov 2023 10:11:53 +0000
From: Donald Hunter <donald.hunter@...il.com>
To: netdev@...r.kernel.org,
	Jakub Kicinski <kuba@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Paolo Abeni <pabeni@...hat.com>,
	Jonathan Corbet <corbet@....net>,
	linux-doc@...r.kernel.org
Cc: donald.hunter@...hat.com,
	Donald Hunter <donald.hunter@...il.com>
Subject: [RFC PATCH net-next v1 0/6] tools/net/ynl: Add dynamic selector for options attrs

This patchset adds a dynamic selector mechanism to YNL for kind-specific
options attributes. I am sending this as an RFC solicit feedback on a
couple of issues before I complete the patchset.

I started adding this feature for the rt_link spec which is monomorphic,
i.e. the kind-specific 'data' attribute is always a nest. The selector
looked like this:

  -
    name: data
    type: dynamic
    selector:
      attribute: kind
      list:
        -
          value: bridge
          nested-attributes: linkinfo-bridge-attrs
        -
          value: erspan
          nested-attributes: linkinfo-gre-attrs

Then I started working on tc and found that the 'options' attribute is
poymorphic. It is typically either a C struct or a nest. So I extended the
dynamic selector to include a 'type' field and type-specific sub-fields:

  -
    name: options
    type: dynamic
    selector:
      attribute: kind
      list:
        -
          value: bfifo
          type: binary
          struct: tc-fifo-qopt
        -
          value: cake
          type: nest
          nested-attributes: tc-cake-attrs
        -
          value: cbs
          type: nest
          nested-attributes: tc-cbs-attrs

Then I encountered 'netem' which has a nest with a C struct header. I
realised that maybe my mental model had been wrong and that all cases
could be supported by a nest type with an optional fixed-header followed
by zero or more nlattrs.

  -
    value: netem
    type: nest
    fixed-header: tc-netem-qopt
    nested-attributes: tc-netem-attrs

Perhaps it is attribute-sets in general that should have an optional
fixed-header, which would also work for fixed-headers at the start of
genetlink messages. I originally added fixed-header support to
operations for genetlink, but fixed headers on attribute sets would work
for all these cases.

I now see a few possible ways forward and would like feedback on the
preferred approach:

1. Simplify the current patchset to implement fixed-header & nest
   support in the dynamic selector. This would leave existing
   fixed-header support for messages unchanged. We could drop the 'type'
   field.

   -
     value: netem
     fixed-header: tc-netem-qopt
     nested-attributes: tc-netem-attrs

2. Keep the 'type' field and support for the 'binary' type which is
   useful for specifying nests with unknown attribute spaces. An
   alternative would be to default to 'binary' behaviour if there is no
   selector entry.

3. Refactor the existing fixed-header support to be an optional part of
   all attribute sets instead of just messages (in legacy and raw specs)
   and dynamic attribute nests (in raw specs).

   attribute-sets:
     -
       name: tc-netem-attrs
       fixed-header: tc-netem-qopt
       attributes:
         ...

Thoughts?

Patch 1 adds missing scalars to the netlink-raw schema
Patch 2 and 3 add dynamic nest support to the schema and ynl
Patches 4 and 5 contain specs that use dynamic nests
Patch 6 adds support for nests with a fixed-header

Donald Hunter (6):
  doc/netlink: Add bitfield32, s8, s16 to the netlink-raw schema
  doc/netlink: Add a nest selector to netlink-raw schema
  tools/net/ynl: Add dynamic attribute decoding to ynl
  doc/netlink/specs: add dynamic nest selector for rt_link data
  doc/netlink/specs: Add a spec for tc
  tools/net/ynl: Add optional fixed-header to dynamic nests

 Documentation/netlink/netlink-raw.yaml   |   43 +-
 Documentation/netlink/specs/rt_link.yaml |  278 ++-
 Documentation/netlink/specs/tc.yaml      | 2074 ++++++++++++++++++++++
 tools/net/ynl/lib/nlspec.py              |   27 +
 tools/net/ynl/lib/ynl.py                 |   54 +-
 5 files changed, 2462 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/netlink/specs/tc.yaml

-- 
2.42.0

Powered by blists - more mailing lists