Skip to content

Commit 0cd55ef

Browse files
author
Paolo Abeni
committed
Merge branch 'mptcp-fix-inconsistent-backup-usage'
Matthieu Baerts says: ==================== mptcp: fix inconsistent backup usage In all the MPTCP backup related tests, the backup flag was set on one side, and the expected behaviour is to have both sides respecting this decision. That's also the "natural" way, and what the users seem to expect. On the scheduler side, only the 'backup' field was checked, which is supposed to be set only if the other peer flagged a subflow as backup. But in various places, this flag was also set when the local host flagged the subflow as backup, certainly to have the expected behaviour mentioned above. Patch 1 modifies the packet scheduler to check if the backup flag has been set on both directions, not to change its behaviour after having applied the following patches. That's what the default packet scheduler should have done since the beginning in v5.7. Patch 2 fixes the backup flag being mirrored on the MPJ+SYN+ACK by accident since its introduction in v5.7. Instead, the received and sent backup flags are properly distinguished in requests. Patch 3 stops setting the received backup flag as well when sending an MP_PRIO, something that was done since the MP_PRIO support in v5.12. Patch 4 adds related and missing MIB counters to be able to easily check if MP_JOIN are sent with a backup flag. Certainly because these counters were not there, the behaviour that is fixed by patches here was not properly verified. Patch 5 validates the previous patch by extending the MPTCP Join selftest. Patch 6 fixes the backup support in signal endpoints: if a signal endpoint had the backup flag, it was not set in the MPJ+SYN+ACK as expected. It was only set for ongoing connections, but not future ones as expected, since the introduction of the backup flag in endpoints in v5.10. Patch 7 validates the previous patch by extending the MPTCP Join selftest as well. Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> --- Matthieu Baerts (NGI0) (7): mptcp: sched: check both directions for backup mptcp: distinguish rcv vs sent backup flag in requests mptcp: pm: only set request_bkup flag when sending MP_PRIO mptcp: mib: count MPJ with backup flag selftests: mptcp: join: validate backup in MPJ mptcp: pm: fix backup support in signal endpoints selftests: mptcp: join: check backup support in signal endp include/trace/events/mptcp.h | 2 +- net/mptcp/mib.c | 2 + net/mptcp/mib.h | 2 + net/mptcp/options.c | 2 +- net/mptcp/pm.c | 12 +++++ net/mptcp/pm_netlink.c | 19 ++++++- net/mptcp/pm_userspace.c | 18 +++++++ net/mptcp/protocol.c | 10 ++-- net/mptcp/protocol.h | 4 ++ net/mptcp/subflow.c | 10 ++++ tools/testing/selftests/net/mptcp/mptcp_join.sh | 72 ++++++++++++++++++++----- 11 files changed, 132 insertions(+), 21 deletions(-) ==================== Link: https://patch.msgid.link/20240727-upstream-net-20240727-mptcp-backup-signal-v1-0-f50b31604cf1@kernel.org Signed-off-by: Paolo Abeni <[email protected]>
2 parents 039564d + f833470 commit 0cd55ef

File tree

11 files changed

+132
-21
lines changed

11 files changed

+132
-21
lines changed

