[PATCH net-next 0/6] psp: correct notifications and device info around device assoc

[PATCH net-next 0/6] psp: correct notifications and device info around device assoc

[Jakub Kicinski]

We recently added the ability to associate a HW PSP device with
a software device like netkit or veth to be able to make PSP
usable from containers. This stretches the visibility of a PSP
device to multiple netns's.

Correct two minor issues:

[Patch 2] skip reporting the main ifindex in remote netns. I'm not sure
if we had some justification for this but looking back it's pretty
confusing. The ifindex is meaningless in another netns, better
not to have it.

[Patch 4] correct the change notification when net_device is removed.
The "leaving" notification had the removed netdev still listed among
the associated devices. This made the code simpler but really it makes
the notification impossible to interpret. Add the extra code to handle
this right, exclude the removed device, and send a del notification
when last device in a netns is chopped off.

Both of these were discovered by an LLM scan of the YAML spec,
the spec itself appears to not have major bugs.

Jakub Kicinski (6):
  selftests: drv-net: psp: fix linter issues
  psp: don't report the main netdevice's ifindex to associated
    namespaces
  selftests: drv-net: psp: check the ifindex an associated netns sees
  psp: notify about a disassociation once it has happened
  selftests: drv-net: psp: factor out creating a netkit in the test
    netns
  selftests: drv-net: psp: check the PSP disassociation notifications

 Documentation/netlink/specs/psp.yaml       |   2 +
 net/psp/psp.h                              |   1 +
 net/psp/psp_main.c                         |  16 +-
 net/psp/psp_nl.c                           |  51 ++++--
 tools/testing/selftests/drivers/net/psp.py | 180 +++++++++++++++++----
 5 files changed, 207 insertions(+), 43 deletions(-)

-- 
2.55.0

[PATCH net-next 1/6] selftests: drv-net: psp: fix linter issues

[Jakub Kicinski]

Fix ruff 0.16 warnings:

  C403 Unnecessary list comprehension (rewrite as a set comprehension)

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/psp.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index 315648a770d0..5e87fb50c348 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -525,11 +525,11 @@ from lib.py import ip
 
 def __nsim_psp_rereg(cfg):
     # The PSP dev ID will change, remember what was there before
-    before = set([x['id'] for x in cfg.pspnl.dev_get({}, dump=True)])
+    before = {x['id'] for x in cfg.pspnl.dev_get({}, dump=True)}
 
     cfg._ns.nsims[0].dfs_write('psp_rereg', '1')
 
-    after = set([x['id'] for x in cfg.pspnl.dev_get({}, dump=True)])
+    after = {x['id'] for x in cfg.pspnl.dev_get({}, dump=True)}
 
     new_devs = list(after - before)
     ksft_eq(len(new_devs), 1)
-- 
2.55.0

[PATCH net-next 2/6] psp: don't report the main netdevice's ifindex to associated namespaces

[Jakub Kicinski]

PSP device is visible in a netns if any of the devices (eg. netkit)
are associated with that PSP device. In the main netns we show
all the associated netdevs + their netns id. In the "container"
netns we show only the local devices. But we were listing the main
netdev in all cases, even though it's meaningless outside of
the main netns.

Report ifindex only in the main netdevice's namespace. Absence is
already unambiguous, the by-association flag is set exactly in the
messages which no longer carry the ifindex.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/psp.yaml | 2 ++
 net/psp/psp_nl.c                     | 7 +++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/netlink/specs/psp.yaml b/Documentation/netlink/specs/psp.yaml
index e9c2ee7e28e0..f3266763c325 100644
--- a/Documentation/netlink/specs/psp.yaml
+++ b/Documentation/netlink/specs/psp.yaml
@@ -38,6 +38,8 @@ name: psp
         doc: |
           ifindex of the main netdevice linked to the PSP device,
           or the ifindex to associate with the PSP device.
+          Only reported to the network namespace the main netdevice
+          lives in, an ifindex has no meaning outside of it.
         type: u32
       -
         name: psp-versions-cap
diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
index f91665748dde..b57366b5e032 100644
--- a/net/psp/psp_nl.c
+++ b/net/psp/psp_nl.c
@@ -294,13 +294,16 @@ psp_nl_dev_fill(struct psp_dev *psd, struct sk_buff *rsp,
 		return -EMSGSIZE;
 
 	if (nla_put_u32(rsp, PSP_A_DEV_ID, psd->id) ||
-	    nla_put_u32(rsp, PSP_A_DEV_IFINDEX, psd->main_netdev->ifindex) ||
 	    nla_put_u32(rsp, PSP_A_DEV_PSP_VERSIONS_CAP, psd->caps->versions) ||
 	    nla_put_u32(rsp, PSP_A_DEV_PSP_VERSIONS_ENA, psd->config.versions))
 		goto err_cancel_msg;
 
 	if (cur_net == dev_net(psd->main_netdev)) {
-		/* Primary device - dump assoc list */
+		/* Primary device - report the netdev, dump assoc list. */
+		if (nla_put_u32(rsp, PSP_A_DEV_IFINDEX,
+				psd->main_netdev->ifindex))
+			goto err_cancel_msg;
+
 		err = psp_nl_fill_assoc_dev_list(psd, rsp, cur_net, NULL);
 		if (err)
 			goto err_cancel_msg;
-- 
2.55.0

[PATCH net-next 3/6] selftests: drv-net: psp: check the ifindex an associated netns sees

[Jakub Kicinski]

dev-get must not report the ifindex outside of the main netns.
The dev-get checks for an associated namespace are already there,
add the "no main ifindex" assertion.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/psp.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index 5e87fb50c348..43780efb84ed 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -749,6 +749,10 @@ from lib.py import ip
 
         ksft_not_none(peer_dev, "No PSP device found with by-association flag in guest netns")
 
+        # ifindex of the main netdevice means nothing in this namespace
+        ksft_true('ifindex' not in peer_dev,
+                  "ifindex reported to an associated namespace")
+
         # Verify assoc-list contains the nk_guest device
         ksft_true('assoc-list' in peer_dev and len(peer_dev['assoc-list']) > 0,
                   "Guest device should have assoc-list with local devices")
-- 
2.55.0

[PATCH net-next 4/6] psp: notify about a disassociation once it has happened

[Jakub Kicinski]

The dev-change-ntf generated by dev-disassoc was built before the
association was unlinked, so the assoc-list it carried still contained
the device which was going away, and nothing corrected it afterwards.
We don't really expect associations to change during a lifetime of
a netns but this is still wrong. Netlink listeners need to be able
to tell the current "state of the world" based on notifications.

The notification had to be sent early because psp_nl_multicast_per_ns()
derives the set of namespaces to notify from the association list, so
a namespace losing its last associated device becomes unreachable once
the entry is gone. In that case - get the netns from the netdev itself,
and send that namespace a dev-del-ntf. If the netns had multiple
associated netdevs and only one disassoc'd we'll still send a change
notification, just with a correct list.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/psp/psp.h      |  1 +
 net/psp/psp_main.c | 16 +++++++++++-----
 net/psp/psp_nl.c   | 44 +++++++++++++++++++++++++++++++++++++-------
 3 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/net/psp/psp.h b/net/psp/psp.h
index bbb39e2f5b0a..b123c2427905 100644
--- a/net/psp/psp.h
+++ b/net/psp/psp.h
@@ -19,6 +19,7 @@ bool psp_has_assoc_dev_in_ns(struct psp_dev *psd, struct net *net);
 int psp_attach_netdev_notifier(void);
 
 void psp_nl_notify_dev(struct psp_dev *psd, u32 cmd);
+void psp_nl_notify_disassoc(struct psp_dev *psd, struct net *net);
 
 struct psp_assoc *psp_assoc_create(struct psp_dev *psd);
 struct psp_dev *psp_dev_get_for_sock(struct sock *sk);
diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index 91473f96ad21..273b010d2355 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -408,7 +408,7 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
 }
 EXPORT_SYMBOL(psp_dev_rcv);
 
-static void psp_dev_disassoc_one(struct psp_dev *psd, struct net_device *dev)
+static bool psp_dev_disassoc_one(struct psp_dev *psd, struct net_device *dev)
 {
 	struct psp_assoc_dev *entry;
 
@@ -419,9 +419,11 @@ static void psp_dev_disassoc_one(struct psp_dev *psd, struct net_device *dev)
 			rcu_assign_pointer(entry->assoc_dev->psp_dev, NULL);
 			netdev_put(entry->assoc_dev, &entry->dev_tracker);
 			kfree(entry);
-			return;
+			return true;
 		}
 	}
