public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Donald Hunter <donald.hunter@gmail.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
	pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org,
	shuah@kernel.org, sdf@fomichev.me,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH net-next 0/5] tools: ynl: policy query support
Date: Wed, 11 Mar 2026 11:35:50 -0700	[thread overview]
Message-ID: <20260311113550.6b493008@kernel.org> (raw)
In-Reply-To: <m2jyviob64.fsf@gmail.com>

On Wed, 11 Mar 2026 11:30:59 +0000 Donald Hunter wrote:
> > On Mon,  9 Mar 2026 17:53:32 -0700 you wrote:  
> >> Improve the Netlink policy support in YNL. This series grew out of
> >> improvements to policy checking, when writing selftests I realized
> >> that instead of doing all the policy parsing in the test we're
> >> better off making it part of YNL itself.
> >> 
> >> Patch 1 adds pad handling, apparently we never hit pad with commonly
> >> used families. nlctrl policy dumps use pad more frequently.
> >> Patch 2 is a trivial refactor.
> >> Patch 3 pays off some technical debt in terms of documentation.
> >> The YnlFamily class is growing in size and it's quite hard to
> >> find its members. So document it a little bit.
> >> Patch 4 is the main dish, the implementation of get_policy(op)
> >> in YnlFamily.
> >> Patch 5 plugs the new functionality into the CLI.  
> 
> I was mid review

Sorry about that! :( I didn't see any reply to the one liner for a few
days I thought you may be AFK :)

> looking at a couple of issues:
> 
> - It would be good to fail more gracefully for netlink-raw families

This one I saw, but I figured the message that gets printed is...
reasonable. For low level functionality like policies we should 
assume user is relatively advanced? The policies are spotty
even in genetlink, a lot of families don't link them up properly :(
So explaining all of this is a bit of a rabbit hole.

> - I get "RecursionError: maximum recursion depth exceeded" for nl80211

:o Missed this one completely! My feeling is that we should lean into
the NlPolicy class more, and avoid rendering the full structure upfront.
WDYT about the following?

diff --git a/tools/net/ynl/pyynl/cli.py b/tools/net/ynl/pyynl/cli.py
index fc9e84754e4b..ff0cfbdc82e4 100755
--- a/tools/net/ynl/pyynl/cli.py
+++ b/tools/net/ynl/pyynl/cli.py
@@ -16,7 +16,8 @@ import textwrap
 
 # pylint: disable=no-name-in-module,wrong-import-position
 sys.path.append(pathlib.Path(__file__).resolve().parent.as_posix())
-from lib import YnlFamily, Netlink, NlError, SpecFamily, SpecException, YnlException
+from lib import SpecFamily, SpecException
+from lib import YnlFamily, Netlink, NlError, NlPolicy, YnlException
 
 SYS_SCHEMA_DIR='/usr/share/ynl'
 RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
@@ -79,6 +80,8 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
             return bytes.hex(o)
         if isinstance(o, set):
             return sorted(o)
+        if isinstance(o, NlPolicy):
+            return o.to_dict()
         return json.JSONEncoder.default(self, o)
 
 
@@ -313,11 +316,11 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
     if args.policy:
         if args.do:
             pol = ynl.get_policy(args.do, 'do')
-            output(pol.attrs if pol else None)
+            output(pol if pol else None)
             args.do = None
         if args.dump:
             pol = ynl.get_policy(args.dump, 'dump')
-            output(pol.attrs if pol else None)
+            output(pol if pol else None)
             args.dump = None
 
     if args.ntf:
diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
index 0eedeee465d8..4411f1902ae4 100644
--- a/tools/net/ynl/pyynl/lib/ynl.py
+++ b/tools/net/ynl/pyynl/lib/ynl.py
@@ -143,32 +143,113 @@ from .nlspec import SpecFamily
     pass
 
 
-# pylint: disable=too-few-public-methods
 class NlPolicy:
     """Kernel policy for one mode (do or dump) of one operation.
 
-    Returned by YnlFamily.get_policy(). Contains a dict of attributes
-    the kernel accepts, with their validation constraints.
+    Returned by YnlFamily.get_policy(). Attributes of the policy
+    are accessible as attributes of the object. Nested policies
+    can be accessed indexing the object like a dictionary::
 
-    Attributes:
-        attrs: dict mapping attribute names to policy dicts, e.g.
-        page-pool-stats-get do policy::
+        pol = ynl.get_policy('page-pool-stats-get', 'do')
+        pol['info'].type            # 'nested'
+        pol['info']['id'].type      # 'uint'
+        pol['info']['id'].min_value # 1
 
-            {
-                'info': {'type': 'nested', 'policy': {
-                    'id':      {'type': 'uint', 'min-value': 1,
-                                'max-value': 4294967295},
-                    'ifindex': {'type': 'u32', 'min-value': 1,
-                                'max-value': 2147483647},
-                }},
-            }
+    Each policy entry always has a 'type' attribute (e.g. u32, string,
+    nested). Optional attributes depending on the 'type': min-value,
+    max-value, min-length, max-length, mask.
 
-        Each policy dict always contains 'type' (e.g. u32, string,
-        nested). Optional keys: min-value, max-value, min-length,
-        max-length, mask, policy.
+    Policies can form infinite nesting loops. These loops are trimmed
+    when policy is converted to a dict with pol.to_dict().
     """