‎include/trace/events/mptcp.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ TRACE_EVENT(mptcp_subflow_get_send,
3434
struct sock *ssk;
3535

3636
__entry->active = mptcp_subflow_active(subflow);
37-
__entry->backup = subflow->backup;
37+
__entry->backup = subflow->backup || subflow->request_bkup;
3838

3939
if (subflow->tcp_sock && sk_fullsock(subflow->tcp_sock))
4040
__entry->free = sk_stream_memory_free(subflow->tcp_sock);

‎net/mptcp/mib.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ static const struct snmp_mib mptcp_snmp_list[] = {
1919
SNMP_MIB_ITEM("MPTCPRetrans", MPTCP_MIB_RETRANSSEGS),
2020
SNMP_MIB_ITEM("MPJoinNoTokenFound", MPTCP_MIB_JOINNOTOKEN),
2121
SNMP_MIB_ITEM("MPJoinSynRx", MPTCP_MIB_JOINSYNRX),
22+
SNMP_MIB_ITEM("MPJoinSynBackupRx", MPTCP_MIB_JOINSYNBACKUPRX),
2223
SNMP_MIB_ITEM("MPJoinSynAckRx", MPTCP_MIB_JOINSYNACKRX),
24+
SNMP_MIB_ITEM("MPJoinSynAckBackupRx", MPTCP_MIB_JOINSYNACKBACKUPRX),
2325
SNMP_MIB_ITEM("MPJoinSynAckHMacFailure", MPTCP_MIB_JOINSYNACKMAC),
2426
SNMP_MIB_ITEM("MPJoinAckRx", MPTCP_MIB_JOINACKRX),
2527
SNMP_MIB_ITEM("MPJoinAckHMacFailure", MPTCP_MIB_JOINACKMAC),

‎net/mptcp/mib.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ enum linux_mptcp_mib_field {
1414
MPTCP_MIB_RETRANSSEGS, /* Segments retransmitted at the MPTCP-level */
1515
MPTCP_MIB_JOINNOTOKEN, /* Received MP_JOIN but the token was not found */
1616
MPTCP_MIB_JOINSYNRX, /* Received a SYN + MP_JOIN */
17+
MPTCP_MIB_JOINSYNBACKUPRX, /* Received a SYN + MP_JOIN + backup flag */
1718
MPTCP_MIB_JOINSYNACKRX, /* Received a SYN/ACK + MP_JOIN */
19+
MPTCP_MIB_JOINSYNACKBACKUPRX, /* Received a SYN/ACK + MP_JOIN + backup flag */
1820
MPTCP_MIB_JOINSYNACKMAC, /* HMAC was wrong on SYN/ACK + MP_JOIN */
1921
MPTCP_MIB_JOINACKRX, /* Received an ACK + MP_JOIN */
2022
MPTCP_MIB_JOINACKMAC, /* HMAC was wrong on ACK + MP_JOIN */

‎net/mptcp/options.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,7 @@ bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
909909
return true;
910910
} else if (subflow_req->mp_join) {
911911
opts->suboptions = OPTION_MPTCP_MPJ_SYNACK;
912-
opts->backup = subflow_req->backup;
912+
opts->backup = subflow_req->request_bkup;
913913
opts->join_id = subflow_req->local_id;
914914
opts->thmac = subflow_req->thmac;
915915
opts->nonce = subflow_req->local_nonce;

‎net/mptcp/pm.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,18 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
426426
return mptcp_pm_nl_get_local_id(msk, &skc_local);
427427
}
428428

429+
bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
430+
{
431+
struct mptcp_addr_info skc_local;
432+
433+
mptcp_local_address((struct sock_common *)skc, &skc_local);
434+
435+
if (mptcp_pm_is_userspace(msk))
436+
return mptcp_userspace_pm_is_backup(msk, &skc_local);
437+
438+
return mptcp_pm_nl_is_backup(msk, &skc_local);
439+
}
440+
429441
int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
430442
u8 *flags, int *ifindex)
431443
{

‎net/mptcp/pm_netlink.c

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,6 @@ static void __mptcp_pm_send_ack(struct mptcp_sock *msk, struct mptcp_subflow_con
471471
slow = lock_sock_fast(ssk);
472472
if (prio) {
473473
subflow->send_mp_prio = 1;
474-
subflow->backup = backup;
475474
subflow->request_bkup = backup;
476475
}
477476

@@ -1102,6 +1101,24 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc
11021101
return ret;
11031102
}
11041103

1104+
bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
1105+
{
1106+
struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
1107+
struct mptcp_pm_addr_entry *entry;
1108+
bool backup = false;
1109+
1110+
rcu_read_lock();
1111+
list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
1112+
if (mptcp_addresses_equal(&entry->addr, skc, entry->addr.port)) {
1113+
backup = !!(entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
1114+
break;
1115+
}
1116+
}
1117+
rcu_read_unlock();
1118+
1119+
return backup;
1120+
}
1121+
11051122
#define MPTCP_PM_CMD_GRP_OFFSET 0
11061123
#define MPTCP_PM_EV_GRP_OFFSET 1
11071124

‎net/mptcp/pm_userspace.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,24 @@ int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
165165
return mptcp_userspace_pm_append_new_local_addr(msk, &new_entry, true);
166166
}
167167

168+
bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
169+
struct mptcp_addr_info *skc)
170+
{
171+
struct mptcp_pm_addr_entry *entry;
172+
bool backup = false;
173+
174+
spin_lock_bh(&msk->pm.lock);
175+
list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
176+
if (mptcp_addresses_equal(&entry->addr, skc, false)) {
177+
backup = !!(entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
178+
break;
179+
}
180+
}
181+
spin_unlock_bh(&msk->pm.lock);
182+
183+
return backup;
184+
}
185+
168186
int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
169187
{
170188
struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];

