Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 22 Oct 2002 16:27:04 +0400 (MSD)
From:      Maxim Konovalov <maxim@macomnet.ru>
To:        Dan Pelleg <daniel+bsd@pelleg.org>
Cc:        stable@FreeBSD.ORG, <ipfw@FreeBSD.ORG>
Subject:   Re: Call for testers: ipfw(8) limit patch
Message-ID:  <20021022154503.U59161-100000@news1.macomnet.ru>
In-Reply-To: <u2s3cqzsgw2.fsf@gs166.sp.cs.cmu.edu>
References:  <20021021174100.Q1221-100000@news1.macomnet.ru> <u2s3cqzsgw2.fsf@gs166.sp.cs.cmu.edu>

Next in thread | Previous in thread | Raw E-Mail | Index | Archive | Help

Hello Dan,

On 23:58+0400, Oct 21, 2002, Dan Pelleg wrote:

> Maxim Konovalov <maxim@macomnet.ru> writes:
>
> > Hello -stable,
> >
> > A patch below fixes an incorrect logic in remove_dyn_rule() which
> > produces that famous message "OUCH! cannot remove rule..". The second
> > part of the patch limits "drop session" message rate.
> >
> > If you are using or would like to use ipfw(8) limit rules in RELENG_4
> > please try this patch. Please sent your reports directly to me.
> >
> > Thanks in advance.
> >
>
>  Is this for ipfw or for ipfw2? If it's for ipfw, please see kern/32600.
>
> http://www.freebsd.org/cgi/query-pr.cgi?pr=32600

Thanks, your analysis seems correct to me. My fix in remove_dyn_rule()
is pretty the same. Parent re-lookup in install_state() after
EXPIRE_DYN_CHAIN() looks like correct work around too. Here is an
updated patch:

Index: sys/netinet/ip_fw.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_fw.c,v
retrieving revision 1.131.2.35
diff -u -r1.131.2.35 ip_fw.c
--- sys/netinet/ip_fw.c	29 Jul 2002 02:04:25 -0000	1.131.2.35
+++ sys/netinet/ip_fw.c	22 Oct 2002 11:29:42 -0000
@@ -696,11 +696,11 @@
 	    if (zap)
 		zap = force || TIME_LEQ( q->expire , time_second );
 	    /* do not zap parent in first pass, record we need a second pass */
-	    if (q->dyn_type == DYN_LIMIT_PARENT) {
+	    if (zap && q->dyn_type == DYN_LIMIT_PARENT) {
 		max_pass = 1; /* we need a second pass */
-		if (zap == 1 && (pass == 0 || q->count != 0) ) {
+		if (pass == 0 || q->count != 0) {
 		    zap = 0 ;
-		    if (pass == 1) /* should not happen */
+		    if (pass == 1 && force) /* should not happen */
 			printf("OUCH! cannot remove rule, count %d\n",
 				q->count);
 		}
@@ -987,8 +987,20 @@
 	}
 	if (parent->count >= conn_limit) {
 	    EXPIRE_DYN_CHAIN(rule); /* try to expire some */
+	    /*
+	     * The expiry might have removed the parent too.
+	     * We lookup again, which will re-create if necessary.
+	     */
+	    parent = lookup_dyn_parent(&id, rule);
+	    if (parent == NULL) {
+		printf("add parent failed\n");
+		return 1;
+	    }
 	    if (parent->count >= conn_limit) {
-		printf("drop session, too many entries\n");
+		if (fw_verbose && last_log != time_second) {
+			last_log = time_second;
+			printf("drop session, too many entries\n");
+		}
 		return 1;
 	    }
 	}

%%%

-- 
Maxim Konovalov, MAcomnet, Internet Dept., system engineer
phone: +7 (095) 796-9079, mailto:maxim@macomnet.ru




To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-ipfw" in the body of the message




Want to link to this message? Use this URL: <http://docs.FreeBSD.org/cgi/mid.cgi?20021022154503.U59161-100000>