Somewhat re-factor the read/write locking mechanism associated with the packet

filtering mechanisms to use the new rwlock(9) locking API:

- Drop the variables stored in the phil_head structure which were specific to
  conditions and the home rolled read/write locking mechanism.
- Drop some includes which were used for condition variables
- Drop the inline functions, and convert them to macros. Also, move these
  macros into pfil.h
- Move pfil list locking macros intp phil.h as well
- Rename ph_busy_count to ph_nhooks. This variable will represent the number
  of IN/OUT hooks registered with the pfil head structure
- Define PFIL_HOOKED macro which evaluates to true if there are any
  hooks to be ran by pfil_run_hooks
- In the IP/IP6 stacks, change the ph_busy_count comparison to use the new
  PFIL_HOOKED macro.
- Drop optimization in pfil_run_hooks which checks to see if there are any
  hooks to be ran, and returns if not. This check is already performed by the
  IP stacks when they call:

        if (!PFIL_HOOKED(ph))
                goto skip_hooks;

- Drop in assertion which makes sure that the number of hooks never drops
  below 0 for good measure. This in theory should never happen, and if it
  does than there are problems somewhere
- Drop special logic around PFIL_WAITOK because rw_wlock(9) does not sleep
- Drop variables which support home rolled read/write locking mechanism from
  the IPFW firewall chain structure.
- Swap out the read/write firewall chain lock internal to use the rwlock(9)
  API instead of our home rolled version
- Convert the inlined functions to macros

Reviewed by:	mlaier, andre, glebius
Thanks to:	jhb for the new locking API
This commit is contained in:
Christian S.J. Peron 2006-02-02 03:13:16 +00:00
parent 06f2859f6d
commit 604afec496
9 changed files with 50 additions and 163 deletions

View file

