Skip to content

Commit

Permalink
mptcp: pm: fix backup support in signal endpoints
Browse files Browse the repository at this point in the history 
There was a support for signal endpoints, but only when the endpoint's
flag was changed during a connection. If an endpoint with the signal and
backup was already present, the MP_JOIN reply was not containing the
backup flag as expected.

That's confusing to have this inconsistent behaviour. On the other hand,
the infrastructure to set the backup flag in the SYN + ACK + MP_JOIN was
already there, it was just never set before. Now when requesting the
local ID from the path-manager, the backup status is also requested.

Note that when the userspace PM is used, the backup flag can be set if
the local address was already used before with a backup flag, e.g. if
the address was announced with the 'backup' flag, or a subflow was
created with the 'backup' flag.

The MPTCP Join selftest has been modified to validate this case: the
test "single address, backup", is now validating the MPJ with a backup
flag. The previous version has been kept, but renamed to "single
address, switch to backup" to avoid confusions. The test "single address
with port, backup" is also now validating the MPJ with a backup flag,
which makes more sense.

Fixes: 4596a2c ("mptcp: allow creating non-backup subflows")
Closes: multipath-tcp/mptcp_net-next#507
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
  • Loading branch information
@matttbe @intel-lab-lkp
matttbe authored and intel-lab-lkp committed Jul 11, 2024
1 parent 096fa4b commit 47f60d9
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 14 deletions.
9 changes: 6 additions & 3 deletions net/mptcp/pm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,14 +405,17 @@ bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
return ret;
}

int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc,
bool *backup)
{
struct mptcp_addr_info skc_local;
struct mptcp_addr_info msk_local;

if (WARN_ON_ONCE(!msk))
return -1;

*backup = false;

/* The 0 ID mapping is defined by the first subflow, copied into the msk
* addr
*/
Expand All @@ -422,8 +425,8 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
return 0;

if (mptcp_pm_is_userspace(msk))
return mptcp_userspace_pm_get_local_id(msk, &skc_local);
return mptcp_pm_nl_get_local_id(msk, &skc_local);
return mptcp_userspace_pm_get_local_id(msk, &skc_local, backup);
return mptcp_pm_nl_get_local_id(msk, &skc_local, backup);
}

int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
Expand Down
4 changes: 3 additions & 1 deletion net/mptcp/pm_netlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,8 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
return err;
}

int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc,
bool *backup)
{
struct mptcp_pm_addr_entry *entry;
struct pm_nl_pernet *pernet;
Expand All @@ -1076,6 +1077,7 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc
list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
if (mptcp_addresses_equal(&entry->addr, skc, entry->addr.port)) {
ret = entry->addr.id;
*backup = !!(entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
break;
}
}
Expand Down
6 changes: 4 additions & 2 deletions net/mptcp/pm_userspace.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
}

int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
struct mptcp_addr_info *skc)
struct mptcp_addr_info *skc, bool *backup)
{
struct mptcp_pm_addr_entry *entry = NULL, *e, new_entry;
__be16 msk_sport = ((struct inet_sock *)
Expand All @@ -151,8 +151,10 @@ int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
}
}
spin_unlock_bh(&msk->pm.lock);
if (entry)
if (entry) {
*backup = !!(entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
return entry->addr.id;
}

memset(&new_entry, 0, sizeof(struct mptcp_pm_addr_entry));
new_entry.addr = *skc;
Expand Down
9 changes: 6 additions & 3 deletions net/mptcp/protocol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1111,9 +1111,12 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb,
bool *drop_other_suboptions);
bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
struct mptcp_rm_list *rm_list);
int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc,
bool *backup);
int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc,
bool *backup);
int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc,
bool *backup);
int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb);
int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
struct netlink_callback *cb);
Expand Down
7 changes: 5 additions & 2 deletions net/mptcp/subflow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,21 @@ static struct mptcp_sock *subflow_token_join_request(struct request_sock *req)
struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
struct mptcp_sock *msk;
int local_id;
bool backup;

msk = mptcp_token_get_sock(sock_net(req_to_sk(req)), subflow_req->token);
if (!msk) {
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINNOTOKEN);
return NULL;
}

local_id = mptcp_pm_get_local_id(msk, (struct sock_common *)req);
local_id = mptcp_pm_get_local_id(msk, (struct sock_common *)req, &backup);
if (local_id < 0) {
sock_put((struct sock *)msk);
return NULL;
}
subflow_req->local_id = local_id;
subflow_req->request_bkup = backup;

return msk;
}
Expand Down Expand Up @@ -604,12 +606,13 @@ static int subflow_chk_local_id(struct sock *sk)
{
struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
struct mptcp_sock *msk = mptcp_sk(subflow->conn);
bool backup;
int err;

if (likely(subflow->local_id >= 0))
return 0;

err = mptcp_pm_get_local_id(msk, (struct sock_common *)sk);
err = mptcp_pm_get_local_id(msk, (struct sock_common *)sk, &backup);
if (err < 0)
return err;

Expand Down
19 changes: 16 additions & 3 deletions tools/testing/selftests/net/mptcp/mptcp_join.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2617,6 +2617,19 @@ backup_tests()

# single address, backup
if reset "single address, backup" &&
continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
pm_nl_set_limits $ns1 0 1
pm_nl_add_endpoint $ns1 10.0.2.1 flags signal,backup
pm_nl_set_limits $ns2 1 1
sflags=nobackup speed=slow \
run_tests $ns1 $ns2 10.0.1.1
chk_join_nr 1 1 1
chk_add_nr 1 1
chk_prio_nr 1 0
fi

# single address, switch to backup
if reset "single address, switch to backup" &&
continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
pm_nl_set_limits $ns1 0 1
pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
Expand All @@ -2632,13 +2645,13 @@ backup_tests()
if reset "single address with port, backup" &&
continue_if mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
pm_nl_set_limits $ns1 0 1
pm_nl_add_endpoint $ns1 10.0.2.1 flags signal port 10100
pm_nl_add_endpoint $ns1 10.0.2.1 flags signal,backup port 10100
pm_nl_set_limits $ns2 1 1
sflags=backup speed=slow \
sflags=nobackup speed=slow \
run_tests $ns1 $ns2 10.0.1.1
chk_join_nr 1 1 1
chk_add_nr 1 1
chk_prio_nr 1 1
chk_prio_nr 1 0
fi

if reset "mpc backup" &&
Expand Down

0 comments on commit 47f60d9

Please sign in to comment.