+
+	return false;
 }
 
 static int psp_netdev_event(struct notifier_block *nb, unsigned long event,
@@ -438,9 +440,13 @@ static int psp_netdev_event(struct notifier_block *nb, unsigned long event,
 	if (psd && psp_dev_tryget(psd)) {
 		rcu_read_unlock();
 		mutex_lock(&psd->lock);
-		if (psp_dev_is_registered(psd))
-			psp_nl_notify_dev(psd, PSP_CMD_DEV_CHANGE_NTF);
-		psp_dev_disassoc_one(psd, dev);
+		/* Nothing to report if the device was never on the list,
+		 * dev-assoc may have failed after publishing dev->psp_dev,
+		 * and this is also the main netdevice's path.
+		 */
+		if (psp_dev_disassoc_one(psd, dev) &&
+		    psp_dev_is_registered(psd))
+			psp_nl_notify_disassoc(psd, dev_net(dev));
 		mutex_unlock(&psd->lock);
 		psp_dev_put(psd);
 	} else {
diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
index b57366b5e032..cdfc2d72fb39 100644
--- a/net/psp/psp_nl.c
+++ b/net/psp/psp_nl.c
@@ -356,6 +356,40 @@ void psp_nl_notify_dev(struct psp_dev *psd, u32 cmd)
 				psp_nl_build_dev_ntf, &cmd);
 }
 
+/**
+ * psp_nl_notify_disassoc() - notify about a device losing an association
+ * @psd: PSP device (must be locked)
+ * @net: netns of the netdevice which got disassociated
+ *
+ * Must be called once @psd no longer has the association, so that the
+ * notifications carry the state after the change.
+ */
+void psp_nl_notify_disassoc(struct psp_dev *psd, struct net *net)
+{
+	struct sk_buff *ntf;
+	bool still_visible;
+	u32 cmd;
+
+	lockdep_assert_held(&psd->lock);
+
+	psp_nl_notify_dev(psd, PSP_CMD_DEV_CHANGE_NTF);
+
+	/* psp_nl_notify_dev() reaches the main netdevice's netns and every
+	 * netns which still has an associated device. If @net is neither,
+	 * the device is gone from @net and we should send a delete ntf.
+	 */
+	still_visible = !psp_dev_check_access(psd, net, false);
+	if (still_visible || !maybe_get_net(net))
+		return;
+
+	cmd = PSP_CMD_DEV_DEL_NTF;
+	ntf = psp_nl_build_dev_ntf(psd, net, &cmd);
+	if (ntf)
+		genlmsg_multicast_netns(&psp_nl_family, net, ntf, 0,
+					PSP_NLGRP_MGMT, GFP_KERNEL);
+	put_net(net);
+}
+
 int psp_nl_dev_get_doit(struct sk_buff *req, struct genl_info *info)
 {
 	struct psp_dev *psd = info->user_ptr[0];
@@ -620,13 +654,6 @@ int psp_nl_dev_disassoc_doit(struct sk_buff *skb, struct genl_info *info)
 		return -ENOMEM;
 	}
 
-	put_net(net);
-
-	/* Notify before removal so listeners in the disassociated namespace
-	 * still receive the notification.
-	 */
-	psp_nl_notify_dev(psd, PSP_CMD_DEV_CHANGE_NTF);
-
 	/* Remove from the association list */
 	list_del(&found->dev_list);
 	psd->assoc_dev_cnt--;
@@ -634,6 +661,9 @@ int psp_nl_dev_disassoc_doit(struct sk_buff *skb, struct genl_info *info)
 	netdev_put(found->assoc_dev, &found->dev_tracker);
 	kfree(found);
 
+	psp_nl_notify_disassoc(psd, net);
+	put_net(net);
+
 	return psp_nl_reply_send(rsp, info);
 }
 
-- 
2.55.0

[PATCH net-next 5/6] selftests: drv-net: psp: factor out creating a netkit in the test netns

[Jakub Kicinski]

The netkit removal test builds a disposable netkit pair and moves its
peer into the test namespace. The next commit needs a second associated
device there, so move that to a helper. No functional change, other
than looking the new peer up among all netkit devices rather than the
two the environment created, which is what makes it reusable.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/psp.py | 60 ++++++++++++----------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index 43780efb84ed..d44d2c83473e 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -798,25 +798,18 @@ from lib.py import ip
     ksft_true(not found, "Device should not be in assoc-list after disassociation")
 
 
