abe wrote:
> On Thu, Oct 10, 2002 at 09:50:13PM -0700, Bill Fumerola wrote:
> > On Fri, Oct 11, 2002 at 12:46:36AM -0400, abe wrote:
> > static u_int32_t dyn_buckets = 256; /* must be power of 2 */
>
> Well another issue solved, need thicker glasses it appears. Thanks much
> Bill. Funny thing is, it's been running without issue for almost a year
> now. Interesting.
Try setting it back to 500, and see if you can get a crash.
The value of dyn_buckets is seperate from the curr_dyn_buckets
in both netinet/ipfw.c and netinet/ipfw2.c.
In theory, the value is checked for a power of 2 value before
it is used, and that's used to resize. If it isn't a power of
2, then it gets reset to back to curr_dyn_buckets.
There is a window in the resize in netinet/ipfw2.c that could
cause a probem (the old array is freed before the new array
has been successfully allocated -- order of operation bug, IMO).
But there should not be an issue with the size being changed...
particularly on ip_output() called from send() called from user
space.
There's also a problem with initial sizing, and a problem if the
initial allocation fails, and a couple other problems. I have
attached a patch which fixes these problems.
Note: This patch may not fix your "500" problem... the correct
way to fix that is probably to have the sysctl for dyn_buckets
use a set procedure, which refuses the set if it's not a power
of 2 and/or rounds it up to the next power of 2 < 65536.
-- Terry
Index: ip_fw.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_fw.c,v
retrieving revision 1.188
diff -u -r1.188 ip_fw.c
--- ip_fw.c 22 Jun 2002 11:51:02 -0000 1.188
+++ ip_fw.c 11 Oct 2002 03:01:38 -0000
@@ -862,23 +862,40 @@
struct ipfw_dyn_rule *r ;
int i ;
+
+ /* new allocation or reallocation */
if (ipfw_dyn_v == NULL ||
(dyn_count == 0 && dyn_buckets != curr_dyn_buckets)) {
- /* try reallocation, make sure we have a power of 2 */
+ /* make sure we have a power of 2 */
u_int32_t i = dyn_buckets ;
while ( i > 0 && (i & 1) == 0 )
i >>= 1 ;
if (i != 1) /* not a power of 2 */
dyn_buckets = curr_dyn_buckets ; /* reset */
- else {
- curr_dyn_buckets = dyn_buckets ;
- if (ipfw_dyn_v != NULL)
- free(ipfw_dyn_v, M_IPFW);
- ipfw_dyn_v = malloc(curr_dyn_buckets * sizeof r,
+
+ /* new allocation or reallocation; avoid the realloction on reset */
+ if (ipfw_dyn_v == NULL ||
+ (dyn_count == 0 && dyn_buckets != curr_dyn_buckets)) {
+ ipfw_dyn_rule **new_ipfw_dyn_v;
+
+ /*
+ * try the allocation; if it's a reallocation, and the malloc
+ * fails, keep the old area and reset, instead.
+ */
+ new_ipfw_dyn_v = malloc(dyn_buckets * sizeof r,
M_IPFW, M_DONTWAIT | M_ZERO);
- if (ipfw_dyn_v == NULL)
- return NULL; /* failed ! */
+ if (new_ipfw_dyn_v != NULL) {
+ if (ipfw_dyn_v != NULL)
+ free(ipfw_dyn_v, M_IPFW);
+ ipfw_dyn_v = new_ipfw_dyn_v;
+ curr_dyn_buckets = dyn_buckets ;
+ } else {
+ dyn_buckets = curr_dyn_buckets ; /* reset */
+ }
}
+
+ if (ipfw_dyn_v == NULL)
+ return NULL; /* failed ! */
}
i = hash_packet(id);
