bridge: allow IP addresses on members to be disabled

[ivy]

Summary

add a new sysctl, net.link.bridge.member_ifaddrs, which defaults to 1.

if it is set to 1, bridge behaviour is unchanged.

if it is set to 0:

• an interface which has AF_INET6 or AF_INET addresses assigned cannot be added to a bridge.
• an interface in a bridge cannot have an AF_INET6 or AF_INET address assigned to it.
• the bridge will no longer consider the lladdrs on bridge members to be local addresses, i.e. frames sent to member lladdrs will not be processed by the host.

update bridge.4 to document this behaviour, as well as the existing
recommendation that IP addresses should not be configured on bridge
members anyway, even if it currently partially works.

in testing, setting this to 0 on a bridge with 50 member interfaces
improved throughput by 22% (4.61Gb/s -> 5.67Gb/s) across two member
epairs due to eliding the bridge member list walk in GRAB_OUR_PACKETS.

[ivy]

some previous discussion on this at https://github.com/freebsd/freebsd-src/pull/1641, i'm moving it here to get a bit more feedback.

[kp]

I'm very happy to see this work, not so much for the performance improvement (although we're not going to say no to that!), but because users keep getting caught out when they assign addresses to bridge member interfaces. That breaks multicast, for example, which is bad for IPv4 and catastrophic for IPv6. This has been documented for a long time, but users keep doing it anyway. Making it impossible will save a lot of frustration.

We've discussed (on GitHub) defaulting this to 0 (i.e. disallow addresses on the member interfaces) for 16 or perhaps even 15. Once this lands we should make noise about that idea on appropriate lists (freebsd-net for starters).

Useful follow-up work:

• improve ifconfig's error handling
• default change

[guest-patmaddox]

In D49995#1140226, @kp wrote:

I'm very happy to see this work, not so much for the performance improvement (although we're not going to say no to that!), but because users keep getting caught out when they assign addresses to bridge member interfaces. That breaks multicast, for example, which is bad for IPv4 and catastrophic for IPv6. This has been documented for a long time, but users keep doing it anyway. Making it impossible will save a lot of frustration.

Where is this currently documented? In if_bridge(4) the only somewhat-related part I see is:

in some of such cases it is better to assign the IP address only to the if_bridge interface and not to the bridge members.

Which to me at least implies that it is reasonable to assign IPs to bridge members.

The way I’ve been (mis?)- using it is to add my hardware interface, as well as a bunch of epairs for jails. The hardware interface has an IP address. Based on my understanding of this review, that setup is not recommended and may eventually be unsupported? However, this is the setup documented in the handbook on vnet jails [1].

Have I misunderstood what is being discussed here, or the current docs, or are some of these in conflict with one another?

[1] https://docs.freebsd.org/en/books/handbook/jails/#creating-vnet-jail

[kp]

In D49995#1140390, @guest-patmaddox wrote:

In D49995#1140226, @kp wrote:

I'm very happy to see this work, not so much for the performance improvement (although we're not going to say no to that!), but because users keep getting caught out when they assign addresses to bridge member interfaces. That breaks multicast, for example, which is bad for IPv4 and catastrophic for IPv6. This has been documented for a long time, but users keep doing it anyway. Making it impossible will save a lot of frustration.

Where is this currently documented? In if_bridge(4) the only somewhat-related part I see is:

https://docs.freebsd.org/en/books/handbook/advanced-networking/
"If the bridge host needs an IP address, set it on the bridge interface, not on the member interfaces."

That should probably be a bit more prominent, but actually making incorrect configuration impossible is a better solution than documenting the pitfalls in a bigger font.

in some of such cases it is better to assign the IP address only to the if_bridge interface and not to the bridge members.

Which to me at least implies that it is reasonable to assign IPs to bridge members.

It isn't. It's broken. It's always been broken.

The way I’ve been (mis?)- using it is to add my hardware interface, as well as a bunch of epairs for jails. The hardware interface has an IP address. Based on my understanding of this review, that setup is not recommended and may eventually be unsupported? However, this is the setup documented in the handbook on vnet jails [1].

Have I misunderstood what is being discussed here, or the current docs, or are some of these in conflict with one another?

[1] https://docs.freebsd.org/en/books/handbook/jails/#creating-vnet-jail

This assigns the IP address to the bridge, which is the correct configuration.

[guest-patmaddox]

In D49995#1140411, @kp wrote:

In D49995#1140390, @guest-patmaddox wrote:

In D49995#1140226, @kp wrote:

I'm very happy to see this work, not so much for the performance improvement (although we're not going to say no to that!), but because users keep getting caught out when they assign addresses to bridge member interfaces. That breaks multicast, for example, which is bad for IPv4 and catastrophic for IPv6. This has been documented for a long time, but users keep doing it anyway. Making it impossible will save a lot of frustration.

Where is this currently documented? In if_bridge(4) the only somewhat-related part I see is:

https://docs.freebsd.org/en/books/handbook/advanced-networking/
"If the bridge host needs an IP address, set it on the bridge interface, not on the member interfaces."

That should probably be a bit more prominent, but actually making incorrect configuration impossible is a better solution than documenting the pitfalls in a bigger font.

Agreed. Glad to see this change, and the additional documentation. I've been doing it wrong for a couple years :)

I wonder if the handbook would benefit from a followup update? At least in my case, I interpreted bridge with vnet jails as _adding_ a bridge interface, and thus maintaining the existing hardware interface config. But as I've learned here, introducing a bridge interface actually requires _changing_ the hardware interface config. I think the current docs are clear now that I understand the underlying principle... but I'm trying to view it from the standpoint of someone progressing from a single hardware interface to a bridge.

in some of such cases it is better to assign the IP address only to the if_bridge interface and not to the bridge members.

Which to me at least implies that it is reasonable to assign IPs to bridge members.

It isn't. It's broken. It's always been broken.

Perhaps this passage needs to be removed from if_bridge(4) then?

The way I’ve been (mis?)- using it is to add my hardware interface, as well as a bunch of epairs for jails. The hardware interface has an IP address. Based on my understanding of this review, that setup is not recommended and may eventually be unsupported? However, this is the setup documented in the handbook on vnet jails [1].

Have I misunderstood what is being discussed here, or the current docs, or are some of these in conflict with one another?

[1] https://docs.freebsd.org/en/books/handbook/jails/#creating-vnet-jail

This assigns the IP address to the bridge, which is the correct configuration.

Thanks! I saw em0 and assumed it had an IP, but as the example clearly shows, it is only up with no IP.

[ivy]

In D49995#1140390, @guest-patmaddox wrote:

in some of such cases it is better to assign the IP address only to the if_bridge interface and not to the bridge members.

Which to me at least implies that it is reasonable to assign IPs to bridge members.

the language could possibly be clarified here but as stated i don't think it's wrong; it's just describing one particular situation (among many) where assigning addresses to the member doesn't work properly, not saying that it's fine to assign addresses to members if you don't have this problem.

(yes, it is unfortunate that bridge(4) didn't make this more explicit in the past, but as kp mentions this has been documented in the handbook for a while.)