[busybox] 1.37.0-8: add support for networkmanager & fix mdev helper mac addr bug #2983

JulianDroske · 2025-01-15T20:19:37Z

PR hanged up due to

NM won't use a nic if starts after: mdev's already notifies it.
- Investigating

Concluded from the commit message:

Fixes "infinite increasing interface names" bug, will get original mac address if either ethtool or iproute2 is installed.
Reverts 9806fe4
Will notify NetworkManager if possible (i.e. after libudev-zero is installed)

…mac addr bug mdev-helper-settle-nics now supports another argument '--notify-udev' and may only be used in mdev.conf. PROBLEMS Some programs e.g. NetworkManager relies on udev events to wait for udev to populate devices, which may not work on pure mdev-based systems. Since NetworkManager 1.4.0, MAC address spoofing is enabled by default, making mdev-helper-settle-nics unable to get original mac addresses (also called permaddrs) from nics, which will cause infinite increasing interface names when booting OS or nics are replugged. SOLUTIONS By using libudev-helper provided by libudev-zero, the script can trigger NetworkManager *correctly*; By using external tools like iproute2 or ethtool, it is now able to get permaddrs and works well even when NetworkManager is running. If neither of the tools above exist, the original method, i.e. reading from sysfs, will be used. reverts: 9806fe4 (incorrect way to fix mac addr bug) refs: NetworkManager/NetworkManager@4f6c91d (spoofing enabled by default), NetworkManager/NetworkManager@1b49f94 (spoofing implementation)

ziyao233 · 2025-01-16T10:25:31Z

Excellent work! So in conclusion, for solving issue of unstable interface names, we need to

Remove the downstream patch1 to make networkmanager wait for netcards being configured
Apply this PR, to
2.1 avoid mdev-helper-settle-nics misreading a random-generated MAC address
2.2 generate ready events for netcards, which networkmanager listens to

Here are my questions,

Is 2.1 really necessary? If networkmanager waits for mdev-helper-settle-nics finish and only sets a random MAC address after it, the helper shouldn't see the spoofed address.
What's the exact type of event generated in 2.2? I've seen libudev-helper send a NETLINK message, but not sure who's interesting in it: is it a networkmanager-specific event, a kernel-defined event that libudev-zero cares about, or something else?

ziyao233 · 2025-01-16T10:28:24Z

mdev-helper-settle-nics

@@ -1,5 +1,6 @@
 #!/bin/sh


No action required. Actually we should fix this: this script makes use of bash features (local for example).
We could create a split repository to maintain these helpers later.

ziyao233 · 2025-01-16T10:30:44Z

mdev-helper-settle-nics

@@ -28,7 +29,9 @@
 # First it will run nameif to rename interfaces if configured in /etc/mactab
 # Then it will dump known interfaces to new mactab file.
 # Next it will parse old /etc/mactab and copy all interfaces which wasnt added in step two.
-# On the end, it will replace /etc/mactab with the new one.
+# And it will replace /etc/mactab with the new one, controlled by '--write-mactab'.


Suggested change

# And it will replace /etc/mactab with the new one, controlled by '--write-mactab'.

# And it will replace /etc/mactab with the new one if '--write-mactab' is supplied.

ziyao233 · 2025-01-16T10:33:44Z

mdev-helper-settle-nics

+# Finally it will dispatch a udev event for current nic if executed by mdev, controlled by
+# '--notify-udev'.


Suggested change

# Finally it will dispatch a udev event for current nic if executed by mdev, controlled by

# '--notify-udev'.

# Finally it will dispatch a udev event for current nic if '--notify-udev' is supplied. This relies on

# a correct environment variable `$SEQNUM`, which mdev sets.

ziyao233 · 2025-01-16T10:34:47Z

mdev-helper-settle-nics

 fi

-mv "${tmpfile}" '/etc/mactab'
+# It's time to dispatch a udev event
+# This only works when in mdev.conf as which sets a necessary envar SEQNUM, we may not set a custom one


Suggested change

# This only works when in mdev.conf as which sets a necessary envar SEQNUM, we may not set a custom one

# This is only possible when a correct `SEQNUM` is supplied, which is the case when being invoked from mdev.conf

ziyao233 · 2025-01-16T10:36:50Z

mdev-helper-settle-nics

+			*_tmp)
 			;;
-			*)
-				if [ -n "${macaddr}" ] && ! in_comma_list "${macaddr}" "${detected_macs}" && ! in_comma_list "${device}" "${detected_nics}"; then
+				eth[0-9]*|wlan[0-9]*|ath[0-9]*|wifi[0-9]*|ra[0-9]*)
+				macaddr="$(get_nic_macaddr ${device} 2>/dev/null)"
+				if [ -n "${macaddr}" ] && ! in_comma_list "${macaddr}" "${inconf_macs}" && ! in_comma_list "${device}" "${inconf_nics}"; then
+					detected_nics="${detected_nics},${device}"
+					detected_macs="${detected_macs},${macaddr}"


