Control: reopen 1022843
Control: found 1022843 0.8.40
Control: severity 1022843 serious
Hi,
Cyril Brulebois <kibi@debian.org> (2023-01-22):
> d-i believes that everything should be allow-hotplug, and we end up
> using that everywhere. I'm wondering whether to just use auto everywhere
> instead, since the hotplug detection is busted.
>
> More details in:
> https://bugs.debian.org/1029352
>
> (which is mainly about wireless interfaces but ends up in the third
> issue about other kinds of interfaces as well.)
I've done some testing, and I think the patch is making matters worse:
- networking.service can start very early, at which point interfaces
might not be ready yet, firmware loaded, etc.
- with the extra ExecStart, forcing allow-hotplug entries, and ignoring
failures, I strongly suspect (but didn't triple-check) that we end up
with interfaces getting considered up even if they aren't, which
interfers with the udev integration (the /lib/udev/ifupdown-hotplug
script no longer works: the interface is supposed to be up already!).
It looks to me that this patch should be reverted for bookworm, as it
aims to make “restart” work, but it breaks simple “start”.
It seems systemd supports an ExecReload directive, but no matching
ExecRestart, so I think we end up with restart forcingly calling
ExecStop then ExecStart (plus relevant Pre/Post if any).
I have no idea about ifupdown's architecture, but it looks to me that it
might make sense to have some way of knowing whether that's the first
time the unit is started, if which case stick to the working, existing
logic. And if it's not, add --allow=hotplug.
I don't think we can use systemd unit's NRestarts since it counts
restarts triggered via Restart=always and friends, so maintaining a flag
file instead might work?
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).
If people want to discuss this further, please feel free to, but I'd
welcome a quick revert of the initial patch for #1022843: unbreaking
“start” seems much more important than fixing a longstanding shortage
in “restart”. But I'm sure Oleg can help us figure out whether this
proposed change makes sense (maybe also testing on bullseye), so that
we can fix the regression and the original issue by keeping track of
this single bug report. Otherwise, I can file a different bug report
about the regression specifically. Reopening and bumping severity on
#1022843 for the time being.
This might mean considering the third issue listed in #1029352 as a
non-issue, at least for bookworm: we could keep listing (apparently)
all interfaces as “allow-hotplug”.
Cheers,
--
Cyril Brulebois (kibi@debian.org) <https://debamax.com/>
D-I release manager -- Release team member -- Freelance Consultant
Attachment:
signature.asc
Description: PGP signature