[PATCH net 0/5] netlink: fixes for cross-namespace nsid reporting

[PATCH net 0/5] netlink: fixes for cross-namespace nsid reporting

[Ilya Maximets]

While working on some new features for OVS and OVN we discovered that
self-referential NSIDs get unintentionally allocated in the system as
well as unexpectedly reported for local events on all-nsid listeners.

More details in the patches.  They change user-visible behavior, but
the current behavior is arguably a bug, as it makes it hard to use
all-nsid sockets without a decent amount of extra unrelated work of
tracking when new NSIDs are allocated for your local namespace.

Tests are added to check the expected behavior and YNL is extended to
support all-nsid sockets in the tests.

Ilya Maximets (5):
  net: rtnetlink: fix link nsid reported when the link is local
  selftests: net: add a test case for cross-namespace peer netns
  net: netlink: don't set nsid on local notifications
  tools: ynl: support listening on all nsids
  selftests: net: add a test case for nsid in all nsid notifications

 net/core/rtnetlink.c                      |   2 +-
 net/netlink/af_netlink.c                  |   8 +-
 tools/net/ynl/pyynl/lib/ynl.py            |  32 +++++--
 tools/testing/selftests/net/link_netns.py | 101 +++++++++++++++++++++-
 4 files changed, 132 insertions(+), 11 deletions(-)

-- 
2.53.0

[PATCH net 1/5] net: rtnetlink: fix link nsid reported when the link is local

[Ilya Maximets]

For most netlink replies and notifications the expected behavior is:

- if NSID is not reported, then the device is local to the querier.
- if NSID is reported, then the device is remote, i.e., located in
  the provided namespace that is not the same as the querier's.

Userspace applications like ovs-vswitchd expect this behavior.  And
ip monitor uses this logic for printing out [nsid current] vs [nsid N].

But this doesn't work for link nsid in cross-namespace RTM_GETLINK
requests.  For some reason the code checks if the original device
and the link are in the same namespace and not if the querier's
namespace is the same as the link's.  So the logic becomes:

- if NSID is not reported, then the link is in the same namespace
  as the queried device.
- if NSID is reported, then the link is not in the same namespace
  with the queried device.

If the link is in the same namespace as the querier, the code will
allocate a self-referential nsid for the querier's namespace and
report it as a link nsid.  This is problematic because:

1. Application doesn't expect to see nsid reported for its own
   namespace.

2. Application can't know if this nsid is the nsid of the current
   namespace without making extra requests.  So a lot of extra
   logic is needed to understand if the link is local or not.

3. Implicit allocation of self-referential nsid for the current
   namespace affects notifications on sockets listening on all
   namespaces, since this nsid is now reported in every notification.
   And so those notification handlers also now need extra logic to
   understand which namespace the events are coming from.

4. A seemingly read-only RTM_GETLINK request for a different namespace
   allocates a self-referential nsid for the current namespace, which
   is a little unexpected.

Let's fix that by applying the same rules to cross-namespace requests
as for standard ones, which is:

- Report NSID if it is different from the querier's namespace.

This changes two things:

1. If both the device and the link are in the same namespace which
   is not the querier's namespace, the LINK_NSID will now be reported.
   This just gives more info and the user can check if the reported
   id is the same as TARGET_NSID, which is in the same message.

2. If the link is in the same namespace as the querier, but the
   device isn't, the LINK_NSID will no longer be reported.
   This is the main change, as previously this would mean that the
   link is in the TARGET namespace, but now it will mean that the
   link is in the SOURCE namespace.

There are no changes in logic for queries that are not cross-namespace
queries.

A research across open-source projects doesn't show any projects that
rely on the things that are being changed.  I couldn't find any
project that uses the reported LINK_NSID with cross-namespace requests.
And no projects that use cross-namespace requests seem to even parse
the reported LINK_NSID.

