Skip to content
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

network interface is renamed although NAME has been set by udev rule #11436

Closed
mbiebl opened this Issue Jan 15, 2019 · 37 comments

Comments

9 participants
@mbiebl
Copy link
Contributor

mbiebl commented Jan 15, 2019

systemd version the issue has been seen with

v240

Used distribution

Debian sid
Downstream bug report at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=919390

To reproduce the issue, create a file /etc/udev/rules.d/70-persistent-net.rules containing

SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="<MAC>", KERNEL=="eth*", NAME="lan0"

with the mac address of your ethernet network interface.
Unload the network module (in my case 8139cp), then load it again.
Notice how the interface is properly renamed:

[ 3750.870434] 8139cp 0000:00:03.0 lan0: renamed from eth0

Now run udevadm trigger --action=add

[ 3752.509458] 8139cp 0000:00:03.0 ens3: renamed from lan0

The interface is renamed although a custom NAME has been set.

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

that's a regression compared to v239, and I'm inclined to add it to the v241 milestone, given that it can mean loss of network access.
@yuwata, @keszybz wdyt?

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

udevadm info after loading the kernel module:

P: /devices/pci0000:00/0000:00:03.0/net/lan0
L: 0
E: DEVPATH=/devices/pci0000:00/0000:00:03.0/net/lan0
E: INTERFACE=lan0
E: IFINDEX=3
E: SUBSYSTEM=net
E: USEC_INITIALIZED=4242831867
E: ID_NET_NAMING_SCHEME=v240
E: ID_NET_NAME_MAC=enx525400f34de9
E: ID_NET_NAME_PATH=enp0s3
E: ID_NET_NAME_SLOT=ens3
E: ID_BUS=pci
E: ID_VENDOR_ID=0x10ec
E: ID_MODEL_ID=0x8139
E: ID_PCI_CLASS_FROM_DATABASE=Network controller
E: ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
E: ID_VENDOR_FROM_DATABASE=Realtek Semiconductor Co., Ltd.
E: ID_MODEL_FROM_DATABASE=RTL-8100/8101L/8139 PCI Fast Ethernet Adapter (QEMU Virtual Machine)
E: ID_PATH=pci-0000:00:03.0
E: ID_PATH_TAG=pci-0000_00_03_0
E: ID_NET_DRIVER=8139cp
E: ID_NET_LINK_FILE=/usr/lib/systemd/network/99-default.link
E: ID_NET_NAME=ens3
E: SYSTEMD_ALIAS=/sys/subsystem/net/devices/lan0 /sys/subsystem/net/devices/lan0
E: TAGS=:systemd:

After running udevadm trigger:

P: /devices/pci0000:00/0000:00:03.0/net/ens3
L: 0
E: DEVPATH=/devices/pci0000:00/0000:00:03.0/net/ens3
E: INTERFACE=ens3
E: IFINDEX=3
E: SUBSYSTEM=net
E: USEC_INITIALIZED=4242831867
E: ID_NET_NAMING_SCHEME=v240
E: ID_NET_NAME_MAC=enx525400f34de9
E: ID_NET_NAME_PATH=enp0s3
E: ID_NET_NAME_SLOT=ens3
E: ID_BUS=pci
E: ID_VENDOR_ID=0x10ec
E: ID_MODEL_ID=0x8139
E: ID_PCI_CLASS_FROM_DATABASE=Network controller
E: ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
E: ID_VENDOR_FROM_DATABASE=Realtek Semiconductor Co., Ltd.
E: ID_MODEL_FROM_DATABASE=RTL-8100/8101L/8139 PCI Fast Ethernet Adapter (QEMU Virtual Machine)
E: ID_PATH=pci-0000:00:03.0
E: ID_PATH_TAG=pci-0000_00_03_0
E: ID_NET_DRIVER=8139cp
E: ID_NET_LINK_FILE=/usr/lib/systemd/network/99-default.link
E: ID_NET_NAME=ens3
E: SYSTEMD_ALIAS=/sys/subsystem/net/devices/ens3 /sys/subsystem/net/devices/ens3
E: TAGS=:systemd:

@keszybz keszybz added this to the v241 milestone Jan 15, 2019

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

With v239
after modprobe:

P: /devices/pci0000:00/0000:00:03.0/net/lan0
E: DEVPATH=/devices/pci0000:00/0000:00:03.0/net/lan0
E: ID_BUS=pci
E: ID_MODEL_FROM_DATABASE=RTL-8100/8101L/8139 PCI Fast Ethernet Adapter (QEMU Virtual Machine)
E: ID_MODEL_ID=0x8139
E: ID_NET_DRIVER=8139cp
E: ID_NET_LINK_FILE=/usr/lib/systemd/network/99-default.link
E: ID_NET_NAME=ens3
E: ID_NET_NAME_MAC=enx525400f34de9
E: ID_NET_NAME_PATH=enp0s3
E: ID_NET_NAME_SLOT=ens3
E: ID_PATH=pci-0000:00:03.0
E: ID_PATH_TAG=pci-0000_00_03_0
E: ID_PCI_CLASS_FROM_DATABASE=Network controller
E: ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
E: ID_VENDOR_FROM_DATABASE=Realtek Semiconductor Co., Ltd.
E: ID_VENDOR_ID=0x10ec
E: IFINDEX=4
E: INTERFACE=lan0
E: SUBSYSTEM=net
E: SYSTEMD_ALIAS=/sys/subsystem/net/devices/lan0 /sys/subsystem/net/devices/lan0
E: TAGS=:systemd:
E: USEC_INITIALIZED=151929036