‎net/mptcp/protocol.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,13 +1422,15 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
14221422
}
14231423

14241424
mptcp_for_each_subflow(msk, subflow) {
1425+
bool backup = subflow->backup || subflow->request_bkup;
1426+
14251427
trace_mptcp_subflow_get_send(subflow);
14261428
ssk = mptcp_subflow_tcp_sock(subflow);
14271429
if (!mptcp_subflow_active(subflow))
14281430
continue;
14291431

14301432
tout = max(tout, mptcp_timeout_from_subflow(subflow));
1431-
nr_active += !subflow->backup;
1433+
nr_active += !backup;
14321434
pace = subflow->avg_pacing_rate;
14331435
if (unlikely(!pace)) {
14341436
/* init pacing rate from socket */
@@ -1439,9 +1441,9 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
14391441
}
14401442

14411443
linger_time = div_u64((u64)READ_ONCE(ssk->sk_wmem_queued) << 32, pace);
1442-
if (linger_time < send_info[subflow->backup].linger_time) {
1443-
send_info[subflow->backup].ssk = ssk;
1444-
send_info[subflow->backup].linger_time = linger_time;
1444+
if (linger_time < send_info[backup].linger_time) {
1445+
send_info[backup].ssk = ssk;
1446+
send_info[backup].linger_time = linger_time;
14451447
}
14461448
}
14471449
__mptcp_set_timeout(sk, tout);

‎net/mptcp/protocol.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ struct mptcp_subflow_request_sock {
448448
u16 mp_capable : 1,
449449
mp_join : 1,
450450
backup : 1,
451+
request_bkup : 1,
451452
csum_reqd : 1,
452453
allow_join_id0 : 1;
453454
u8 local_id;
@@ -1108,6 +1109,9 @@ bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
11081109
int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
11091110
int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
11101111
int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
1112+
bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc);
1113+
bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
1114+
bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
11111115
int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb);
11121116
int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
11131117
struct netlink_callback *cb);

‎net/mptcp/subflow.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ static struct mptcp_sock *subflow_token_join_request(struct request_sock *req)
100100
return NULL;
101101
}
102102
subflow_req->local_id = local_id;
103+
subflow_req->request_bkup = mptcp_pm_is_backup(msk, (struct sock_common *)req);
103104

104105
return msk;
105106
}
@@ -168,6 +169,9 @@ static int subflow_check_req(struct request_sock *req,
168169
return 0;
169170
} else if (opt_mp_join) {
170171
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINSYNRX);
172+
173+
if (mp_opt.backup)
174+
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINSYNBACKUPRX);
171175
}
172176

173177
if (opt_mp_capable && listener->request_mptcp) {
@@ -577,6 +581,9 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
577581
subflow->mp_join = 1;
578582
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_JOINSYNACKRX);
579583

584+
if (subflow->backup)
585+
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_JOINSYNACKBACKUPRX);
586+
580587
if (subflow_use_different_dport(msk, sk)) {
581588
pr_debug("synack inet_dport=%d %d",
582589
ntohs(inet_sk(sk)->inet_dport),
@@ -614,6 +621,8 @@ static int subflow_chk_local_id(struct sock *sk)
614621
return err;
615622

616623
subflow_set_local_id(subflow, err);
624+
subflow->request_bkup = mptcp_pm_is_backup(msk, (struct sock_common *)sk);
625+
617626
return 0;
618627
}
619628

@@ -2005,6 +2014,7 @@ static void subflow_ulp_clone(const struct request_sock *req,
20052014
new_ctx->fully_established = 1;
20062015
new_ctx->remote_key_valid = 1;
20072016
new_ctx->backup = subflow_req->backup;
2017+
new_ctx->request_bkup = subflow_req->request_bkup;
20082018
WRITE_ONCE(new_ctx->remote_id, subflow_req->remote_id);
20092019
new_ctx->token = subflow_req->token;
20102020
new_ctx->thmac = subflow_req->thmac;

0 commit comments

Comments
 (0)