Push netlock down to pppx_add_session(). The 'pppx_if' structure has
the `pxi_ready' member to prevent access to incomplete `pxi', so we
don't need to hold netlock during all initialisation process. This
removes potential PR_WAITOK/M_WAITOK allocations impact on packet
processing. Also this removes relock dances around if_attach() and
if_detach() calls.
Do not grab netlock for FIONREAD. mbuf(9) queue doesn't rely on it.
Do not grab netlock around pipex_ioctl() call. pipex(4) has its own
protection and doesn't rely on netlock. We need to unlink pipex(4)
session before destroy associated `pxi', it can't be killed
concurrently. Also this stops to block packet processing when npppd(8)
periodically does PIPEXGCLOSED ioctl(2) commands.
The dummy FIONBIO case doesn't require any lock to be held.
The netlock remains to be taken around pppx_del_session() and
pppx_set_session_descr() because pppx(4) data structures rely on it.
Index: sys/net/if_pppx.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.122
diff -u -p -r1.122 if_pppx.c
--- sys/net/if_pppx.c 29 Aug 2022 07:51:45 -0000 1.122
+++ sys/net/if_pppx.c 1 Nov 2022 10:08:37 -0000
@@ -414,7 +414,6 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
struct pppx_dev *pxd = pppx_dev2pxd(dev);
int error = 0;
- NET_LOCK();
switch (cmd) {
case PIPEXASESSION:
error = pppx_add_session(pxd,
@@ -422,13 +421,17 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
break;
case PIPEXDSESSION:
+ NET_LOCK();
error = pppx_del_session(pxd,
(struct pipex_session_close_req *)addr);
+ NET_UNLOCK();
break;
case PIPEXSIFDESCR:
+ NET_LOCK();
error = pppx_set_session_descr(pxd,
(struct pipex_session_descr_req *)addr);
+ NET_UNLOCK();
break;
case FIONBIO:
@@ -441,7 +444,6 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
error = pipex_ioctl(pxd, cmd, addr);
break;
}
- NET_UNLOCK();
return (error);
}
@@ -607,6 +609,7 @@ pppx_add_session(struct pppx_dev *pxd, s
pxi->pxi_session = session;
+ NET_LOCK();
/* try to set the interface up */
unit = pppx_if_next_unit();
if (unit < 0) {
@@ -624,6 +627,7 @@ pppx_add_session(struct pppx_dev *pxd, s
goto out;
}
LIST_INSERT_HEAD(&pxd->pxd_pxis, pxi, pxi_list);
+ NET_UNLOCK();
snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit);
ifp->if_mtu = req->pr_peer_mru; /* XXX */
@@ -638,13 +642,12 @@ pppx_add_session(struct pppx_dev *pxd, s
/* ifp->if_rdomain = req->pr_rdomain; */
if_counters_alloc(ifp);
- /* XXXSMP breaks atomicity */
- NET_UNLOCK();
if_attach(ifp);
- NET_LOCK();
+ NET_LOCK();
if_addgroup(ifp, "pppx");
if_alloc_sadl(ifp);
+ NET_UNLOCK();
#if NBPFILTER > 0
bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t));
@@ -680,6 +683,7 @@ pppx_add_session(struct pppx_dev *pxd, s
ia->ia_netmask = ia->ia_sockmask.sin_addr.s_addr;
+ NET_LOCK();
error = in_ifinit(ifp, ia, &ifaddr, 1);
if (error) {
printf("pppx: unable to set addresses for %s, error=%d\n",
@@ -687,26 +691,29 @@ pppx_add_session(struct pppx_dev *pxd, s
} else {
if_addrhooks_run(ifp);
}
+ NET_UNLOCK();
error = pipex_link_session(session, ifp, pxd);
if (error)
goto detach;
+ NET_LOCK();
SET(ifp->if_flags, IFF_RUNNING);
pxi->pxi_ready = 1;
+ NET_UNLOCK();
return (error);
detach:
- /* XXXSMP breaks atomicity */
- NET_UNLOCK();
if_detach(ifp);
- NET_LOCK();
+ NET_LOCK();
if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
panic("%s: inconsistent RB tree", __func__);
LIST_REMOVE(pxi, pxi_list);
out:
+ NET_UNLOCK();
+
pool_put(&pppx_if_pl, pxi);
pipex_rele_session(session);
