Source-Changes-HG archive

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

[src/trunk]: src/sys/net Integrate two locks used to protect PPPoE softc. Con...



details:   https://anonhg.NetBSD.org/src/rev/e1827de9c382
branches:  trunk
changeset: 827081:e1827de9c382
user:      knakahara <knakahara%NetBSD.org@localhost>
date:      Thu Oct 12 09:47:21 2017 +0000

description:
Integrate two locks used to protect PPPoE softc. Contributed by s-yamaguchi@IIJ.

PPPOE_SESSION_LOCK protects variables used in PPP packet
processing, on the other hand PPPOE_PARAM_LOCK protects
the other variables used to establish a PPPoE session id.

Those locks isn't acquired in the same time because the
PPP packet processing doesn't work without PPPoE session id.
By the reason, the locks can be integrated into PPPOE_LOCK.

Add locking notes later.

diffstat:

 sys/net/if_pppoe.c |  282 +++++++++++++++++-----------------------------------
 1 files changed, 94 insertions(+), 188 deletions(-)

diffs (truncated from 829 to 300 lines):

diff -r f2558b5f91cf -r e1827de9c382 sys/net/if_pppoe.c
--- a/sys/net/if_pppoe.c        Thu Oct 12 05:51:51 2017 +0000
+++ b/sys/net/if_pppoe.c        Thu Oct 12 09:47:21 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_pppoe.c,v 1.126 2017/07/20 02:34:24 knakahara Exp $ */
+/* $NetBSD: if_pppoe.c,v 1.127 2017/10/12 09:47:21 knakahara Exp $ */
 
 /*-
  * Copyright (c) 2002, 2008 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_pppoe.c,v 1.126 2017/07/20 02:34:24 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_pppoe.c,v 1.127 2017/10/12 09:47:21 knakahara Exp $");
 
 #ifdef _KERNEL_OPT
 #include "pppoe.h"
@@ -133,18 +133,12 @@
 #define        IFF_PASSIVE     IFF_LINK0       /* wait passively for connection */
 #endif
 
-#define PPPOE_SESSION_LOCK(_sc, _op)   rw_enter(&(_sc)->sc_session_lock, (_op))
-#define PPPOE_SESSION_UNLOCK(_sc)      rw_exit(&(_sc)->sc_session_lock)
-#define PPPOE_SESSION_LOCKED(_sc)      rw_lock_held(&(_sc)->sc_session_lock)
-#define PPPOE_SESSION_WLOCKED(_sc)     rw_write_held(&(_sc)->sc_session_lock)
-#define PPPOE_SESSION_RLOCKED(_sc)     rw_read_held(&(_sc)->sc_session_lock)
+#define PPPOE_LOCK(_sc, _op)   rw_enter(&(_sc)->sc_lock, (_op))
+#define PPPOE_UNLOCK(_sc)      rw_exit(&(_sc)->sc_lock)
+#define PPPOE_LOCKED(_sc)      rw_lock_held(&(_sc)->sc_lock)
+#define PPPOE_WLOCKED(_sc)     rw_write_held(&(_sc)->sc_lock)
+#define PPPOE_RLOCKED(_sc)     rw_read_held(&(_sc)->sc_lock)
 
-#define PPPOE_PARAM_LOCK(_sc)          if ((_sc)->sc_lock) \
-                                               mutex_enter((_sc)->sc_lock)
-#define PPPOE_PARAM_UNLOCK(_sc)                if ((_sc)->sc_lock) \
-                                               mutex_exit((_sc)->sc_lock)
-#define PPPOE_PARAM_LOCKED(_sc)                (!(_sc)->sc_lock || \
-                                               mutex_owned((_sc)->sc_lock))
 #ifdef PPPOE_MPSAFE
 #define DECLARE_SPLNET_VARIABLE
 #define ACQUIRE_SPLNET()       do { } while (0)
@@ -165,7 +159,6 @@
        struct ifnet *sc_eth_if;        /* ethernet interface we are using */
 
        int sc_state;                   /* discovery phase or session connected */
-       bool sc_state_updating;         /* state update in other components */
        struct ether_addr sc_dest;      /* hardware address of concentrator */
        uint16_t sc_session;            /* PPPoE session id */
 
@@ -182,8 +175,7 @@
        callout_t sc_timeout;   /* timeout while not in session state */
        int sc_padi_retried;            /* number of PADI retries already done */
        int sc_padr_retried;            /* number of PADR retries already done */
-       krwlock_t sc_session_lock;      /* lock of sc_state, sc_session, and sc_eth_if */
-       kmutex_t *sc_lock;              /* lock of other parameters */
+       krwlock_t sc_lock;      /* lock of sc_state, sc_session, and sc_eth_if */
 };
 
 /* incoming traffic will be queued here */
@@ -343,8 +335,7 @@
                pfil_add_ihook(pppoe_ifattach_hook, NULL, PFIL_IFNET, if_pfil);
        }
 
