New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

udevd: `Could not generate persistent MAC address for $name: No such file or directory` #3374

Open
tomty89 opened this Issue May 28, 2016 · 27 comments

Comments

@tomty89
Copy link
Contributor

tomty89 commented May 28, 2016

Submission type

  • Bug report
  • Request for enhancement (RFE)

systemd version the issue has been seen with

230

Used distribution

Arch Linux

In case of bug report: Expected behaviour you didn't see

no message is logged when I add an interface with ip link

In case of bug report: Unexpected behaviour you saw

Could not generate persistent MAC address for $name: No such file or directory logged when I add an interface (e.g. bridge, bond) with ip link. If I create them with networkd/.netdev, no such message is logged.

In case of bug report: Steps to reproduce the problem

[tom@localhost ~]$ sudo ip l add name br0 type bridge
[tom@localhost ~]$ sudo ip l add name bond1 type bond
[tom@localhost ~]$ journalctl -b -u systemd-udevd
-- Logs begin at Wed 2016-05-04 14:49:19 HKT, end at Sat 2016-05-28 15:39:18 HKT. --
May 28 15:39:13 localhost systemd-udevd[351]: Could not generate persistent MAC address for br0: No such file or directory
May 28 15:39:18 localhost systemd-udevd[358]: Could not generate persistent MAC address for bond0: No such file or directory
May 28 15:39:18 localhost systemd-udevd[357]: Could not generate persistent MAC address for bond1: No such file or directory

@tomty89

This comment has been minimized.

Copy link
Contributor

tomty89 commented Jun 4, 2016

So this is the cause of the issue:

systemd.link(5), for MACAddressPolicy=persistent:

This feature depends on ID_NET_NAME_* properties to exist for the link. On hardware where these properties are not set, the generation of a persistent MAC address will fail.

which does not really make sense for "virtual devices" (i.e. bridge, bond...), but they are "affected" since we generally defaults to MACAddressPolicy=persistent.

What do you think, @teg? Do you think this makes sense?

diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c
index 046b0f9..863ce26 100644
--- a/src/libsystemd-network/network-internal.c
+++ b/src/libsystemd-network/network-internal.c
@@ -44,6 +44,12 @@ const char *net_get_name(struct udev_device *device) {

         assert(device);

+        if (!udev_device_get_property_value(device, "ID_BUS")) {
+                name = udev_device_get_property_value(device, "INTERFACE");
+                if (name)
+                        return name;
+        }
+
         /* fetch some persistent data unique (on this machine) to this device */
         FOREACH_STRING(field, "ID_NET_NAME_ONBOARD", "ID_NET_NAME_SLOT", "ID_NET_NAME_PATH", "ID_NET_NAME_MAC") {
                 name = udev_device_get_property_value(device, field);

@poettering poettering added this to the v231 milestone Jun 7, 2016

@teg

This comment has been minimized.

Copy link
Contributor

teg commented Jun 8, 2016

Hm... I see the problem. But not sure I follow the patch. I guess what we want is in case the name of a device is set explicitly (i.e., one created from userspace), we can just use that as a seed for the MAC, that's what you are doing here, right?

@teg

This comment has been minimized.

Copy link
Contributor

teg commented Jun 8, 2016

If I got that right, I think a better way is, in case the current logic fails, to look at name_assign_type (see should_rename()) and use the current ifname as seed if we know the name to be stable. Something like if (should_rename(device, true)). Does that make sense?

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jul 22, 2016

@teg? @tomty89 any progress on this one?

@poettering poettering removed this from the v231 milestone Jul 22, 2016

@tomty89

This comment has been minimized.

Copy link
Contributor

tomty89 commented Jul 22, 2016

Well I guess it's alright to use should_rename(). So when it returns false, we assume that the name can be used to seed the persistent MAC.

However, be noted that it will not cover virtual device that does not have the name_assign_type attribute. For example, bond0 that is created by default when the bonding module is loaded. I assume that's what we want though?

Shall we replace the following:

        /* fetch some persistent data unique (on this machine) to this device */
        FOREACH_STRING(field, "ID_NET_NAME_ONBOARD", "ID_NET_NAME_SLOT", "ID_NET_NAME_PATH", "ID_NET_NAME_MAC") {
                name = udev_device_get_property_value(device, field);
                if (name)
                        return name;
        }

with:

        if (!should_rename(device, true))
                return udev_device_get_sysname(device)
@nezero

This comment has been minimized.

Copy link

nezero commented Dec 22, 2016

Is there a reason the proposed fix hasn't been applied? This is blocking OpenVPN being able to bridge in some configurations.

@naphthalene

This comment has been minimized.

Copy link

naphthalene commented Jan 16, 2017

Ping

@grafi-tt

This comment has been minimized.

Copy link

grafi-tt commented Feb 22, 2017

@tomty89 @teg @poettering
If I understand correctly, changing the default behavior of the function net_get_name would cause unexpected changes of persistent MAC addresses, so the current logic should be kept.
And we can't call should_rename directly, as it is static.

I made the patch, and confirmed it is working for a bridge device.
Is there any problem on this solution?

diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c
index 092a1eabb..d8fae2235 100644
--- a/src/libsystemd-network/network-internal.c
+++ b/src/libsystemd-network/network-internal.c
@@ -40,7 +40,8 @@
 #include "util.h"
 
 const char *net_get_name(struct udev_device *device) {
-        const char *name, *field;
+        const char *name, *field, *s;
+        unsigned type;
 
         assert(device);
 
@@ -51,6 +52,12 @@ const char *net_get_name(struct udev_device *device) {
                         return name;
         }
 
+        /* if the machine doesn't provide data about the device, use the ifname specified by userspace
+        * (this is the case when the device is virtual, e.g., bridge or bond) */
+        s = udev_device_get_sysattr_value(device, "name_assign_type");
+        if (s && safe_atou(s, &type) >= 0 && type == NET_NAME_USER)
+                return udev_device_get_sysname(device);
+
         return NULL;
 }
 
@grafi-tt

This comment has been minimized.

Copy link

grafi-tt commented Feb 24, 2017

I noticed that the above patch didn't fixed the issue for tun/tap devices.
Apparently the ioctl interface to create tun/tap devices does not set name_assign_type properly.
I locally patched drivers/net/tun.c on the kernel to pass NET_NAME_USER, then the problem is gone away.

@alfonsovng

This comment has been minimized.

Copy link

alfonsovng commented Feb 24, 2017

Please @grafi-tt , can you share your patch? Thanks

@djvdorp

This comment has been minimized.

Copy link

djvdorp commented Mar 2, 2017

Any update on this matter? This also seems to cause issues for me in systemd v232 when using LXC:

/var/log/syslog:

Mar  2 16:00:59 localhost systemd-udevd[690]: Could not generate persistent MAC address for veth9A4JCF: No such file or directory

lxc-start output (as root) that triggers the above syslog entry:

lxc-start: conf.c: instantiate_veth: 2669 failed to attach 'veth9A4JCF' to the bridge 'lxcbr0': Operation not permitted
lxc-start: conf.c: lxc_create_network: 2962 failed to create netdev
lxc-start: start.c: lxc_spawn: 1088 Failed to create the network.
lxc-start: start.c: __lxc_start: 1346 Failed to spawn container "REDACTED"
lxc-start: tools/lxc_start.c: main: 366 The container failed to start.
lxc-start: tools/lxc_start.c: main: 370 Additional information can be obtained by setting the --logfile and --logpriority options.

A patch or workaround, maybe from @grafi-tt, would be greatly appreciated as I am stuck right now.

@pawiecki

This comment has been minimized.

Copy link

pawiecki commented Mar 8, 2017

As @nezero mentioned earlier, it affects OpenVPN on Fedora.

Fedora 25
Linux 4.9.13-200.fc25.x86_64
systemd 231
OpenVPN 2.3.14

mar 08 20:27:25 e320pw NetworkManager[1244]: <info>  [1489001245.0672] manager: (tap0): new Tun device (/org/freedesktop/NetworkManager/Devices/10)
mar 08 20:27:25 e320pw systemd-udevd[4164]: Could not generate persistent MAC address for tap0: No such file or directory
mar 08 20:27:28 e320pw gnome-shell[1675]: JS LOG: Removing a network device that was not added
@tomty89

This comment has been minimized.

Copy link
Contributor

tomty89 commented Mar 8, 2017

@grafi-tt I'm not sure what ioctl interface you were referring to but make you are aware the names of the virtual devices are not always "set from userspace", like the case of bond0 that is created upon the loading of the bonding modules (with default parameters, that said).

Also NET_NAME_USER may not be the only type that you want to "skip" (if we consider should_rename to be the reference, so you may want a clone of it here instead, or something like that that is sane in terms of programming)

I am not really sure it is the right way to address the issue though. As I said, virtual device that is not exactly created by user will still make udev error out. I wonder if we need to by some means make udev actually ignore them when it applies the MACAddressPolicy in any case, so that this issue is fully addressed.

@freddysdad

This comment has been minimized.

Copy link

freddysdad commented Mar 20, 2017

any update on this one? it's still breaking openvpn

@djvdorp

This comment has been minimized.

Copy link

djvdorp commented Mar 21, 2017

Another bump from me as this is rather annoying to work with, because you can't.

@tomty89

This comment has been minimized.

Copy link
Contributor

tomty89 commented Mar 23, 2017

For those who are having problem with this, you should be able work around it by copying /usr/lib/systemd/network/99-default.link to /etc/systemd/network/99-default.link and replace MACAddressPolicy=persistent with MACAddressPolicy=none in the latter, which should prevent udev from doing anything relevant.

Now it makes me wonder what's the point of MACAddressPolicy=. I mean, what's the sense of having systemd-wide (i.e. including devices that are not managed by networkd) general (i.e. physical and virtual devices that are either created by user explicitly or in some enumerated way) MAC Address Policy?

Physical devices mostly have "real" MAC Address which are inheritly persistent. Virtual devices created by users can be assigned with one explicitly if a random one is not acceptable. "Enumerated" device in special cases (e.g. bond0 created by the bonding module with its default parameter) probably wont have acceptable seed for persistent MAC Address generation anyway. So how come there should be a MAC policy applied by default "generally"? Let's say in some of the cases we do not want typical behavior, shouldn't that be switched per device or at most per type though?

@teg @poettering What do you think? Do you think we should/can at least change the default policy from persistent to none, until anyone is willing to spend time on making the option sensible?

@grafi-tt

This comment has been minimized.

Copy link

grafi-tt commented Mar 30, 2017

@alfonsovng Sorry for the late response.
The patch is:

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index cc88cd7856f5..6ddda452884d 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1777,6 +1777,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 	}
 	else {
 		char *name;
+		unsigned char name_assign_type = NET_NAME_ENUM;
 		unsigned long flags = 0;
 		int queues = ifr->ifr_flags & IFF_MULTI_QUEUE ?
 			     MAX_TAP_QUEUES : 1;
@@ -1799,11 +1800,13 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 		} else
 			return -EINVAL;
 
-		if (*ifr->ifr_name)
+		if (*ifr->ifr_name) {
 			name = ifr->ifr_name;
+			name_assign_type = NET_NAME_USER;
+		}
 
 		dev = alloc_netdev_mqs(sizeof(struct tun_struct), name,
-				       NET_NAME_UNKNOWN, tun_setup, queues,
+				       name_assign_type, tun_setup, queues,
 				       queues);
 
 		if (!dev)

@tomty89 Thank you for pointing out the bond0 example. I don't have specific opinion, but maybe it is acceptable to just put warning when the name bond0 is matched. (If I understand correctly, there is another unsolved problem associated with bond0. https://bugs.freedesktop.org/show_bug.cgi?id=82956)

@freddysdad

This comment has been minimized.

Copy link

freddysdad commented Jul 4, 2017

wow, over 12 months since initial report, still breaking openvpn and still no fix. why?

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jul 10, 2017

Bugs don't fix themselves. Generally, the best way to ensure a bug is fixed is becoming active yourself. We rely on contributions, like any Open Source project does.

We have a lot of work to do, and may priorize things differently from you, I hope that's understandable. that's why we can't work on all issues that arise with the same pressure. We are happy however for any contributions from outside

@systemd systemd deleted a comment from jmiahman Jul 11, 2017

@systemd systemd deleted a comment from unitylinux Jul 12, 2017

@1605200517

This comment has been minimized.

Copy link

1605200517 commented Aug 2, 2017

@poettering @grafi-tt
hi - we have encountered this problem, too. If I step in and try things out would you help with some guidance?

To start with,

  • So, the patch posted above by @grafi-tt is not yet part of v234?
  • If so, what can I do to test that patch?
  • I have found the HACKING file but I cannot find the file tun.c after cloning the repo?.. neither in master nor tag 234
@GamerSource

This comment has been minimized.

Copy link

GamerSource commented Sep 5, 2017

@1605200517

I have found the HACKING file but I cannot find the file tun.c after cloning the repo?.. neither in master nor tag 234

The last patch regarding TUN device from @grafi-tt is a Linux Kernel patch, not a systemd one.
You need to clone the kernel repo (either upstream or you distribution one) and add it there as a patch and compile the kernel yourself.
Together with the former patch from @grafi-tt (which is in fact a systemd) one you should be able to resolve this for all network device types, as far as I understand.

@1605200517

This comment has been minimized.

Copy link

1605200517 commented Sep 7, 2017

@GamerSource thank you; currently we no more get this

@sathieu

This comment has been minimized.

Copy link

sathieu commented Oct 25, 2017

I've added (systemd 232, Debian 9 stretch):

# /etc/systemd/network/99-default.link
[Link]
NamePolicy=kernel database onboard slot path
MACAddressPolicy=none

dzavalkinolx added a commit to naspersclassifieds-shared/coreos-kubernetes that referenced this issue Apr 10, 2018

t3chn0m4g3 added a commit to dtag-dev-sec/tpotce that referenced this issue Jun 26, 2018

Fix a systemd error
This is a temporary fix for systemd/systemd#3374.
@jimisan

This comment has been minimized.

Copy link

jimisan commented Sep 18, 2018

You broke my heart again. I'll watching the issue.

dm0- pushed a commit to dm0-/systemd that referenced this issue Oct 30, 2018

Merge pull request systemd#3374 from bgilbert/ucode
sys-firmware/intel-microcode: Update to 20180807
@jsdevel

This comment has been minimized.

Copy link

jsdevel commented Dec 11, 2018

This also broke docker for me on Fedora 29. #3374 (comment) fixed it.

@norbertkiszka

This comment has been minimized.

Copy link

norbertkiszka commented Jan 4, 2019

Same problem on my server: Debian 9 with lxc containters (without docker).

When it will be fixed?

@antoinep92

This comment has been minimized.

Copy link

antoinep92 commented Jan 9, 2019

We also fixed this with the workaround from sathieu but I think it's safer to only apply this change to virtual devices, so that the behaviour on hardware interfaces remains unchanged.

# /etc/systemd/network/99-default.link
[Match]
Path=/devices/virtual/net/*

[Link]
NamePolicy=kernel database onboard slot path
MACAddressPolicy=none

keszybz added a commit to keszybz/systemd that referenced this issue Jan 10, 2019

udev,networkd: use the interface name as fallback basis for MAC and I…
…Pv4LL seed

Fixes systemd#3374. The problem is that we set MACPolicy=persistent (i.e. we would
like to generate persistent MAC addresses for interfaces which don't have a
fixed MAC address), but various virtual interfaces including bridges, tun/tap,
bonds, etc., do not not have the necessary ID_NET_NAME_* attributes and udev
would not assing the address and warn:
  Could not generate persistent MAC address for $name: No such file or directory

Basic requirements which I think a solution for this needs to satisfy:

1. No changes to MAC address generation for those cases which are currently
  handled successfully. This means that net_get_unique_predictable_data() must
  keep returning the same answer, which in turn means net_get_name() must keep
  returning the same answer. We can only add more things we look at with lower
  priority so that we start to cover cases which were not covered before.

2. Like 1, but for IPvLL seed and DHCP IAD. This is less important, but "nice
  to have".

3. Keep MACPolicy=persistent. If people don't want it, they can always apply
  local configuration, but in general stable MACs are a good thing. I have never
  seen anyone complain about that.

== Various approaches that have been proposed

=== systemd#3374 (comment) (tomty89)
if !ID_BUS and INTERFACE, use INTERFACE

I think this almost does the good thing, but I don't see the reason to reject ID_BUS
(i.e. physical hardware). Stable MACs are very useful for physical hardware that has
no physical MAC.

=== systemd#3374 (comment) (teg)
if (should_rename(device, true))

This means looking at name_assign_type. In particular for
NET_NAME_USER should_rename(..., true) returns true. It only returns false
for NET_NAME_PREDICTABLE. So this would cover stuff like br0, bond0, etc,
but would not cover lo and other devices with predictable names. That doesn't
make much sense.

But did teg mean should_rename() or !should_rename()?

=== systemd#3374 (comment) (tomty89):
+ if (!should_rename(device, true))
+        return udev_device_get_sysname(device)

This covers only devices with NET_NAME_PREDICTABLE. Since the problem applies as
much to bridges and such, this isn't neough.

=== systemd#3374 (comment)  (grafi-tt)
+        /* if the machine doesn't provide data about the device, use the ifname specified by userspace
+        * (this is the case when the device is virtual, e.g., bridge or bond) */
+        s = udev_device_get_sysattr_value(device, "name_assign_type");
+        if (s && safe_atou(s, &type) >= 0 && type == NET_NAME_USER)
+                return udev_device_get_sysname(device);

This does not cover bond0, vnet0, tun/tap and similar.
grafi-tt also proposes patching the kernel, but *not* setting name_assign_type
seems intentional in those cases, because the device name is a result of
enumeration, not set by the userspace.

=== systemd#3374 (comment) (tomty89)
(also PR systemd#11372)
- MACAddressPolicy=persistent

This break requirement 3. above. It would solve the immediate problem, but I
think the disruption is too big.

=== This patch

This patch means that we will set a "stable" MAC for pretty much any virtual
device by default, where "stable" means keyed off the machine-id and interface
name.

It seems like a big change, but we already did this for most physical devices.
Doing it also for virtual devices doesn't seem like a big issue. It will make
the setup and monitoring of virtualized networks slightly nicer. I don't think
anyone is depending on having the MAC address changed when those devices are
destoryed and recreated. If they do, they'd have to change MACAddressPolicy=.

The only case which is not covered is devices connected to a bus (i.e. physical
devices), that don't have any identifying information. While setting a fixed
mac address to "bond1" seems OK, doing the same each time a USB dongle is
plugged in and gets the name eth0 seems wrong. So physical devices are
excluded. We can always extend the policy to cover them later.

== Implementation
net_get_name() is called from dhcp_ident_set_iaid() so I didn't change
net_get_name() like in grafi-tt's patch, but net_get_unique_predictable_data().

net_get_unique_predictable_data() is called from get_mac() in link-config.c
and sd_ipv4ll_set_address_seed(), so both of those code paths are affected
and will now get data in some cases where they errored out previously.

The return code is changed to -ENODATA since that gives a nicer error string.

keszybz added a commit to keszybz/systemd that referenced this issue Jan 10, 2019

udev,networkd: use the interface name as fallback basis for MAC and I…
…Pv4LL seed

Fixes systemd#3374. The problem is that we set MACPolicy=persistent (i.e. we would
like to generate persistent MAC addresses for interfaces which don't have a
fixed MAC address), but various virtual interfaces including bridges, tun/tap,
bonds, etc., do not not have the necessary ID_NET_NAME_* attributes and udev
would not assing the address and warn:
  Could not generate persistent MAC address for $name: No such file or directory

Basic requirements which I think a solution for this needs to satisfy:

1. No changes to MAC address generation for those cases which are currently
  handled successfully. This means that net_get_unique_predictable_data() must
  keep returning the same answer, which in turn means net_get_name() must keep
  returning the same answer. We can only add more things we look at with lower
  priority so that we start to cover cases which were not covered before.

2. Like 1, but for IPvLL seed and DHCP IAD. This is less important, but "nice
  to have".

3. Keep MACPolicy=persistent. If people don't want it, they can always apply
  local configuration, but in general stable MACs are a good thing. I have never
  seen anyone complain about that.

== Various approaches that have been proposed

=== systemd#3374 (comment) (tomty89)
if !ID_BUS and INTERFACE, use INTERFACE

I think this almost does the good thing, but I don't see the reason to reject ID_BUS
(i.e. physical hardware). Stable MACs are very useful for physical hardware that has
no physical MAC.

=== systemd#3374 (comment) (teg)
if (should_rename(device, true))

This means looking at name_assign_type. In particular for
NET_NAME_USER should_rename(..., true) returns true. It only returns false
for NET_NAME_PREDICTABLE. So this would cover stuff like br0, bond0, etc,
but would not cover lo and other devices with predictable names. That doesn't
make much sense.

But did teg mean should_rename() or !should_rename()?

=== systemd#3374 (comment) (tomty89):
+ if (!should_rename(device, true))
+        return udev_device_get_sysname(device)

This covers only devices with NET_NAME_PREDICTABLE. Since the problem applies as
much to bridges and such, this isn't neough.

=== systemd#3374 (comment)  (grafi-tt)
+        /* if the machine doesn't provide data about the device, use the ifname specified by userspace
+        * (this is the case when the device is virtual, e.g., bridge or bond) */
+        s = udev_device_get_sysattr_value(device, "name_assign_type");
+        if (s && safe_atou(s, &type) >= 0 && type == NET_NAME_USER)
+                return udev_device_get_sysname(device);

This does not cover bond0, vnet0, tun/tap and similar.
grafi-tt also proposes patching the kernel, but *not* setting name_assign_type
seems intentional in those cases, because the device name is a result of
enumeration, not set by the userspace.

=== systemd#3374 (comment) (tomty89)
(also PR systemd#11372)
- MACAddressPolicy=persistent

This break requirement 3. above. It would solve the immediate problem, but I
think the disruption is too big.

=== This patch

This patch means that we will set a "stable" MAC for pretty much any virtual
device by default, where "stable" means keyed off the machine-id and interface
name.

It seems like a big change, but we already did this for most physical devices.
Doing it also for virtual devices doesn't seem like a big issue. It will make
the setup and monitoring of virtualized networks slightly nicer. I don't think
anyone is depending on having the MAC address changed when those devices are
destoryed and recreated. If they do, they'd have to change MACAddressPolicy=.

The only case which is not covered is devices connected to a bus (i.e. physical
devices), that don't have any identifying information. While setting a fixed
mac address to "bond1" seems OK, doing the same each time a USB dongle is
plugged in and gets the name eth0 seems wrong. So physical devices are
excluded. We can always extend the policy to cover them later.

== Implementation
net_get_name() is called from dhcp_ident_set_iaid() so I didn't change
net_get_name() like in grafi-tt's patch, but net_get_unique_predictable_data().

net_get_unique_predictable_data() is called from get_mac() in link-config.c
and sd_ipv4ll_set_address_seed(), so both of those code paths are affected
and will now get data in some cases where they errored out previously.

The return code is changed to -ENODATA since that gives a nicer error string.

@yuwata yuwata added the has-pr label Jan 13, 2019

keszybz added a commit to keszybz/systemd that referenced this issue Jan 19, 2019

udev,networkd: use the interface name as fallback basis for MAC and I…
…Pv4LL seed

Fixes systemd#3374. The problem is that we set MACPolicy=persistent (i.e. we would
like to generate persistent MAC addresses for interfaces which don't have a
fixed MAC address), but various virtual interfaces including bridges, tun/tap,
bonds, etc., do not not have the necessary ID_NET_NAME_* attributes and udev
would not assing the address and warn:
  Could not generate persistent MAC address for $name: No such file or directory

Basic requirements which I think a solution for this needs to satisfy:

1. No changes to MAC address generation for those cases which are currently
  handled successfully. This means that net_get_unique_predictable_data() must
  keep returning the same answer, which in turn means net_get_name() must keep
  returning the same answer. We can only add more things we look at with lower
  priority so that we start to cover cases which were not covered before.

2. Like 1, but for IPvLL seed and DHCP IAD. This is less important, but "nice
  to have".

3. Keep MACPolicy=persistent. If people don't want it, they can always apply
  local configuration, but in general stable MACs are a good thing. I have never
  seen anyone complain about that.

== Various approaches that have been proposed

=== systemd#3374 (comment) (tomty89)
if !ID_BUS and INTERFACE, use INTERFACE

I think this almost does the good thing, but I don't see the reason to reject ID_BUS
(i.e. physical hardware). Stable MACs are very useful for physical hardware that has
no physical MAC.

=== systemd#3374 (comment) (teg)
if (should_rename(device, true))

This means looking at name_assign_type. In particular for
NET_NAME_USER should_rename(..., true) returns true. It only returns false
for NET_NAME_PREDICTABLE. So this would cover stuff like br0, bond0, etc,
but would not cover lo and other devices with predictable names. That doesn't
make much sense.

But did teg mean should_rename() or !should_rename()?

=== systemd#3374 (comment) (tomty89):
+ if (!should_rename(device, true))
+        return udev_device_get_sysname(device)

This covers only devices with NET_NAME_PREDICTABLE. Since the problem applies as
much to bridges and such, this isn't neough.

=== systemd#3374 (comment)  (grafi-tt)
+        /* if the machine doesn't provide data about the device, use the ifname specified by userspace
+        * (this is the case when the device is virtual, e.g., bridge or bond) */
+        s = udev_device_get_sysattr_value(device, "name_assign_type");
+        if (s && safe_atou(s, &type) >= 0 && type == NET_NAME_USER)
+                return udev_device_get_sysname(device);

This does not cover bond0, vnet0, tun/tap and similar.
grafi-tt also proposes patching the kernel, but *not* setting name_assign_type
seems intentional in those cases, because the device name is a result of
enumeration, not set by the userspace.

=== systemd#3374 (comment) (tomty89)
(also PR systemd#11372)
- MACAddressPolicy=persistent

This break requirement 3. above. It would solve the immediate problem, but I
think the disruption is too big.

=== This patch

This patch means that we will set a "stable" MAC for pretty much any virtual
device by default, where "stable" means keyed off the machine-id and interface
name.

It seems like a big change, but we already did this for most physical devices.
Doing it also for virtual devices doesn't seem like a big issue. It will make
the setup and monitoring of virtualized networks slightly nicer. I don't think
anyone is depending on having the MAC address changed when those devices are
destoryed and recreated. If they do, they'd have to change MACAddressPolicy=.

The only case which is not covered is devices connected to a bus (i.e. physical
devices), that don't have any identifying information. While setting a fixed
mac address to "bond1" seems OK, doing the same each time a USB dongle is
plugged in and gets the name eth0 seems wrong. So physical devices are
excluded. We can always extend the policy to cover them later.

== Implementation
net_get_name() is called from dhcp_ident_set_iaid() so I didn't change
net_get_name() like in grafi-tt's patch, but net_get_unique_predictable_data().

net_get_unique_predictable_data() is called from get_mac() in link-config.c
and sd_ipv4ll_set_address_seed(), so both of those code paths are affected
and will now get data in some cases where they errored out previously.

The return code is changed to -ENODATA since that gives a nicer error string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment