[PATCH net-next v6 0/2] net/rds: scope-aware sendmsg connection cache

[PATCH net-next v6 0/2] net/rds: scope-aware sendmsg connection cache

[Allison Henderson]

Hi all,

This is v6 of the sendmsg connection-cache scope fix (v1 at [1], v2 at
[2], v3 at [3], v4 at [4], v5 at [5]), against net-next as Jakub
suggested: neither issue has been observed in the field, and both were
found by inspection while reworking this cache for the
connection-lifetime series.

  Patch 1 compares c_dev_if with the request's scope in the cache-hit
  test, so the cache uses the same key as rds_conn_lookup().

  Patch 2 gives the connected peer's link-local scope its own field.
  rds_connect() used to store it in rs_bound_scope_id, the scope of
  the socket's own bound address, which a later bind() to a
  non-link-local address resets to 0 and which a later connect()
  pollutes for a globally bound socket.  A send without a destination
  and getpeername() take the connected peer's scope from that field;
  recvmsg() reports a link-local sender with the interface recorded
  on its connection.

Changes since v5 [5]:
 - Swapped the two patches.  The scope-field patch lets a globally
   bound socket connect() to the same link-local peer through a second
   interface, and without the cache check in place first the next
   send() would reuse the first interface's connection, so the series
   is now bisectable (review of v5).
 - recvmsg() comment and changelog no longer describe c_dev_if as the
   arrival interface: that holds for TCP, while the IB passive side
   resolves it from the local address (review of v5).
 - No code change beyond the comment.

Changes since v4 [4]:
 - recvmsg() takes a link-local sender's scope from the connection the
   message arrived on (c_dev_if) and reports 0 for any other sender,
   instead of the socket's connected or bound scope - a connected
   socket receives from any peer, so a per-socket scope was wrong for
   every sender but the connected one and could send a reply out the
   wrong link (review of v4).  getpeername() and recvmsg() now agree
   for the connected peer.
 - Retargeted at net-next.

Changes since v3 [3]:
 - A destination-less send falls back to the bound scope for a
   non-link-local peer, so send() and sendto() to the connected peer
   compute the same scope; getpeername() and recvmsg() report the
   connected peer's scope instead of the bound scope.

[1] https://lore.kernel.org/netdev/20260921215046.174745-1-achender@kernel.org/
[2] https://lore.kernel.org/netdev/20260927062312.169097-1-achender@kernel.org/
[3] https://lore.kernel.org/netdev/20261003163526.250805-1-achender@kernel.org/
[4] https://lore.kernel.org/netdev/20261008031426.1142344-1-achender@kernel.org/
[5] https://lore.kernel.org/netdev/20261017032358.2563121-1-achender@kernel.org/

Thank you,
Allison

Allison Henderson (2):
  net/rds: include the scope id in the sendmsg connection cache check
  net/rds: keep the connected peer's scope id apart from the bound one

 net/rds/af_rds.c | 15 +++++++++------
 net/rds/bind.c   |  4 ++--
 net/rds/rds.h    |  2 ++
 net/rds/recv.c   | 11 ++++++++++-
 net/rds/send.c   |  9 +++++++--
 5 files changed, 30 insertions(+), 11 deletions(-)

-- 
2.50.1

[PATCH net-next v6 1/2] net/rds: include the scope id in the sendmsg connection cache check

[Allison Henderson]

rds_sendmsg() reuses the connection cached in rs->rs_conn when its
peer address and ToS match the request.  The interface index is part
of a connection's identity as well: rds_conn_create_outgoing() passes
the request's scope_id down as dev_if, and rds_conn_lookup() compares
c_dev_if, so sends to the same link-local address through two
interfaces are two different connections.  The cache-hit test never
looked at it.

A socket bound to a non-link-local address has rs_bound_scope_id 0,
and the scope check at the top of rds_sendmsg() accepts any non-zero
destination scope for such a socket.  So after a send to fe80::x%ifA,
a send to fe80::x%ifB hits the cached ifA connection and the datagram
leaves through ifA, to whichever peer answers to that address there.

Compare c_dev_if with the request's scope_id in the cache test, so
that such a send takes the lookup path and finds, or creates, the ifB
connection - the same key the lookup uses.  A send without a
destination takes its scope from the socket, and the connection it
created was keyed on that same scope, so the comparison holds for it
too; the next patch gives that send the connected peer's own scope.

Found by inspection while reworking this cache for the connection
lifetime series on net-next; not observed in the field.

Fixes: 1e2b44e78eea ("rds: Enable RDS IPv6 support")
Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
---
 net/rds/send.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/rds/send.c b/net/rds/send.c
index 1afa981e5c06..38b9cc9e0d20 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -1342,7 +1342,8 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
 	/* rds_conn_create has a spinlock that runs with IRQ off.
 	 * Caching the conn in the socket helps a lot. */
 	if (rs->rs_conn && ipv6_addr_equal(&rs->rs_conn->c_faddr, &daddr) &&
-	    rs->rs_tos == rs->rs_conn->c_tos) {
+	    rs->rs_tos == rs->rs_conn->c_tos &&
+	    rs->rs_conn->c_dev_if == scope_id) {
 		conn = rs->rs_conn;
 	} else {
 		conn = rds_conn_create_outgoing(sock_net(sock->sk),
-- 
2.50.1

[PATCH net-next v6 2/2] net/rds: keep the connected peer's scope id apart from the bound one

[Allison Henderson]

rds_connect() has nowhere to keep the scope of a link-local peer, so
it stores it in rs_bound_scope_id, the scope of the socket's own
bound address, for rds_bind() to check a later link-local bind
against.  That field is then also what a send without a destination
uses as the request's scope, what getpeername() and recvmsg() report
as the peer's scope, and what a later bind() overwrites:
rds_add_bound() stores the bound address's scope unconditionally,
which for a non-link-local address is 0.

So after connect(fe80::x%ifA) followed by bind(global), a send() with
no destination asks for fe80::x with scope 0.  rds_conn_lookup() keys
on the interface, so that finds or creates a connection with c_dev_if
0, which the TCP transport then tries to connect through
sin6_scope_id 0 and tcp_v6_connect() rejects for a link-local peer:
the connected socket's data is queued on a connection that can never
come up.  In the other order, bind(global) then connect(fe80::x%ifB),
the connect leaves a global-bound socket with rs_bound_scope_id ifB,
so sends to other link-local peers are refused as off-link and sends
to global peers inherit a meaningless interface.

Give the connected peer its own rs_conn_scope_id.  rds_connect()
records it there and leaves the bound scope alone, rds_bind()'s
connected-socket check compares against it, and the destination-less
send takes its scope from it - falling back to the bound scope for a
non-link-local peer, so that send() and sendto() to the connected
peer compute the same scope, and so the same connection.
getpeername() reports the connected peer with its own scope.
recvmsg() used to report every sender with the socket's bound scope;
a connected socket receives from any peer, so neither the bound scope
nor the connected peer's scope is right for an arbitrary sender.  It
now reports a link-local sender with the interface recorded on its
connection, c_dev_if: the TCP accept path takes it from the incoming
socket, and the IB passive side resolves it from the local address,
the closest the CM gets to the arrival interface.  Any other sender
is reported with scope 0, the inet6 convention.  Either way, a reply
to the address an application was given reaches the same connection.

Found by inspection while reworking the sendmsg connection cache; not
observed in the field.

Fixes: 1e2b44e78eea ("rds: Enable RDS IPv6 support")
Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
---
 net/rds/af_rds.c | 15 +++++++++------
 net/rds/bind.c   |  4 ++--
 net/rds/rds.h    |  2 ++
 net/rds/recv.c   | 11 ++++++++++-
 net/rds/send.c   |  6 +++++-
 5 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
index d5defe9172e3..e84d00bbe9a1 100644
--- a/net/rds/af_rds.c
+++ b/net/rds/af_rds.c
@@ -136,8 +136,7 @@ static int rds_getname(struct socket *sock, struct sockaddr *uaddr,
 			sin6->sin6_port = rs->rs_conn_port;
 			sin6->sin6_addr = rs->rs_conn_addr;
 			sin6->sin6_flowinfo = 0;
-			/* scope_id is the same as in the bound address. */
-			sin6->sin6_scope_id = rs->rs_bound_scope_id;
+			sin6->sin6_scope_id = rs->rs_conn_scope_id;
 			uaddr_len = sizeof(*sin6);
 		}
 	} else {
@@ -572,6 +571,7 @@ static int rds_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
 		}
 		ipv6_addr_set_v4mapped(sin->sin_addr.s_addr, &rs->rs_conn_addr);
 		rs->rs_conn_port = sin->sin_port;
+		rs->rs_conn_scope_id = 0;
 		break;
 
 #if IS_ENABLED(CONFIG_IPV6)
@@ -616,11 +616,14 @@ static int rds_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
 				ret = -EINVAL;
 				break;
 			}
