MPTCP Linux Development
 help / color / mirror / Atom feed
* [PATCH mptcp-next v2 0/4] Add MPTCP support for NBD
@ 2026-10-26 10:35 Geliang Tang
  2026-10-26 10:35 ` [PATCH mptcp-next v2 1/4] mptcp: give MPTCP sockets a distinct lockdep class Geliang Tang
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Geliang Tang @ 2026-10-26 10:35 UTC (permalink / raw)
  To: mptcp; +Cc: 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


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next v2 4/4] selftests: mptcp: add mptcp nbd tests
@ 2026-10-27 23:46 netdev-bot+sashiko
  0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-27 23:46 UTC (permalink / raw)
  To: geliang; +Cc: mptcp, tanggeliang, kuba

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

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

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] In tools/testing/selftests/net/mptcp/mptcp_nbd.sh, the EXIT trap
  cleanup() always runs `ip netns exec "$ns2" nbd-client -d /dev/nbd0`…
- [Medium] The MPTCP check in run_io_test() passes as long as
  `subflows_total` is present in `ss -Mia` output (`elif [ -n…
- [Low] In run_io_test(), the 'data integrity' check writes 10 MiB to
  /dev/nbd0 with buffered dd plus conv=fdatasync, then reads it back…
- [Low] The native-MPTCP handling does not fit together.
- [Low] run_io_test() creates two temp files in the host's /tmp (`mktemp
  /tmp/nbd_test_XXXXXX.bin` and `/tmp/nbd_read_XXXXXX.bin`, about 10 MiB…
- [Low] cleanup() sends `kill` and then immediately, without checking,
  `kill -9` to server_pid.

--

selftests: mptcp: add mptcp nbd tests

This commit adds mptcp_nbd.sh. The script sets up two namespaces joined by
four rate-limited veth pairs and exports a 256MB file with nbd-server. It
connects /dev/nbd0 with nbd-client over TCP or MPTCP and runs dd and fio
I/O tests. It also adds nbd and fio cases to mptcp_lib_check_tools() and
enables CONFIG_BLK_DEV_NBD in the selftest config.

> 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

[Severity: High]
Can this disconnect an nbd0 that this test never connected?

cleanup() runs from the EXIT trap. The trap is installed before init(),
run_server() and run_client(), so cleanup() runs on every failure path.
Before that point, the script only checks that the device node exists:

	if [ ! -b /dev/nbd0 ]; then
		mptcp_lib_pr_skip "/dev/nbd0 not found"
		exit ${KSFT_SKIP}
	fi
	...
	trap cleanup EXIT

NBD devices are global, not per-netns. The genl family sets .netnsok = 1,
and nbd_genl_disconnect() only does this check:

drivers/block/nbd.c:nbd_genl_disconnect() {
    ...
	if (!netlink_capable(skb, CAP_SYS_ADMIN))
		return -EPERM;
    ...
}

The NBD_DISCONNECT ioctl also only needs CAP_SYS_ADMIN. So running
nbd-client -d inside ns2 gives no isolation from the host.

If the host is already using /dev/nbd0 (a mounted filesystem, a VM disk,
root on NBD), the connect in run_client() fails in nbd_genl_connect():

drivers/block/nbd.c:nbd_genl_connect() {
    ...
		pr_err("nbd%d already in use\n", index);
		return -EBUSY;
    ...
}

The test then reports FAIL instead of SKIP. After that, the EXIT trap
disconnects the host's live nbd0 connection.

Two concurrent runs hit the same path, for example mptcp_nbd.sh tcp and
mptcp_nbd.sh mptcp. The second run fails to connect, and its cleanup()
disconnects the first run's device in the middle of dd or fio I/O.

Could the script skip when /dev/nbd0 is already in use, and have cleanup()
disconnect only if this run's connect succeeded?

> +
> +	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

[Severity: Low]
Can these kill calls signal an unrelated process?

server_pid comes from run_server(). That function starts nbd-server in the
background from a short-lived ns1 bash and saves $! to pid_file:

	${use_mptcpize} nbd-server -C "${conf_file}" -n > /dev/null 2>&1 &
	server_pid=$!

Nothing checks that this PID still belongs to nbd-server when cleanup()
runs. ip netns exec does not create a PID namespace, so this is a host PID.

Suppose nbd-server exits early (crash, bind failure, an mptcpize problem),
gets reaped, and its PID is reused before cleanup() runs. Then the SIGTERM
and the immediate SIGKILL go to another process, sent as root. This needs
the PID to be reused within a few seconds, so it is unlikely.

[ ... ]

> +# 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)

[Severity: Low]
Are these two files left in /tmp if the run is interrupted?

The file names exist only as locals in the ns2 bash child that run_test()
starts. The normal return paths remove them. However, the child has no
trap, and the parent's cleanup() only knows about temp_file, conf_file and
pid_file.

If the child is killed during the dd or cmp steps (SIGINT or SIGTERM), both
files stay in /tmp. Each one is about 10 MiB.

> +
> +	# 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

[Severity: Low]
Does this read-back always go over the network?

Both dd calls are buffered. conv=fdatasync writes the dirty pages back, but
the pages stay in the bdev page cache. blkdev_read_iter() serves buffered
reads through filemap_read(). The cache is dropped only when the last
opener closes the device:

block/bdev.c:blkdev_put_whole() {
	if (atomic_dec_and_test(&bdev->bd_openers))
		blkdev_flush_mapping(bdev);
    ...
}

Something else may hold /dev/nbd0 open, such as an ioctl-mode nbd-client
child waiting in NBD_DO_IT, or a udev probe. In that case the read-back can
come from the page cache instead of from nbd-server. "PASS: Data integrity
verified" then says nothing about the network receive path.

With the default netlink-mode nbd-client, nothing else holds the device
open. So whether this happens depends on the setup.

The fio randread and randwrite jobs use --direct=1 but not --verify, so
they do not check the data either.

Would iflag=direct on the read-back, or blockdev --flushbufs before it,
make this check reliable?

Also, a later comment in this function reads:

	# dd write test (larger, for performance measurement)

That write is bs=4k count=1000, about 4 MB, which is smaller than the
10 MiB integrity write. Its output is also discarded, so nothing is
measured.

[ ... ]

> +	# 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

[Severity: Medium]
Would this check catch a regression where no extra subflows are created?

The PASS branch only needs sf_total to be non-empty.
mptcp_diag_fill_info() includes the initial subflow in the count:

net/mptcp/sockopt.c:mptcp_diag_fill_info() {
    ...
	info->mptcpi_subflows_total = info->mptcpi_extra_subflows +
		__mptcp_has_initial_subflow(msk);
    ...
}

So any MPTCP connection that did not fall back reports at least 1.

init() configures 4 signal endpoints and 4 subflow endpoints with 8/8
limits. If no extra subflow is ever created (ADD_ADDR ignored, MP_JOIN
failing, limits not applied), this still prints PASS with
subflows_total:1.

The opposite case also goes wrong. If ss supports -M but does not print
subflows_total, sf_total is empty and the test reports "FAIL: No MPTCP
subflows info found". That is an iproute2 version problem reported as a
test failure.

chk_subflows_total() in mptcp_join.sh already handles this case:

	# if subflows_total counter is supported, use it:
	if [ -n "$(ss -N $ns1 -inmHM | mptcp_lib_get_info_value $info $info)" ]; then

Could this check compare against the expected number of subflows, and skip
or use another check when the counter is not available?

[ ... ]

> +# 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

[Severity: Low]
Does this top-level check leave the error branches in run_server() and
run_client() unreachable?

When run_server() and run_client() run, mptcpize is always present. So
these branches can never be taken:

	echo "ERROR: no MPTCP support in nbd-server"
	return 1
	...
	echo "ERROR: no MPTCP support in nbd-client"
	return 1

A system whose nbd-server and nbd-client support MPTCP natively, but that
has no mptcpize installed, is also skipped. The native path would work
there.

The server detection in run_server() also looks unreliable:

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

The binary path is hard-coded. mptcp_lib_check_tools() finds the tool
through PATH with command -v instead.

The pattern also includes the double quotes, which a compiled string
literal normally does not contain. In practice, detection relies on
nbd-server --help alone.

When native support is detected, "mptcp = true" goes into the config file.
Nothing checks that this nbd-server version accepts that key.

Could the mptcpize requirement apply only when native support is not
detected?

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

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-10-27 23:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-26 10:35 [PATCH mptcp-next v2 0/4] Add MPTCP support for NBD Geliang Tang
2026-10-26 10:35 ` [PATCH mptcp-next v2 1/4] mptcp: give MPTCP sockets a distinct lockdep class Geliang Tang
2026-10-26 11:27   ` sashiko-bot
2026-10-26 10:35 ` [PATCH mptcp-next v2 2/4] sock: add set_memalloc protocol callback Geliang Tang
2026-10-26 11:46   ` sashiko-bot
2026-10-26 10:35 ` [PATCH mptcp-next v2 3/4] nbd: allow MPTCP sockets Geliang Tang
2026-10-26 12:14   ` sashiko-bot
2026-10-26 10:35 ` [PATCH mptcp-next v2 4/4] selftests: mptcp: add mptcp nbd tests Geliang Tang
2026-10-26 12:32   ` sashiko-bot
2026-10-26 12:04 ` [PATCH mptcp-next v2 0/4] Add MPTCP support for NBD MPTCP CI
  -- strict thread matches above, loose matches on Subject: below --
2026-10-27 23:46 [PATCH mptcp-next v2 4/4] selftests: mptcp: add mptcp nbd tests netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox