Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/net/npf NPF ifmap: rework and fix a few small bugs.
details: https://anonhg.NetBSD.org/src/rev/1d26771326d7
branches: trunk
changeset: 1003768:1d26771326d7
user: rmind <rmind%NetBSD.org@localhost>
date: Sun Sep 29 17:00:29 2019 +0000
description:
NPF ifmap: rework and fix a few small bugs.
diffstat:
sys/net/npf/npf_conn.c | 5 +-
sys/net/npf/npf_if.c | 197 +++++++++++++++++++++++++++------------------
sys/net/npf/npf_impl.h | 6 +-
sys/net/npf/npf_ruleset.c | 5 +-
4 files changed, 128 insertions(+), 85 deletions(-)
diffs (truncated from 365 to 300 lines):
diff -r 80354d3660c4 -r 1d26771326d7 sys/net/npf/npf_conn.c
--- a/sys/net/npf/npf_conn.c Sun Sep 29 16:58:35 2019 +0000
+++ b/sys/net/npf/npf_conn.c Sun Sep 29 17:00:29 2019 +0000
@@ -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 @@
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));
diff -r 80354d3660c4 -r 1d26771326d7 sys/net/npf/npf_if.c
--- a/sys/net/npf/npf_if.c Sun Sep 29 16:58:35 2019 +0000
+++ b/sys/net/npf/npf_if.c Sun Sep 29 17:00:29 2019 +0000
@@ -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). These 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:
+ *
+ * - Bind a symbolic interface name to NPF interface ID.
+ * - Associate NPF interface ID when the network interface is attached.
*
- * 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.
+ * 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.
*
- * IDs start from 1. Zero is reserved to indicate "no interface" case or
- * an interface of no interest (i.e. not registered).
+ * 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.
*
- * The IDs are mapped synchronously based on interface events which are
- * monitored using pfil(9) hooks.
+ * 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 @@
#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 @@
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 @@
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 (npf->ifmap_cnt == NPF_MAX_IFMAP) {
+ printf("npf_ifmap_new: out of slots; bump NPF_MAX_IFMAP\n");
+ id = NPF_IFMAP_NOID;
goto out;
}
- if ((i = npf_ifmap_new(npf)) == 0) {
- 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 @@
* 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);
Home |
Main Index |
Thread Index |
Old Index