ipfw: Handle count > 1 in commit_rules()

[nc]

Summary

In commit_rules(), allow a chain of multiple rules (count > 1) to be added.

Submitted by: Neel Chauhan <neel AT neelc DOT org>

Test Plan

Attempt to add multiple rules at once in ipfw.

[donner]

There is no implemented use case for count != 1.

The only call with allows a variable count is IP_FW_XADD, which is used in sbin/ipfw/ipfw2.c

if (ts.count != 0) {
        ...
        ctlv->count = 1;
        memcpy(ctlv + 1, rule, rbufsize);
} else {
        ...
        ctlv->count = 1;
}
                
if (do_get3(IP_FW_XADD, op3, &sz) != 0)
        err(EX_UNAVAILABLE, "getsockopt(%s)", "IP_FW_XADD");

So I'd suggest to remove the whole iteration code and replace it by

KASSERT(count == 1, "single rule only");

and reduce code complexity.

[nc]

We do have this piece of code in ipfw_add() in ip_fw_sockopt.c (which I missed in the first try) making your assert unnecessary if we do not make this change:

if (ctlv->count != 1)
        return (ENOTSUP);

However, I believe IP_FW_XADD will call commit_rules() via add_rules() (which is called via do_get3()), so I'm uploading an updated patch removing that code.

[donner]

In D22915#524996, @neel_neelc.org wrote:

However, I believe IP_FW_XADD will call commit_rules() via add_rules() (which is called via do_get3()), so I'm uploading an updated patch removing that code.

If you follow all code paths, you will end in ipfw2.c
If you try to allow more than one rule there, you need to invent a new syntax.
Unless you are really going this path, I oppose to extent the code (due to dead code paths).

[nc]

Makes sense. Abandoning.

[nc]

I changed my mind, I do plan to add new syntax. However, this code hasn't been written yet.