after udevadm trigger --action=add:

P: /devices/pci0000:00/0000:00:03.0/net/lan0
E: DEVPATH=/devices/pci0000:00/0000:00:03.0/net/lan0
E: ID_BUS=pci
E: ID_MODEL_FROM_DATABASE=RTL-8100/8101L/8139 PCI Fast Ethernet Adapter (QEMU Virtual Machine)
E: ID_MODEL_ID=0x8139
E: ID_NET_DRIVER=8139cp
E: ID_NET_LINK_FILE=/usr/lib/systemd/network/99-default.link
E: ID_NET_NAME_MAC=enx525400f34de9
E: ID_NET_NAME_PATH=enp0s3
E: ID_NET_NAME_SLOT=ens3
E: ID_PATH=pci-0000:00:03.0
E: ID_PATH_TAG=pci-0000_00_03_0
E: ID_PCI_CLASS_FROM_DATABASE=Network controller
E: ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
E: ID_VENDOR_FROM_DATABASE=Realtek Semiconductor Co., Ltd.
E: ID_VENDOR_ID=0x10ec
E: IFINDEX=4
E: INTERFACE=lan0
E: SUBSYSTEM=net
E: SYSTEMD_ALIAS=/sys/subsystem/net/devices/lan0
E: TAGS=:systemd:
E: USEC_INITIALIZED=151929036
@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jan 15, 2019

Hmm, I wonder if this is intended effect of 55b6530

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jan 15, 2019

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jan 15, 2019

Also see #9006

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jan 15, 2019

And of course #9088

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jan 15, 2019

Not sure what the right approach here is

@yuwata

This comment has been minimized.

Copy link
Member

yuwata commented Jan 15, 2019

Why it is named lan0 when the driver is loaded?? If, the final result is ens3 then I expect it should be always ens3.

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

It should always be named lan0 because of the udev rule.

@yuwata

This comment has been minimized.

Copy link
Member

yuwata commented Jan 15, 2019

What happens when the condition KERNEL=="eth*" is dropped?

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

@yuwata does that really matter? It's a udev rule that worked for years, it shouldn't suddenly stop working.

@yuwata

This comment has been minimized.

Copy link
Member

yuwata commented Jan 15, 2019

I've not tested that, but I guess that, because of the condition,

  • initially it is named to lan0, as the initial name is ethX.
  • the later evaluation results ens3, as the interface is not ethX anymore.

v239 or before, it is evaluated only once. So, the name was stayed at lan0. But, v240, 55b6530 makes the rule is always evaluated.

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

I think once a custom name has been set, it shouldn't be renamed by udev again.

@yuwata

This comment has been minimized.

Copy link
Member

yuwata commented Jan 15, 2019

If I remember correctly, the original motivation of the commit is that interface name can be changed by udev rules without updating initrd or changing kernel command line. So, if my above guess is correct,,, what should we do?

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

imho, revert and find another solution for #9006

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

@yuwata a default policy like /lib/systemd/network/99-default.link should never trump explicit user configuration.

@keszybz

This comment has been minimized.

Copy link
Member

keszybz commented Jan 15, 2019

@mbiebl There is no split between "explicit user configuration" and the rest. Files like 99-default are named this way to have the lower priority. But there is no magic of "oh, this was set by the user, let's not touch it" anywhere. The problem with trying to make a division like that is that nobody seems to ever agree what is "user" configuration, and what is "explicit". Current approach of simply executing the configuration as we find it is much clearer and sustainable. #9006 was applied precisely because users were creating configuration and were peeved that some very specific parts of that configuration are not applied.

The rules in the bug report were relying on an implementation detail of the rule engine, and changing this implementation detail is something that we are allowed to do. The rule should never have had kernel=="eth*" if it was supposed to apply also after the interface was renamed. Current behaviour seems correct to me: when the device is called lan0, there's just one rule that applies to the devices, and when udevadm trigger fires off, we execute all rules that apply. This is also much nicer for the user, because they can create configuration and apply it, without jumping through hoops to reset the state.

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

It's not nicer to users, as it breaks existing user configurations. Which is bad

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

I'm amazed that I have to point this out....

@keszybz

This comment has been minimized.

Copy link
Member

keszybz commented Jan 15, 2019

Not every facet of program behaviour is guaranteed to be stable. Generally, only things that are documented are promised to be kept unchanged. Is the fact that link renames are applied only once documented anywhere?

You seem to suggest that we should never change any user visible detail. I think this user rule was in error, and it worked for a while by luck, and now it doesn't. This happens all the time.

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

sigh

@mbiebl

This comment has been minimized.

Copy link
Contributor Author

mbiebl commented Jan 15, 2019

I guess I'll stop filing bug reports

@vcaputo

This comment has been minimized.

Copy link
Member

vcaputo commented Jan 15, 2019

wouldn't it make sense to only apply kernel= matching to the kernel-assigned name, not a potentially renamed name?

@bl33pbl0p

This comment has been minimized.

Copy link
Contributor

bl33pbl0p commented Jan 15, 2019

@filbranden

This comment has been minimized.

Copy link
Member

filbranden commented Jan 15, 2019

From the peanut gallery...

Would it make sense to somehow record the original name of this interface and match KERNEL=="eth*" against that name as well?

When reading the rule from the OP, that was my impression of how that should have worked (and the previous one-rename rule was a good approximation of it...)

Not really sure whether that makes sense, would be feasible and would address the other issues that commit was supposed to fix...

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jan 15, 2019

Urks, I find this breakage quite unfortunate I must say. I mean, writing a rule that matches effects of a previous rule (like the one debian is using there) is probably not a great idea (rules really should match stuff that is not going to change for a device, so that they are idempotent), but I am not sure that simply breaking systems like this is a good idea.

I don't have a strong opinion on the way out, but I am slightly leaning towards reverting the patch and simply asking downstreams to drop any rules that rename ifaces whatsoever from their initrds, as that means the host's policy will never be enforced. I mean, afaiu the patch was done precisely to ensure that host policy applies if initrds already renamed devices, no?

I mean, users/downstreams assumed that these rules where OK, and we probably should tell them they are a bad idea, but also, I think we shouldn't break so many installed systems this way...

Another idea might be make it configurable in the .link file whether to rename already renamed interfaces, with a default to on. Then, change our 99-default.link file to turn off the option. This way, .link files supplied by users will by default override everything, but the fallback one we provide won't.

Other ideas, opinions?

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jan 15, 2019

i mean, with the naming scheme cmdline option and stuff we now go into great lengths to stabilize names over releases, it would be a pity if we'd then break existing rules so galantly (even if they aren't written in particularly good style)...

@fsateler

This comment has been minimized.

Copy link
Member

fsateler commented Jan 15, 2019

Other ideas, opinions?

The current documentation does not specify whether user modifications are read, and there are two factors suggesting they shouldn't:

  1. The name
  2. The existence of a NAME variable for matching against userspace changes.

Honestly, having a variable named KERNEL map to a value provided by userspace is weird.

@yuwata

This comment has been minimized.

Copy link
Member

yuwata commented Jan 16, 2019

Not every facet of program behaviour is guaranteed to be stable. Generally, only things that are documented are promised to be kept unchanged. Is the fact that link renames are applied only once documented anywhere?

Hmm, we have already documented that implicitly...

<literal>NAME</literal>. If the name has already been set by
userspace, no renaming is performed. The available policies

At least, we need to update this.

yuwata added a commit to yuwata/systemd that referenced this issue Jan 16, 2019

udev: introduce Link.NamePolicy=once
And let's use that by 99-default.link.

Closes systemd#11436.

yuwata added a commit to yuwata/systemd that referenced this issue Jan 16, 2019

yuwata added a commit to yuwata/systemd that referenced this issue Jan 16, 2019

@yuwata

This comment has been minimized.

Copy link
Member

yuwata commented Jan 16, 2019

@poettering's proposal is implemented in #11443.

@keszybz

This comment has been minimized.

Copy link
Member

keszybz commented Jan 16, 2019

NAME. If the name has already been set by
userspace, no renaming is performed. The available policies

OK, that is enough for me to consider the previous behaviour documented. So I agree that we should preserve compatibility for this.

Another idea might be make it configurable in the .link file whether to rename already renamed interfaces, with a default to on. Then, change our 99-default.link file to turn off the option. This way, .link files supplied by users will by default override everything, but the fallback one we provide won't.

I like this approach. The question is whether to default to "on" or "off" with current link files. If we default to "off", we preserve more backwards compatibility. If we default to "on", we get saner behaviour.

@yuwata

This comment has been minimized.

Copy link
Member

yuwata commented Jan 16, 2019

Or, how about disabling the new behavior if naming scheme is specified earlier than v240?

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jan 16, 2019

@yuwata interesting idea. Maybe like this: if naming scheme is < v240 we default to RenameOnce=yes and otherwise to RenameOnce=no (and 99-default.link would set RenameOnce=yes either way)

@thecodingdude

This comment was marked as off-topic.

Copy link

thecodingdude commented Jan 16, 2019

@keszybz:

Not every facet of program behaviour is guaranteed to be stable

Please go talk to Linus Torvalds. If it works today and you make a change to break users, you are in the wrong. I should hope you wouldn't need yelling at to understand that.

Software developers need to internalize this and abide by it at all times, life would be much easier.

@bl33pbl0p

This comment has been minimized.

Copy link
Contributor

bl33pbl0p commented Jan 16, 2019

Please don't post offtopic comments.

(This is making rounds elsewhere, locking this for now might be best).

@systemd systemd locked as too heated and limited conversation to collaborators Jan 16, 2019

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jan 17, 2019

BTW, for those coming late, a good fix (including a revert) has been merged now. I think all should be good now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.