[PATCH mptcp-net 2/2] selftests: mptcp: join: validate C-flag + def limit

Matthieu Baerts (NGI0) posted 2 patches 1 day, 22 hours ago
There is a newer version of this series
[PATCH mptcp-net 2/2] selftests: mptcp: join: validate C-flag + def limit
Posted by Matthieu Baerts (NGI0) 1 day, 22 hours ago
The previous commit adds an exception for the C-flag case. The
'mptcp_join.sh' selftest is extended to validate this case.

In this subtest, there is a typical CDN deployment with a client where
MPTCP endpoints have been 'automatically' configured:

- the server set net.mptcp.allow_join_initial_addr_port=0

- the client has multiple 'subflow' endpoints, and the default limits:
  not accepting ADD_ADDRs.

Without the parent patch, the client is not able to establish new
subflows using its 'subflow' endpoints. The parent commit fixes that.

The 'Fixes' tag here below is the same as the one from the previous
commit: this patch here is not fixing anything wrong in the selftests,
but it validates the previous fix for an issue introduced by this commit
ID.

Fixes: df377be38725 ("mptcp: add deny_join_id0 in mptcp_options_received")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c                                  |  4 +++-
 net/mptcp/pm_kernel.c                           |  1 +
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 +++++++++++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index fda8d1880224dedbbfb20954f93a04da435346df..5d037f9d4e0b4d158beaaa5a48c4efb50fe9b5cb 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -637,7 +637,9 @@ void mptcp_pm_add_addr_received(const struct sock *ssk,
 		} else {
 			__MPTCP_INC_STATS(sock_net((struct sock *)msk), MPTCP_MIB_ADDADDRDROP);
 		}
