Re: [PATCH MPTCH-next] selftests: add test for userspace PM address ID overflow

[Matthieu Baerts <matttbe@kernel.org>]

Hi luoqing,

On 04/08/2026 04:23, luoqing wrote:
> From: Qing Luo <luoqing@kylinos.cn>
> 
> Add a test that verifies the userspace PM correctly returns an error
> when all address IDs (1-255) are exhausted, instead of overflowing.
> 
> The test first announces 255 unique addresses (IDs 1-255) to fill the
> ID bitmap. It then attempts to create a subflow (CSF) without specifying
> a local ID, which triggers auto-allocation via
> mptcp_userspace_pm_get_local_id(). With all IDs in use, the allocation
> should fail with -ENOSPC.
> 
> Also modify pm_nl_ctl to make the 'lid' parameter optional for the CSF
> command. When omitted, the kernel auto-allocates a local ID.

Same as for the other patch: should be in a different patch, but can
certainly be dropped.

> MPTCP_PM_MAX_ADDR_ID has been 255 since the userspace PM was introduced,
> so no version-dependent limit adjustment is needed (unlike pm_netlink.sh).
> 
> Assisted-by: LLM # code
> Signed-off-by: Qing Luo <luoqing@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/pm_nl_ctl.c |  8 +--
>  .../selftests/net/mptcp/userspace_pm.sh       | 63 +++++++++++++++++++
>  2 files changed, 67 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> index 78180da1efcc..a9dd650805a9 100644
> --- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> +++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> @@ -32,7 +32,7 @@ static void syntax(char *argv[])
>  	fprintf(stderr, "\tadd [flags signal|subflow|backup|fullmesh] [id <nr>] [dev <name>] <ip>\n");
>  	fprintf(stderr, "\tann <local-ip> id <local-id> token <token> [port <local-port>] [dev <name>]\n");
>  	fprintf(stderr, "\trem id <local-id> token <token>\n");
> -	fprintf(stderr, "\tcsf lip <local-ip> lid <local-id> rip <remote-ip> rport <remote-port> token <token>\n");
> +	fprintf(stderr, "\tcsf lip <local-ip> [lid <local-id>] rip <remote-ip> rport <remote-port> token <token>\n");
>  	fprintf(stderr, "\tdsf lip <local-ip> lport <local-port> rip <remote-ip> rport <remote-port> token <token>\n");
>  	fprintf(stderr, "\tdel <id> [<ip>]\n");
>  	fprintf(stderr, "\tget <id>\n");
> @@ -481,7 +481,7 @@ int csf(int fd, int pm_family, int argc, char *argv[])
>  	off = init_genl_req(data, pm_family, MPTCP_PM_CMD_SUBFLOW_CREATE,
>  			    MPTCP_PM_VER);
>  
> -	if (argc < 12)
> +	if (argc < 10)
>  		syntax(argv);
>  
>  	/* Params recorded in this order:
> @@ -557,9 +557,9 @@ int csf(int fd, int pm_family, int argc, char *argv[])
>  			off += NLMSG_ALIGN(rta->rta_len);
>  		}
>  
> -		if (arg == 0) {
> +		if (arg == 0 && params[1]) {
>  			/* id */
> -			id = atoi(params[arg + 1]);
> +			id = atoi(params[1]);
>  			rta = (void *)(data + off);
>  			rta->rta_type = MPTCP_PM_ADDR_ATTR_ID;
>  			rta->rta_len = RTA_LENGTH(1);
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index 30a809752d1b..45b040a8c0e5 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -847,6 +847,68 @@ test_subflows_v4_v6_mix()
>  	sleep 0.5
>  }
>  
> +test_addr_id_overflow()
> +{
> +	print_title "Address ID overflow tests"
> +
> +	local i announced=0
> +
> +	:>"$server_evts"
> +
> +	# Clear leftover addresses from previous tests
> +	for i in $(seq 0 255); do
> +		ip netns exec "$ns2" ./pm_nl_ctl rem token "$client4_token" id "$i" > /dev/null 2>&1
> +	done

Is this really needed?

> +
> +	# Announce 255 addresses (IDs 1-255) to exhaust all available IDs
> +	for i in $(seq 1 255); do
> +		if ip netns exec "$ns2" ./pm_nl_ctl ann 10.0.3."${i}" token "$client4_token" id \
> +			"$i" dev ns2eth1 > /dev/null 2>&1; then

Why hiding errors? (OK to hide if it is just to use all bits, then try
one extra to check the error)

> +			announced=$((announced + 1))

Set but not used?

> +		fi
> +	done

Mmh, all of this is very slow: 19 seconds on the CI with a "normal"
kconfig, 52 seconds with a "debug" one...

That's a lot... Any ideas on how to reduce this time? (maybe not possible?)

> +
> +	print_test "ADD_ADDR with all IDs 1-255 exhausted"
> +	sleep 1

Why this "sleep 1"?

> +	if [ -s "$server_evts" ]; then
> +		test_pass
> +	else
> +		test_fail "No events generated"
> +		return
> +	fi
> +
> +	# Start listener to ensure subflow creation doesn't fail on connectivity

Really needed?

> +	ip netns exec "$ns1" ./pm_nl_ctl listen 10.0.1.1 "$app4_port" >/dev/null 2>&1 &
> +	local listener_pid=$!
> +	sleep 0.5
> +
> +	# Try to create a subflow without specifying a local ID.
> +	# With all IDs exhausted, this should fail with -ENOSPC.
> +	print_test "CSF without local ID after all IDs exhausted - expect failure"
> +	local out
> +	if out=$(ip netns exec "$ns2" ./pm_nl_ctl csf lip 10.0.1.2 \
> +		rip 10.0.1.1 rport "$app4_port" token "$client4_token" 2>&1); then

I guess you cannot have an overflow with csf, because an ID is required.

> +		test_fail "Expected failure but CSF succeeded"
> +	else
> +		# pm_nl_ctl prints the kernel error as "netlink error -28 (No space
> +		# left on device)" for -ENOSPC. Match either form.

Really needed? The pm_nl_ctl should fail.

> +		if echo "$out" | grep -qE "netlink error -?28|No space left on device"; then
> +			test_pass
> +		else
> +			test_fail "CSF failed, but not with the expected ENOSPC: ${out}"
> +		fi
> +	fi
> +
> +	# Delete the listener from the server ns, if one was created
> +	mptcp_lib_kill_wait $listener_pid
> +
> +	# Cleanup: remove all announced addresses
> +	for i in $(seq 1 255); do
> +		ip netns exec "$ns2" ./pm_nl_ctl rem token "$client4_token" id "$i" > /dev/null 2>&1
> +	done

Maybe not needed?

> +	sleep 1

Clearly not needed.

> +}
> +
>  test_prio()
>  {
>  	print_title "Prio tests"
> @@ -940,6 +1002,7 @@ test_subflows
>  test_subflows_v4_v6_mix
>  test_prio
>  test_listener
> +test_addr_id_overflow
>  
>  mptcp_lib_result_print_all_tap
>  exit ${ret}

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.