-			/* Remember the connected address scope ID.  It will
-			 * be checked against the binding local address when
-			 * the socket is bound.
+			/* Remember the connected address scope ID.  It is
+			 * checked against the binding local address when
+			 * the socket is bound, and gives a send without a
+			 * destination its scope.
 			 */
-			rs->rs_bound_scope_id = sin6->sin6_scope_id;
+			rs->rs_conn_scope_id = sin6->sin6_scope_id;
+		} else {
+			rs->rs_conn_scope_id = 0;
 		}
 		rs->rs_conn_addr = sin6->sin6_addr;
 		rs->rs_conn_port = sin6->sin6_port;
diff --git a/net/rds/bind.c b/net/rds/bind.c
index f800d920d969..3ac59cd512a2 100644
--- a/net/rds/bind.c
+++ b/net/rds/bind.c
@@ -233,8 +233,8 @@ int rds_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int addr_len)
 	 * non-link local address (scope_id is 0).
 	 */
 	if (!ipv6_addr_any(&rs->rs_conn_addr) && scope_id &&
-	    rs->rs_bound_scope_id &&
-	    scope_id != rs->rs_bound_scope_id) {
+	    rs->rs_conn_scope_id &&
+	    scope_id != rs->rs_conn_scope_id) {
 		ret = -EINVAL;
 		goto out;
 	}
diff --git a/net/rds/rds.h b/net/rds/rds.h
index 2db49573dacd..9b1ffc49c0a6 100644
--- a/net/rds/rds.h
+++ b/net/rds/rds.h
@@ -646,6 +646,8 @@ struct rds_sock {
 	struct in6_addr		rs_conn_addr;
 #define rs_conn_addr_v4		rs_conn_addr.s6_addr32[3]
 	__be16			rs_conn_port;
+	/* scope of rs_conn_addr when it is link-local, 0 otherwise */
+	__u32			rs_conn_scope_id;
 	struct rds_transport    *rs_transport;
 
 	/*
diff --git a/net/rds/recv.c b/net/rds/recv.c
index 6204e577a90a..2458cd4ce73e 100644
--- a/net/rds/recv.c
+++ b/net/rds/recv.c
@@ -789,7 +789,16 @@ int rds_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 				sin6->sin6_port = inc->i_hdr.h_sport;
 				sin6->sin6_addr = inc->i_saddr;
 				sin6->sin6_flowinfo = 0;
-				sin6->sin6_scope_id = rs->rs_bound_scope_id;
+				/* A link-local sender is reported with the
+				 * interface recorded on its connection (the
+				 * accepting socket's device for TCP, the one
+				 * resolved for the local address on IB); any
+				 * other sender has no scope.
+				 */
+				sin6->sin6_scope_id =
+					(ipv6_addr_type(&inc->i_saddr) &
+					 IPV6_ADDR_LINKLOCAL) ?
+					inc->i_conn->c_dev_if : 0;
 				msg->msg_namelen = sizeof(*sin6);
 			}
 		}