-def _psp_dev_assoc_cleanup_on_netkit_del(cfg):
-    """Test that assoc-list is cleared when associated netkit is deleted.
+def _add_netkit_guest(cfg, host_name, guest_name):
+    """Create a netkit pair and move its peer into the test namespace.
 
-    Creates a disposable netkit pair for this test to avoid destroying
-    the shared environment.
+    Returns the peer's ifindex there and the defer() deleting the pair.
     """
-    _init_psp_dev(cfg, True)
-    defer(delattr, cfg, 'psp_dev_id')
-    defer(delattr, cfg, 'psp_info')
+    existing = {link['ifindex'] for link in ip("-d link show", json=True)
+                if link.get('linkinfo', {}).get('info_kind') == 'netkit'}
 
-    existing = {cfg.nk_host_ifindex, cfg.nk_guest_ifindex}
-
-    # Create a temporary netkit pair
-    tmp_host_name = "tmp_nk_host"
-    tmp_guest_name = "tmp_nk_guest"
     rtnl = RtnlFamily()
     rtnl.newlink(
         {
-            "ifname": tmp_host_name,
+            "ifname": host_name,
             "linkinfo": {
                 "kind": "netkit",
                 "data": {
@@ -828,25 +821,38 @@ from lib.py import ip
         },
         flags=[Netlink.NLM_F_CREATE, Netlink.NLM_F_EXCL],
     )
-    cleanup_netkit = defer(ip, f"link del {tmp_host_name}")
+    cleanup = defer(ip, f"link del {host_name}")
 
     # Find the peer by diffing against existing netkit ifindexes
     all_links = ip("-d link show", json=True)
-    tmp_peer = [link for link in all_links
-                if link.get('linkinfo', {}).get('info_kind') == 'netkit'
-                and link['ifindex'] not in existing
-                and link['ifname'] != tmp_host_name]
-    ksft_eq(len(tmp_peer), 1,
-            "Failed to find temporary netkit peer")
-    guest_name = tmp_peer[0]['ifname']
+    peer = [link for link in all_links
+            if link.get('linkinfo', {}).get('info_kind') == 'netkit'
+            and link['ifindex'] not in existing
+            and link['ifname'] != host_name]
+    ksft_eq(len(peer), 1, "Failed to find the new netkit peer")
 
     # Rename and move guest end into the test namespace
-    ip(f"link set dev {guest_name} name {tmp_guest_name}")
-    ip(f"link set dev {tmp_guest_name} netns {cfg.netns.name}")
-    tmp_guest_dev = ip(f"link show dev {tmp_guest_name}",
-                       json=True, ns=cfg.netns)[0]
-    tmp_guest_ifindex = tmp_guest_dev['ifindex']
-    ip(f"link set dev {tmp_guest_name} up", ns=cfg.netns)
+    ip(f"link set dev {peer[0]['ifname']} name {guest_name}")
+    ip(f"link set dev {guest_name} netns {cfg.netns.name}")
+    guest_dev = ip(f"link show dev {guest_name}", json=True, ns=cfg.netns)[0]
+    ip(f"link set dev {guest_name} up", ns=cfg.netns)
+
+    return guest_dev['ifindex'], cleanup
+
+
+def _psp_dev_assoc_cleanup_on_netkit_del(cfg):
+    """Test that assoc-list is cleared when associated netkit is deleted.
+
+    Creates a disposable netkit pair for this test to avoid destroying
+    the shared environment.
+    """
+    _init_psp_dev(cfg, True)
+    defer(delattr, cfg, 'psp_dev_id')
+    defer(delattr, cfg, 'psp_info')
+
+    tmp_host_name = "tmp_nk_host"
+    tmp_guest_ifindex, cleanup_netkit = _add_netkit_guest(cfg, tmp_host_name,
+                                                          "tmp_nk_guest")
 
     # Associate PSP device with the temporary guest interface
     cfg.pspnl.dev_assoc({'id': cfg.psp_dev_id,
-- 
2.55.0

[PATCH net-next 6/6] selftests: drv-net: psp: check the PSP disassociation notifications

[Jakub Kicinski]

The main namespace must see a change which no longer lists the device,
and the namespace which lost its last association must see the device
go away. Check that on both paths which generate the notifications,
dev-disassoc and netdevice removal, and check that a namespace which
still has another association is only told about the change.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/psp.py | 112 +++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index d44d2c83473e..13ff5188cca3 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -723,6 +723,111 @@ from lib.py import ip
                 f" in {label} namespace")
 
 
+def _subscribe_mgmt(cfg):
+    """Listen on the mgmt group in the guest and in the main namespace."""
+    # Listener in the guest namespace; socket stays bound to that ns
+    with NetNSEnter(cfg.netns.name):
+        peer_pspnl = PSPFamily()
+        peer_pspnl.ntf_subscribe('mgmt')
+
+    main_pspnl = PSPFamily()
+    main_pspnl.ntf_subscribe('mgmt')
+
+    return main_pspnl, peer_pspnl
+
+
+def _get_dev_ntf(cfg, pspnl, label):
+    """Wait for the next notification about the PSP device under test."""
+    for ntf in pspnl.poll_ntf(duration=10):
+        if ntf['msg'].get('id') == cfg.psp_dev_id:
+            return ntf
+    raise KsftFailEx(f"No notification received in the {label} namespace")
+
+
+def _check_disassoc_ntf(cfg, main_pspnl, peer_pspnl, ifindex):
+    """Check the notifications for a netns losing its last association."""
+    ntf = _get_dev_ntf(cfg, main_pspnl, "main")
+    ksft_eq(ntf['name'], 'dev-change-ntf')
+    for assoc in ntf['msg'].get('assoc-list', []):
+        if assoc['nsid'] != cfg.psp_dev_peer_nsid:
+            continue
+        ksft_ne(assoc['ifindex'], ifindex,
+                "Disassociated device still listed in the notification")
+
+    # The device is gone as far as the disassociated namespace is concerned
+    ntf = _get_dev_ntf(cfg, peer_pspnl, "guest")
+    ksft_eq(ntf['name'], 'dev-del-ntf')
+    ksft_true('ifindex' not in ntf['msg'],
+              "ifindex reported to an associated namespace")
+
+
+def _dev_disassoc_notify_multi_ns_netkit(cfg):
+    """ Test the notifications dev-disassoc generates in both namespaces """
+    _init_psp_dev(cfg, True)
+    defer(delattr, cfg, 'psp_dev_id')
+    defer(delattr, cfg, 'psp_info')
+
+    cfg.pspnl.dev_assoc({'id': cfg.psp_dev_id,
+                         'ifindex': cfg.nk_guest_ifindex,
+                         'nsid': cfg.psp_dev_peer_nsid})
+    defer(_try_disassoc, cfg, cfg.psp_dev_id, cfg.nk_guest_ifindex,
+          cfg.psp_dev_peer_nsid)
+
+    main_pspnl, peer_pspnl = _subscribe_mgmt(cfg)
+
+    cfg.pspnl.dev_disassoc({'id': cfg.psp_dev_id,
+                            'ifindex': cfg.nk_guest_ifindex,
+                            'nsid': cfg.psp_dev_peer_nsid})
+
+    _check_disassoc_ntf(cfg, main_pspnl, peer_pspnl, cfg.nk_guest_ifindex)
+
+
+def _dev_disassoc_notify_one_of_two_netkit(cfg):
+    """Test the notifications with two netkits associated in one netns.
+
+    Disassociating the first netkit leaves the PSP device visible in the
+    guest namespace, generates a dev-change-ntf.
+    Disassociating the second one takes the device out of its view,
+    generates 'dev-del-ntf'.
+    """
+    _init_psp_dev(cfg, True)
+    defer(delattr, cfg, 'psp_dev_id')
+    defer(delattr, cfg, 'psp_info')
+
+    tmp_ifindex, _ = _add_netkit_guest(cfg, "tmp_nk_host", "tmp_nk_guest")
+
+    for ifindex in [cfg.nk_guest_ifindex, tmp_ifindex]:
+        cfg.pspnl.dev_assoc({'id': cfg.psp_dev_id, 'ifindex': ifindex,
+                             'nsid': cfg.psp_dev_peer_nsid})
+        defer(_try_disassoc, cfg, cfg.psp_dev_id, ifindex,
+              cfg.psp_dev_peer_nsid)
+
+    main_pspnl, peer_pspnl = _subscribe_mgmt(cfg)
+
+    # One of the two goes away, the device stays visible in the guest netns
+    cfg.pspnl.dev_disassoc({'id': cfg.psp_dev_id, 'ifindex': tmp_ifindex,
+                            'nsid': cfg.psp_dev_peer_nsid})
+
+    ntf = _get_dev_ntf(cfg, main_pspnl, "main")
+    ksft_eq(ntf['name'], 'dev-change-ntf')
+
+    ntf = _get_dev_ntf(cfg, peer_pspnl, "guest")
+    ksft_eq(ntf['name'], 'dev-change-ntf')
+    found = False
+    for assoc in ntf['msg'].get('assoc-list', []):
+        ksft_ne(assoc['ifindex'], tmp_ifindex,
+                "Disassociated device still listed in the notification")
+        found |= assoc['ifindex'] == cfg.nk_guest_ifindex
+    ksft_true(found, "Remaining association missing from the notification")
+
+    # And now the last one, the device disappears from the guest netns
+    cfg.pspnl.dev_disassoc({'id': cfg.psp_dev_id,
+                            'ifindex': cfg.nk_guest_ifindex,
+                            'nsid': cfg.psp_dev_peer_nsid})
+
+    _check_disassoc_ntf(cfg, main_pspnl, peer_pspnl, cfg.nk_guest_ifindex)
+
+
 def _psp_dev_get_check_netkit_psp_assoc(cfg):
     """ Check psp dev-get output with netkit interface associated with PSP dev """
     _assoc_nk_guest(cfg)
@@ -863,6 +968,9 @@ from lib.py import ip
     _check_assoc_list(cfg, cfg.psp_dev_id, tmp_guest_ifindex,
                       cfg.psp_dev_peer_nsid)
 
+    # Removing the netdevice is notified like a disassociation
+    main_pspnl, peer_pspnl = _subscribe_mgmt(cfg)
+
     # Delete the temporary netkit pair (deleting one end removes both)
     ip(f"link del {tmp_host_name}")
     cleanup_netkit.cancel()
@@ -873,6 +981,8 @@ from lib.py import ip
               or len(dev_info['assoc-list']) == 0,
               "assoc-list should be empty after netkit deletion")
 
+    _check_disassoc_ntf(cfg, main_pspnl, peer_pspnl, tmp_guest_ifindex)
+
 
 def _try_disassoc(cfg, psp_dev_id, ifindex, nsid=None):
     """Best-effort disassociate, ignoring errors if already removed."""
@@ -991,6 +1101,8 @@ from lib.py import ip
                         data_basic_send_netkit_psp_assoc,
                         _key_rotation_notify_multi_ns_netkit,
                         _dev_change_notify_multi_ns_netkit,
+                        _dev_disassoc_notify_multi_ns_netkit,
+                        _dev_disassoc_notify_one_of_two_netkit,
                         _psp_dev_get_check_netkit_psp_assoc,
                         _dev_assoc_no_nsid,
                         _psp_dev_assoc_cleanup_on_netkit_del,
-- 
2.55.0

Re: [PATCH net-next 1/6] selftests: drv-net: psp: fix linter issues

[Daniel Zahka]

On Sat Sep 12, 2026 at 4:04 PM EDT, Jakub Kicinski wrote:
> Fix ruff 0.16 warnings:
>
>   C403 Unnecessary list comprehension (rewrite as a set comprehension)
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net-next 2/6] psp: don't report the main netdevice's ifindex to associated namespaces

[Daniel Zahka]

On Sat Sep 12, 2026 at 4:04 PM EDT, Jakub Kicinski wrote:
> PSP device is visible in a netns if any of the devices (eg. netkit)
> are associated with that PSP device. In the main netns we show
> all the associated netdevs + their netns id. In the "container"
> netns we show only the local devices. But we were listing the main
> netdev in all cases, even though it's meaningless outside of
> the main netns.
>
> Report ifindex only in the main netdevice's namespace. Absence is
> already unambiguous, the by-association flag is set exactly in the
> messages which no longer carry the ifindex.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  Documentation/netlink/specs/psp.yaml | 2 ++
>  net/psp/psp_nl.c                     | 7 +++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/netlink/specs/psp.yaml b/Documentation/netlink/specs/psp.yaml
> index e9c2ee7e28e0..f3266763c325 100644
> --- a/Documentation/netlink/specs/psp.yaml
> +++ b/Documentation/netlink/specs/psp.yaml
> @@ -38,6 +38,8 @@ name: psp
>          doc: |
>            ifindex of the main netdevice linked to the PSP device,
>            or the ifindex to associate with the PSP device.
> +          Only reported to the network namespace the main netdevice
> +          lives in, an ifindex has no meaning outside of it.
>          type: u32
>        -
>          name: psp-versions-cap
> diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
> index f91665748dde..b57366b5e032 100644
> --- a/net/psp/psp_nl.c
> +++ b/net/psp/psp_nl.c
> @@ -294,13 +294,16 @@ psp_nl_dev_fill(struct psp_dev *psd, struct sk_buff *rsp,
>  		return -EMSGSIZE;
>  
>  	if (nla_put_u32(rsp, PSP_A_DEV_ID, psd->id) ||
> -	    nla_put_u32(rsp, PSP_A_DEV_IFINDEX, psd->main_netdev->ifindex) ||
>  	    nla_put_u32(rsp, PSP_A_DEV_PSP_VERSIONS_CAP, psd->caps->versions) ||
>  	    nla_put_u32(rsp, PSP_A_DEV_PSP_VERSIONS_ENA, psd->config.versions))
>  		goto err_cancel_msg;
>  
>  	if (cur_net == dev_net(psd->main_netdev)) {
> -		/* Primary device - dump assoc list */
> +		/* Primary device - report the netdev, dump assoc list. */
> +		if (nla_put_u32(rsp, PSP_A_DEV_IFINDEX,
> +				psd->main_netdev->ifindex))
> +			goto err_cancel_msg;
> +
>  		err = psp_nl_fill_assoc_dev_list(psd, rsp, cur_net, NULL);
>  		if (err)
>  			goto err_cancel_msg;

As an aside, this got me looking at psp_nl_fill_assoc_dev_list() again.
The PSP_A_ASSOC_DEV_INFO_NSID handling there looks a bit buggy. Caller
sees -1 when the assoc dev is in their namespace, unless they are in the
psp_dev's main_netdev's netns. In that case, a self referential nsid is
allocated with peernet2id_alloc(). It probably would have made more
sense to just only include PSP_A_ASSOC_DEV_INFO_NSID if !net_eq(cur_net,
dev_net_ns). I don't suppose it makes any real bugs reachable.

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net-next 3/6] selftests: drv-net: psp: check the ifindex an associated netns sees

[Daniel Zahka]

On Sat Sep 12, 2026 at 4:04 PM EDT, Jakub Kicinski wrote:
> dev-get must not report the ifindex outside of the main netns.
> The dev-get checks for an associated namespace are already there,
> add the "no main ifindex" assertion.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  tools/testing/selftests/drivers/net/psp.py | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
> index 5e87fb50c348..43780efb84ed 100755
> --- a/tools/testing/selftests/drivers/net/psp.py
> +++ b/tools/testing/selftests/drivers/net/psp.py
> @@ -749,6 +749,10 @@ from lib.py import ip
>  
>          ksft_not_none(peer_dev, "No PSP device found with by-association flag in guest netns")
>  
> +        # ifindex of the main netdevice means nothing in this namespace
> +        ksft_true('ifindex' not in peer_dev,
> +                  "ifindex reported to an associated namespace")
> +
>          # Verify assoc-list contains the nk_guest device
>          ksft_true('assoc-list' in peer_dev and len(peer_dev['assoc-list']) > 0,
>                    "Guest device should have assoc-list with local devices")

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net-next 4/6] psp: notify about a disassociation once it has happened

[Daniel Zahka]

On Sat Sep 12, 2026 at 4:04 PM EDT, Jakub Kicinski wrote:
> The dev-change-ntf generated by dev-disassoc was built before the
> association was unlinked, so the assoc-list it carried still contained
> the device which was going away, and nothing corrected it afterwards.
> We don't really expect associations to change during a lifetime of
> a netns but this is still wrong. Netlink listeners need to be able
> to tell the current "state of the world" based on notifications.
>
> The notification had to be sent early because psp_nl_multicast_per_ns()
> derives the set of namespaces to notify from the association list, so
> a namespace losing its last associated device becomes unreachable once
> the entry is gone. In that case - get the netns from the netdev itself,
> and send that namespace a dev-del-ntf. If the netns had multiple
> associated netdevs and only one disassoc'd we'll still send a change
> notification, just with a correct list.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net-next 5/6] selftests: drv-net: psp: factor out creating a netkit in the test netns

[Daniel Zahka]

On Sat Sep 12, 2026 at 4:04 PM EDT, Jakub Kicinski wrote:
> The netkit removal test builds a disposable netkit pair and moves its
> peer into the test namespace. The next commit needs a second associated
> device there, so move that to a helper. No functional change, other
> than looking the new peer up among all netkit devices rather than the
> two the environment created, which is what makes it reusable.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net-next 6/6] selftests: drv-net: psp: check the PSP disassociation notifications

[Daniel Zahka]

On Sat Sep 12, 2026 at 4:04 PM EDT, Jakub Kicinski wrote:
> The main namespace must see a change which no longer lists the device,
> and the namespace which lost its last association must see the device
> go away. Check that on both paths which generate the notifications,
> dev-disassoc and netdevice removal, and check that a namespace which
> still has another association is only told about the change.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net-next 2/6] psp: don't report the main netdevice's ifindex to associated namespaces

[Jakub Kicinski]

On Mon, 14 Sep 2026 08:02:23 -0400 Daniel Zahka wrote:
> As an aside, this got me looking at psp_nl_fill_assoc_dev_list() again.
> The PSP_A_ASSOC_DEV_INFO_NSID handling there looks a bit buggy. Caller
> sees -1 when the assoc dev is in their namespace, unless they are in the
> psp_dev's main_netdev's netns. In that case, a self referential nsid is
> allocated with peernet2id_alloc(). It probably would have made more
> sense to just only include PSP_A_ASSOC_DEV_INFO_NSID if !net_eq(cur_net,
> dev_net_ns). I don't suppose it makes any real bugs reachable.

Ack, I looked at the netns id situation briefly but deferred.
I think we need a split between the "netns went away" and ENOMEM
cases, and then have the dump act accordingly. This is a problem
all over the uAPI tho, so felt quite orthogonal.

Re: [PATCH net-next 2/6] psp: don't report the main netdevice's ifindex to associated namespaces

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 3

- [Medium] psp_nl_dev_fill() (net/psp/psp_nl.c:294-312) stops emitting
  PSP_A_DEV_IFINDEX in every dev-get reply / dump entry / dev-…
- [Low] The emitter contract changed (PSP_A_DEV_IFINDEX is now optional
  in dev-get replies/dumps) but the in-tree consumers still treat it as…
- [Low] As of this patch alone, psp.yaml documents that PSP_A_DEV_IFINDEX
  is 'Only reported to the network namespace the main netdevice lives…
- [Low] psp_nl_dev_fill() (net/psp/psp_nl.c:301-305) decides whether to
  report PSP_A_DEV_IFINDEX with `cur_net == dev_net(psd->main_netdev)`…

--

psp: don't report the main netdevice's ifindex to associated namespaces

PSP_A_DEV_IFINDEX is moved inside the branch that handles the main
netdevice's namespace in psp_nl_dev_fill(), so namespaces which only see
the PSP device by association no longer get the main netdev's ifindex.
The psp.yaml spec documentation for the attribute is updated to describe
the new rule.

A few questions below.

> diff --git a/Documentation/netlink/specs/psp.yaml b/Documentation/netlink/specs/psp.yaml
> index e9c2ee7e28e02..f3266763c3253 100644
> --- a/Documentation/netlink/specs/psp.yaml
> +++ b/Documentation/netlink/specs/psp.yaml
> @@ -38,6 +38,8 @@ attribute-sets:
>          doc: |
>            ifindex of the main netdevice linked to the PSP device,
>            or the ifindex to associate with the PSP device.
> +          Only reported to the network namespace the main netdevice
> +          lives in, an ifindex has no meaning outside of it.
>          type: u32

[Severity: Low]
As of this commit, is the newly documented rule verified anywhere in
tree?  The only test which inspects dev-get output taken from an
associated namespace is _psp_dev_get_check_netkit_psp_assoc() (Check 2)
in tools/testing/selftests/drivers/net/psp.py, and at this commit it only
looks for the by-association entry and validates the assoc-list.

Note the following patch in the series, "selftests: drv-net: psp: check
the ifindex an associated netns sees", adds exactly the missing
assertions:

        ksft_true('ifindex' not in peer_dev,
                  "ifindex reported to an associated namespace")

and the equivalent check in _check_disassoc_ntf(), so the gap is closed
before the end of the series.  Mentioning this only for completeness.

> diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
> index f91665748dde0..b57366b5e032b 100644
> --- a/net/psp/psp_nl.c
> +++ b/net/psp/psp_nl.c
> @@ -294,13 +294,16 @@ psp_nl_dev_fill(struct psp_dev *psd, struct sk_buff *rsp,
>  		return -EMSGSIZE;
>  
>  	if (nla_put_u32(rsp, PSP_A_DEV_ID, psd->id) ||
> -	    nla_put_u32(rsp, PSP_A_DEV_IFINDEX, psd->main_netdev->ifindex) ||
>  	    nla_put_u32(rsp, PSP_A_DEV_PSP_VERSIONS_CAP, psd->caps->versions) ||
>  	    nla_put_u32(rsp, PSP_A_DEV_PSP_VERSIONS_ENA, psd->config.versions))
>  		goto err_cancel_msg;
>  
>  	if (cur_net == dev_net(psd->main_netdev)) {
> -		/* Primary device - dump assoc list */
> +		/* Primary device - report the netdev, dump assoc list. */
> +		if (nla_put_u32(rsp, PSP_A_DEV_IFINDEX,
> +				psd->main_netdev->ifindex))
> +			goto err_cancel_msg;
> +

[Severity: Medium]
Should this carry a Fixes: tag?

The unconditional PSP_A_DEV_IFINDEX in psp_nl_dev_fill() has been in the
dev-get reply, the dump entries and the dev add/change/del notifications
since 06c2dce2d0f69 ("psp: add new netlink cmd for dev-assoc and
dev-disassoc"), which was based on v7.1-rc7 and therefore shipped in the
released v7.2.  The tree here is v7.3.0-rc2.

That means the message layout narrowed between two released kernels for
the same command: an application built against v7.2 sees the attribute in
by-association messages, while v7.3 and later omit it, and netlink gives
userspace no way to probe for the difference.

Would adding

  Fixes: 06c2dce2d0f69 ("psp: add new netlink cmd for dev-assoc and dev-disassoc")

plus a sentence in the changelog about the userspace-visible narrowing be
appropriate, so the correction lands in the same stable trees as the
feature?

[Severity: Low]
With the attribute now optional, do the in-tree consumers still handle a
dump that mixes entries with and without it?

tools/testing/selftests/drivers/net/psp.py, _init_psp_dev():

        for dev in cfg.pspnl.dev_get({}, dump=True):
            if dev['ifindex'] == target_ifindex:

ynl omits absent attributes, so a dump containing a by-association entry
raises a Python KeyError here and aborts the test file, where before this
patch it fell through to the clean:

        raise KsftSkipEx("No PSP devices found")

Would dev.get('ifindex') be better?  In the suite as configured every
_init_psp_dev() call uses the host-namespace socket, so the current tests
should not hit it, but the dump is no longer guaranteed to contain the
key.

The same applies more mildly to tools/testing/selftests/drivers/net/
psp_responder.c, in the -i selection loop in main():

        if (opts.ifindex) {
            if (d->ifindex != opts.ifindex)
                continue;

With the attribute absent d->ifindex is 0, so such a device is silently
skipped and the responder reports the device as not found.  This one is
not a behaviour regression as such, since the top-level ifindex it
compared previously belonged to another namespace anyway.

[Severity: Low]
Can the namespace test and the ifindex read disagree here?

The branch predicate reads dev_net(psd->main_netdev) and the attribute
then performs a second, independent read of psd->main_netdev->ifindex.
The psp family is registered with parallel_ops and the dev-get and
notification paths take only psd->lock, no RTNL, while a concurrent
migration in netif_change_net_namespace() publishes the netns before the
new ifindex:

net/core/dev.c:netif_change_net_namespace() {
    ...
    netdev_lock(dev);
    dev_net_set(dev, net);
    netdev_unlock(dev);
    dev->ifindex = new_ifindex;
    ...
}

Nothing in psp re-pins the psd on a netns move - psp_netdev_event() bails
out for anything other than NETDEV_UNREGISTER:

net/psp/psp_main.c:psp_netdev_event() {
    if (event != NETDEV_UNREGISTER)
        return NOTIFY_DONE;
    ...
}

so a reader can pass the test with the old netns and then serialize the
ifindex the device has in its new namespace, which is the case the doc
hunk declares meaningless.  Symmetrically, a notification built for the
main_net already pinned by psp_nl_multicast_per_ns() can flip to the
by-association encoding, since psp_nl_dev_fill() re-reads dev_net()
rather than using that pinned net:

net/psp/psp_nl.c:psp_nl_multicast_per_ns() {
    /* device may be changing netns in parallel */
    rcu_read_lock();
    main_net = maybe_get_net(dev_net_rcu(psd->main_netdev));
    rcu_read_unlock();
    ...
}

Would reading the netns and the ifindex once under a single
rcu_read_lock() with dev_net_rcu(), or passing the already-pinned
main_net down into psp_nl_dev_fill(), close the window?  The
consequence is only a stale or foreign u32 in one message.

>  		err = psp_nl_fill_assoc_dev_list(psd, rsp, cur_net, NULL);
>  		if (err)
>  			goto err_cancel_msg;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260912200426.121025-1-kuba%40kernel.org

Re: [PATCH net-next 4/6] psp: notify about a disassociation once it has happened

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 2

- [Low] The patch establishes 'PSP device stopped being visible in a
  netns' => PSP_CMD_DEV_DEL_NTF (net/psp/psp_nl.c:381-388), but the…
- [Low] The commit fixes user-visible Generic Netlink behaviour ('the
  assoc-list it carried still contained the device which was going away…

--

psp: notify about a disassociation once it has happened

The disassociation notification is moved so it is generated after the
association entry has been unlinked, and a netns which loses its last
associated device is sent a dev-del-ntf instead (using the netns taken
from the netdev). Both the netlink dev-disassoc path and the
NETDEV_UNREGISTER path are converted to the new helper.

> diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
> index 91473f96ad211..273b010d2355c 100644
> --- a/net/psp/psp_main.c
> +++ b/net/psp/psp_main.c

[ ... ]

> @@ -438,9 +440,13 @@ static int psp_netdev_event(struct notifier_block *nb, unsigned long event,
>  	if (psd && psp_dev_tryget(psd)) {
>  		rcu_read_unlock();
>  		mutex_lock(&psd->lock);
> -		if (psp_dev_is_registered(psd))
> -			psp_nl_notify_dev(psd, PSP_CMD_DEV_CHANGE_NTF);
> -		psp_dev_disassoc_one(psd, dev);
> +		/* Nothing to report if the device was never on the list,
> +		 * dev-assoc may have failed after publishing dev->psp_dev,
> +		 * and this is also the main netdevice's path.
> +		 */
> +		if (psp_dev_disassoc_one(psd, dev) &&
> +		    psp_dev_is_registered(psd))
> +			psp_nl_notify_disassoc(psd, dev_net(dev));
>  		mutex_unlock(&psd->lock);
>  		psp_dev_put(psd);
>  	} else {

> diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
> index b57366b5e032b..cdfc2d72fb390 100644
> --- a/net/psp/psp_nl.c
> +++ b/net/psp/psp_nl.c
> @@ -356,6 +356,40 @@ void psp_nl_notify_dev(struct psp_dev *psd, u32 cmd)
>  				psp_nl_build_dev_ntf, &cmd);
>  }
>  
> +/**
> + * psp_nl_notify_disassoc() - notify about a device losing an association
> + * @psd: PSP device (must be locked)
> + * @net: netns of the netdevice which got disassociated
> + *
> + * Must be called once @psd no longer has the association, so that the
> + * notifications carry the state after the change.
> + */
> +void psp_nl_notify_disassoc(struct psp_dev *psd, struct net *net)
> +{
> +	struct sk_buff *ntf;
> +	bool still_visible;
> +	u32 cmd;
> +
> +	lockdep_assert_held(&psd->lock);
> +
> +	psp_nl_notify_dev(psd, PSP_CMD_DEV_CHANGE_NTF);
> +
> +	/* psp_nl_notify_dev() reaches the main netdevice's netns and every
> +	 * netns which still has an associated device. If @net is neither,
> +	 * the device is gone from @net and we should send a delete ntf.
> +	 */
> +	still_visible = !psp_dev_check_access(psd, net, false);
> +	if (still_visible || !maybe_get_net(net))
> +		return;
> +
> +	cmd = PSP_CMD_DEV_DEL_NTF;
> +	ntf = psp_nl_build_dev_ntf(psd, net, &cmd);
> +	if (ntf)
> +		genlmsg_multicast_netns(&psp_nl_family, net, ntf, 0,
> +					PSP_NLGRP_MGMT, GFP_KERNEL);
> +	put_net(net);
> +}

[Severity: Low]

This isn't a bug, but should the assoc side be made symmetric with this?

After this change, losing visibility of a PSP device in a netns produces
PSP_CMD_DEV_DEL_NTF, while gaining visibility for the first time still
produces only a change notification in psp_nl_dev_assoc_doit():

	list_add_tail(&psp_assoc_dev->dev_list, &psd->assoc_dev_list);
	psd->assoc_dev_cnt++;

	put_net(net);

	psp_nl_notify_dev(psd, PSP_CMD_DEV_CHANGE_NTF);

For a netns where this is the first association, psp_dev_check_access(psd,
net, false) would have returned -ENOENT just before the list_add_tail(),
i.e. the device was not visible there at all, yet the netns is only told
about it with a change notification.

Documentation/netlink/specs/psp.yaml describes dev-add-ntf as "Notification
about device appearing" and dev-del-ntf as "Notification about device
disappearing", and psp_dev_create()/psp_dev_unregister() use that add/del
pair. A mgmt listener in an associated namespace would then get a change-ntf
for an object it was never told about, followed later by a del-ntf for it.

Since notification semantics become uAPI once released, would it make sense
to emit PSP_CMD_DEV_ADD_NTF to a namespace which becomes able to see the
device for the first time?

> +
>  int psp_nl_dev_get_doit(struct sk_buff *req, struct genl_info *info)
>  {
>  	struct psp_dev *psd = info->user_ptr[0];

[ ... ]

> @@ -620,13 +654,6 @@ int psp_nl_dev_disassoc_doit(struct sk_buff *skb, struct genl_info *info)
>  		return -ENOMEM;
>  	}
>  
> -	put_net(net);
> -
> -	/* Notify before removal so listeners in the disassociated namespace
> -	 * still receive the notification.
> -	 */
> -	psp_nl_notify_dev(psd, PSP_CMD_DEV_CHANGE_NTF);
> -
>  	/* Remove from the association list */
>  	list_del(&found->dev_list);
>  	psd->assoc_dev_cnt--;
> @@ -634,6 +661,9 @@ int psp_nl_dev_disassoc_doit(struct sk_buff *skb, struct genl_info *info)
>  	netdev_put(found->assoc_dev, &found->dev_tracker);
>  	kfree(found);
>  
> +	psp_nl_notify_disassoc(psd, net);
> +	put_net(net);
> +
>  	return psp_nl_reply_send(rsp, info);
>  }

[Severity: Low]

Should this carry Fixes: tags?

The changelog says the old ordering "is still wrong", and the corrected
behaviour is user-visible netlink content in two paths that both exist in
released code:

- psp_nl_dev_disassoc_doit(), the notify-before-removal block removed here,
  came from commit 06c2dce2d0f6 ("psp: add new netlink cmd for dev-assoc
  and dev-disassoc")
- psp_netdev_event(), converted to psp_nl_notify_disassoc(), came from
  commit 0ddb69e2406e ("psp: add a new netdev event for dev unregister")

Documentation/process/maintainer-netdev.rst says "for fixes the ``Fixes:``
tag is required, regardless of the tree", so adding the two trailers would
make stable selection and backporting reliable.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260912200426.121025-1-kuba%40kernel.org

Re: [PATCH net-next 6/6] selftests: drv-net: psp: check the PSP disassociation notifications

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] `_dev_disassoc_notify_one_of_two_netkit()` in
  tools/testing/selftests/drivers/net/psp.py does not verify the…

--

selftests: drv-net: psp: check the PSP disassociation notifications

Adds two new psp.py subtests which subscribe to the mgmt multicast group in
the main and guest namespaces, then verify the notifications generated by
dev-disassoc and by netdevice removal.  Also extends
_psp_dev_assoc_cleanup_on_netkit_del() to check the same notifications.

> diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
> index d44d2c83473ee..13ff5188cca3f 100755
> --- a/tools/testing/selftests/drivers/net/psp.py
> +++ b/tools/testing/selftests/drivers/net/psp.py

[ ... ]

> @@ -723,6 +723,111 @@ def _dev_change_notify_multi_ns_netkit(cfg):

[ ... ]

> +def _get_dev_ntf(cfg, pspnl, label):
> +    """Wait for the next notification about the PSP device under test."""
> +    for ntf in pspnl.poll_ntf(duration=10):
> +        if ntf['msg'].get('id') == cfg.psp_dev_id:
> +            return ntf
> +    raise KsftFailEx(f"No notification received in the {label} namespace")
> +
> +
> +def _check_disassoc_ntf(cfg, main_pspnl, peer_pspnl, ifindex):
> +    """Check the notifications for a netns losing its last association."""
> +    ntf = _get_dev_ntf(cfg, main_pspnl, "main")
> +    ksft_eq(ntf['name'], 'dev-change-ntf')
> +    for assoc in ntf['msg'].get('assoc-list', []):
> +        if assoc['nsid'] != cfg.psp_dev_peer_nsid:
> +            continue
> +        ksft_ne(assoc['ifindex'], ifindex,
> +                "Disassociated device still listed in the notification")
> +
> +    # The device is gone as far as the disassociated namespace is concerned
> +    ntf = _get_dev_ntf(cfg, peer_pspnl, "guest")
> +    ksft_eq(ntf['name'], 'dev-del-ntf')
> +    ksft_true('ifindex' not in ntf['msg'],
> +              "ifindex reported to an associated namespace")

[ ... ]

> +def _dev_disassoc_notify_one_of_two_netkit(cfg):
> +    """Test the notifications with two netkits associated in one netns.
> +
> +    Disassociating the first netkit leaves the PSP device visible in the
> +    guest namespace, generates a dev-change-ntf.
> +    Disassociating the second one takes the device out of its view,
> +    generates 'dev-del-ntf'.
> +    """

[ ... ]

> +    main_pspnl, peer_pspnl = _subscribe_mgmt(cfg)
> +
> +    # One of the two goes away, the device stays visible in the guest netns
> +    cfg.pspnl.dev_disassoc({'id': cfg.psp_dev_id, 'ifindex': tmp_ifindex,
> +                            'nsid': cfg.psp_dev_peer_nsid})
> +
> +    ntf = _get_dev_ntf(cfg, main_pspnl, "main")
> +    ksft_eq(ntf['name'], 'dev-change-ntf')
> +
> +    ntf = _get_dev_ntf(cfg, peer_pspnl, "guest")
> +    ksft_eq(ntf['name'], 'dev-change-ntf')
> +    found = False
> +    for assoc in ntf['msg'].get('assoc-list', []):
> +        ksft_ne(assoc['ifindex'], tmp_ifindex,
> +                "Disassociated device still listed in the notification")
> +        found |= assoc['ifindex'] == cfg.nk_guest_ifindex
> +    ksft_true(found, "Remaining association missing from the notification")

[Severity: Medium]

Does this block actually check the property the commit message describes,
that "a namespace which still has another association is only told about
the change"?

_get_dev_ntf() returns as soon as it sees one message for the device, so
only the first guest notification is inspected.  If the kernel also sent a
dev-del-ntf to the guest here (i.e. still_visible in
psp_nl_notify_disassoc() were computed wrongly), nothing in this phase
would notice it.

The stale message is not just ignored, it is later consumed as if it were
fresh.  ynl queues notifications that poll_ntf() did not hand out:

tools/net/ynl/pyynl/lib/ynl.py:poll_ntf() {
	...
	while True:
		try:
			yield self.async_msg_queue.get_nowait()
	...
}

so a bogus dev-del-ntf from the first dev_disassoc() stays in
peer_pspnl's queue, and the second phase picks it up:

_check_disassoc_ntf() {
	...
	ntf = _get_dev_ntf(cfg, peer_pspnl, "guest")
	ksft_eq(ntf['name'], 'dev-del-ntf')
	ksft_true('ifindex' not in ntf['msg'],
		  "ifindex reported to an associated namespace")
}

Both assertions pass on the stale message, because psp_nl_dev_fill() only
emits PSP_A_DEV_IFINDEX when cur_net == dev_net(psd->main_netdev), so any
del-ntf destined for a non-main netns lacks ifindex.

That leaves the test only able to catch a missing notification, not an
extra one.  Would it make sense to drain both sockets after the first
change-ntf and assert that no further notification for this device arrives
(a short poll_ntf() that must yield nothing), before running the second
dev_disassoc()?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260912200426.121025-1-kuba%40kernel.org