I think this is a copy of original code? Could we split a function out and make it easy for reading?

I think this is a copy of original code?

Yes, it's a wrapper of the original impl of "--write-mactab", and I intended to make it be the same level as that of "--notify-udev", so that the structure will be more clear.

Could we split a function out and make it easy for reading?

Well, according to the original code, the structure is like:

declare basic functions

lock file

parse nics

(options) write mactab, notify udev

So I was trying to keep it. But it's okay to split them out.

And I also think it's time to refactor it, because it's pretty old and didn't share the same idea as the impl of "--notify-udev". i.e. the latter is intended to be executed only by mdev & expects only one device per instance, while the original code can be run by user & processes all nics.

I think this is a copy of original code?

Yes, it's a wrapper of the original impl of "--write-mactab", and I intended to make it be the same level as that of "--notify-udev", so that the structure will be more clear.

Could we split a function out and make it easy for reading?

Well, according to the original code, the structure is like:

* declare basic functions * lock file * parse nics * (options) write mactab, notify udev

So I was trying to keep it. But it's okay to split them out.

I'm willing to see 3 being split into a function, then you could save some indentation.

And I also think it's time to refactor it, because it's pretty old and didn't share the same idea as the impl of "--notify-udev". i.e. the latter is intended to be executed only by mdev & expects only one device per instance, while the original code can be run by user & processes all nics.

I would second the idea, but let's make everything works first :)

ziyao233 · 2025-01-16T10:43:51Z

For the commit message, please use

Reference: https://github.com/NetworkManager/NetworkManager/commit/4f6c91d
Reference: https://github.com/NetworkManager/NetworkManager/commit/1b49f94

instead of refs:. Or use [1] [2] as reference and list these URLs at the end of the message, like what I've done in1. This reads better.

Keep the first line short (less than 72 characters), I think [busybox] 1.37.0-8: Correctly dispatch udev event and use permanent MAC is appropriate.

JulianDroske · 2025-01-16T11:11:58Z

Is 2.1 really necessary?

Actually.. yes, definitely. The problem was that the script will parse all nics on each run. When a second nic is plugged, the script will also reprocess old nics which may already be held by networkmanager, thus we may only impl this function, unless we refactor this script.

ziyao233 · 2025-01-16T11:19:37Z

Is 2.1 really necessary?

Actually.. yes, definitely. The problem was that the script will parse all nics on each run. When a second nic is plugged, the script will also reprocess old nics which may already be held by networkmanager, thus we may only impl this function, unless we refactor this script.

Understand, that's reasonable. As for now, let's keep the original structure and workaround this.

ziyao233 · 2025-01-16T11:24:55Z

2. What's the exact type of event generated in 2.2? I've seen `libudev-helper` send a NETLINK message, but not sure who's interesting in it: is it a networkmanager-specific event, a kernel-defined event that libudev-zero cares about, or something else?

I found the answer: libudev-zero relies on NETLINK for hotplugging support. The device manager should rebroadcast uevents on NETLINK_KOBJECT_UEVENT, then libudev-zero (the library) could know hotplugging is happening.. busybox mdev doesn't support this, so comes libudev-helper, which rebroadcasts the event to group 2 (0x4, 1 << 2) on NETLINK_KOBJECT_UEVENT.

ziyao233 · 2025-01-16T11:31:39Z

2. What's the exact type of event generated in 2.2? I've seen `libudev-helper` send a NETLINK message, but not sure who's interesting in it: is it a networkmanager-specific event, a kernel-defined event that libudev-zero cares about, or something else?
I found the answer: libudev-zero relies on NETLINK for hotplugging support. The device manager should rebroadcast uevents on NETLINK_KOBJECT_UEVENT, then libudev-zero (the library) could know hotplugging is happening.. busybox mdev doesn't support this, so comes libudev-helper, which rebroadcasts the event to group 2 (0x4, 1 << 2) on NETLINK_KOBJECT_UEVENT.

Then comes another problem: we don't rebroadcast uevents for most subsystems (now only for drm and input, and probably net if we merge this). At least usb should be handled as well.

Is it possible to write a generic rule that make mdev invoke libudev-helper on each uevent?

JulianDroske · 2025-01-16T11:37:47Z

What's the exact type of event generated in 2.2?

alias NM=NetworkManager

