Re: [OE-core] [PATCH] udev-extraconf: fix ifupdown for non hotplug devices

[Mike Looijmans <mike.looijmans@topic.nl>]

On 09-04-2025 10:37, Richard Purdie via lists.openembedded.org wrote:
> On Mon, 2025-04-07 at 19:49 +0200, Jeroen Hofstee via lists.openembedded.org wrote:
>> From: Jeroen Hofstee <jhofstee@victronenergy.com>
>>
>> Commit 160f713917 (udev-extraconf: fix network.sh script did not
>> configure hotplugged interfaces, 2024-10-18) fixed ifupdown for
>> hotplug devices, but also calls it for non hotplug devices. That
>> can cause issue, since they might not expect the ifupdown from
>> udev, since it wasn't called before mentioned patch got merged.
>>
>> For util-linux this can simply be fixed by adding --allow=hotplug.
>> Unfortunately busybox doesn't have that option, so a function is
>> added to check if the device is marked as allow-hotplug.
>>
>> Since wilcards are supported adding 'allow-hotplug *' allows to
>> restore behaviour of mentioned patch, while this restores the
>> original behaviour.
>>
>> Signed-off-by: Jeroen Hofstee <jhofstee@victronenergy.com>
>> ---
>>   .../udev/udev-extraconf/network.sh            | 28 +++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/meta/recipes-core/udev/udev-extraconf/network.sh b/meta/recipes-core/udev/udev-extraconf/network.sh
>> index 500e60ae61..da79f00d5a 100644
>> --- a/meta/recipes-core/udev/udev-extraconf/network.sh
>> +++ b/meta/recipes-core/udev/udev-extraconf/network.sh
>> @@ -6,6 +6,34 @@ echo "$INTERFACE" | grep -q wifi && exit 0
>>   # udevd does clearenv(). Export shell PATH to children.
>>   export PATH
>>   
>> +# udev should only trigger ifupdown for interfaces marked as allow-hotplug
>> +# and with util-linux that is as simple as adding --allow=hotplug.
>> +# Busybox unfortunately doesn't have this option.
>> +# allow-hotplug is a pattern like eth0 /eth* /eth*/1 /eth*=eth.
>> +# This function checks if INTERFACE matches an allow-hotplug pattern.
>> +
>> +allow_hotplug() {
>> +    allow_hotplug="$(sed -n -e 's/^allow-hotplug \+\([^= ]*\).*/\1/p' /etc/network/interfaces)"
>> +    for pattern in $allow_hotplug; do
>> +        options="$(echo $pattern | sed -n -e 's,^/\?[^ /]\+/\(.*\),\1,p')"
>> +        value="$(echo $pattern | sed -n -e 's,^/\?\([^ /]\+\).*,\1,p')"
>> +        interfaces="$(ls -d /sys/class/net/$value 2>/dev/null | xargs -r -n 1 basename)"
>> +        if [ "$options" != "" ]; then
>> +            interfaces="$(echo $interfaces | awk -v n=$options '{print $n }')"
>> +        fi
>> +        echo "$interfaces" | grep -w -q "$INTERFACE"
>> +        if [ $? -eq 0 ]; then
>> +            return 0
>> +        fi
>> +    done
>> +
>> +    return 1
>> +}
>> +
>> +if ! allow_hotplug; then
>> +    exit 0
>> +fi
>> +
>>   # if this interface has an entry in /etc/network/interfaces, let ifupdown
>>   # handle it
>>   if grep -q "iface \+$INTERFACE" /etc/network/interfaces; then
>>
> Firstly thanks for the patch, we should try and fix issues like this. I
> feel I should mention our code comes from a backdrop of resource
> constrained devices and the above code makes me cringe a bit due to the
> execution overhead of it.

That was my first reaction as well. Immediately followed by "luckily I use 
mdev on constrained devices".


> We've purposefully kept the code called from udev and in our
> initscripts relatively minimal/simple as the overhead of executing
> multiple programs does build up over time. Taking the above, we have
> loops, then pipelines, each of which runs more commands. Each command
> has a fork/exec overhead.
>
> Is there some way we can simplify this rather than all the shell
> pipelines and loops?

I've always found that the way that initscripts starts the network interfaces 
is broken.

It immediately tries to bring up an interface when it detects it. It will send 
out DHCP requests, of which the first few usually get dropped because the 
interface doesn't have a carrier yet. Causing a delay of sometimes 10 seconds 
before the interface actually configures.

So what I have been putting in there for years is to start "ifplugd" for 
interfaces when they arrive (cold and hotplug treated the same). And once 
ifplugd reports a link "up", I call "ifup" on that interface. Then the DHCP 
actually works immediately and the configuration finishes quickly.

That also works great for wifi devices, because a "link up" means "associated 
with an AP", so all it takes for a wifi link is to start wpa_supplicant when 
it registers.

This works for mdev, and should work for udev as well (after adding device 
add/remove hooks).

Recipes are on github (ifplugd-auto-net):
https://github.com/topic-embedded-products/topic-platform/tree/scarthgap/meta-topic-platform/recipes-topic/network

Maybe it'd be interesting to move these to core?

Mike.