* [PATCH mptcp-net v5 01/12] mptcp: explicitly specify sock family at subflow creation time
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-net v5 02/12] mptcp: netlink: respect v4/v6-only sockets Matthieu Baerts
` (11 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Matthieu Baerts
From: Paolo Abeni <pabeni@redhat.com>
Let the caller specify the to-be-created subflow family.
For a given MPTCP socket created with the AF_INET6 family, the current
userspace PM can already ask the kernel to create subflows in v4 and v6.
If "plain" IPv4 addresses are passed to the kernel, they are
automatically mapped in v6 addresses "by accident". This can be
problematic because the userspace will need to pass different addresses,
now the v4-mapped-v6 addresses to destroy this new subflow.
On the other hand, if the MPTCP socket has been created with the AF_INET
family, the command to create a subflow in v6 will be accepted but the
result will not be the one as expected as new subflow will be created in
IPv4 using part of the v6 addresses passed to the kernel: not creating
the expected subflow then.
No functional change intended for the in-kernel PM where an explicit
enforcement is currently in place. This arbitrary enforcement will be
leveraged by other patches in a future version.
Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
Co-developed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Notes:
v2->v3:
- Update the commit message because the behaviour is modified for the
userspace PM and needed not to create subflows from/to wrong IP
addresses.
net/mptcp/protocol.c | 2 +-
net/mptcp/protocol.h | 3 ++-
net/mptcp/subflow.c | 9 +++++----
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index aaafdae73b6a..003b44a79fce 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -98,7 +98,7 @@ static int __mptcp_socket_create(struct mptcp_sock *msk)
struct socket *ssock;
int err;
- err = mptcp_subflow_create_socket(sk, &ssock);
+ err = mptcp_subflow_create_socket(sk, sk->sk_family, &ssock);
if (err)
return err;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 5b92906cb91f..b2b56a80e817 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -643,7 +643,8 @@ bool mptcp_addresses_equal(const struct mptcp_addr_info *a,
/* called with sk socket lock held */
int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
const struct mptcp_addr_info *remote);
-int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock);
+int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
+ struct socket **new_sock);
void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
struct sockaddr_storage *addr,
unsigned short family);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index bd387d4b5a38..ec54413fb31f 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1547,7 +1547,7 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
if (!mptcp_is_fully_established(sk))
goto err_out;
- err = mptcp_subflow_create_socket(sk, &sf);
+ err = mptcp_subflow_create_socket(sk, loc->family, &sf);
if (err)
goto err_out;
@@ -1660,7 +1660,9 @@ static void mptcp_subflow_ops_undo_override(struct sock *ssk)
#endif
ssk->sk_prot = &tcp_prot;
}
-int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
+
+int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
+ struct socket **new_sock)
{
struct mptcp_subflow_context *subflow;
struct net *net = sock_net(sk);
@@ -1673,8 +1675,7 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
if (unlikely(!sk->sk_socket))
return -EINVAL;
- err = sock_create_kern(net, sk->sk_family, SOCK_STREAM, IPPROTO_TCP,
- &sf);
+ err = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP, &sf);
if (err)
return err;
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH mptcp-net v5 02/12] mptcp: netlink: respect v4/v6-only sockets
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-net v5 01/12] mptcp: explicitly specify sock family at subflow creation time Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-net v5 03/12] selftests: mptcp: userspace: validate v4-v6 subflows mix Matthieu Baerts
` (10 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts
If an MPTCP socket has been created with AF_INET6 and the IPV6_V6ONLY
option has been set, the userspace PM would allow creating subflows
using IPv4 addresses, e.g. mapped in v6.
The kernel side of userspace PM will also accept creating subflows with
local and remote addresses having different families. Depending on the
subflow socket's family, different behaviours are expected:
- If AF_INET is forced with a v6 address, the kernel will take the last
byte of the IP and try to connect to that: a new subflow is created
but to a non expected address.
- If AF_INET6 is forced with a v4 address, the kernel will try to
connect to a v4 address (v4-mapped-v6). A -EBADF error from the
connect() part is then expected.
It is then required to check the given families can be accepted. This is
done by using a new helper for addresses family matching, taking care of
IPv4 vs IPv4-mapped-IPv6 addresses. This helper will be re-used later by
the in-kernel path-manager to use mixed IPv4 and IPv6 addresses.
While at it, a clear error message is now reported if there are some
conflicts with the families that have been passed by the userspace.
Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
Notes:
v4 -> v5:
- fix typo (s/&&/==/) in mptcp_pm_addr_families_match() without IPv6
support (Mat)
- more details about the unexpected behaviour in the commit msg (Mat)
net/mptcp/pm.c | 25 +++++++++++++++++++++++++
net/mptcp/pm_userspace.c | 7 +++++++
net/mptcp/protocol.h | 3 +++
3 files changed, 35 insertions(+)
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index cdeb7280ac76..8e0cf6275e94 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -413,6 +413,31 @@ void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
}
}
+/* if sk is ipv4 or ipv6_only allows only same-family local and remote addresses,
+ * otherwise allow any matching local/remote pair
+ */
+bool mptcp_pm_addr_families_match(const struct sock *sk,
+ const struct mptcp_addr_info *loc,
+ const struct mptcp_addr_info *rem)
+{
+ bool mptcp_is_v4 = sk->sk_family == AF_INET;
+
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+ bool loc_is_v4 = loc->family == AF_INET || ipv6_addr_v4mapped(&loc->addr6);
+ bool rem_is_v4 = rem->family == AF_INET || ipv6_addr_v4mapped(&rem->addr6);
+
+ if (mptcp_is_v4)
+ return loc_is_v4 && rem_is_v4;
+
+ if (ipv6_only_sock(sk))
+ return !loc_is_v4 && !rem_is_v4;
+
+ return loc_is_v4 == rem_is_v4;
+#else
+ return mptcp_is_v4 && loc->family == AF_INET && rem->family == AF_INET;
+#endif
+}
+
void mptcp_pm_data_reset(struct mptcp_sock *msk)
{
u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 65dcc55a8ad8..ea6ad9da7493 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -294,6 +294,13 @@ int mptcp_nl_cmd_sf_create(struct sk_buff *skb, struct genl_info *info)
}
sk = (struct sock *)msk;
+
+ if (!mptcp_pm_addr_families_match(sk, &addr_l, &addr_r)) {
+ GENL_SET_ERR_MSG(info, "families mismatch");
+ err = -EINVAL;
+ goto create_err;
+ }
+
lock_sock(sk);
err = __mptcp_subflow_connect(sk, &addr_l, &addr_r);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index b2b56a80e817..871ec3e93314 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -793,6 +793,9 @@ int mptcp_pm_parse_addr(struct nlattr *attr, struct genl_info *info,
int mptcp_pm_parse_entry(struct nlattr *attr, struct genl_info *info,
bool require_family,
struct mptcp_pm_addr_entry *entry);
+bool mptcp_pm_addr_families_match(const struct sock *sk,
+ const struct mptcp_addr_info *loc,
+ const struct mptcp_addr_info *rem);
void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk);
void mptcp_pm_nl_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk);
void mptcp_pm_new_connection(struct mptcp_sock *msk, const struct sock *ssk, int server_side);
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH mptcp-net v5 03/12] selftests: mptcp: userspace: validate v4-v6 subflows mix
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-net v5 01/12] mptcp: explicitly specify sock family at subflow creation time Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-net v5 02/12] mptcp: netlink: respect v4/v6-only sockets Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-next v5 04/12] mptcp: let the in-kernel PM use mixed IPv4 and IPv6 addresses Matthieu Baerts
` (9 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts
MPTCP protocol supports having subflows in both IPv4 and IPv6. In Linux,
it is possible to have that if the MPTCP socket has been created with
AF_INET6 family without the IPV6_V6ONLY option.
Here, a new IPv4 subflow is being added to the initial IPv6 connection,
then being removed using Netlink commands.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
.../selftests/net/mptcp/userspace_pm.sh | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index a29deb9fa024..ab2d581f28a1 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -752,6 +752,52 @@ test_subflows()
"$server4_token" > /dev/null 2>&1
}
+test_subflows_v4_v6_mix()
+{
+ # Attempt to add a listener at 10.0.2.1:<subflow-port>
+ ip netns exec "$ns1" ./pm_nl_ctl listen 10.0.2.1\
+ $app6_port > /dev/null 2>&1 &
+ local listener_pid=$!
+
+ # ADD_ADDR4 from server to client machine reusing the subflow port on
+ # the established v6 connection
+ :>"$client_evts"
+ ip netns exec "$ns1" ./pm_nl_ctl ann 10.0.2.1 token "$server6_token" id\
+ $server_addr_id dev ns1eth2 > /dev/null 2>&1
+ stdbuf -o0 -e0 printf "ADD_ADDR4 id:%d 10.0.2.1 (ns1) => ns2, reuse port\t\t" $server_addr_id
+ sleep 0.5
+ verify_announce_event "$client_evts" "$ANNOUNCED" "$client6_token" "10.0.2.1"\
+ "$server_addr_id" "$app6_port"
+
+ # CREATE_SUBFLOW from client to server machine
+ :>"$client_evts"
+ ip netns exec "$ns2" ./pm_nl_ctl csf lip 10.0.2.2 lid 23 rip 10.0.2.1 rport\
+ $app6_port token "$client6_token" > /dev/null 2>&1
+ sleep 0.5
+ verify_subflow_events "$client_evts" "$SUB_ESTABLISHED" "$client6_token"\
+ "$AF_INET" "10.0.2.2" "10.0.2.1" "$app6_port" "23"\
+ "$server_addr_id" "ns2" "ns1"
+
+ # Delete the listener from the server ns, if one was created
+ kill_wait $listener_pid
+
+ sport=$(sed --unbuffered -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q' "$client_evts")
+
+ # DESTROY_SUBFLOW from client to server machine
+ :>"$client_evts"
+ ip netns exec "$ns2" ./pm_nl_ctl dsf lip 10.0.2.2 lport "$sport" rip 10.0.2.1 rport\
+ $app6_port token "$client6_token" > /dev/null 2>&1
+ sleep 0.5
+ verify_subflow_events "$client_evts" "$SUB_CLOSED" "$client6_token" \
+ "$AF_INET" "10.0.2.2" "10.0.2.1" "$app6_port" "23"\
+ "$server_addr_id" "ns2" "ns1"
+
+ # RM_ADDR from server to client machine
+ ip netns exec "$ns1" ./pm_nl_ctl rem id $server_addr_id token\
+ "$server6_token" > /dev/null 2>&1
+ sleep 0.5
+}
+
test_prio()
{
local count
@@ -861,6 +907,7 @@ make_connection "v6"
test_announce
test_remove
test_subflows
+test_subflows_v4_v6_mix
test_prio
test_listener
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH mptcp-next v5 04/12] mptcp: let the in-kernel PM use mixed IPv4 and IPv6 addresses
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (2 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-net v5 03/12] selftests: mptcp: userspace: validate v4-v6 subflows mix Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-next v5 05/12] mptcp: propagate sk_ipv6only to subflows Matthieu Baerts
` (8 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Matthieu Baerts
From: Paolo Abeni <pabeni@redhat.com>
Currently the in-kernel PM arbitrary enforces that created subflow's
family must match the main MPTCP socket while the RFC allows mixing
IPv4 and IPv6 subflows.
This patch changes the in-kernel PM logic to create subflows matching
the currently selected source (or destination) address. IPv4 sockets
can pick only IPv4 addresses (and v4 mapped in v6), while IPv6 sockets
not restricted to V6ONLY can pick either IPv4 and IPv6 addresses as
long as the source and destination matches.
A helper, previously introduced is used to ease family matching checks,
taking care of IPv4 vs IPv4-mapped-IPv6 vs IPv6 only addresses.
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/269
Co-developed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Notes:
v3->v4:
- fix compilation error without CONFIG_MPTCP_IPV6
v2->v3:
- the helper has been moved to an earlier commit ("mptcp: netlink:
respect v4/v6-only sockets") and to pm.c because needed to fix a bug.
- the helper also takes into account different cases of IPv4 addresses
mapped in v6 and has been renamed.
- fill_local_addresses_vec() now considers v4-mapped-v6 as AF_INET not
to change the behaviour and break selftests.
- add_addr_accepted is not incremented if the ADD_ADDR cannot be used
(e.g. v4/v6-only sockets)
- if no subflows need to be created, the PM lock is no longer
manipulated.
net/mptcp/pm_netlink.c | 58 ++++++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 27 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index f2a43e13bacd..e82a112b8779 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -152,7 +152,6 @@ static struct mptcp_pm_addr_entry *
select_local_address(const struct pm_nl_pernet *pernet,
const struct mptcp_sock *msk)
{
- const struct sock *sk = (const struct sock *)msk;
struct mptcp_pm_addr_entry *entry, *ret = NULL;
msk_owned_by_me(msk);
@@ -165,16 +164,6 @@ select_local_address(const struct pm_nl_pernet *pernet,
if (!test_bit(entry->addr.id, msk->pm.id_avail_bitmap))
continue;
- if (entry->addr.family != sk->sk_family) {
-#if IS_ENABLED(CONFIG_MPTCP_IPV6)
- if ((entry->addr.family == AF_INET &&
- !ipv6_addr_v4mapped(&sk->sk_v6_daddr)) ||
- (sk->sk_family == AF_INET &&
- !ipv6_addr_v4mapped(&entry->addr.addr6)))
-#endif
- continue;
- }
-
ret = entry;
break;
}
@@ -423,7 +412,9 @@ static bool lookup_address_in_vec(const struct mptcp_addr_info *addrs, unsigned
/* Fill all the remote addresses into the array addrs[],
* and return the array size.
*/
-static unsigned int fill_remote_addresses_vec(struct mptcp_sock *msk, bool fullmesh,
+static unsigned int fill_remote_addresses_vec(struct mptcp_sock *msk,
+ struct mptcp_addr_info *local,
+ bool fullmesh,
struct mptcp_addr_info *addrs)
{
bool deny_id0 = READ_ONCE(msk->pm.remote_deny_join_id0);
@@ -443,6 +434,9 @@ static unsigned int fill_remote_addresses_vec(struct mptcp_sock *msk, bool fullm
if (deny_id0)
return 0;
+ if (!mptcp_pm_addr_families_match(sk, local, &remote))
+ return 0;
+
msk->pm.subflows++;
addrs[i++] = remote;
} else {
@@ -453,6 +447,9 @@ static unsigned int fill_remote_addresses_vec(struct mptcp_sock *msk, bool fullm
if (deny_id0 && !addrs[i].id)
continue;
+ if (!mptcp_pm_addr_families_match(sk, local, &addrs[i]))
+ continue;
+
if (!lookup_address_in_vec(addrs, i, &addrs[i]) &&
msk->pm.subflows < subflows_max) {
msk->pm.subflows++;
@@ -600,9 +597,11 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
fullmesh = !!(local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH);
msk->pm.local_addr_used++;
- nr = fill_remote_addresses_vec(msk, fullmesh, addrs);
- if (nr)
- __clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
+ __clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
+ nr = fill_remote_addresses_vec(msk, &local->addr, fullmesh, addrs);
+ if (nr == 0)
+ continue;
+
spin_unlock_bh(&msk->pm.lock);
for (i = 0; i < nr; i++)
__mptcp_subflow_connect(sk, &local->addr, &addrs[i]);
@@ -625,11 +624,11 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk)
* and return the array size.
*/
static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
+ struct mptcp_addr_info *remote,
struct mptcp_addr_info *addrs)
{
struct sock *sk = (struct sock *)msk;
struct mptcp_pm_addr_entry *entry;
- struct mptcp_addr_info local;
struct pm_nl_pernet *pernet;
unsigned int subflows_max;
int i = 0;
@@ -642,15 +641,8 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
if (!(entry->flags & MPTCP_PM_ADDR_FLAG_FULLMESH))
continue;
- if (entry->addr.family != sk->sk_family) {
-#if IS_ENABLED(CONFIG_MPTCP_IPV6)
- if ((entry->addr.family == AF_INET &&
- !ipv6_addr_v4mapped(&sk->sk_v6_daddr)) ||
- (sk->sk_family == AF_INET &&
- !ipv6_addr_v4mapped(&entry->addr.addr6)))
-#endif
- continue;
- }
+ if (!mptcp_pm_addr_families_match(sk, &entry->addr, remote))
+ continue;
if (msk->pm.subflows < subflows_max) {
msk->pm.subflows++;
@@ -663,8 +655,18 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
* 'IPADDRANY' local address
*/
if (!i) {
+ struct mptcp_addr_info local;
+
memset(&local, 0, sizeof(local));
- local.family = msk->pm.remote.family;
+ local.family =
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+ remote->family == AF_INET6 &&
+ ipv6_addr_v4mapped(&remote->addr6) ? AF_INET :
+#endif
+ remote->family;
+
+ if (!mptcp_pm_addr_families_match(sk, &local, remote))
+ return 0;
msk->pm.subflows++;
addrs[i++] = local;
@@ -703,7 +705,9 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
/* connect to the specified remote address, using whatever
* local address the routing configuration will pick.
*/
- nr = fill_local_addresses_vec(msk, addrs);
+ nr = fill_local_addresses_vec(msk, &remote, addrs);
+ if (nr == 0)
+ return;
msk->pm.add_addr_accepted++;
if (msk->pm.add_addr_accepted >= add_addr_accept_max ||
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH mptcp-next v5 05/12] mptcp: propagate sk_ipv6only to subflows
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (3 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-next v5 04/12] mptcp: let the in-kernel PM use mixed IPv4 and IPv6 addresses Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-next v5 06/12] selftests: mptcp: add test-cases for mixed v4/v6 subflows Matthieu Baerts
` (7 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts
Usually, attributes are propagated to subflows as well.
Here, if subflows are created by other ways than the MPTCP path-manager,
it is important to make sure they are in v6 if it is asked by the
userspace.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
net/mptcp/sockopt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 582ed93bcc8a..9986681aaf40 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1255,6 +1255,7 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
ssk->sk_priority = sk->sk_priority;
ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
ssk->sk_incoming_cpu = sk->sk_incoming_cpu;
+ ssk->sk_ipv6only = sk->sk_ipv6only;
__ip_sock_set_tos(ssk, inet_sk(sk)->tos);
if (sk->sk_userlocks & tx_rx_locks) {
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH mptcp-next v5 06/12] selftests: mptcp: add test-cases for mixed v4/v6 subflows
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (4 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-next v5 05/12] mptcp: propagate sk_ipv6only to subflows Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-next v5 07/12] mptcp: remove assigned but unused value Matthieu Baerts
` (6 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Paolo Abeni, Matthieu Baerts
From: Paolo Abeni <pabeni@redhat.com>
Note that we can't guess the listener family anymore based on the client
target address: always use IPv6.
The fullmesh flag with endpoints from different families is also
validated here.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
.../testing/selftests/net/mptcp/mptcp_join.sh | 53 +++++++++++++++----
1 file changed, 44 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index d11d3d566608..387abdcec011 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -774,24 +774,17 @@ do_transfer()
addr_nr_ns2=${addr_nr_ns2:9}
fi
- local local_addr
- if is_v6 "${connect_addr}"; then
- local_addr="::"
- else
- local_addr="0.0.0.0"
- fi
-
extra_srv_args="$extra_args $extra_srv_args"
if [ "$test_link_fail" -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 ${local_addr} < "$sinfail" > "$sout" &
+ $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 ${local_addr} < "$sin" > "$sout" &
+ $extra_srv_args "::" < "$sin" > "$sout" &
fi
local spid=$!
@@ -2448,6 +2441,47 @@ v4mapped_tests()
fi
}
+mixed_tests()
+{
+ if reset "IPv4 sockets do not use IPv6 addresses"; then
+ pm_nl_set_limits $ns1 0 1
+ pm_nl_set_limits $ns2 1 1
+ pm_nl_add_endpoint $ns1 dead:beef:2::1 flags signal
+ run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow
+ chk_join_nr 0 0 0
+ fi
+
+ # Need an IPv6 mptcp socket to allow subflows of both families
+ if reset "simult IPv4 and IPv6 subflows"; then
+ pm_nl_set_limits $ns1 0 1
+ pm_nl_set_limits $ns2 1 1
+ pm_nl_add_endpoint $ns1 10.0.1.1 flags signal
+ run_tests $ns1 $ns2 dead:beef:2::1 0 0 0 slow
+ chk_join_nr 1 1 1
+ fi
+
+ # cross families subflows will not be created even in fullmesh mode
+ if reset "simult IPv4 and IPv6 subflows, fullmesh 1x1"; then
+ pm_nl_set_limits $ns1 0 4
+ pm_nl_set_limits $ns2 1 4
+ pm_nl_add_endpoint $ns2 dead:beef:2::2 flags subflow,fullmesh
+ pm_nl_add_endpoint $ns1 10.0.1.1 flags signal
+ run_tests $ns1 $ns2 dead:beef:2::1 0 0 0 slow
+ chk_join_nr 1 1 1
+ fi
+
+ # fullmesh still tries to create all the possibly subflows with
+ # matching family
+ if reset "simult IPv4 and IPv6 subflows, fullmesh 2x2"; then
+ pm_nl_set_limits $ns1 0 4
+ pm_nl_set_limits $ns2 2 4
+ pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
+ pm_nl_add_endpoint $ns1 dead:beef:2::1 flags signal
+ run_tests $ns1 $ns2 dead:beef:1::1 0 0 fullmesh_1 slow
+ chk_join_nr 4 4 4
+ fi
+}
+
backup_tests()
{
# single subflow, backup
@@ -3120,6 +3154,7 @@ all_tests_sorted=(
a@add_tests
6@ipv6_tests
4@v4mapped_tests
+ M@mixed_tests
b@backup_tests
p@add_addr_ports_tests
k@syncookies_tests
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH mptcp-next v5 07/12] mptcp: remove assigned but unused value
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (5 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-next v5 06/12] selftests: mptcp: add test-cases for mixed v4/v6 subflows Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-10 9:20 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-next v5 08/12] mptcp: userspace pm: use a single point of exit Matthieu Baerts
` (5 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts
'ret' is always set later before being used.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
net/mptcp/pm_userspace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index ea6ad9da7493..be389dad3f0a 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -34,7 +34,7 @@ int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
struct mptcp_pm_addr_entry *e;
bool addr_match = false;
bool id_match = false;
- int ret = -EINVAL;
+ int ret;
bitmap_zero(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH mptcp-next v5 07/12] mptcp: remove assigned but unused value
2023-01-04 17:15 ` [PATCH mptcp-next v5 07/12] mptcp: remove assigned but unused value Matthieu Baerts
@ 2023-01-10 9:20 ` Matthieu Baerts
0 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-10 9:20 UTC (permalink / raw)
To: MPTCP Upstream
Hello,
On 04/01/2023 18:15, Matthieu Baerts wrote:
> 'ret' is always set later before being used.
>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
> net/mptcp/pm_userspace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index ea6ad9da7493..be389dad3f0a 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -34,7 +34,7 @@ int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
> struct mptcp_pm_addr_entry *e;
> bool addr_match = false;
> bool id_match = false;
> - int ret = -EINVAL;
> + int ret;
I don't know how I got the warning nor how I missed that but as reported
by the kernel test robot, "ret" could be used unassigned below. So I'm
going to revert this patch directly in our tree, sorry for the noise.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH mptcp-next v5 08/12] mptcp: userspace pm: use a single point of exit
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (6 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-next v5 07/12] mptcp: remove assigned but unused value Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles Matthieu Baerts
` (4 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts
Like in all other functions in this file, a single point of exit is used
when extra operations are needed: unlock, decrement refcount, etc.
There is no functional change for the moment but it is better to do the
same here to make sure all cleanups are done in case of intermediate
errors.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
net/mptcp/pm_userspace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index be389dad3f0a..27badb5a5820 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -59,8 +59,8 @@ int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
*/
e = sock_kmalloc(sk, sizeof(*e), GFP_ATOMIC);
if (!e) {
- spin_unlock_bh(&msk->pm.lock);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto append_err;
}
*e = *entry;
@@ -74,6 +74,7 @@ int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
ret = entry->addr.id;
}
+append_err:
spin_unlock_bh(&msk->pm.lock);
return ret;
}
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (7 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-next v5 08/12] mptcp: userspace pm: use a single point of exit Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-06 1:19 ` Mat Martineau
2023-01-04 17:15 ` [PATCH mptcp-next v5 10/12] selftests: mptcp: userspace: refactor asserts Matthieu Baerts
` (3 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts
This script is running a few tests after having setup the environment.
Printing titles helps understand what is being tested.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
Notes:
v3->v4:
- Add a title for the Cleanup (+ 'Done' at the end)
.../selftests/net/mptcp/userspace_pm.sh | 24 ++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index ab2d581f28a1..f9a03e6e968f 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -43,6 +43,11 @@ rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
ns1="ns1-$rndh"
ns2="ns2-$rndh"
+print_title()
+{
+ stdbuf -o0 -e0 printf "\t%s:\n" "${1}"
+}
+
kill_wait()
{
kill $1 > /dev/null 2>&1
@@ -51,7 +56,7 @@ kill_wait()
cleanup()
{
- echo "cleanup"
+ print_title "Cleanup"
rm -rf $file $client_evts $server_evts
@@ -78,6 +83,8 @@ cleanup()
for netns in "$ns1" "$ns2" ;do
ip netns del "$netns"
done
+
+ stdbuf -o0 -e0 printf "Done\n"
}
trap cleanup EXIT
@@ -108,6 +115,7 @@ ip -net "$ns2" addr add dead:beef:1::2/64 dev ns2eth1 nodad
ip -net "$ns2" addr add dead:beef:2::2/64 dev ns2eth1 nodad
ip -net "$ns2" link set ns2eth1 up
+print_title "Init"
stdbuf -o0 -e0 printf "Created network namespaces ns1, ns2 \t\t\t[OK]\n"
make_file()
@@ -255,6 +263,8 @@ verify_announce_event()
test_announce()
{
+ print_title "Announce tests"
+
# Capture events on the network namespace running the server
:>"$server_evts"
@@ -359,6 +369,8 @@ verify_remove_event()
test_remove()
{
+ print_title "Remove tests"
+
# Capture events on the network namespace running the server
:>"$server_evts"
@@ -521,6 +533,8 @@ verify_subflow_events()
test_subflows()
{
+ print_title "Subflows v4 or v6 only tests"
+
# Capture events on the network namespace running the server
:>"$server_evts"
@@ -754,6 +768,8 @@ test_subflows()
test_subflows_v4_v6_mix()
{
+ print_title "Subflows v4 and v6 mix tests"
+
# Attempt to add a listener at 10.0.2.1:<subflow-port>
ip netns exec "$ns1" ./pm_nl_ctl listen 10.0.2.1\
$app6_port > /dev/null 2>&1 &
@@ -800,6 +816,8 @@ test_subflows_v4_v6_mix()
test_prio()
{
+ print_title "Prio tests"
+
local count
# Send MP_PRIO signal from client to server machine
@@ -876,6 +894,8 @@ verify_listener_events()
test_listener()
{
+ print_title "Listener tests"
+
# Capture events on the network namespace running the client
:>$client_evts
@@ -902,8 +922,10 @@ test_listener()
verify_listener_events $client_evts $LISTENER_CLOSED $AF_INET 10.0.2.2 $client4_port
}
+print_title "Make connections"
make_connection
make_connection "v6"
+
test_announce
test_remove
test_subflows
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles
2023-01-04 17:15 ` [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles Matthieu Baerts
@ 2023-01-06 1:19 ` Mat Martineau
2023-01-06 6:34 ` Matthieu Baerts
0 siblings, 1 reply; 23+ messages in thread
From: Mat Martineau @ 2023-01-06 1:19 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp
On Wed, 4 Jan 2023, Matthieu Baerts wrote:
> This script is running a few tests after having setup the environment.
>
> Printing titles helps understand what is being tested.
>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>
> Notes:
> v3->v4:
> - Add a title for the Cleanup (+ 'Done' at the end)
>
> .../selftests/net/mptcp/userspace_pm.sh | 24 ++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index ab2d581f28a1..f9a03e6e968f 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -43,6 +43,11 @@ rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
> ns1="ns1-$rndh"
> ns2="ns2-$rndh"
>
> +print_title()
> +{
> + stdbuf -o0 -e0 printf "\t%s:\n" "${1}"
The indented titles seem kind of inverted to me:
"""
Init:
Created network namespaces ns1, ns2 [OK]
Make connections:
Established IPv4 MPTCP Connection ns2 => ns1 [OK]
Established IPv6 MPTCP Connection ns2 => ns1 [OK]
Announce tests:
ADD_ADDR 10.0.2.2 (ns2) => ns1, invalid token [OK]
ADD_ADDR id:55 10.0.2.2 (ns2) => ns1, reuse port [OK]
...
"""
It would read better to have the titles left-justified and the test
results indented, but that makes all of the test results extra long. Seems
like you are trying to avoid that (for good reason!).
What do you think about copying the "INFO: ..." style from
mptcp_connect.sh?
"""
INFO: Init
Created network namespaces ns1, ns2 [OK]
INFO: Make connections
Established IPv4 MPTCP Connection ns2 => ns1 [OK]
Established IPv6 MPTCP Connection ns2 => ns1 [OK]
INFO: Announce tests
ADD_ADDR 10.0.2.2 (ns2) => ns1, invalid token [OK]
ADD_ADDR id:55 10.0.2.2 (ns2) => ns1, reuse port [OK]
...
"""
- Mat
> +}
> +
> kill_wait()
> {
> kill $1 > /dev/null 2>&1
> @@ -51,7 +56,7 @@ kill_wait()
>
> cleanup()
> {
> - echo "cleanup"
> + print_title "Cleanup"
>
> rm -rf $file $client_evts $server_evts
>
> @@ -78,6 +83,8 @@ cleanup()
> for netns in "$ns1" "$ns2" ;do
> ip netns del "$netns"
> done
> +
> + stdbuf -o0 -e0 printf "Done\n"
> }
>
> trap cleanup EXIT
> @@ -108,6 +115,7 @@ ip -net "$ns2" addr add dead:beef:1::2/64 dev ns2eth1 nodad
> ip -net "$ns2" addr add dead:beef:2::2/64 dev ns2eth1 nodad
> ip -net "$ns2" link set ns2eth1 up
>
> +print_title "Init"
> stdbuf -o0 -e0 printf "Created network namespaces ns1, ns2 \t\t\t[OK]\n"
>
> make_file()
> @@ -255,6 +263,8 @@ verify_announce_event()
>
> test_announce()
> {
> + print_title "Announce tests"
> +
> # Capture events on the network namespace running the server
> :>"$server_evts"
>
> @@ -359,6 +369,8 @@ verify_remove_event()
>
> test_remove()
> {
> + print_title "Remove tests"
> +
> # Capture events on the network namespace running the server
> :>"$server_evts"
>
> @@ -521,6 +533,8 @@ verify_subflow_events()
>
> test_subflows()
> {
> + print_title "Subflows v4 or v6 only tests"
> +
> # Capture events on the network namespace running the server
> :>"$server_evts"
>
> @@ -754,6 +768,8 @@ test_subflows()
>
> test_subflows_v4_v6_mix()
> {
> + print_title "Subflows v4 and v6 mix tests"
> +
> # Attempt to add a listener at 10.0.2.1:<subflow-port>
> ip netns exec "$ns1" ./pm_nl_ctl listen 10.0.2.1\
> $app6_port > /dev/null 2>&1 &
> @@ -800,6 +816,8 @@ test_subflows_v4_v6_mix()
>
> test_prio()
> {
> + print_title "Prio tests"
> +
> local count
>
> # Send MP_PRIO signal from client to server machine
> @@ -876,6 +894,8 @@ verify_listener_events()
>
> test_listener()
> {
> + print_title "Listener tests"
> +
> # Capture events on the network namespace running the client
> :>$client_evts
>
> @@ -902,8 +922,10 @@ test_listener()
> verify_listener_events $client_evts $LISTENER_CLOSED $AF_INET 10.0.2.2 $client4_port
> }
>
> +print_title "Make connections"
> make_connection
> make_connection "v6"
> +
> test_announce
> test_remove
> test_subflows
> --
> 2.37.2
>
>
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles
2023-01-06 1:19 ` Mat Martineau
@ 2023-01-06 6:34 ` Matthieu Baerts
2023-01-06 16:34 ` Mat Martineau
0 siblings, 1 reply; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-06 6:34 UTC (permalink / raw)
To: Mat Martineau; +Cc: mptcp
Hi Mat,
Thank you for the review!
6 Jan 2023 02:19:26 Mat Martineau <mathew.j.martineau@linux.intel.com>:
> On Wed, 4 Jan 2023, Matthieu Baerts wrote:
>
>> This script is running a few tests after having setup the environment.
>>
>> Printing titles helps understand what is being tested.
>>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>
>> Notes:
>> v3->v4:
>> - Add a title for the Cleanup (+ 'Done' at the end)
>>
>> .../selftests/net/mptcp/userspace_pm.sh | 24 ++++++++++++++++++-
>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>> index ab2d581f28a1..f9a03e6e968f 100755
>> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
>> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>> @@ -43,6 +43,11 @@ rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
>> ns1="ns1-$rndh"
>> ns2="ns2-$rndh"
>>
>> +print_title()
>> +{
>> + stdbuf -o0 -e0 printf "\t%s:\n" "${1}"
>
> The indented titles seem kind of inverted to me:
>
> """
> Init:
> Created network namespaces ns1, ns2 [OK]
> Make connections:
> Established IPv4 MPTCP Connection ns2 => ns1 [OK]
> Established IPv6 MPTCP Connection ns2 => ns1 [OK]
> Announce tests:
> ADD_ADDR 10.0.2.2 (ns2) => ns1, invalid token [OK]
> ADD_ADDR id:55 10.0.2.2 (ns2) => ns1, reuse port [OK]
> ...
> """
>
> It would read better to have the titles left-justified and the test results indented, but that makes all of the test results extra long. Seems like you are trying to avoid that (for good reason!).
Indeed :)
> What do you think about copying the "INFO: ..." style from mptcp_connect.sh?
Good idea, that works for me!
I can do the modification (s/\t/INFO: /) when applying the patches if that's OK for you (if there is nothing else to change of course).
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles
2023-01-06 6:34 ` Matthieu Baerts
@ 2023-01-06 16:34 ` Mat Martineau
0 siblings, 0 replies; 23+ messages in thread
From: Mat Martineau @ 2023-01-06 16:34 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp
[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]
On Fri, 6 Jan 2023, Matthieu Baerts wrote:
> Hi Mat,
>
> Thank you for the review!
>
> 6 Jan 2023 02:19:26 Mat Martineau <mathew.j.martineau@linux.intel.com>:
>
>> On Wed, 4 Jan 2023, Matthieu Baerts wrote:
>>
>>> This script is running a few tests after having setup the environment.
>>>
>>> Printing titles helps understand what is being tested.
>>>
>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>> ---
>>>
>>> Notes:
>>> v3->v4:
>>> - Add a title for the Cleanup (+ 'Done' at the end)
>>>
>>> .../selftests/net/mptcp/userspace_pm.sh | 24 ++++++++++++++++++-
>>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>>> index ab2d581f28a1..f9a03e6e968f 100755
>>> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
>>> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
>>> @@ -43,6 +43,11 @@ rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
>>> ns1="ns1-$rndh"
>>> ns2="ns2-$rndh"
>>>
>>> +print_title()
>>> +{
>>> + stdbuf -o0 -e0 printf "\t%s:\n" "${1}"
>>
>> The indented titles seem kind of inverted to me:
>>
>> """
>> Init:
>> Created network namespaces ns1, ns2 [OK]
>> Make connections:
>> Established IPv4 MPTCP Connection ns2 => ns1 [OK]
>> Established IPv6 MPTCP Connection ns2 => ns1 [OK]
>> Announce tests:
>> ADD_ADDR 10.0.2.2 (ns2) => ns1, invalid token [OK]
>> ADD_ADDR id:55 10.0.2.2 (ns2) => ns1, reuse port [OK]
>> ...
>> """
>>
>> It would read better to have the titles left-justified and the test results indented, but that makes all of the test results extra long. Seems like you are trying to avoid that (for good reason!).
>
> Indeed :)
>
>> What do you think about copying the "INFO: ..." style from mptcp_connect.sh?
>
> Good idea, that works for me!
>
> I can do the modification (s/\t/INFO: /) when applying the patches if that's OK for you (if there is nothing else to change of course).
>
Sure, that's fine with me. For patches 9-12:
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH mptcp-next v5 10/12] selftests: mptcp: userspace: refactor asserts
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (8 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-next v5 09/12] selftests: mptcp: userspace: print titles Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-next v5 11/12] selftests: mptcp: userspace: print error details if any Matthieu Baerts
` (2 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts
Instead of having a long list of conditions to check, it is possible to
give a list of variable names to compare with their 'e_XXX' version.
This will ease the introduction of the following commit which will print
which condition has failed (if any).
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
.../selftests/net/mptcp/userspace_pm.sh | 72 +++++++++----------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index f9a03e6e968f..9edd1abcc067 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -225,6 +225,36 @@ make_connection()
fi
}
+# $1: var name
+check_expected_one()
+{
+ local var="${1}"
+ local exp="e_${var}"
+
+ [ "${!var}" = "${!exp}" ]
+}
+
+# $@: all var names to check
+check_expected()
+{
+ local ret=0
+ local var
+
+ for var in "${@}"
+ do
+ check_expected_one "${var}" || ret=1
+ done
+
+ if [ ${ret} -eq 0 ]
+ then
+ stdbuf -o0 -e0 printf "[OK]\n"
+ return 0
+ fi
+
+ stdbuf -o0 -e0 printf "[FAIL]\n"
+ exit 1
+}
+
verify_announce_event()
{
local evt=$1
@@ -250,15 +280,8 @@ verify_announce_event()
fi
dport=$(sed --unbuffered -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q' "$evt")
id=$(sed --unbuffered -n 's/.*\(rem_id:\)\([[:digit:]]*\).*$/\2/p;q' "$evt")
- if [ "$type" = "$e_type" ] && [ "$token" = "$e_token" ] &&
- [ "$addr" = "$e_addr" ] && [ "$dport" = "$e_dport" ] &&
- [ "$id" = "$e_id" ]
- then
- stdbuf -o0 -e0 printf "[OK]\n"
- return 0
- fi
- stdbuf -o0 -e0 printf "[FAIL]\n"
- exit 1
+
+ check_expected "type" "token" "addr" "dport" "id"
}
test_announce()
@@ -357,14 +380,8 @@ verify_remove_event()
type=$(sed --unbuffered -n 's/.*\(type:\)\([[:digit:]]*\).*$/\2/p;q' "$evt")
token=$(sed --unbuffered -n 's/.*\(token:\)\([[:digit:]]*\).*$/\2/p;q' "$evt")
id=$(sed --unbuffered -n 's/.*\(rem_id:\)\([[:digit:]]*\).*$/\2/p;q' "$evt")
- if [ "$type" = "$e_type" ] && [ "$token" = "$e_token" ] &&
- [ "$id" = "$e_id" ]
- then
- stdbuf -o0 -e0 printf "[OK]\n"
- return 0
- fi
- stdbuf -o0 -e0 printf "[FAIL]\n"
- exit 1
+
+ check_expected "type" "token" "id"
}
test_remove()
@@ -519,16 +536,7 @@ verify_subflow_events()
daddr=$(sed --unbuffered -n 's/.*\(daddr4:\)\([0-9.]*\).*$/\2/p;q' "$evt")
fi
- if [ "$type" = "$e_type" ] && [ "$token" = "$e_token" ] &&
- [ "$daddr" = "$e_daddr" ] && [ "$e_dport" = "$dport" ] &&
- [ "$family" = "$e_family" ] && [ "$saddr" = "$e_saddr" ] &&
- [ "$e_locid" = "$locid" ] && [ "$e_remid" = "$remid" ]
- then
- stdbuf -o0 -e0 printf "[OK]\n"
- return 0
- fi
- stdbuf -o0 -e0 printf "[FAIL]\n"
- exit 1
+ check_expected "type" "token" "daddr" "dport" "family" "saddr" "locid" "remid"
}
test_subflows()
@@ -881,15 +889,7 @@ verify_listener_events()
sed --unbuffered -n 's/.*\(saddr4:\)\([0-9.]*\).*$/\2/p;q')
fi
- if [ $type ] && [ $type = $e_type ] &&
- [ $family ] && [ $family = $e_family ] &&
- [ $saddr ] && [ $saddr = $e_saddr ] &&
- [ $sport ] && [ $sport = $e_sport ]; then
- stdbuf -o0 -e0 printf "[OK]\n"
- return 0
- fi
- stdbuf -o0 -e0 printf "[FAIL]\n"
- exit 1
+ check_expected "type" "family" "saddr" "sport"
}
test_listener()
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH mptcp-next v5 11/12] selftests: mptcp: userspace: print error details if any
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (9 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-next v5 10/12] selftests: mptcp: userspace: refactor asserts Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-04 17:15 ` [PATCH mptcp-next v5 12/12] selftests: mptcp: userspace: avoid read errors Matthieu Baerts
2023-01-06 1:15 ` [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Mat Martineau
12 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts
Before, only '[FAIL]' was printed in case of error during the validation
phase.
Now, in case of failure, the variable name, its value and expected one
are displayed to help understand what was wrong.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
.../selftests/net/mptcp/userspace_pm.sh | 33 ++++++++++++++-----
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 9edd1abcc067..0bd35768c1aa 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -201,11 +201,16 @@ make_connection()
server_serverside=$(grep "type:1," "$server_evts" |
sed --unbuffered -n 's/.*\(server_side:\)\([[:digit:]]*\).*$/\2/p;q')
+ stdbuf -o0 -e0 printf "Established IP%s MPTCP Connection ns2 => ns1 \t\t" $is_v6
if [ "$client_token" != "" ] && [ "$server_token" != "" ] && [ "$client_serverside" = 0 ] &&
[ "$server_serverside" = 1 ]
then
- stdbuf -o0 -e0 printf "Established IP%s MPTCP Connection ns2 => ns1 \t\t[OK]\n" $is_v6
+ stdbuf -o0 -e0 printf "[OK]\n"
else
+ stdbuf -o0 -e0 printf "[FAIL]\n"
+ stdbuf -o0 -e0 printf "\tExpected tokens (c:%s - s:%s) and server (c:%d - s:%d)\n" \
+ "${client_token}" "${server_token}" \
+ "${client_serverside}" "${server_serverside}"
exit 1
fi
@@ -225,13 +230,26 @@ make_connection()
fi
}
-# $1: var name
+# $1: var name ; $2: prev ret
check_expected_one()
{
local var="${1}"
local exp="e_${var}"
+ local prev_ret="${2}"
- [ "${!var}" = "${!exp}" ]
+ if [ "${!var}" = "${!exp}" ]
+ then
+ return 0
+ fi
+
+ if [ "${prev_ret}" = "0" ]
+ then
+ stdbuf -o0 -e0 printf "[FAIL]\n"
+ fi
+
+ stdbuf -o0 -e0 printf "\tExpected value for '%s': '%s', got '%s'.\n" \
+ "${var}" "${!var}" "${!exp}"
+ return 1
}
# $@: all var names to check
@@ -242,7 +260,7 @@ check_expected()
for var in "${@}"
do
- check_expected_one "${var}" || ret=1
+ check_expected_one "${var}" "${ret}" || ret=1
done
if [ ${ret} -eq 0 ]
@@ -251,7 +269,6 @@ check_expected()
return 0
fi
- stdbuf -o0 -e0 printf "[FAIL]\n"
exit 1
}
@@ -303,7 +320,7 @@ test_announce()
then
stdbuf -o0 -e0 printf "[OK]\n"
else
- stdbuf -o0 -e0 printf "[FAIL]\n"
+ stdbuf -o0 -e0 printf "[FAIL]\n\ttype defined: %s\n" "${type}"
exit 1
fi
@@ -837,7 +854,7 @@ test_prio()
count=$(ip netns exec "$ns2" nstat -as | grep MPTcpExtMPPrioTx | awk '{print $2}')
[ -z "$count" ] && count=0
if [ $count != 1 ]; then
- stdbuf -o0 -e0 printf "[FAIL]\n"
+ stdbuf -o0 -e0 printf "[FAIL]\n\tCount != 1: %d\n" "${count}"
exit 1
else
stdbuf -o0 -e0 printf "[OK]\n"
@@ -848,7 +865,7 @@ test_prio()
count=$(ip netns exec "$ns1" nstat -as | grep MPTcpExtMPPrioRx | awk '{print $2}')
[ -z "$count" ] && count=0
if [ $count != 1 ]; then
- stdbuf -o0 -e0 printf "[FAIL]\n"
+ stdbuf -o0 -e0 printf "[FAIL]\n\tCount != 1: %d\n" "${count}"
exit 1
else
stdbuf -o0 -e0 printf "[OK]\n"
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH mptcp-next v5 12/12] selftests: mptcp: userspace: avoid read errors
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (10 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-next v5 11/12] selftests: mptcp: userspace: print error details if any Matthieu Baerts
@ 2023-01-04 17:15 ` Matthieu Baerts
2023-01-05 8:32 ` selftests: mptcp: userspace: avoid read errors: Tests Results MPTCP CI
` (2 more replies)
2023-01-06 1:15 ` [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Mat Martineau
12 siblings, 3 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-04 17:15 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts
During the cleanup phase, the server pids were killed with a SIGTERM
directly, not using a SIGUSR1 first to quit safely. As a result, this
test was often ending with two error messages:
read: Connection reset by peer
While at it, use a for-loop to terminal all the PIDs the same way.
Also the different files are now removed after having killed the PIDs
using them. It makes more sense to do that in this order.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
.../selftests/net/mptcp/userspace_pm.sh | 32 +++++++------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 0bd35768c1aa..8b4f130800b9 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -50,6 +50,9 @@ print_title()
kill_wait()
{
+ [ $1 -eq 0 ] && return 0
+
+ kill -SIGUSR1 $1 > /dev/null 2>&1
kill $1 > /dev/null 2>&1
wait $1 2>/dev/null
}
@@ -58,32 +61,21 @@ cleanup()
{
print_title "Cleanup"
- rm -rf $file $client_evts $server_evts
-
# Terminate the MPTCP connection and related processes
- if [ $client4_pid -ne 0 ]; then
- kill -SIGUSR1 $client4_pid > /dev/null 2>&1
- fi
- if [ $server4_pid -ne 0 ]; then
- kill_wait $server4_pid
- fi
- if [ $client6_pid -ne 0 ]; then
- kill -SIGUSR1 $client6_pid > /dev/null 2>&1
- fi
- if [ $server6_pid -ne 0 ]; then
- kill_wait $server6_pid
- fi
- if [ $server_evts_pid -ne 0 ]; then
- kill_wait $server_evts_pid
- fi
- if [ $client_evts_pid -ne 0 ]; then
- kill_wait $client_evts_pid
- fi
+ local pid
+ for pid in $client4_pid $server4_pid $client6_pid $server6_pid\
+ $server_evts_pid $client_evts_pid
+ do
+ kill_wait $pid
+ done
+
local netns
for netns in "$ns1" "$ns2" ;do
ip netns del "$netns"
done
+ rm -rf $file $client_evts $server_evts
+
stdbuf -o0 -e0 printf "Done\n"
}
--
2.37.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: selftests: mptcp: userspace: avoid read errors: Tests Results
2023-01-04 17:15 ` [PATCH mptcp-next v5 12/12] selftests: mptcp: userspace: avoid read errors Matthieu Baerts
@ 2023-01-05 8:32 ` MPTCP CI
2023-01-06 2:39 ` MPTCP CI
2023-01-06 17:39 ` MPTCP CI
2 siblings, 0 replies; 23+ messages in thread
From: MPTCP CI @ 2023-01-05 8:32 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):
- Success! ✅:
- Task: https://cirrus-ci.com/task/6007793140891648
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6007793140891648/summary/summary.txt
- KVM Validation: normal (only selftest_mptcp_join):
- Success! ✅:
- Task: https://cirrus-ci.com/task/5444843187470336
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5444843187470336/summary/summary.txt
- KVM Validation: debug (except selftest_mptcp_join):
- Success! ✅:
- Task: https://cirrus-ci.com/task/6570743094312960
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6570743094312960/summary/summary.txt
- KVM Validation: debug (only selftest_mptcp_join):
- Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
- Task: https://cirrus-ci.com/task/4741155745693696
- Summary: https://api.cirrus-ci.com/v1/artifact/task/4741155745693696/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/b224e1e9ab22
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-debug
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 (Tessares)
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: selftests: mptcp: userspace: avoid read errors: Tests Results
2023-01-04 17:15 ` [PATCH mptcp-next v5 12/12] selftests: mptcp: userspace: avoid read errors Matthieu Baerts
2023-01-05 8:32 ` selftests: mptcp: userspace: avoid read errors: Tests Results MPTCP CI
@ 2023-01-06 2:39 ` MPTCP CI
2023-01-06 17:39 ` MPTCP CI
2 siblings, 0 replies; 23+ messages in thread
From: MPTCP CI @ 2023-01-06 2:39 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):
- Success! ✅:
- Task: https://cirrus-ci.com/task/5545190199394304
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5545190199394304/summary/summary.txt
- KVM Validation: debug (except selftest_mptcp_join):
- Success! ✅:
- Task: https://cirrus-ci.com/task/4630396525084672
- Summary: https://api.cirrus-ci.com/v1/artifact/task/4630396525084672/summary/summary.txt
- KVM Validation: normal (only selftest_mptcp_join):
- Success! ✅:
- Task: https://cirrus-ci.com/task/6671090106236928
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6671090106236928/summary/summary.txt
- KVM Validation: debug (only selftest_mptcp_join):
- Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
- Task: https://cirrus-ci.com/task/5756296431927296
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5756296431927296/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/d15ee7ad18a9
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-debug
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 (Tessares)
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: selftests: mptcp: userspace: avoid read errors: Tests Results
2023-01-04 17:15 ` [PATCH mptcp-next v5 12/12] selftests: mptcp: userspace: avoid read errors Matthieu Baerts
2023-01-05 8:32 ` selftests: mptcp: userspace: avoid read errors: Tests Results MPTCP CI
2023-01-06 2:39 ` MPTCP CI
@ 2023-01-06 17:39 ` MPTCP CI
2 siblings, 0 replies; 23+ messages in thread
From: MPTCP CI @ 2023-01-06 17:39 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):
- Success! ✅:
- Task: https://cirrus-ci.com/task/5454374122553344
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5454374122553344/summary/summary.txt
- KVM Validation: normal (only selftest_mptcp_join):
- Success! ✅:
- Task: https://cirrus-ci.com/task/6580274029395968
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6580274029395968/summary/summary.txt
- KVM Validation: debug (except selftest_mptcp_join):
- Success! ✅:
- Task: https://cirrus-ci.com/task/4750686680776704
- Summary: https://api.cirrus-ci.com/v1/artifact/task/4750686680776704/summary/summary.txt
- KVM Validation: debug (only selftest_mptcp_join):
- Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
- Task: https://cirrus-ci.com/task/5876586587619328
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5876586587619328/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/006896eb902f
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-debug
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 (Tessares)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6
2023-01-04 17:15 [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Matthieu Baerts
` (11 preceding siblings ...)
2023-01-04 17:15 ` [PATCH mptcp-next v5 12/12] selftests: mptcp: userspace: avoid read errors Matthieu Baerts
@ 2023-01-06 1:15 ` Mat Martineau
2023-01-09 17:04 ` Matthieu Baerts
12 siblings, 1 reply; 23+ messages in thread
From: Mat Martineau @ 2023-01-06 1:15 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp
On Wed, 4 Jan 2023, Matthieu Baerts wrote:
> This is a v5 of Paolo's series with the same name but with some
> modifications and additional patches.
>
> ChangeLog:
> - v3 -> v4:
> - Fix a compilation error without CONFIG_MPTCP_IPV6 in patch 4/12.
> - Add a title for the cleanup part in patch 9/12
> - Add the patch 12/12 to avoid "read" errors
> - v4 -> v5:
> - Fix typo in patch 2/12 (Mat)
> - More details in the commit message of patch 2/12 (Mat)
> - Rebased on top of the latest export branch
>
> This series can be split in 2 parts: the 3 first patches are for -net
> while the rest is for net-next. Patches for net-next depends on patches
> for -net, that's why everything is being sent together.
>
> Patch 1 lets the userspace PM selects the proper family to avoid
> creating subflows with wrong source and/or destination addresses because
> the family is not the expected one.
>
> Patch 2 makes sure the userspace PM doesn't allow the userspace to
> create subflows for a family that is not allowed.
>
> The core MPTCP implementation is just a few bits of properly supporting
> a mix of v4 and v6 subflows, we just need to allow specifying the
> subflow family explicitly (patch 1) and remove artificial constraints in
> the in-kernel PM currently enforcing no mixed subflow in place (patch
> 4).
>
> Patch 5 makes sure the sk_ipv6only attribute is also propagated to
> subflows, just in case the PM doesn't respect it.
>
> Some selftests have also been added for the userspace PM (patch 3) and
> the in-kernel PM (patch 6).
>
> Patches 7 and 8 are just some cleanups in the userspace PM, not related
> to the rest but I saw them when modifying the file.
>
Patches 1-8 look good for further testing in the export (and export-net)
branches:
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> Patches 9 to 12 improve the messages printed by the userspace PM,
> especially in case of error during the validation.
>
I have a very minor formatting comment on patch 9 (separate reply)
- Mat
> Matthieu Baerts (9):
> mptcp: netlink: respect v4/v6-only sockets
> selftests: mptcp: userspace: validate v4-v6 subflows mix
> mptcp: propagate sk_ipv6only to subflows
> mptcp: remove assigned but unused value
> mptcp: userspace pm: use a single point of exit
> selftests: mptcp: userspace: print titles
> selftests: mptcp: userspace: refactor asserts
> selftests: mptcp: userspace: print error details if any
> selftests: mptcp: userspace: avoid read errors
>
> Paolo Abeni (3):
> mptcp: explicitly specify sock family at subflow creation time
> mptcp: let the in-kernel PM use mixed IPv4 and IPv6 addresses
> selftests: mptcp: add test-cases for mixed v4/v6 subflows
>
> net/mptcp/pm.c | 25 +++
> net/mptcp/pm_netlink.c | 58 ++---
> net/mptcp/pm_userspace.c | 14 +-
> net/mptcp/protocol.c | 2 +-
> net/mptcp/protocol.h | 6 +-
> net/mptcp/sockopt.c | 1 +
> net/mptcp/subflow.c | 9 +-
> .../testing/selftests/net/mptcp/mptcp_join.sh | 53 ++++-
> .../selftests/net/mptcp/userspace_pm.sh | 200 ++++++++++++------
> 9 files changed, 262 insertions(+), 106 deletions(-)
>
>
> base-commit: 0e6e165b9b9b5ea418067cfac1e861892ee7e12c
> --
> 2.37.2
>
>
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6
2023-01-06 1:15 ` [PATCH mptcp-net/next v5 00/12] mptcp: add support for mixed v4/v6 Mat Martineau
@ 2023-01-09 17:04 ` Matthieu Baerts
0 siblings, 0 replies; 23+ messages in thread
From: Matthieu Baerts @ 2023-01-09 17:04 UTC (permalink / raw)
To: Mat Martineau; +Cc: mptcp, Paolo Abeni
Hi Mat,
On 06/01/2023 02:15, Mat Martineau wrote:
> On Wed, 4 Jan 2023, Matthieu Baerts wrote:
>
>> This is a v5 of Paolo's series with the same name but with some
>> modifications and additional patches.
>>
>> ChangeLog:
>> - v3 -> v4:
>> - Fix a compilation error without CONFIG_MPTCP_IPV6 in patch 4/12.
>> - Add a title for the cleanup part in patch 9/12
>> - Add the patch 12/12 to avoid "read" errors
>> - v4 -> v5:
>> - Fix typo in patch 2/12 (Mat)
>> - More details in the commit message of patch 2/12 (Mat)
>> - Rebased on top of the latest export branch
>>
>> This series can be split in 2 parts: the 3 first patches are for -net
>> while the rest is for net-next. Patches for net-next depends on patches
>> for -net, that's why everything is being sent together.
>>
>> Patch 1 lets the userspace PM selects the proper family to avoid
>> creating subflows with wrong source and/or destination addresses because
>> the family is not the expected one.
>>
>> Patch 2 makes sure the userspace PM doesn't allow the userspace to
>> create subflows for a family that is not allowed.
>>
>> The core MPTCP implementation is just a few bits of properly supporting
>> a mix of v4 and v6 subflows, we just need to allow specifying the
>> subflow family explicitly (patch 1) and remove artificial constraints in
>> the in-kernel PM currently enforcing no mixed subflow in place (patch
>> 4).
>>
>> Patch 5 makes sure the sk_ipv6only attribute is also propagated to
>> subflows, just in case the PM doesn't respect it.
>>
>> Some selftests have also been added for the userspace PM (patch 3) and
>> the in-kernel PM (patch 6).
>>
>> Patches 7 and 8 are just some cleanups in the userspace PM, not related
>> to the rest but I saw them when modifying the file.
>>
>
> Patches 1-8 look good for further testing in the export (and export-net)
> branches:
>
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
>
>
>
>> Patches 9 to 12 improve the messages printed by the userspace PM,
>> especially in case of error during the validation.
>>
>
> I have a very minor formatting comment on patch 9 (separate reply)
Thank you for the review!
I just applied these patches. I can of course modify them later if Paolo
is not OK with the modifications I did or to add Acked-by! (But I
understand Paolo probably has a huge email backlog to process, no hurry,
but at least here we can have more validations from the CI :-) )
New patches for t/upstream-net:
- 232e5ef7e4fb: mptcp: explicitly specify sock family at subflow
creation time
- 8215e56b14f2: mptcp: netlink: respect v4/v6-only sockets
- 13ce9dd3a3da: selftests: mptcp: userspace: validate v4-v6 subflows mix
- Results: 4d3787dfafff..e8000101a67b (export-net)
Tests are now in progress:
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20230109T163931
New patches for t/upstream:
- d7f79d4ba386: mptcp: let the in-kernel PM use mixed IPv4 and IPv6
addresses
- 4b28be013860: mptcp: propagate sk_ipv6only to subflows
- 371aa633fcf3: selftests: mptcp: add test-cases for mixed v4/v6 subflows
- 0e67a2df934f: mptcp: remove assigned but unused value
- 082c80d3fdbd: mptcp: userspace pm: use a single point of exit
- 621e00114ef6: selftests: mptcp: userspace: print titles
- 7c48d44f2d7d: selftests: mptcp: userspace: refactor asserts
- 543addc028f6: selftests: mptcp: userspace: print error details if any
- 4d29130d0eb7: selftests: mptcp: userspace: avoid read errors
- Results: 4ae7362cc1dc..24e3778f30ac (export)
Tests are now in progress:
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230109T165807
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 23+ messages in thread