[PATCH mptcp-next v2 0/4] Add MPTCP support for NBD

[PATCH mptcp-next v2 0/4] Add MPTCP support for NBD

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This series adds MPTCP (Multipath TCP) support to NBD (Network Block
Device) and includes comprehensive tests.

MPTCP can aggregate bandwidth from multiple network interfaces, providing
significant performance improvements in multi-NIC scenarios. Our tests
with 4 virtual NICs (each limited to 1000 Mbit/s / 125 MB/s) show ~4x
bandwidth improvement: TCP achieves ~110 MiB/s while MPTCP achieves
~400 MiB/s for both read and write operations.

Patch 1 gives MPTCP sockets a distinct lockdep class to prevent
false positive lockdep warnings when MPTCP master sockets and their
subflow sockets are used together in the same lock chain.

Patch 2 adds a set_memalloc protocol callback to struct proto that
allows protocols to propagate SOCK_MEMALLOC flag and allocation
settings to their sub-sockets automatically. MPTCP implements this
callback to sync memalloc settings to all subflow sockets.

Patch 3 adds MPTCP socket support to the kernel NBD driver by
allowing MPTCP sockets in nbd_get_socket().

Patch 4 adds a comprehensive test script (mptcp_nbd.sh) that
validates NBD over both TCP and MPTCP.

v2:
- Add lockdep class distinction for MPTCP sockets (patch 1)
- Add set_memalloc protocol callback for automatic subflow sync (patch 2)
- update selfetsts.

v1:
- https://patchwork.kernel.org/project/mptcp/cover/cover.1792746800.git.tanggeliang@kylinos.cn/

Geliang Tang (4):
  mptcp: give MPTCP sockets a distinct lockdep class
  sock: add set_memalloc protocol callback
  nbd: allow MPTCP sockets
  selftests: mptcp: add mptcp nbd tests

 drivers/block/nbd.c                           |   5 +-
 include/net/sock.h                            |   1 +
 net/core/sock.c                               |   8 +
 net/mptcp/pm_kernel.c                         |  16 -
 net/mptcp/protocol.c                          |  58 ++-
 tools/testing/selftests/net/mptcp/Makefile    |   1 +
 tools/testing/selftests/net/mptcp/config      |   1 +
 .../testing/selftests/net/mptcp/mptcp_lib.sh  |  13 +
 .../testing/selftests/net/mptcp/mptcp_nbd.sh  | 373 ++++++++++++++++++
 9 files changed, 458 insertions(+), 18 deletions(-)
 create mode 100755 tools/testing/selftests/net/mptcp/mptcp_nbd.sh

-- 
2.55.0

[PATCH mptcp-next v2 1/4] mptcp: give MPTCP sockets a distinct lockdep class

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

MPTCP sockets and their subflows share the same k-sk_lock-AF_INET
lockdep class. When both are kernel sockets, lockdep flags every path
that holds the MPTCP socket lock and then locks a subflow (setsockopt,
connect, bind, worker send-ack) as "possible recursive locking".

Extract the lock reclassification logic from PM listener socket creation
into mptcp_reclassify_socket() helper. Assign distinct lock classes
based on both socket type (kernel or user) and address family (IPv4 or
IPv6) to prevent lockdep from conflating kernel MPTCP socket usage
(which may nest with fs/storage locks) with user-space socket API usage
(which records mmap_lock dependencies through page faults).

Call mptcp_reclassify_socket() from mptcp_init_sock() for actively
created sockets, and from mptcp_sk_clone_init() before bh_lock_sock()
for passively cloned sockets. The reclassification must happen before
the lock is held to avoid lockdep warnings. PM listener sockets are
automatically reclassified through the mptcp_init_sock() path when
created via sock_create_kern().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_kernel.c | 16 ----------------
 net/mptcp/protocol.c  | 37 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index d3014bf57bf3..03603defdf6c 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -846,13 +846,9 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 	return ret;
 }
 
-static struct lock_class_key mptcp_slock_keys[2];
-static struct lock_class_key mptcp_keys[2];
-
 static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
 					    struct mptcp_pm_addr_entry *entry)
 {
-	bool is_ipv6 = entry->addr.family == AF_INET6;
 	int addrlen = sizeof(struct sockaddr_in);
 	struct sockaddr_storage addr;
 	struct sock *newsk, *ssk;
@@ -868,18 +864,6 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
 	if (!newsk)
 		return -EINVAL;
 
-	/* The subflow socket lock is acquired in a nested to the msk one
-	 * in several places, even by the TCP stack, and this msk is a kernel
-	 * socket: lockdep complains. Instead of propagating the _nested
-	 * modifiers in several places, re-init the lock class for the msk
-	 * socket to an mptcp specific one.
-	 */
-	sock_lock_init_class_and_name(newsk,
-				      is_ipv6 ? "mlock-AF_INET6" : "mlock-AF_INET",
-				      &mptcp_slock_keys[is_ipv6],
-				      is_ipv6 ? "msk_lock-AF_INET6" : "msk_lock-AF_INET",
-				      &mptcp_keys[is_ipv6]);
-
 	lock_sock(newsk);
 	ssk = __mptcp_nmpc_sk(mptcp_sk(newsk));
 	release_sock(newsk);
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 02a4f479d9d8..d4f42dfc227e 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3267,11 +3267,43 @@ static void mptcp_ca_reset(struct sock *sk)
 	icsk->icsk_ca_ops = NULL;
 }
 
