ipfw: Support radix tables and table lookup for MAC addresses

[smalukav_gmail.com]

Summary

At the moment, there is only one way to check a packet's src or dst MAC addresses in ipfw - mac dst-mac src-mac command.
If you want to check a packet against many MAC addresses, you have to add a rule for every of them.
It can be quite slow, e.g. https://lists.freebsd.org/archives/freebsd-ipfw/2021-August/000078.html

By analogy with IP address matching, I've implemented a way to use ipfw radix tables for MAC matching.
I've added a new ipfw table with mac:radix type, and added src-mac and dst-mac lookup commands.

Usage example:

1. ipfw table 1 create type mac
2. ipfw table 1 add 11:22:33:44:55:66/48
3. ipfw add skipto tablearg src-mac 'table(1)' or ipfw add deny src-mac 'table(1, 100)'. ipfw add deny lookup dst-mac 1 syntax is also supported.

Notice that you need to set sysctl net.link.ether.ipfw=1 to enable ipfw filtering on L2 level.

[melifaro]

Thank you for the submission! That's indeed a really good & needed addition for ipfw tables!
Could you please upload a version w/context? (shouldn't be hard to install archanist).
Generally looks good to me, one question though - what was the driver for using radix? In my mental model there's nearly no room for the mac lookups other than direct match. Like, "mark all Apple devices"?

Please see some minor comments inline

sys/netpfil/ipfw/ip_fw2.c

2048

Looks like there's plenty of options here.
Probably worth rethinking this part of code & consider using switch() instead of many if's for each vidx?

sys/netpfil/ipfw/ip_fw_table_algo.c

383

Could you please explain why those renamings are required?

4039

Are there other key lengths? There's a plenty of other places (such as KEY_LEN_MAC) where the length is already hardcoded.
Also: if you want it configurable, probably worth just storing key length on per-table basis?

[smalukav_gmail.com]

In D35103#796527, @melifaro wrote:

what was the driver for using radix?

Only because I thought radix was easier to implement as a POC. I don't know if any mask other than /48 is practical. Perhaps I should implement both radix and hash.

[smalukav_gmail.com]

sys/netpfil/ipfw/ip_fw2.c

2048

Yeah, switch sounds good

sys/netpfil/ipfw/ip_fw_table_algo.c

383

addr_radix functions are used in addr:radix tables, mac_radix functions are used in mac:radix tables, just radix are used in both.

4039

There can be in the future, for example, for InfiniBand address. I used the approach from IPv4 and IPv6 addresses.
If you want to add InfiniBand address support, you should leave KEY_LEN_MAC as if and add KEY_LEN_IB = 20.

[smalukav_gmail.com]

And enum and use switch for lookup instruction

[smalukav_gmail.com]

Remove duplicates

[melifaro]

LGTM, added some minor comments.

sys/netinet/ip_fw.h

795

That's a public header, please add IPFW_ prefix to the name (IPFW_MAX_L2_ADDR_LEN or IPFW_MAXL2SIZE or similar)? Otherwise it may classify with other similar definitions in other headers/programs.

sys/netpfil/ipfw/ip_fw_table_algo.c

359

Worth naming the fields according to other SA-like structures (mac_len, mac_addr).
Also - I'd rather add an explicit spare field between len and addr, so we get 2-byte aligned address & 8 byte-total structure (for ethernet).
You can also store prefixlen in this field (as it's before the radix offset).

4025

Worth putting masklen into sa.

[smalukav_gmail.com]

Swap fields in mac_radix_entry

[smalukav_gmail.com]

Rename sa_mac fields