Package: ifupdown Version: 0.8.1 Tags: patch Hello Guus,
many thanks for adding a native networking.service! Michael and I found some corrections and tweaks to be made, and we would also like to remove the /lib/systemd/system/networking.service.d/systemd.conf drop-in. The attached patch to networking.service changes the following: - Drop Wants=network-pre.target. This is a passive target which should be pulled in by e. g. firewalls, not network management service. See man systemd.special(7). - Add Wants=network.target instead. It's passive target too, and as networking.service is a provider it needs to pull it in. - Add Before=network.target. This is important during shutdown to avoid stopping networking.service before any consumer of it (e. g. NFS, apache, etc.) -- otherwise shutting down these might hang due to cutting the network too early. The above are similar to systemd-networkd.service or NetworkManager.service. - Add After=local-fs.target. Depending on a read-only root partition might not be enough, as there might be actions in /e/n/i which require a writable root, or other mounted partitions. The current sysvinit file has "Required-Start: $local_fs" and we should probably keep that until we are really sure that we can drop it. - Add After=apparmor.service. Apparmor needs to run very early as it commonly installs profiles for network-facing software like dhcp-client. Note that this is a no-op when apparmor isn't being used, as there is no Wants= (i. e. just ordering, no dependency). - Drop systemd-udev-settle.service. Please let's not do this unconditionally. udev-settle is a really bad workaround for software which does not know about hotplugging, it's racy, might take a long time unnecessarily, and isn't required at all when one does not actually use ifupdown. The init.d script does this conditionally at least: if [ -n "$(ifquery --list --exclude=lo)" ] || [ -n "$(ifquery --list --allow=hotplug)" ]; then udevadm settle || true Although this looks overzealous to me -- I thought the intention was to make sure that all "static" interfaces are present before you start ifup'ing them, i. e. for the "auto" class. We specifically want "allow-hotplug" devices to *not* block the boot, only for the "auto" ones, no? So if you want to keep the current approach, I suggest adding this instead: ExecStartPre=-/bin/sh -c '[ -n "$(ifquery --list --exclude=lo)" ] && udevadm settle' (which is what the patch does ATM). In the medium term I'd be interested in cleaning this up, though. IMHO there needs to be a proper ifupdown-wait-online.service which just starts the virtual interfaces, let all the real ones be handleld by udev hotplug rules, and just waits until all "auto" services are up. This avoids the race condition and unnecessary blocking on udev settle. But this should be discussed in a separate bug, and I'll also send one for moving the "allow-hotplug" handling bits into ifupdown (where they fit much better than in the udev package). But the above just mimics what the sysvinit script does, so it's a good iteration. :-) Thanks for considering, Martin -- Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
--- /lib/systemd/system/networking.service.orig 2015-12-03 12:12:15.299546792 +0100 +++ /lib/systemd/system/networking.service 2015-12-03 12:58:07.456211751 +0100 @@ -2,10 +2,9 @@ Description=Raise network interfaces Documentation=man:interfaces(5) DefaultDependencies=no -Wants=network-pre.target -Wants=systemd-udev-settle.service -After=network-pre.target systemd-udev-settle.service -Before=network-online.target shutdown.target +Wants=network.target +After=local-fs.target network-pre.target apparmor.service +Before=network.target shutdown.target Conflicts=shutdown.target [Install] @@ -14,6 +13,7 @@ [Service] Type=oneshot EnvironmentFile=-/etc/default/networking +ExecStartPre=-/bin/sh -c '[ -n "$(ifquery --list --exclude=lo)" ] && udevadm settle' ExecStart=/sbin/ifup -a --read-environment ExecStop=/sbin/ifdown -a --read-environment RemainAfterExit=true
signature.asc
Description: Digital signature
_______________________________________________ Pkg-systemd-maintainers mailing list Pkg-systemd-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-systemd-maintainers