-    def __init__(self, attrs):
-        self.attrs = attrs
+    def __init__(self, ynl, policy_idx, policy_table, attr_set, props=None):
+        self._policy_idx = policy_idx
+        self._policy_table = policy_table
+        self._ynl = ynl
+        self._props = props or {}
+        self._entries = {}
+        if policy_idx is not None and policy_idx in policy_table:
+            for attr_id, decoded in policy_table[policy_idx].items():
+                if attr_set and attr_id in attr_set.attrs_by_val:
+                    spec = attr_set.attrs_by_val[attr_id]
+                    name = spec['name']
+                else:
+                    spec = None
+                    name = f'attr-{attr_id}'
+                self._entries[name] = (spec, decoded)
+
+    def __getitem__(self, name):
+        """Descend into a nested policy by attribute name."""
+        spec, decoded = self._entries[name]
+        props = dict(decoded)
+        child_idx = None
+        child_set = None
+        if 'policy-idx' in props:
+            child_idx = props.pop('policy-idx')
+            if spec and 'nested-attributes' in spec.yaml:
+                child_set = self._ynl.attr_sets[spec.yaml['nested-attributes']]
+        return NlPolicy(self._ynl, child_idx, self._policy_table,
+                        child_set, props)
+
+    def __getattr__(self, name):
+        """Access this policy entry's own properties (type, min-value, etc.).
+
+        Underscores in the name are converted to dashes, so that
+        pol.min_value looks up "min-value".
+        """
+        key = name.replace('_', '-')
+        try:
+            # Hack for level-0 which we still want to have .type but we don't
+            # want type to pointlessly show up in the dict / JSON form.
+            if not self._props and name == "type":
+                return "nested"
+            return self._props[key]
+        except KeyError:
+            raise AttributeError(name)
+
+    def get(self, name, default=None):
+        """Look up a child policy entry by attribute name, with a default."""
+        try:
+            return self[name]
+        except KeyError:
+            return default
+
+    def __contains__(self, name):
+        return name in self._entries
+
+    def __len__(self):
+        return len(self._entries)
+
+    def __iter__(self):
+        return iter(self._entries)
+
+    def keys(self):
+        """Return attribute names accepted by this policy."""
+        return self._entries.keys()
+
+    def to_dict(self, seen=None):
+        """Convert to a plain dict, suitable for JSON serialization.
+
+        Nested NlPolicy objects are expanded recursively. Cyclic
+        references are trimmed (resolved to just {"type": "nested"}).
+        """
+        if seen is None:
+            seen = set()
+        result = dict(self._props)
+        if self._policy_idx is not None:
+            if self._policy_idx not in seen:
+                seen = seen | {self._policy_idx}
+                children = {}
+                for name in self:
+                    children[name] = self[name].to_dict(seen)
+                if self._props:
+                    result['policy'] = children
+                else:
+                    result = children
+        return result
+
+    def __repr__(self):
+        return repr(self.to_dict())
 
 
 class NlAttr:
@@ -1308,28 +1389,6 @@ from .nlspec import SpecFamily
     def do_multi(self, ops):
         return self._ops(ops)
 
-    def _resolve_policy(self, policy_idx, policy_table, attr_set):
-        attrs = {}
-        if policy_idx not in policy_table:
-            return attrs
-        for attr_id, decoded in policy_table[policy_idx].items():
-            if attr_set and attr_id in attr_set.attrs_by_val:
-                spec = attr_set.attrs_by_val[attr_id]
-                name = spec['name']
-            else:
-                spec = None
-                name = f'attr-{attr_id}'
-            if 'policy-idx' in decoded:
-                sub_set = None
-                if spec and 'nested-attributes' in spec.yaml:
-                    sub_set = self.attr_sets[spec.yaml['nested-attributes']]
-                nested = self._resolve_policy(decoded['policy-idx'],
-                                              policy_table, sub_set)
-                del decoded['policy-idx']
-                decoded['policy'] = nested
-            attrs[name] = decoded
-        return attrs
-
     def get_policy(self, op_name, mode):
         """Query running kernel for the Netlink policy of an operation.
 
@@ -1341,8 +1400,8 @@ from .nlspec import SpecFamily
             mode: 'do' or 'dump'
 
         Returns:
-            NlPolicy with an attrs dict mapping attribute names to
-            their policy properties (type, min/max, nested, etc.),
+            NlPolicy acting as a read-only dict mapping attribute names
+            to their policy properties (type, min/max, nested, etc.),
             or None if the operation has no policy for the given mode.
             Empty policy usually implies that the operation rejects
             all attributes.
@@ -1353,5 +1412,4 @@ from .nlspec import SpecFamily
         if mode not in op_policy:
             return None
         policy_idx = op_policy[mode]
-        attrs = self._resolve_policy(policy_idx, policy_table, op.attr_set)
-        return NlPolicy(attrs)
+        return NlPolicy(self, policy_idx, policy_table, op.attr_set)

  reply	other threads:[~2026-03-11 18:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-10  0:53 [PATCH net-next 0/5] tools: ynl: policy query support Jakub Kicinski
2026-03-10  0:53 ` [PATCH net-next 1/5] tools: ynl: handle pad type during decode Jakub Kicinski
2026-03-10  0:53 ` [PATCH net-next 2/5] tools: ynl: move policy decoding out of NlMsg Jakub Kicinski
2026-03-10  0:53 ` [PATCH net-next 3/5] tools: ynl: add short doc to class YnlFamily Jakub Kicinski
2026-03-10  0:53 ` [PATCH net-next 4/5] tools: ynl: add Python API for easier access to policies Jakub Kicinski
2026-03-10  0:53 ` [PATCH net-next 5/5] tools: ynl: cli: add --policy support Jakub Kicinski
2026-03-11  2:40 ` [PATCH net-next 0/5] tools: ynl: policy query support patchwork-bot+netdevbpf
2026-03-11 11:30   ` Donald Hunter
2026-03-11 18:35     ` Jakub Kicinski [this message]
2026-03-12 17:17       ` Donald Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260311113550.6b493008@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox