Here's a start at struct pppoe_softc; for every member I went through
code paths looking for *_LOCK() or *_ASSERT_LOCKED().
Pretty much all members are under the net lock, some are proctected by
both net and kernel lock, e.g. the start routine is called with
KERNEL_LOCK().
I did not go through the sppp struct members yet.
Does this look correct?
>From here on, I think we can start and already pull out a few of those
members and put them under a new pppoe(4) specific lock.
Index: if_pppoe.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.70
diff -u -p -r1.70 if_pppoe.c
--- if_pppoe.c 28 Jul 2020 09:52:32 -0000 1.70
+++ if_pppoe.c 20 Aug 2020 15:27:09 -0000
@@ -114,27 +115,34 @@ struct pppoetag {
#define PPPOE_DISC_MAXPADI 4 /* retry PADI four times
(quickly) */
#define PPPOE_DISC_MAXPADR 2 /* retry PADR twice */
+/*
+ * Locks used to protect struct members and global data
+ * I immutable after creation
+ * K kernel lock
+ * N net lock
+ */
+
struct pppoe_softc {
struct sppp sc_sppp; /* contains a struct ifnet as first
element */
- LIST_ENTRY(pppoe_softc) sc_list;
- unsigned int sc_eth_ifidx;
+ LIST_ENTRY(pppoe_softc) sc_list;/* [N] */
+ unsigned int sc_eth_ifidx; /* [N] */
- int sc_state; /* discovery phase or session connected
*/
- struct ether_addr sc_dest; /* hardware address of concentrator */
- u_int16_t sc_session; /* PPPoE session id */
-
- char *sc_service_name; /* if != NULL: requested name of
service */
- char *sc_concentrator_name; /* if != NULL: requested concentrator
id */
- u_int8_t *sc_ac_cookie; /* content of AC cookie we must echo
back */
- size_t sc_ac_cookie_len; /* length of cookie data */
- u_int8_t *sc_relay_sid; /* content of relay SID we must echo
back */
- size_t sc_relay_sid_len; /* length of relay SID data */
- u_int32_t sc_unique; /* our unique id */
- struct timeout 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
*/
+ int sc_state; /* [N] discovery phase or session
connected */
+ struct ether_addr sc_dest; /* [N] hardware address of concentrator
*/
+ u_int16_t sc_session; /* [N] PPPoE session id */
+
+ char *sc_service_name; /* [N] if != NULL: requested name of
service */
+ char *sc_concentrator_name; /* [N] if != NULL: requested
concentrator id */
+ u_int8_t *sc_ac_cookie; /* [N] content of AC cookie we must
echo back */
+ size_t sc_ac_cookie_len; /* [N] length of cookie data */
+ u_int8_t *sc_relay_sid; /* [N] content of relay SID we must
echo back */
+ size_t sc_relay_sid_len; /* [N] length of relay SID data */
+ u_int32_t sc_unique; /* [I] our unique id */
+ struct timeout sc_timeout; /* [N] timeout while not in session
state */
+ int sc_padi_retried; /* [N] number of PADI retries already
done */
+ int sc_padr_retried; /* [N] number of PADR retries already
done */
- struct timeval sc_session_time; /* time the session was established */
+ struct timeval sc_session_time; /* [N] time the session was established
*/
};
/* incoming traffic will be queued here */
