netdev - Re: [PATCH net 1/2] selftests: mptcp: join: fix 'delete and re-add' test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <94a77161-2299-e470-c0d5-c14cf828cd92@tessares.net>
Date: Mon, 26 Jun 2023 13:31:07 +0200
From: Matthieu Baerts <matthieu.baerts@...sares.net>
To: Andrea Claudi <aclaudi@...hat.com>, netdev@...r.kernel.org,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com
Cc: mptcp@...ts.linux.dev, martineau@...nel.org, geliang.tang@...e.com
Subject: Re: [PATCH net 1/2] selftests: mptcp: join: fix 'delete and re-add'
 test

Hi Andrea,

On 23/06/2023 14:19, Andrea Claudi wrote:
> mptcp_join '002 delete and re-add' test currently fails in the 'after
> delete' testcase.

I guess it only fails if you use "-i" option to use "ip mptcp" instead
of "pm_nl_ctl", right?

MPTCP CI doesn't launch the tests with the "-i" option.

Can you mention that it fails only when using "ip mptcp" which is not
the default mode please? It might be good to include that in the title
too not to think the test is broken and the CI didn't complain about that.

BTW, how did you launch mptcp_join.sh selftest to have this test
launched as second position ("002")? With "-Ii"?

(you can remove this "002": it is specific to the way you launched the
test, not using the default mode)

> This happens because endpoint delete includes an ip address while id is
> not 0, contrary to what is indicated in the ip mptcp man page:
> 
> "When used with the delete id operation, an IFADDR is only included when
> the ID is 0."
> 
> This fixes the issue simply not using the $addr variable in
> pm_nl_del_endpoint().

If you do that, are you not going to break other tests? e.g.
- "remove id 0 subflow"
- "remove id 0 address"

(I didn't check all possibilities, maybe not or maybe there are others)

Because if you specify the ID 0, you do need to specify the address, no?

> Fixes: 34aa6e3bccd8 ("selftests: mptcp: add ip mptcp wrappers")
> Signed-off-by: Andrea Claudi <aclaudi@...hat.com>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 0ae8cafde439..5424dcacfffa 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -678,7 +678,7 @@ pm_nl_del_endpoint()
>  	local addr=$3
>  
>  	if [ $ip_mptcp -eq 1 ]; then
> -		ip -n $ns mptcp endpoint delete id $id $addr
> +		ip -n $ns mptcp endpoint delete id $id

Should you not add "${addr}" only if ${id} == 1?

>  	else
>  		ip netns exec $ns ./pm_nl_ctl del $id $addr
>  	fi

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net