This is my first try trading global locks for interface specific ones.
pppoe(4) keeps a list of all its interfaces which is then obviously
traversed during create and destroy.
Currently, the net lock is grabbed for this, but there seems to be no
justification other than reusing^Wabusing an existing lock.
I run this diff with WITNESS and kern.witness=2 on my edgerouter 4
providing my home uplink via pppoe0: the kernel runs stable, there's
not witness log showing up and creating and destroying hundreds of
additional pppoe(4) devices works without disruption.
Is this the right direction?
Index: if_pppoe.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.73
diff -u -p -r1.73 if_pppoe.c
--- if_pppoe.c 13 Sep 2020 11:00:40 -0000 1.73
+++ if_pppoe.c 13 Sep 2020 11:31:12 -0000
@@ -114,15 +114,18 @@ struct pppoetag {
#define PPPOE_DISC_MAXPADI 4 /* retry PADI four times
(quickly) */
#define PPPOE_DISC_MAXPADR 2 /* retry PADR twice */
+struct rwlock pppoe_lock = RWLOCK_INITIALIZER("pppoe");
+
/*
* Locks used to protect struct members and global data
* I immutable after creation
* N net lock
+ * p pppoe lock
*/
struct pppoe_softc {
struct sppp sc_sppp; /* contains a struct ifnet as first
element */
- LIST_ENTRY(pppoe_softc) sc_list;/* [N] */
+ LIST_ENTRY(pppoe_softc) sc_list;/* [p] */
unsigned int sc_eth_ifidx; /* [N] */
int sc_state; /* [N] discovery phase or session
connected */
@@ -233,7 +236,7 @@ pppoe_clone_create(struct if_clone *ifc,
bpfattach(&sc->sc_sppp.pp_if.if_bpf, &sc->sc_sppp.pp_if, DLT_PPP_ETHER,
0);
#endif
- NET_LOCK();
+ rw_enter_write(&pppoe_lock);
retry:
unique = arc4random();
LIST_FOREACH(tmpsc, &pppoe_softc_list, sc_list)
@@ -241,7 +244,7 @@ retry:
goto retry;
sc->sc_unique = unique;
LIST_INSERT_HEAD(&pppoe_softc_list, sc, sc_list);
- NET_UNLOCK();
+ rw_exit_write(&pppoe_lock);
return (0);
}
@@ -252,9 +255,9 @@ pppoe_clone_destroy(struct ifnet *ifp)
{
struct pppoe_softc *sc = ifp->if_softc;
- NET_LOCK();
+ rw_enter_write(&pppoe_lock);
LIST_REMOVE(sc, sc_list);
- NET_UNLOCK();
+ rw_exit_write(&pppoe_lock);
timeout_del(&sc->sc_timeout);
@@ -460,8 +463,10 @@ static void pppoe_dispatch_disc_pkt(stru
err_msg = "TAG HUNIQUE ERROR";
break;
}
+ rw_enter_read(&pppoe_lock);
sc = pppoe_find_softc_by_hunique(mtod(n, caddr_t) +
noff,
len, m->m_pkthdr.ph_ifidx);
+ rw_exit_read(&pppoe_lock);
if (sc != NULL)
devname = sc->sc_sppp.pp_if.if_xname;
break;
@@ -668,8 +673,12 @@ pppoe_data_input(struct mbuf *m)
#ifdef PPPOE_TERM_UNKNOWN_SESSIONS
u_int8_t shost[ETHER_ADDR_LEN];
#endif
- if (LIST_EMPTY(&pppoe_softc_list))
+ rw_enter_read(&pppoe_lock);
+ if (LIST_EMPTY(&pppoe_softc_list)) {
+ rw_exit_read(&pppoe_lock);
goto drop;
+ }
+ rw_exit_read(&pppoe_lock);
KASSERT(m->m_flags & M_PKTHDR);
@@ -699,7 +708,9 @@ pppoe_data_input(struct mbuf *m)
goto drop;
session = ntohs(ph->session);
+ rw_enter_read(&pppoe_lock);
sc = pppoe_find_softc_by_session(session, m->m_pkthdr.ph_ifidx);
+ rw_exit_read(&pppoe_lock);
if (sc == NULL) {
#ifdef PPPOE_TERM_UNKNOWN_SESSIONS
printf("pppoe (data): input for unknown session 0x%x, sending
PADT\n",
