Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/net wg: Simplify locking.



details:   https://anonhg.NetBSD.org/src/rev/c5a7b65f77a5
branches:  trunk
changeset: 943359:c5a7b65f77a5
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Mon Aug 31 20:33:58 2020 +0000

description:
wg: Simplify locking.

Summary: Access to a stable established session is still allowed via
psref; all other access to peer and session state is now serialized
by struct wg_peer::wgp_lock, with no dancing around a per-session
lock.  This way, the handshake paths are locked, while the data
transmission paths are pserialized.

- Eliminate struct wg_session::wgs_lock.

- Eliminate wg_get_unstable_session -- access to the unstable session
  is allowed only with struct wgp_peer::wgp_lock held.

- Push INIT_PASSIVE->ESTABLISHED transition down into a thread task.

- Push rekey down into a thread task.

- Allocate session indices only on transition from UNKNOWN and free
  them only on transition back to UNKNOWN.

- Be a little more explicit about allowed state transitions, and
  reject some nonsensical ones.

- Sprinkle assertions and comments.

- Reduce atomic r/m/w swap operations that can just as well be
  store-release.

diffstat:

 sys/net/if_wg.c |  882 ++++++++++++++++++++++++++++++-------------------------
 1 files changed, 481 insertions(+), 401 deletions(-)

diffs (truncated from 1568 to 300 lines):

diff -r 7f9f396c3515 -r c5a7b65f77a5 sys/net/if_wg.c
--- a/sys/net/if_wg.c   Mon Aug 31 20:32:58 2020 +0000
+++ b/sys/net/if_wg.c   Mon Aug 31 20:33:58 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_wg.c,v 1.48 2020/08/31 20:31:43 riastradh Exp $     */
+/*     $NetBSD: if_wg.c,v 1.49 2020/08/31 20:33:58 riastradh Exp $     */
 
 /*
  * Copyright (C) Ryota Ozaki <ozaki.ryota%gmail.com@localhost>
@@ -41,7 +41,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_wg.c,v 1.48 2020/08/31 20:31:43 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_wg.c,v 1.49 2020/08/31 20:33:58 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -137,7 +137,7 @@
  * - struct wg_session represents a session of a secure tunnel with a peer
  *   - Two instances of sessions belong to a peer; a stable session and a
  *     unstable session
- *   - A handshake process of a session always starts with a unstable instace
+ *   - A handshake process of a session always starts with a unstable instance
  *   - Once a session is established, its instance becomes stable and the
  *     other becomes unstable instead
  *   - Data messages are always sent via a stable session
@@ -145,25 +145,22 @@
  * Locking notes:
  * - wg interfaces (struct wg_softc, wg) is listed in wg_softcs.list and
  *   protected by wg_softcs.lock
- * - Each wg has a mutex(9) and a rwlock(9)
- *   - The mutex (wg_lock) protects its peer list (wg_peers)
- *   - A peer on the list is also protected by pserialize(9) or psref(9)
+ * - Each wg has a mutex(9) wg_lock, and a rwlock(9) wg_rwlock
+ *   - Changes to the peer list are serialized by wg_lock
+ *   - The peer list may be read with pserialize(9) and psref(9)
  *   - The rwlock (wg_rwlock) protects the routing tables (wg_rtable_ipv[46])
- * - Each peer (struct wg_peer, wgp) has a mutex
- *   - The mutex (wgp_lock) protects wgp_session_unstable and wgp_state
- * - Each session (struct wg_session, wgs) has a mutex
- *   - The mutex (wgs_lock) protects its state (wgs_state) and its handshake
- *     states
- *   - wgs_state of a unstable session can be changed while it never be
- *     changed on a stable session, so once get a session instace via
- *     wgp_session_stable we can safely access wgs_state without
- *     holding wgs_lock
- *   - A session is protected by pserialize or psref like wgp
+ *     => XXX replace by pserialize when routing table is psz-safe
+ * - Each peer (struct wg_peer, wgp) has a mutex wgp_lock, which can be taken
+ *   only in thread context and serializes:
+ *   - the stable and unstable session pointers
+ *   - all unstable session state
+ * - Packet processing may be done in softint context:
+ *   - The stable session can be read under pserialize(9) or psref(9)
+ *     - The stable session is always ESTABLISHED
  *     - On a session swap, we must wait for all readers to release a
  *       reference to a stable session before changing wgs_state and
  *       session states
- *
- * Lock order: wg_lock -> wgp_lock -> wgs_lock
+ * - Lock order: wg_lock -> wgp_lock
  */
 
 
@@ -444,7 +441,6 @@
        struct wg_peer  *wgs_peer;
        struct psref_target
                        wgs_psref;
