Page MenuHomeFreeBSD

bridge: add vlan filtering support
Needs ReviewPublic

Authored by ivy on Fri, May 23, 11:55 PM.
Tags
None
Referenced Files
F120540532: D50503.id156157.diff
Wed, Jun 18, 11:00 PM
F120477239: D50503.id156211.diff
Wed, Jun 18, 7:29 AM
Unknown Object (File)
Tue, Jun 17, 3:23 AM
Unknown Object (File)
Sat, Jun 14, 4:29 PM
Unknown Object (File)
Fri, Jun 13, 10:37 AM
Unknown Object (File)
Thu, Jun 12, 11:57 PM
Unknown Object (File)
Fri, Jun 6, 5:15 PM
Unknown Object (File)
Sat, May 31, 6:02 PM

Details

Reviewers
des
kevans
kp
Group Reviewers
network
manpages
Summary

The new ifconfig options 'ifvlan', '+ifvlans' and '-ifvlans' allow the
vlan access list of a bridge port to be configured.

Incoming tagged frames will be dropped if the incoming vlan tag isn't
in the port's access list.

Outgoing frames will be dropped if the outgoing vlan ID isn't in the
port's access list (e.g., for BUM traffic).

This is only enabled if the port's pvid is set to something other than
zero, which is treated as the flag to enable vlan filtering.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 64526
Build 61410: arc lint + arc unit

Event Timeline

ivy requested review of this revision.Fri, May 23, 11:55 PM
sbin/ifconfig/ifbridge.c
161

Declaration needs to move up to the top of this block at a minimum, when someone provides more useful feedback to warrant another iteration.

sbin/ifconfig/ifbridge.c
161

i keep forgetting we're not allowed to do this. i'll fix it in my local branch.

ivy marked an inline comment as done.Sat, May 24, 12:16 AM
kp added inline comments.
sbin/ifconfig/ifbridge.c
155

Maybe <= DOT1Q_VID_MAX ?

sys/net/if_bridge.c
1944

(Random example picked)

This might be easier if you used BITSET(9). It has macros for bitwise AND/OR/XOR/.. on the entire set at once.

This revision is now accepted and ready to land.Tue, May 27, 9:08 AM

update for new vlan filtering behaviour

This revision now requires review to proceed.Wed, May 28, 9:32 AM
sbin/ifconfig/ifbridge.c
155

we can't use DOT1Q_VID_* here since they're behind _KERNEL. i've been meaning to fix that and move them to a different header, but i might do that in a followup commit (and fix this at the same time) rather than adding more commits to this stack.

sys/net/if_bridge.c
1944

according to the manual page, bitset(9) isn't available to userland unless _WANT_FREEBSD_BITSET is defined, which means every userland program that includes if_bridgevar.h would have to do that. i'm not very fond of that since it makes the header unnecessarily difficult to use.

sbin/ifconfig/ifbridge.c
155

i decided to just do this: D50570. i'll fix this if that's approved.

use DOT1Q_VID_* constants consistently, now they're available to userland.

also neaten up parse_vlans() a little.

kp added inline comments.
sbin/ifconfig/ifbridge.c
766

We have three near-identical functions here. It might be worth doing something like

static void
setbridge_tagged(if_ctx *ctx, const char *ifn, const char *vlans, int cmd)
...

static void
addbridge_tagged(if_ctx *ctx, const char *ifn, const char *vlans)
{
    setbridge_tagged(ctx, ifn, vlans, BRDG_VLAN_OP_SET);
}
This revision is now accepted and ready to land.Wed, May 28, 7:54 PM
  • remove some redundant code in ifconfig
  • use bitset(9) for ifbvlan_set_t instead of rolling our own
This revision now requires review to proceed.Thu, May 29, 2:33 AM