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-stable" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021022154503.U59161-100000>