Skip to content

Commit

Permalink
mptcp: pm: reduce entries iterations on connect
Browse files Browse the repository at this point in the history 
__mptcp_subflow_connect() is currently called from the path-managers,
which have all the required information to create subflows. No need to
call the PM again to re-iterate over the list of entries with RCU lock
to get more info.

Instead, it is possible to pass a mptcp_pm_addr_entry structure, instead
of a mptcp_addr_info one. The former contains the ifindex and the flags
that are required when creating the new subflow.

This is a partial revert of commit ee28525 ("mptcp: drop flags and
ifindex arguments").

While at it, the local ID can also be set if it is known and 0, to avoid
having to set it in the 'rebuild_header' hook, which will cause a new
iteration of the endpoint entries.

Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
  • Loading branch information
@matttbe @intel-lab-lkp
matttbe authored and intel-lab-lkp committed Jul 22, 2024
1 parent c47601d commit c311948
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 85 deletions.
11 changes: 0 additions & 11 deletions net/mptcp/pm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,17 +416,6 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
return mptcp_pm_nl_get_local_id(msk, &skc_local);
}

int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
u8 *flags, int *ifindex)
{
*flags = 0;
*ifindex = 0;

if (mptcp_pm_is_userspace(msk))
return mptcp_userspace_pm_get_flags_and_ifindex_by_id(msk, id, flags, ifindex);
return mptcp_pm_nl_get_flags_and_ifindex_by_id(msk, id, flags, ifindex);
}

int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
{
if (info->attrs[MPTCP_PM_ATTR_TOKEN])
Expand Down
48 changes: 12 additions & 36 deletions net/mptcp/pm_netlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)

spin_unlock_bh(&msk->pm.lock);
for (i = 0; i < nr; i++)
__mptcp_subflow_connect(sk, &local.addr, &addrs[i]);
__mptcp_subflow_connect(sk, &local, &addrs[i]);
spin_lock_bh(&msk->pm.lock);
}
mptcp_pm_nl_check_work_pending(msk);
Expand All @@ -646,7 +646,7 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk)
*/
static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
struct mptcp_addr_info *remote,
struct mptcp_addr_info *addrs)
struct mptcp_pm_addr_entry *entries)
{
struct sock *sk = (struct sock *)msk;
struct mptcp_pm_addr_entry *entry;
Expand All @@ -670,14 +670,14 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
continue;

if (msk->pm.subflows < subflows_max) {
msk->pm.subflows++;
addrs[i] = entry->addr;
memcpy(&entries[i], entry, sizeof(entries[i]));

/* Special case for ID0: set the correct ID */
if (msk->first &&
mptcp_addresses_equal(&entry->addr, &mpc_addr, entry->addr.port))
addrs[i].id = 0;
entries[i].addr.id = 0;

msk->pm.subflows++;
i++;
}
}
Expand All @@ -687,29 +687,27 @@ 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 =
memset(&entries[i], 0, sizeof(entries[i]));
entries[i].addr.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))
if (!mptcp_pm_addr_families_match(sk, &entries[i].addr, remote))
return 0;

msk->pm.subflows++;
addrs[i++] = local;
i++;
}

return i;
}

static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
{
struct mptcp_addr_info addrs[MPTCP_PM_ADDR_MAX];
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;
Expand Down Expand Up @@ -738,13 +736,13 @@ 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, &remote, addrs);
nr = fill_local_addresses_vec(msk, &remote, entries);
if (nr == 0)
return;

spin_unlock_bh(&msk->pm.lock);
for (i = 0; i < nr; i++)
if (__mptcp_subflow_connect(sk, &addrs[i], &remote) == 0)
if (__mptcp_subflow_connect(sk, &entries[i], &remote) == 0)
sf_created = true;
spin_lock_bh(&msk->pm.lock);

Expand Down Expand Up @@ -1395,28 +1393,6 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
return ret;
}

int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
u8 *flags, int *ifindex)
{
struct mptcp_pm_addr_entry *entry;
struct sock *sk = (struct sock *)msk;
struct net *net = sock_net(sk);

/* No entries with ID 0 */
if (id == 0)
return 0;

rcu_read_lock();
entry = __lookup_addr_by_id(pm_nl_get_pernet(net), id);
if (entry) {
*flags = entry->flags;
*ifindex = entry->ifindex;
}
rcu_read_unlock();

return 0;
}

