NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
kern/38456: ipf mostly ignores locking in NetBSD
>Number: 38456
>Category: kern
>Synopsis: ipf mostly ignores locking in NetBSD
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Apr 18 15:15:00 +0000 2008
>Originator: Wolfgang Solfrank
>Release: NetBSD 4.99.58
>Organization:
Tools GmbH
>Environment:
System: NetBSD sdsl1.tools.de 4.99.58 NetBSD 4.99.58 (sdsl1) #0: Thu Apr 10
18:40:23 MEST 2008
ws%sdsl1.tools.de@localhost:/src/obj/sys/arch/i386/compile/sdsl1 i386
Architecture: i386
Machine: i386
>Description:
Although the code in src/sys/dist/ipf/netinet is ridden with mutexes and
rwlocks,
NetBSD defines most of this stuff away. This can at least result in incorrect
statistics.
>How-To-Repeat:
Code inspection.
In addition I at least once saw output from "ipfstat -f" like this
(reconstructed from memory):
IP fragment states:
26 new
30 expired
30 hits
0 retrans
0 too short
0 no memory
4294967292 already exist
0 inuse
>Fix:
Since the mutexes and rwlocks used in ipf seem to be developed with Solaris
in mind, we can mostly adapt the code for that OS (Note that I didn't bother
to look what exact version of NetBSD comes with the correct locking semantics
as I expect this to just be included in the relevant version(s)):
Index: ip_compat.h
===================================================================
RCS file: /cvsroot/src/sys/dist/ipf/netinet/ip_compat.h,v
retrieving revision 1.19
diff -u -r1.19 ip_compat.h
--- ip_compat.h 1 Mar 2008 14:16:51 -0000 1.19
+++ ip_compat.h 18 Apr 2008 12:16:19 -0000
@@ -737,6 +737,27 @@
# ifdef _KERNEL
# if (__NetBSD_Version__ >= 499000000)
typedef char * caddr_t;
+#include <sys/rwlock.h>
+# define USE_MUTEXES 1
+# define KMUTEX_T kmutex_t
+# define KRWLOCK_T krwlock_t
+# define MUTEX_DESTROY(x) mutex_destroy(&(x)->ipf_lk)
+/* Misleading name: works on rwlocks, not mutexes */
+# define MUTEX_DOWNGRADE(x) rw_downgrade(&(x)->ipf_lk)
+# define MUTEX_ENTER(x) mutex_enter(&(x)->ipf_lk)
+# define MUTEX_EXIT(x) mutex_exit(&(x)->ipf_lk)
+# define MUTEX_INIT(x,y) mutex_init(&(x)->ipf_lk, MUTEX_DRIVER, \
+ IPL_SOFTNET)
+# define MUTEX_NUKE(x) bzero((x), sizeof *(x))
+# define READ_ENTER(x) rw_enter(&(x)->ipf_lk, RW_READER)
+# define RWLOCK_INIT(x,y) rw_init(&(x)->ipf_lk)
+# define RWLOCK_EXIT(x) rw_exit(&(x)->ipf_lk)
+# define RW_DESTROY(x) rw_destroy(&(x)->ipf_lk)
+# define WRITE_ENTER(x) rw_enter(&(x)->ipf_lk, RW_WRITER)
+# define SPL_SCHED(x) ;
+# define SPL_NET(x) ;
+# define SPL_IMP(x) ;
+# define SPL_X(x) ;
# endif
# if (__NetBSD_Version__ >= 399001400)
# define KMALLOCS(a, b, c) (a) = (b)malloc((c), _M_IPF, M_NOWAIT)
Index: ip_fil_netbsd.c
===================================================================
RCS file: /cvsroot/src/sys/dist/ipf/netinet/ip_fil_netbsd.c,v
retrieving revision 1.45
diff -u -r1.45 ip_fil_netbsd.c
--- ip_fil_netbsd.c 1 Mar 2008 14:16:51 -0000 1.45
+++ ip_fil_netbsd.c 18 Apr 2008 12:16:19 -0000
@@ -95,6 +95,14 @@
extern int ip_optcopy __P((struct ip *, struct ip *));
#endif
+ipfmutex_t ipl_mutex, ipf_authmx, ipf_rw;
+ipfmutex_t ipf_timeoutlock, ipf_stinsert, ipf_natio, ipf_nat_new;
+ipfrwlock_t ipf_mutex, ipf_global, ipf_ipidfrag;
+ipfrwlock_t ipf_frag, ipf_state, ipf_nat, ipf_natfrag, ipf_auth;
+ipfrwlock_t ipf_frcache, ipf_tokens;
+
+int ipf_locks_done = 0;
+
#ifdef IPFILTER_M_IPFILTER
MALLOC_DEFINE(M_IPFILTER, "IP Filter", "IP Filter packet filter data
structures");
#endif
@@ -302,9 +310,8 @@
#endif
-int ipfattach()
+int ipfattach(void)
{
- int s;
#if (__NetBSD_Version__ >= 499005500)
int i;
#endif
@@ -321,17 +328,19 @@
# endif
#endif
- SPL_NET(s);
if ((fr_running > 0) || (fr_checkp == fr_check)) {
printf("IP Filter: already initialized\n");
- SPL_X(s);
return EBUSY;
}
- if (fr_initialise() < 0) {
- SPL_X(s);
+ MUTEX_INIT(&ipf_rw, 0);
+ MUTEX_INIT(&ipf_timeoutlock, 0);
+ RWLOCK_INIT(&ipf_ipidfrag, 0);
+ RWLOCK_INIT(&ipf_tokens, 0);
+ ipf_locks_done = 1;
+
+ if (fr_initialise() < 0)
return EIO;
- }
#ifdef NETBSD_PF
# if (__NetBSD_Version__ >= 104200000)
@@ -416,8 +425,6 @@
ipforwarding = 1;
#endif
- SPL_X(s);
-
#if (__NetBSD_Version__ >= 104010000)
callout_init(&fr_slowtimer_ch, 0);
callout_reset(&fr_slowtimer_ch, (hz / IPF_HZ_DIVIDE) * IPF_HZ_MULT,
@@ -430,7 +437,6 @@
#if __NetBSD_Version__ >= 105110000
pfil_error:
fr_deinitialise();
- SPL_X(s);
return error;
#endif
}
@@ -442,7 +448,6 @@
*/
int ipfdetach()
{
- int s;
#if (__NetBSD_Version__ >= 499005500)
int i;
#endif
@@ -459,8 +464,6 @@
# endif
#endif
- SPL_NET(s);
-
#if (__NetBSD_Version__ >= 104010000)
callout_stop(&fr_slowtimer_ch);
#else
@@ -515,12 +518,19 @@
#endif
fr_deinitialise();
- SPL_X(s);
-
#if (__NetBSD_Version__ >= 499005500)
for (i = 0; i < IPL_LOGSIZE; i++)
seldestroy(&ipfselwait[i]);
#endif
+
+ if (ipf_locks_done) {
+ MUTEX_DESTROY(&ipf_rw);
+ MUTEX_DESTROY(&ipf_timeoutlock);
+ RW_DESTROY(&ipf_ipidfrag);
+ RW_DESTROY(&ipf_tokens);
+ ipf_locks_done = 1;
+ }
+
return 0;
}
@@ -555,7 +565,6 @@
int mode;
{
int error = 0, unit = 0;
- SPL_INT(s);
#if (__NetBSD_Version__ >= 399002000)
if ((mode & FWRITE) &&
@@ -583,11 +592,11 @@
return EIO;
}
- SPL_NET(s);
+ READ_ENTER(&ipf_global);
error = fr_ioctlswitch(unit, data, cmd, mode, UID(p), p);
- SPL_X(s);
+ RWLOCK_EXIT(&ipf_global);
return error;
}
@@ -647,12 +656,22 @@
# endif
)
{
+ static int iplocks = 0;
u_int xmin = GET_MINOR(dev);
if (IPL_LOGMAX < xmin)
xmin = ENXIO;
- else
+ else {
xmin = 0;
+
+ if (!iplocks) {
+ RWLOCK_INIT(&ipf_global, 0);
+ RWLOCK_INIT(&ipf_mutex, 0);
+ RWLOCK_INIT(&ipf_frcache, 0);
+ iplocks = 1;
+ }
+ }
+
return xmin;
}
Index: ip_sync.c
===================================================================
RCS file: /cvsroot/src/sys/dist/ipf/netinet/ip_sync.c,v
retrieving revision 1.9
diff -u -r1.9 ip_sync.c
--- ip_sync.c 11 Dec 2007 04:55:04 -0000 1.9
+++ ip_sync.c 18 Apr 2008 12:16:20 -0000
@@ -898,7 +898,7 @@
# else
MUTEX_EXIT(&ipsl_mutex);
# ifdef _KERNEL
- wakeup(&sl_tail);
+ WAKEUP(&sl_tail, 0);
# endif
# endif
return sl;
@@ -986,7 +986,7 @@
# else
MUTEX_EXIT(&ipsl_mutex);
# ifdef _KERNEL
- wakeup(&sl_tail);
+ WAKEUP(&sl_tail, 0);
# endif
# endif
}
Home |
Main Index |
Thread Index |
Old Index