On Mon, May 17, 2021 at 04:24:15PM +0200, Alexandr Nedvedicky wrote:
> in current tree the ether_input() is protected by NET_LOCK(), which is grabbed
> by caller as a writer. bluhm's diff changes NET_LOCK() readlock, so
> ether_input() can run concurrently. Switching NET_LOCK() to r-lock has
> implications on smr read section above. The ting is the call to eb->eb_input()
> can sleep now. This is something what needs to be avoided within smr section.
Do you think the sleeping occures in PF_LOCK()? Could it be that
this rwlock did never sleep before, but was acquired instantly?
Having sleeps in the packet forwarding path is a bad idea. And
refcounting per packet looks like a workarund. It does not make
things faster.
How about implementing PF_LOCK() with a mutex?
I made such a diff a few months ago. It is not well tested yet.
There were some problems when allocating pf mutex in ioctl path.
mpi@ did not like it, as it removes the pf lock in pf ioctl and
relies on net lock for reconfigure.
It is a design question. Do we want to keep net lock for reconfigure?
If we process packets with shared netlock and reconfigure pf with
exclusive netlock this looks reasonable.
Do we want to go this way? Then I will polish and test this diff.
bluhm
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1116
diff -u -p -r1.1116 pf.c
--- net/pf.c 27 Apr 2021 09:38:29 -0000 1.1116
+++ net/pf.c 17 May 2021 17:38:20 -0000
@@ -6819,6 +6819,8 @@ pf_test(sa_family_t af, int fwdir, struc
if (!pf_status.running)
return (PF_PASS);
+ NET_ASSERT_LOCKED();
+
#if NCARP > 0
if (ifp->if_type == IFT_CARP &&
(ifp0 = if_get(ifp->if_carpdevidx)) != NULL) {
Index: net/pf_ioctl.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.363
diff -u -p -r1.363 pf_ioctl.c
--- net/pf_ioctl.c 9 Feb 2021 23:37:54 -0000 1.363
+++ net/pf_ioctl.c 17 May 2021 17:38:20 -0000
@@ -146,8 +146,8 @@ TAILQ_HEAD(pf_tags, pf_tagname) pf_tags
* grab the lock as writer. Whenever packet creates state it grabs pf_lock
* first then it locks pf_state_lock as the writer.
*/
-struct rwlock pf_lock = RWLOCK_INITIALIZER("pf_lock");
-struct rwlock pf_state_lock = RWLOCK_INITIALIZER("pf_state_lock");
+struct mutex pf_lock = MUTEX_INITIALIZER(IPL_SOFTNET);
+struct mutex pf_state_lock = MUTEX_INITIALIZER(IPL_SOFTNET);
#if (PF_QNAME_SIZE != PF_TAG_NAME_SIZE)
#error PF_QNAME_SIZE must be equal to PF_TAG_NAME_SIZE
@@ -1009,7 +1009,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
case DIOCSTART:
NET_LOCK();
- PF_LOCK();
if (pf_status.running)
error = EEXIST;
else {
@@ -1023,13 +1022,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_create_queues();
DPFPRINTF(LOG_NOTICE, "pf: started");
}
- PF_UNLOCK();
NET_UNLOCK();
break;
case DIOCSTOP:
NET_LOCK();
- PF_LOCK();
if (!pf_status.running)
error = ENOENT;
else {
@@ -1038,7 +1035,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_remove_queues();
DPFPRINTF(LOG_NOTICE, "pf: stopped");
}
- PF_UNLOCK();
NET_UNLOCK();
break;
@@ -1048,7 +1044,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
u_int32_t nr = 0;
NET_LOCK();
- PF_LOCK();
pq->ticket = pf_main_ruleset.rules.active.ticket;
/* save state to not run over them all each time? */
@@ -1058,7 +1053,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
nr++;
}
pq->nr = nr;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1069,10 +1063,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
u_int32_t nr = 0;
NET_LOCK();
- PF_LOCK();
if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1083,12 +1075,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
qs = TAILQ_NEXT(qs, entries);
if (qs == NULL) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
memcpy(&pq->queue, qs, sizeof(pq->queue));
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1100,10 +1090,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
int nbytes;
NET_LOCK();
- PF_LOCK();
if (pq->ticket != pf_main_ruleset.rules.active.ticket) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1116,7 +1104,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
qs = TAILQ_NEXT(qs, entries);
if (qs == NULL) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1131,7 +1118,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
&nbytes);
if (error == 0)
pq->nbytes = nbytes;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1147,10 +1133,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
NET_LOCK();
- PF_LOCK();
if (q->ticket != pf_main_ruleset.rules.inactive.ticket) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_queue_pl, qs);
break;
@@ -1159,7 +1143,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
qs->qid = pf_qname2qid(qs->qname, 1);
if (qs->qid == 0) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_queue_pl, qs);
break;
@@ -1167,7 +1150,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (qs->parent[0] && (qs->parent_qid =
pf_qname2qid(qs->parent, 0)) == 0) {
error = ESRCH;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_queue_pl, qs);
break;
@@ -1175,7 +1157,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
qs->kif = pfi_kif_get(qs->ifname);
if (qs->kif == NULL) {
error = ESRCH;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_queue_pl, qs);
break;
@@ -1184,7 +1165,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pfi_kif_ref(qs->kif, PFI_KIF_REF_RULE);
TAILQ_INSERT_TAIL(pf_queues_inactive, qs, entries);
- PF_UNLOCK();
NET_UNLOCK();
break;
@@ -1202,26 +1182,22 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
NET_LOCK();
- PF_LOCK();
pr->anchor[sizeof(pr->anchor) - 1] = '\0';
ruleset = pf_find_ruleset(pr->anchor);
if (ruleset == NULL) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_rule_pl, rule);
break;
}
if (pr->rule.return_icmp >> 8 > ICMP_MAXTYPE) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_rule_pl, rule);
break;
}
if (pr->ticket != ruleset->rules.inactive.ticket) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_rule_pl, rule);
break;
@@ -1229,7 +1205,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if ((error = pf_rule_copyin(&pr->rule, rule, ruleset))) {
pf_rm_rule(NULL, rule);
rule = NULL;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1249,7 +1224,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_rm_rule(NULL, rule);
rule = NULL;
error = EAFNOSUPPORT;
- PF_UNLOCK();
NET_UNLOCK();
goto fail;
}
@@ -1285,7 +1259,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (error) {
pf_rm_rule(NULL, rule);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1293,7 +1266,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
rule, entries);
rule->ruleset = ruleset;
ruleset->rules.inactive.rcount++;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1304,12 +1276,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pf_rule *tail;
NET_LOCK();
- PF_LOCK();
pr->anchor[sizeof(pr->anchor) - 1] = '\0';
ruleset = pf_find_ruleset(pr->anchor);
if (ruleset == NULL) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1319,7 +1289,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
else
pr->nr = 0;
pr->ticket = ruleset->rules.active.ticket;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1331,18 +1300,15 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
int i;
NET_LOCK();
- PF_LOCK();
pr->anchor[sizeof(pr->anchor) - 1] = '\0';
ruleset = pf_find_ruleset(pr->anchor);
if (ruleset == NULL) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
if (pr->ticket != ruleset->rules.active.ticket) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1351,7 +1317,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
rule = TAILQ_NEXT(rule, entries);
if (rule == NULL) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1369,7 +1334,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pr->rule.ruleset = NULL;
if (pf_anchor_copyout(ruleset, rule, pr)) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1391,7 +1355,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
rule->bytes[0] = rule->bytes[1] = 0;
rule->states_tot = 0;
}
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1415,11 +1378,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
NET_LOCK();
- PF_LOCK();
ruleset = pf_find_ruleset(pcr->anchor);
if (ruleset == NULL) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_rule_pl, newrule);
break;
@@ -1427,7 +1388,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (pcr->action == PF_CHANGE_GET_TICKET) {
pcr->ticket = ++ruleset->rules.active.ticket;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_rule_pl, newrule);
break;
@@ -1435,14 +1395,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (pcr->ticket !=
ruleset->rules.active.ticket) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_rule_pl, newrule);
break;
}
if (pcr->rule.return_icmp >> 8 > ICMP_MAXTYPE) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
pool_put(&pf_rule_pl, newrule);
break;
@@ -1466,7 +1424,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
default:
pf_rm_rule(NULL, newrule);
error = EAFNOSUPPORT;
- PF_UNLOCK();
NET_UNLOCK();
goto fail;
}
@@ -1488,7 +1445,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (error) {
pf_rm_rule(NULL, newrule);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1507,7 +1463,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (newrule != NULL)
pf_rm_rule(NULL, newrule);
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1540,7 +1495,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_calc_skip_steps(ruleset->rules.active.ptr);
pf_remove_if_empty_ruleset(ruleset);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1551,7 +1505,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
u_int killed = 0;
NET_LOCK();
- PF_LOCK();
PF_STATE_ENTER_WRITE();
for (s = RB_MIN(pf_state_tree_id, &tree_id); s; s = nexts) {
nexts = RB_NEXT(pf_state_tree_id, &tree_id, s);
@@ -1571,7 +1524,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
#if NPFSYNC > 0
pfsync_clear_states(pf_status.hostid, psk->psk_ifname);
#endif /* NPFSYNC > 0 */
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1591,14 +1543,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (psk->psk_pfcmp.creatorid == 0)
psk->psk_pfcmp.creatorid = pf_status.hostid;
NET_LOCK();
- PF_LOCK();
PF_STATE_ENTER_WRITE();
if ((s = pf_find_state_byid(&psk->psk_pfcmp))) {
pf_remove_state(s);
psk->psk_killed = 1;
}
PF_STATE_EXIT_WRITE();
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1612,7 +1562,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
key.rdomain = psk->psk_rdomain;
NET_LOCK();
- PF_LOCK();
PF_STATE_ENTER_WRITE();
for (i = 0; i < nitems(dirs); i++) {
if (dirs[i] == PF_IN) {
@@ -1655,13 +1604,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
if (killed)
psk->psk_killed = killed;
PF_STATE_EXIT_WRITE();
- PF_UNLOCK();
NET_UNLOCK();
break;
}
NET_LOCK();
- PF_LOCK();
PF_STATE_ENTER_WRITE();
for (s = RB_MIN(pf_state_tree_id, &tree_id); s;
s = nexts) {
@@ -1709,7 +1656,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
psk->psk_killed = killed;
PF_STATE_EXIT_WRITE();
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1724,9 +1670,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfsync_state_import(sp, PFSYNC_SI_IOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1805,10 +1749,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
case DIOCGETSTATUS: {
struct pf_status *s = (struct pf_status *)addr;
NET_LOCK();
- PF_LOCK();
memcpy(s, &pf_status, sizeof(struct pf_status));
pfi_update_status(s->ifname, s);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1817,16 +1759,13 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pfioc_iface *pi = (struct pfioc_iface *)addr;
NET_LOCK();
- PF_LOCK();
if (pi->pfiio_name[0] == 0) {
memset(pf_status.ifname, 0, IFNAMSIZ);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
strlcpy(pf_trans_set.statusif, pi->pfiio_name, IFNAMSIZ);
pf_trans_set.mask |= PF_TSET_STATUSIF;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1835,11 +1774,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pfioc_iface *pi = (struct pfioc_iface *)addr;
NET_LOCK();
- PF_LOCK();
/* if ifname is specified, clear counters there only */
if (pi->pfiio_name[0]) {
pfi_update_status(pi->pfiio_name, NULL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1849,7 +1786,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
memset(pf_status.scounters, 0, sizeof(pf_status.scounters));
pf_status.since = getuptime();
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1929,12 +1865,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
goto fail;
}
NET_LOCK();
- PF_LOCK();
if (pt->timeout == PFTM_INTERVAL && pt->seconds == 0)
pt->seconds = 1;
pf_default_rule_new.timeout[pt->timeout] = pt->seconds;
pt->seconds = pf_default_rule.timeout[pt->timeout];
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1947,9 +1881,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
goto fail;
}
NET_LOCK();
- PF_LOCK();
pt->seconds = pf_default_rule.timeout[pt->timeout];
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1962,9 +1894,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
goto fail;
}
NET_LOCK();
- PF_LOCK();
pl->limit = pf_pool_limits[pl->index].limit;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -1973,32 +1903,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pfioc_limit *pl = (struct pfioc_limit *)addr;
NET_LOCK();
- PF_LOCK();
if (pl->index < 0 || pl->index >= PF_LIMIT_MAX ||
pf_pool_limits[pl->index].pp == NULL) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
goto fail;
}
if (((struct pool *)pf_pool_limits[pl->index].pp)->pr_nout >
pl->limit) {
error = EBUSY;
- PF_UNLOCK();
NET_UNLOCK();
goto fail;
}
/* Fragments reference mbuf clusters. */
if (pl->index == PF_LIMIT_FRAGS && pl->limit > nmbclust) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
goto fail;
}
pf_pool_limits[pl->index].limit_new = pl->limit;
pl->limit = pf_pool_limits[pl->index].limit;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2007,10 +1932,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
u_int32_t *level = (u_int32_t *)addr;
NET_LOCK();
- PF_LOCK();
pf_trans_set.debug = *level;
pf_trans_set.mask |= PF_TSET_DEBUG;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2021,11 +1944,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pf_anchor *anchor;
NET_LOCK();
- PF_LOCK();
pr->path[sizeof(pr->path) - 1] = '\0';
if ((ruleset = pf_find_ruleset(pr->path)) == NULL) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2040,7 +1961,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
&ruleset->anchor->children)
pr->nr++;
}
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2052,11 +1972,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
u_int32_t nr = 0;
NET_LOCK();
- PF_LOCK();
pr->path[sizeof(pr->path) - 1] = '\0';
if ((ruleset = pf_find_ruleset(pr->path)) == NULL) {
error = EINVAL;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2078,7 +1996,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
}
- PF_UNLOCK();
NET_UNLOCK();
if (!pr->name[0])
error = EBUSY;
@@ -2093,10 +2010,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_clr_tables(&io->pfrio_table, &io->pfrio_ndel,
io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2109,10 +2024,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_add_tables(io->pfrio_buffer, io->pfrio_size,
&io->pfrio_nadd, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2125,10 +2038,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_del_tables(io->pfrio_buffer, io->pfrio_size,
&io->pfrio_ndel, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2141,10 +2052,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_get_tables(&io->pfrio_table, io->pfrio_buffer,
&io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2157,10 +2066,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_get_tstats(&io->pfrio_table, io->pfrio_buffer,
&io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2173,10 +2080,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_clr_tstats(io->pfrio_buffer, io->pfrio_size,
&io->pfrio_nzero, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2189,11 +2094,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_set_tflags(io->pfrio_buffer, io->pfrio_size,
io->pfrio_setflag, io->pfrio_clrflag, &io->pfrio_nchange,
&io->pfrio_ndel, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2206,10 +2109,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_clr_addrs(&io->pfrio_table, &io->pfrio_ndel,
io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2222,11 +2123,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_add_addrs(&io->pfrio_table, io->pfrio_buffer,
io->pfrio_size, &io->pfrio_nadd, io->pfrio_flags |
PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2239,11 +2138,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_del_addrs(&io->pfrio_table, io->pfrio_buffer,
io->pfrio_size, &io->pfrio_ndel, io->pfrio_flags |
PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2256,12 +2153,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_set_addrs(&io->pfrio_table, io->pfrio_buffer,
io->pfrio_size, &io->pfrio_size2, &io->pfrio_nadd,
&io->pfrio_ndel, &io->pfrio_nchange, io->pfrio_flags |
PFR_FLAG_USERIOCTL, 0);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2274,10 +2169,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_get_addrs(&io->pfrio_table, io->pfrio_buffer,
&io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2290,10 +2183,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_get_astats(&io->pfrio_table, io->pfrio_buffer,
&io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2306,11 +2197,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_clr_astats(&io->pfrio_table, io->pfrio_buffer,
io->pfrio_size, &io->pfrio_nzero, io->pfrio_flags |
PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2323,11 +2212,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_tst_addrs(&io->pfrio_table, io->pfrio_buffer,
io->pfrio_size, &io->pfrio_nmatch, io->pfrio_flags |
PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2340,11 +2227,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfr_ina_define(&io->pfrio_table, io->pfrio_buffer,
io->pfrio_size, &io->pfrio_nadd, &io->pfrio_naddr,
io->pfrio_ticket, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2374,12 +2259,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
ioe = malloc(sizeof(*ioe), M_TEMP, M_WAITOK);
table = malloc(sizeof(*table), M_TEMP, M_WAITOK);
NET_LOCK();
- PF_LOCK();
pf_default_rule_new = pf_default_rule;
memset(&pf_trans_set, 0, sizeof(pf_trans_set));
for (i = 0; i < io->size; i++) {
if (copyin(io->array+i, ioe, sizeof(*ioe))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2388,7 +2271,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
sizeof(ioe->anchor)) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2402,7 +2284,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
sizeof(table->pfrt_anchor));
if ((error = pfr_ina_begin(table,
&ioe->ticket, NULL, 0))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2412,7 +2293,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
case PF_TRANS_RULESET:
if ((error = pf_begin_rules(&ioe->ticket,
ioe->anchor))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2420,7 +2300,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
break;
default:
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2428,7 +2307,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
goto fail;
}
if (copyout(ioe, io->array+i, sizeof(io->array[i]))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2436,7 +2314,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
goto fail;
}
}
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2456,10 +2333,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
ioe = malloc(sizeof(*ioe), M_TEMP, M_WAITOK);
table = malloc(sizeof(*table), M_TEMP, M_WAITOK);
NET_LOCK();
- PF_LOCK();
for (i = 0; i < io->size; i++) {
if (copyin(io->array+i, ioe, sizeof(*ioe))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2468,7 +2343,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
sizeof(ioe->anchor)) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2482,7 +2356,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
sizeof(table->pfrt_anchor));
if ((error = pfr_ina_rollback(table,
ioe->ticket, NULL, 0))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2492,7 +2365,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
case PF_TRANS_RULESET:
if ((error = pf_rollback_rules(ioe->ticket,
ioe->anchor))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2500,7 +2372,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
break;
default:
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2508,7 +2379,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
goto fail; /* really bad */
}
}
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2529,11 +2399,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
ioe = malloc(sizeof(*ioe), M_TEMP, M_WAITOK);
table = malloc(sizeof(*table), M_TEMP, M_WAITOK);
NET_LOCK();
- PF_LOCK();
/* first makes sure everything will succeed */
for (i = 0; i < io->size; i++) {
if (copyin(io->array+i, ioe, sizeof(*ioe))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2542,7 +2410,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
sizeof(ioe->anchor)) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2554,7 +2421,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
rs = pf_find_ruleset(ioe->anchor);
if (rs == NULL || !rs->topen || ioe->ticket !=
rs->tticket) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2568,7 +2434,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
!rs->rules.inactive.open ||
rs->rules.inactive.ticket !=
ioe->ticket) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2577,7 +2442,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
break;
default:
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2593,7 +2457,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
for (i = 0; i < PF_LIMIT_MAX; i++) {
if (((struct pool *)pf_pool_limits[i].pp)->pr_nout >
pf_pool_limits[i].limit_new) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2604,7 +2467,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
/* now do the commit - no errors should happen here */
for (i = 0; i < io->size; i++) {
if (copyin(io->array+i, ioe, sizeof(*ioe))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2613,7 +2475,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
if (strnlen(ioe->anchor, sizeof(ioe->anchor)) ==
sizeof(ioe->anchor)) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2627,7 +2488,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
sizeof(table->pfrt_anchor));
if ((error = pfr_ina_commit(table, ioe->ticket,
NULL, NULL, 0))) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2635,6 +2495,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
break;
case PF_TRANS_RULESET:
+ PF_LOCK();
if ((error = pf_commit_rules(ioe->ticket,
ioe->anchor))) {
PF_UNLOCK();
@@ -2643,9 +2504,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
free(ioe, M_TEMP, sizeof(*ioe));
goto fail; /* really bad */
}
+ PF_UNLOCK();
break;
default:
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2658,7 +2519,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_pool_limits[i].limit &&
pool_sethardlimit(pf_pool_limits[i].pp,
pf_pool_limits[i].limit_new, NULL, 0) != 0) {
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2678,7 +2538,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
pfi_xcommit();
pf_trans_set_commit();
- PF_UNLOCK();
NET_UNLOCK();
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
@@ -2694,12 +2553,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pstore = malloc(sizeof(*pstore), M_TEMP, M_WAITOK);
NET_LOCK();
- PF_LOCK();
if (space == 0) {
RB_FOREACH(n, pf_src_tree, &tree_src_tracking)
nr++;
psn->psn_len = sizeof(struct pf_src_node) * nr;
- PF_UNLOCK();
NET_UNLOCK();
free(pstore, M_TEMP, sizeof(*pstore));
break;
@@ -2734,7 +2591,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
error = copyout(pstore, p, sizeof(*p));
if (error) {
- PF_UNLOCK();
NET_UNLOCK();
free(pstore, M_TEMP, sizeof(*pstore));
goto fail;
@@ -2744,7 +2600,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
}
psn->psn_len = sizeof(struct pf_src_node) * nr;
- PF_UNLOCK();
NET_UNLOCK();
free(pstore, M_TEMP, sizeof(*pstore));
break;
@@ -2755,7 +2610,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pf_state *state;
NET_LOCK();
- PF_LOCK();
PF_STATE_ENTER_WRITE();
RB_FOREACH(state, pf_state_tree_id, &tree_id)
pf_src_tree_remove_state(state);
@@ -2763,7 +2617,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
RB_FOREACH(n, pf_src_tree, &tree_src_tracking)
n->expire = 1;
pf_purge_expired_src_nodes();
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2776,7 +2629,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
u_int killed = 0;
NET_LOCK();
- PF_LOCK();
RB_FOREACH(sn, pf_src_tree, &tree_src_tracking) {
if (pf_match_addr(psnk->psnk_src.neg,
&psnk->psnk_src.addr.v.a.addr,
@@ -2804,7 +2656,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_purge_expired_src_nodes();
psnk->psnk_killed = killed;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2813,13 +2664,11 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
u_int32_t *hostid = (u_int32_t *)addr;
NET_LOCK();
- PF_LOCK();
if (*hostid == 0)
pf_trans_set.hostid = arc4random();
else
pf_trans_set.hostid = *hostid;
pf_trans_set.mask |= PF_TSET_HOSTID;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2836,10 +2685,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
break;
}
NET_LOCK();
- PF_LOCK();
error = pfi_get_ifaces(io->pfiio_name, io->pfiio_buffer,
&io->pfiio_size);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2848,9 +2695,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pfioc_iface *io = (struct pfioc_iface *)addr;
NET_LOCK();
- PF_LOCK();
error = pfi_set_flags(io->pfiio_name, io->pfiio_flags);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2859,9 +2704,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pfioc_iface *io = (struct pfioc_iface *)addr;
NET_LOCK();
- PF_LOCK();
error = pfi_clear_flags(io->pfiio_name, io->pfiio_flags);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2870,10 +2713,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
u_int32_t *reass = (u_int32_t *)addr;
NET_LOCK();
- PF_LOCK();
pf_trans_set.reass = *reass;
pf_trans_set.mask |= PF_TSET_REASS;
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2882,9 +2723,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pfioc_synflwats *io = (struct pfioc_synflwats *)addr;
NET_LOCK();
- PF_LOCK();
error = pf_syncookies_setwats(io->hiwat, io->lowat);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2893,9 +2732,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
struct pfioc_synflwats *io = (struct pfioc_synflwats *)addr;
NET_LOCK();
- PF_LOCK();
error = pf_syncookies_getwats(io);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
@@ -2904,9 +2741,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
u_int8_t *mode = (u_int8_t *)addr;
NET_LOCK();
- PF_LOCK();
error = pf_syncookies_setmode(*mode);
- PF_UNLOCK();
NET_UNLOCK();
break;
}
Index: net/pfvar_priv.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfvar_priv.h,v
retrieving revision 1.6
diff -u -p -r1.6 pfvar_priv.h
--- net/pfvar_priv.h 9 Feb 2021 14:06:19 -0000 1.6
+++ net/pfvar_priv.h 17 May 2021 17:38:20 -0000
@@ -37,9 +37,7 @@
#ifdef _KERNEL
-#include <sys/rwlock.h>
-
-extern struct rwlock pf_lock;
+#include <sys/mutex.h>
struct pf_pdesc {
struct {
@@ -104,51 +102,46 @@ extern struct timeout pf_purge_to;
struct pf_state *pf_state_ref(struct pf_state *);
void pf_state_unref(struct pf_state *);
-extern struct rwlock pf_lock;
-extern struct rwlock pf_state_lock;
+extern struct mutex pf_lock;
+extern struct mutex pf_state_lock;
#define PF_LOCK() do { \
NET_ASSERT_LOCKED(); \
- rw_enter_write(&pf_lock); \
+ mtx_enter(&pf_lock); \
} while (0)
#define PF_UNLOCK() do { \
PF_ASSERT_LOCKED(); \
- rw_exit_write(&pf_lock); \
+ mtx_leave(&pf_lock); \
} while (0)
#define PF_ASSERT_LOCKED() do { \
- if (rw_status(&pf_lock) != RW_WRITE) \
- splassert_fail(RW_WRITE, \
- rw_status(&pf_lock),__func__);\
+ MUTEX_ASSERT_LOCKED(&pf_lock); \
} while (0)
#define PF_ASSERT_UNLOCKED() do { \
- if (rw_status(&pf_lock) == RW_WRITE) \
- splassert_fail(0, rw_status(&pf_lock), __func__);\
+ MUTEX_ASSERT_UNLOCKED(&pf_lock); \
} while (0)
#define PF_STATE_ENTER_READ() do { \
- rw_enter_read(&pf_state_lock); \
+ mtx_enter(&pf_state_lock); \
} while (0)
#define PF_STATE_EXIT_READ() do { \
- rw_exit_read(&pf_state_lock); \
+ mtx_leave(&pf_state_lock); \
} while (0)
#define PF_STATE_ENTER_WRITE() do { \
- rw_enter_write(&pf_state_lock); \
+ mtx_enter(&pf_state_lock); \
} while (0)
#define PF_STATE_EXIT_WRITE() do { \
PF_ASSERT_STATE_LOCKED(); \
- rw_exit_write(&pf_state_lock); \
+ mtx_leave(&pf_state_lock); \
} while (0)
#define PF_ASSERT_STATE_LOCKED() do { \
- if (rw_status(&pf_state_lock) != RW_WRITE)\
- splassert_fail(RW_WRITE, \
- rw_status(&pf_state_lock), __func__);\
+ MUTEX_ASSERT_LOCKED(&pf_state_lock); \
} while (0)
extern void pf_purge_timeout(void *);
