| Message ID | bde1048019780ecc5f2e553bbb4eb21213f76ba7.1627372396.git.geliangtang@xiaomi.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mat Martineau |
| Headers | show |
| Series | fullmesh path manager support | expand |
On Tue, 2021-07-27 at 15:58 +0800, Geliang Tang wrote: > From: Geliang Tang <geliangtang@xiaomi.com> > > In mptcp_pm_nl_add_addr_received(), fill a temporary allocate array of > all local address corresponding to the fullmesh endpoint. If such array > is empty, keep the current behavior. > > Elsewhere loop on such array and create a subflow for each local address > towards the given remote address > > Suggested-by: Paolo Abeni <pabeni@redhat.com> > Signed-off-by: Geliang Tang <geliangtang@xiaomi.com> > --- > net/mptcp/pm_netlink.c | 60 ++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 55 insertions(+), 5 deletions(-) > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index 2259c424485f..a85bac950f3b 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -554,13 +554,62 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk) > mptcp_pm_create_subflow_or_signal_addr(msk); > } > > +static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk, > + struct mptcp_addr_info *remote, > + struct mptcp_pm_addr_entry *entries) Minor nit: some comments here before the function describing it would be helpful, thanks! Also 'remote' could be 'const', I think. > +{ > + struct mptcp_pm_addr_entry local, *entry; > + struct sock *sk = (struct sock *)msk; > + struct pm_nl_pernet *pernet; > + unsigned int subflows_max; > + int i = 0; > + > + pernet = net_generic(sock_net(sk), pm_nl_pernet_id); > + subflows_max = mptcp_pm_get_subflows_max(msk); > + > + rcu_read_lock(); > + __mptcp_flush_join_list(msk); > + list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) { > + 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 (!lookup_subflow_by_addrs(&msk->conn_list, &entry->addr, remote) && > + msk->pm.subflows < subflows_max) { > + msk->pm.subflows++; > + entries[i++] = *entry; > + } > + } > + rcu_read_unlock(); > + > + if (!i) { > + memset(&local, 0, sizeof(local)); > + local.addr.family = remote->family; > + > + msk->pm.subflows++; > + entries[i++] = local; > + } > + > + return i; > +} > + > static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) > { > + struct mptcp_pm_addr_entry entries[MPTCP_PM_ADDR_MAX]; mptcp_pm_addr_entry is quite larger than mptcp_addr_info (should be 64 bytes vs 24). 64 * 8 == 512 bytes could be a bit too much storage for the stack. What about using instead: struct mptcp_pm_addr_info addresses[MPTCP_PM_ADDR_MAX]; u8 flags[MPTCP_PM_ADDR_MAX]; ? And than pass the 2 arguments to fill_local_addresses_vec(), instead of the single 'entries' arg. Cheers, Paolo
Paolo Abeni <pabeni@redhat.com> 于2021年7月27日周二 下午5:52写道： > > On Tue, 2021-07-27 at 15:58 +0800, Geliang Tang wrote: > > From: Geliang Tang <geliangtang@xiaomi.com> > > > > In mptcp_pm_nl_add_addr_received(), fill a temporary allocate array of > > all local address corresponding to the fullmesh endpoint. If such array > > is empty, keep the current behavior. > > > > Elsewhere loop on such array and create a subflow for each local address > > towards the given remote address > > > > Suggested-by: Paolo Abeni <pabeni@redhat.com> > > Signed-off-by: Geliang Tang <geliangtang@xiaomi.com> > > --- > > net/mptcp/pm_netlink.c | 60 ++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 55 insertions(+), 5 deletions(-) > > > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > > index 2259c424485f..a85bac950f3b 100644 > > --- a/net/mptcp/pm_netlink.c > > +++ b/net/mptcp/pm_netlink.c > > @@ -554,13 +554,62 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk) > > mptcp_pm_create_subflow_or_signal_addr(msk); > > } > > > > +static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk, > > + struct mptcp_addr_info *remote, > > + struct mptcp_pm_addr_entry *entries) > > Minor nit: some comments here before the function describing it would > be helpful, thanks! Sure. added in v6. > > Also 'remote' could be 'const', I think. > > > +{ > > + struct mptcp_pm_addr_entry local, *entry; > > + struct sock *sk = (struct sock *)msk; > > + struct pm_nl_pernet *pernet; > > + unsigned int subflows_max; > > + int i = 0; > > + > > + pernet = net_generic(sock_net(sk), pm_nl_pernet_id); > > + subflows_max = mptcp_pm_get_subflows_max(msk); > > + > > + rcu_read_lock(); > > + __mptcp_flush_join_list(msk); > > + list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) { > > + 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 (!lookup_subflow_by_addrs(&msk->conn_list, &entry->addr, remote) && > > + msk->pm.subflows < subflows_max) { > > + msk->pm.subflows++; > > + entries[i++] = *entry; > > + } > > + } > > + rcu_read_unlock(); > > + > > + if (!i) { > > + memset(&local, 0, sizeof(local)); > > + local.addr.family = remote->family; > > + > > + msk->pm.subflows++; > > + entries[i++] = local; > > + } > > + > > + return i; > > +} > > + > > static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) > > { > > + struct mptcp_pm_addr_entry entries[MPTCP_PM_ADDR_MAX]; > > mptcp_pm_addr_entry is quite larger than mptcp_addr_info (should be 64 > bytes vs 24). 64 * 8 == 512 bytes could be a bit too much storage for > the stack. What about using instead: > > struct mptcp_pm_addr_info addresses[MPTCP_PM_ADDR_MAX]; > u8 flags[MPTCP_PM_ADDR_MAX]; > > ? > > And than pass the 2 arguments to fill_local_addresses_vec(), instead of > the single 'entries' arg. Pass 3 arguments 'addr', 'flags' and 'ifindex' instead of 'entries' in v6. Thanks, -Geliang > > Cheers, > > Paolo >
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 2259c424485f..a85bac950f3b 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -554,13 +554,62 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk) mptcp_pm_create_subflow_or_signal_addr(msk); } +static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk, + struct mptcp_addr_info *remote, + struct mptcp_pm_addr_entry *entries) +{ + struct mptcp_pm_addr_entry local, *entry; + struct sock *sk = (struct sock *)msk; + struct pm_nl_pernet *pernet; + unsigned int subflows_max; + int i = 0; + + pernet = net_generic(sock_net(sk), pm_nl_pernet_id); + subflows_max = mptcp_pm_get_subflows_max(msk); + + rcu_read_lock(); + __mptcp_flush_join_list(msk); + list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) { + 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 (!lookup_subflow_by_addrs(&msk->conn_list, &entry->addr, remote) && + msk->pm.subflows < subflows_max) { + msk->pm.subflows++; + entries[i++] = *entry; + } + } + rcu_read_unlock(); + + if (!i) { + memset(&local, 0, sizeof(local)); + local.addr.family = remote->family; + + msk->pm.subflows++; + entries[i++] = local; + } + + return i; +} + static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) { + struct mptcp_pm_addr_entry entries[MPTCP_PM_ADDR_MAX]; struct sock *sk = (struct sock *)msk; unsigned int add_addr_accept_max; struct mptcp_addr_info remote; - struct mptcp_addr_info local; unsigned int subflows_max; + int i, nr; add_addr_accept_max = mptcp_pm_get_add_addr_accept_max(msk); subflows_max = mptcp_pm_get_subflows_max(msk); @@ -584,11 +633,12 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) remote = msk->pm.remote; if (!remote.port) remote.port = sk->sk_dport; - memset(&local, 0, sizeof(local)); - local.family = remote.family; - + nr = fill_local_addresses_vec(msk, &remote, entries); spin_unlock_bh(&msk->pm.lock); - __mptcp_subflow_connect(sk, &local, &remote, 0, 0); + for (i = 0; i < nr; i++) { + __mptcp_subflow_connect(sk, &entries[i].addr, &remote, + entries[i].flags, entries[i].ifindex); + } spin_lock_bh(&msk->pm.lock); add_addr_echo: