Currently the in kernel PM arbitrary enforces that created subflow's
family must match the main MPTCP socket. The RFC allow instead for
mixing IPv4 and IPv6 subflows with no constraint.
This patch changes the in kernel PM logic to create subflows matching
the currently selected source (or destination) address, effectively
allowing the creation of IPv4 and IPv6 subflows under the same msk.
While at that, factor out a new helper for address family matching,
taking care of ipv4 vs ipv4-mapped-ipv6.
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/269
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/pm_netlink.c | 57 ++++++++++++++++++++++--------------------
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index f2a43e13bacd..1d4ba6716a98 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;
}
@@ -420,10 +409,25 @@ static bool lookup_address_in_vec(const struct mptcp_addr_info *addrs, unsigned
return false;
}
+static bool addr_families_match(const struct mptcp_addr_info *a,
+ const struct mptcp_addr_info *b)
+{
+ if (a->family == b->family)
+ return true;
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+ if ((a->family == AF_INET && ipv6_addr_v4mapped(&b->addr6)) ||
+ (b->family == AF_INET && ipv6_addr_v4mapped(&a->addr6)))
+ return true;
+#endif
+ return false;
+}
+
/* 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 +447,9 @@ static unsigned int fill_remote_addresses_vec(struct mptcp_sock *msk, bool fullm
if (deny_id0)
return 0;
+ if (!addr_families_match(local, &remote))
+ return 0;
+
msk->pm.subflows++;
addrs[i++] = remote;
} else {
@@ -453,6 +460,9 @@ static unsigned int fill_remote_addresses_vec(struct mptcp_sock *msk, bool fullm
if (deny_id0 && !addrs[i].id)
continue;
+ if (!addr_families_match(local, &addrs[i]))
+ continue;
+
if (!lookup_address_in_vec(addrs, i, &addrs[i]) &&
msk->pm.subflows < subflows_max) {
msk->pm.subflows++;
@@ -600,10 +610,10 @@ 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);
+ nr = fill_remote_addresses_vec(msk, &local->addr, fullmesh, addrs);
+ __clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
spin_unlock_bh(&msk->pm.lock);
+
for (i = 0; i < nr; i++)
__mptcp_subflow_connect(sk, &local->addr, &addrs[i]);
spin_lock_bh(&msk->pm.lock);
@@ -625,9 +635,9 @@ 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;
@@ -642,15 +652,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 (!addr_families_match(&entry->addr, remote))
+ continue;
if (msk->pm.subflows < subflows_max) {
msk->pm.subflows++;
@@ -664,7 +667,7 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
*/
if (!i) {
memset(&local, 0, sizeof(local));
- local.family = msk->pm.remote.family;
+ local.family = remote->family;
msk->pm.subflows++;
addrs[i++] = local;
@@ -703,7 +706,7 @@ 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);
msk->pm.add_addr_accepted++;
if (msk->pm.add_addr_accepted >= add_addr_accept_max ||
--
2.38.1Hi Paolo, Thank you for having looked at these patches! On 23/12/2022 13:51, Paolo Abeni wrote: > Currently the in kernel PM arbitrary enforces that created subflow's > family must match the main MPTCP socket. The RFC allow instead for > mixing IPv4 and IPv6 subflows with no constraint. > > This patch changes the in kernel PM logic to create subflows matching > the currently selected source (or destination) address, effectively > allowing the creation of IPv4 and IPv6 subflows under the same msk. > > While at that, factor out a new helper for address family matching, > taking care of ipv4 vs ipv4-mapped-ipv6. > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/269 > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > net/mptcp/pm_netlink.c | 57 ++++++++++++++++++++++-------------------- > 1 file changed, 30 insertions(+), 27 deletions(-) > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index f2a43e13bacd..1d4ba6716a98 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c (...) > @@ -600,10 +610,10 @@ 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); > + nr = fill_remote_addresses_vec(msk, &local->addr, fullmesh, addrs); > + __clear_bit(local->addr.id, msk->pm.id_avail_bitmap); I see that you removed the 'if (nr)' condition: was there a bug before? If no addresses have been filled, no new subflows will be created below and this ID is still available. > spin_unlock_bh(&msk->pm.lock); > + > for (i = 0; i < nr; i++) > __mptcp_subflow_connect(sk, &local->addr, &addrs[i]); > spin_lock_bh(&msk->pm.lock); Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
On Fri, 2022-12-23 at 15:35 +0100, Matthieu Baerts wrote: > Hi Paolo, > > Thank you for having looked at these patches! > > On 23/12/2022 13:51, Paolo Abeni wrote: > > Currently the in kernel PM arbitrary enforces that created subflow's > > family must match the main MPTCP socket. The RFC allow instead for > > mixing IPv4 and IPv6 subflows with no constraint. > > > > This patch changes the in kernel PM logic to create subflows matching > > the currently selected source (or destination) address, effectively > > allowing the creation of IPv4 and IPv6 subflows under the same msk. > > > > While at that, factor out a new helper for address family matching, > > taking care of ipv4 vs ipv4-mapped-ipv6. > > > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/269 > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > --- > > net/mptcp/pm_netlink.c | 57 ++++++++++++++++++++++-------------------- > > 1 file changed, 30 insertions(+), 27 deletions(-) > > > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > > index f2a43e13bacd..1d4ba6716a98 100644 > > --- a/net/mptcp/pm_netlink.c > > +++ b/net/mptcp/pm_netlink.c > > (...) > > > @@ -600,10 +610,10 @@ 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); > > + nr = fill_remote_addresses_vec(msk, &local->addr, fullmesh, addrs); > > + __clear_bit(local->addr.id, msk->pm.id_avail_bitmap); > > I see that you removed the 'if (nr)' condition: was there a bug before? > If no addresses have been filled, no new subflows will be created below > and this ID is still available. The previous behavior was uncorrect, I think. It's cleaner if we always consider consumed an endpoint once we tried to process it, but I can't easily think of a scenario demonstrating a real "bug". I think we can squash the change in here. /P
© 2016 - 2026 Red Hat, Inc.