Bug#1022843: ifupdown: network down after systemctl restart

[Oleg A. Arkhangelsky]

Package: ifupdown
Version: 0.8.36
Severity: important
X-Debbugs-Cc: sys...@yandex.ru

Hello!

(Not sure why such rather important issue is not filled yet, but anyway. Sorry if I'm creating duplicate.)

My network is down after running `systemctl restart networking` on fresh installed Debian 11.5 (but the same issue is on Debian 10). There is no way to start it with `systemctl start networking`. Not sure if this relevant but I'm using static IPv4 config, no DHCP. The installation is very minimal without any systemd-networkd or NetworkManager stuff.

The reason for this is that systemd unit file networking.service using `ifdown -a` to stop network and then `ifup -a` to start it. According to the manual, the first command unconditionally deconfigure _all_ interfaces defined in /etc/network/interfaces, but the second one configure only interfaces which is defined as "auto" in the configuraton and not as "allow-hotplug".

During the installation all interfaces (fixed PCI NICs too, because udev events generated for PCI devices too [1]) in /etc/network/interfaces is created with "allow-hotplug", so restarting networking with systemd leads to non-functional network.

When using old-school `/etc/init.d/networking restart` (don't forget to set SYSTEMCTL_SKIP_REDIRECT=1) the issue is not present. This is because of special treatment for "allow-hotplug" interfaces in /etc/init.d/networking. See ifup_hotplug () function. The functionality is lacking in systemd unit file.

Forgot to mention that the network starting normally during the system startup. This is because of ifup@.service template unit which is started from udev rule via /lib/udev/ifupdown-hotplug script which runs unit for specific interface. This leads to running `ifup eth0`. When you run ifup this way, there is no need to define interface as "auto".

Yes, everything can be fixed using "auto" instead of "allow-hotplug", but "allow-hotplug" is set by Debian Installer [2]. This leads to the issue just "by default".

Should we sync systemd unit file behaviour with old SysV script by implementing something like ifup_hotplug() logic in unit file? Network must not be broken after silly systemctl restart.

[1] https://salsa.debian.org/installer-team/hw-detect/-/blob/master/net-hotplug.sh
[2] https://salsa.debian.org/installer-team/netcfg/-/blob/master/write_interface.c


-- Package-specific info:
--- /etc/network/interfaces:
# interfaces(5) file used by ifup(8) and ifdown(8)
# Include files from /etc/network/interfaces.d:
source /etc/network/interfaces.d/*

allow-hotplug eth0
iface eth0 inet static
address 172.17.0.3
netmask 255.255.0.0
gateway 172.17.0.1

--- /etc/network/interfaces.d/*:
cat: '/etc/network/interfaces.d/*': No such file or directory

--- up and down scripts installed:
/etc/network/if-down.d:
total 0

/etc/network/if-post-down.d:
total 0

/etc/network/if-pre-up.d:
total 0

/etc/network/if-up.d:
total 0


-- System Information:
Debian Release: 11.5
APT prefers stable-updates
APT policy: (500, 'stable-updates'), (500, 'stable-security'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 5.12.2 (SMP w/1 CPU thread)
Kernel taint flags: TAINT_FORCED_MODULE, TAINT_WARN, TAINT_UNSIGNED_MODULE
Locale: LANG=C, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages ifupdown depends on:
ii adduser 3.118
ii iproute2 5.10.0-4
ii libc6 2.31-13+deb11u5
ii lsb-base 11.1.0

Versions of packages ifupdown recommends:
ii isc-dhcp-client [dhcp-client] 4.4.1-2.3+deb11u1

Versions of packages ifupdown suggests:
pn ppp <none>
pn rdnssd <none>

-- no debconf information

[Oleg A. Arkhangelsky]

Package: ifupdown
Tags: patch

Here is proposed patch.

Since the unit type is oneshot, we can have multiple ExecStart statements.

Note that we have to use '--ignore-errors'. Otherwise if we have real
hotplug interface that is not present at the moment of restart, `ifup`
returns non-zero and systemd unit fail.


diff --git a/debian/networking.service b/debian/networking.service
index 593172b..7ad246b 100644
--- a/debian/networking.service
+++ b/debian/networking.service
@@ -16,6 +16,7 @@ WantedBy=network-online.target
Type=oneshot
EnvironmentFile=-/etc/default/networking
ExecStart=/sbin/ifup -a --read-environment
+ExecStart=/sbin/ifup -a --allow=hotplug --ignore-errors --read-environment
ExecStop=/sbin/ifdown -a --read-environment --exclude=lo
RemainAfterExit=true
TimeoutStartSec=5min

[Santiago Ruano Rincón]

Control: tags -1 + pending

Hi!

El 29/10/22 a las 10:33, Oleg A. Arkhangelsky escribió:

I applied the patch here:
https://salsa.debian.org/debian/ifupdown/-/merge_requests/17

Thanks,

-- Santiago

[Jeff King]

On Sat, Oct 29, 2022 at 10:33:26AM +0000, Oleg A. Arkhangelsky wrote:

> Since the unit type is oneshot, we can have multiple ExecStart statements.
>
> Note that we have to use '--ignore-errors'. Otherwise if we have real
> hotplug interface that is not present at the moment of restart, `ifup`
> returns non-zero and systemd unit fail.
>
>
> diff --git a/debian/networking.service b/debian/networking.service
> index 593172b..7ad246b 100644
> --- a/debian/networking.service
> +++ b/debian/networking.service
> @@ -16,6 +16,7 @@ WantedBy=network-online.target
> Type=oneshot
> EnvironmentFile=-/etc/default/networking
> ExecStart=/sbin/ifup -a --read-environment
> +ExecStart=/sbin/ifup -a --allow=hotplug --ignore-errors --read-environment

This has the minor downside that on system startup, we'll spend time
trying to bring up allow-hotplug interfaces, even if they're not
available. So on my system, for example, with:

allow-hotplug eth0
iface eth0 inet dhcp

auto wlan0
iface wlan0 inet manual
wpa-roam /etc/wpa_supplicant/wpa_supplicant.conf

this introduces a 10-second lag to the boot process as the dhcp client
broadcasts over eth0, which has no cable plugged in (it's udhcpc in my
case, but I imagine it would be the same for any client).

It's definitely less bad than the problem you were fixing. But it makes
me wonder if the solution here is neglecting the reason that these
interfaces are marked allow-hotplug and not "auto" in the first place.

-Peff

[Oleg A. Arkhangelsky]

On Fri Jan 20, 2023 at 5:26 PM UTC, Jeff King wrote:

> This has the minor downside that on system startup, we'll spend time
> trying to bring up allow-hotplug interfaces, even if they're not
> available. So on my system, for example, with:
>
> allow-hotplug eth0
> iface eth0 inet dhcp
>
> auto wlan0
> iface wlan0 inet manual
> wpa-roam /etc/wpa_supplicant/wpa_supplicant.conf
>
> this introduces a 10-second lag to the boot process as the dhcp client
> broadcasts over eth0, which has no cable plugged in (it's udhcpc in my
> case, but I imagine it would be the same for any client).
>
> It's definitely less bad than the problem you were fixing. But it makes
> me wonder if the solution here is neglecting the reason that these
> interfaces are marked allow-hotplug and not "auto" in the first place.

What you describing is correct behavior.

According to "man 5 interface":

"Interfaces marked "allow-hotplug" are brought up when udev detects them.
This can either be during boot if the interface is already present, or at
a later time, for example when plugging in a USB network card. Please
note that this does not have anything to do with detecting a network
cable being pluggied in."

So unplugged cable doesn't mean that the device is not plugged.

But now we have a boot delay in configurations like Jeff, which is bad.

The reason for this is that dhclient goes background only after trying to
get lease by sending DHCPDISCOVER six times which can take up to one
minute. This leads to a longer networking.service systemd unit start time
and postpone reaching systemd network target which in most cases leads to
a longer boot times (due to systemd unit dependencies).

Currently I don't know how to correctly fix this. Given that the problem
I initially described is very old (probably dates back to systemd
migration) and no-one complained, maybe the right thing to do is to
revert the change.

Thank you.

P.S.: Yes, probably Debian installer should add PCI NICs as auto, not as
allow-hotplug, but in this case after fresh installing Debian and just
adding your wlan0 configuration (not changing eth0 which will be auto)
you'll get the same boot delay even without my change.

>
> -Peff

[Cyril Brulebois]

Cyril Brulebois <ki...@debian.org> (2023-01-22):
> The configuration found below manages to:
> - let my allow-hotplug slow-to-appear wireless interface come up at
> boot-up, via the udev integration;
> - allow me to stop and start networking, losing then regaining all
> relevant configs (main connection is wireless, with WPA, with DHCP,
> and SLAAC via RAs, without /e/n/i conf for SLAAC, see #1029352);
> - allow me to restart networking, [same as previous entry].
>
> This is just an example of something that seems to be working fine
> enough during my very limited testing, there might be cleaner ways to do
> this, better names to choose (I picked a neutral “environment” but my
> first thought was an explicit “maybe-redo-hotplug”), etc.:
>
> [Service]
> Type=oneshot
> EnvironmentFile=-/etc/default/networking
> EnvironmentFile=-/run/network/environment
> ExecStart=/sbin/ifup -a --read-environment $ALLOW_HOTPLUG
> ExecStop=/sbin/ifdown -a --read-environment --exclude=lo
> ExecStopPost=/bin/sh -c "echo ALLOW_HOTPLUG=--allow=hotplug > /run/network/environment"
> RemainAfterExit=true
> TimeoutStartSec=5min
>
> Changes (compared to the original ifupdown service unit):
> - an extra /run/network/environment is read if present, no errors
> otherwise; of course it doesn't exist during boot-up, since
> /run is brand new and nothing else is creating it;
> - ExecStopPost populates that file with ALLOW_HOTPLUG=--allow=hotplug
> after each “stop” (possibly part of a “restart”) is done;
> - ExecStart gets an extra $ALLOW_HOTPLUG parameter, that's either
> an empty string (at boot-up) or --allow=hotplug (after boot-up).

Of course I'm not familiar with the ifup interface so I fudged it. Now,
with an extra ens3 declared as auto, the following seems to work fine
for boot-up, stop and start, and restart:

[Service]

Type=oneshot
EnvironmentFile=-/etc/default/networking
ExecStart=/sbin/ifup -a --read-environment

ExecStart=/bin/sh -c 'if [ -f /run/network/restart-hotplug ]; then /sbin/ifup -a --read-environment --allow=hotplug; fi'
ExecStop=/sbin/ifdown -a --read-environment --exclude=lo
ExecStopPost=/usr/bin/touch /run/network/restart-hotplug
RemainAfterExit=true
TimeoutStartSec=5min

I have no opinions on how to best handle possible errors regarding the
hotplug interfaces; but again, that's secondary compared to fixing the
“start” use case.


Cheers,
--
Cyril Brulebois (ki...@debian.org) <https://debamax.com/>
D-I release manager -- Release team member -- Freelance Consultant

[Cyril Brulebois]

Hi Santiago,

Santiago Ruano Rincón <santi...@riseup.net> (2023-01-23):
> Thanks everybody for the inputs. I've applied Paul's solution, and the
> generated .deb can be downloaded from here:
>
> https://salsa.debian.org/debian/ifupdown/-/jobs/3841392/artifacts/raw/debian/output/ifupdown_0.8.41~1.gbp3a6fae+salsaci+20230123+42_amd64.deb
>
> Would it be possible for you (Oleg, Paul, Jeff, kibi et al.) to give it
> a try?

(Reading your mail with s/Paul/Pascal/g in mind.)

Tests yesterday seem to indicate successful results, but again I've only
tested a few combinations in a VM (to keep the feedback loop short).

From the installer team point of view, I'd welcome a swift upload with
this patch, possibly with urgency=high so that the fix reaches testing
soon. This will another blocker out of the way for the next D-I release!

Thanks!

[Jeff King]

On Mon, Jan 23, 2023 at 03:55:50PM +0100, Santiago Ruano Rincón wrote:

> > That seems needlessly convoluted. What about this:

> >
> > [Service]
> > Type=oneshot
> > EnvironmentFile=-/etc/default/networking
> > ExecStart=/sbin/ifup -a --read-environment

> > ExecStart=-/sbin/ifup -a --read-environment --allow=hotplug

> > ExecStop=/sbin/ifdown -a --read-environment --exclude=lo
> > RemainAfterExit=true
> > TimeoutStartSec=5min
> >

> > "start" and "restart" configure all existing "auto" and "allow-hotplug"
> > interfaces.
> > Missing "allow-hotplug" interfaces do not be marked as configured (so that
> > they can be configured by udev) and do not make "start" or "restart" fail.

>
> Thanks everybody for the inputs. I've applied Paul's solution, and the
> generated .deb can be downloaded from here:
>
> https://salsa.debian.org/debian/ifupdown/-/jobs/3841392/artifacts/raw/debian/output/ifupdown_0.8.41~1.gbp3a6fae+salsaci+20230123+42_amd64.deb
>
> Would it be possible for you (Oleg, Paul, Jeff, kibi et al.) to give it
> a try?

I tried it, and it still results in a 10-second pause during startup
while broadcasting dhcp over eth0. Which is not too surprising. You
can't get around that if you run "ifup --allow=hotplug", and it's
reasonable for systemd to wait for the process to finish, even if the
exit code doesn't affect success.

That may be an acceptable casualty for fixing the other problems, but I
suspect it's something that many laptop users will encounter (they have
wifi that is normally used and an ethernet port that is not; d-i sets
only the wifi as "auto").

I didn't test it, but I believe Cyril's approach to avoid trying to
bring up hotplug interfaces at all (unless during restart) would fix
that.

-Peff

[Cyril Brulebois]

Hi Oleg,

Oleg A. Arkhangelsky <sys...@yandex.ru> (2023-01-23):
> I tried this *.deb (Pascal approach). It doesn't change behaviour
> introduced after this patch [1]. Yes, restart for "allow-hotplug"
> interfaces work but I got the same system boot lag in Jeff configuration.

Right, I mostly tested that an available “allow-hotplug” and an available
“auto” interfaces in my VM work fine when I tested Pascal's approach, I
didn't try to disable any of them in my hypervisor.

> Cyril's latest solution seems to work. I just added "-" to /bin/sh to
> deal with cases when we have a device that is really hotplug and
> absent: […]

Right, thanks for implementing and testing the part: as I mentioned, I
wasn't sure what to do about possible errors, and didn't investigate; your
approach seems reasonable given hotpluggable interfaces may or may not be
present.

> I got fast boot (as without bad patch) and working restart. One minute
> long blocking during `systemctl restart networking` when DHCP is not
> respoding (as in Jeff config) is expected.
>
> Thanks to all!

Thanks to both Jeff and you for your tests!


I do note that taking precautions, maintaining the status quo for working
use cases, while addressing other use cases… is sometimes not that bad;
even if that seems “needlessly convoluted”!

[Amy Kos]

Hi,

boot online:
No delay.

boot offline:
60 seconds boot delay (dhcp tiemout) per defined interface.

Please abort or do the timeout in background like ifupdown 0.8.39 used
to,
instead of delaying boot.

Didn't expect this bug to be reopened, opened another one yesterday.
Please feel free to close or merge it.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1029449

Cheers,
Amy

[Santiago Ruano Rincón]

Hi!

El 23/01/23 a las 16:19, Cyril Brulebois escribió:

> Hi Santiago,
>
> Santiago Ruano Rincón <santi...@riseup.net> (2023-01-23):
> > Thanks everybody for the inputs. I've applied Paul's solution, and the
> > generated .deb can be downloaded from here:
> >
> > https://salsa.debian.org/debian/ifupdown/-/jobs/3841392/artifacts/raw/debian/output/ifupdown_0.8.41~1.gbp3a6fae+salsaci+20230123+42_amd64.deb
> >
> > Would it be possible for you (Oleg, Paul, Jeff, kibi et al.) to give it
> > a try?
>
> (Reading your mail with s/Paul/Pascal/g in mind.)

Oups, sorry! Thanks for patching that too.

>
> Tests yesterday seem to indicate successful results, but again I've only
> tested a few combinations in a VM (to keep the feedback loop short).
>
> From the installer team point of view, I'd welcome a swift upload with
> this patch, possibly with urgency=high so that the fix reaches testing
> soon. This will another blocker out of the way for the next D-I release!

Just uploaded with your patched patch.

Cheers,

-- S

[Jeff King]

On Tue, Jan 24, 2023 at 02:32:07PM +0100, Santiago Ruano Rincón wrote:

> > Tests yesterday seem to indicate successful results, but again I've only
> > tested a few combinations in a VM (to keep the feedback loop short).
> >
> > From the installer team point of view, I'd welcome a swift upload with
> > this patch, possibly with urgency=high so that the fix reaches testing
> > soon. This will another blocker out of the way for the next D-I release!
>
> Just uploaded with your patched patch.

Thanks, I just booted with 0.8.41, and the result looks good to me!

-Peff

[Oleg A. Arkhangelsky]

On Wed Jan 25, 2023 at 11:31 PM UTC, Pascal Hambourg wrote:

> Maybe there is a more elegant way to stop ifup@*.service, e.g. using
> systemd unit dependencies but I am not familiar enough with this topic.
>
> This setup causes no delay when (re)starting networking.service by hand
> at boot time or by hand. However if an interface marked "allow-hotplug"
> was manually stopped with ifdown without stopping the associated
> ifup@.service, it is not restarted when restarting networking.service.
>
> Any thoughts ?
>

Pascal, thank you for ideas!

After some digging I think that there is more elegant way to stop
ifup@*.service when stopping (or restarting) networking.serivce, we just
need to add PartOf to /lib/systemd/system/ifup@.service:

[Unit]
...
PartOf=networking.service

And this ExecStart to /lib/systemd/system/networking.service, to make
networking.service restart workable for "allow-hotplug" interfaces (as
per your suggestion):

[Service]
...
ExecStart=/usr/bin/udevadm trigger --action=add --subsystem-match=net
...

This changes should be on top of *.service files, before any Bug#1029352
modifications, of course.

Seems like more clearer way, than to use /bin/sh invocation and flag file
for the non-first run condition check.

Any pitfalls for this approach?

[Cyril Brulebois]

Hi Oleg,

Oleg A. Arkhangelsky <sys...@yandex.ru> (2023-01-26):

> After some digging I think that there is more elegant way to stop
> ifup@*.service when stopping (or restarting) networking.serivce, we just
> need to add PartOf to /lib/systemd/system/ifup@.service:
>
> [Unit]
> ...
> PartOf=networking.service
>
> And this ExecStart to /lib/systemd/system/networking.service, to make
> networking.service restart workable for "allow-hotplug" interfaces (as
> per your suggestion):
>
> [Service]
> ...
> ExecStart=/usr/bin/udevadm trigger --action=add --subsystem-match=net
> ...
>
> This changes should be on top of *.service files, before any Bug#1029352
> modifications, of course.
>
> Seems like more clearer way, than to use /bin/sh invocation and flag file
> for the non-first run condition check.
>
> Any pitfalls for this approach?

Just to clarify: I was mostly interested in getting the initial regression
fixed, as it was in the way of finally fixing wireless support (via /e/n/i
rather than NM) in the installer. I tried to keep the proposed enhancement
while making sure the regression wouldn't come back, hence the “convoluted”
approach.

I'm happy if you folks keep digging into this to find a better solution by
tweaking systemd units. I'll just mention that the freeze is underway, that
a simple enhancement (making restart work) totally broke a much more
important use case (keeping start working), and someone probably needs to
weigh pros (getting things better, in a clean way) versus cons (not a lot of
time to discover and track down possible side effects, be them positive or
negative).

If things break for the installer again:

(in a deep voice) I'll be back!

;)

[Santiago Ruano Rincón]

El 26/01/23 a las 19:29, Cyril Brulebois escribió:

:-)

Just to add: I am also happy for more clean, elegant, robust, etc.
better approaches. But life is keeping me more busy than expected these
days.
Please, feel free to propose MRs that enhance also debian/tests/hotplug,
or create any other autopkgtest that could help to identify and prevent
regressions. (No guarantee that those MR could be included in bookworm.)

Cheers!

-- Santiago