Re: [PATCH mptcp-net v4 7/8] selftests: mptcp: join: new ID0 subflow from the right IP

[netdev-bot+sashiko@kernel.org]

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

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

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).

- [Medium] In userspace_tests()
  (tools/testing/selftests/net/mptcp/mptcp_join.sh, test "userspace pm…

--

selftests: mptcp: join: new ID0 subflow from the right IP

This patch updates the "userspace pm create id 0 subflow" test so that the
ID0 subflow is created from the initial address 10.0.1.2. It still tries
10.0.3.2 first, which the kernel is now expected to reject. The expected
dump output becomes an empty list, because the initial address is not
dumped.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index c3ccce96b45b1..190ab48acbc61 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -4214,10 +4214,11 @@ userspace_tests()
>  		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
>  		chk_mptcp_info subflows 0 subflows 0
>  		chk_subflows_total 1 1
> -		userspace_pm_add_sf $ns2 10.0.3.2 0
> +		# from an IP not linked to ID0: failure expected, no new MPJ
> +		userspace_pm_add_sf $ns2 10.0.3.2 0 2>/dev/null
> +		userspace_pm_add_sf $ns2 10.0.1.2 0
>  		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
> -		userspace_pm_chk_dump_addr "${ns2}" \
> -			"id 0 flags subflow 10.0.3.2" "id 0 subflow"
> +		userspace_pm_chk_dump_addr "${ns2}" "" "id 0 subflow"
>  		chk_join_nr 1 1 1
>  		chk_mptcp_info subflows 1 subflows 1
>  		chk_subflows_total 2 2

[Severity: Medium]
Does anything here check that the 10.0.3.2 request actually fails?

userspace_pm_add_sf() returns the exit status of pm_nl_ctl csf. That
status is 1 when the kernel rejects the request. Here it is ignored and
stderr goes to /dev/null. The status of the 10.0.1.2 call is not checked
either.

All the checks that follow only count subflows:

    wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
    chk_join_nr 1 1 1
    chk_mptcp_info subflows 1 subflows 1
    chk_subflows_total 2 2

So they cannot tell which of the two requests created the extra subflow.

The new empty dump expectation cannot tell them apart either.
mptcp_userspace_pm_dump_addr() skips every entry that matches
is_init_id0(), and that check ignores the address:

net/mptcp/pm_userspace.c:is_init_id0() {
	return entry->addr.id == 0 && entry->addr.port == 0;
}

Suppose the initial ID0 entry were missing from the userspace PM list.
That can happen if the GFP_ATOMIC sock_kmalloc() in
mptcp_pm_userspace_created() fails, or if a later change breaks that path.
In that case, mptcp_userspace_pm_append_new_local_addr() would accept the
10.0.3.2 request and create an ID0 subflow from the wrong IP. The
10.0.1.2 request would then fail with -EINVAL, because the ID matches but
the address does not.

Would every assertion above still pass in that case?

The commit message says the wrong IP is kept:

    "to check that the kernel is correctly not allowing the creation of
    an ID0 subflow with the wrong IP address"

As written, though, the test passes whether or not the kernel enforces
this. It only fails if both requests succeed.

Could the failure be asserted explicitly, for example:

	if userspace_pm_add_sf $ns2 10.0.3.2 0 2>/dev/null; then
		fail_test "ID0 subflow from wrong IP accepted"
	fi

Alternatively, could the test check that the saddr4 of the SUB_ESTABLISHED
event is 10.0.1.2?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261029-mptcp-pm-userspace-id0-case-v4-0-555066fa4292%40kernel.org