Actually I answered it once here. But now I'm diving into it. Information below was concluded from my own experience (and source codes).

For NM, only five arguments are used, four of them are checked by libudev-zero 1 and may be used by NM:

SEQNUM (I remembered it also came from an issue for libudev-zero & nm)
- It's a number
- I haven't figure out if it should be unique, but a random number was tested to work for NM.
SUBSYSTEM
- NM only receives =net
DEVPATH/DEVNAME (either of them should appear)
- NM uses DEVPATH as nics don't have a /dev/* node so no DEVNAME
ACTION
- NM processes at least =add

And one necessary argument is used by NetworkManager 2:

IFINDEX

NM uses IFINDEX to find nic and manage it.

JulianDroske · 2025-01-17T06:52:15Z

This PR contains fixable bugs as we cannot figure out the behavior of NM & udev for now, so it's closed.

All suggestions under this PR may be usable for further development.

Wait until libudev-zero supports NM.

eweos-helper bot approved these changes Jan 15, 2025

View reviewed changes

YukariChiba requested a review from ziyao233 January 16, 2025 08:40

ziyao233 requested changes Jan 16, 2025

View reviewed changes

JulianDroske closed this Jan 17, 2025

JulianDroske changed the title ~~[busybox] 1.37.0-8: add support for networkmanager & fix mdev helper mac addr bug~~ [busybox] 1.37.0-8: correctly dispatch udev event and use permanent MAC Jan 17, 2025

JulianDroske changed the title ~~[busybox] 1.37.0-8: correctly dispatch udev event and use permanent MAC~~ [busybox] 1.37.0-8: add support for networkmanager & fix mdev helper mac addr bug Jan 17, 2025

JulianDroske mentioned this pull request Jan 17, 2025

[busybox] 1.37.0-8: fix generating incorrect interface names #2995

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[busybox] 1.37.0-8: add support for networkmanager & fix mdev helper mac addr bug #2983

[busybox] 1.37.0-8: add support for networkmanager & fix mdev helper mac addr bug #2983

JulianDroske commented Jan 15, 2025 •

edited

Loading

ziyao233 commented Jan 16, 2025 •

edited

Loading

ziyao233 Jan 16, 2025

ziyao233 Jan 16, 2025

ziyao233 Jan 16, 2025

ziyao233 Jan 16, 2025

ziyao233 Jan 16, 2025

JulianDroske Jan 16, 2025 •

edited

Loading

ziyao233 Jan 16, 2025

ziyao233 commented Jan 16, 2025

JulianDroske commented Jan 16, 2025

ziyao233 commented Jan 16, 2025

ziyao233 commented Jan 16, 2025 •

edited

Loading

ziyao233 commented Jan 16, 2025

JulianDroske commented Jan 16, 2025

JulianDroske commented Jan 17, 2025

	# And it will replace /etc/mactab with the new one, controlled by '--write-mactab'.
	# And it will replace /etc/mactab with the new one if '--write-mactab' is supplied.

		# Finally it will dispatch a udev event for current nic if executed by mdev, controlled by
		# '--notify-udev'.

	# This only works when in mdev.conf as which sets a necessary envar SEQNUM, we may not set a custom one
	# This is only possible when a correct `SEQNUM` is supplied, which is the case when being invoked from mdev.conf

[busybox] 1.37.0-8: add support for networkmanager & fix mdev helper mac addr bug #2983

[busybox] 1.37.0-8: add support for networkmanager & fix mdev helper mac addr bug #2983

Conversation

JulianDroske commented Jan 15, 2025 • edited Loading

ziyao233 commented Jan 16, 2025 • edited Loading

ziyao233 Jan 16, 2025

Choose a reason for hiding this comment

ziyao233 Jan 16, 2025

Choose a reason for hiding this comment

ziyao233 Jan 16, 2025

Choose a reason for hiding this comment

ziyao233 Jan 16, 2025

Choose a reason for hiding this comment

ziyao233 Jan 16, 2025

Choose a reason for hiding this comment

JulianDroske Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

ziyao233 Jan 16, 2025

Choose a reason for hiding this comment

ziyao233 commented Jan 16, 2025

JulianDroske commented Jan 16, 2025

ziyao233 commented Jan 16, 2025

ziyao233 commented Jan 16, 2025 • edited Loading

ziyao233 commented Jan 16, 2025

JulianDroske commented Jan 16, 2025

JulianDroske commented Jan 17, 2025

JulianDroske commented Jan 15, 2025 •

edited

Loading

ziyao233 commented Jan 16, 2025 •

edited

Loading

JulianDroske Jan 16, 2025 •

edited

Loading

ziyao233 commented Jan 16, 2025 •

edited

Loading