-       sc->sc_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_SOFTNET);
-       rw_init(&sc->sc_session_lock);
+       rw_init(&sc->sc_lock);
 
        rw_enter(&pppoe_softc_list_lock, RW_WRITER);
        LIST_INSERT_HEAD(&pppoe_softc_list, sc, sc_list);
@@ -359,7 +350,7 @@
 
        rw_enter(&pppoe_softc_list_lock, RW_WRITER);
 
-       PPPOE_PARAM_LOCK(sc);
+       PPPOE_LOCK(sc, RW_WRITER);
        callout_halt(&sc->sc_timeout, NULL);
 
        LIST_REMOVE(sc, sc_list);
@@ -369,7 +360,6 @@
        }
        rw_exit(&pppoe_softc_list_lock);
 
-       PPPOE_SESSION_LOCK(sc, RW_WRITER);
 
        bpf_detach(ifp);
        sppp_detach(&sc->sc_sppp.pp_if);
@@ -384,11 +374,8 @@
                free(sc->sc_relay_sid, M_DEVBUF);
        callout_destroy(&sc->sc_timeout);
 
-       PPPOE_PARAM_UNLOCK(sc);
-       if (sc->sc_lock)
-               mutex_obj_free(sc->sc_lock);
-       PPPOE_SESSION_UNLOCK(sc);
-       rw_destroy(&sc->sc_session_lock);
+       PPPOE_UNLOCK(sc);
+       rw_destroy(&sc->sc_lock);
 
        free(sc, M_DEVBUF);
 
@@ -410,14 +397,13 @@
                return NULL;
        rw_enter(&pppoe_softc_list_lock, RW_READER);
        LIST_FOREACH(sc, &pppoe_softc_list, sc_list) {
-               PPPOE_SESSION_LOCK(sc, lock);
-               if (!sc->sc_state_updating
-                   && sc->sc_state == PPPOE_STATE_SESSION
+               PPPOE_LOCK(sc, lock);
+               if ( sc->sc_state == PPPOE_STATE_SESSION
                    && sc->sc_session == session
                    && sc->sc_eth_if == rcvif)
                        break;
 
-               PPPOE_SESSION_UNLOCK(sc);
+               PPPOE_UNLOCK(sc);
        }
        rw_exit(&pppoe_softc_list_lock);
        return sc;
@@ -441,7 +427,7 @@
        rw_enter(&pppoe_softc_list_lock, RW_READER);
        LIST_FOREACH(sc, &pppoe_softc_list, sc_list) {
                if (sc == t) {
-                       PPPOE_SESSION_LOCK(sc, lock);
+                       PPPOE_LOCK(sc, lock);
                        break;
                }
        }
@@ -454,26 +440,17 @@
                return NULL;
        }
 
-       if (sc->sc_state_updating) {
-#ifdef PPPOE_DEBUG
-               printf("%s: host unique tag found, but its state is updating\n",
-                   sc->sc_sppp.pp_if.if_xname);
-#endif
-               PPPOE_SESSION_UNLOCK(sc);
-               return NULL;
-       }
-
        /* should be safe to access *sc now */
        if (sc->sc_state < PPPOE_STATE_PADI_SENT || sc->sc_state >= PPPOE_STATE_SESSION) {
                printf("%s: host unique tag found, but it belongs to a connection in state %d\n",
                        sc->sc_sppp.pp_if.if_xname, sc->sc_state);
-               PPPOE_SESSION_UNLOCK(sc);
+               PPPOE_UNLOCK(sc);
                return NULL;
        }
        if (sc->sc_eth_if != rcvif) {
                printf("%s: wrong interface, not accepting host unique\n",
                        sc->sc_sppp.pp_if.if_xname);
-               PPPOE_SESSION_UNLOCK(sc);
+               PPPOE_UNLOCK(sc);
                return NULL;
        }
        return sc;
@@ -664,7 +641,7 @@
                        if (sc != NULL) {
                                strlcpy(devname, sc->sc_sppp.pp_if.if_xname,
                                    sizeof(devname));
-                               PPPOE_SESSION_UNLOCK(sc);
+                               PPPOE_UNLOCK(sc);
                        }
                        break;
                }
@@ -744,21 +721,20 @@
                        goto done;
                rw_enter(&pppoe_softc_list_lock, RW_READER);
                LIST_FOREACH(sc, &pppoe_softc_list, sc_list) {
-                       PPPOE_SESSION_LOCK(sc, RW_WRITER);
+                       PPPOE_LOCK(sc, RW_WRITER);
                        if (!(sc->sc_sppp.pp_if.if_flags & IFF_UP)) {
-                               PPPOE_SESSION_UNLOCK(sc);
+                               PPPOE_UNLOCK(sc);
                                continue;
                        }
                        if (!(sc->sc_sppp.pp_if.if_flags & IFF_PASSIVE)) {
-                               PPPOE_SESSION_UNLOCK(sc);
+                               PPPOE_UNLOCK(sc);
                                continue;
                        }
 
-                       if (!sc->sc_state_updating
-                           && sc->sc_state == PPPOE_STATE_INITIAL)
+                       if (sc->sc_state == PPPOE_STATE_INITIAL)
                                break;
 
-                       PPPOE_SESSION_UNLOCK(sc);
+                       PPPOE_UNLOCK(sc);
                }
                rw_exit(&pppoe_softc_list_lock);
 