+/* The subflow socket lock is acquired in a nested to the msk one
+ * in several places, even by the TCP stack: lockdep complains when
+ * both are kernel sockets. Instead of propagating the _nested modifiers
+ * in several places, re-init the lock class for the msk socket to an
+ * mptcp specific one. Separate lock classes are used for kernel and
+ * user sockets to prevent lockdep from conflating their usage patterns.
+ */
+static struct lock_class_key mptcp_slock_keys[2][2];
+static struct lock_class_key mptcp_keys[2][2];
+
+static void mptcp_reclassify_socket(struct sock *sk)
+{
+	bool is_ipv6 = sk->sk_family == AF_INET6;
+	const char *slock_name, *lock_name;
+	bool is_kern = !!sk->sk_kern_sock;
+
+	if (is_kern) {
+		slock_name = is_ipv6 ? "kmlock-AF_INET6" : "kmlock-AF_INET";
+		lock_name = is_ipv6 ? "kmsk_lock-AF_INET6" :
+				      "kmsk_lock-AF_INET";
+	} else {
+		slock_name = is_ipv6 ? "mlock-AF_INET6" : "mlock-AF_INET";
+		lock_name = is_ipv6 ? "msk_lock-AF_INET6" : "msk_lock-AF_INET";
+	}
+
+	sock_lock_init_class_and_name(sk, slock_name,
+				      &mptcp_slock_keys[is_kern][is_ipv6],
+				      lock_name, &mptcp_keys[is_kern][is_ipv6]);
+}
+
 static int mptcp_init_sock(struct sock *sk)
 {
 	struct net *net = sock_net(sk);
 	int ret;
 
+	mptcp_reclassify_socket(sk);
+
 	__mptcp_init_sock(sk);
 
 	if (!mptcp_is_enabled(net))
@@ -3772,13 +3804,16 @@ struct sock *mptcp_sk_clone_init(const struct sock *sk,
 				 struct request_sock *req)
 {
 	struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
-	struct sock *nsk = sk_clone_lock(sk, GFP_ATOMIC);
+	struct sock *nsk = sk_clone(sk, GFP_ATOMIC, false);
 	struct mptcp_subflow_context *subflow;
 	struct mptcp_sock *msk;
 
 	if (!nsk)
 		return NULL;
 
+	mptcp_reclassify_socket(nsk);
+
+	bh_lock_sock(nsk);
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
 	if (nsk->sk_family == AF_INET6)
 		inet_sk(nsk)->pinet6 = mptcp_inet6_sk(nsk);
-- 
2.55.0

[PATCH mptcp-next v2 2/4] sock: add set_memalloc protocol callback

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Add a protocol-specific callback set_memalloc() to struct proto that
gets invoked when SOCK_MEMALLOC flag or allocation settings change on
a socket. This allows protocols like MPTCP to propagate these settings
to their sub-sockets automatically.

For MPTCP, when the master socket's memalloc settings change (via
sk_set_memalloc/sk_clear_memalloc), the callback synchronizes
SOCK_MEMALLOC flag, sk_allocation, and sk_use_task_frag to all subflow
sockets immediately. This ensures subflows can use emergency memory
reserves when needed, preventing deadlocks in memory pressure scenarios.

NBD (Network Block Device) benefits from this when used over MPTCP for
swap devices. Previously, NBD would set sk_allocation directly in
__sock_xmit() but this wouldn't propagate to MPTCP subflows. Now it
calls sk_set_memalloc() which triggers the MPTCP callback to sync all
subflows.

The callback mechanism maintains clean layering: core socket code
doesn't need MPTCP-specific knowledge, and MPTCP handles its own
subflow synchronization transparently.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 drivers/block/nbd.c  |  1 +
 include/net/sock.h   |  1 +
 net/core/sock.c      |  8 ++++++++
 net/mptcp/protocol.c | 21 +++++++++++++++++++++
 4 files changed, 31 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index ffce519bf008..c5e26fc98d1a 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -579,6 +579,7 @@ static int __sock_xmit(struct nbd_device *nbd, struct socket *sock, int send,
 		do {
 			sock->sk->sk_allocation = GFP_NOIO | __GFP_MEMALLOC;
 			sock->sk->sk_use_task_frag = false;
+			sk_set_memalloc(sock->sk);
 			msg.msg_flags = msg_flags | MSG_NOSIGNAL;
 
 			if (send)
diff --git a/include/net/sock.h b/include/net/sock.h
index 173b37b2b8bb..aaf50293ea37 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1361,6 +1361,7 @@ struct proto {
 	/* Memory pressure */
 	void			(*enter_memory_pressure)(struct sock *sk);
 	void			(*leave_memory_pressure)(struct sock *sk);
+	void			(*set_memalloc)(struct sock *sk);
 	atomic_long_t		*memory_allocated;	/* Current allocated memory. */
 	int  __percpu		*per_cpu_fw_alloc;
 	struct percpu_counter	*sockets_allocated;	/* Current number of sockets. */
diff --git a/net/core/sock.c b/net/core/sock.c
index e7405be8941f..dc5fbe2f63fc 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -306,6 +306,10 @@ void sk_set_memalloc(struct sock *sk)
 	sock_set_flag(sk, SOCK_MEMALLOC);
 	sk->sk_allocation |= __GFP_MEMALLOC;
 	static_branch_inc(&memalloc_socks_key);
+
+	/* Call protocol-specific callback if available */
+	if (sk->sk_prot->set_memalloc)
+		sk->sk_prot->set_memalloc(sk);
 }
 EXPORT_SYMBOL_GPL(sk_set_memalloc);
 
@@ -315,6 +319,10 @@ void sk_clear_memalloc(struct sock *sk)
 	sk->sk_allocation &= ~__GFP_MEMALLOC;
 	static_branch_dec(&memalloc_socks_key);
 
+	/* Call protocol-specific callback if available */
+	if (sk->sk_prot->set_memalloc)
+		sk->sk_prot->set_memalloc(sk);
+
 	/*
 	 * SOCK_MEMALLOC is allowed to ignore rmem limits to ensure forward
 	 * progress of swapping. SOCK_MEMALLOC may be cleared while
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index d4f42dfc227e..5eccff04151d 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -4327,6 +4327,26 @@ static int mptcp_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 	return 0;
 }
 
+static void mptcp_set_memalloc(struct sock *sk)
+{
+	struct mptcp_sock *msk = mptcp_sk(sk);
+	struct mptcp_subflow_context *subflow;
+	bool memalloc;
+
+	lock_sock(sk);
+	memalloc = sock_flag(sk, SOCK_MEMALLOC);
+	mptcp_for_each_subflow(msk, subflow) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+		lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
+		sock_valbool_flag(ssk, SOCK_MEMALLOC, memalloc);
+		ssk->sk_allocation = sk->sk_allocation;
+		ssk->sk_use_task_frag = sk->sk_use_task_frag;
+		release_sock(ssk);
+	}
+	release_sock(sk);
+}
+
 static struct proto mptcp_prot = {
 	.name		= "MPTCP",
 	.owner		= THIS_MODULE,
@@ -4358,6 +4378,7 @@ static struct proto mptcp_prot = {
 	.obj_size	= sizeof(struct mptcp_sock),
 	.slab_flags	= SLAB_TYPESAFE_BY_RCU,
 	.no_autobind	= true,
+	.set_memalloc	= mptcp_set_memalloc,
 };
 
 static int mptcp_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int addr_len)
-- 
2.55.0

[PATCH mptcp-next v2 3/4] nbd: allow MPTCP sockets

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

MPTCP (Multipath TCP, RFC 8684) is an extension to TCP introduced in
Linux 5.6 that can combine bandwidth from multiple network interfaces.

For NBD (Network Block Device), MPTCP provides significant bandwidth
aggregation in multi-NIC scenarios. Our tests with 4 virtual NICs
(each limited to 1000 Mbit/s / 125 MB/s) show ~4x improvement,
TCP achieves ~110 MiB/s while MPTCP achieves ~400 MiB/s for both
read and write:

 # ./mptcp_nbd.sh mptcp
 INFO: nbd-server lacks native MPTCP, using mptcpize
 INFO: nbd-client lacks native MPTCP, using mptcpize
 Negotiation: ..size = 256MB
 Connected /dev/nbd0
 connected

 fio-3.41
 ... ...
 Run status group 0 (all jobs):
    READ: bw=406MiB/s (426MB/s), 406MiB/s-406MiB/s (426MB/s-426MB/s), \
		io=4063MiB (4261MB), run=10001-10001msec
 ... ...
 Run status group 0 (all jobs):
   WRITE: bw=376MiB/s (394MB/s), 376MiB/s-376MiB/s (394MB/s-394MB/s), \
		io=3758MiB (3940MB), run=10001-10001msec

MPTCP sockets can be created with:

  fd = socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP);

Or enabled transparently using the mptcpize tool without modifying
applications (mptcpize run nbd-server, mptcpize run nbd-client).
Future patches will add -M (--mptcp) options to nbd-server and
nbd-client for native MPTCP support.

Currently, the kernel NBD driver only accepts TCP and UNIX sockets.
This patch adds MPTCP socket support by adding msk_is_mptcp() check
in nbd_get_socket().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 drivers/block/nbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index c5e26fc98d1a..9cf89d53839a 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -33,6 +33,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <net/sock.h>
+#include <net/mptcp.h>
 #include <linux/net.h>
 #include <linux/kthread.h>
 #include <linux/types.h>
@@ -1222,8 +1223,9 @@ static struct socket *nbd_get_socket(struct nbd_device *nbd, unsigned long fd,
 		return NULL;
 
 	if (!sk_is_tcp(sock->sk) &&
+	    !msk_is_mptcp(sock->sk) &&
 	    !sk_is_stream_unix(sock->sk)) {
-		dev_err(disk_to_dev(nbd->disk), "Unsupported socket: should be TCP or UNIX.\n");
+		dev_err(disk_to_dev(nbd->disk), "Unsupported socket: should be TCP, MPTCP or UNIX.\n");
 		*err = -EINVAL;
 		sockfd_put(sock);
 		return NULL;
-- 
2.55.0

[PATCH mptcp-next v2 4/4] selftests: mptcp: add mptcp nbd tests

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Add mptcp_nbd.sh to verify NBD (Network Block Device) works correctly
over both TCP and MPTCP.

Both ns1 and ns2 have 4 virtual network interfaces, each limited to
1000 Mbit/s (125 MB/s). When running NBD over TCP, fio tests show
read/write bandwidth of about 110 MiB/s:

 # ./mptcp_nbd.sh tcp
 Negotiation: ..size = 256MB
 Connected /dev/nbd0
 connected

 fio-3.41
 ... ...
 Run status group 0 (all jobs):
    READ: bw=112MiB/s (117MB/s), 112MiB/s-112MiB/s (117MB/s-117MB/s), \
		io=1119MiB (1173MB), run=10002-10002msec
 ... ...
 Run status group 0 (all jobs):
   WRITE: bw=113MiB/s (118MB/s), 113MiB/s-113MiB/s (118MB/s-118MB/s), \
		io=1129MiB (1184MB), run=10002-10002msec

With simple MPTCP configuration and running MPTCP NBD, we can achieve
about 400 MiB/s read/write bandwidth:

 # ./mptcp_nbd.sh mptcp
 INFO: nbd-server lacks native MPTCP, using mptcpize
 INFO: nbd-client lacks native MPTCP, using mptcpize
 Negotiation: ..size = 256MB
 Connected /dev/nbd0
 connected

 fio-3.41
 ... ...
 Run status group 0 (all jobs):
    READ: bw=406MiB/s (426MB/s), 406MiB/s-406MiB/s (426MB/s-426MB/s), \
		io=4063MiB (4261MB), run=10001-10001msec
 ... ...
 Run status group 0 (all jobs):
   WRITE: bw=376MiB/s (394MB/s), 376MiB/s-376MiB/s (394MB/s-394MB/s), \
		io=3758MiB (3940MB), run=10001-10001msec

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/Makefile    |   1 +
 tools/testing/selftests/net/mptcp/config      |   1 +
 .../testing/selftests/net/mptcp/mptcp_lib.sh  |  13 +
 .../testing/selftests/net/mptcp/mptcp_nbd.sh  | 373 ++++++++++++++++++
 4 files changed, 388 insertions(+)
 create mode 100755 tools/testing/selftests/net/mptcp/mptcp_nbd.sh

diff --git a/tools/testing/selftests/net/mptcp/Makefile b/tools/testing/selftests/net/mptcp/Makefile
index 22ba0da2adb8..2c8b12866f81 100644
--- a/tools/testing/selftests/net/mptcp/Makefile
+++ b/tools/testing/selftests/net/mptcp/Makefile
@@ -13,6 +13,7 @@ TEST_PROGS := \
 	mptcp_connect_sendfile.sh \
 	mptcp_connect_splice.sh \
 	mptcp_join.sh \
+	mptcp_nbd.sh \
 	mptcp_sockopt.sh \
 	pm_netlink.sh \
 	simult_flows.sh \
diff --git a/tools/testing/selftests/net/mptcp/config b/tools/testing/selftests/net/mptcp/config
index 6a96cfbbc461..f6f266211bee 100644
--- a/tools/testing/selftests/net/mptcp/config
+++ b/tools/testing/selftests/net/mptcp/config
@@ -1,3 +1,4 @@
+CONFIG_BLK_DEV_NBD=y
 CONFIG_INET_DIAG=m
 CONFIG_INET_MPTCP_DIAG=m
 CONFIG_IP_ADVANCED_ROUTER=y
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index e1c8c6fd8820..aee7a73d9772 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -541,6 +541,19 @@ mptcp_lib_check_tools() {
 				exit ${KSFT_SKIP}
 			fi
 			;;
+		"nbd")
+			if ! command -v nbd-client &> /dev/null || \
+			   ! command -v nbd-server &> /dev/null; then
+				mptcp_lib_pr_skip "nbd tools not found"
+				exit ${KSFT_SKIP}
+			fi
+			;;
+		"fio")
+			if ! fio -h &> /dev/null; then
+				mptcp_lib_pr_skip "fio tool not found"
+				exit ${KSFT_SKIP}
+			fi
+			;;
 		*)
 			mptcp_lib_pr_fail "Internal error: unsupported tool: ${tool}"
 			exit ${KSFT_FAIL}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_nbd.sh b/tools/testing/selftests/net/mptcp/mptcp_nbd.sh
new file mode 100755
index 000000000000..29998be71a98
--- /dev/null
+++ b/tools/testing/selftests/net/mptcp/mptcp_nbd.sh
@@ -0,0 +1,373 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+. "$(dirname "$0")/mptcp_lib.sh"
+
+ret=0
+trtype="${1:-mptcp}"
+ns1=""
+ns2=""
+port=$((RANDOM % 10000 + 20000))
+temp_file=""
+conf_file=""
+pid_file=""
+server_pid=""
+
+export trtype port
+export temp_file conf_file pid_file server_pid
+
+usage()
+{
+	cat << EOF
+
+Usage:
+
+	$(basename "$0") [trtype]
+
+	trtype   Transport type (tcp|mptcp) - default: mptcp
+
+EOF
+exit ${KSFT_FAIL}
+}
+
+validate_params()
+{
+	if [[ ! "${trtype}" =~ ^(tcp|mptcp)$ ]]; then
+		echo "Invalid trtype ${trtype}. Must be tcp or mptcp"
+		usage
+	fi
+}
+
+# This function is used in the cleanup trap
+#shellcheck disable=SC2317,SC2329
+cleanup()
+{
+	# Disconnect nbd device in ns2
+	ip netns exec "$ns2" nbd-client -d /dev/nbd0 2>/dev/null || true
+
+	sleep 1
+
+	# Kill nbd-server in ns1
+	if [ -n "${server_pid}" ]; then
+		ip netns exec "$ns1" kill "${server_pid}" 2>/dev/null || true
+		ip netns exec "$ns1" kill -9 "${server_pid}" 2>/dev/null || true
+	fi
+
+	# Clean up temp files
+	if [ -n "${temp_file}" ]; then
+		ip netns exec "$ns1" rm -f "${temp_file}" 2>/dev/null || true
+	fi
+	if [ -n "${conf_file}" ]; then
+		ip netns exec "$ns1" rm -f "${conf_file}" 2>/dev/null || true
+	fi
+	if [ -n "${pid_file}" ]; then
+		ip netns exec "$ns1" rm -f "${pid_file}" 2>/dev/null || true
+	fi
+
+	mptcp_lib_ns_exit "$ns1" "$ns2"
+
+	unset -v trtype port
+	unset -v temp_file server_pid
+}
+
+init()
+{
+	mptcp_lib_ns_init ns1 ns2
+
+	ip netns exec "$ns1" sysctl -qw net.ipv4.tcp_reflect_tos=1
+
+	# ns1		ns2
+	# 10.1.1.1	10.1.1.2
+	# 10.1.2.1	10.1.2.2
+	# 10.1.3.1	10.1.3.2
+	# 10.1.4.1	10.1.4.2
+	for i in {1..4}; do
+		ip link add ns1eth"$i" netns "$ns1" type veth peer \
+			name ns2eth"$i" netns "$ns2"
+		ip -net "$ns1" addr add 10.1."$i".1/24 dev ns1eth"$i"
+		ip -net "$ns1" link set ns1eth"$i" up
+		ip -net "$ns2" addr add 10.1."$i".2/24 dev ns2eth"$i"
+		ip -net "$ns2" link set ns2eth"$i" up
+		ip -net "$ns2" route add default via 10.1."$i".1 \
+			dev ns2eth"$i" metric 10"$i"
+
+		# Rate limit to 1000mbit
+		tc -n "$ns1" qdisc add dev ns1eth"$i" root netem rate 1000mbit
+		tc -n "$ns2" qdisc add dev ns2eth"$i" root netem rate 1000mbit
+	done
+
+	mptcp_lib_pm_nl_set_limits "${ns1}" 8 8
+	mptcp_lib_pm_nl_add_endpoint "$ns1" 10.1.1.1 flags signal
+	mptcp_lib_pm_nl_add_endpoint "$ns1" 10.1.2.1 flags signal
+	mptcp_lib_pm_nl_add_endpoint "$ns1" 10.1.3.1 flags signal
+	mptcp_lib_pm_nl_add_endpoint "$ns1" 10.1.4.1 flags signal
+
+	mptcp_lib_pm_nl_set_limits "${ns2}" 8 8
+	mptcp_lib_pm_nl_add_endpoint "$ns2" 10.1.1.2 flags subflow
+	mptcp_lib_pm_nl_add_endpoint "$ns2" 10.1.2.2 flags subflow
+	mptcp_lib_pm_nl_add_endpoint "$ns2" 10.1.3.2 flags subflow
+	mptcp_lib_pm_nl_add_endpoint "$ns2" 10.1.4.2 flags subflow
+
+	# Create temp files using mktemp for security
+	temp_file=$(mktemp /tmp/nbd_backing_XXXXXX.raw)
+	conf_file=$(mktemp /tmp/nbd_server_XXXXXX.conf)
+	pid_file=$(mktemp /tmp/nbd_server_XXXXXX.pid)
+}
+
+# This function is invoked indirectly
+#shellcheck disable=SC2317,SC2329
+run_server()
+{
+	# Create backing file
+	dd if=/dev/zero of="${temp_file}" bs=1M count=0 seek=256 2>/dev/null
+
+	# Detect MPTCP support: native config or mptcpize fallback
+	local use_mptcpize=""
+	local mptcp_opt=""
+	if [ "${trtype}" = "mptcp" ]; then
+		if ! grep -q '"mptcp"' /usr/bin/nbd-server 2>/dev/null && \
+		   ! nbd-server --help 2>&1 | grep -qi mptcp; then
+			if command -v mptcpize &> /dev/null; then
+				use_mptcpize="mptcpize run -d"
+				echo "INFO: nbd-server lacks native MPTCP," \
+					"using mptcpize"
+			else
+				echo "ERROR: no MPTCP support in nbd-server"
+				return 1
+			fi
+		else
+			mptcp_opt="mptcp = true"
+			echo "INFO: nbd-server has native MPTCP support"
+		fi
+	fi
+
+	cat > "${conf_file}" << CONF
+[generic]
+	port = ${port}
+	listenaddr = 0.0.0.0
+	${mptcp_opt}
+
+[test]
+	exportname = ${temp_file}
+CONF
+
+	# Use -n (nodaemon) and background manually for better control
+	${use_mptcpize} nbd-server -C "${conf_file}" -n > /dev/null 2>&1 &
+	server_pid=$!
+
+	# Write PID to file for parent process to read
+	echo "${server_pid}" > "${pid_file}"
+
+	# Verify server is running
+	if ! kill -0 "${server_pid}" 2>/dev/null; then
+		echo "nbd-server failed to start"
+		return 1
+	fi
+}
+
+# This function is invoked indirectly
+#shellcheck disable=SC2317,SC2329
+run_client()
+{
+	local extra_args=()
+	local nbd_dev="/dev/nbd0"
+	local server_addr="10.1.1.1"
+
+	local use_mptcpize=""
+	if [ "${trtype}" = "mptcp" ]; then
+		if nbd-client 2>&1 | grep -qE "\-M|\-\-mptcp"; then
+			extra_args+=("-M")
+			echo "INFO: nbd-client has native MPTCP support (-M)"
+		elif command -v mptcpize &> /dev/null; then
+			use_mptcpize="mptcpize run -d"
+			echo "INFO: nbd-client lacks native MPTCP," \
+				"using mptcpize"
+		else
+			echo "ERROR: no MPTCP support in nbd-client"
+			return 1
+		fi
+	fi
+
+	# Options before positional args: -N name host port device
+	if ! ${use_mptcpize} nbd-client -N test "${extra_args[@]}" \
+			"${server_addr}" "${port}" "${nbd_dev}"; then
+		echo "Failed to connect ${nbd_dev} to ${server_addr}"
+		return 1
+	fi
+
+	sleep 1
+
+	# Verify connection
+	if ! nbd-client -c /dev/nbd0; then
+		echo "nbd-client check failed"
+		return 1
+	fi
+
+	return 0
+}
+
+# This function is invoked indirectly
+#shellcheck disable=SC2317,SC2329
+run_io_test()
+{
+	local nbd_dev="/dev/nbd0"
+	local test_data
+	local read_data
+
+	# Create temp files for data integrity test
+	test_data=$(mktemp /tmp/nbd_test_XXXXXX.bin)
+	read_data=$(mktemp /tmp/nbd_read_XXXXXX.bin)
+
+	# Data integrity test: write and verify
+	dd if=/dev/urandom of="${test_data}" bs=1M count=10 2>/dev/null
+	if ! dd if="${test_data}" of="${nbd_dev}" bs=1M \
+			conv=fdatasync 2>/dev/null; then
+		echo "dd write failed"
+		rm -f "${test_data}" "${read_data}"
+		return 1
+	fi
+
+	if ! dd if="${nbd_dev}" of="${read_data}" bs=1M count=10 \
+			2>/dev/null; then
+		echo "dd read failed"
+		rm -f "${test_data}" "${read_data}"
+		return 1
+	fi
+
+	if ! cmp -s "${test_data}" "${read_data}"; then
+		echo "FAIL: Data integrity check failed"
+		rm -f "${test_data}" "${read_data}"
+		return 1
+	fi
+	echo "PASS: Data integrity verified"
+	rm -f "${test_data}" "${read_data}"
+
+	# dd write test (larger, for performance measurement)
+	if ! dd if=/dev/urandom of="${nbd_dev}" bs=4k count=1000 \
+			conv=fdatasync 2>/dev/null; then
+		echo "dd write failed"
+		return 1
+	fi
+
+	# dd read test
+	if ! dd if="${nbd_dev}" of=/dev/null bs=4k count=1000 2>/dev/null; then
+		echo "dd read failed"
+		return 1
+	fi
+
+	# fio random read test
+	if ! fio --name=randread --direct=1 --ioengine=libaio \
+		 --blocksize=4k --runtime=10 --time_based \
+		 --rw=randread --numjobs=1 --iodepth=32 \
+		 --group_reporting --size=128M \
+		 --filename="${nbd_dev}"; then
+		echo "fio randread failed"
+		return 1
+	fi
+
+	# fio random write test
+	if ! fio --name=randwrite --direct=1 --ioengine=libaio \
+		 --blocksize=4k --runtime=10 --time_based \
+		 --rw=randwrite --numjobs=1 --iodepth=32 \
+		 --group_reporting --size=128M \
+		 --filename="${nbd_dev}"; then
+		echo "fio randwrite failed"
+		return 1
+	fi
+
+	# Verify MPTCP subflows if trtype is mptcp
+	if [ "${trtype}" = "mptcp" ]; then
+		local sf_total sf_max tcp_conns is_fallback
+		sf_total=$(ss -Mia | grep -oP "subflows_total:\K[0-9]+" | \
+			head -1)
+		sf_max=$(ss -Mia | grep -oP "subflows_max:\K[0-9]+" | head -1)
+		tcp_conns=$(ss -tia | grep -c "tcp-ulp-mptcp" || true)
+		is_fallback=$(ss -Mia | grep -c "fallback" || true)
+		if [ "${is_fallback}" -gt 0 ]; then
+			echo "FAIL: MPTCP fell back to TCP!" \
+				"subflows_total:${sf_total}" \
+				"tcp_subflows:${tcp_conns}"
+			return 1
+		elif [ -n "${sf_total}" ]; then
+			echo "PASS: MPTCP subflows_total:${sf_total}" \
+				"subflows_max:${sf_max}" \
+				"tcp_subflows:${tcp_conns}"
+		else
+			echo "FAIL: No MPTCP subflows info found"
+			return 1
+		fi
+	fi
+
+	return 0
+}
+
+# Check MPTCP support in kernel
+mptcp_lib_check_mptcp
+
+# Check for required tools
+mptcp_lib_check_tools ip tc ss nbd fio
+
+# Check for /dev/nbd0
+if [ ! -b /dev/nbd0 ]; then
+	mptcp_lib_pr_skip "/dev/nbd0 not found"
+	exit ${KSFT_SKIP}
+fi
+
+validate_params
+
+# Check for mptcpize if using MPTCP transport
+if [ "${trtype}" = "mptcp" ]; then
+	if ! command -v mptcpize &> /dev/null; then
+		mptcp_lib_pr_skip "mptcpize tool not found"
+		exit ${KSFT_SKIP}
+	fi
+fi
+
+trap cleanup EXIT
+
+init
+
+run_test()
+{
+	if ! ip netns exec "$ns1" bash <<- EOF
+		$(declare -f run_server)
+		export trtype port
+		run_server
+		exit \$?
+	EOF
+	then
+		ret="${KSFT_FAIL}"
+		return
+	fi
+
+	# Get server pid for cleanup from file
+	server_pid=$(ip netns exec "$ns1" cat "${pid_file}" 2>/dev/null)
+	server_pid="${server_pid:-}"
+
+	# Wait for server to be ready instead of fixed sleep
+	mptcp_lib_wait_local_port_listen "$ns1" "$port"
+
+	if ! ip netns exec "$ns2" bash <<- EOF
+		$(declare -f run_client run_io_test)
+		export trtype port
+		run_client || exit 1
+		run_io_test || exit 1
+		exit \$?
+	EOF
+	then
+		ret="${KSFT_FAIL}"
+	fi
+
+	sleep 1
+}
+
+run_test "$@"
+
+if [ "${ret}" -eq 0 ]; then
+	mptcp_lib_result_pass "nbd over ${trtype} test"
+else
+	mptcp_lib_result_fail "nbd over ${trtype} test"
+fi
+
+mptcp_lib_result_print_all_tap
+exit "$ret"
-- 
2.55.0

Re: [PATCH mptcp-next v2 1/4] mptcp: give MPTCP sockets a distinct lockdep class

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mptcp: race on sk_lock reinit and unlocked clone with SLAB_TYPESAFE_BY_RCU
--

commit 1535a4c9bef6857111a0222f084ae0666d8e7461
Author: Geliang Tang <geliang@kernel.org>

mptcp: give MPTCP sockets a distinct lockdep class

This commit introduces mptcp_reclassify_socket() to assign distinct lockdep
classes to MPTCP sockets based on address family and kernel vs user socket
type. It calls this helper from mptcp_init_sock() and from
mptcp_sk_clone_init() after switching from sk_clone_lock() to unlocked
sk_clone().

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 02a4f479d9d8c..d4f42dfc227e2 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3267,11 +3267,43 @@ static void mptcp_ca_reset(struct sock *sk)
>  	icsk->icsk_ca_ops = NULL;
>  }
>  
> +/* The subflow socket lock is acquired in a nested to the msk one
> + * in several places, even by the TCP stack: lockdep complains when
> + * both are kernel sockets. Instead of propagating the _nested modifiers
> + * in several places, re-init the lock class for the msk socket to an
> + * mptcp specific one. Separate lock classes are used for kernel and
> + * user sockets to prevent lockdep from conflating their usage patterns.
> + */
> +static struct lock_class_key mptcp_slock_keys[2][2];
> +static struct lock_class_key mptcp_keys[2][2];
> +
> +static void mptcp_reclassify_socket(struct sock *sk)
> +{
> +	bool is_ipv6 = sk->sk_family == AF_INET6;
> +	const char *slock_name, *lock_name;
> +	bool is_kern = !!sk->sk_kern_sock;
> +
> +	if (is_kern) {
> +		slock_name = is_ipv6 ? "kmlock-AF_INET6" : "kmlock-AF_INET";
> +		lock_name = is_ipv6 ? "kmsk_lock-AF_INET6" :
> +				      "kmsk_lock-AF_INET";
> +	} else {
> +		slock_name = is_ipv6 ? "mlock-AF_INET6" : "mlock-AF_INET";
> +		lock_name = is_ipv6 ? "msk_lock-AF_INET6" : "msk_lock-AF_INET";
> +	}
> +
> +	sock_lock_init_class_and_name(sk, slock_name,
> +				      &mptcp_slock_keys[is_kern][is_ipv6],
> +				      lock_name, &mptcp_keys[is_kern][is_ipv6]);
> +}
> +
>  static int mptcp_init_sock(struct sock *sk)
>  {
>  	struct net *net = sock_net(sk);
>  	int ret;
>  
> +	mptcp_reclassify_socket(sk);
> +
>  	__mptcp_init_sock(sk);
>  
>  	if (!mptcp_is_enabled(net))
> @@ -3772,13 +3804,16 @@ struct sock *mptcp_sk_clone_init(const struct sock *sk,
>  				 struct request_sock *req)
>  {
>  	struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
> -	struct sock *nsk = sk_clone_lock(sk, GFP_ATOMIC);
> +	struct sock *nsk = sk_clone(sk, GFP_ATOMIC, false);
>  	struct mptcp_subflow_context *subflow;
>  	struct mptcp_sock *msk;
>  
>  	if (!nsk)
>  		return NULL;
>  
> +	mptcp_reclassify_socket(nsk);
> +
> +	bh_lock_sock(nsk);

[Severity: High]
Can calling mptcp_reclassify_socket() and unlocked sk_clone() race with
lockless RCU readers on SLAB_TYPESAFE_BY_RCU MPTCP sockets?

Both mptcp_prot and mptcp_v6_prot use SLAB_TYPESAFE_BY_RCU, and accepted
MPTCP sockets clear SOCK_RCU_FREE in mptcp_sk_clone_init(), so a closed
mptcp_sock can be immediately recycled from the slab cache while a
concurrent RCU reader in mptcp_token_iter_next() or mptcp_token_get_sock()
is walking bucket->msk_chain:

net/mptcp/token.c:mptcp_token_iter_next() {
	...
	rcu_read_lock();
	sk_nulls_for_each_rcu(sk, pos, &bucket->msk_chain) {
		++num;
		if (!net_eq(sock_net(sk), net))
			continue;

		if (num <= *s_num)
			continue;

		if (!refcount_inc_not_zero(&sk->sk_refcnt))
			continue;
	...
}

Once refcount_inc_not_zero(&sk->sk_refcnt) succeeds on the recycled socket,
callers of mptcp_token_iter_next() (such as mptcp_diag_dump(),
mptcp_nl_remove_subflow_and_signal_addr(), mptcp_nl_remove_id_zero_address(),
mptcp_nl_flush_addrs_list(), and mptcp_nl_set_flags()) immediately call
lock_sock(sk) or lock_sock_fast(sk).

In mptcp_sk_clone_init(), sk_clone_lock() previously initialized newsk's
lock and acquired bh_lock_sock(newsk) before setting newsk->sk_refcnt to 2:

net/core/sock.c:sk_clone() {
	...
	sock_lock_init(newsk);

	if (lock)
		bh_lock_sock(newsk);
	...
	smp_wmb();
	refcount_set(&newsk->sk_refcnt, 2);
}

Holding bh_lock_sock(newsk) before sk_refcnt became non-zero serialized any
concurrent lock_sock() or lock_sock_fast() reader until __mptcp_init_sock()
completed and bh_unlock_sock(nsk) was called.

With sk_clone(sk, GFP_ATOMIC, false), nsk->sk_refcnt becomes 2 while nsk is
unlocked. Can a concurrent RCU reader grab a reference on nsk and call
lock_sock(nsk) or lock_sock_fast(nsk):

1. Concurrently with mptcp_reclassify_socket(nsk) (and similarly in
   mptcp_init_sock() after sock_init_data() has set sk->sk_refcnt to 1),
   where sock_lock_init_class_and_name() zeroes sk_lock.owned and
   re-initializes sk_lock.wq, sk_lock.slock, and the lockdep maps while
   another CPU is acquiring or holding sk_lock?

2. In mptcp_sk_clone_init(), after mptcp_reclassify_socket(nsk) finishes
   and before bh_lock_sock(nsk) is called? If the reader acquires
   lock_sock(nsk) first, it sets nsk->sk_lock.owned = 1 and drops
   nsk->sk_lock.slock, which does not prevent bh_lock_sock(nsk) from
   proceeding into __mptcp_init_sock(nsk) concurrently, and the reader can
   iterate msk->conn_list while it still contains the raw copy of the
   listening socket's list head from sock_copy()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793010744.git.tanggeliang@kylinos.cn?part=1

Re: [PATCH mptcp-next v2 2/4] sock: add set_memalloc protocol callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] nbd: per-I/O sk_set_memalloc() in __sock_xmit() leaks static key and deadlocks
- [High] mptcp: mptcp_set_memalloc() misses subflows created or joined later
--

commit b144942ec73c0d6666c170f8c7917db0e4e17462
Author: Geliang Tang <geliang@kernel.org>

sock: add set_memalloc protocol callback

This patch adds a set_memalloc() callback to struct proto so protocols such as
MPTCP can synchronize SOCK_MEMALLOC, sk_allocation, and sk_use_task_frag to
their subflows when sk_set_memalloc() or sk_clear_memalloc() is called. It
also updates NBD's __sock_xmit() to call sk_set_memalloc() on each transmit or
receive loop iteration.

> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index ffce519bf0087..c5e26fc98d1aa 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -579,6 +579,7 @@ static int __sock_xmit(struct nbd_device *nbd, struct socket *sock, int send,
>  		do {
>  			sock->sk->sk_allocation = GFP_NOIO | __GFP_MEMALLOC;
>  			sock->sk->sk_use_task_frag = false;
> +			sk_set_memalloc(sock->sk);

[Severity: High]
Should sk_set_memalloc() really be called inside the per-I/O loop in
__sock_xmit()?

NBD already calls sk_set_memalloc() once per socket during setup in
nbd_start_device() and during reconnection in nbd_genl_reconnect_sock_cb():

drivers/block/nbd.c:nbd_start_device() {
	...
	sk_set_memalloc(config->socks[i]->sock->sk);
	if (nbd->tag_set.timeout)
		config->socks[i]->sock->sk->sk_sndtimeo =
			nbd->tag_set.timeout;
	...
}

Calling sk_set_memalloc(sock->sk) on every send and receive iteration in
__sock_xmit() invokes static_branch_inc(&memalloc_socks_key) without a
matching sk_clear_memalloc(), which leaks a reference on
memalloc_socks_key.key.enabled on every I/O until the counter saturates at
INT_MAX and triggers WARN_ON_ONCE(!static_key_fast_inc_not_disabled(key)) in
static_key_slow_inc_cpuslocked().

In addition, static_branch_inc() -> static_key_slow_inc() acquires
cpus_read_lock() on every call while __sock_xmit() is running in the block I/O
and swap-out path under memalloc_noreclaim_save(). During CPU hot-unplug,
blk_mq_hctx_notify_offline() holds cpus_write_lock() while waiting for
in-flight requests on the queue to complete, which can deadlock if an NBD
request is waiting on cpus_read_lock() in __sock_xmit().

Furthermore, nbd_send_cmd() and recv_work() -> nbd_read_reply() call
__sock_xmit() concurrently on the same socket without holding lock_sock(), so
sock_set_flag(sk, SOCK_MEMALLOC) performs an unlocked non-atomic __set_bit()
on sk->sk_flags, and MPTCP sockets will acquire lock_sock(sk) and each subflow
lock on every packet.

Would it work to initialize sk_allocation and sk_use_task_frag before the
existing one-time sk_set_memalloc() calls in nbd_start_device() and
nbd_genl_reconnect_sock_cb() instead of calling sk_set_memalloc() in
__sock_xmit()?

[ ... ]

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index d4f42dfc227e2..5eccff04151d7 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -4327,6 +4327,26 @@ static int mptcp_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
>  	return 0;
>  }
>  
> +static void mptcp_set_memalloc(struct sock *sk)
> +{
> +	struct mptcp_sock *msk = mptcp_sk(sk);
> +	struct mptcp_subflow_context *subflow;
> +	bool memalloc;
> +
> +	lock_sock(sk);
> +	memalloc = sock_flag(sk, SOCK_MEMALLOC);
> +	mptcp_for_each_subflow(msk, subflow) {
> +		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +
> +		lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
> +		sock_valbool_flag(ssk, SOCK_MEMALLOC, memalloc);
> +		ssk->sk_allocation = sk->sk_allocation;
> +		ssk->sk_use_task_frag = sk->sk_use_task_frag;
> +		release_sock(ssk);
> +	}
> +	release_sock(sk);
> +}

[Severity: High]
How are subflows that are created or joined after sk_set_memalloc() is called
handled?

mptcp_set_memalloc() only iterates over subflows currently in msk->conn_list.
If a caller invokes sk_set_memalloc() before connect() (where the first
subflow is created lazily via __mptcp_nmpc_sk() -> __mptcp_socket_create() ->
mptcp_subflow_create_socket()), or when additional MP_JOIN subflows are
created later via __mptcp_subflow_connect() -> mptcp_subflow_create_socket()
or joined via mptcp_finish_join() -> __mptcp_finish_join() (including subflows
queued on msk->join_list while sk is locked), those subflows only run
mptcp_sockopt_sync_locked() -> sync_socket_options():

net/mptcp/sockopt.c:mptcp_sockopt_sync_locked() {
	...
	if (READ_ONCE(subflow->setsockopt_seq) != msk->setsockopt_seq) {
		sync_socket_options(msk, ssk);

		subflow->setsockopt_seq = msk->setsockopt_seq;
	}
}

Because neither mptcp_sockopt_sync_locked() nor sync_socket_options()
propagates SOCK_MEMALLOC, sk_allocation, or sk_use_task_frag, any subflow
created or joined after sk_set_memalloc() will miss those settings and can
drop incoming pfmemalloc packets or allocate without __GFP_MEMALLOC under
memory pressure.

Also, when sk_clear_memalloc() clears SOCK_MEMALLOC, should
mptcp_set_memalloc() call sk_mem_reclaim(ssk) on each subflow socket just as
sk_clear_memalloc() does on sk?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793010744.git.tanggeliang@kylinos.cn?part=2

Re: [PATCH mptcp-next v2 0/4] Add MPTCP support for NBD

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/40311189299

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/a787dfbbb9da
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1193610


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next v2 3/4] nbd: allow MPTCP sockets

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] nbd: per-I/O sk_set_memalloc() in __sock_xmit() leaks static key and locks
- [High] nbd: unreclassified MPTCP subflows and GFP_KERNEL in mptcp_worker() deadlock
- [High] mptcp: new subflows miss SOCK_MEMALLOC sync in mptcp_sockopt_sync_locked()
--

commit 1d164aaf19b3c6998b999ca3f8b551a6fe753173
Author: Geliang Tang <geliang@kernel.org>

nbd: allow MPTCP sockets

This commit allows MPTCP sockets to be attached to NBD devices by checking
msk_is_mptcp(sock->sk) in nbd_get_socket().

> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index c5e26fc98d1aa..9cf89d53839a1 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
[ ... ]
> @@ -1222,8 +1223,9 @@ static struct socket *nbd_get_socket(struct nbd_device *nbd, unsigned long fd,
>  		return NULL;
>  
>  	if (!sk_is_tcp(sock->sk) &&
> +	    !msk_is_mptcp(sock->sk) &&
>  	    !sk_is_stream_unix(sock->sk)) {

[Severity: High]
With MPTCP sockets enabled here in drivers/block/nbd.c:nbd_get_socket(),
an earlier patch in this series (commit b144942ec73c, "sock: add
set_memalloc protocol callback") added a call to sk_set_memalloc(sock->sk)
inside the per-segment loop of drivers/block/nbd.c:__sock_xmit():

drivers/block/nbd.c:__sock_xmit() {
	...
		do {
			sock->sk->sk_allocation = GFP_NOIO | __GFP_MEMALLOC;
			sock->sk->sk_use_task_frag = false;
			sk_set_memalloc(sock->sk);
			msg.msg_flags = msg_flags | MSG_NOSIGNAL;
}

Looking at net/core/sock.c:sk_set_memalloc():

void sk_set_memalloc(struct sock *sk)
{
	sock_set_flag(sk, SOCK_MEMALLOC);
	sk->sk_allocation |= __GFP_MEMALLOC;
	static_branch_inc(&memalloc_socks_key);

	/* Call protocol-specific callback if available */
	if (sk->sk_prot->set_memalloc)
		sk->sk_prot->set_memalloc(sk);
}

Since NBD already calls sk_set_memalloc() at device setup in
nbd_start_device() and nbd_genl_reconnect_sock_cb() (and never calls
sk_clear_memalloc() on teardown), won't calling sk_set_memalloc() on every
send and receive iteration in __sock_xmit() cause several regressions?

1. Unbounded static key refcount leak and overflow:
   static_branch_inc(&memalloc_socks_key) is called unconditionally on
   every request header, write bvec, reply header, and read bvec until
   memalloc_socks_key.key.enabled saturates at INT_MAX, at which point
   kernel/jump_label.c:static_key_slow_inc_cpuslocked() takes
   jump_label_mutex and triggers
   WARN_ON_ONCE(!static_key_fast_inc_not_disabled(key)) on every I/O.

2. Lock hierarchy inversion in the block I/O path:
   static_branch_inc() calls static_key_slow_inc(), which acquires
   cpus_read_lock() (cpu_hotplug_lock) inside __sock_xmit() while
   nbd_queue_rq() -> nbd_handle_cmd() -> nbd_send_cmd() holds
   &q->q_usage_counter(io), cmd->lock, and nsock->tx_lock under
   memalloc_noreclaim_save().

3. Unlocked non-atomic race on sk->sk_flags:
   __sock_xmit() runs concurrently in the send path (under nsock->tx_lock)
   and the receive path (recv_work, without nsock->tx_lock or
   lock_sock(sk)), so sock_set_flag(sk, SOCK_MEMALLOC) performs an
   unlocked non-atomic __set_bit() racing with sk->sk_flags updates under
   lock_sock(sk) in sock_sendmsg() and sock_recvmsg().

4. Per-segment subflow lock contention:
   For MPTCP sockets, sk_set_memalloc() calls
   net/mptcp/protocol.c:mptcp_set_memalloc(), which acquires lock_sock(sk)
   and lock_sock_nested(ssk, SINGLE_DEPTH_NESTING) across all subflows
   before every sock_sendmsg() and sock_recvmsg() call.

[Severity: High]
Can attaching an MPTCP socket via nbd_add_socket() -> nbd_get_socket() ->
nbd_reclassify_socket() cause lockdep circular locking dependency warnings
and memory-reclaim deadlocks with MPTCP subflows and mptcp_worker()?

In drivers/block/nbd.c:nbd_reclassify_socket():

static void nbd_reclassify_socket(struct socket *sock)
{
	struct sock *sk = sock->sk;

	if (!sock_allow_reclassification(sk))
		return;

	switch (sk->sk_family) {
	case AF_INET:
		sock_lock_init_class_and_name(sk, "slock-AF_INET-NBD",
					      &nbd_slock_key[0],
					      "sk_lock-AF_INET-NBD",
					      &nbd_key[0]);
		break;

This only reclassifies the outer MPTCP master socket (sock->sk), while its
underlying TCP subflow sockets (ssk) created via sock_create_kern() in
net/mptcp/subflow.c:mptcp_subflow_create_socket() remain in the generic
kernel socket lock class (k-sk_lock-AF_INET / k-sk_lock-AF_INET6):

net/mptcp/subflow.c:mptcp_subflow_create_socket() {
	err = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP, &sf);
	if (err)
		return err;

	lock_sock_nested(sf->sk, SINGLE_DEPTH_NESTING);
	...
	sk_net_refcnt_upgrade(sf->sk);
	err = tcp_set_ulp(sf->sk, "mptcp");
}

Here mptcp_subflow_create_socket() holds sf->sk's lock (k-sk_lock-AF_INET)
while calling sk_net_refcnt_upgrade() (which calls get_net_track() with
GFP_KERNEL) and tcp_set_ulp() -> subflow_ulp_init() -> subflow_create_ctx()
with GFP_KERNEL, establishing k-sk_lock-AF_INET -> fs_reclaim.

When NBD submits I/O via nbd_queue_rq() -> nbd_handle_cmd() ->
nbd_send_cmd() -> sock_xmit() -> __sock_xmit(), it holds
&q->q_usage_counter(io), cmd->lock, and nsock->tx_lock and calls into
mptcp_set_memalloc() and mptcp_sendmsg() -> __mptcp_push_pending(), which
acquire lock_sock(sk) (sk_lock-AF_INET-NBD) and lock_sock(ssk)
(k-sk_lock-AF_INET). Since blk_alloc_queue() establishes
fs_reclaim -> &q->q_usage_counter(io), doesn't this create a circular lock
dependency chain:

  k-sk_lock-AF_INET -> fs_reclaim -> &q->q_usage_counter(io) ->
  cmd->lock -> nsock->tx_lock -> sk_lock-AF_INET-NBD -> k-sk_lock-AF_INET

Additionally, in net/mptcp/protocol.c:mptcp_worker():

static void mptcp_worker(struct work_struct *work)
{
	struct mptcp_sock *msk = container_of(work, struct mptcp_sock, work);
	struct sock *sk = (struct sock *)msk;
	unsigned long fail_tout;
	int state;

	lock_sock(sk);
	...
	mptcp_pm_worker(msk);

mptcp_worker() acquires lock_sock(sk) (sk_lock-AF_INET-NBD) without
memalloc_noreclaim_save() or memalloc_noio_save() and runs
mptcp_pm_worker() -> __mptcp_subflow_connect() ->
mptcp_subflow_create_socket() and kernel_connect() (as well as
__mptcp_close_ssk() -> mptcp_event(..., GFP_KERNEL)), performing GFP_KERNEL
allocations under lock_sock(sk). If memory reclaim recurses into NBD I/O,
won't __sock_xmit() deadlock waiting for lock_sock(sk) held by
mptcp_worker()?

[Severity: High]
How do newly created or passively joined MPTCP subflows inherit
SOCK_MEMALLOC, sk_allocation, and sk_use_task_frag when an MPTCP socket is
used by NBD?

When the MPTCP path manager creates an additional subflow in
mptcp_worker() -> mptcp_pm_worker() -> __mptcp_subflow_connect() ->
mptcp_subflow_create_socket(), or finishes a passive MP_JOIN in
__mptcp_finish_join(), socket options are synchronized from the master
socket to the subflow via net/mptcp/sockopt.c:mptcp_sockopt_sync_locked():

void mptcp_sockopt_sync_locked(struct mptcp_sock *msk, struct sock *ssk)
{
	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);

	msk_owned_by_me(msk);

	ssk->sk_rcvlowat = 0;

	/* subflows must ignore any latency-related settings: will not affect
	 * the user-space - only the msk is relevant - but will foul the
	 * mptcp scheduler
	 */
	tcp_sk(ssk)->notsent_lowat = UINT_MAX;

	if (READ_ONCE(subflow->setsockopt_seq) != msk->setsockopt_seq) {
		sync_socket_options(msk, ssk);

		subflow->setsockopt_seq = msk->setsockopt_seq;
	}
}

Neither mptcp_sockopt_sync_locked() nor sync_socket_options() propagates
SOCK_MEMALLOC, sk->sk_allocation, or sk->sk_use_task_frag from msk to ssk.
Meanwhile, net/mptcp/protocol.c:mptcp_set_memalloc() only iterates over
subflows already in msk->conn_list when sk_set_memalloc() is called (which
also misses subflows still on msk->join_list waiting for mptcp_release_cb(),
and in nbd_start_device() and nbd_genl_reconnect_sock_cb() is called before
sk_allocation = GFP_NOIO | __GFP_MEMALLOC and sk_use_task_frag = false are
assigned).

Won't this leave new subflows with ssk->sk_allocation == GFP_KERNEL during
kernel_connect() and without SOCK_MEMALLOC set (for instance, while
recv_work is already blocked in mptcp_recvmsg()), causing GFP_KERNEL
allocations during connection establishment and causing incoming pfmemalloc
packets on the new subflow to be dropped by sk_filter_trim_cap() under
memory pressure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793010744.git.tanggeliang@kylinos.cn?part=3

Re: [PATCH mptcp-next v2 4/4] selftests: mptcp: add mptcp nbd tests

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] selftests: mptcp: broken native MPTCP and mptcpize checks in mptcp_nbd.sh
- [Medium] selftests: mptcp: weak and non-portable subflow check in run_io_test()
- [Medium] selftests: mptcp: mptcp_nbd.sh disconnects active host /dev/nbd0 in cleanup()
--

commit 7951aabe0d1917726a0a08d246614408762a837e
Author: Geliang Tang <geliang@kernel.org>

selftests: mptcp: add mptcp nbd tests

This commit adds mptcp_nbd.sh to test NBD over TCP and MPTCP using four
rate-limited virtual interfaces between two network namespaces, verifying
data integrity with dd/cmp and measuring I/O with fio.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_nbd.sh b/tools/testing/selftests/net/mptcp/mptcp_nbd.sh
> new file mode 100755
> index 0000000000000..29998be71a982
> --- /dev/null
> +++ b/tools/testing/selftests/net/mptcp/mptcp_nbd.sh

[ ... ]

> +# This function is used in the cleanup trap
> +#shellcheck disable=SC2317,SC2329
> +cleanup()
> +{
> +	# Disconnect nbd device in ns2
> +	ip netns exec "$ns2" nbd-client -d /dev/nbd0 2>/dev/null || true
> +
> +	sleep 1
> +
> +	# Kill nbd-server in ns1
> +	if [ -n "${server_pid}" ]; then
> +		ip netns exec "$ns1" kill "${server_pid}" 2>/dev/null || true
> +		ip netns exec "$ns1" kill -9 "${server_pid}" 2>/dev/null || true
> +	fi
> +
> +	# Clean up temp files
> +	if [ -n "${temp_file}" ]; then
> +		ip netns exec "$ns1" rm -f "${temp_file}" 2>/dev/null || true
> +	fi
> +	if [ -n "${conf_file}" ]; then
> +		ip netns exec "$ns1" rm -f "${conf_file}" 2>/dev/null || true
> +	fi
> +	if [ -n "${pid_file}" ]; then
> +		ip netns exec "$ns1" rm -f "${pid_file}" 2>/dev/null || true
> +	fi
> +
> +	mptcp_lib_ns_exit "$ns1" "$ns2"
> +
> +	unset -v trtype port
> +	unset -v temp_file server_pid
> +}

[Severity: Medium]
Can cleanup() disconnect an active /dev/nbd0 device on the host if /dev/nbd0
was already in use before the test started?

Before registering the cleanup trap, the script only checks whether
/dev/nbd0 is a block device node:

	# Check for /dev/nbd0
	if [ ! -b /dev/nbd0 ]; then
		mptcp_lib_pr_skip "/dev/nbd0 not found"
		exit ${KSFT_SKIP}
	fi

Because NBD devices in drivers/block/nbd.c are global across network
namespaces (nbd_index_idr is global and nbd_genl_family sets .netnsok = 1),
if /dev/nbd0 is already connected on the host, run_client() will fail to
attach it (or run_server() may fail earlier) and cleanup() will still execute
nbd-client -d /dev/nbd0, disconnecting the host's active block device. Should
the script check whether /dev/nbd0 is already connected via nbd-client -c
/dev/nbd0 before setting up the trap, or only disconnect /dev/nbd0 if
run_client() actually connected it?

Also, in run_io_test(), two 10 MiB temporary files are created inside the
child subshell:

	test_data=$(mktemp /tmp/nbd_test_XXXXXX.bin)
	read_data=$(mktemp /tmp/nbd_read_XXXXXX.bin)

If the test is interrupted or killed by a timeout during dd or cmp, will
test_data and read_data be leaked in /tmp since they are not tracked by
cleanup()? And in cleanup(), since temp_file, conf_file, and pid_file are
created on the host filesystem in init(), is ip netns exec "$ns1" needed to
remove them?

[ ... ]

> +run_server()
> +{
> +	# Create backing file
> +	dd if=/dev/zero of="${temp_file}" bs=1M count=0 seek=256 2>/dev/null
> +
> +	# Detect MPTCP support: native config or mptcpize fallback
> +	local use_mptcpize=""
> +	local mptcp_opt=""
> +	if [ "${trtype}" = "mptcp" ]; then
> +		if ! grep -q '"mptcp"' /usr/bin/nbd-server 2>/dev/null && \
> +		   ! nbd-server --help 2>&1 | grep -qi mptcp; then
> +			if command -v mptcpize &> /dev/null; then
> +				use_mptcpize="mptcpize run -d"
> +				echo "INFO: nbd-server lacks native MPTCP," \
> +					"using mptcpize"
> +			else
> +				echo "ERROR: no MPTCP support in nbd-server"
> +				return 1
> +			fi
> +		else
> +			mptcp_opt="mptcp = true"
> +			echo "INFO: nbd-server has native MPTCP support"
> +		fi
> +	fi

[ ... ]

> +run_client()
> +{
> +	local extra_args=()
> +	local nbd_dev="/dev/nbd0"
> +	local server_addr="10.1.1.1"
> +
> +	local use_mptcpize=""
> +	if [ "${trtype}" = "mptcp" ]; then
> +		if nbd-client 2>&1 | grep -qE "\-M|\-\-mptcp"; then
> +			extra_args+=("-M")
> +			echo "INFO: nbd-client has native MPTCP support (-M)"
> +		elif command -v mptcpize &> /dev/null; then
> +			use_mptcpize="mptcpize run -d"
> +			echo "INFO: nbd-client lacks native MPTCP," \
> +				"using mptcpize"
> +		else
> +			echo "ERROR: no MPTCP support in nbd-client"
> +			return 1
> +		fi
> +	fi

[Severity: Medium]
Does the native MPTCP detection in run_server() and run_client() work as
intended when the top-level script unconditionally skips if mptcpize is
missing?

Later in the script, before run_test() is called, there is an unconditional
check for mptcpize:

	# Check for mptcpize if using MPTCP transport
	if [ "${trtype}" = "mptcp" ]; then
		if ! command -v mptcpize &> /dev/null; then
			mptcp_lib_pr_skip "mptcpize tool not found"
			exit ${KSFT_SKIP}
		fi
	fi

Because that check exits with KSFT_SKIP whenever mptcpize is not installed,
systems where nbd-server and nbd-client have native MPTCP support will still
skip the test if mptcpize is absent, and the else branches in run_server()
and run_client() can never be reached.

In addition, in run_server():

	if ! grep -q '"mptcp"' /usr/bin/nbd-server 2>/dev/null && \
	   ! nbd-server --help 2>&1 | grep -qi mptcp; then

Can grep -q '"mptcp"' /usr/bin/nbd-server ever match a compiled ELF binary?
In a compiled C binary, string literals in .rodata do not include the
surrounding C source double quotes, and /usr/bin/nbd-server is also
hardcoded rather than resolved via command -v nbd-server (for example when
installed in /usr/sbin). Since mptcp = true is a configuration file directive
rather than a command-line option printed by nbd-server --help, won't
run_server() always fall back to mptcpize even when nbd-server supports
native MPTCP?

[ ... ]

> +	# Verify MPTCP subflows if trtype is mptcp
> +	if [ "${trtype}" = "mptcp" ]; then
> +		local sf_total sf_max tcp_conns is_fallback
> +		sf_total=$(ss -Mia | grep -oP "subflows_total:\K[0-9]+" | \
> +			head -1)
> +		sf_max=$(ss -Mia | grep -oP "subflows_max:\K[0-9]+" | head -1)
> +		tcp_conns=$(ss -tia | grep -c "tcp-ulp-mptcp" || true)
> +		is_fallback=$(ss -Mia | grep -c "fallback" || true)
> +		if [ "${is_fallback}" -gt 0 ]; then
> +			echo "FAIL: MPTCP fell back to TCP!" \
> +				"subflows_total:${sf_total}" \
> +				"tcp_subflows:${tcp_conns}"
> +			return 1
> +		elif [ -n "${sf_total}" ]; then
> +			echo "PASS: MPTCP subflows_total:${sf_total}" \
> +				"subflows_max:${sf_max}" \
> +				"tcp_subflows:${tcp_conns}"
> +		else
> +			echo "FAIL: No MPTCP subflows info found"
> +			return 1
> +		fi
> +	fi

[Severity: Medium]
In run_io_test(), is checking [ -n "${sf_total}" ] sufficient to verify that
the additional MPTCP subflows were established?

In net/mptcp/sockopt.c:mptcp_diag_fill_info():

	info->mptcpi_subflows_total = info->mptcpi_extra_subflows +
		__mptcp_has_initial_subflow(msk);

Even if all extra subflows fail to establish across the four interfaces
configured in init(), subflows_total is still 1 for the initial subflow, so
elif [ -n "${sf_total}" ] will pass with only a single subflow.

Also, on systems where ss supports MPTCP but predates the subflows_total
field, sf_total will be empty and this check will fail with "No MPTCP
subflows info found" even though tcp_conns was computed right above. Could
this use mptcp_lib_get_info_value() instead of grep -oP, fall back to
tcp_conns when subflows_total is unavailable (similar to chk_subflows_total()
in mptcp_join.sh), and verify that multiple subflows were actually created?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793010744.git.tanggeliang@kylinos.cn?part=4

Re: [PATCH mptcp-next v2 3/4] nbd: allow MPTCP sockets

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] __sock_xmit() (drivers/block/nbd.c:583) calls
  sk_set_memalloc(sock->sk) inside the do/while loop, so it runs once…
- [High] Once NBD accepts an MPTCP msk, an in-kernel agent can hold the
  NBD data socket's lock while doing allocations that may reclaim.
- [Medium] With this patch an MPTCP msk can be attached to NBD.
- [Medium] nbd_reclassify_socket() (drivers/block/nbd.c:1248-1276)
  switches on sk_family and moves only the top-level socket lock into…
- [Low] The commit author is 'Geliang Tang <geliang@kernel.org>', but the
  only Signed-off-by is 'Geliang Tang <tanggeliang@kylinos.cn>'.

--

nbd: allow MPTCP sockets

This commit makes nbd_get_socket() accept MPTCP sockets as well as TCP
and stream UNIX sockets, by adding an msk_is_mptcp() check. The goal is
to get MPTCP bandwidth aggregation across several NICs for NBD traffic.

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>

[Severity: Low]
This isn't a bug, but the author address (geliang@kernel.org) doesn't
match the Signed-off-by address (tanggeliang@kylinos.cn). checkpatch
reports this as FROM_SIGN_OFF_MISMATCH.

Could a matching Signed-off-by or From: line be added? The other patches
in the series seem to have the same mismatch.

> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index c5e26fc98d1aa..9cf89d53839a1 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c

[ ... ]

> @@ -1222,8 +1223,9 @@ static struct socket *nbd_get_socket(struct nbd_device *nbd, unsigned long fd,
>  		return NULL;
>  
>  	if (!sk_is_tcp(sock->sk) &&
> +	    !msk_is_mptcp(sock->sk) &&

[Severity: High]
Now that MPTCP sockets are accepted here, does the sk_set_memalloc() call
that runs on every loop iteration in __sock_xmit() cause problems on the
I/O path?

drivers/block/nbd.c:__sock_xmit() {
    ...
		do {
			sock->sk->sk_allocation = GFP_NOIO | __GFP_MEMALLOC;
			sock->sk->sk_use_task_frag = false;
			sk_set_memalloc(sock->sk);
			msg.msg_flags = msg_flags | MSG_NOSIGNAL;
    ...
}

That call came from the earlier "sock: add set_memalloc protocol
callback" commit in this series. sk_set_memalloc() increments the global
static key every time, even when SOCK_MEMALLOC is already set:

net/core/sock.c:sk_set_memalloc() {
	sock_set_flag(sk, SOCK_MEMALLOC);
	sk->sk_allocation |= __GFP_MEMALLOC;
	static_branch_inc(&memalloc_socks_key);
	...
}

NBD never calls sk_clear_memalloc(). So memalloc_socks_key goes up once
per send/recv fragment and never comes down. Each increment takes
cpus_read_lock() through static_key_slow_inc().

When the count reaches INT_MAX, static_key_slow_inc_cpuslocked() hits
WARN_ON_ONCE(). After that, every call takes jump_label_mutex.

For MPTCP, each iteration also calls mptcp_set_memalloc(). That takes
lock_sock() on the msk and lock_sock_nested() on every subflow before the
actual sendmsg/recvmsg:

net/mptcp/protocol.c:mptcp_set_memalloc() {
	...
	lock_sock(sk);
	memalloc = sock_flag(sk, SOCK_MEMALLOC);
	mptcp_for_each_subflow(msk, subflow) {
		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);

		lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
	...
}

Should this setup happen once at attach time, as nbd_start_device() and
nbd_reconnect_socket() already do it, and be undone at teardown?

[Severity: Medium]
Do MPTCP subflows created after the socket is attached get the
reclaim-safe settings?

mptcp_set_memalloc() copies SOCK_MEMALLOC, sk_allocation and
sk_use_task_frag only to the subflows that are on msk->conn_list when it
runs. Subflows still on join_list are skipped.

The PM worker creates new subflows through:

mptcp_worker()->mptcp_pm_worker()->__mptcp_subflow_connect()->
    mptcp_subflow_create_socket()->sock_create_kern()

These start with GFP_KERNEL, SOCK_MEMALLOC clear and sk_use_task_frag
set. mptcp_sockopt_sync_locked() doesn't copy these fields, and nothing
else in net/mptcp/ appears to write them.

Until some later __sock_xmit() iteration resyncs them, can
sk_filter_trim_cap() drop pfmemalloc skbs that arrive on such a subflow?
Can the subflow's allocations also enter reclaim in a swap-over-NBD
setup?

And if the sk_set_memalloc() call is moved to attach time, would late
subflows ever be covered?

[Severity: High]
Can this add a reclaim deadlock that plain TCP NBD sockets don't have?

mptcp_worker() holds the msk socket lock while the PM creates subflows:

net/mptcp/protocol.c:mptcp_worker() {
	...
	lock_sock(sk);
	...
	mptcp_pm_worker(msk);
	...
}

mptcp_pm_worker() reaches __mptcp_subflow_connect(). That calls
mptcp_subflow_create_socket()->sock_create_kern(), which does a
GFP_KERNEL sk_alloc. It then calls kernel_connect(), where tcp_connect()
allocates with the subflow's default GFP_KERNEL sk_allocation. The
userspace PM does the same under lock_sock(). None of this runs under
memalloc_noio_save().

Suppose one of those allocations enters direct reclaim and waits on
writeback or swap-out to the NBD device. The NBD I/O that reclaim is
waiting for also needs the msk lock:

__sock_xmit()->sk_set_memalloc()->mptcp_set_memalloc()->lock_sock(msk)
__sock_xmit()->sock_sendmsg()->mptcp_sendmsg()->lock_sock(msk)
__sock_xmit()->sock_recvmsg()->mptcp_recvmsg()->lock_sock(msk)

The timeout path doesn't seem to break the cycle either:
nbd_xmit_timeout()->sock_shutdown()->kernel_sock_shutdown()->
mptcp_shutdown() also takes lock_sock(msk).

The NBD peer can trigger subflow creation remotely, for example with an
ADD_ADDR. That is the multi-subflow setup the commit message describes.

Does anything prevent this cycle: reclaim waits on NBD, NBD waits on the
msk lock, and the msk lock holder waits on reclaim?

[Severity: Medium]
Could this bring back the lockdep circular dependency that commit
d532cddb6c60 ("nbd: Reclassify sockets to avoid lockdep circular
dependency") removed?

nbd_reclassify_socket() reclassifies only the top-level sock. For MPTCP,
that means only the msk lock moves to the sk_lock-AF_INET(6)-NBD class:

drivers/block/nbd.c:nbd_reclassify_socket() {
	...
	switch (sk->sk_family) {
	case AF_INET:
		sock_lock_init_class_and_name(sk, "slock-AF_INET-NBD",
	...
}

The subflows come from sock_create_kern(). They stay in the shared
k-sk_lock-AF_INET(6) class that every kernel TCP socket uses.

NBD I/O takes those subflow locks while holding the NBD-classed msk lock.
Examples are lock_sock(ssk) in __mptcp_push_pending() and
lock_sock_nested(ssk) in mptcp_set_memalloc().

Lockdep already records fs_reclaim -> sk_lock-AF_INET-NBD, so this adds
fs_reclaim -> k-sk_lock-AF_INET.

Any kernel TCP socket in that class that allocates with GFP_KERNEL under
lock_sock() records the reverse edge. One example is
__mptcp_subflow_connect()->kernel_connect()->tcp_connect() on a join
subflow.

Do the subflow locks need to be reclassified too?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1793010744.git.tanggeliang%40kylinos.cn