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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

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.

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?

Cheers,

Richard