We already have "nat tablearg" feature for this task:

single rule

ipfw add nat tablearg ip from any to 'table(1)'

for each $k and $i

ipfw table 1 add 192.168.$k.$i $k

Have you tried measuring performance of such configuration? It uses RADIX tree instead of linear seach.

In D23586#517583, @eugen_grosbein.net wrote:

We already have "nat tablearg" feature for this task:

single rule

ipfw add nat tablearg ip from any to 'table(1)'

for each $k and $i

ipfw table 1 add 192.168.$k.$i $k

Have you tried measuring performance of such configuration? It uses RADIX tree instead of linear seach.

You missed the point: tablearg is for selecting the "number" of the nat instance given the rule processing.
This patch is about selecting the nat instance itself given the "number".

For some background about the amount of instances see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219316

In D23586#517583, @eugen_grosbein.net wrote:

We already have "nat tablearg" feature for this task:

The relevant lookup part in ip_fw2.c is

 t = ((ipfw_insn_nat *)cmd)->nat;
 if (t == NULL) {
    nat_id = TARG(cmd->arg1, nat);
    t = (*lookup_nat_ptr)(&chain->nat, nat_id);

    if (t == NULL) {
       retval = IP_FW_DENY;
       break;
    }
    if (cmd->arg1 != IP_FW_TARG)
        ((ipfw_insn_nat *)cmd)->nat = t;
}

The lookup for explicit referencing is cached (in cmd->nat), so the patch has no useful impact at all.
Using tableargs (what I want to do), the caching is disabled, and the lookup for the result of the tablearg is performed for each packet traversing the kernel. So the speedup of this patch is only realized, if the nat table is selected by a tablearg.

General comment: I'd prefer not to add non-resizable hashes. It should be system job, not user, to resize the hash. Unfortunately, there is no existing generic resizable hash primitive in the kernel code currently.

Speaking of this particular case, I would suggest doing it slightly differently.
We know that nat numbers are limited to 65k. Given that, we can simply allocate 65k array of pointers on the first addition of the nat rule, w/o bothering about hash efficiency, resizing, etc.

In D23586#519046, @melifaro wrote:

General comment: I'd prefer not to add non-resizable hashes. It should be system job, not user, to resize the hash. Unfortunately, there is no existing generic resizable hash primitive in the kernel code currently.

Speaking of this particular case, I would suggest doing it slightly differently.
We know that nat numbers are limited to 65k. Given that, we can simply allocate 65k array of pointers on the first addition of the nat rule, w/o bothering about hash efficiency, resizing, etc.

This approach has serious consequences:

• It takes an non-trivial amount of kernel memory. I tend to avoid this.
• It causes a significant performance hit in all cases where the full scan of all instances is required (i.e. dealiasing via "global" or "ipfw show"). This is the reason, why I tried to keep the hash table as small as possible.
• Most use cases for the non-experienced user (or embedded devices) involves only a single NAT instance at all. I don't want to burden them with performance issues of large CGN providers.

Futhermore the benefit of the patch is only realized for tablearg selection of NAT instances. All other cases do access the NAT instances either by caching or by full scan.

In D23586#519077, @lutz_donnerhacke.de wrote:

In D23586#519046, @melifaro wrote:

General comment: I'd prefer not to add non-resizable hashes. It should be system job, not user, to resize the hash. Unfortunately, there is no existing generic resizable hash primitive in the kernel code currently.

Speaking of this particular case, I would suggest doing it slightly differently.
We know that nat numbers are limited to 65k. Given that, we can simply allocate 65k array of pointers on the first addition of the nat rule, w/o bothering about hash efficiency, resizing, etc.

This approach has serious consequences:

• It takes an non-trivial amount of kernel memory. I tend to avoid this.

Well, we already allocate 2 such arrays for the rule index, so 512k won't drastically increase the footprint.
Anyway, I think even the dynamic-sized array capped by max instance number could be implemented relatively easily.

• It causes a significant performance hit in all cases where the full scan of all instances is required (i.e. dealiasing via "global" or "ipfw show"). This is the reason, why I tried to keep the hash table as small as possible.

No. You still keep the list.

• Most use cases for the non-experienced user (or embedded devices) involves only a single NAT instance at all. I don't want to burden them with performance issues of large CGN providers.

Not sure how this is applicable, as the array option does not have any configurable options.

Futhermore the benefit of the patch is only realized for tablearg selection of NAT instances. All other cases do access the NAT instances either by caching or by full scan.

Well, we already allocate 2 such arrays for the rule index, so 512k won't drastically increase the footprint.

Then we can get rid of the whole caching framework, which will decrease the code size considerably. Just lookup the nat instance directly (if the number is static or from tablearg), so more branches in this path. This way the idea become much more interesting.

• It causes a significant performance hit in all cases where the full scan of all instances is required (i.e. dealiasing via "global" or "ipfw show"). This is the reason, why I tried to keep the hash table as small as possible.

No. You still keep the list.

Yep. Only for the purpose of scanning all instances.

This implementation certainly looks better, both eliminating old hacks and improving performance.
Some comments inline.

@melifaro Are your concerns resolved?

This patch causes a hanging kernel, if the ipfw ruleset is modified under stress.
To modify the ruleset, the system needs to be rebooted.

Removing manpages since no man page change is involved.

ipfw_nat is loadable as separate module.
Why we are use M_IPFW memory type ?