-       kmutex_t        *wgs_lock;
 
        int             wgs_state;
 #define WGS_STATE_UNKNOWN      0
@@ -457,8 +453,8 @@
        time_t          wgs_time_last_data_sent;
        bool            wgs_is_initiator;
 
-       uint32_t        wgs_sender_index;
-       uint32_t        wgs_receiver_index;
+       uint32_t        wgs_local_index;
+       uint32_t        wgs_remote_index;
 #ifdef __HAVE_ATOMIC64_LOADSTORE
        volatile uint64_t
                        wgs_send_counter;
@@ -537,18 +533,12 @@
        uint8_t wgp_pubkey[WG_STATIC_KEY_LEN];
        struct wg_sockaddr      *wgp_endpoint;
        struct wg_sockaddr      *wgp_endpoint0;
-       bool                    wgp_endpoint_changing;
+       volatile unsigned       wgp_endpoint_changing;
        bool                    wgp_endpoint_available;
 
                        /* The preshared key (optional) */
        uint8_t         wgp_psk[WG_PRESHARED_KEY_LEN];
 
-       int wgp_state;
-#define WGP_STATE_INIT         0
-#define WGP_STATE_ESTABLISHED  1
-#define WGP_STATE_GIVEUP       2
-#define WGP_STATE_DESTROYING   3
-
        void            *wgp_si;
        pcq_t           *wgp_q;
 
@@ -585,9 +575,11 @@
 
        volatile unsigned int   wgp_tasks;
 #define WGP_TASK_SEND_INIT_MESSAGE             __BIT(0)
-#define WGP_TASK_ENDPOINT_CHANGED              __BIT(1)
-#define WGP_TASK_SEND_KEEPALIVE_MESSAGE                __BIT(2)
-#define WGP_TASK_DESTROY_PREV_SESSION          __BIT(3)
+#define WGP_TASK_RETRY_HANDSHAKE               __BIT(1)
+#define WGP_TASK_ESTABLISH_SESSION             __BIT(2)
+#define WGP_TASK_ENDPOINT_CHANGED              __BIT(3)
+#define WGP_TASK_SEND_KEEPALIVE_MESSAGE                __BIT(4)
+#define WGP_TASK_DESTROY_PREV_SESSION          __BIT(5)
 };
 
 struct wg_ops;
@@ -652,8 +644,8 @@
                    struct mbuf *);
 static int     wg_send_cookie_msg(struct wg_softc *, struct wg_peer *,
                    const uint32_t, const uint8_t [], const struct sockaddr *);
-static int     wg_send_handshake_msg_resp(struct wg_softc *,
-                   struct wg_peer *, const struct wg_msg_init *);
+static int     wg_send_handshake_msg_resp(struct wg_softc *, struct wg_peer *,
+                   struct wg_session *, const struct wg_msg_init *);
 static void    wg_send_keepalive_msg(struct wg_peer *, struct wg_session *);
 
 static struct wg_peer *
@@ -691,6 +683,8 @@
 static int     wg_init(struct ifnet *);
 static void    wg_stop(struct ifnet *, int);
 
+static void    wg_purge_pending_packets(struct wg_peer *);
+
 static int     wg_clone_create(struct if_clone *, int);
 static int     wg_clone_destroy(struct ifnet *);
 
@@ -1100,19 +1094,17 @@
        be32enc(timestamp + 8, ts.tv_nsec);
 }
 
-static struct wg_session *
-wg_get_unstable_session(struct wg_peer *wgp, struct psref *psref)
-{
-       int s;
-       struct wg_session *wgs;
-
-       s = pserialize_read_enter();
-       wgs = wgp->wgp_session_unstable;
-       psref_acquire(psref, &wgs->wgs_psref, wg_psref_class);
-       pserialize_read_exit(s);
-       return wgs;
-}
-
+/*
+ * wg_get_stable_session(wgp, psref)
+ *
+ *     Get a passive reference to the current stable session, or
+ *     return NULL if there is no current stable session.
+ *
+ *     The pointer is always there but the session is not necessarily
+ *     ESTABLISHED; if it is not ESTABLISHED, return NULL.  However,
+ *     the session may transition from ESTABLISHED to DESTROYING while
+ *     holding the passive reference.
+ */
 static struct wg_session *
 wg_get_stable_session(struct wg_peer *wgp, struct psref *psref)
 {
@@ -1120,91 +1112,104 @@
        struct wg_session *wgs;
 
        s = pserialize_read_enter();
-       wgs = wgp->wgp_session_stable;
-       psref_acquire(psref, &wgs->wgs_psref, wg_psref_class);
+       wgs = atomic_load_consume(&wgp->wgp_session_stable);
+       if (__predict_false(wgs->wgs_state != WGS_STATE_ESTABLISHED))
+               wgs = NULL;
+       else
+               psref_acquire(psref, &wgs->wgs_psref, wg_psref_class);
        pserialize_read_exit(s);
+
        return wgs;
 }
 
 static void
