This time it's not clean which lock is really required to protect pipex(4) data structures. In fact KERNL_LOCK() and NET_LOCK() are used simultaneously. It's time to document it.
Only [k] used do mark mutable members due to NET_LOCK() looks be unnecesary and can be killed in future. Index: sys/net/pipex.h =================================================================== RCS file: /cvs/src/sys/net/pipex.h,v retrieving revision 1.21 diff -u -p -r1.21 pipex.h --- sys/net/pipex.h 24 Jan 2017 10:08:30 -0000 1.21 +++ sys/net/pipex.h 25 May 2020 10:12:33 -0000 @@ -183,11 +183,16 @@ extern int pipex_enable; struct pipex_session; -/* pipex context for a interface. */ +/* pipex context for a interface + * + * Locks used to protect struct members: + * I immutable after creation + * k kernel lock + */ struct pipex_iface_context { - struct ifnet *ifnet_this; /* outer interface */ - u_int pipexmode; /* pipex mode */ - /* virtual pipex_session entry for multicast routing */ + struct ifnet *ifnet_this; /* [I] outer interface */ + u_int pipexmode; /* [k] pipex mode */ + /* [I] virtual pipex_session entry for multicast routing */ struct pipex_session *multicast_session; }; Index: sys/net/pipex_local.h =================================================================== RCS file: /cvs/src/sys/net/pipex_local.h,v retrieving revision 1.33 diff -u -p -r1.33 pipex_local.h --- sys/net/pipex_local.h 6 Apr 2020 12:31:30 -0000 1.33 +++ sys/net/pipex_local.h 25 May 2020 10:12:33 -0000 @@ -53,43 +53,50 @@ #define PIPEX_MPPE_NOLDKEY 64 /* should be power of two */ #define PIPEX_MPPE_OLDKEYMASK (PIPEX_MPPE_NOLDKEY - 1) +/* + * Locks used to protect struct members: + * I immutable after creation + * k kernel lock + */ + #ifdef PIPEX_MPPE /* mppe rc4 key */ struct pipex_mppe { - int16_t stateless:1, /* key change mode */ - resetreq:1, + int16_t stateless:1, /* [I] key change mode */ + resetreq:1, /* [k] */ reserved:14; - int16_t keylenbits; /* key length */ - int16_t keylen; - uint16_t coher_cnt; /* cohency counter */ - struct rc4_ctx rc4ctx; - u_char master_key[PIPEX_MPPE_KEYLEN]; /* master key of MPPE */ - u_char session_key[PIPEX_MPPE_KEYLEN]; /* session key of MPPE */ - u_char (*old_session_keys)[PIPEX_MPPE_KEYLEN]; /* old session keys */ + int16_t keylenbits; /* [I] key length */ + int16_t keylen; /* [I] */ + uint16_t coher_cnt; /* [k] cohency counter */ + struct rc4_ctx rc4ctx; /* [k] */ + u_char master_key[PIPEX_MPPE_KEYLEN]; /* [k] master key of MPPE */ + u_char session_key[PIPEX_MPPE_KEYLEN]; /* [k] session key of MPPE */ + u_char (*old_session_keys)[PIPEX_MPPE_KEYLEN]; + /* [k] old session keys */ }; #endif /* PIPEX_MPPE */ #ifdef PIPEX_PPPOE struct pipex_pppoe_session { - u_int over_ifidx; /* ether interface */ + u_int over_ifidx; /* [I] ether interface */ }; #endif /* PIPEX_PPPOE */ #ifdef PIPEX_PPTP struct pipex_pptp_session { /* sequence number gap between pipex and userland */ - int32_t snd_gap; /* gap of our sequence */ - int32_t rcv_gap; /* gap of peer's sequence */ - int32_t ul_snd_una; /* userland send acked seq */ - - uint32_t snd_nxt; /* send next */ - uint32_t rcv_nxt; /* receive next */ - uint32_t snd_una; /* send acked sequence */ - uint32_t rcv_acked; /* recv acked sequence */ - - int winsz; /* windows size */ - int maxwinsz; /* max windows size */ - int peer_maxwinsz; /* peer's max windows size */ + int32_t snd_gap; /* [k] gap of our sequence */ + int32_t rcv_gap; /* [k] gap of peer's sequence */ + int32_t ul_snd_una; /* [k] userland send acked seq */ + + uint32_t snd_nxt; /* [k] send next */ + uint32_t rcv_nxt; /* [k] receive next */ + uint32_t snd_una; /* [k] send acked sequence */ + uint32_t rcv_acked; /* [k] recv acked sequence */ + + int winsz; /* [I] windows size */ + int maxwinsz; /* [I] max windows size */ + int peer_maxwinsz; /* [I] peer's max windows size */ }; #endif /* PIPEX_PPTP */ @@ -123,67 +130,68 @@ struct pipex_pptp_session { */ struct pipex_l2tp_session { /* KEYS for session lookup (host byte order) */ - uint16_t tunnel_id; /* our tunnel-id */ - uint16_t peer_tunnel_id; /* peer's tunnel-id */ + uint16_t tunnel_id; /* [I] our tunnel-id */ + uint16_t peer_tunnel_id; /* [I] peer's tunnel-id */ + + uint32_t option_flags; /* [I] protocol options */ + + int16_t ns_gap; /* [k] gap between userland and pipex */ + int16_t nr_gap; /* [k] gap between userland and pipex */ + uint16_t ul_ns_una; /* [k] unacked sequence number (userland) */ - /* protocol options */ - uint32_t option_flags; + uint16_t ns_nxt; /* [k] next sequence number to send */ + uint16_t ns_una; /* [k] unacked sequence number to send */ - int16_t ns_gap; /* gap between userland and pipex */ - int16_t nr_gap; /* gap between userland and pipex */ - uint16_t ul_ns_una; /* unacked sequence number (userland) */ - - uint16_t ns_nxt; /* next sequence number to send */ - uint16_t ns_una; /* unacked sequence number to send*/ - - uint16_t nr_nxt; /* next sequence number to recv */ - uint16_t nr_acked; /* acked sequence number to recv */ - uint32_t ipsecflowinfo; /* IPsec SA flow id for NAT-T */ + uint16_t nr_nxt; /* [k] next sequence number to recv */ + uint16_t nr_acked; /* [k] acked sequence number to recv */ + uint32_t ipsecflowinfo; /* [k] IPsec SA flow id for NAT-T */ }; #endif /* PIPEX_L2TP */ /* pppac ip-extension sessoin table */ struct pipex_session { - struct radix_node ps4_rn[2]; /* tree glue, and other values */ - struct radix_node ps6_rn[2]; /* tree glue, and other values */ - LIST_ENTRY(pipex_session) session_list; /* all session chain */ - LIST_ENTRY(pipex_session) state_list; /* state list chain */ - LIST_ENTRY(pipex_session) id_chain; /* id hash chain */ + struct radix_node ps4_rn[2]; + /* [k] tree glue, and other values */ + struct radix_node ps6_rn[2]; + /* [k] tree glue, and other values */ + LIST_ENTRY(pipex_session) session_list; /* [k] all session chain */ + LIST_ENTRY(pipex_session) state_list; /* [k] state list chain */ + LIST_ENTRY(pipex_session) id_chain; /* [k] id hash chain */ LIST_ENTRY(pipex_session) peer_addr_chain; - /* peer's address hash chain */ - uint16_t state; /* pipex session state */ + /* [k] peer's address hash chain */ + uint16_t state; /* [k] pipex session state */ #define PIPEX_STATE_INITIAL 0x0000 #define PIPEX_STATE_OPENED 0x0001 #define PIPEX_STATE_CLOSE_WAIT 0x0002 #define PIPEX_STATE_CLOSE_WAIT2 0x0003 #define PIPEX_STATE_CLOSED 0x0004 - uint16_t ip_forward:1, /* {en|dis}ableIP forwarding */ - ip6_forward:1, /* {en|dis}able IPv6 forwarding */ - is_multicast:1, /* virtual entry for multicast */ + uint16_t ip_forward:1, /* [k] {en|dis}ableIP forwarding */ + ip6_forward:1, /* [I] {en|dis}able IPv6 forwarding */ + is_multicast:1, /* [I] virtual entry for multicast */ reserved:13; - uint16_t protocol; /* tunnel protocol (PK) */ - uint16_t session_id; /* session-id (PK) */ - uint16_t peer_session_id; /* peer's session-id */ - uint16_t peer_mru; /* peer's MRU */ - uint32_t timeout_sec; /* idle timeout */ - int ppp_id; /* PPP id */ - - struct sockaddr_in ip_address; /* remote address (AK) */ - struct sockaddr_in ip_netmask; /* remote address mask (AK) */ - struct sockaddr_in6 ip6_address; /* remote IPv6 address */ - int ip6_prefixlen; /* remote IPv6 prefixlen */ + uint16_t protocol; /* [I] tunnel protocol (PK) */ + uint16_t session_id; /* [I] session-id (PK) */ + uint16_t peer_session_id; /* [I] peer's session-id */ + uint16_t peer_mru; /* [I] peer's MRU */ + uint32_t timeout_sec; /* [I] idle timeout */ + int ppp_id; /* [I] PPP id */ + + struct sockaddr_in ip_address; /* [I] remote address (AK) */ + struct sockaddr_in ip_netmask; /* [I] remote address mask (AK) */ + struct sockaddr_in6 ip6_address; /* [I] remote IPv6 address */ + int ip6_prefixlen; /* [I] remote IPv6 prefixlen */ - struct pipex_iface_context* pipex_iface;/* context for interface */ + struct pipex_iface_context* pipex_iface; /* [I] context for interface */ - uint32_t ppp_flags; /* configure flags */ + uint32_t ppp_flags; /* [I] configure flags */ #ifdef PIPEX_MPPE - int ccp_id; /* CCP packet id */ + int ccp_id; /* [k] CCP packet id */ struct pipex_mppe mppe_recv, /* MPPE context for incoming */ mppe_send; /* MPPE context for outgoing */ #endif /*PIPEXMPPE */ - struct pipex_statistics stat; /* statistics */ + struct pipex_statistics stat; /* [k] statistics */ union { #ifdef PIPEX_PPPOE struct pipex_pppoe_session pppoe; /* context for PPPoE */ @@ -201,7 +209,7 @@ struct pipex_session { struct sockaddr_in sin4; struct sockaddr_in6 sin6; struct sockaddr_dl sdl; - } peer, local; + } peer, local; /* [I] */ }; /* gre header */