@@ -767,15 +743,13 @@
                        goto done;
                }
 
-               PPPOE_PARAM_LOCK(sc);
                if (hunique) {
                        if (sc->sc_hunique)
                                free(sc->sc_hunique, M_DEVBUF);
                        sc->sc_hunique = malloc(hunique_len, M_DEVBUF,
                            M_DONTWAIT);
                        if (sc->sc_hunique == NULL) {
-                               PPPOE_PARAM_UNLOCK(sc);
-                               PPPOE_SESSION_UNLOCK(sc);
+                               PPPOE_UNLOCK(sc);
                                goto done;
                        }
                        sc->sc_hunique_len = hunique_len;
@@ -784,8 +758,7 @@
                memcpy(&sc->sc_dest, eh->ether_shost, sizeof sc->sc_dest);
                sc->sc_state = PPPOE_STATE_PADO_SENT;
                pppoe_send_pado(sc);
-               PPPOE_PARAM_UNLOCK(sc);
-               PPPOE_SESSION_UNLOCK(sc);
+               PPPOE_UNLOCK(sc);
                break;
 #endif /* PPPOE_SERVER */
        case PPPOE_CODE_PADR:
@@ -817,22 +790,20 @@
                        goto done;
                }
 
-               if (sc->sc_state_updating
-                   || sc->sc_state != PPPOE_STATE_PADO_SENT) {
+               if (sc->sc_state != PPPOE_STATE_PADO_SENT) {
                        printf("%s: received unexpected PADR\n",
                            sc->sc_sppp.pp_if.if_xname);
-                       PPPOE_SESSION_UNLOCK(sc);
+                       PPPOE_UNLOCK(sc);
                        goto done;
                }
-               PPPOE_PARAM_LOCK(sc);
+
                if (hunique) {
                        if (sc->sc_hunique)
                                free(sc->sc_hunique, M_DEVBUF);
                        sc->sc_hunique = malloc(hunique_len, M_DEVBUF,
                            M_DONTWAIT);
                        if (sc->sc_hunique == NULL) {
-                               PPPOE_PARAM_UNLOCK(sc);
-                               PPPOE_SESSION_UNLOCK(sc);
+                               PPPOE_UNLOCK(sc);
                                goto done;
                        }
                        sc->sc_hunique_len = hunique_len;
@@ -840,8 +811,7 @@
                }
                pppoe_send_pads(sc);
                sc->sc_state = PPPOE_STATE_SESSION;
-               PPPOE_PARAM_UNLOCK(sc);
-               PPPOE_SESSION_UNLOCK(sc);
+               PPPOE_UNLOCK(sc);
 
                sppp_lock_enter(&sc->sc_sppp);
                sc->sc_sppp.pp_up(&sc->sc_sppp);
@@ -860,17 +830,15 @@
                        goto done;
                }
 
-               PPPOE_SESSION_LOCK(sc, RW_WRITER);
+               PPPOE_LOCK(sc, RW_WRITER);
 
-               if (sc->sc_state_updating
-                   || sc->sc_state != PPPOE_STATE_PADI_SENT) {
+               if (sc->sc_state != PPPOE_STATE_PADI_SENT) {
                        printf("%s: received unexpected PADO\n",
                            sc->sc_sppp.pp_if.if_xname);
-                       PPPOE_SESSION_UNLOCK(sc);
+                       PPPOE_UNLOCK(sc);
                        goto done;
                }
 
-               PPPOE_PARAM_LOCK(sc);
                if (ac_cookie) {
                        if (sc->sc_ac_cookie)
                                free(sc->sc_ac_cookie, M_DEVBUF);
@@ -880,8 +848,7 @@
                                printf("%s: FATAL: could not allocate memory "
                                    "for AC cookie\n",
                                    sc->sc_sppp.pp_if.if_xname);
-                               PPPOE_PARAM_UNLOCK(sc);
-                               PPPOE_SESSION_UNLOCK(sc);
+                               PPPOE_UNLOCK(sc);
                                goto done;
                        }
                        sc->sc_ac_cookie_len = ac_cookie_len;
@@ -896,8 +863,7 @@
                                printf("%s: FATAL: could not allocate memory "
                                    "for relay SID\n",
                                    sc->sc_sppp.pp_if.if_xname);
-                               PPPOE_PARAM_UNLOCK(sc);
-                               PPPOE_SESSION_UNLOCK(sc);
+                               PPPOE_UNLOCK(sc);
                                goto done;
                        }
                        sc->sc_relay_sid_len = relay_sid_len;
@@ -917,21 +883,13 @@
                    PPPOE_DISC_TIMEOUT * (1 + sc->sc_padr_retried),



Home | Main Index | Thread Index | Old Index