-wg_get_session(struct wg_session *wgs, struct psref *psref)
-{
-
-       psref_acquire(psref, &wgs->wgs_psref, wg_psref_class);
-}
-
-static void
 wg_put_session(struct wg_session *wgs, struct psref *psref)
 {
 
        psref_release(psref, &wgs->wgs_psref, wg_psref_class);
 }
 
-static struct wg_session *
-wg_lock_unstable_session(struct wg_peer *wgp)
-{
-       struct wg_session *wgs;
-
-       mutex_enter(wgp->wgp_lock);
-       wgs = wgp->wgp_session_unstable;
-       mutex_enter(wgs->wgs_lock);
-       mutex_exit(wgp->wgp_lock);
-       return wgs;
-}
-
-#if 0
 static void
-wg_unlock_session(struct wg_peer *wgp, struct wg_session *wgs)
-{
-
-       mutex_exit(wgs->wgs_lock);
-}
-#endif
-
-static uint32_t
-wg_assign_sender_index(struct wg_softc *wg, struct wg_session *wgs)
+wg_destroy_session(struct wg_softc *wg, struct wg_session *wgs)
 {
        struct wg_peer *wgp = wgs->wgs_peer;
+       struct wg_session *wgs0 __diagused;
+       void *garbage;
+
+       KASSERT(mutex_owned(wgp->wgp_lock));
+       KASSERT(wgs->wgs_state != WGS_STATE_UNKNOWN);
+
+       /* Remove the session from the table.  */
+       wgs0 = thmap_del(wg->wg_sessions_byindex,
+           &wgs->wgs_local_index, sizeof(wgs->wgs_local_index));
+       KASSERT(wgs0 == wgs);
+       garbage = thmap_stage_gc(wg->wg_sessions_byindex);
+
+       /* Wait for passive references to drain.  */
+       pserialize_perform(wgp->wgp_psz);
+       psref_target_destroy(&wgs->wgs_psref, wg_psref_class);
+
+       /* Free memory, zero state, and transition to UNKNOWN.  */
+       thmap_gc(wg->wg_sessions_byindex, garbage);
+       wg_clear_states(wgs);
+       wgs->wgs_state = WGS_STATE_UNKNOWN;
+}
+
+/*
+ * wg_get_session_index(wg, wgs)
+ *
+ *     Choose a session index for wgs->wgs_local_index, and store it
+ *     in wg's table of sessions by index.
+ *
+ *     wgs must be the unstable session of its peer, and must be
+ *     transitioning out of the UNKNOWN state.
+ */
+static void
+wg_get_session_index(struct wg_softc *wg, struct wg_session *wgs)
+{
+       struct wg_peer *wgp __diagused = wgs->wgs_peer;
        struct wg_session *wgs0;
        uint32_t index;
-       void *garbage;
-
-       mutex_enter(wgs->wgs_lock);
-
-       /* Release the current index, if there is one.  */
-       while ((index = wgs->wgs_sender_index) != 0) {
-               /* Remove the session by index.  */
-               thmap_del(wg->wg_sessions_byindex, &index, sizeof index);
-               wgs->wgs_sender_index = 0;
-               mutex_exit(wgs->wgs_lock);
-
-               /* Wait for all thmap_gets to complete, and GC.  */
-               garbage = thmap_stage_gc(wg->wg_sessions_byindex);
-               mutex_enter(wgs->wgs_peer->wgp_lock);
-               pserialize_perform(wgp->wgp_psz);
-               mutex_exit(wgs->wgs_peer->wgp_lock);
-               thmap_gc(wg->wg_sessions_byindex, garbage);
-
-               mutex_enter(wgs->wgs_lock);
-       }
-
-restart:
-       /* Pick a uniform random nonzero index.  */
-       while (__predict_false((index = cprng_strong32()) == 0))
-               continue;
-
-       /* Try to take it.  */
-       wgs->wgs_sender_index = index;
-       wgs0 = thmap_put(wg->wg_sessions_byindex,
-           &wgs->wgs_sender_index, sizeof wgs->wgs_sender_index, wgs);
-
-       /* If someone else beat us, start over.  */
-       if (__predict_false(wgs0 != wgs))
-               goto restart;
-
-       mutex_exit(wgs->wgs_lock);
-



Home | Main Index | Thread Index | Old Index