diff --git a/net/rds/send.c b/net/rds/send.c
index 38b9cc9e0d20..f38967f65af1 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -1256,7 +1256,11 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
 		lock_sock(sk);
 		daddr = rs->rs_conn_addr;
 		dport = rs->rs_conn_port;
-		scope_id = rs->rs_bound_scope_id;
+		/* The connected peer's scope for a link-local peer, else
+		 * the bound scope, as an explicit send to that peer would
+		 * compute below.
+		 */
+		scope_id = rs->rs_conn_scope_id ?: rs->rs_bound_scope_id;
 		release_sock(sk);
 	}
 
-- 
2.50.1

Re: [PATCH net-next v6 1/2] net/rds: include the scope id in the sendmsg connection cache check

[sashiko-bot]

> rds_sendmsg() reuses the connection cached in rs->rs_conn when its
> peer address and ToS match the request.  The interface index is part
> of a connection's identity as well: rds_conn_create_outgoing() passes
> the request's scope_id down as dev_if, and rds_conn_lookup() compares
> c_dev_if, so sends to the same link-local address through two
> interfaces are two different connections.  The cache-hit test never
> looked at it.
> 
> A socket bound to a non-link-local address has rs_bound_scope_id 0,
> and the scope check at the top of rds_sendmsg() accepts any non-zero
> destination scope for such a socket.  So after a send to fe80::x%ifA,
> a send to fe80::x%ifB hits the cached ifA connection and the datagram
> leaves through ifA, to whichever peer answers to that address there.
> 
> Compare c_dev_if with the request's scope_id in the cache test, so
> that such a send takes the lookup path and finds, or creates, the ifB
> connection - the same key the lookup uses.  A send without a
> destination takes its scope from the socket, and the connection it
> created was keyed on that same scope, so the comparison holds for it
> too; the next patch gives that send the connected peer's own scope.
> 
> Found by inspection while reworking this cache for the connection
> lifetime series on net-next; not observed in the field.
> 
> Fixes: 1e2b44e78eea ("rds: Enable RDS IPv6 support")
> Assisted-by: Claude-Code:claude-fable-5
> Signed-off-by: Allison Henderson <achender@kernel.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020002816.3087414-2-achender@kernel.org?part=1

Re: [PATCH net-next v6 2/2] net/rds: keep the connected peer's scope id apart from the bound one

[sashiko-bot]

> rds_connect() has nowhere to keep the scope of a link-local peer, so
> it stores it in rs_bound_scope_id, the scope of the socket's own
> bound address, for rds_bind() to check a later link-local bind
> against.  That field is then also what a send without a destination
> uses as the request's scope, what getpeername() and recvmsg() report
> as the peer's scope, and what a later bind() overwrites:
> rds_add_bound() stores the bound address's scope unconditionally,
> which for a non-link-local address is 0.
> 
> So after connect(fe80::x%ifA) followed by bind(global), a send() with
> no destination asks for fe80::x with scope 0.  rds_conn_lookup() keys
> on the interface, so that finds or creates a connection with c_dev_if
> 0, which the TCP transport then tries to connect through
> sin6_scope_id 0 and tcp_v6_connect() rejects for a link-local peer:
> the connected socket's data is queued on a connection that can never
> [ ... ]
> observed in the field.
> 
> Fixes: 1e2b44e78eea ("rds: Enable RDS IPv6 support")
> Assisted-by: Claude-Code:claude-fable-5
> Signed-off-by: Allison Henderson <achender@kernel.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020002816.3087414-2-achender@kernel.org?part=2