From owner-freebsd-ipfw@FreeBSD.ORG Tue May 8 19:38:18 2007 Return-Path: X-Original-To: freebsd-ipfw@freebsd.org Delivered-To: freebsd-ipfw@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 52EA016A400 for ; Tue, 8 May 2007 19:38:18 +0000 (UTC) (envelope-from julian@elischer.org) Received: from outY.internet-mail-service.net (outY.internet-mail-service.net [216.240.47.248]) by mx1.freebsd.org (Postfix) with ESMTP id 3F75013C44C for ; Tue, 8 May 2007 19:38:18 +0000 (UTC) (envelope-from julian@elischer.org) Received: from mx0.idiom.com (HELO idiom.com) (216.240.32.160) by out.internet-mail-service.net (qpsmtpd/0.32) with ESMTP; Tue, 08 May 2007 12:38:17 -0700 Received: from julian-mac.elischer.org (nat.ironport.com [63.251.108.100]) by idiom.com (Postfix) with ESMTP id 7CB30125B24; Tue, 8 May 2007 12:38:17 -0700 (PDT) Message-ID: <4640D1A3.20605@elischer.org> Date: Tue, 08 May 2007 12:38:11 -0700 From: Julian Elischer User-Agent: Thunderbird 2.0.0.0 (Macintosh/20070326) MIME-Version: 1.0 To: "A. Skrobov" References: <2a38baa40705081130o52ad24d6l9ab1e6d6647e81ef@mail.gmail.com> In-Reply-To: <2a38baa40705081130o52ad24d6l9ab1e6d6647e81ef@mail.gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-ipfw@freebsd.org Subject: Re: a sysctl variable to query last ipfw rule number X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 May 2007 19:38:18 -0000 A. Skrobov wrote: > Such a variable is useful in scripts that add blocks of rules > containing skipto actions; instead of hardcoding numbers for all the > rules, they could be derived dynamically. I'm also looking at a version of skipto that uses RELATIVE numbering. (called just 'skip') i.e. ipfw add 100 skip 50 ip from xxx to yyy ipfw add 120 some rule ipfw add 150 count log ip from any to any # skip rule comes here. > > As an additional bonus, keeping track of the last rule number reduces > overhead in add_rule when no rule number is specified (and partially > puts that overhead to remove_rule instead). Since rules are added more > often than they are deleted, this seems a performance improvement as > well. The one problem I see with this is that you are using a sysctl. This is ok for now but I'm (in the background) working on having more that one instance of a firewall. > > Could someone please review my patch? It's made for a very old ipfw2 > version, the one bundled with 5.4-RELEASE, but the relevant code > doesn't seem to have changed since then. > > *** ip_fw2.c.orig Sun Feb 6 21:16:20 2005 > --- ip_fw2.c Tue May 8 23:38:37 2007 > *************** > *** 191,196 **** > --- 191,197 ---- > > static int fw_debug = 1; > static int autoinc_step = 100; /* bounded to 1..1000 in add_rule() */ > + static unsigned int last_rule = 0; > > #ifdef SYSCTL_NODE > SYSCTL_NODE(_net_inet_ip, OID_AUTO, fw, CTLFLAG_RW, 0, "Firewall"); > *************** > *** 199,204 **** > --- 200,207 ---- > &fw_enable, 0, "Enable ipfw"); > SYSCTL_INT(_net_inet_ip_fw, OID_AUTO, autoinc_step, CTLFLAG_RW, > &autoinc_step, 0, "Rule number autincrement step"); > + SYSCTL_UINT(_net_inet_ip_fw, OID_AUTO, last_rule, CTLFLAG_RD, > + &last_rule, 0, "Number of last added rule"); > SYSCTL_INT(_net_inet_ip_fw, OID_AUTO, one_pass, > CTLFLAG_RW | CTLFLAG_SECURE3, > &fw_one_pass, 0, > *************** > *** 2585,2595 **** > /* > * locate the highest numbered rule before default > */ > ! for (f = chain->rules; f; f = f->next) { > ! if (f->rulenum == IPFW_DEFAULT_RULE) > ! break; > ! rule->rulenum = f->rulenum; > ! } > if (rule->rulenum < IPFW_DEFAULT_RULE - autoinc_step) > rule->rulenum += autoinc_step; > input_rule->rulenum = rule->rulenum; > --- 2588,2594 ---- > /* > * locate the highest numbered rule before default > */ > ! rule->rulenum = last_rule; > if (rule->rulenum < IPFW_DEFAULT_RULE - autoinc_step) > rule->rulenum += autoinc_step; > input_rule->rulenum = rule->rulenum; > *************** > *** 2612,2617 **** > --- 2611,2618 ---- > } > flush_rule_ptrs(chain); > done: > + if (last_rule < rule->rulenum) > + last_rule = rule->rulenum; > static_count++; > static_len += l; > IPFW_WUNLOCK(chain); > *************** > *** 2631,2637 **** > static struct ip_fw * > remove_rule(struct ip_fw_chain *chain, struct ip_fw *rule, struct ip_fw > *prev) > { > ! struct ip_fw *n; > int l = RULESIZE(rule); > > IPFW_WLOCK_ASSERT(chain); > --- 2632,2638 ---- > static struct ip_fw * > remove_rule(struct ip_fw_chain *chain, struct ip_fw *rule, struct ip_fw > *prev) > { > ! struct ip_fw *n, *f; > int l = RULESIZE(rule); > > IPFW_WLOCK_ASSERT(chain); > *************** > *** 2647,2652 **** > --- 2648,2660 ---- > static_count--; > static_len -= l; > > + if (rule->rulenum >= last_rule) /* it should always be <=, but who > knows */ > + for (f = chain->rules; f; f = f->next) { > + if (f->rulenum == IPFW_DEFAULT_RULE) > + break; > + last_rule = f->rulenum; > + } > + > rule->next = chain->reap; > chain->reap = rule; > > *************** > *** 2690,2695 **** > --- 2698,2705 ---- > prev = rule; > rule = rule->next; > } > + > + last_rule = 0; /* how come static_count doesn't need the explicit > reset? */ > } > > /** > *************** > *** 3454,3459 **** > --- 3464,3470 ---- > IPFW_LOCK_DESTROY(&layer3_chain); > return (error); > } > + last_rule = 0; > > ip_fw_default_rule = layer3_chain.rules; > printf("ipfw2 initialized, divert %s, " > _______________________________________________ > freebsd-ipfw@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw > To unsubscribe, send any mail to "freebsd-ipfw-unsubscribe@freebsd.org"