* [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests
@ 2025-10-09 17:33 Matthieu Baerts (NGI0)
2025-10-09 17:33 ` [PATCH mptcp-next 1/2] selftests: mptcp: join: do_transfer: reduce code dup Matthieu Baerts (NGI0)
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-10-09 17:33 UTC (permalink / raw)
To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)
A clean-up patch, followed by a few new quick tests validating cases
where the listener 'bind()' to a specific IP address.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (2):
selftests: mptcp: join: do_transfer: reduce code dup
selftests: mptcp: join: validate extra bind cases
tools/testing/selftests/net/mptcp/mptcp_connect.c | 10 +-
tools/testing/selftests/net/mptcp/mptcp_join.sh | 166 ++++++++++++++++++++--
2 files changed, 166 insertions(+), 10 deletions(-)
---
base-commit: 3afc295b5e6e698696cf1fd75db194db2c7f64cd
change-id: 20251008-bind-extra-listen-7335e35454d2
Best regards,
--
Matthieu Baerts (NGI0) <matttbe@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH mptcp-next 1/2] selftests: mptcp: join: do_transfer: reduce code dup
2025-10-09 17:33 [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests Matthieu Baerts (NGI0)
@ 2025-10-09 17:33 ` Matthieu Baerts (NGI0)
2025-10-10 10:19 ` Geliang Tang
2025-10-09 17:33 ` [PATCH mptcp-next 2/2] selftests: mptcp: join: validate extra bind cases Matthieu Baerts (NGI0)
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-10-09 17:33 UTC (permalink / raw)
To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)
The same extra long command is present twice, with one small different:
the var for the stdin file is different.
Use a new dedicated var in one command to avoid this code duplication.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
| 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
--git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index c90d8e8b95cb..c5169020a515 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -951,6 +951,7 @@ do_transfer()
local FAILING_LINKS=${FAILING_LINKS:-""}
local fastclose=${fastclose:-""}
local speed=${speed:-"fast"}
+ local in="${sin}"
port=$(get_port)
:> "$cout"
@@ -999,16 +1000,12 @@ do_transfer()
extra_srv_args="$extra_args $extra_srv_args"
if [ "$test_linkfail" -gt 1 ];then
- timeout ${timeout_test} \
- ip netns exec ${listener_ns} \
- ./mptcp_connect -t ${timeout_poll} -l -p $port -s ${srv_proto} \
- $extra_srv_args "::" < "$sinfail" > "$sout" &
- else
- timeout ${timeout_test} \
- ip netns exec ${listener_ns} \
- ./mptcp_connect -t ${timeout_poll} -l -p $port -s ${srv_proto} \
- $extra_srv_args "::" < "$sin" > "$sout" &
+ in="${sinfail}"
fi
+ timeout ${timeout_test} \
+ ip netns exec ${listener_ns} \
+ ./mptcp_connect -t ${timeout_poll} -l -p ${port} -s ${srv_proto} \
+ ${extra_srv_args} "::" < "${in}" > "${sout}" &
local spid=$!
mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
--
2.51.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH mptcp-next 2/2] selftests: mptcp: join: validate extra bind cases
2025-10-09 17:33 [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests Matthieu Baerts (NGI0)
2025-10-09 17:33 ` [PATCH mptcp-next 1/2] selftests: mptcp: join: do_transfer: reduce code dup Matthieu Baerts (NGI0)
@ 2025-10-09 17:33 ` Matthieu Baerts (NGI0)
2025-10-14 6:46 ` Geliang Tang
2025-10-09 18:48 ` [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests MPTCP CI
2025-10-14 10:43 ` Matthieu Baerts
3 siblings, 1 reply; 9+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-10-09 17:33 UTC (permalink / raw)
To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)
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>
---
| 10 +-
| 153 +++++++++++++++++++++-
2 files changed, 161 insertions(+), 2 deletions(-)
--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;
+
+ if (strncmp(cfg_host, a, len) || strcmp(cfg_port, b))
fprintf(stderr, "%s: %s vs %s, %s vs %s\n", __func__,
cfg_host, a, cfg_port, b);
}
--git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index c5169020a515..e323f81cdc02 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -62,6 +62,7 @@ unset sflags
unset fastclose
unset fullmesh
unset speed
+unset bind_addr
unset join_syn_rej
unset join_csum_ns1
unset join_csum_ns2
@@ -645,6 +646,27 @@ wait_mpj()
done
}
+wait_ll_ready()
+{
+ local ns="${1}"
+
+ local i
+ for i in $(seq 50); do
+ ip -n "${ns}" -6 addr show scope link | grep "inet6 fe80" |
+ grep -qw "tentative" || break
+ sleep 0.1
+ done
+}
+
+get_ll_addr()
+{
+ local ns="${1}"
+ local iface="${2}"
+
+ ip -n "${ns}" -6 addr show dev "${iface}" scope link |
+ grep "inet6 fe80" | sed 's#.*\(fe80::.*\)/.*#\1#'
+}
+
kill_events_pids()
{
mptcp_lib_kill_wait $evts_ns1_pid
@@ -952,6 +974,7 @@ do_transfer()
local fastclose=${fastclose:-""}
local speed=${speed:-"fast"}
local in="${sin}"
+ local bind_addr=${bind_addr:-"::"}
port=$(get_port)
:> "$cout"
@@ -1005,7 +1028,7 @@ do_transfer()
timeout ${timeout_test} \
ip netns exec ${listener_ns} \
./mptcp_connect -t ${timeout_poll} -l -p ${port} -s ${srv_proto} \
- ${extra_srv_args} "::" < "${in}" > "${sout}" &
+ ${extra_srv_args} "${bind_addr}" < "${in}" > "${sout}" &
local spid=$!
mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
@@ -3230,6 +3253,133 @@ add_addr_ports_tests()
fi
}
+bind_tests()
+{
+ # bind to one address should not allow extra subflows to other addresses
+ if reset "bind main address v4, no join v4"; then
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 2 2
+ pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
+ bind_addr="10.0.1.1" \
+ run_tests $ns1 $ns2 10.0.1.1
+ join_syn_tx=1 \
+ chk_join_nr 0 0 0
+ chk_add_nr 1 1
+ fi
+
+ # bind to one address should not allow extra subflows to other addresses
+ if reset "bind main address v6, no join v6"; then
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 2 2
+ pm_nl_add_endpoint $ns1 dead:beef:2::1 flags signal
+ bind_addr="dead:beef:1::1" \
+ run_tests $ns1 $ns2 dead:beef:1::1
+ join_syn_tx=1 \
+ chk_join_nr 0 0 0
+ chk_add_nr 1 1
+ fi
+
+ # multiple binds to allow extra subflows to other addresses
+ if reset "multiple bind to allow joins v4"; then
+ local extra_bind
+
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 2 2
+ pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
+
+ # Launching another app listening on a different address
+ # Note: it could be a totally different app, e.g. nc, socat, ...
+ ip netns exec ${ns1} ./mptcp_connect -l -p "$(get_port)" \
+ -s MPTCP 10.0.2.1 &
+ extra_bind=$!
+
+ bind_addr="10.0.1.1" \
+ run_tests $ns1 $ns2 10.0.1.1
+ chk_join_nr 1 1 1
+ chk_add_nr 1 1
+
+ kill ${extra_bind}
+ fi
+
+ # multiple binds to allow extra subflows to other addresses
+ if reset "multiple bind to allow joins v6"; then
+ local extra_bind
+
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 2 2
+ pm_nl_add_endpoint $ns1 dead:beef:2::1 flags signal
+
+ # Launching another app listening on a different address
+ # Note: it could be a totally different app, e.g. nc, socat, ...
+ ip netns exec ${ns1} ./mptcp_connect -l -p "$(get_port)" \
+ -s MPTCP dead:beef:2::1 &
+ extra_bind=$!
+
+ bind_addr="dead:beef:1::1" \
+ run_tests $ns1 $ns2 dead:beef:1::1
+ chk_join_nr 1 1 1
+ chk_add_nr 1 1
+
+ kill ${extra_bind}
+ fi
+
+ # multiple binds to allow extra subflows to other addresses: v6 LL case
+ if reset "multiple bind to allow joins v6 link-local routing"; then
+ local extra_bind ns1ll1 ns1ll2
+
+ ns1ll1="$(get_ll_addr $ns1 ns1eth1)"
+ ns1ll2="$(get_ll_addr $ns1 ns1eth2)"
+
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 2 2
+ pm_nl_add_endpoint $ns1 "${ns1ll2}" flags signal
+
+ wait_ll_ready $ns1 # to be able to bind
+ wait_ll_ready $ns2 # also needed to bind on the client side
+ ip netns exec ${ns1} ./mptcp_connect -l -p "$(get_port)" \
+ -s MPTCP "${ns1ll2}%ns1eth2" &
+ extra_bind=$!
+
+ bind_addr="${ns1ll1}%ns1eth1" \
+ run_tests $ns1 $ns2 "${ns1ll1}%ns2eth1"
+ # it is not possible to connect to the announced LL addr without
+ # specifying the outgoing interface.
+ join_connect_err=1 \
+ chk_join_nr 0 0 0
+ chk_add_nr 1 1
+
+ kill ${extra_bind}
+ fi
+
+ # multiple binds to allow extra subflows to v6 LL addresses: laminar
+ if reset "multiple bind to allow joins v6 link-local laminar" &&
+ continue_if mptcp_lib_kallsyms_has "mptcp_pm_get_endp_laminar_max$"; then
+ local extra_bind ns1ll1 ns1ll2 ns2ll2
+
+ ns1ll1="$(get_ll_addr $ns1 ns1eth1)"
+ ns1ll2="$(get_ll_addr $ns1 ns1eth2)"
+ ns2ll2="$(get_ll_addr $ns2 ns2eth2)"
+
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 2 2
+ pm_nl_add_endpoint $ns1 "${ns1ll2}" flags signal
+ pm_nl_add_endpoint $ns2 "${ns2ll2}" flags laminar dev ns2eth2
+
+ wait_ll_ready $ns1 # to be able to bind
+ wait_ll_ready $ns2 # also needed to bind on the client side
+ ip netns exec ${ns1} ./mptcp_connect -l -p "$(get_port)" \
+ -s MPTCP "${ns1ll2}%ns1eth2" &
+ extra_bind=$!
+
+ bind_addr="${ns1ll1}%ns1eth1" \
+ run_tests $ns1 $ns2 "${ns1ll1}%ns2eth1"
+ chk_join_nr 1 1 1
+ chk_add_nr 1 1
+
+ kill ${extra_bind}
+ fi
+}
+
syncookies_tests()
{
# single subflow, syncookies
@@ -4184,6 +4334,7 @@ all_tests_sorted=(
M@mixed_tests
b@backup_tests
p@add_addr_ports_tests
+ B@bind_tests
k@syncookies_tests
S@checksum_tests
d@deny_join_id0_tests
--
2.51.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests
2025-10-09 17:33 [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests Matthieu Baerts (NGI0)
2025-10-09 17:33 ` [PATCH mptcp-next 1/2] selftests: mptcp: join: do_transfer: reduce code dup Matthieu Baerts (NGI0)
2025-10-09 17:33 ` [PATCH mptcp-next 2/2] selftests: mptcp: join: validate extra bind cases Matthieu Baerts (NGI0)
@ 2025-10-09 18:48 ` MPTCP CI
2025-10-14 10:43 ` Matthieu Baerts
3 siblings, 0 replies; 9+ messages in thread
From: MPTCP CI @ 2025-10-09 18:48 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp
Hi Matthieu,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal (except selftest_mptcp_join): Unstable: 1 failed test(s): selftest_simult_flows 🔴
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/18384474239
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/cb24879849d8
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1009827
If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:
$ cd [kernel source code]
$ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
--pull always mptcp/mptcp-upstream-virtme-docker:latest \
auto-normal
For more details:
https://github.com/multipath-tcp/mptcp-upstream-virtme-docker
Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)
Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next 1/2] selftests: mptcp: join: do_transfer: reduce code dup
2025-10-09 17:33 ` [PATCH mptcp-next 1/2] selftests: mptcp: join: do_transfer: reduce code dup Matthieu Baerts (NGI0)
@ 2025-10-10 10:19 ` Geliang Tang
0 siblings, 0 replies; 9+ messages in thread
From: Geliang Tang @ 2025-10-10 10:19 UTC (permalink / raw)
To: Matthieu Baerts (NGI0), MPTCP Upstream
Hi Matt,
On Thu, 2025-10-09 at 19:33 +0200, Matthieu Baerts (NGI0) wrote:
> The same extra long command is present twice, with one small
> different:
> the var for the stdin file is different.
>
> Use a new dedicated var in one command to avoid this code
> duplication.
>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
LGTM!
Reviewed-by: Geliang Tang <geliang@kernel.org>
Thanks,
-Geliang
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index c90d8e8b95cb..c5169020a515 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -951,6 +951,7 @@ do_transfer()
> local FAILING_LINKS=${FAILING_LINKS:-""}
> local fastclose=${fastclose:-""}
> local speed=${speed:-"fast"}
> + local in="${sin}"
> port=$(get_port)
>
> :> "$cout"
> @@ -999,16 +1000,12 @@ do_transfer()
>
> extra_srv_args="$extra_args $extra_srv_args"
> if [ "$test_linkfail" -gt 1 ];then
> - timeout ${timeout_test} \
> - ip netns exec ${listener_ns} \
> - ./mptcp_connect -t ${timeout_poll} -
> l -p $port -s ${srv_proto} \
> - $extra_srv_args "::" <
> "$sinfail" > "$sout" &
> - else
> - timeout ${timeout_test} \
> - ip netns exec ${listener_ns} \
> - ./mptcp_connect -t ${timeout_poll} -
> l -p $port -s ${srv_proto} \
> - $extra_srv_args "::" <
> "$sin" > "$sout" &
> + in="${sinfail}"
> fi
> + timeout ${timeout_test} \
> + ip netns exec ${listener_ns} \
> + ./mptcp_connect -t ${timeout_poll} -l -p
> ${port} -s ${srv_proto} \
> + ${extra_srv_args} "::" < "${in}" >
> "${sout}" &
> local spid=$!
>
> mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next 2/2] selftests: mptcp: join: validate extra bind cases
2025-10-09 17:33 ` [PATCH mptcp-next 2/2] selftests: mptcp: join: validate extra bind cases Matthieu Baerts (NGI0)
@ 2025-10-14 6:46 ` Geliang Tang
2025-10-14 9:57 ` Matthieu Baerts
0 siblings, 1 reply; 9+ messages in thread
From: Geliang Tang @ 2025-10-14 6:46 UTC (permalink / raw)
To: Matthieu Baerts (NGI0), MPTCP Upstream
Hi Matt,
It took me some time to understand this test.
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.
Other than this minor comment, everything looks good to me. No need for
a v2; it can be modified when applying if needed.
Reviewed-by: Geliang Tang <geliang@kernel.org>
Thanks,
-Geliang
> +
> + if (strncmp(cfg_host, a, len) || strcmp(cfg_port, b))
> fprintf(stderr, "%s: %s vs %s, %s vs %s\n",
> __func__,
> cfg_host, a, cfg_port, b);
> }
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index c5169020a515..e323f81cdc02 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -62,6 +62,7 @@ unset sflags
> unset fastclose
> unset fullmesh
> unset speed
> +unset bind_addr
> unset join_syn_rej
> unset join_csum_ns1
> unset join_csum_ns2
> @@ -645,6 +646,27 @@ wait_mpj()
> done
> }
>
> +wait_ll_ready()
> +{
> + local ns="${1}"
> +
> + local i
> + for i in $(seq 50); do
> + ip -n "${ns}" -6 addr show scope link | grep "inet6
> fe80" |
> + grep -qw "tentative" || break
> + sleep 0.1
> + done
> +}
> +
> +get_ll_addr()
> +{
> + local ns="${1}"
> + local iface="${2}"
> +
> + ip -n "${ns}" -6 addr show dev "${iface}" scope link |
> + grep "inet6 fe80" | sed 's#.*\(fe80::.*\)/.*#\1#'
> +}
> +
> kill_events_pids()
> {
> mptcp_lib_kill_wait $evts_ns1_pid
> @@ -952,6 +974,7 @@ do_transfer()
> local fastclose=${fastclose:-""}
> local speed=${speed:-"fast"}
> local in="${sin}"
> + local bind_addr=${bind_addr:-"::"}
> port=$(get_port)
>
> :> "$cout"
> @@ -1005,7 +1028,7 @@ do_transfer()
> timeout ${timeout_test} \
> ip netns exec ${listener_ns} \
> ./mptcp_connect -t ${timeout_poll} -l -p
> ${port} -s ${srv_proto} \
> - ${extra_srv_args} "::" < "${in}" >
> "${sout}" &
> + ${extra_srv_args} "${bind_addr}" <
> "${in}" > "${sout}" &
> local spid=$!
>
> mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
> @@ -3230,6 +3253,133 @@ add_addr_ports_tests()
> fi
> }
>
> +bind_tests()
> +{
> + # bind to one address should not allow extra subflows to
> other addresses
> + if reset "bind main address v4, no join v4"; then
> + pm_nl_set_limits $ns1 0 2
> + pm_nl_set_limits $ns2 2 2
> + pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
> + bind_addr="10.0.1.1" \
> + run_tests $ns1 $ns2 10.0.1.1
> + join_syn_tx=1 \
> + chk_join_nr 0 0 0
> + chk_add_nr 1 1
> + fi
> +
> + # bind to one address should not allow extra subflows to
> other addresses
> + if reset "bind main address v6, no join v6"; then
> + pm_nl_set_limits $ns1 0 2
> + pm_nl_set_limits $ns2 2 2
> + pm_nl_add_endpoint $ns1 dead:beef:2::1 flags signal
> + bind_addr="dead:beef:1::1" \
> + run_tests $ns1 $ns2 dead:beef:1::1
> + join_syn_tx=1 \
> + chk_join_nr 0 0 0
> + chk_add_nr 1 1
> + fi
> +
> + # multiple binds to allow extra subflows to other addresses
> + if reset "multiple bind to allow joins v4"; then
> + local extra_bind
> +
> + pm_nl_set_limits $ns1 0 2
> + pm_nl_set_limits $ns2 2 2
> + pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
> +
> + # Launching another app listening on a different
> address
> + # Note: it could be a totally different app, e.g.
> nc, socat, ...
> + ip netns exec ${ns1} ./mptcp_connect -l -p
> "$(get_port)" \
> + -s MPTCP 10.0.2.1 &
> + extra_bind=$!
> +
> + bind_addr="10.0.1.1" \
> + run_tests $ns1 $ns2 10.0.1.1
> + chk_join_nr 1 1 1
> + chk_add_nr 1 1
> +
> + kill ${extra_bind}
> + fi
> +
> + # multiple binds to allow extra subflows to other addresses
> + if reset "multiple bind to allow joins v6"; then
> + local extra_bind
> +
> + pm_nl_set_limits $ns1 0 2
> + pm_nl_set_limits $ns2 2 2
> + pm_nl_add_endpoint $ns1 dead:beef:2::1 flags signal
> +
> + # Launching another app listening on a different
> address
> + # Note: it could be a totally different app, e.g.
> nc, socat, ...
> + ip netns exec ${ns1} ./mptcp_connect -l -p
> "$(get_port)" \
> + -s MPTCP dead:beef:2::1 &
> + extra_bind=$!
> +
> + bind_addr="dead:beef:1::1" \
> + run_tests $ns1 $ns2 dead:beef:1::1
> + chk_join_nr 1 1 1
> + chk_add_nr 1 1
> +
> + kill ${extra_bind}
> + fi
> +
> + # multiple binds to allow extra subflows to other addresses:
> v6 LL case
> + if reset "multiple bind to allow joins v6 link-local
> routing"; then
> + local extra_bind ns1ll1 ns1ll2
> +
> + ns1ll1="$(get_ll_addr $ns1 ns1eth1)"
> + ns1ll2="$(get_ll_addr $ns1 ns1eth2)"
> +
> + pm_nl_set_limits $ns1 0 2
> + pm_nl_set_limits $ns2 2 2
> + pm_nl_add_endpoint $ns1 "${ns1ll2}" flags signal
> +
> + wait_ll_ready $ns1 # to be able to bind
> + wait_ll_ready $ns2 # also needed to bind on the
> client side
> + ip netns exec ${ns1} ./mptcp_connect -l -p
> "$(get_port)" \
> + -s MPTCP "${ns1ll2}%ns1eth2" &
> + extra_bind=$!
> +
> + bind_addr="${ns1ll1}%ns1eth1" \
> + run_tests $ns1 $ns2 "${ns1ll1}%ns2eth1"
> + # it is not possible to connect to the announced LL
> addr without
> + # specifying the outgoing interface.
> + join_connect_err=1 \
> + chk_join_nr 0 0 0
> + chk_add_nr 1 1
> +
> + kill ${extra_bind}
> + fi
> +
> + # multiple binds to allow extra subflows to v6 LL addresses:
> laminar
> + if reset "multiple bind to allow joins v6 link-local
> laminar" &&
> + continue_if mptcp_lib_kallsyms_has
> "mptcp_pm_get_endp_laminar_max$"; then
> + local extra_bind ns1ll1 ns1ll2 ns2ll2
> +
> + ns1ll1="$(get_ll_addr $ns1 ns1eth1)"
> + ns1ll2="$(get_ll_addr $ns1 ns1eth2)"
> + ns2ll2="$(get_ll_addr $ns2 ns2eth2)"
> +
> + pm_nl_set_limits $ns1 0 2
> + pm_nl_set_limits $ns2 2 2
> + pm_nl_add_endpoint $ns1 "${ns1ll2}" flags signal
> + pm_nl_add_endpoint $ns2 "${ns2ll2}" flags laminar
> dev ns2eth2
> +
> + wait_ll_ready $ns1 # to be able to bind
> + wait_ll_ready $ns2 # also needed to bind on the
> client side
> + ip netns exec ${ns1} ./mptcp_connect -l -p
> "$(get_port)" \
> + -s MPTCP "${ns1ll2}%ns1eth2" &
> + extra_bind=$!
> +
> + bind_addr="${ns1ll1}%ns1eth1" \
> + run_tests $ns1 $ns2 "${ns1ll1}%ns2eth1"
> + chk_join_nr 1 1 1
> + chk_add_nr 1 1
> +
> + kill ${extra_bind}
> + fi
> +}
> +
> syncookies_tests()
> {
> # single subflow, syncookies
> @@ -4184,6 +4334,7 @@ all_tests_sorted=(
> M@mixed_tests
> b@backup_tests
> p@add_addr_ports_tests
> + B@bind_tests
> k@syncookies_tests
> S@checksum_tests
> d@deny_join_id0_tests
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next 2/2] selftests: mptcp: join: validate extra bind cases
2025-10-14 6:46 ` Geliang Tang
@ 2025-10-14 9:57 ` Matthieu Baerts
2025-10-14 10:14 ` Geliang Tang
0 siblings, 1 reply; 9+ messages in thread
From: Matthieu Baerts @ 2025-10-14 9:57 UTC (permalink / raw)
To: Geliang Tang, MPTCP Upstream
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next 2/2] selftests: mptcp: join: validate extra bind cases
2025-10-14 9:57 ` Matthieu Baerts
@ 2025-10-14 10:14 ` Geliang Tang
0 siblings, 0 replies; 9+ messages in thread
From: Geliang Tang @ 2025-10-14 10:14 UTC (permalink / raw)
To: Matthieu Baerts, MPTCP Upstream
Hi Matt,
On Tue, 2025-10-14 at 11:57 +0200, Matthieu Baerts wrote:
> 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.
Sure, thanks for your explanation. I have no other comments, let's
apply this set.
Thanks,
-Geliang
>
> Cheers,
> Matt
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests
2025-10-09 17:33 [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests Matthieu Baerts (NGI0)
` (2 preceding siblings ...)
2025-10-09 18:48 ` [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests MPTCP CI
@ 2025-10-14 10:43 ` Matthieu Baerts
3 siblings, 0 replies; 9+ messages in thread
From: Matthieu Baerts @ 2025-10-14 10:43 UTC (permalink / raw)
To: Geliang Tang; +Cc: MPTCP Linux
Hi Geliang,
On 09/10/2025 19:33, Matthieu Baerts (NGI0) wrote:
> A clean-up patch, followed by a few new quick tests validating cases
> where the listener 'bind()' to a specific IP address.
Thank you for the review!
Now in our tree (feat. for net-next):
New patches for t/upstream:
- f421ad2aa981: selftests: mptcp: join: do_transfer: reduce code dup
- 4fd0cfa3ab1a: selftests: mptcp: join: validate extra bind cases
- Results: fe9c3cb19630..f6f85b548772 (export)
Tests are now in progress:
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/56d043498eb548955160044d50b127c4a2572c13/checks
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-10-14 10:44 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-09 17:33 [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests Matthieu Baerts (NGI0)
2025-10-09 17:33 ` [PATCH mptcp-next 1/2] selftests: mptcp: join: do_transfer: reduce code dup Matthieu Baerts (NGI0)
2025-10-10 10:19 ` Geliang Tang
2025-10-09 17:33 ` [PATCH mptcp-next 2/2] selftests: mptcp: join: validate extra bind cases Matthieu Baerts (NGI0)
2025-10-14 6:46 ` Geliang Tang
2025-10-14 9:57 ` Matthieu Baerts
2025-10-14 10:14 ` Geliang Tang
2025-10-09 18:48 ` [PATCH mptcp-next 0/2] selftests: mptcp: join: code dup + new 'bind' tests MPTCP CI
2025-10-14 10:43 ` Matthieu Baerts
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.