static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr)
{
Expand Down
19 changes: 1 addition & 18 deletions net/mptcp/pm_userspace.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,23 +119,6 @@ mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id)
return NULL;
}

int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
unsigned int id,
u8 *flags, int *ifindex)
{
struct mptcp_pm_addr_entry *match;

spin_lock_bh(&msk->pm.lock);
match = mptcp_userspace_pm_lookup_addr_by_id(msk, id);
spin_unlock_bh(&msk->pm.lock);
if (match) {
*flags = match->flags;
*ifindex = match->ifindex;
}

return 0;
}

int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
struct mptcp_addr_info *skc)
{
Expand Down Expand Up @@ -394,7 +377,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)

lock_sock(sk);

err = __mptcp_subflow_connect(sk, &local.addr, &addr_r);
err = __mptcp_subflow_connect(sk, &local, &addr_r);

release_sock(sk);

Expand Down
10 changes: 1 addition & 9 deletions net/mptcp/protocol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ bool mptcp_addresses_equal(const struct mptcp_addr_info *a,
void mptcp_local_address(const struct sock_common *skc, struct mptcp_addr_info *addr);

/* called with sk socket lock held */
int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
const struct mptcp_addr_info *remote);
int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
struct socket **new_sock);
Expand Down Expand Up @@ -1015,14 +1015,6 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
struct mptcp_pm_add_entry *
mptcp_lookup_anno_list_by_saddr(const struct mptcp_sock *msk,
const struct mptcp_addr_info *addr);
int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
unsigned int id,
u8 *flags, int *ifindex);
int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
u8 *flags, int *ifindex);
int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
unsigned int id,
u8 *flags, int *ifindex);
int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info);
int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
Expand Down
29 changes: 18 additions & 11 deletions net/mptcp/subflow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1544,26 +1544,24 @@ void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
#endif
}

int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
const struct mptcp_addr_info *remote)
{
struct mptcp_sock *msk = mptcp_sk(sk);
struct mptcp_subflow_context *subflow;
int local_id = local->addr.id;
struct sockaddr_storage addr;
int remote_id = remote->id;
int local_id = loc->id;
int err = -ENOTCONN;
struct socket *sf;
struct sock *ssk;
u32 remote_token;
int addrlen;
int ifindex;
u8 flags;

if (!mptcp_is_fully_established(sk))
goto err_out;

err = mptcp_subflow_create_socket(sk, loc->family, &sf);
err = mptcp_subflow_create_socket(sk, local->addr.family, &sf);
if (err)
goto err_out;

Expand All @@ -1573,23 +1571,32 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
get_random_bytes(&subflow->local_nonce, sizeof(u32));
} while (!subflow->local_nonce);

if (local_id)
/* if 'IPADDRANY', the ID will be set later, after the routing */
if (local->addr.family == AF_INET) {
if (!local->addr.addr.s_addr)
local_id = -1;
#if IS_ENABLED(CONFIG_IPV6)
} else if (sk->sk_family == AF_INET6) {
if (ipv6_addr_any(&local->addr.addr6))
local_id = -1;
#endif
}

if (local_id >= 0)
subflow_set_local_id(subflow, local_id);

mptcp_pm_get_flags_and_ifindex_by_id(msk, local_id,
&flags, &ifindex);
subflow->remote_key_valid = 1;
subflow->remote_key = READ_ONCE(msk->remote_key);
subflow->local_key = READ_ONCE(msk->local_key);
subflow->token = msk->token;
mptcp_info2sockaddr(loc, &addr, ssk->sk_family);
mptcp_info2sockaddr(&local->addr, &addr, ssk->sk_family);

addrlen = sizeof(struct sockaddr_in);
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
if (addr.ss_family == AF_INET6)
addrlen = sizeof(struct sockaddr_in6);
#endif
ssk->sk_bound_dev_if = ifindex;
ssk->sk_bound_dev_if = local->ifindex;
err = kernel_bind(sf, (struct sockaddr *)&addr, addrlen);
if (err)
goto failed;
Expand All @@ -1600,7 +1607,7 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
subflow->remote_token = remote_token;
WRITE_ONCE(subflow->remote_id, remote_id);
subflow->request_join = 1;
subflow->request_bkup = !!(flags & MPTCP_PM_ADDR_FLAG_BACKUP);
subflow->request_bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
subflow->subflow_id = msk->subflow_id++;
mptcp_info2sockaddr(remote, &addr, ssk->sk_family);

Expand Down

0 comments on commit c311948

Please sign in to comment.