bridge: Fix hwaddr set race between us and udev#2380
Merged
selansen merged 1 commit intomoby:masterfrom Jan 1, 2020
Merged
Conversation
systemd and udev in their default configuration attempt to set a persistent MAC address for network interfaces that don't have one already [systemd-def-link]. We set the address only after creating the interface, so there is a race between us and udev. There are several outcomes (that actually occur, this race is very much not a theoretical one): * We set the address before udev gets to the networking rules, so udev sees `/sys/devices/virtual/net/docker0/addr_assign_type = 3` (NET_ADDR_SET). This means there's no need to assign a different address and everything is fine. * udev reads `/sys/devices/virtual/net/docker0/addr_assign_type` before we set the address, gets `1` (NET_ADDR_RANDOM), and proceeds to generate and set a persistent address. Old versions of udev (pre-v242, i.e. without [udev-patch]) would then fail to generate an address, spit out "Could not generate persistent MAC address for docker0: No such file or directory" (see [udev-issue], and everything would be probably fine as well. Current version of udev (with [udev-patch]) will generate an address just fine and then race us setting it. As udev does more work than we, the most probable outcome is that udev will overwrite the address we set and possibly cause some trouble later on. On a clean Debian Buster (from Vagrant) VM with systemd/udev 242 from Debian Experimental, `docker network create net1` up to `net7` resulted in 3 bridges having a 02:42: address and 4 bridges having a seemingly random (actually generated from interface name) address. With systemd 241, the result would be all bridges having a 02:42:, but some "Could not generate persistent MAC address for" messages in the log. The fix is to revert the MAC address setting fix from 79b3e77, as it is no longer necessary with current netlink [netlink-addr-add], and set the address atomically when creating the bridge interface, not after that. [systemd-def-link]: https://github.com/systemd/systemd/blob/a166cd3aacdbfd4df196bb4ca9f43cff19cf9fec/network/99-default.link [udev-patch]: systemd/systemd@6d36464 [udev-issue]: systemd/systemd#3374 [netlink-addr-add]: vishvananda/netlink@7d9b424 ... Do note that a similar race happens when creating veth devices as well. I wasn't able to reproduce getting a wrong (non-02:42:) address, possibly because the address is set by docker later, maybe only after the interface is moved to another network namespace (but I'm just guessing here). Still, different timings result in various error messages being logged ("link_config: could not get ethtool features for vethd9c938e" and the like) depending on when the interface disappears from the primary network namespace. I'm not sure how to fix this and I don't intend to dig deeper into this. Signed-off-by: Tomas Janousek <tomi@nomi.cz>
2 tasks
Contributor
Author
|
Additionally, the race makes it difficult to match on address in udev rules, causing trouble for NetworkManager: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/15#note_162509 |
|
Any updates on when this issue will be merged into master? |
|
Seeing the same issues here: |
Member
|
ping @arkodg PTAL |
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
systemd and udev in their default configuration attempt to set a
persistent MAC address for network interfaces that don't have one
already (systemd-def-link). We set the address only after creating the
interface, so there is a race between us and udev. There are several
outcomes (that actually occur, this race is very much not a theoretical
one):
We set the address before udev gets to the networking rules, so udev
sees
/sys/devices/virtual/net/docker0/addr_assign_type = 3(NET_ADDR_SET). This means there's no need to assign a different
address and everything is fine.
udev reads
/sys/devices/virtual/net/docker0/addr_assign_typebeforewe set the address, gets
1(NET_ADDR_RANDOM), and proceeds togenerate and set a persistent address.
Old versions of udev (pre-v242, i.e. without udev-patch) would then
fail to generate an address, spit out "Could not generate persistent
MAC address for docker0: No such file or directory" (see udev-issue,
and everything would be probably fine as well.
Current version of udev (with udev-patch) will generate an address
just fine and then race us setting it. As udev does more work than we,
the most probable outcome is that udev will overwrite the address we
set and possibly cause some trouble later on.
On a clean Debian Buster (from Vagrant) VM with systemd/udev 242 from
Debian Experimental,
docker network create net1up tonet7resultedin 3 bridges having a 02:42: address and 4 bridges having a seemingly
random (actually generated from interface name) address. With systemd
241, the result would be all bridges having a 02:42:, but some "Could
not generate persistent MAC address for" messages in the log.
The fix is to revert the MAC address setting fix from 79b3e77,
as it is no longer necessary with current netlink (netlink-addr-add),
and set the address atomically when creating the bridge interface, not
after that.
Do note that a similar race happens when creating veth devices as well.
I wasn't able to reproduce getting a wrong (non-02:42:) address,
possibly because the address is set by docker later, maybe only after
the interface is moved to another network namespace (but I'm just
guessing here). Still, different timings result in various error
messages being logged ("link_config: could not get ethtool features for
vethd9c938e" and the like) depending on when the interface disappears
from the primary network namespace. I'm not sure how to fix this and I
don't intend to dig deeper into this.