Module Name: src
Committed By: rmind
Date: Sun Sep 29 17:00:29 UTC 2019
Modified Files:
src/sys/net/npf: npf_conn.c npf_if.c npf_impl.h npf_ruleset.c
Log Message:
NPF ifmap: rework and fix a few small bugs.
To generate a diff of this commit:
cvs rdiff -u -r1.29 -r1.30 src/sys/net/npf/npf_conn.c
cvs rdiff -u -r1.10 -r1.11 src/sys/net/npf/npf_if.c
cvs rdiff -u -r1.79 -r1.80 src/sys/net/npf/npf_impl.h
cvs rdiff -u -r1.48 -r1.49 src/sys/net/npf/npf_ruleset.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/net/npf/npf_conn.c
diff -u src/sys/net/npf/npf_conn.c:1.29 src/sys/net/npf/npf_conn.c:1.30
--- src/sys/net/npf/npf_conn.c:1.29 Tue Aug 6 11:40:15 2019
+++ src/sys/net/npf/npf_conn.c Sun Sep 29 17:00:29 2019
@@ -107,7 +107,7 @@
#ifdef _KERNEL
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_conn.c,v 1.29 2019/08/06 11:40:15 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_conn.c,v 1.30 2019/09/29 17:00:29 rmind Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -782,7 +782,8 @@ npf_conn_export(npf_t *npf, npf_conn_t *
nvlist_add_number(cdict, "flags", con->c_flags);
nvlist_add_number(cdict, "proto", con->c_proto);
if (con->c_ifid) {
- const char *ifname = npf_ifmap_getname(npf, con->c_ifid);
+ char ifname[IFNAMSIZ];
+ npf_ifmap_copyname(npf, con->c_ifid, ifname, sizeof(ifname));
nvlist_add_string(cdict, "ifname", ifname);
}
nvlist_add_binary(cdict, "state", &con->c_state, sizeof(npf_state_t));
Index: src/sys/net/npf/npf_if.c
diff -u src/sys/net/npf/npf_if.c:1.10 src/sys/net/npf/npf_if.c:1.11
--- src/sys/net/npf/npf_if.c:1.10 Sun Aug 11 20:26:33 2019
+++ src/sys/net/npf/npf_if.c Sun Sep 29 17:00:29 2019
@@ -1,4 +1,5 @@
/*-
+ * Copyright (c) 2019 Mindaugas Rasiukevicius <rmind at noxt eu>
* Copyright (c) 2013 The NetBSD Foundation, Inc.
* All rights reserved.
*
@@ -28,23 +29,34 @@
*/
/*
- * NPF network interface handling module.
+ * NPF network interface handling.
*
- * NPF uses its own interface IDs (npf-if-id). When NPF configuration is
- * (re)loaded, each required interface name is registered and a matching
- * network interface gets an ID assigned. If an interface is not present,
- * it gets an ID on attach.
+ * NPF uses its own interface IDs (npf-if-id). These IDs start from 1.
+ * Zero is reserved to indicate "no interface" case or an interface of
+ * no interest (i.e. not registered).
*
- * IDs start from 1. Zero is reserved to indicate "no interface" case or
- * an interface of no interest (i.e. not registered).
+ * This module provides an interface to primarily handle the following:
*
- * The IDs are mapped synchronously based on interface events which are
- * monitored using pfil(9) hooks.
+ * - Bind a symbolic interface name to NPF interface ID.
+ * - Associate NPF interface ID when the network interface is attached.
+ *
+ * When NPF configuration is (re)loaded, each referenced network interface
+ * name is registered with a unique ID. If the network interface is already
+ * attached, then the ID is associated with it immediately; otherwise, IDs
+ * are associated/disassociated on interface events which are monitored
+ * using pfil(9) hooks.
+ *
+ * To avoid race conditions when an active NPF configuration is updated or
+ * interfaces are detached/attached, the interface names are never removed
+ * and therefore IDs are never re-assigned. The only point when interface
+ * names and IDs are cleared is when the configuration is flushed.
+ *
+ * A linear counter is used for IDs.
*/
#ifdef _KERNEL
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_if.c,v 1.10 2019/08/11 20:26:33 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_if.c,v 1.11 2019/09/29 17:00:29 rmind Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -55,9 +67,13 @@ __KERNEL_RCSID(0, "$NetBSD: npf_if.c,v 1
#include "npf_impl.h"
typedef struct npf_ifmap {
- char n_ifname[IFNAMSIZ];
+ char ifname[IFNAMSIZ + 1];
} npf_ifmap_t;
+#define NPF_IFMAP_NOID (0U)
+#define NPF_IFMAP_SLOT2ID(npf, slot) ((npf)->ifmap_off + (slot) + 1)
+#define NPF_IFMAP_ID2SLOT(npf, id) ((id) - (npf)->ifmap_off - 1)
+
void
npf_ifmap_init(npf_t *npf, const npf_ifops_t *ifops)
{
@@ -66,8 +82,10 @@ npf_ifmap_init(npf_t *npf, const npf_ifo
KASSERT(ifops != NULL);
ifops->flush((void *)(uintptr_t)0);
+ mutex_init(&npf->ifmap_lock, MUTEX_DEFAULT, IPL_SOFTNET);
npf->ifmap = kmem_zalloc(nbytes, KM_SLEEP);
npf->ifmap_cnt = 0;
+ npf->ifmap_off = 0;
npf->ifops = ifops;
}
@@ -75,82 +93,101 @@ void
npf_ifmap_fini(npf_t *npf)
{
const size_t nbytes = sizeof(npf_ifmap_t) * NPF_MAX_IFMAP;
+ mutex_destroy(&npf->ifmap_lock);
kmem_free(npf->ifmap, nbytes);
}
-static u_int
-npf_ifmap_new(npf_t *npf)
-{
- KASSERT(npf_config_locked_p(npf));
-
- for (u_int i = 0; i < npf->ifmap_cnt; i++)
- if (npf->ifmap[i].n_ifname[0] == '\0')
- return i + 1;
-
- if (npf->ifmap_cnt == NPF_MAX_IFMAP) {
- printf("npf_ifmap_new: out of slots; bump NPF_MAX_IFMAP\n");
- return 0;
- }
- return ++npf->ifmap_cnt;
-}
-
-static u_int
+static unsigned
npf_ifmap_lookup(npf_t *npf, const char *ifname)
{
- KASSERT(npf_config_locked_p(npf));
+ KASSERT(mutex_owned(&npf->ifmap_lock));
- for (u_int i = 0; i < npf->ifmap_cnt; i++) {
- npf_ifmap_t *nim = &npf->ifmap[i];
+ for (unsigned i = 0; i < npf->ifmap_cnt; i++) {
+ npf_ifmap_t *ifmap = &npf->ifmap[i];
- if (nim->n_ifname[0] && strcmp(nim->n_ifname, ifname) == 0)
- return i + 1;
+ if (strcmp(ifmap->ifname, ifname) == 0) {
+ return NPF_IFMAP_SLOT2ID(npf, i);
+ }
}
- return 0;
+ return NPF_IFMAP_NOID;
}
-u_int
+/*
+ * npf_ifmap_register: register an interface name; return an assigned
+ * NPF network ID on success (non-zero).
+ *
+ * This routine is mostly called on NPF configuration (re)load for the
+ * interfaces names referenced by the rules.
+ */
+unsigned
npf_ifmap_register(npf_t *npf, const char *ifname)
{
- npf_ifmap_t *nim;
+ npf_ifmap_t *ifmap;
+ unsigned id, i;
ifnet_t *ifp;
- u_int i;
- npf_config_enter(npf);
- if ((i = npf_ifmap_lookup(npf, ifname)) != 0) {
+ mutex_enter(&npf->ifmap_lock);
+ if ((id = npf_ifmap_lookup(npf, ifname)) != NPF_IFMAP_NOID) {
goto out;
}
- if ((i = npf_ifmap_new(npf)) == 0) {
+ if (npf->ifmap_cnt == NPF_MAX_IFMAP) {
+ printf("npf_ifmap_new: out of slots; bump NPF_MAX_IFMAP\n");
+ id = NPF_IFMAP_NOID;
goto out;
}
- nim = &npf->ifmap[i - 1];
- strlcpy(nim->n_ifname, ifname, IFNAMSIZ);
+ KASSERT(npf->ifmap_cnt < NPF_MAX_IFMAP);
+
+ /* Allocate a new slot and convert and assign an ID. */
+ i = npf->ifmap_cnt++;
+ ifmap = &npf->ifmap[i];
+ strlcpy(ifmap->ifname, ifname, IFNAMSIZ);
+ id = NPF_IFMAP_SLOT2ID(npf, i);
if ((ifp = npf->ifops->lookup(ifname)) != NULL) {
- npf->ifops->setmeta(ifp, (void *)(uintptr_t)i);
+ npf->ifops->setmeta(ifp, (void *)(uintptr_t)id);
}
out:
- npf_config_exit(npf);
- return i;
+ mutex_exit(&npf->ifmap_lock);
+ return id;
}
void
npf_ifmap_flush(npf_t *npf)
{
- KASSERT(npf_config_locked_p(npf));
-
- for (u_int i = 0; i < npf->ifmap_cnt; i++) {
- npf->ifmap[i].n_ifname[0] = '\0';
+ mutex_enter(&npf->ifmap_lock);
+ npf->ifops->flush((void *)(uintptr_t)NPF_IFMAP_NOID);
+ for (unsigned i = 0; i < npf->ifmap_cnt; i++) {
+ npf->ifmap[i].ifname[0] = '\0';
}
npf->ifmap_cnt = 0;
- npf->ifops->flush((void *)(uintptr_t)0);
+
+ /*
+ * Reset the ID counter if reaching the overflow; this is not
+ * realistic, but we maintain correctness.
+ */
+ if (npf->ifmap_off < (UINT_MAX - NPF_MAX_IFMAP)) {
+ npf->ifmap_off += NPF_MAX_IFMAP;
+ } else {
+ npf->ifmap_off = 0;
+ }
+ mutex_exit(&npf->ifmap_lock);
}
-u_int
+/*
+ * npf_ifmap_getid: get the ID for the given network interface.
+ *
+ * => This routine is typically called from the packet handler when
+ * matching whether the packet is on particular network interface.
+ *
+ * => This routine is lock-free; if the NPF configuration is flushed
+ * while the packet is in-flight, the ID will not match because we
+ * keep the IDs linear.
+ */
+unsigned
npf_ifmap_getid(npf_t *npf, const ifnet_t *ifp)
{
- const u_int i = (uintptr_t)npf->ifops->getmeta(ifp);
- KASSERT(i <= npf->ifmap_cnt);
- return i;
+ const unsigned id = (uintptr_t)npf->ifops->getmeta(ifp);
+ return id;
}
/*
@@ -158,45 +195,47 @@ npf_ifmap_getid(npf_t *npf, const ifnet_
* lock, but it is only used temporarily and only for logging.
*/
void
-npf_ifmap_copyname(npf_t *npf, u_int id, char *buf, size_t len)
+npf_ifmap_copylogname(npf_t *npf, unsigned id, char *buf, size_t len)
{
- if (id > 0 && id < npf->ifmap_cnt)
- strlcpy(buf, npf->ifmap[id - 1].n_ifname,
- MIN(len, sizeof(npf->ifmap[id - 1].n_ifname)));
- else
+ if (id != NPF_IFMAP_NOID) {
+ const unsigned i = NPF_IFMAP_ID2SLOT(npf, id);
+ npf_ifmap_t *ifmap = &npf->ifmap[i];
+
+ /*
+ * Lock-free access is safe as there is an extra byte
+ * with a permanent NUL terminator at the end.
+ */
+ strlcpy(buf, ifmap->ifname, MIN(len, IFNAMSIZ));
+ } else {
strlcpy(buf, "???", len);
+ }
}
-const char *
-npf_ifmap_getname(npf_t *npf, const u_int id)
+void
+npf_ifmap_copyname(npf_t *npf, unsigned id, char *buf, size_t len)
{
- const char *ifname;
-
- KASSERT(npf_config_locked_p(npf));
- KASSERT(id > 0 && id <= npf->ifmap_cnt);
-
- ifname = npf->ifmap[id - 1].n_ifname;
- KASSERT(ifname[0] != '\0');
- return ifname;
+ mutex_enter(&npf->ifmap_lock);
+ npf_ifmap_copylogname(npf, id, buf, len);
+ mutex_exit(&npf->ifmap_lock);
}
__dso_public void
npfk_ifmap_attach(npf_t *npf, ifnet_t *ifp)
{
const npf_ifops_t *ifops = npf->ifops;
- u_int i;
+ unsigned id;
- npf_config_enter(npf);
- i = npf_ifmap_lookup(npf, ifops->getname(ifp));
- ifops->setmeta(ifp, (void *)(uintptr_t)i);
- npf_config_exit(npf);
+ mutex_enter(&npf->ifmap_lock);
+ id = npf_ifmap_lookup(npf, ifops->getname(ifp));
+ ifops->setmeta(ifp, (void *)(uintptr_t)id);
+ mutex_exit(&npf->ifmap_lock);
}
__dso_public void
npfk_ifmap_detach(npf_t *npf, ifnet_t *ifp)
{
/* Diagnostic. */
- npf_config_enter(npf);
- npf->ifops->setmeta(ifp, (void *)(uintptr_t)0);
- npf_config_exit(npf);
+ mutex_enter(&npf->ifmap_lock);
+ npf->ifops->setmeta(ifp, (void *)(uintptr_t)NPF_IFMAP_NOID);
+ mutex_exit(&npf->ifmap_lock);
}
Index: src/sys/net/npf/npf_impl.h
diff -u src/sys/net/npf/npf_impl.h:1.79 src/sys/net/npf/npf_impl.h:1.80
--- src/sys/net/npf/npf_impl.h:1.79 Sun Aug 25 17:38:25 2019
+++ src/sys/net/npf/npf_impl.h Sun Sep 29 17:00:29 2019
@@ -234,6 +234,8 @@ struct npf {
const npf_ifops_t * ifops;
struct npf_ifmap * ifmap;
unsigned ifmap_cnt;
+ unsigned ifmap_off;
+ kmutex_t ifmap_lock;
/* Associated worker thread. */
unsigned worker_id;
@@ -319,8 +321,8 @@ void npf_ifmap_fini(npf_t *);
u_int npf_ifmap_register(npf_t *, const char *);
void npf_ifmap_flush(npf_t *);
u_int npf_ifmap_getid(npf_t *, const ifnet_t *);
-const char * npf_ifmap_getname(npf_t *, const u_int);
-void npf_ifmap_copyname(npf_t *, u_int, char *, size_t);
+void npf_ifmap_copylogname(npf_t *, unsigned, char *, size_t);
+void npf_ifmap_copyname(npf_t *, unsigned, char *, size_t);
void npf_ifaddr_sync(npf_t *, ifnet_t *);
void npf_ifaddr_flush(npf_t *, ifnet_t *);
Index: src/sys/net/npf/npf_ruleset.c
diff -u src/sys/net/npf/npf_ruleset.c:1.48 src/sys/net/npf/npf_ruleset.c:1.49
--- src/sys/net/npf/npf_ruleset.c:1.48 Tue Jul 23 00:52:01 2019
+++ src/sys/net/npf/npf_ruleset.c Sun Sep 29 17:00:29 2019
@@ -33,7 +33,7 @@
#ifdef _KERNEL
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_ruleset.c,v 1.48 2019/07/23 00:52:01 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_ruleset.c,v 1.49 2019/09/29 17:00:29 rmind Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -680,7 +680,8 @@ npf_rule_export(npf_t *npf, const npf_ru
nvlist_add_binary(rule, "code", rl->r_code, rl->r_clen);
}
if (rl->r_ifid) {
- const char *ifname = npf_ifmap_getname(npf, rl->r_ifid);
+ char ifname[IFNAMSIZ];
+ npf_ifmap_copyname(npf, rl->r_ifid, ifname, sizeof(ifname));
nvlist_add_string(rule, "ifname", ifname);
}
nvlist_add_number(rule, "id", rl->r_id);
