VLAN ID 0 is supposed to be interpreted as having no VLAN with
a bit of priority on the side, but the kernel is not able to
decapsulate this on the fly so dhclient needs to take care of it.
Details
Details
- Reviewers
kevans markj - Commits
- rGabf5bff71d38: dhclient: support VID 0 (no vlan) decapsulation
In production in OPNsense since 21.7 (July 2021)
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 41121 Build 38010: arc lint + arc unit
Event Timeline
| sbin/dhclient/bpf.c | ||
|---|---|---|
| 200 | EVL_VLID_MASK? | |
| sbin/dhclient/bpf.c | ||
|---|---|---|
| 200 | nice catch, thanks | |
| sbin/dhclient/bpf.c | ||
|---|---|---|
| 201 | Yes, saves time. | |
| 218 | I did this first back in 2018 so I had to take a closer look. Comments may not be well-rounded. The biggest issue was that BPF_MSH wasn't going to the index based on variable offset through IP or VLAN situation requiring a static comparison between both cases and calculating the result moving it back. | |
Comment Actions
I think I'm ok with this change. I would suggest adding an additional comment to the beginning of the filter explaining why we look for VLAN tags at all.
Comment Actions
Hide as in hide the vlan header from the BPF filter? Or hide it from all kernel packet processing?
Comment Actions
There is an older discussion about it here https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=224961
Handling it transparently in BPF would avoid the need to do this particular dance, fix all potential consumers and is probably faster. Handling it in the network stack on the other hand is probably a lot of work but would fix non-BPF use cases as well.
FWIW, so far our only report about this in OPNsense was dhclient, but I could look into the BPF approach instead?
Comment Actions
Added motivation for checking for untagged priority to the filter program comments.
After scanning the code I don't feel comfortable bringing the support to bpf(4) in general so this dhclient change is all I can offer at this point.
In any case thanks for review and questions!
Comment Actions
Sorry for the delay. I have no objections to the change and the implementation looks fine. I am a little wary of committing it without some wider approval: could I ask you to post a short note to freebsd-net@ linking this diff and soliciting opinions? If there are no objections after a week or so I will commit this.
Comment Actions
Dispatched a message on Feb 3 but no additional feedback until now, see https://lists.freebsd.org/archives/freebsd-net/2022-February/001248.html