Page MenuHomeFreeBSD

ipfw: Support radix tables and table lookup for MAC addresses

Authored by on Apr 30 2022, 5:40 PM.
Referenced Files
F86202761: D35103.id106113.diff
Sun, Jun 16, 10:45 PM
F86199351: D35103.diff
Sun, Jun 16, 9:33 PM
Unknown Object (File)
Sun, Jun 16, 6:59 AM
Unknown Object (File)
Tue, May 28, 1:30 PM
Unknown Object (File)
Sat, May 25, 4:08 AM
Unknown Object (File)
Tue, May 21, 1:28 AM
Unknown Object (File)
Sun, May 19, 9:50 AM
Unknown Object (File)
May 13 2024, 2:07 PM



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.

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 to enable ipfw filtering on L2 level.

Diff Detail

rG FreeBSD src repository
Lint Skipped
Tests Skipped

Event Timeline

zlei retitled this revision from Support radix tables and table lookup for MAC addresses to ipfw: Support radix tables and table lookup for MAC addresses.May 5 2022, 1:54 AM

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


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?


Could you please explain why those renamings are required?


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?

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.


Yeah, switch sounds good


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


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.

And enum and use switch for lookup instruction marked an inline comment as done.

Remove duplicates

LGTM, added some minor comments.


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.


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).


Worth putting masklen into sa.

This revision is now accepted and ready to land.May 16 2022, 7:15 PM
This revision now requires review to proceed.May 17 2022, 5:48 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 4 2022, 4:20 PM
This revision was automatically updated to reflect the committed changes.