@ -32,7 +32,9 @@
#include <sys/param.h>
#include <sys/kernel.h>
#include <sys/errno.h>
#include <sys/lock.h>
#include <sys/malloc.h>
#include <sys/rwlock.h>
#include <sys/socket.h>
#include <sys/socketvar.h>
#include <sys/systm.h>
@ -57,57 +59,6 @@ static int pfil_list_remove(pfil_list_t *,
LIST_HEAD(, pfil_head) pfil_head_list =
LIST_HEAD_INITIALIZER(&pfil_head_list);
static __inline void
PFIL_RLOCK(struct pfil_head *ph)
{
mtx_lock(&ph->ph_mtx);
ph->ph_busy_count++;
mtx_unlock(&ph->ph_mtx);
}
static __inline void
PFIL_RUNLOCK(struct pfil_head *ph)
{
mtx_lock(&ph->ph_mtx);
ph->ph_busy_count--;
if (ph->ph_busy_count == 0 && ph->ph_want_write)
cv_signal(&ph->ph_cv);
mtx_unlock(&ph->ph_mtx);
}
static __inline void
PFIL_WLOCK(struct pfil_head *ph)
{
mtx_lock(&ph->ph_mtx);
ph->ph_want_write = 1;
while (ph->ph_busy_count > 0)
cv_wait(&ph->ph_cv, &ph->ph_mtx);
}
static __inline int
PFIL_TRY_WLOCK(struct pfil_head *ph)
{
mtx_lock(&ph->ph_mtx);
ph->ph_want_write = 1;
if (ph->ph_busy_count > 0) {
ph->ph_want_write = 0;
mtx_unlock(&ph->ph_mtx);
return EBUSY;
}
return 0;
}
static __inline void
PFIL_WUNLOCK(struct pfil_head *ph)
{
ph->ph_want_write = 0;
cv_signal(&ph->ph_cv);
mtx_unlock(&ph->ph_mtx);
}
#define PFIL_LIST_LOCK() mtx_lock(&pfil_global_lock)
#define PFIL_LIST_UNLOCK() mtx_unlock(&pfil_global_lock)
/*
* pfil_run_hooks() runs the specified packet filter hooks.
*/
@ -119,20 +70,8 @@ pfil_run_hooks(struct pfil_head *ph, struct mbuf **mp, struct ifnet *ifp,
struct mbuf *m = *mp;
int rv = 0;
if (ph->ph_busy_count == -1)
return (0);
/*
* Prevent packet filtering from starving the modification of
* the packet filters. We would prefer a reader/writer locking
* mechanism with guaranteed ordering, though.
*/
if (ph->ph_want_write) {
m_freem(*mp);
*mp = NULL;
return (ENOBUFS);
}
PFIL_RLOCK(ph);
KASSERT(ph->ph_nhooks >= 0, ("Pfil hook count dropped < 0"));
for (pfh = pfil_hook_get(dir, ph); pfh != NULL;
pfh = TAILQ_NEXT(pfh, pfil_link)) {
if (pfh->pfil_func != NULL) {
@ -165,16 +104,9 @@ pfil_head_register(struct pfil_head *ph)
}
PFIL_LIST_UNLOCK();
if (mtx_initialized(&ph->ph_mtx)) { /* should not happen */
KASSERT((0), ("%s: allready initialized!", __func__));
return EBUSY;
} else {
ph->ph_busy_count = -1;
ph->ph_want_write = 1;
mtx_init(&ph->ph_mtx, "pfil_head_mtx", NULL, MTX_DEF);
cv_init(&ph->ph_cv, "pfil_head_cv");
mtx_lock(&ph->ph_mtx); /* XXX: race? */
}
rw_init(&ph->ph_mtx, "PFil hook read/write mutex");
PFIL_WLOCK(ph);
ph->ph_nhooks = 0;
TAILQ_INIT(&ph->ph_in);
TAILQ_INIT(&ph->ph_out);
@ -182,9 +114,9 @@ pfil_head_register(struct pfil_head *ph)
PFIL_LIST_LOCK();
LIST_INSERT_HEAD(&pfil_head_list, ph, ph_list);
PFIL_LIST_UNLOCK();
PFIL_WUNLOCK(ph);
return (0);
}
@ -205,14 +137,13 @@ pfil_head_unregister(struct pfil_head *ph)
LIST_REMOVE(ph, ph_list);
PFIL_LIST_UNLOCK();
PFIL_WLOCK(ph); /* XXX: may sleep (cv_wait)! */
PFIL_WLOCK(ph);
TAILQ_FOREACH_SAFE(pfh, &ph->ph_in, pfil_link, pfnext)
free(pfh, M_IFADDR);
TAILQ_FOREACH_SAFE(pfh, &ph->ph_out, pfil_link, pfnext)
free(pfh, M_IFADDR);
cv_destroy(&ph->ph_cv);
mtx_destroy(&ph->ph_mtx);
rw_destroy(&ph->ph_mtx);
return (0);
}
@ -269,13 +200,7 @@ pfil_add_hook(int (*func)(void *, struct mbuf **, struct ifnet *, int, struct in
}
/* Lock */
if (flags & PFIL_WAITOK)
PFIL_WLOCK(ph);
else {
err = PFIL_TRY_WLOCK(ph);
if (err)
goto error;
}
PFIL_WLOCK(ph);
/* Add */
if (flags & PFIL_IN) {
@ -284,6 +209,7 @@ pfil_add_hook(int (*func)(void *, struct mbuf **, struct ifnet *, int, struct in
err = pfil_list_add(&ph->ph_in, pfh1, flags & ~PFIL_OUT);
if (err)
goto done;
ph->ph_nhooks++;
}
if (flags & PFIL_OUT) {
pfh2->pfil_func = func;
@ -294,9 +220,9 @@ pfil_add_hook(int (*func)(void *, struct mbuf **, struct ifnet *, int, struct in
pfil_list_remove(&ph->ph_in, func, arg);
goto done;
}
ph->ph_nhooks++;
}
ph->ph_busy_count = 0;
PFIL_WUNLOCK(ph);
return 0;
@ -320,22 +246,18 @@ pfil_remove_hook(int (*func)(void *, struct mbuf **, struct ifnet *, int, struct
{
int err = 0;
if (flags & PFIL_WAITOK)
PFIL_WLOCK(ph);
else {
err = PFIL_TRY_WLOCK(ph);
if (err)
return err;
}
PFIL_WLOCK(ph);
if (flags & PFIL_IN)
if (flags & PFIL_IN) {
err = pfil_list_remove(&ph->ph_in, func, arg);
if ((err == 0) && (flags & PFIL_OUT))
if (err == 0)
ph->ph_nhooks--;
}
if ((err == 0) && (flags & PFIL_OUT)) {
err = pfil_list_remove(&ph->ph_out, func, arg);
if (TAILQ_EMPTY(&ph->ph_in) && TAILQ_EMPTY(&ph->ph_out))
ph->ph_busy_count = -1;
if (err == 0)
ph->ph_nhooks--;
}
PFIL_WUNLOCK(ph);
return err;

View file

@ -36,7 +36,7 @@
#include <sys/queue.h>
#include <sys/_lock.h>
#include <sys/_mutex.h>
#include <sys/condvar.h> /* XXX */
#include <sys/rwlock.h>
struct mbuf;
struct ifnet;
@ -67,14 +67,8 @@ struct pfil_head {
pfil_list_t ph_in;
pfil_list_t ph_out;
int ph_type;
/*
* Locking: use a busycounter per pfil_head.
* Use ph_busy_count = -1 to indicate pfil_head is empty.
*/
int ph_busy_count; /* count of threads with read lock */
int ph_want_write; /* want write lock flag */
struct cv ph_cv; /* for waking up writers */
struct mtx ph_mtx; /* mutex on locking state */
int ph_nhooks;
struct rwlock ph_mtx;
union {
u_long phu_val;
void *phu_ptr;
@ -97,11 +91,17 @@ int pfil_head_unregister(struct pfil_head *);
struct pfil_head *pfil_head_get(int, u_long);
#define PFIL_HOOKED(p) (&(p)->ph_nhooks > 0)
#define PFIL_RLOCK(p) rw_rlock(&(p)->ph_mtx)
#define PFIL_WLOCK(p) rw_wlock(&(p)->ph_mtx)
#define PFIL_RUNLOCK(p) rw_runlock(&(p)->ph_mtx)
#define PFIL_WUNLOCK(p) rw_wunlock(&(p)->ph_mtx)
#define PFIL_LIST_LOCK() mtx_lock(&pfil_global_lock)
#define PFIL_LIST_UNLOCK() mtx_unlock(&pfil_global_lock)
static __inline struct packet_filter_hook *
pfil_hook_get(int dir, struct pfil_head *ph)
{
KASSERT(ph->ph_busy_count > 0,
("pfil_hook_get: called on unbusy pfil_head"));
if (dir == PFIL_IN)
return (TAILQ_FIRST(&ph->ph_in));
else if (dir == PFIL_OUT)

View file

@ -345,7 +345,7 @@ ip_fastforward(struct mbuf *m)
/*
* Run through list of ipfilter hooks for input packets
*/
if (inet_pfil_hook.ph_busy_count == -1)
if (!PFIL_HOOKED(&inet_pfil_hook))
goto passin;
if (pfil_run_hooks(&inet_pfil_hook, &m, m->m_pkthdr.rcvif, PFIL_IN, NULL) ||
@ -430,7 +430,7 @@ passin:
/*
* Run through list of hooks for output packets.
*/
if (inet_pfil_hook.ph_busy_count == -1)
if (!PFIL_HOOKED(&inet_pfil_hook))
goto passout;
if (pfil_run_hooks(&inet_pfil_hook, &m, ifp, PFIL_OUT, NULL) || m == NULL) {

View file

@ -50,9 +50,11 @@
#include <sys/malloc.h>
#include <sys/mbuf.h>
#include <sys/kernel.h>
#include <sys/lock.h>
#include <sys/jail.h>
#include <sys/module.h>
#include <sys/proc.h>
#include <sys/rwlock.h>
#include <sys/socket.h>
#include <sys/socketvar.h>
#include <sys/sysctl.h>
@ -131,54 +133,20 @@ struct ip_fw_chain {
struct ip_fw *rules; /* list of rules */
struct ip_fw *reap; /* list of rules to reap */
struct radix_node_head *tables[IPFW_TABLES_MAX];
struct mtx mtx; /* lock guarding rule list */
int busy_count; /* busy count for rw locks */
int want_write;
struct cv cv;
struct rwlock rwmtx;
};
#define IPFW_LOCK_INIT(_chain) \
mtx_init(&(_chain)->mtx, "IPFW static rules", NULL, \
MTX_DEF | MTX_RECURSE)
#define IPFW_LOCK_DESTROY(_chain) mtx_destroy(&(_chain)->mtx)
rw_init(&(_chain)->rwmtx, "IPFW static rules")
#define IPFW_LOCK_DESTROY(_chain) rw_destroy(&(_chain)->rwmtx)
#define IPFW_WLOCK_ASSERT(_chain) do { \
mtx_assert(&(_chain)->mtx, MA_OWNED); \
rw_assert(rw, RA_WLOCKED); \
NET_ASSERT_GIANT(); \
} while (0)
static __inline void
IPFW_RLOCK(struct ip_fw_chain *chain)
{
mtx_lock(&chain->mtx);
chain->busy_count++;
mtx_unlock(&chain->mtx);
}
static __inline void
IPFW_RUNLOCK(struct ip_fw_chain *chain)
{
mtx_lock(&chain->mtx);
chain->busy_count--;
if (chain->busy_count == 0 && chain->want_write)
cv_signal(&chain->cv);
mtx_unlock(&chain->mtx);
}
static __inline void
IPFW_WLOCK(struct ip_fw_chain *chain)
{
mtx_lock(&chain->mtx);
chain->want_write++;
while (chain->busy_count > 0)
cv_wait(&chain->cv, &chain->mtx);
}
static __inline void
IPFW_WUNLOCK(struct ip_fw_chain *chain)
{
chain->want_write--;
cv_signal(&chain->cv);
mtx_unlock(&chain->mtx);
}
#define IPFW_RLOCK(p) rw_rlock(&(p)->rwmtx)
#define IPFW_RUNLOCK(p) rw_runlock(&(p)->rwmtx)
#define IPFW_WLOCK(p) rw_wlock(&(p)->rwmtx)
#define IPFW_WUNLOCK(p) rw_wunlock(&(p)->rwmtx)
/*
* list of rules for layer 3
@ -4155,9 +4123,6 @@ ipfw_init(void)
#endif
layer3_chain.rules = NULL;
layer3_chain.want_write = 0;
layer3_chain.busy_count = 0;
cv_init(&layer3_chain.cv, "Condition variable for IPFW rw locks");
IPFW_LOCK_INIT(&layer3_chain);
ipfw_dyn_rule_zone = uma_zcreate("IPFW dynamic rule zone",
sizeof(ipfw_dyn_rule), NULL, NULL, NULL, NULL,

View file

@ -403,7 +403,7 @@ tooshort:
*/
/* Jump over all PFIL processing if hooks are not active. */
if (inet_pfil_hook.ph_busy_count == -1)
if (!PFIL_HOOKED(&inet_pfil_hook))
goto passin;
odst = ip->ip_dst;

View file

@ -405,7 +405,7 @@ sendit:
#endif /* IPSEC */
/* Jump over all PFIL processing if hooks are not active. */
if (inet_pfil_hook.ph_busy_count == -1)
if (!PFIL_HOOKED(&inet_pfil_hook))
goto passout;
/* Run through list of hooks for output packets. */

View file

@ -611,7 +611,7 @@ ip6_forward(m, srcrt)
in6_clearscope(&ip6->ip6_dst);
/* Jump over all PFIL processing if hooks are not active. */
if (inet6_pfil_hook.ph_busy_count == -1)
if (!PFIL_HOOKED(&inet6_pfil_hook))
goto pass;
/* Run through list of hooks for output packets. */

View file

@ -414,7 +414,7 @@ ip6_input(m)
odst = ip6->ip6_dst;
/* Jump over all PFIL processing if hooks are not active. */
if (inet6_pfil_hook.ph_busy_count == -1)
if (!PFIL_HOOKED(&inet6_pfil_hook))
goto passin;
if (pfil_run_hooks(&inet6_pfil_hook, &m, m->m_pkthdr.rcvif, PFIL_IN, NULL))

View file

@ -889,7 +889,7 @@ again:
}
/* Jump over all PFIL processing if hooks are not active. */
if (inet6_pfil_hook.ph_busy_count == -1)
if (!PFIL_HOOKED(&inet6_pfil_hook))
goto passout;
odst = ip6->ip6_dst;