From f50bc84145a6fc6648f6fedd398064a1042d3258 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Thu, 9 Jun 2022 17:01:07 +0200 Subject: [PATCH] mptcp: invoke MP_FAIL response when needed mptcp_mp_fail_no_response shouldn't be invoked on each worker run, it should be invoked only when MP_FAIL response timeout occurs. This patch refactors the MP_FAIL response logic. Add fail_tout in mptcp_sock to record the MP_FAIL timestamp. Check it in mptcp_worker() before invoking mptcp_mp_fail_no_response(). Drop the code to reuse sk_timer for MP_FAIL response. Add fail_ssk struct member in struct mptcp_sock to record the MP_FAIL subsocket. It can replace the mp_fail_response_expect flag in struct mptcp_subflow_context. Drop mp_fail_response_expect_subflow() helper, just use this fail_ssk in mptcp_mp_fail_no_response() to reset the subflow. Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/281 Fixes: d9fb797046c5 ("mptcp: Do not traverse the subflow connection list without lock") Acked-by: Paolo Abeni Reviewed-by: Mat Martineau Signed-off-by: Geliang Tang --- net/mptcp/pm.c | 7 ++----- net/mptcp/protocol.c | 37 +++++++++++-------------------------- net/mptcp/protocol.h | 3 ++- net/mptcp/subflow.c | 12 +++--------- 4 files changed, 18 insertions(+), 41 deletions(-) diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 59a85220edc9c..2a57d95d54923 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -299,23 +299,20 @@ void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq) { struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk); struct mptcp_sock *msk = mptcp_sk(subflow->conn); - struct sock *s = (struct sock *)msk; pr_debug("fail_seq=%llu", fail_seq); if (!READ_ONCE(msk->allow_infinite_fallback)) return; - if (!READ_ONCE(subflow->mp_fail_response_expect)) { + if (!msk->fail_ssk) { pr_debug("send MP_FAIL response and infinite map"); subflow->send_mp_fail = 1; MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPFAILTX); subflow->send_infinite_map = 1; - } else if (!sock_flag(sk, SOCK_DEAD)) { + } else { pr_debug("MP_FAIL response received"); - - sk_stop_timer(s, &s->sk_timer); } } diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 17e13396024ad..b74c6ca52aee7 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2175,21 +2175,6 @@ static void mptcp_retransmit_timer(struct timer_list *t) sock_put(sk); } -static struct mptcp_subflow_context * -mp_fail_response_expect_subflow(struct mptcp_sock *msk) -{ - struct mptcp_subflow_context *subflow, *ret = NULL; - - mptcp_for_each_subflow(msk, subflow) { - if (READ_ONCE(subflow->mp_fail_response_expect)) { - ret = subflow; - break; - } - } - - return ret; -} - static void mptcp_timeout_timer(struct timer_list *t) { struct sock *sk = from_timer(sk, t, sk_timer); @@ -2520,19 +2505,16 @@ static void __mptcp_retrans(struct sock *sk) static void mptcp_mp_fail_no_response(struct mptcp_sock *msk) { - struct mptcp_subflow_context *subflow; - struct sock *ssk; + struct sock *ssk = msk->fail_ssk; bool slow; - subflow = mp_fail_response_expect_subflow(msk); - if (subflow) { - pr_debug("MP_FAIL doesn't respond, reset the subflow"); + pr_debug("MP_FAIL doesn't respond, reset the subflow"); - ssk = mptcp_subflow_tcp_sock(subflow); - slow = lock_sock_fast(ssk); - mptcp_subflow_reset(ssk); - unlock_sock_fast(ssk, slow); - } + slow = lock_sock_fast(ssk); + mptcp_subflow_reset(ssk); + unlock_sock_fast(ssk, slow); + + msk->fail_ssk = NULL; } static void mptcp_worker(struct work_struct *work) @@ -2575,7 +2557,8 @@ static void mptcp_worker(struct work_struct *work) if (test_and_clear_bit(MPTCP_WORK_RTX, &msk->flags)) __mptcp_retrans(sk); - mptcp_mp_fail_no_response(msk); + if (msk->fail_ssk && time_after(jiffies, msk->fail_tout)) + mptcp_mp_fail_no_response(msk); unlock: release_sock(sk); @@ -2602,6 +2585,8 @@ static int __mptcp_init_sock(struct sock *sk) WRITE_ONCE(msk->csum_enabled, mptcp_is_checksum_enabled(sock_net(sk))); WRITE_ONCE(msk->allow_infinite_fallback, true); msk->recovery = false; + msk->fail_ssk = NULL; + msk->fail_tout = 0; mptcp_pm_data_init(msk); diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 200f89f6d62f6..e420a441c0560 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -306,6 +306,8 @@ struct mptcp_sock { u32 setsockopt_seq; char ca_name[TCP_CA_NAME_MAX]; + struct sock *fail_ssk; + unsigned long fail_tout; }; #define mptcp_data_lock(sk) spin_lock_bh(&(sk)->sk_lock.slock) @@ -468,7 +470,6 @@ struct mptcp_subflow_context { local_id_valid : 1, /* local_id is correctly initialized */ valid_csum_seen : 1; /* at least one csum validated */ enum mptcp_data_avail data_avail; - bool mp_fail_response_expect; u32 remote_nonce; u64 thmac; u32 local_nonce; diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 8841e8cd9ad8d..5351d54e514ac 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -974,7 +974,6 @@ static enum mapping_status get_mapping_status(struct sock *ssk, { struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk); bool csum_reqd = READ_ONCE(msk->csum_enabled); - struct sock *sk = (struct sock *)msk; struct mptcp_ext *mpext; struct sk_buff *skb; u16 data_len; @@ -1016,9 +1015,6 @@ static enum mapping_status get_mapping_status(struct sock *ssk, pr_debug("infinite mapping received"); MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPRX); subflow->map_data_len = 0; - if (!sock_flag(ssk, SOCK_DEAD)) - sk_stop_timer(sk, &sk->sk_timer); - return MAPPING_INVALID; } @@ -1238,11 +1234,9 @@ static bool subflow_check_data_avail(struct sock *ssk) tcp_send_active_reset(ssk, GFP_ATOMIC); while ((skb = skb_peek(&ssk->sk_receive_queue))) sk_eat_skb(ssk, skb); - } else if (!sock_flag(ssk, SOCK_DEAD)) { - WRITE_ONCE(subflow->mp_fail_response_expect, true); - sk_reset_timer((struct sock *)msk, - &((struct sock *)msk)->sk_timer, - jiffies + TCP_RTO_MAX); + } else { + msk->fail_ssk = ssk; + msk->fail_tout = jiffies + TCP_RTO_MAX; } WRITE_ONCE(subflow->data_avail, MPTCP_SUBFLOW_NODATA); return true;