mptcp: pm: avoid using del_timer directly

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 <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Link: https://patch.msgid.link/20260605-net-next-mptcp-add-addr6-port-ts-v2-14-758e7ca73f4d@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 4215270bfba7fa781006431e34f2ca9e56dbbc18..614a561c1f7f4a526751adbc5e2808e5db12b5f7 100644 (file)
--- 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 f4604611f10f022b961e54c6c1871b137a7c6127..6afd39aea110a26678bc8f01d15423bf20303c76 100644 (file)
--- 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 @@ out:
        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 7bc8fd486e817f1a29f47d6bb84960f27ed9a8ea..4a2d40cd7b132540c3cce76be6f23eaf1604b37e 100644 (file)
--- 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);