Re: [PATCH mptcp-next 2/2] selftests: mptcp: join: validate extra bind cases

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

Thank you for the review!

On 14/10/2025 08:46, Geliang Tang wrote:
> It took me some time to understand this test.
Is there something I should add in the commit message to make this clearer?

> On Thu, 2025-10-09 at 19:33 +0200, Matthieu Baerts (NGI0) wrote:
>> By design, an MPTCP connection will not accept extra subflows where
>> no
>> MPTCP listening sockets can accept such requests.
>>
>> In other words, it means that if the 'server' listens on a specific
>> address / device, it cannot accept MP_JOIN sent to a different
>> address /
>> device. Except if there is another MPTCP listening socket accepting
>> them.
>>
>> This is what the new tests are validating:
>>
>>  - Forcing a bind on the main v4/v6 address, and checking that
>> MP_JOIN
>>    to announced addresses are not accepted.
>>
>>  - Also forcing a bind on the main v4/v6 address, but before, another
>>    listening socket is created to accept additional subflows. Note
>> that
>>    'mptcpize run nc -l' -- or something else only doing:
>> socket(MPTCP),
>>    bind(<IP>), listen(0) -- would be enough, but here mptcp_connect
>> is
>>    reused not to depend on another tool just for that.
>>
>>  - Same as the previous one, but using v6 link-local addresses: this
>> is
>>    a bit particular because it is required to specify the outgoing
>>    network interface when connecting to a link-local address
>> announced
>>    by the other peer. When using the routing rules, this doesn't work
>>    (the outgoing interface is not known) ; but it does work with a
>>    'laminar' endpoint having a specified interface.
>>
>> Note that extra small modifications are needed for these tests to
>> work:
>>
>>  - mptcp_connect's check_getpeername_connect() check should strip the
>>    specified interface when comparing addresses.
>>
>>  - With IPv6 link-local addresses, it is required to wait for them to
>>    be ready (no longer in 'tentative' mode) before using them,
>> otherwise
>>    the bind() will not be allowed.
>>
>> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/591
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  tools/testing/selftests/net/mptcp/mptcp_connect.c |  10 +-
>>  tools/testing/selftests/net/mptcp/mptcp_join.sh   | 153
>> +++++++++++++++++++++-
>>  2 files changed, 161 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c
>> b/tools/testing/selftests/net/mptcp/mptcp_connect.c
>> index b148cadb96d0..c030b08a7195 100644
>> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
>> @@ -1064,6 +1064,8 @@ static void check_getpeername_connect(int fd)
>>  	socklen_t salen = sizeof(ss);
>>  	char a[INET6_ADDRSTRLEN];
>>  	char b[INET6_ADDRSTRLEN];
>> +	const char *iface;
>> +	size_t len;
>>  
>>  	if (getpeername(fd, (struct sockaddr *)&ss, &salen) < 0) {
>>  		perror("getpeername");
>> @@ -1073,7 +1075,13 @@ static void check_getpeername_connect(int fd)
>>  	xgetnameinfo((struct sockaddr *)&ss, salen,
>>  		     a, sizeof(a), b, sizeof(b));
>>  
>> -	if (strcmp(cfg_host, a) || strcmp(cfg_port, b))
>> +	iface = strchr(cfg_host, '%');
>> +	if (iface)
>> +		len = iface - cfg_host;
>> +	else
>> +		len = strlen(cfg_host) + 1;
> 
> Why do we need to add 1 here? I tested it and it works without adding
> 1.

If I don't add 1, I will not include '\0' in the comparison with "a".

In other words, if you have:

  cfg_host = "abc";
  a = "abc123";
  len = strlen(cfg_host); /* = 3 */

Then strncmp(cfg_host, a, len) will return 0 because the 3 first chars
are "abc". With 4 chars, they are different: "abc\0" vs "abc1".

So it is important to take the delimiter into account, just in case one
is the prefix of the other one.

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