Bug#894511: Please emphasize in the documentation a possible race condition in if-*.d hooks with systemd

2018-04-04 Thread Guus Sliepen
On Sat, Mar 31, 2018 at 06:01:18PM +0200, Raphaël Halimi wrote:

> With /etc/init.d/networking script, network interfaces configured with
> class "auto" are raised first, and only when that's done, interfaces
> configured with class "allow-hotplug" are then raised:
> 
> ifup -a $exclusions $verbose && ifup_hotplug $exclusions $verbose
> 
> With systemd, the unit file only calls:
> 
> /sbin/ifup -a --read-environment
> 
> ...letting udev (I guess) take care of the interfaces configured with
> class "allow-hotplug", the major difference being that both steps are
> done concurrently.

Hm, but that is more of a coincidence than by design. Ifupdown still has
a udev rule, regardless of the init system, that will bring up hotplug
interfaces. Also, the /etc/init.d/networking script might call
ifup_hotplug, but this only covers interfaces that are present at boot
time, while future hotplugged interfaces might still result in
concurrent calls to ifup.

> This leads to situation where a given hook in /etc/network/if-*.d can
> end up running multiple times simultaneously.

Indeed. I can see how this is a problem, but I don't think serializing
ifupdown is the right solution.

> The NEWS.Debian file's entry for 0.8 states that:
> 
> "Ifupdown will now be more strict when errors occur, and will also
> properly return a non-zero exit code when (de)configuring an interface
> fails. Please ensure your /etc/network/interfaces is correct and that
> your interfaces can be brought up and down without errors, especially
> during system startup."
> 
> IMHO, this doesn't emphasize enough on the fact that a given hook in
> if-*.d must be able to run several times simultaneously,

Yes, that should indeed be emphasized more. And it should be able to run
simultaneously both for SysV and systemd, although the problem happens
more often with the latter.

> I suppose the aggressive parallelizing done by systemd is expected
> behavior,

Debian's SysV system also runs init scripts in parallel! It's just that
the ordering is different than with systemd. I'm not sure why we still
call ifup_hotplug from the SysV init script as we also have the udev
rule, but it's pure luck that the init script configures the hotplug
interfaces before udev runs (unless udev really behaves differently
under SysV for some reason).

> IMHO, there should be at least some kind of warning documented somewhere

You are right. I'll add a an entry to the manual and to NEWS.Debian, so
there will be a clear warning when people are upgrading ifupdown.

-- 
Met vriendelijke groet / with kind regards,
  Guus Sliepen 


signature.asc
Description: PGP signature


Bug#894511: Please emphasize in the documentation a possible race condition in if-*.d hooks with systemd

2018-03-31 Thread Raphaël Halimi
Package: ifupdown
Version: 0.8.31
Severity: wishlist

tl;dr A major difference in handling the "auto" and "allow-hotplug"
interfaces between systemd and sysinit script leads to a possible race
condition when a hook in if-*.d isn't protected against multiple
concurrent runs (example: iptables scripts)

Long version:

With /etc/init.d/networking script, network interfaces configured with
class "auto" are raised first, and only when that's done, interfaces
configured with class "allow-hotplug" are then raised:

ifup -a $exclusions $verbose && ifup_hotplug $exclusions $verbose

With systemd, the unit file only calls:

/sbin/ifup -a --read-environment

...letting udev (I guess) take care of the interfaces configured with
class "allow-hotplug", the major difference being that both steps are
done concurrently.

(by the way, it may deserve its own bug report, but "--read-environment"
isn't documented anywhere)

This leads to situation where a given hook in /etc/network/if-*.d can
end up running multiple times simultaneously.

To be precise, I encountered the problem with a hook which sets iptables
rules (the idea of putting it in pre-up.d was that the interfaces were
firewalled before even being raised; I admit this may be a questionable
practice, but that's not the subject of this bug report), with iptables
producing errors when a chain was flushed by a second instance of the
script, while the first instance, which already flushed this same chain
before, now tried to add rules to it. This never posed a problem with
SysV, but this happens with systemd, because it calls "ifupdown -a"
(which runs all hooks with IFACE="--all", even when no interface is
configured with class "auto", as described in the manual page), and
simultaneously lets udev try to raise the single network interface
(which is configured with class "allow-hotplug" by debian-installer by
default). Note that in my case, replacing "allow-hotplug" with "auto" in
/etc/network/interfaces was sufficient to work around the problem, but
things may be different with custom hooks doing other stuff.

The NEWS.Debian file's entry for 0.8 states that:

"Ifupdown will now be more strict when errors occur, and will also
properly return a non-zero exit code when (de)configuring an interface
fails. Please ensure your /etc/network/interfaces is correct and that
your interfaces can be brought up and down without errors, especially
during system startup."

IMHO, this doesn't emphasize enough on the fact that a given hook in
if-*.d must be able to run several times simultaneously, as the SysV
init script did everything sequentially, whereas systemd raises "auto"
and "allow-hotplug" interfaces concurrently, so a hook which worked
perfectly fine with SysV can produce errors when run with systemd.

I suppose the aggressive parallelizing done by systemd is expected
behavior, and my goal is not to question that here - by no means am I
suggesting to fix the unit file so that it mimics the init script - but
IMHO, there should be at least some kind of warning documented somewhere
(hence the wishlist severity). It may seem late, but I guess I'm not the
only admin who decided to wait until Jessie+N to start letting systemd
handle the init process, and such a warning in the docs may help other
people in the future, saving them a lot of time if they wonder why
systemd randomly fails to raise their network interfaces, when SysV didn't.

Regards,

-- 
Raphaël Halimi



signature.asc
Description: OpenPGP digital signature