9:06 p.m.
This patchset adds IPv4-mapped address support and testcases.
v4:
- do not try to create subflows between v4 and v6 addresses: with v3, we would
try to do that and it could succeed if the IPv6 address ends with a valid v4
address. See patch 2/3
- add two testcases checking that in 3/3: the 1st one was failing with v3.
- add #ifdef MPTCP_IPV6 in patch 1/3 (Mat)
- split the code I previously sent into two commits to allow backports: to be
checked if they can really be considered as "fixes" or "new
features".
- @Geliang: I hope this is OK for you, I quickly did these modifications during
a lunch break and ran mp_join selftests + mpjoin packetdrill. Please tell me
if I did anything wrong :)
v3:
- add Matt's code in it.
- deal with the IPv4-mapped back to IPv4 case too.
- add two new IPv4-mapped back to IPv4 testcases.
v2:
- v1 calls mptcp_pm_addr_convert_v4mapped in mptcp_pm_nl_add_addr_received,
on the receiving side. v2 calls it in select_signal_address, on the
sending side.
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/122
Geliang Tang (1):
selftests: mptcp: add IPv4-mapped IPv6 testcases
Matthieu Baerts (2):
mptcp: support MPJoin with IPv4 mapped in v6 sk
mptcp: pm nl: support IPv4 mapped in v6 addresses
net/mptcp/pm_netlink.c | 37 ++++++----
net/mptcp/subflow.c | 24 +++++--
.../testing/selftests/net/mptcp/mptcp_join.sh | 67 +++++++++++++++++++
3 files changed, 109 insertions(+), 19 deletions(-)
--
2.29.2
9:06 p.m.
New subject: [PATCH mptcp-next v4 1/3] mptcp: support MPJoin with IPv4 mapped in v6 sk
With an IPv4 mapped in v6 socket, we were trying to call inet6_bind()
with an IPv4 address resulting in a -EINVAL error because the given
addr_len -- size of the address structure -- was too short.
We now make sure to use address structures for the same family as the
MPTCP socket for both the bind() and the connect(). It means we convert
v4 addresses to v4 mapped in v6 or the opposite if needed.
Fixes: ec3edaa7ca6c ("mptcp: Add handling of outgoing MP_JOIN requests")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/122
Co-developed-by: Geliang Tang <geliangtang(a)gmail.com>
Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
---
net/mptcp/subflow.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 278cbe3e539e..8dc4d9f86773 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1074,21 +1074,31 @@ void mptcpv6_handle_mapped(struct sock *sk, bool mapped)
#endif
static void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
- struct sockaddr_storage *addr)
+ struct sockaddr_storage *addr,
+ unsigned short family)
{
memset(addr, 0, sizeof(*addr));
- addr->ss_family = info->family;
+ addr->ss_family = family;
if (addr->ss_family == AF_INET) {
struct sockaddr_in *in_addr = (struct sockaddr_in *)addr;
- in_addr->sin_addr = info->addr;
+ if (info->family == AF_INET)
+ in_addr->sin_addr = info->addr;
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+ else if (ipv6_addr_v4mapped(&info->addr6))
+ in_addr->sin_addr.s_addr = info->addr6.s6_addr32[3];
+#endif
in_addr->sin_port = info->port;
}
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
else if (addr->ss_family == AF_INET6) {
struct sockaddr_in6 *in6_addr = (struct sockaddr_in6 *)addr;
- in6_addr->sin6_addr = info->addr6;
+ if (info->family == AF_INET)
+ ipv6_addr_set_v4mapped(info->addr.s_addr,
+ &in6_addr->sin6_addr);
+ else
+ in6_addr->sin6_addr = info->addr6;
in6_addr->sin6_port = info->port;
}
#endif
@@ -1132,11 +1142,11 @@ int __mptcp_subflow_connect(struct sock *sk, const struct
mptcp_addr_info *loc,
subflow->remote_key = msk->remote_key;
subflow->local_key = msk->local_key;
subflow->token = msk->token;
- mptcp_info2sockaddr(loc, &addr);
+ mptcp_info2sockaddr(loc, &addr, ssk->sk_family);
addrlen = sizeof(struct sockaddr_in);
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
- if (loc->family == AF_INET6)
+ if (addr.ss_family == AF_INET6)
addrlen = sizeof(struct sockaddr_in6);
#endif
ssk->sk_bound_dev_if = loc->ifindex;
@@ -1152,7 +1162,7 @@ int __mptcp_subflow_connect(struct sock *sk, const struct
mptcp_addr_info *loc,
subflow->remote_id = remote_id;
subflow->request_join = 1;
subflow->request_bkup = !!(loc->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
- mptcp_info2sockaddr(remote, &addr);
+ mptcp_info2sockaddr(remote, &addr, ssk->sk_family);
mptcp_add_pending_subflow(msk, subflow);
err = kernel_connect(sf, (struct sockaddr *)&addr, addrlen, O_NONBLOCK);
--
2.29.2
9:06 p.m.
New subject: [PATCH mptcp-next v4 2/3] mptcp: pm nl: support IPv4 mapped in v6 addresses
On one side, we can allow the creation of subflows between v4 mapped in
v6 and v4 addresses. For that we look for v4mapped addresses between the
local address we want to select and the remote one.
On the other side, we also properly deal with received v4mapped
addresses, either announced ones or set via Netlink.
Fixes: 01cacb00b35c ("mptcp: add netlink-based PM")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/122
Co-developed-by: Geliang Tang <geliangtang(a)gmail.com>
Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
---
net/mptcp/pm_netlink.c | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 9b1f6298bbdb..6ef00b3799d4 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -60,15 +60,20 @@ static bool addresses_equal(const struct mptcp_addr_info *a,
{
bool addr_equals = false;
- if (a->family != b->family)
- return false;
-
- if (a->family == AF_INET)
- addr_equals = a->addr.s_addr == b->addr.s_addr;
+ if (a->family == b->family) {
+ if (a->family == AF_INET)
+ addr_equals = a->addr.s_addr == b->addr.s_addr;
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
- else
- addr_equals = !ipv6_addr_cmp(&a->addr6, &b->addr6);
+ else
+ addr_equals = !ipv6_addr_cmp(&a->addr6, &b->addr6);
+ } else if (a->family == AF_INET) {
+ if (ipv6_addr_v4mapped(&b->addr6))
+ addr_equals = a->addr.s_addr == b->addr6.s6_addr32[3];
+ } else if (b->family == AF_INET) {
+ if (ipv6_addr_v4mapped(&a->addr6))
+ addr_equals = a->addr6.s6_addr32[3] == b->addr.s_addr;
#endif
+ }
if (!addr_equals)
return false;
@@ -133,8 +138,8 @@ static bool lookup_subflow_by_saddr(const struct list_head *list,
}
static struct mptcp_pm_addr_entry *
-select_local_address(const struct pm_nl_pernet *pernet,
- struct mptcp_sock *msk)
+select_local_address(const struct pm_nl_pernet *pernet, struct mptcp_sock *msk,
+ struct mptcp_addr_info *remote)
{
struct mptcp_pm_addr_entry *entry, *ret = NULL;
@@ -144,11 +149,19 @@ select_local_address(const struct pm_nl_pernet *pernet,
if (!(entry->addr.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
continue;
+ if (entry->addr.family != remote->family)
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+ if ((entry->addr.family == AF_INET &&
+ !ipv6_addr_v4mapped(&remote->addr6)) ||
+ (remote->family == AF_INET &&
+ !ipv6_addr_v4mapped(&entry->addr.addr6)))
+#endif
+ continue;
+
/* avoid any address already in use by subflows and
* pending join
*/
- if (entry->addr.family == ((struct sock *)msk)->sk_family &&
- !lookup_subflow_by_saddr(&msk->conn_list, &entry->addr)) {
+ if (!lookup_subflow_by_saddr(&msk->conn_list, &entry->addr)) {
ret = entry;
break;
}
@@ -346,7 +359,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock
*msk)
msk->pm.subflows < msk->pm.subflows_max) {
remote_address((struct sock_common *)sk, &remote);
- local = select_local_address(pernet, msk);
+ local = select_local_address(pernet, msk, &remote);
if (local) {
msk->pm.local_addr_used++;
msk->pm.subflows++;
--
2.29.2
9:07 p.m.
New subject: [PATCH mptcp-next v4 3/3] selftests: mptcp: add IPv4-mapped IPv6 testcases
From: Geliang Tang <geliangtang(a)gmail.com>
Here, we make sure we support IPv4-mapped in IPv6 addresses in different
contexts:
- a v4-mapped address is received by the PM and can be used as v4.
- a v4 address is received by the PM and can be used even with a v4
mapped socket.
We also make sure we don't try to establish subflows between v4 and v6
addresses, e.g. if a real v6 address ends with a valid v4 address.
Co-developed-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
---
.../testing/selftests/net/mptcp/mptcp_join.sh | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index f74cd993b168..eff813819e6a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -790,6 +790,73 @@ chk_join_nr "remove subflow and signal IPv6" 2 2 2
chk_add_nr 1 1
chk_rm_nr 1 1
+# subflow IPv4-mapped to IPv4-mapped
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add "::ffff:10.0.3.2" flags subflow
+run_tests $ns1 $ns2 "::ffff:10.0.1.1"
+chk_join_nr "single subflow IPv4-mapped" 1 1 1
+
+# signal address IPv4-mapped with IPv4-mapped sk
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+ip netns exec $ns1 ./pm_nl_ctl add "::ffff:10.0.2.1" flags signal
+run_tests $ns1 $ns2 "::ffff:10.0.1.1"
+chk_join_nr "signal address IPv4-mapped" 1 1 1
+chk_add_nr 1 1
+
+# subflow v4-map-v6
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
+run_tests $ns1 $ns2 "::ffff:10.0.1.1"
+chk_join_nr "single subflow v4-map-v6" 1 1 1
+
+# signal address v4-map-v6
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
+run_tests $ns1 $ns2 "::ffff:10.0.1.1"
+chk_join_nr "signal address v4-map-v6" 1 1 1
+chk_add_nr 1 1
+
+# subflow v6-map-v4
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add "::ffff:10.0.3.2" flags subflow
+run_tests $ns1 $ns2 10.0.1.1
+chk_join_nr "single subflow v6-map-v4" 1 1 1
+
+# signal address v6-map-v4
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+ip netns exec $ns1 ./pm_nl_ctl add "::ffff:10.0.2.1" flags signal
+run_tests $ns1 $ns2 10.0.1.1
+chk_join_nr "signal address v6-map-v4" 1 1 1
+chk_add_nr 1 1
+
+# no subflow IPv6 to v4 address
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add dead:beef:2::10.0.3.2 flags subflow
+run_tests $ns1 $ns2 "10.0.1.1"
+chk_join_nr "no JOIN with different families v4-v6" 0 0 0
+
+# no subflow IPv4 to v6 address
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
+run_tests $ns1 $ns2 dead:beef:1::1
+chk_join_nr "no JOIN with different families v6-v4" 0 0 0
+
# single subflow, backup
reset
ip netns exec $ns1 ./pm_nl_ctl limits 0 1
--
2.29.2
6:16 p.m.
New subject: [PATCH mptcp-next v4 2/3] mptcp: pm nl: support IPv4 mapped in v6 addresses
Hi Matt,
Thanks for your patches, and for your help.
Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月19日周六 下午9:07写道：
On one side, we can allow the creation of subflows between v4 mapped in
v6 and v4 addresses. For that we look for v4mapped addresses between the
local address we want to select and the remote one.
On the other side, we also properly deal with received v4mapped
addresses, either announced ones or set via Netlink.
Fixes: 01cacb00b35c ("mptcp: add netlink-based PM")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/122
Co-developed-by: Geliang Tang <geliangtang(a)gmail.com>
Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
---
net/mptcp/pm_netlink.c | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 9b1f6298bbdb..6ef00b3799d4 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -60,15 +60,20 @@ static bool addresses_equal(const struct mptcp_addr_info *a,
{
bool addr_equals = false;
- if (a->family != b->family)
- return false;
-
- if (a->family == AF_INET)
- addr_equals = a->addr.s_addr == b->addr.s_addr;
+ if (a->family == b->family) {
+ if (a->family == AF_INET)
+ addr_equals = a->addr.s_addr == b->addr.s_addr;
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
- else
- addr_equals = !ipv6_addr_cmp(&a->addr6, &b->addr6);
+ else
+ addr_equals = !ipv6_addr_cmp(&a->addr6,
&b->addr6);
+ } else if (a->family == AF_INET) {
+ if (ipv6_addr_v4mapped(&b->addr6))
+ addr_equals = a->addr.s_addr == b->addr6.s6_addr32[3];
+ } else if (b->family == AF_INET) {
+ if (ipv6_addr_v4mapped(&a->addr6))
+ addr_equals = a->addr6.s6_addr32[3] == b->addr.s_addr;
#endif
+ }
if (!addr_equals)
return false;
@@ -133,8 +138,8 @@ static bool lookup_subflow_by_saddr(const struct list_head *list,
}
static struct mptcp_pm_addr_entry *
-select_local_address(const struct pm_nl_pernet *pernet,
- struct mptcp_sock *msk)
+select_local_address(const struct pm_nl_pernet *pernet, struct mptcp_sock *msk,
+ struct mptcp_addr_info *remote)
{
struct mptcp_pm_addr_entry *entry, *ret = NULL;
I think we should avoid adding a new parameter here, we can call
remote_address again in this function to get the remote address.
@@ -144,11 +149,19 @@ select_local_address(const struct pm_nl_pernet
*pernet,
if (!(entry->addr.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
continue;
+ if (entry->addr.family != remote->family)
I think we should add a "{" here ...
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+ if ((entry->addr.family == AF_INET &&
+ !ipv6_addr_v4mapped(&remote->addr6)) ||
+ (remote->family == AF_INET &&
+ !ipv6_addr_v4mapped(&entry->addr.addr6)))
+#endif
+ continue;
... and add a "}" here.
-Geliang
> +
> /* avoid any address already in use by subflows and
> * pending join
> */
> - if (entry->addr.family == ((struct sock *)msk)->sk_family
&&
> - !lookup_subflow_by_saddr(&msk->conn_list,
&entry->addr)) {
> + if (!lookup_subflow_by_saddr(&msk->conn_list,
&entry->addr)) {
> ret = entry;
> break;
> }
> @@ -346,7 +359,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct
mptcp_sock *msk)
> msk->pm.subflows < msk->pm.subflows_max) {
> remote_address((struct sock_common *)sk, &remote);
>
> - local = select_local_address(pernet, msk);
> + local = select_local_address(pernet, msk, &remote);
> if (local) {
> msk->pm.local_addr_used++;
> msk->pm.subflows++;
> --
> 2.29.2
>
6:16 p.m.
New subject: [PATCH mptcp-next v4 3/3] selftests: mptcp: add IPv4-mapped IPv6 testcases
Hi Matt,
Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月19日周六 下午9:07写道：
From: Geliang Tang <geliangtang(a)gmail.com>
Here, we make sure we support IPv4-mapped in IPv6 addresses in different
contexts:
- a v4-mapped address is received by the PM and can be used as v4.
- a v4 address is received by the PM and can be used even with a v4
mapped socket.
We also make sure we don't try to establish subflows between v4 and v6
addresses, e.g. if a real v6 address ends with a valid v4 address.
Co-developed-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
---
.../testing/selftests/net/mptcp/mptcp_join.sh | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index f74cd993b168..eff813819e6a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -790,6 +790,73 @@ chk_join_nr "remove subflow and signal IPv6" 2 2 2
chk_add_nr 1 1
chk_rm_nr 1 1
+# subflow IPv4-mapped to IPv4-mapped
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add "::ffff:10.0.3.2" flags subflow
+run_tests $ns1 $ns2 "::ffff:10.0.1.1"
+chk_join_nr "single subflow IPv4-mapped" 1 1 1
+
+# signal address IPv4-mapped with IPv4-mapped sk
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+ip netns exec $ns1 ./pm_nl_ctl add "::ffff:10.0.2.1" flags signal
+run_tests $ns1 $ns2 "::ffff:10.0.1.1"
+chk_join_nr "signal address IPv4-mapped" 1 1 1
+chk_add_nr 1 1
+
+# subflow v4-map-v6
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
+run_tests $ns1 $ns2 "::ffff:10.0.1.1"
+chk_join_nr "single subflow v4-map-v6" 1 1 1
+
+# signal address v4-map-v6
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
+run_tests $ns1 $ns2 "::ffff:10.0.1.1"
+chk_join_nr "signal address v4-map-v6" 1 1 1
+chk_add_nr 1 1
+
+# subflow v6-map-v4
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add "::ffff:10.0.3.2" flags subflow
+run_tests $ns1 $ns2 10.0.1.1
+chk_join_nr "single subflow v6-map-v4" 1 1 1
+
+# signal address v6-map-v4
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 1 1
+ip netns exec $ns1 ./pm_nl_ctl add "::ffff:10.0.2.1" flags signal
+run_tests $ns1 $ns2 10.0.1.1
+chk_join_nr "signal address v6-map-v4" 1 1 1
+chk_add_nr 1 1
+
+# no subflow IPv6 to v4 address
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add dead:beef:2::10.0.3.2 flags subflow
dead:beef:2::10.0.3.2 is a invalid address, should we use dead:beef:3::2 here?
+run_tests $ns1 $ns2 "10.0.1.1"
We can drop the quotation marks here, only the IPv4-mapped address need
the quotation marks.
And add "0 0 0 slow" behind it, since the test need to slow down when use
the IPv6 address.
+chk_join_nr "no JOIN with different families v4-v6" 0 0 0
This string is too long, it break the alignment of the output. Can we use
"diff" here instead of "different"?
+
+# no subflow IPv4 to v6 address
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl limits 0 1
+ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
+run_tests $ns1 $ns2 dead:beef:1::1
Add "0 0 0 slow" here.
+chk_join_nr "no JOIN with different families v6-v4" 0 0 0
This string is too long too.
-Geliang
> +
> # single subflow, backup
> reset
> ip netns exec $ns1 ./pm_nl_ctl limits 0 1
> --
> 2.29.2
>
7:48 p.m.
New subject: [PATCH mptcp-next v4 2/3] mptcp: pm nl: support IPv4 mapped in v6 addresses
Hi Geliang,
Thank you for reviewing these patches!
On 21/12/2020 11:16, Geliang Tang wrote:
Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月19日周六
下午9:07写道：
>
> On one side, we can allow the creation of subflows between v4 mapped in
> v6 and v4 addresses. For that we look for v4mapped addresses between the
> local address we want to select and the remote one.
>
> On the other side, we also properly deal with received v4mapped
> addresses, either announced ones or set via Netlink.
(...)
> @@ -133,8 +138,8 @@ static bool lookup_subflow_by_saddr(const
struct list_head *list,
> }
>
> static struct mptcp_pm_addr_entry *
> -select_local_address(const struct pm_nl_pernet *pernet,
> - struct mptcp_sock *msk)
> +select_local_address(const struct pm_nl_pernet *pernet, struct mptcp_sock *msk,
> + struct mptcp_addr_info *remote)
> {
> struct mptcp_pm_addr_entry *entry, *ret = NULL;
>
I think we should avoid adding a new parameter here, we can call
remote_address again in this function to get the remote address.
I agree we can build "remote" from the msk but I didn't do it because
select_local_address() is only used at one place just after having built
this "remote".
Do you think we still should rebuild "remote"?
> @@ -144,11 +149,19 @@ select_local_address(const struct
pm_nl_pernet *pernet,
> if (!(entry->addr.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
> continue;
>
> + if (entry->addr.family != remote->family)
I think we should add a "{" here ...
Do you mean I should add them to avoid ambiguities? Checkpatch and the
compiler are OK with both but I don't mind changing if it is clearer.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
7:49 p.m.
New subject: [PATCH mptcp-next v4 3/3] selftests: mptcp: add IPv4-mapped IPv6 testcases
Hi Geliang,
On 21/12/2020 11:16, Geliang Tang wrote:
Hi Matt,
Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月19日周六 下午9:07写道：
>
> From: Geliang Tang <geliangtang(a)gmail.com>
>
> Here, we make sure we support IPv4-mapped in IPv6 addresses in different
> contexts:
(...)
> +# no subflow IPv6 to v4 address
> +reset
> +ip netns exec $ns1 ./pm_nl_ctl limits 0 1
> +ip netns exec $ns2 ./pm_nl_ctl limits 0 1
> +ip netns exec $ns2 ./pm_nl_ctl add dead:beef:2::10.0.3.2 flags subflow
dead:beef:2::10.0.3.2 is a invalid address, should we use dead:beef:3::2 here?
I used "dead:beef:2::10.0.3.2" on purpose just to have a valid and
usable IPv4 address at the end of the v6 one. If we try to use the IPv4
address at the end of this v6 one, we will send the SYN+MPJ to the
correct host (no routing issue). With the previous version of this
patchset (v3), this was creating a subflow with 10.0.3.2!
I should certainly add a new comment here.
Maybe you would prefer an additional test with "dead:beef:3::2" as well?
> +run_tests $ns1 $ns2 "10.0.1.1"
We can drop the quotation marks here, only the IPv4-mapped address need
the quotation marks.
Yes, I usually add quotes in Bash for each string but I see we didn't do
that for the other tests, I will remove that!
And add "0 0 0 slow" behind it, since the test need to slow
down when use
the IPv6 address.
Do I need to do that here as well even if we don't create v6 subflows?
> +chk_join_nr "no JOIN with different families v4-v6" 0
0 0
This string is too long, it break the alignment of the output. Can we use
"diff" here instead of "different"?
Good point, I didn't check that! I will short "different", good idea!
> +
> +# no subflow IPv4 to v6 address
> +reset
> +ip netns exec $ns1 ./pm_nl_ctl limits 0 1
> +ip netns exec $ns2 ./pm_nl_ctl limits 0 1
> +ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
> +run_tests $ns1 $ns2 dead:beef:1::1
Add "0 0 0 slow" here.
Same here, is it needed if we don't create any IPv6 subflow?
> +chk_join_nr "no JOIN with different families v6-v4" 0
0 0
This string is too long too.
Will do in v5!
Thanks for the review!
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
8 p.m.
New subject: [PATCH mptcp-next v4 2/3] mptcp: pm nl: support IPv4 mapped in v6 addresses
Hi Matt,
Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月21日周一 下午7:48写道：
Hi Geliang,
Thank you for reviewing these patches!
On 21/12/2020 11:16, Geliang Tang wrote:
> Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月19日周六 下午9:07写道：
>>
>> On one side, we can allow the creation of subflows between v4 mapped in
>> v6 and v4 addresses. For that we look for v4mapped addresses between the
>> local address we want to select and the remote one.
>>
>> On the other side, we also properly deal with received v4mapped
>> addresses, either announced ones or set via Netlink.
(...)
>> @@ -133,8 +138,8 @@ static bool lookup_subflow_by_saddr(const struct list_head
*list,
>> }
>>
>> static struct mptcp_pm_addr_entry *
>> -select_local_address(const struct pm_nl_pernet *pernet,
>> - struct mptcp_sock *msk)
>> +select_local_address(const struct pm_nl_pernet *pernet, struct mptcp_sock
*msk,
>> + struct mptcp_addr_info *remote)
>> {
>> struct mptcp_pm_addr_entry *entry, *ret = NULL;
>>
>
> I think we should avoid adding a new parameter here, we can call
> remote_address again in this function to get the remote address.
I agree we can build "remote" from the msk but I didn't do it because
select_local_address() is only used at one place just after having built
this "remote".
Do you think we still should rebuild "remote"?
I think we should use as few function parameters as possible.
>> @@ -144,11 +149,19 @@ select_local_address(const struct pm_nl_pernet *pernet,
>> if (!(entry->addr.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
>> continue;
>>
>> + if (entry->addr.family != remote->family)
>
> I think we should add a "{" here ...
Do you mean I should add them to avoid ambiguities? Checkpatch and the
compiler are OK with both but I don't mind changing if it is clearer.
Yes, we should avoid ambiguities here.
Thanks.
-Geliang
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
8:10 p.m.
New subject: [PATCH mptcp-next v4 3/3] selftests: mptcp: add IPv4-mapped IPv6 testcases
Hi Matt,
Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月21日周一 下午7:49写道：
Hi Geliang,
On 21/12/2020 11:16, Geliang Tang wrote:
> Hi Matt,
>
> Matthieu Baerts <matthieu.baerts(a)tessares.net> 于2020年12月19日周六 下午9:07写道：
>>
>> From: Geliang Tang <geliangtang(a)gmail.com>
>>
>> Here, we make sure we support IPv4-mapped in IPv6 addresses in different
>> contexts:
(...)
>> +# no subflow IPv6 to v4 address
>> +reset
>> +ip netns exec $ns1 ./pm_nl_ctl limits 0 1
>> +ip netns exec $ns2 ./pm_nl_ctl limits 0 1
>> +ip netns exec $ns2 ./pm_nl_ctl add dead:beef:2::10.0.3.2 flags subflow
>
> dead:beef:2::10.0.3.2 is a invalid address, should we use dead:beef:3::2 here?
I used "dead:beef:2::10.0.3.2" on purpose just to have a valid and
usable IPv4 address at the end of the v6 one. If we try to use the IPv4
address at the end of this v6 one, we will send the SYN+MPJ to the
correct host (no routing issue). With the previous version of this
patchset (v3), this was creating a subflow with 10.0.3.2!
Sorry, I didn't realize you did it on purpose, please keep it here.
I should certainly add a new comment here.
Maybe you would prefer an additional test with "dead:beef:3::2" as well?
Please add this additional test.
>> +run_tests $ns1 $ns2 "10.0.1.1"
>
> We can drop the quotation marks here, only the IPv4-mapped address need
> the quotation marks.
Yes, I usually add quotes in Bash for each string but I see we didn't do
that for the other tests, I will remove that!
> And add "0 0 0 slow" behind it, since the test need to slow down when use
> the IPv6 address.
Do I need to do that here as well even if we don't create v6 subflows?
I think this can be consistent with other IPv6 tests, but it's up to you :)
Thanks.
-Geliang
>
> >> +chk_join_nr "no JOIN with different families v4-v6" 0 0 0
> >
> > This string is too long, it break the alignment of the output. Can we use
> > "diff" here instead of "different"?
>
> Good point, I didn't check that! I will short "different", good idea!
>
> >> +
> >> +# no subflow IPv4 to v6 address
> >> +reset
> >> +ip netns exec $ns1 ./pm_nl_ctl limits 0 1
> >> +ip netns exec $ns2 ./pm_nl_ctl limits 0 1
> >> +ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
> >> +run_tests $ns1 $ns2 dead:beef:1::1
> >
> > Add "0 0 0 slow" here.
>
> Same here, is it needed if we don't create any IPv6 subflow?
>
> >> +chk_join_nr "no JOIN with different families v6-v4" 0 0 0
> >
> > This string is too long too.
>
> Will do in v5!
>
> Thanks for the review!
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
122
days inactive
124
days old
9 comments
2 participants
participants (2)
-
Geliang Tang -
Matthieu Baerts