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 "[EMAIL PROTECTED]"

_______________________________________________
freebsd-ipfw@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to