Use per-session `pxs_mtx' mutex(9) to protect session context. Except
MPPE encryption, PPPOE sessions are mostly immutable, so no lock
required for that case.
Global pipex(4) data is already protected by `pipex_list_mtx', so
pipex(4) doesn't rely on netlock anymore. Netlock could be also removed
from pppx(4)/pppac(4) layer with next diffs.
This diff doesn't contain (*if_output)() magic.
PPPOE/L2TP cases were additionally tested by Hrvoje Popovski.
Index: sys/net/if_pppx.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.118
diff -u -p -r1.118 if_pppx.c
--- sys/net/if_pppx.c 2 Jul 2022 08:50:42 -0000 1.118
+++ sys/net/if_pppx.c 13 Jul 2022 09:47:24 -0000
@@ -637,8 +637,6 @@ pppx_add_session(struct pppx_dev *pxd, s
ifp->if_softc = pxi;
/* ifp->if_rdomain = req->pr_rdomain; */
if_counters_alloc(ifp);
- /* XXXSMP: be sure pppx_if_qstart() called with NET_LOCK held */
- ifq_set_maxlen(&ifp->if_snd, 1);
/* XXXSMP breaks atomicity */
NET_UNLOCK();
@@ -777,15 +775,27 @@ pppx_if_qstart(struct ifqueue *ifq)
struct ifnet *ifp = ifq->ifq_if;
struct pppx_if *pxi = (struct pppx_if *)ifp->if_softc;
struct mbuf *m;
- int proto;
+ int proto, lock = 0;
+
+ switch (pxi->pxi_session->protocol) {
+ case PIPEX_PROTO_PPTP:
+ case PIPEX_PROTO_L2TP:
+ lock = 1;
+ break;
+ }
+
+ if (lock)
+ mtx_enter(&pxi->pxi_session->pxs_mtx);
- NET_ASSERT_LOCKED();
while ((m = ifq_dequeue(ifq)) != NULL) {
proto = *mtod(m, int *);
m_adj(m, sizeof(proto));
pipex_ppp_output(m, pxi->pxi_session, proto);
}
+
+ if (lock)
+ mtx_leave(&pxi->pxi_session->pxs_mtx);
}
int
@@ -1022,8 +1032,6 @@ pppacopen(dev_t dev, int flags, int mode
ifp->if_output = pppac_output;
ifp->if_qstart = pppac_qstart;
ifp->if_ioctl = pppac_ioctl;
- /* XXXSMP: be sure pppac_qstart() called with NET_LOCK held */
- ifq_set_maxlen(&ifp->if_snd, 1);
if_counters_alloc(ifp);
if_attach(ifp);
@@ -1398,7 +1406,6 @@ pppac_qstart(struct ifqueue *ifq)
struct ip ip;
int rv;
- NET_ASSERT_LOCKED();
while ((m = ifq_dequeue(ifq)) != NULL) {
#if NBPFILTER > 0
if (ifp->if_bpf) {
Index: sys/net/pipex.c
===================================================================
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.145
diff -u -p -r1.145 pipex.c
--- sys/net/pipex.c 12 Jul 2022 08:58:53 -0000 1.145
+++ sys/net/pipex.c 13 Jul 2022 09:47:24 -0000
@@ -90,7 +90,6 @@ struct pool mppe_key_pool;
* Locks used to protect global data
* A atomic operation
* I immutable after creation
- * N net lock
* L pipex_list_mtx
*/
@@ -171,7 +170,6 @@ pipex_ioctl(void *ownersc, u_long cmd, c
{
int ret = 0;
- NET_ASSERT_LOCKED();
switch (cmd) {
case PIPEXGSTAT:
ret = pipex_get_stat((struct pipex_session_stat_req *)data,
@@ -567,8 +565,6 @@ pipex_get_stat(struct pipex_session_stat
struct pipex_session *session;
int error = 0;
- NET_ASSERT_LOCKED();
-
session = pipex_lookup_by_session_id(req->psr_protocol,
req->psr_session_id);
if (session == NULL)
@@ -771,7 +767,7 @@ pipex_timer(void *ignored_arg)
Static void
pipex_ip_output(struct mbuf *m0, struct pipex_session *session)
{
- int is_idle;
+ int is_idle, lock = 0;
if ((session->flags & PIPEX_SFLAGS_MULTICAST) == 0) {
/*
@@ -800,7 +796,20 @@ pipex_ip_output(struct mbuf *m0, struct
goto dropped;
}
+ switch (session->protocol) {
+ case PIPEX_PROTO_PPTP:
+ case PIPEX_PROTO_L2TP:
+ lock = 1;
+ break;
+ }
+
+ if (lock)
+ mtx_enter(&session->pxs_mtx);
+
pipex_ppp_output(m0, session, PPP_IP);
+
+ if (lock)
+ mtx_leave(&session->pxs_mtx);
} else {
struct pipex_session *session_tmp;
struct mbuf *m;
@@ -820,9 +829,22 @@ pipex_ip_output(struct mbuf *m0, struct
mtx_leave(&pipex_list_mtx);
m = m_copym(m0, 0, M_COPYALL, M_NOWAIT);
- if (m != NULL)
+ if (m != NULL) {
+ switch (session_tmp->protocol) {
+ case PIPEX_PROTO_PPTP:
+ case PIPEX_PROTO_L2TP:
+ lock = 1;
+ break;
+ }
+
+ if (lock)
+ mtx_enter(&session_tmp->pxs_mtx);
+
pipex_ppp_output(m, session_tmp, PPP_IP);
- else
+
+ if (lock)
+ mtx_leave(&session_tmp->pxs_mtx);
+ } else
counters_inc(session_tmp->stat_counters,
pxc_oerrors);
@@ -849,8 +871,6 @@ pipex_ppp_output(struct mbuf *m0, struct
{
u_char *cp, hdr[16];
- NET_ASSERT_LOCKED();
-
#ifdef PIPEX_MPPE
if (pipex_session_is_mppe_enabled(session)) {
if (proto == PPP_IP) {
@@ -901,15 +921,22 @@ drop:
Static void
pipex_ppp_input(struct mbuf *m0, struct pipex_session *session, int decrypted)
{
- int proto, hlen = 0;
+ int proto, hlen = 0, locked = 0;
struct mbuf *n;
+ switch (session->protocol) {
+ case PIPEX_PROTO_PPTP:
+ case PIPEX_PROTO_L2TP:
+ locked = 1;
+ break;
+ }
+
KASSERT(m0->m_pkthdr.len >= PIPEX_PPPMINLEN);
proto = pipex_ppp_proto(m0, session, 0, &hlen);
#ifdef PIPEX_MPPE
if (proto == PPP_COMP) {
if (decrypted)
- goto drop;
+ goto drop_locked;
/* checked this on ppp_common_input() already. */
KASSERT(pipex_session_is_mppe_accepted(session));
@@ -918,6 +945,10 @@ pipex_ppp_input(struct mbuf *m0, struct
pipex_mppe_input(m0, session);
return;
}
+
+ if (locked)
+ mtx_leave(&session->pxs_mtx);
+
if (proto == PPP_CCP) {
if (decrypted)
goto drop;
@@ -935,7 +966,7 @@ pipex_ppp_input(struct mbuf *m0, struct
#endif
m_adj(m0, hlen);
pipex_ccp_input(m0, session);
- return;
+ goto out;
}
#endif
m_adj(m0, hlen);
@@ -956,7 +987,7 @@ pipex_ppp_input(struct mbuf *m0, struct
*/
goto drop;
pipex_ip_input(m0, session);
- return;
+ break;
#ifdef INET6
case PPP_IPV6:
if (!decrypted && pipex_session_is_mppe_required(session))
@@ -966,7 +997,7 @@ pipex_ppp_input(struct mbuf *m0, struct
*/
goto drop;
pipex_ip6_input(m0, session);
- return;
+ break;
#endif
default:
if (decrypted)
@@ -976,8 +1007,15 @@ pipex_ppp_input(struct mbuf *m0, struct
goto drop;
}
+out:
+ if (locked)
+ mtx_enter(&session->pxs_mtx);
return;
+
drop:
+ if (locked)
+ mtx_enter(&session->pxs_mtx);
+drop_locked:
m_freem(m0);
counters_inc(session->stat_counters, pxc_ierrors);
}
@@ -1261,6 +1299,7 @@ pipex_pppoe_input(struct mbuf *m0, struc
sizeof(struct pipex_pppoe_header), &pppoe);
hlen = sizeof(struct ether_header) + sizeof(struct pipex_pppoe_header);
+
if ((m0 = pipex_common_input(session, m0, hlen, ntohs(pppoe.length)))
== NULL)
return (NULL);
@@ -1327,6 +1366,8 @@ pipex_pptp_output(struct mbuf *m0, struc
struct ip *ip;
u_char *cp;
+ MUTEX_ASSERT_LOCKED(&session->pxs_mtx);
+
reqlen = PIPEX_IPGRE_HDRLEN + (has_seq + has_ack) * 4;
len = 0;
@@ -1480,7 +1521,6 @@ pipex_pptp_input(struct mbuf *m0, struct
struct pipex_pptp_session *pptp_session;
int rewind = 0;
- NET_ASSERT_LOCKED();
KASSERT(m0->m_pkthdr.len >= PIPEX_IPGRE_HDRLEN);
pptp_session = &session->proto.pptp;
@@ -1510,6 +1550,9 @@ pipex_pptp_input(struct mbuf *m0, struct
seqp = cp;
GETLONG(seq, cp);
}
+
+ mtx_enter(&session->pxs_mtx);
+
if (has_ack) {
ackp = cp;
GETLONG(ack, cp);
@@ -1553,16 +1596,28 @@ pipex_pptp_input(struct mbuf *m0, struct
pipex_pptp_output(NULL, session, 0, 1);
}
+ /*
+ * The following pipex_common_input() will release `pxs_mtx'
+ * deep within if the packet will be consumed. In the error
+ * path lock will be held all the time. So increment `rcv_gap'
+ * here, and on the error path back it out, no atomicity will
+ * be lost in all cases.
+ */
+ if (!rewind)
+ session->proto.pptp.rcv_gap += nseq;
if ((m0 = pipex_common_input(session, m0, hlen, ntohs(gre->len)))
== NULL) {
/* ok, The packet is for PIPEX */
- if (!rewind)
- session->proto.pptp.rcv_gap += nseq;
+ mtx_leave(&session->pxs_mtx);
return (NULL);
}
if (rewind)
goto out_seq;
+ else {
+ /* The packet is not ours, back out `rcv_gap'. */
+ session->proto.pptp.rcv_gap -= nseq;
+ }
not_ours:
seq--; /* revert original seq value */
@@ -1591,6 +1646,8 @@ not_ours:
PUTLONG(ack, ackp);
}
+ mtx_leave(&session->pxs_mtx);
+
return (m0);
out_seq:
pipex_session_log(session, LOG_DEBUG,
@@ -1599,6 +1656,7 @@ out_seq:
pptp_session->rcv_nxt + pptp_session->maxwinsz,
ack, pptp_session->snd_una,
pptp_session->snd_nxt);
+ mtx_leave(&session->pxs_mtx);
/* FALLTHROUGH */
drop:
@@ -1728,6 +1786,8 @@ pipex_pptp_userland_output(struct mbuf *
gre = mtod(m0, struct pipex_gre_header *);
cp = PIPEX_SEEK_NEXTHDR(gre, sizeof(struct pipex_gre_header), u_char *);
+ mtx_enter(&session->pxs_mtx);
+
/*
* overwrite sequence numbers to adjust a gap between pipex and
* userland.
@@ -1748,6 +1808,8 @@ pipex_pptp_userland_output(struct mbuf *
session->proto.pptp.rcv_acked = val32;
}
+ mtx_leave(&session->pxs_mtx);
+
return (m0);
}
#endif /* PIPEX_PPTP */
@@ -1769,6 +1831,8 @@ pipex_l2tp_output(struct mbuf *m0, struc
#endif
struct m_tag *mtag;
+ MUTEX_ASSERT_LOCKED(&session->pxs_mtx);
+
hlen = sizeof(struct pipex_l2tp_header) +
((pipex_session_is_l2tp_data_sequencing_on(session))
? sizeof(struct pipex_l2tp_seq_header) : 0) +
@@ -1940,17 +2004,15 @@ pipex_l2tp_input(struct mbuf *m0, int of
uint32_t ipsecflowinfo)
{
struct pipex_l2tp_session *l2tp_session;
- int length, offset, hlen, nseq;
- u_char *cp, *nsp, *nrp;
+ int length = 0, offset = 0, hlen, nseq;
+ u_char *cp, *nsp = NULL, *nrp = NULL;
uint16_t flags, ns = 0, nr = 0;
int rewind = 0;
- NET_ASSERT_LOCKED();
+ mtx_enter(&session->pxs_mtx);
- length = offset = ns = nr = 0;
l2tp_session = &session->proto.l2tp;
l2tp_session->ipsecflowinfo = ipsecflowinfo;
- nsp = nrp = NULL;
m_copydata(m0, off0, sizeof(flags), &flags);
@@ -2008,15 +2070,28 @@ pipex_l2tp_input(struct mbuf *m0, int of
length -= hlen + offset;
hlen += off0 + offset;
+
+ /*
+ * The following pipex_common_input() will release `pxs_mtx'
+ * deep within if the packet will be consumed. In the error
+ * path lock will be held all the time. So increment `nr_gap'
+ * here, and on the error path back it out, no atomicity will
+ * be lost in all cases.
+ */
+ if (!rewind)
+ session->proto.l2tp.nr_gap += nseq;
if ((m0 = pipex_common_input(session, m0, hlen, length)) == NULL) {
/* ok, The packet is for PIPEX */
- if (!rewind)
- session->proto.l2tp.nr_gap += nseq;
+ mtx_leave(&session->pxs_mtx);
return (NULL);
}
if (rewind)
goto out_seq;
+ else {
+ /* The packet is not ours, backout `nr_gap'. */
+ session->proto.l2tp.nr_gap -= nseq;
+ }
/*
* overwrite sequence numbers to adjust a gap between pipex and
@@ -2042,15 +2117,18 @@ pipex_l2tp_input(struct mbuf *m0, int of
PUTSHORT(nr, nrp);
}
+ mtx_leave(&session->pxs_mtx);
+
return (m0);
out_seq:
pipex_session_log(session, LOG_DEBUG,
"Received bad data packet: out of sequence: seq=%u(%u-) "
"ack=%u(%u-%u)", ns, l2tp_session->nr_nxt, nr, l2tp_session->ns_una,
l2tp_session->ns_nxt);
-
/* FALLTHROUGH */
drop:
+ mtx_leave(&session->pxs_mtx);
+
m_freem(m0);
counters_inc(session->stat_counters, pxc_ierrors);
@@ -2175,6 +2253,8 @@ pipex_l2tp_userland_output(struct mbuf *
ns = ntohs(seq->ns);
nr = ntohs(seq->nr);
+ mtx_enter(&session->pxs_mtx);
+
ns += session->proto.l2tp.ns_gap;
seq->ns = htons(ns);
session->proto.l2tp.ns_nxt++;
@@ -2184,6 +2264,8 @@ pipex_l2tp_userland_output(struct mbuf *
if (SEQ16_GT(nr, session->proto.l2tp.nr_acked))
session->proto.l2tp.nr_acked = nr;
+ mtx_leave(&session->pxs_mtx);
+
return (m0);
}
#endif /* PIPEX_L2TP */
@@ -2443,11 +2525,15 @@ pipex_mppe_input(struct mbuf *m0, struct
/* Send CCP ResetReq */
PIPEX_DBG((session, LOG_DEBUG, "CCP SendResetReq"));
+
+ if (session->protocol == PIPEX_PROTO_PPPOE)
+ mtx_enter(&session->pxs_mtx);
- mtx_enter(&session->pxs_mtx);
ccp_id = session->ccp_id;
session->ccp_id++;
- mtx_leave(&session->pxs_mtx);
+
+ if (session->protocol == PIPEX_PROTO_PPPOE)
+ mtx_leave(&session->pxs_mtx);
pipex_ccp_output(session, CCP_RESETREQ, ccp_id);
goto drop;
Index: sys/net/pipex_local.h
===================================================================
RCS file: /cvs/src/sys/net/pipex_local.h,v
retrieving revision 1.48
diff -u -p -r1.48 pipex_local.h
--- sys/net/pipex_local.h 12 Jul 2022 08:58:53 -0000 1.48
+++ sys/net/pipex_local.h 13 Jul 2022 09:47:24 -0000
@@ -56,8 +56,8 @@ extern struct mutex pipex_list_mtx;
/*
* Locks used to protect struct members:
+ * A atomic operation
* I immutable after creation
- * N net lock
* L pipex_list_mtx
* s this pipex_session' `pxs_mtx'
* m this pipex_mppe' `pxm_mtx'
@@ -91,14 +91,14 @@ struct pipex_pppoe_session {
#ifdef PIPEX_PPTP
struct pipex_pptp_session {
/* sequence number gap between pipex and userland */
- int32_t snd_gap; /* [N] gap of our sequence */
- int32_t rcv_gap; /* [N] gap of peer's sequence */
- int32_t ul_snd_una; /* [N] userland send acked seq */
-
- uint32_t snd_nxt; /* [N] send next */
- uint32_t rcv_nxt; /* [N] receive next */
- uint32_t snd_una; /* [N] send acked sequence */
- uint32_t rcv_acked; /* [N] recv acked sequence */
+ int32_t snd_gap; /* [s] gap of our sequence */
+ int32_t rcv_gap; /* [s] gap of peer's sequence */
+ int32_t ul_snd_una; /* [s] userland send acked seq */
+
+ uint32_t snd_nxt; /* [s] send next */
+ uint32_t rcv_nxt; /* [s] receive next */
+ uint32_t snd_una; /* [s] send acked sequence */
+ uint32_t rcv_acked; /* [s] recv acked sequence */
int winsz; /* [I] windows size */
int maxwinsz; /* [I] max windows size */
@@ -141,16 +141,16 @@ struct pipex_l2tp_session {
uint32_t option_flags; /* [I] protocol options */
- int16_t ns_gap; /* [N] gap between userland and pipex */
- int16_t nr_gap; /* [N] gap between userland and pipex */
- uint16_t ul_ns_una; /* [N] unacked sequence number (userland) */
-
- uint16_t ns_nxt; /* [N] next sequence number to send */
- uint16_t ns_una; /* [N] unacked sequence number to send */
-
- uint16_t nr_nxt; /* [N] next sequence number to recv */
- uint16_t nr_acked; /* [N] acked sequence number to recv */
- uint32_t ipsecflowinfo; /* [N] IPsec SA flow id for NAT-T */
+ int16_t ns_gap; /* [s] gap between userland and pipex */
+ int16_t nr_gap; /* [s] gap between userland and pipex */
+ uint16_t ul_ns_una; /* [s] unacked sequence number (userland) */
+
+ uint16_t ns_nxt; /* [s] next sequence number to send */
+ uint16_t ns_una; /* [s] unacked sequence number to send */
+
+ uint16_t nr_nxt; /* [s] next sequence number to recv */
+ uint16_t nr_acked; /* [s] acked sequence number to recv */
+ uint32_t ipsecflowinfo; /* [s] IPsec SA flow id for NAT-T */
};
#endif /* PIPEX_L2TP */
@@ -197,7 +197,7 @@ struct pipex_session {
struct sockaddr_in6 ip6_address; /* [I] remote IPv6 address */
int ip6_prefixlen; /* [I] remote IPv6 prefixlen */
- u_int ifindex; /* [N] interface index */
+ u_int ifindex; /* [A] interface index */
void *ownersc; /* [I] owner context */
uint32_t ppp_flags; /* [I] configure flags */
