From c85fe6effa8fa2bb738d91584b44bf73704d1f6d Mon Sep 17 00:00:00 2001 From: mvs Date: Sat, 19 Nov 2022 15:12:38 +0000 Subject: [PATCH] Decrease netlock pressure in pppx(4). 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. Tested by Hrvoje Popovski. ok yasuoka@ --- sys/net/if_pppx.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/sys/net/if_pppx.c b/sys/net/if_pppx.c index 953524b3a31..6ed8bac4628 100644 --- a/sys/net/if_pppx.c +++ b/sys/net/if_pppx.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_pppx.c,v 1.122 2022/08/29 07:51:45 bluhm Exp $ */ +/* $OpenBSD: if_pppx.c,v 1.123 2022/11/19 15:12:38 mvs Exp $ */ /* * Copyright (c) 2010 Claudio Jeker @@ -414,7 +414,6 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) 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 addr, int flags, struct proc *p) 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 addr, int flags, struct proc *p) error = pipex_ioctl(pxd, cmd, addr); break; } - NET_UNLOCK(); return (error); } @@ -607,6 +609,7 @@ pppx_add_session(struct pppx_dev *pxd, struct pipex_session_req *req) 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, struct pipex_session_req *req) 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, struct pipex_session_req *req) /* 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, struct pipex_session_req *req) 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, struct pipex_session_req *req) } 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); -- 2.20.1