-	/* id0 should not have a different address ; special case for C-flag */
+	/* - id0 should not have a different address
+	 * - special case for C-flag: linked to fill_local_addresses_vec()
+	 */
 	} else if ((addr->id == 0 && !mptcp_pm_is_init_remote_addr(msk, addr)) ||
 		   (addr->id > 0 && !READ_ONCE(pm->accept_addr) &&
 		    (!READ_ONCE(msk->pm.remote_deny_join_id0) ||
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 408801e20ed6e874b80ccbf38af76abc13615a3a..aae9b92aea4715dabaf3101a0341ab7eab16f407 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -394,6 +394,7 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
 
 	pernet = pm_nl_get_pernet_from_msk(msk);
 	subflows_max = mptcp_pm_get_subflows_max(msk);
+	/* linked to conditions in mptcp_pm_add_addr_received() */
 	deny_join_id0 = remote->id && READ_ONCE(msk->pm.remote_deny_join_id0) &&
 			!READ_ONCE(msk->pm.accept_addr) &&
 			msk->pm.local_addr_used == 0 &&
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 6055ee5762e13108e5e2924a0e77d58da584d008..e80f90581a2531ac31339ae3543604c68677c232 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3306,6 +3306,17 @@ deny_join_id0_tests()
 		run_tests $ns1 $ns2 10.0.1.1
 		chk_join_nr 1 1 1
 	fi
+
+	# default limit, server deny join id 0 + signal
+	if reset_with_allow_join_id0 "default limit, server deny join id 0" 0 1; then
+		pm_nl_set_limits $ns1 0 2
+		pm_nl_set_limits $ns2 0 2
+		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
+		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
+		pm_nl_add_endpoint $ns2 10.0.4.2 flags subflow
+		run_tests $ns1 $ns2 10.0.1.1
+		chk_join_nr 2 2 2
+	fi
 }
 
 fullmesh_tests()

-- 
2.51.0
Re: [PATCH mptcp-net 2/2] selftests: mptcp: join: validate C-flag + def limit
Posted by Geliang Tang 1 day, 6 hours ago
On Mon, 2025-09-15 at 20:11 +0200, Matthieu Baerts (NGI0) wrote:
> The previous commit adds an exception for the C-flag case. The
> 'mptcp_join.sh' selftest is extended to validate this case.
> 
> In this subtest, there is a typical CDN deployment with a client
> where
> MPTCP endpoints have been 'automatically' configured:
> 
> - the server set net.mptcp.allow_join_initial_addr_port=0
> 
> - the client has multiple 'subflow' endpoints, and the default
> limits:
>   not accepting ADD_ADDRs.
> 
> Without the parent patch, the client is not able to establish new
> subflows using its 'subflow' endpoints. The parent commit fixes that.
> 
> The 'Fixes' tag here below is the same as the one from the previous
> commit: this patch here is not fixing anything wrong in the
> selftests,
> but it validates the previous fix for an issue introduced by this
> commit
> ID.
> 
> Fixes: df377be38725 ("mptcp: add deny_join_id0 in
> mptcp_options_received")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm.c                                  |  4 +++-
>  net/mptcp/pm_kernel.c                           |  1 +
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 +++++++++++
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index
> fda8d1880224dedbbfb20954f93a04da435346df..5d037f9d4e0b4d158beaaa5a48c
> 4efb50fe9b5cb 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -637,7 +637,9 @@ void mptcp_pm_add_addr_received(const struct sock
> *ssk,
>  		} else {
>  			__MPTCP_INC_STATS(sock_net((struct sock
> *)msk), MPTCP_MIB_ADDADDRDROP);
>  		}
> -	/* id0 should not have a different address ; special case
> for C-flag */
> +	/* - id0 should not have a different address
> +	 * - special case for C-flag: linked to
> fill_local_addresses_vec()
> +	 */

How about squashing this into patch 1.

>  	} else if ((addr->id == 0 &&
> !mptcp_pm_is_init_remote_addr(msk, addr)) ||
>  		   (addr->id > 0 && !READ_ONCE(pm->accept_addr) &&
>  		    (!READ_ONCE(msk->pm.remote_deny_join_id0) ||
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index
> 408801e20ed6e874b80ccbf38af76abc13615a3a..aae9b92aea4715dabaf3101a034
> 1ab7eab16f407 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -394,6 +394,7 @@ static unsigned int
> fill_local_addresses_vec(struct mptcp_sock *msk,
>  
>  	pernet = pm_nl_get_pernet_from_msk(msk);
>  	subflows_max = mptcp_pm_get_subflows_max(msk);
> +	/* linked to conditions in mptcp_pm_add_addr_received() */

And squash this too.

WDYT?

Thanks,
-Geliang

>  	deny_join_id0 = remote->id && READ_ONCE(msk-
> >pm.remote_deny_join_id0) &&
>  			!READ_ONCE(msk->pm.accept_addr) &&
>  			msk->pm.local_addr_used == 0 &&
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index
> 6055ee5762e13108e5e2924a0e77d58da584d008..e80f90581a2531ac31339ae3543
> 604c68677c232 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -3306,6 +3306,17 @@ deny_join_id0_tests()
>  		run_tests $ns1 $ns2 10.0.1.1
>  		chk_join_nr 1 1 1
>  	fi
> +
> +	# default limit, server deny join id 0 + signal
> +	if reset_with_allow_join_id0 "default limit, server deny
> join id 0" 0 1; then
> +		pm_nl_set_limits $ns1 0 2
> +		pm_nl_set_limits $ns2 0 2
> +		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
> +		pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow
> +		pm_nl_add_endpoint $ns2 10.0.4.2 flags subflow
> +		run_tests $ns1 $ns2 10.0.1.1
> +		chk_join_nr 2 2 2
> +	fi
>  }
>  
>  fullmesh_tests()
Re: [PATCH mptcp-net 2/2] selftests: mptcp: join: validate C-flag + def limit
Posted by Matthieu Baerts 1 day, 6 hours ago
Hi Geliang,

On 16/09/2025 11:24, Geliang Tang wrote:
> On Mon, 2025-09-15 at 20:11 +0200, Matthieu Baerts (NGI0) wrote:
>> The previous commit adds an exception for the C-flag case. The
>> 'mptcp_join.sh' selftest is extended to validate this case.
>>
>> In this subtest, there is a typical CDN deployment with a client
>> where
>> MPTCP endpoints have been 'automatically' configured:
>>
>> - the server set net.mptcp.allow_join_initial_addr_port=0
>>
>> - the client has multiple 'subflow' endpoints, and the default
>> limits:
>>   not accepting ADD_ADDRs.
>>
>> Without the parent patch, the client is not able to establish new
>> subflows using its 'subflow' endpoints. The parent commit fixes that.
>>
>> The 'Fixes' tag here below is the same as the one from the previous
>> commit: this patch here is not fixing anything wrong in the
>> selftests,
>> but it validates the previous fix for an issue introduced by this
>> commit
>> ID.
>>
>> Fixes: df377be38725 ("mptcp: add deny_join_id0 in
>> mptcp_options_received")
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  net/mptcp/pm.c                                  |  4 +++-
>>  net/mptcp/pm_kernel.c                           |  1 +
>>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 +++++++++++
>>  3 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
>> index
>> fda8d1880224dedbbfb20954f93a04da435346df..5d037f9d4e0b4d158beaaa5a48c
>> 4efb50fe9b5cb 100644
>> --- a/net/mptcp/pm.c
>> +++ b/net/mptcp/pm.c
>> @@ -637,7 +637,9 @@ void mptcp_pm_add_addr_received(const struct sock
>> *ssk,
>>  		} else {
>>  			__MPTCP_INC_STATS(sock_net((struct sock
>> *)msk), MPTCP_MIB_ADDADDRDROP);
>>  		}
>> -	/* id0 should not have a different address ; special case
>> for C-flag */
>> +	/* - id0 should not have a different address
>> +	 * - special case for C-flag: linked to
>> fill_local_addresses_vec()
>> +	 */
> 
> How about squashing this into patch 1.
> 
>>  	} else if ((addr->id == 0 &&
>> !mptcp_pm_is_init_remote_addr(msk, addr)) ||
>>  		   (addr->id > 0 && !READ_ONCE(pm->accept_addr) &&
>>  		    (!READ_ONCE(msk->pm.remote_deny_join_id0) ||
>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>> index
>> 408801e20ed6e874b80ccbf38af76abc13615a3a..aae9b92aea4715dabaf3101a034
>> 1ab7eab16f407 100644
>> --- a/net/mptcp/pm_kernel.c
>> +++ b/net/mptcp/pm_kernel.c
>> @@ -394,6 +394,7 @@ static unsigned int
>> fill_local_addresses_vec(struct mptcp_sock *msk,
>>  
>>  	pernet = pm_nl_get_pernet_from_msk(msk);
>>  	subflows_max = mptcp_pm_get_subflows_max(msk);
>> +	/* linked to conditions in mptcp_pm_add_addr_received() */
> 
> And squash this too.

Arf, sorry, I missed that and squash these modifications in the wrong
commit. Yes, it is for patch 1/2.

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