[PATCH net 1/2] genetlink: report the real command id for dump-only ops in policy dumps

[PATCH net 1/2] genetlink: report the real command id for dump-only ops in policy dumps

[Jakub Kicinski]

The op-to-policy map a CTRL_CMD_GETPOLICY dump returns is the only way
for userspace to find out which policy index belongs to which command.
ctrl_dumppolicy_put_op() tags the nest with doit->cmd, but an op which
only has a dumpit has no doit and every path which fills the split ops
in zeroes it out, so those entries all claim to be command 0.  nlctrl's
own CTRL_CMD_GETPOLICY and NETDEV_CMD_QSTATS_GET are both in that group:

  [{'family-id': 16, 'op-policy': {'do': 0, 'dump': 0, 'op-id': 3}},
   {'family-id': 16, 'op-policy': {'dump': 1, 'op-id': 0}},

ctrl_fill_info() gets this right - it uses the iterator's cmd for
CTRL_ATTR_OP_ID - so the two introspection interfaces of the same family
contradict each other today.

Pass the command in rather than reconstructing it from
doit->cmd | dumpit->cmd inside the helper, both callers already have it.

Fixes: 26588edbef60 ("genetlink: support split policies in ctrl_dumppolicy_put_op()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/netlink/genetlink.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 41d37442f186..5cc1037d4917 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1656,7 +1656,7 @@ static void *ctrl_dumppolicy_prep(struct sk_buff *skb,
 }
 
 static int ctrl_dumppolicy_put_op(struct sk_buff *skb,
-				  struct netlink_callback *cb,
+				  struct netlink_callback *cb, u32 cmd,
 				  struct genl_split_ops *doit,
 				  struct genl_split_ops *dumpit)
 {
@@ -1677,7 +1677,7 @@ static int ctrl_dumppolicy_put_op(struct sk_buff *skb,
 	if (!nest_pol)
 		goto err;
 
-	nest_op = nla_nest_start(skb, doit->cmd);
+	nest_op = nla_nest_start(skb, cmd);
 	if (!nest_op)
 		goto err;
 
@@ -1721,7 +1721,8 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
 						      &doit, &dumpit)))
 				return -ENOENT;
 
-			if (ctrl_dumppolicy_put_op(skb, cb, &doit, &dumpit))
+			if (ctrl_dumppolicy_put_op(skb, cb, ctx->op,
+						   &doit, &dumpit))
 				return skb->len;
 
 			/* done with the per-op policy index list */
