a sysctl variable to query last ipfw rule number

[Julian Elischer]

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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw
> To unsubscribe, send any mail to "freebsd-ipfw-unsubscribe at freebsd.org"