All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.