@@ -1730,6 +1731,7 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
 
 		while (ctx->dump_map) {
 			if (ctrl_dumppolicy_put_op(skb, cb,
+						   ctx->op_iter->cmd,
 						   &ctx->op_iter->doit,
 						   &ctx->op_iter->dumpit))
 				return skb->len;
-- 
2.55.0

[PATCH net 2/2] selftests: net: nl_nlctrl: check the op ids in the policy map

[Jakub Kicinski]

Validate that the op map in the policy dump is correct.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/net/nl_nlctrl.py | 116 +++++++++++++++++++----
 1 file changed, 99 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/net/nl_nlctrl.py b/tools/testing/selftests/net/nl_nlctrl.py
index fe1f66dc9435..237b3d273260 100755
--- a/tools/testing/selftests/net/nl_nlctrl.py
+++ b/tools/testing/selftests/net/nl_nlctrl.py
@@ -9,40 +9,86 @@ from lib.py import ksft_run, ksft_exit
 from lib.py import ksft_eq, ksft_ge, ksft_true, ksft_in, ksft_not_in
 from lib.py import NetdevFamily, EthtoolFamily, NlctrlFamily
 
+# Families we can expect to always be around, and which between them
+# cover ops with a do, with a dump, and with both.
+FAMILIES = ('nlctrl', 'netdev')
 
-def getfamily_do(ctrl) -> None:
-    """Query a single family by name and validate its ops."""
-    fam = ctrl.getfamily({'family-name': 'netdev'})
-    ksft_eq(fam['family-name'], 'netdev')
+
+def _get_ops(ctrl, name):
+    """Get the ops of a family, keyed by command id."""
+    fam = ctrl.getfamily({'family-name': name})
+    ksft_eq(fam['family-name'], name)
     ksft_true(fam['family-id'] > 0)
 
     # The format of ops is quite odd, [{$idx: {"id"...}}, {$idx: {"id"...}}]
     # Discard the indices and re-key by command id.
     ops_by_id = {v['id']: v for op in fam['ops'] for v in op.values()}
-    ksft_eq(len(ops_by_id), len(fam['ops']))
+    ksft_eq(len(ops_by_id), len(fam['ops']),
+            comment=f"{name} lists a command twice")
+    return ops_by_id
 
-    # All ops should have a policy (either do or dump has one)
-    for op in ops_by_id.values():
-        ksft_in('cmd-cap-haspol', op['flags'],
-                comment=f"op {op['id']} missing haspol")
+
+def _get_policy_map(ctrl, req):
+    """
+    The policy map in the Netlink replies looks like this:
+
+         [{'family-id': 16, 'op-policy': {'do': 0, 'dump': 0, 'op-id': 3}},
+          {'family-id': 16, 'op-policy': {'dump': 1, 'op-id': 4}}, ...]
+
+    Return the mapping:
+
+         {3:{'do','dump'}, 4:{'dump'}}
+
+    The policy itself is discarded here, only return which command has policy.
+    """
+    pol_map = {}
+    for msg in ctrl.getpolicy(req, dump=True):
+        if 'op-policy' not in msg:
+            continue
+        modes = dict(msg['op-policy'])
+        cmd = modes.pop('op-id')
+        ksft_not_in(cmd, pol_map, comment=f"command {cmd} reported twice")
+        pol_map[cmd] = set(modes.keys())
+    return pol_map
+
+
+def getfamily_do(ctrl) -> None:
+    """Query single families by name and validate their ops."""
+    ops = {name: _get_ops(ctrl, name) for name in FAMILIES}
+
+    for name, ops_by_id in ops.items():
+        for op in ops_by_id.values():
+            # All ops in nlctrl and netdev have a policy
+            ksft_in('cmd-cap-haspol', op['flags'],
+                    comment=f"{name} op {op['id']} missing haspol")
+            ksft_true(op['flags'] & {'cmd-cap-do', 'cmd-cap-dump'},
+                      comment=f"{name} op {op['id']} has no handler")
+
+    # nlctrl getfamily (id 3) does both, getpolicy (id 10) is dump-only
+    ksft_in('cmd-cap-do', ops['nlctrl'][3]['flags'])
+    ksft_in('cmd-cap-dump', ops['nlctrl'][3]['flags'])
+    ksft_not_in('cmd-cap-do', ops['nlctrl'][10]['flags'])
+    ksft_in('cmd-cap-dump', ops['nlctrl'][10]['flags'])
+
+    netdev = ops['netdev']
 
     # dev-get (id 1) should support both do and dump
-    ksft_in('cmd-cap-do', ops_by_id[1]['flags'])
-    ksft_in('cmd-cap-dump', ops_by_id[1]['flags'])
+    ksft_in('cmd-cap-do', netdev[1]['flags'])
+    ksft_in('cmd-cap-dump', netdev[1]['flags'])
 
     # qstats-get (id 12) is dump-only
-    ksft_not_in('cmd-cap-do', ops_by_id[12]['flags'])
-    ksft_in('cmd-cap-dump', ops_by_id[12]['flags'])
+    ksft_not_in('cmd-cap-do', netdev[12]['flags'])
+    ksft_in('cmd-cap-dump', netdev[12]['flags'])
 
     # napi-set (id 14) is do-only and requires admin
-    ksft_in('cmd-cap-do', ops_by_id[14]['flags'])
-    ksft_not_in('cmd-cap-dump', ops_by_id[14]['flags'])
-    ksft_in('admin-perm', ops_by_id[14]['flags'])
+    ksft_in('cmd-cap-do', netdev[14]['flags'])
+    ksft_not_in('cmd-cap-dump', netdev[14]['flags'])
+    ksft_in('admin-perm', netdev[14]['flags'])
 
     # Notification-only commands (dev-add/del/change-ntf etc.) must
     # not appear in the ops list since they have no do/dump handlers.
     for ntf_id in [2, 3, 4, 6, 7, 8]:
-        ksft_not_in(ntf_id, ops_by_id,
+        ksft_not_in(ntf_id, netdev,
                     comment=f"ntf-only cmd {ntf_id} should not be in ops")
 
 
@@ -103,6 +149,41 @@ from lib.py import NetdevFamily, EthtoolFamily, NlctrlFamily
             comment="linkinfo-set should not have a dump policy")
 
 
+def getpolicy_op_map(ctrl) -> None:
+    """Check the op-to-policy map consistency. Each op with 'haspol' flag
+    has to have a policy. The policy back-references must name only
+    real ops that exist, have given modes (do vs dump) and have 'haspol'.
+    """
+    for name in FAMILIES:
+        ops_by_id = _get_ops(ctrl, name)
+        haspol = {cmd for cmd, op in ops_by_id.items()
+                  if 'cmd-cap-haspol' in op['flags']}
+
+        pol_map = _get_policy_map(ctrl, {'family-name': name})
+        ksft_eq(set(pol_map), haspol,
+                comment=f"{name} policy map does not match the op list")
+
+        # Walk the op list rather than the map, the map may be missing
+        # the very op we are after. Asking for a command the family does
+        # not have is an error, so it must not come from the map either.
+        for cmd in sorted(haspol):
+            modes = pol_map.get(cmd, set())
+
+            # The kernel only reports a mode the op actually has.
+            if 'do' in modes:
+                ksft_in('cmd-cap-do', ops_by_id[cmd]['flags'],
+                        comment=f"{name} cmd {cmd} has no do")
+            if 'dump' in modes:
+                ksft_in('cmd-cap-dump', ops_by_id[cmd]['flags'],
+                        comment=f"{name} cmd {cmd} has no dump")
+
+            # Asking for one op builds the map in a different place in
+            # the kernel, it has to report what the full dump did.
+            single = _get_policy_map(ctrl, {'family-name': name, 'op': cmd})
+            ksft_eq(single, {cmd: modes},
+                    comment=f"{name} cmd {cmd} policy differs from the dump")
+
+
 def getpolicy_by_op(_ctrl) -> None:
     """Query policy for specific ops, check attr names are resolved."""
     ndev = NetdevFamily()
@@ -122,6 +203,7 @@ from lib.py import NetdevFamily, EthtoolFamily, NlctrlFamily
     ksft_run([getfamily_do,
               getfamily_dump,
               getpolicy_dump,
+              getpolicy_op_map,
               getpolicy_by_op],
              args=(ctrl, ))
     ksft_exit()
-- 
2.55.0

Re: [PATCH net 1/2] genetlink: report the real command id for dump-only ops in policy dumps

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 18 Sep 2026 15:29:48 -0700 you wrote:
> The op-to-policy map a CTRL_CMD_GETPOLICY dump returns is the only way
> for userspace to find out which policy index belongs to which command.
> ctrl_dumppolicy_put_op() tags the nest with doit->cmd, but an op which
> only has a dumpit has no doit and every path which fills the split ops
> in zeroes it out, so those entries all claim to be command 0.  nlctrl's
> own CTRL_CMD_GETPOLICY and NETDEV_CMD_QSTATS_GET are both in that group:
> 
> [...]

Here is the summary with links:
  - [net,1/2] genetlink: report the real command id for dump-only ops in policy dumps
    https://git.kernel.org/netdev/net/c/261e8a37ecba
  - [net,2/2] selftests: net: nl_nlctrl: check the op ids in the policy map
    https://git.kernel.org/netdev/net/c/a87529034b9c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html