Page MenuHomeFreeBSD
Differential D35103

ipfw: Support radix tables and table lookup for MAC addresses
ClosedPublic
Actions

Authored by smalukav_gmail.com on Apr 30 2022, 5:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 23, 3:44 PM
Unknown Object (File)
Mon, Dec 23, 9:36 AM
Unknown Object (File)
Wed, Dec 11, 10:20 AM
Unknown Object (File)
Tue, Dec 10, 6:21 PM
Unknown Object (File)
Mon, Dec 9, 3:51 AM
Unknown Object (File)
Wed, Nov 27, 12:37 PM
Unknown Object (File)
Wed, Nov 27, 12:37 PM
Unknown Object (File)
Wed, Nov 27, 12:35 PM
View All Files
Subscribers
ae
donner
glebius
imp
jlduran
melifaro
sddex_ya.ru

Details

Reviewers
melifaro
Group Reviewers
network
Commits
rGc31f8b7bd895: ipfw: add support radix tables and table lookup for MAC addresses
rG81cac3906eb9: ipfw: add support radix tables and table lookup for MAC addresses
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.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

smalukav_gmail.com created this revision.Apr 30 2022, 5:40 PM
Herald added subscribers: glebius, donner, melifaro and 2 others. · View Herald TranscriptApr 30 2022, 5:40 PM
smalukav_gmail.com requested review of this revision.Apr 30 2022, 5:40 PM
jlduran added a subscriber: jlduran.May 2 2022, 10:02 PM
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
melifaro added a comment.May 6 2022, 1:37 PM

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 updated this revision to Diff 105860.May 11 2022, 4:44 PM
smalukav_gmail.com added a comment.May 11 2022, 4:49 PM
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 added inline comments.May 11 2022, 5:00 PM
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 updated this revision to Diff 105999.May 14 2022, 4:06 PM

And enum and use switch for lookup instruction

smalukav_gmail.com updated this revision to Diff 106000.May 14 2022, 4:09 PM
smalukav_gmail.com marked an inline comment as done.

Remove duplicates

melifaro accepted this revision.May 16 2022, 7:15 PM

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.

This revision is now accepted and ready to land.May 16 2022, 7:15 PM
smalukav_gmail.com updated this revision to Diff 106112.May 17 2022, 5:48 PM

Swap fields in mac_radix_entry

This revision now requires review to proceed.May 17 2022, 5:48 PM
smalukav_gmail.com marked 5 inline comments as done.May 17 2022, 5:50 PM
smalukav_gmail.com updated this revision to Diff 106113.May 17 2022, 7:45 PM

Rename sa_mac fields

sddex_ya.ru added a subscriber: sddex_ya.ru.May 18 2022, 9:15 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 4 2022, 4:20 PM
Closed by commit rG81cac3906eb9: ipfw: add support radix tables and table lookup for MAC addresses (authored by smalukav_gmail.com, committed by ae). · Explain Why
This revision was automatically updated to reflect the committed changes.
ae added a commit: rG81cac3906eb9: ipfw: add support radix tables and table lookup for MAC addresses.
ae added a commit: rGc31f8b7bd895: ipfw: add support radix tables and table lookup for MAC addresses.Jul 14 2022, 1:07 PM

Revision Contents
Changeset List

PathSize
sbin/
ipfw/
36 lines
2 lines
132 lines
29 lines
sys/
netinet/
33 lines
netpfil/
ipfw/
153 lines
2 lines
55 lines
484 lines

Diff 106112

View Options

sbin/ipfw/ipfw.8

Loading...
View Options

sbin/ipfw/ipfw2.h

Loading...
View Options

sbin/ipfw/ipfw2.c

Loading...
View Options

sbin/ipfw/tables.c

Loading...
View Options

sys/netinet/ip_fw.h

Loading...
View Options

sys/netpfil/ipfw/ip_fw2.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_sockopt.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_table.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_table_algo.c

Loading...