[PATCH mptcp-net 7/7] mptcp: pm: in kernel: only use fullmesh endp if any

Matthieu Baerts (NGI0) posted 7 patches 4 months ago
[PATCH mptcp-net 7/7] mptcp: pm: in kernel: only use fullmesh endp if any
Posted by Matthieu Baerts (NGI0) 4 months ago
Our documentation is saying that the in-kernel PM is only using fullmesh
endpoints to establish subflows to announced addresses when at least one
endpoint has a fullmesh flag. But this was not totally correct: only
fullmesh endpoints were used if at least one endpoint *from the same
address family as the received ADD_ADDR* has the fullmesh flag.

This is confusing, and it seems clearer not to have differences
depending on the address family.

So, now, when at least one MPTCP endpoint has a fullmesh flag, the local
addresses are picked from all fullmesh endpoints, which might be 0 if
there are no endpoints for the correct address family.

One selftest needs to be adapted for this behaviour change.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_kernel.c                           | 10 +++-------
 tools/testing/selftests/net/mptcp/mptcp_join.sh |  6 +++++-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 054d85045381..40a0572b3149 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -609,15 +609,11 @@ fill_local_addresses_vec(struct mptcp_sock *msk, struct mptcp_addr_info *remote,
 			 struct mptcp_pm_local *locals)
 {
 	bool c_flag_case = remote->id && mptcp_pm_add_addr_c_flag_case(msk);
-	int i;
 
 	/* If there is at least one MPTCP endpoint with a fullmesh flag */
-	if (mptcp_pm_get_endp_fullmesh_max(msk)) {
-		i = fill_local_addresses_vec_fullmesh(msk, remote, locals,
-						      c_flag_case);
-		if (i)
-			return i;
-	}
+	if (mptcp_pm_get_endp_fullmesh_max(msk))
+		return fill_local_addresses_vec_fullmesh(msk, remote, locals,
+							 c_flag_case);
 
 	/* If there is at least one MPTCP endpoint with a laminar flag */
 	if (mptcp_pm_get_endp_laminar_max(msk))
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 78a1aa4ecff2..e7a498dd5a46 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2952,7 +2952,11 @@ mixed_tests()
 		pm_nl_add_endpoint $ns1 10.0.1.1 flags signal
 		speed=slow \
 			run_tests $ns1 $ns2 dead:beef:2::1
-		chk_join_nr 1 1 1
+		if mptcp_lib_kallsyms_has "mptcp_pm_get_endp_fullmesh_max$"; then
+			chk_join_nr 0 0 0
+		else
+			chk_join_nr 1 1 1
+		fi
 	fi
 
 	# fullmesh still tries to create all the possibly subflows with

-- 
2.51.0
Re: [PATCH mptcp-net 7/7] mptcp: pm: in kernel: only use fullmesh endp if any
Posted by Geliang Tang 3 months, 3 weeks ago
Hi Matt,

On Wed, 2025-10-08 at 16:00 +0200, Matthieu Baerts (NGI0) wrote:
> Our documentation is saying that the in-kernel PM is only using
> fullmesh
> endpoints to establish subflows to announced addresses when at least
> one
> endpoint has a fullmesh flag. But this was not totally correct: only
> fullmesh endpoints were used if at least one endpoint *from the same
> address family as the received ADD_ADDR* has the fullmesh flag.
> 
> This is confusing, and it seems clearer not to have differences
> depending on the address family.
> 
> So, now, when at least one MPTCP endpoint has a fullmesh flag, the
> local
> addresses are picked from all fullmesh endpoints, which might be 0 if
> there are no endpoints for the correct address family.
> 
> One selftest needs to be adapted for this behaviour change.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

This patch LGTM!

Reviewed-by: Geliang Tang <geliang@kernel.org>

Except for patch 1, I have added my reviewed-by tags to all other
patches. Please give me some more time to review patch 1. If you're
willing, please apply patches 2-7 first.

Thanks,
-Geliang

> ---
>  net/mptcp/pm_kernel.c                           | 10 +++-------
>  tools/testing/selftests/net/mptcp/mptcp_join.sh |  6 +++++-
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index 054d85045381..40a0572b3149 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -609,15 +609,11 @@ fill_local_addresses_vec(struct mptcp_sock
> *msk, struct mptcp_addr_info *remote,
>  			 struct mptcp_pm_local *locals)
>  {
>  	bool c_flag_case = remote->id &&
> mptcp_pm_add_addr_c_flag_case(msk);
> -	int i;
>  
>  	/* If there is at least one MPTCP endpoint with a fullmesh
> flag */
> -	if (mptcp_pm_get_endp_fullmesh_max(msk)) {
> -		i = fill_local_addresses_vec_fullmesh(msk, remote,
> locals,
> -						      c_flag_case);
> -		if (i)
> -			return i;
> -	}
> +	if (mptcp_pm_get_endp_fullmesh_max(msk))
> +		return fill_local_addresses_vec_fullmesh(msk,
> remote, locals,
> +							
> c_flag_case);
>  
>  	/* If there is at least one MPTCP endpoint with a laminar
> flag */
>  	if (mptcp_pm_get_endp_laminar_max(msk))
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 78a1aa4ecff2..e7a498dd5a46 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -2952,7 +2952,11 @@ mixed_tests()
>  		pm_nl_add_endpoint $ns1 10.0.1.1 flags signal
>  		speed=slow \
>  			run_tests $ns1 $ns2 dead:beef:2::1
> -		chk_join_nr 1 1 1
> +		if mptcp_lib_kallsyms_has
> "mptcp_pm_get_endp_fullmesh_max$"; then
> +			chk_join_nr 0 0 0
> +		else
> +			chk_join_nr 1 1 1
> +		fi
>  	fi
>  
>  	# fullmesh still tries to create all the possibly subflows
> with
> 

Re: [PATCH mptcp-net 7/7] mptcp: pm: in kernel: only use fullmesh endp if any
Posted by Matthieu Baerts 3 months, 3 weeks ago
Hi Geliang,

On 14/10/2025 12:10, Geliang Tang wrote:
> Hi Matt,
> 
> On Wed, 2025-10-08 at 16:00 +0200, Matthieu Baerts (NGI0) wrote:
>> Our documentation is saying that the in-kernel PM is only using
>> fullmesh
>> endpoints to establish subflows to announced addresses when at least
>> one
>> endpoint has a fullmesh flag. But this was not totally correct: only
>> fullmesh endpoints were used if at least one endpoint *from the same
>> address family as the received ADD_ADDR* has the fullmesh flag.
>>
>> This is confusing, and it seems clearer not to have differences
>> depending on the address family.
>>
>> So, now, when at least one MPTCP endpoint has a fullmesh flag, the
>> local
>> addresses are picked from all fullmesh endpoints, which might be 0 if
>> there are no endpoints for the correct address family.
>>
>> One selftest needs to be adapted for this behaviour change.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> This patch LGTM!
> 
> Reviewed-by: Geliang Tang <geliang@kernel.org>
> 
> Except for patch 1, I have added my reviewed-by tags to all other
> patches. Please give me some more time to review patch 1. If you're
> willing, please apply patches 2-7 first.

Thank you for your reviews.

I will wait for the review of patch 1: that's the most "urgent" one
(fixing a small behavioural issue), the other ones can wait.

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