mptcp_pm_announced_del_timer() removes the matched ADD_ADDR entry (if found) from the ADD_ADDR list only if check_id is false. That's dangerous, and not clear, because it means the caller should be free the entry only in some cases, and it easy to miss that. Instead, make it static, and call it from mptcp_pm_add_addr_echoed, which is the only other case where mptcp_pm_add_addr_del_timer should be called with check_id set to true. Bonus with that: a second call to mptcp_pm_add_addr_lookup_by_addr() can be avoided. Note that instead of adding the signature above to avoid a compilation issue because this helper is called before the definition of the function, the whole helper is moved above where it is first called. Its content is untouched, except the addition of the 'static' keyboard. Reviewed-by: Mat Martineau Signed-off-by: Matthieu Baerts (NGI0) --- net/mptcp/options.c | 1 - net/mptcp/pm.c | 75 +++++++++++++++++++++++++++------------------------- net/mptcp/protocol.h | 3 --- 3 files changed, 39 insertions(+), 40 deletions(-) diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 4215270bfba7..614a561c1f7f 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -1183,7 +1183,6 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb) MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR); } else { mptcp_pm_add_addr_echoed(msk, &mp_opt.addr); - mptcp_pm_announced_del_timer(msk, &mp_opt.addr, true); MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ECHOADD); } diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index f4604611f10f..6afd39aea110 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -149,6 +149,40 @@ mptcp_pm_announced_lookup(const struct mptcp_sock *msk, return NULL; } +static struct mptcp_pm_add_addr * +mptcp_pm_announced_del_timer(struct mptcp_sock *msk, + const struct mptcp_addr_info *addr, bool check_id) +{ + struct sock *sk = (struct sock *)msk; + struct mptcp_pm_add_addr *entry; + bool stop_timer = false; + + rcu_read_lock(); + + spin_lock_bh(&msk->pm.lock); + entry = mptcp_pm_announced_lookup(msk, addr); + if (entry && (!check_id || entry->addr.id == addr->id)) { + entry->retrans_times = ADD_ADDR_RETRANS_MAX; + stop_timer = true; + } + if (!check_id && entry) + list_del(&entry->list); + spin_unlock_bh(&msk->pm.lock); + + /* Note: entry might have been removed by another thread. + * We hold rcu_read_lock() to ensure it is not freed under us. + */ + if (stop_timer) { + if (check_id) + sk_stop_timer(sk, &entry->timer); + else + sk_stop_timer_sync(sk, &entry->timer); + } + + rcu_read_unlock(); + return entry; +} + bool mptcp_pm_announced_remove(struct mptcp_sock *msk, const struct mptcp_addr_info *addr) { @@ -398,40 +432,6 @@ static void mptcp_pm_add_addr_timer(struct timer_list *timer) sock_put(sk); } -struct mptcp_pm_add_addr * -mptcp_pm_announced_del_timer(struct mptcp_sock *msk, - const struct mptcp_addr_info *addr, bool check_id) -{ - struct sock *sk = (struct sock *)msk; - struct mptcp_pm_add_addr *entry; - bool stop_timer = false; - - rcu_read_lock(); - - spin_lock_bh(&msk->pm.lock); - entry = mptcp_pm_announced_lookup(msk, addr); - if (entry && (!check_id || entry->addr.id == addr->id)) { - entry->retrans_times = ADD_ADDR_RETRANS_MAX; - stop_timer = true; - } - if (!check_id && entry) - list_del(&entry->list); - spin_unlock_bh(&msk->pm.lock); - - /* Note: entry might have been removed by another thread. - * We hold rcu_read_lock() to ensure it is not freed under us. - */ - if (stop_timer) { - if (check_id) - sk_stop_timer(sk, &entry->timer); - else - sk_stop_timer_sync(sk, &entry->timer); - } - - rcu_read_unlock(); - return entry; -} - bool mptcp_pm_announced_alloc(struct mptcp_sock *msk, const struct mptcp_addr_info *addr) { @@ -730,15 +730,18 @@ void mptcp_pm_add_addr_echoed(struct mptcp_sock *msk, const struct mptcp_addr_info *addr) { struct mptcp_pm_data *pm = &msk->pm; + struct mptcp_pm_add_addr *entry; pr_debug("msk=%p\n", msk); - if (!READ_ONCE(pm->work_pending)) + entry = mptcp_pm_announced_del_timer(msk, addr, true); + + if (!entry || !READ_ONCE(pm->work_pending)) return; spin_lock_bh(&pm->lock); - if (mptcp_pm_announced_lookup(msk, addr) && READ_ONCE(pm->work_pending)) + if (READ_ONCE(pm->work_pending)) mptcp_pm_schedule_work(msk, MPTCP_PM_SUBFLOW_ESTABLISHED); spin_unlock_bh(&pm->lock); diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 7bc8fd486e81..4a2d40cd7b13 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1131,9 +1131,6 @@ int mptcp_pm_mp_prio_send_ack(struct mptcp_sock *msk, u8 bkup); bool mptcp_pm_announced_alloc(struct mptcp_sock *msk, const struct mptcp_addr_info *addr); -struct mptcp_pm_add_addr * -mptcp_pm_announced_del_timer(struct mptcp_sock *msk, - const struct mptcp_addr_info *addr, bool check_id); bool mptcp_pm_announced_remove(struct mptcp_sock *msk, const struct mptcp_addr_info *addr); bool mptcp_pm_announced_has_ssk(struct mptcp_sock *msk, const struct sock *ssk); -- 2.53.0