Page MenuHomeFreeBSD

bridge: Simplify mac address generation

Authored by kp on Apr 15 2020, 7:07 PM.



Unconditionally use ether_gen_addr() to generate bridge mac addresses.
This function is now less likely to generate duplicate mac addresses
across jails. The old hand rolled hostid based code adds no value.

Diff Detail

rS FreeBSD src repository - subversion
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bz added inline comments.
746 ↗(On Diff #70617)

I don't fully understand the logic but ether_gen_addr() will always give you the same result in the loop so the outcome of the retry is not going to change, right? Do we need the do { } while() loop still or is that an endless loop now and we should rather have proper error handling? I am not even sure this was sane before... But that's probably a different question to raise.

This revision is now accepted and ready to land.Apr 15 2020, 8:02 PM
746 ↗(On Diff #70617)

Agreed... I suspect the loop and check for a conflict should probably just go away or get fixed (and not loop unless randomness comes back). It's really not (and wasn't) enough to just check bridge interfaces because those aren't the only kind of interfaces that get generated/random/arbitrary MAC addresses.

It also seems like it would've been safer to assume that other vnets on the same machine /could/ be in the same broadcast domain and check bridges in all vnets.

746 ↗(On Diff #70617)

Or for that one of the bridges in one of the 20 machines next to it in the rack running all the same with some similarly broken non-unique setup.
There's things we can do and things we cannot do and if it is an issue for users despite us trying the reasonable things then they can set these "unique" identifiers hard and by hand from the management interface at creation time themselves using an administered namespace.

Remove (now actively harmful) retry loop

This revision now requires review to proceed.Apr 16 2020, 2:58 PM
746 ↗(On Diff #70617)

Yeah, that's a great point. We must get rid of the loop here.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 18 2020, 8:01 AM
This revision was automatically updated to reflect the committed changes.