Re: [PATCH mptcp-net v7 2/2] selftests: mptcp: join: re-add the initial subflow endp with fullmesh

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: mptcp: unset iptables variable in endpoint_tests() breaks test
--

commit 5d801b552c8fda4d06f17bc60e0d917102aa1d3c
Author: Abdul Wasey <w453y.me@gmail.com>

selftests: mptcp: join: re-add the initial subflow endp with fullmesh

This commit adds four subtests to endpoint_tests() in mptcp_join.sh to
validate removing and re-adding the initial subflow endpoint when using the
fullmesh flag.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index f4d8afaeda4f3..98dc8de81cf01 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -4424,6 +4424,135 @@ endpoint_tests()

[ ... ]

> +	# same, but the initial subflow is reset before removing its endpoint
> +	if reset_with_events "delete and re-add, fullmesh, after reset" &&
> +	   continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
> +		pm_nl_set_limits $ns1 0 2
> +		pm_nl_set_limits $ns2 0 2
> +		pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
> +		pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow,fullmesh
> +		{ timeout_test=120 test_linkfail=128 speed=5 \
> +			run_tests $ns1 $ns2 10.0.1.1 & } 2>/dev/null
> +		local tests_pid=$!
> +
> +		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
> +		wait_mpj 1
> +		chk_subflow_nr "before reset" 2
> +
> +		if ip netns exec "${ns1}" ${iptables} -A INPUT -s "10.0.1.2" \
> +		      -p tcp --tcp-option 30 -j REJECT --reject-with tcp-reset; then
> +			wait_event ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1
> +			ip netns exec "${ns1}" ${iptables} -F INPUT
> +			chk_evt_nr ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1 error 1
> +
> +			pm_nl_del_endpoint $ns2 1 10.0.1.2
> +			pm_nl_add_endpoint $ns2 10.0.1.2 id 1 dev ns2eth1 flags subflow,fullmesh
> +			wait_mpj 2
> +			chk_mptcp_info subflows 2 subflows 2
> +			chk_join_nr 2 2 2
> +		else
> +			mark_as_skipped "unable to set the filter rules"
> +		fi

[Severity: Medium]
Will this filter setup in endpoint_tests() always fail?

Commit be3b69f3cedfaf ("selftests: mptcp: convert iptables to nftables for
mptcp_join.sh") converted mptcp_join.sh to nftables and removed the
definition of the iptables variable.

Because ${iptables} is unset and unquoted here, the command expands to:

    ip netns exec "${ns1}" -A INPUT -s "10.0.1.2" ...

which fails immediately, causing this subtest to always fall into the else
branch calling mark_as_skipped (or exit with KSFT_FAIL before killing
$tests_pid and the event listeners when
SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES=1 is set).

Should this subtest call init_nftables and use nft to add and flush the rule,
matching userspace_tests() in mptcp_join.sh:

    init_nftables
    ...
    if ip netns exec "${ns1}" nft add rule ip filter INPUT \
            ip saddr "10.0.1.2" meta l4proto tcp \
            tcp option mptcp exists reject with tcp reset; then
        wait_event ns2 MPTCP_LIB_EVENT_SUB_CLOSED 1
        ip netns exec "${ns1}" nft flush chain ip filter INPUT
        ...

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023110321.403874-1-w453y.me@gmail.com?part=2