diff options
author | csjp <csjp@FreeBSD.org> | 2006-02-02 03:13:16 +0000 |
---|---|---|
committer | csjp <csjp@FreeBSD.org> | 2006-02-02 03:13:16 +0000 |
commit | 31292a14b63b17ba5db98a0848f9166080240c14 (patch) | |
tree | 70762151820deceeeabd2497892bd015c4607785 /sys | |
parent | ee6a12ceacf369efe24e49fa5ba813f41b43bd89 (diff) | |
download | FreeBSD-src-31292a14b63b17ba5db98a0848f9166080240c14.zip FreeBSD-src-31292a14b63b17ba5db98a0848f9166080240c14.tar.gz |
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
Diffstat (limited to 'sys')
-rw-r--r-- | sys/net/pfil.c | 122 | ||||
-rw-r--r-- | sys/net/pfil.h | 22 | ||||
-rw-r--r-- | sys/netinet/ip_fastfwd.c | 4 | ||||
-rw-r--r-- | sys/netinet/ip_fw2.c | 55 | ||||
-rw-r--r-- | sys/netinet/ip_input.c | 2 | ||||
-rw-r--r-- | sys/netinet/ip_output.c | 2 | ||||
-rw-r--r-- | sys/netinet6/ip6_forward.c | 2 | ||||
-rw-r--r-- | sys/netinet6/ip6_input.c | 2 | ||||
-rw-r--r-- | sys/netinet6/ip6_output.c | 2 |
9 files changed, 50 insertions, 163 deletions
diff --git a/sys/net/pfil.c b/sys/net/pfil.c index 35ac338..78b87f3 100644 --- a/sys/net/pfil.c +++ b/sys/net/pfil.c @@ -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; diff --git a/sys/net/pfil.h b/sys/net/pfil.h index da14f5b..ccf7a65 100644 --- a/sys/net/pfil.h +++ b/sys/net/pfil.h @@ -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) diff --git a/sys/netinet/ip_fastfwd.c b/sys/netinet/ip_fastfwd.c index 6ad8e9d..94da84b 100644 --- a/sys/netinet/ip_fastfwd.c +++ b/sys/netinet/ip_fastfwd.c @@ -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) { diff --git a/sys/netinet/ip_fw2.c b/sys/netinet/ip_fw2.c index e286951..ea43ece 100644 --- a/sys/netinet/ip_fw2.c +++ b/sys/netinet/ip_fw2.c @@ -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, diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c index 6f1130c..2250116 100644 --- a/sys/netinet/ip_input.c +++ b/sys/netinet/ip_input.c @@ -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; diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c index a3fa63b..915512e 100644 --- a/sys/netinet/ip_output.c +++ b/sys/netinet/ip_output.c @@ -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. */ diff --git a/sys/netinet6/ip6_forward.c b/sys/netinet6/ip6_forward.c index 5041fe8..3b542cf 100644 --- a/sys/netinet6/ip6_forward.c +++ b/sys/netinet6/ip6_forward.c @@ -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. */ diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c index 3b34557..a7f5a7c 100644 --- a/sys/netinet6/ip6_input.c +++ b/sys/netinet6/ip6_input.c @@ -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)) diff --git a/sys/netinet6/ip6_output.c b/sys/netinet6/ip6_output.c index 4eb7a6e..57999f8 100644 --- a/sys/netinet6/ip6_output.c +++ b/sys/netinet6/ip6_output.c @@ -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; |