Of course, that doesn't mean there are no such applications, but the
current behavior feels like a logical bug that IMO should be fixed,
otherwise it's hard to use all-nsid sockets properly.

Note that the logic for notifications in rtmsg_ifinfo_build_skb()
remains the same as those are formatted from the perspective of the
namespace where event occurred, i.e., they are always "local", but
then distributed to sockets listening on all NSIDs with extra metadata
pointing out the original namespace.  And users need to translate
the reported NSIDs to their reference point.  While RTM_GETLINK
always reports NSID from the querier's reference point.  Hence the
reason it should not be reported if it is the same.

Fixes: 79e1ad148c84 ("rtnetlink: use netnsid to query interface")
Reported-by: Matteo Perin <matteo.perin@canonical.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 net/core/rtnetlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index df042da422ef3..0d539b8e4bf61 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1881,7 +1881,7 @@ static int rtnl_fill_link_netnsid(struct sk_buff *skb,
 	if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) {
 		struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
 
-		if (!net_eq(dev_net(dev), link_net)) {
+		if (!net_eq(src_net, link_net)) {
 			int id = peernet2id_alloc(src_net, link_net, gfp);
 
 			if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
-- 
2.53.0

[PATCH net 2/5] selftests: net: add a test case for cross-namespace peer netns

[Ilya Maximets]

The test makes requests with RTM_GETLINK and TARGET_NETNS and verifies
that reported LINK_NSID is correct and only reported when it is needed
from the querier's perspective.

Assisted-by: OpenCode:claude-opus-4.6
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 tools/testing/selftests/net/link_netns.py | 49 ++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/link_netns.py b/tools/testing/selftests/net/link_netns.py
index aab043c59d695..2aae422d3f8a6 100755
--- a/tools/testing/selftests/net/link_netns.py
+++ b/tools/testing/selftests/net/link_netns.py
@@ -3,13 +3,14 @@
 
 import time
 
-from lib.py import ksft_run, ksft_exit, ksft_true
+from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_true
 from lib.py import ip
 from lib.py import NetNS, NetNSEnter
 from lib.py import RtnlFamily
 
 
 LINK_NETNSID = 100
+LINK_NETNSID2 = 200
 
 
 def test_event() -> None:
@@ -132,8 +133,52 @@ def test_peer_net() -> None:
                 ip(f"link del foo", ns=tgt_net)
 
 
+def test_peer_net_cross_ns() -> None:
+    """Cross-namespace RTM_GETLINK queries using target-netnsid.
+    IFLA_LINK_NETNSID should report the link peer's namespace from the
+    querier's perspective.  Absent means the peer is in the querier's
+    own namespace.  Must not create self-referential nsid mappings."""
+
+    with NetNS() as ns1, NetNS() as ns2, NetNS() as ns3:
+        net1, net2, net3 = str(ns1), str(ns2), str(ns3)
+
+        ip(f"netns set {net2} {LINK_NETNSID}", ns=net1)
+        ip(f"netns set {net3} {LINK_NETNSID2}", ns=net1)
+
+        with NetNSEnter(net1):
+            rtnl = RtnlFamily()
+
+        cases = [
+            # "dev netns", "peer netns", "query nsid", expected link-netnsid.
+            (net2, net1, LINK_NETNSID, None),
+            (net2, net2, LINK_NETNSID, LINK_NETNSID),
+            (net2, net3, LINK_NETNSID, LINK_NETNSID2),
+        ]
+
+        for dev_ns, peer_ns, query_nsid, exp in cases:
+            ip(f"link add foo netns {dev_ns} type veth"
+               f" peer name bar netns {peer_ns}")
+
+            resp = rtnl.getlink({"target-netnsid": query_nsid,
+                                 "ifname": "foo"})
+            ksft_eq(resp.get("link-netnsid"), exp,
+                    f"link-netnsid mismatch for dev={dev_ns} peer={peer_ns}")
+
+            ip("link del foo", ns=dev_ns)
+
+        # Verify no extra nsid was created by the queries.
+        nsids = ip("netns list-id", ns=net1, json=True)
+        ksft_eq(len(nsids), 2,
+                f"unexpected nsid mappings after cross-ns queries: {nsids}")
+
+
 def main() -> None:
-    ksft_run([test_event, test_link_net, test_peer_net])
+    ksft_run([
+        test_event,
+        test_link_net,
+        test_peer_net,
+        test_peer_net_cross_ns,
+    ])
     ksft_exit()
 
 
-- 
2.53.0

[PATCH net 3/5] net: netlink: don't set nsid on local notifications

[Ilya Maximets]

For notifications with NETLINK_LISTEN_ALL_NSID the expected behavior
is the following:

- if NSID is not reported, then the event is local to the listener.
- if NSID is reported, then the event is remote, i.e., originated in
  the provided namespace that is not the same as the listener's.

Userspace applications like ovs-vswitchd expect this behavior.  And
ip monitor uses this logic for printing out [nsid current] vs [nsid N].

However, when a self-referential NSID is allocated for a namespace,
every local notification starts sending this ID to userspace as part
of NETLINK_LISTEN_ALL_NSID CMSG metadata.

This is problematic, because the listener cannot tell if those
notifications are local or not anymore without making extra requests
to figure out if the provided NSID is local or not.  The listener
can also not figure out the local NSID beforehand as it can be
allocated at any point in time by other processes.

The value is practically not useful, since it's the namespace's own
ID that the application has to obtain from other sources in order to
figure out if it's the same or not.  So, for the application it's
just an extra busy work with no benefits.  Moreover, applications
that do not know about this quirk may be mishandling notifications
with NSID set as notifications from remote namespaces while they
are actually local.  This is the case with ovs-vswitchd.

Having a self-referential NSID mapping is not something that happens
under normal circumstances, but it can be a case in specific
environments.  And it can be more common with certain container
runtimes like LXC/LXD/Incus that unintentionally trigger allocation
of the self-referential NSID via cross-namespace RTM_GETLINK requests.

A search though open-source projects doesn't reveal any projects
that use NETNSA_NSID_NOT_ASSIGNED and rely on metadata to contain
self-referential NSIDs.  Quite the opposite, ovs-vswitchd relies
on the metadata to not be present to separate local and remote
events.  And the 'ip monitor' relies on the metadata to not be present
to show '[nsid current]', though this is more like "print 'current'
if there is nothing to print" situation, but still can be a little
confusing for the user to see an ID for a local event.

Fixes: 59324cf35aba ("netlink: allow to listen "all" netns")
Reported-by: Matteo Perin <matteo.perin@canonical.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 net/netlink/af_netlink.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 2aeb0680807d6..607ab4e4ac697 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1482,9 +1482,11 @@ static void do_one_broadcast(struct sock *sk,
 		p->skb2 = NULL;
 		goto out;
 	}
-	NETLINK_CB(p->skb2).nsid = peernet2id(sock_net(sk), p->net);
-	if (NETLINK_CB(p->skb2).nsid != NETNSA_NSID_NOT_ASSIGNED)
-		NETLINK_CB(p->skb2).nsid_is_set = true;
+	if (!net_eq(sock_net(sk), p->net)) {
+		NETLINK_CB(p->skb2).nsid = peernet2id(sock_net(sk), p->net);
+		if (NETLINK_CB(p->skb2).nsid != NETNSA_NSID_NOT_ASSIGNED)
+			NETLINK_CB(p->skb2).nsid_is_set = true;
+	}
 	val = netlink_broadcast_deliver(sk, p->skb2);
 	if (val < 0) {
 		netlink_overrun(sk);
-- 
2.53.0

[PATCH net 4/5] tools: ynl: support listening on all nsids

[Ilya Maximets]

A new method ntf_listen_all_nsid() to enable listening on events from
all namespaces.  Useful for testing cross-namespace functionality.

recv() replaced with recvmsg() to be able to receive NSID through the
ancillary data.

Assisted-by: OpenCode:claude-opus-4.6
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 tools/net/ynl/pyynl/lib/ynl.py | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
index f63c6f8287359..6d96c2b06507e 100644
--- a/tools/net/ynl/pyynl/lib/ynl.py
+++ b/tools/net/ynl/pyynl/lib/ynl.py
@@ -42,6 +42,7 @@ class Netlink:
     SOL_NETLINK = 270
 
     NETLINK_ADD_MEMBERSHIP = 1
+    NETLINK_LISTEN_ALL_NSID = 8
     NETLINK_CAP_ACK = 10
     NETLINK_EXT_ACK = 11
     NETLINK_GET_STRICT_CHK = 12
@@ -748,6 +749,21 @@ class YnlFamily(SpecFamily):
         self.sock.setsockopt(Netlink.SOL_NETLINK, Netlink.NETLINK_ADD_MEMBERSHIP,
                              mcast_id)
 
+    def ntf_listen_all_nsid(self):
+        self.sock.setsockopt(Netlink.SOL_NETLINK,
+                             Netlink.NETLINK_LISTEN_ALL_NSID, 1)
+
+    @staticmethod
+    def _decode_nsid(ancdata):
+        for cmsg_level, cmsg_type, cmsg_data in ancdata:
+            if (cmsg_level == Netlink.SOL_NETLINK and
+                    cmsg_type == Netlink.NETLINK_LISTEN_ALL_NSID):
+                nsid = struct.unpack('i', cmsg_data)[0]
+                if nsid >= 0:
+                    return nsid
+                return None
+        return None
+
     def set_recv_dbg(self, enabled):
         self._recv_dbg = enabled
 
@@ -1235,7 +1251,7 @@ class YnlFamily(SpecFamily):
                             f" when parsing '{attr_spec['name']}'")
         return raw
 
-    def handle_ntf(self, decoded):
+    def handle_ntf(self, decoded, nsid=None):
         msg = {}
         if self.include_raw:
             msg['raw'] = decoded
@@ -1246,15 +1262,20 @@ class YnlFamily(SpecFamily):
 
         msg['name'] = op['name']
         msg['msg'] = attrs
+        if nsid is not None:
+            msg['nsid'] = nsid
         self.async_msg_queue.put(msg)
 
     def check_ntf(self):
         while True:
             try:
-                reply = self.sock.recv(self._recv_size, socket.MSG_DONTWAIT)
+                reply, ancdata, _, _ = self.sock.recvmsg(self._recv_size,
+                                                         4096,
+                                                         socket.MSG_DONTWAIT)
             except BlockingIOError:
                 return
 
+            nsid = self._decode_nsid(ancdata)
             nms = NlMsgs(reply)
             self._recv_dbg_print(reply, nms)
             for nl_msg in nms:
@@ -1271,7 +1292,7 @@ class YnlFamily(SpecFamily):
                     print("Unexpected msg id while checking for ntf", decoded)
                     continue
 
-                self.handle_ntf(decoded)
+                self.handle_ntf(decoded, nsid)
 
     def poll_ntf(self, duration=None):
         start_time = time.time()
@@ -1335,7 +1356,8 @@ class YnlFamily(SpecFamily):
         rsp = []
         op_rsp = []
         while not done:
-            reply = self.sock.recv(self._recv_size)
+            reply, ancdata, _, _ = self.sock.recvmsg(self._recv_size, 4096)
+            nsid = self._decode_nsid(ancdata)
             nms = NlMsgs(reply)
             self._recv_dbg_print(reply, nms)
             for nl_msg in nms:
@@ -1374,7 +1396,7 @@ class YnlFamily(SpecFamily):
                 # Check if this is a reply to our request
                 if nl_msg.nl_seq not in reqs_by_seq or decoded.cmd() != op.rsp_value:
                     if decoded.cmd() in self.async_msg_ids:
-                        self.handle_ntf(decoded)
+                        self.handle_ntf(decoded, nsid)
                         continue
                     print('Unexpected message: ' + repr(decoded))
                     continue
-- 
2.53.0

[PATCH net 5/5] selftests: net: add a test case for nsid in all nsid notifications

[Ilya Maximets]

The test makes subscribes to link events from all namespaces and
makes sure that local events do not carry NSID in their ancillary
data (even if there is a self-referential NSID allocated for the
local namespace), and remote events do.

Assisted-by: OpenCode:claude-opus-4.6
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 tools/testing/selftests/net/link_netns.py | 52 +++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/tools/testing/selftests/net/link_netns.py b/tools/testing/selftests/net/link_netns.py
index 2aae422d3f8a6..1e766154cb573 100755
--- a/tools/testing/selftests/net/link_netns.py
+++ b/tools/testing/selftests/net/link_netns.py
@@ -33,6 +33,57 @@ def test_event() -> None:
                   "Received unexpected link notification")
 
 
+def test_event_all_nsid() -> None:
+    """NETLINK_LISTEN_ALL_NSID notifications: local events must not
+    carry nsid even with a self-referential mapping.  Remote events
+    must carry the correct nsid."""
+
+    with NetNS() as ns1, NetNS() as ns2:
+        net1, net2 = str(ns1), str(ns2)
+
+        with NetNSEnter(net1):
+            rtnl = RtnlFamily()
+        rtnl.ntf_listen_all_nsid()
+        rtnl.ntf_subscribe("rtnlgrp-link")
+
+        # Case 1: no nsid assigned, local event, no nsid expected.
+        ip("link add dummy-lo type dummy", ns=net1)
+
+        # Case 2: self-referential nsid, local event, still no nsid.
+        ip(f"netns set {net1} {LINK_NETNSID}", ns=net1)
+        ip("link add dummy-sr type dummy", ns=net1)
+
+        # Case 3: remote event, nsid present.
+        ip(f"netns set {net2} {LINK_NETNSID2}", ns=net1)
+        ip("link add dummy-re type dummy", ns=net2)
+
+        # Collect the three newlink events, ignoring unrelated noise.
+        events = {}
+        for msg in rtnl.poll_ntf(duration=1):
+            if msg['name'] == 'getlink':
+                ifname = msg['msg'].get('ifname')
+                if ifname in ('dummy-lo', 'dummy-sr', 'dummy-re'):
+                    events[ifname] = msg
+            if len(events) == 3:
+                break
+
+        ksft_true('dummy-lo' in events, "missing local event")
+        ksft_true(events['dummy-lo'].get('nsid') is None,
+                  "local event without nsid should not carry nsid")
+
+        ksft_true('dummy-sr' in events, "missing self-ref event")
+        ksft_true(events['dummy-sr'].get('nsid') is None,
+                  "local event with self-ref nsid should not carry nsid")
+
+        ksft_true('dummy-re' in events, "missing remote event")
+        ksft_eq(events['dummy-re'].get('nsid'), LINK_NETNSID2,
+                "remote event should carry nsid")
+
+        ip("link del dummy-lo", ns=net1)
+        ip("link del dummy-sr", ns=net1)
+        ip("link del dummy-re", ns=net2)
+
+
 def validate_link_netns(netns, ifname, link_netnsid) -> bool:
     link_info = ip(f"-d link show dev {ifname}", ns=netns, json=True)
     if not link_info:
@@ -175,6 +226,7 @@ def test_peer_net_cross_ns() -> None:
 def main() -> None:
     ksft_run([
         test_event,
+        test_event_all_nsid,
         test_link_net,
         test_peer_net,
         test_peer_net_cross_ns,
-- 
2.53.0