Bug#836016: netcfg: Drop unnecessary loopback config in /etc/network/interfaces
On 12/30/2016 12:19 AM, Philipp Kern wrote: > it needs to, yes. Otherwise you end up with the interface being > unmanaged from n-m, despite the n-m snippet having been written. I > suppose what we could do is just empty it out. At least trying that out > was successful for me. I also tested the non n-m case and that was fine. I'd propose this on top of Martin's patch: > diff --git a/finish-install.d/55netcfg-copy-config > b/finish-install.d/55netcfg-copy-config > index f7271509..7e97ef22 100755 > --- a/finish-install.d/55netcfg-copy-config > +++ b/finish-install.d/55netcfg-copy-config > @@ -58,6 +58,10 @@ case $RET in > # Copy NM config file. First make sure the directory exists > mkdir -p /target/$FILE_PATH_NM_CONFIG > cp /$FILE_PATH_NM_CONFIG/* /target/$FILE_PATH_NM_CONFIG/ > + > + # Empty out the generated /etc/network/interfaces to let it be > + # managed by NM. > + echo "# Networking managed by network-manager." > > /target$FILE_INTERFACES > ;; > > $CONFIG_LOOPBACK) Does that sound reasonable? Kind regards Philipp Kern signature.asc Description: OpenPGP digital signature
Bug#836016: netcfg: Drop unnecessary loopback config in /etc/network/interfaces
Hi,
On 08/30/2016 10:17 AM, Martin Pitt wrote:
> Please drop it as this is unnecessary extra parsing work at boot. I
> attach an untested (only build-tested) initial patch. The main thing
> I'm not sure about is whether this actually needs to clear
> /etc/network/interfaces when NetworkManager is installed -- would
> anything write "real" interfaces to /e/n/i in this case?
it needs to, yes. Otherwise you end up with the interface being
unmanaged from n-m, despite the n-m snippet having been written. I
suppose what we could do is just empty it out. At least trying that out
was successful for me. I also tested the non n-m case and that was fine.
> In a more general vein, ifupdown creates an appropriate /e/n/i on
> package installation already. It would be nicer if netcfg would not
> completly overwrite this and instead just add files to
> /etc/network/interfaces.d/
Yeah, I suppose we could do that. Is there already a naming convention
here? Would /etc/network/interfaces.d/${interface}.conf work? (Note that
for n-m we use a generic untranslated "Wired connection 1" with no
adapter matching specified in the snippet.)
Similarly I guess we could pre-configure systemd-networkd in some way,
but right now there's no package to check for. (Which is how we
determine if network-manager configuration needs to be written out.)
Kind regards and thanks
Philipp Kern
signature.asc
Description: OpenPGP digital signature
Bug#836016: netcfg: Drop unnecessary loopback config in /etc/network/interfaces
Hi Martin, Martin Pitt (2016-08-30): > Package: netcfg > Version: 1.139 > > Hello, > > netcfg still configures a "lo" (loopback) device in > /etc/network/interfaces, although this hasn't been necessary since > ifupdown 0.7.41 in 2013: > > > https://anonscm.debian.org/cgit/collab-maint/ifupdown.git/commit/?id=2127aa19d9416 Thanks. Your patch looks rather sane at first glance. I'm not sure what the status is WRT non-Linux ports so I've added bsd and hurd people in cc. Also, what about the /etc/networks file? (I've just discovered it while grepping for "loopback" in the netcfg codebase.) > Please drop it as this is unnecessary extra parsing work at boot. I > attach an untested (only build-tested) initial patch. The main thing > I'm not sure about is whether this actually needs to clear > /etc/network/interfaces when NetworkManager is installed -- would > anything write "real" interfaces to /e/n/i in this case? I can't easily check right now. > OOI, how can an updated netcfg be tested locally? debcheckout debian-installer, put generated netcfg udebs below build/localudebs, and build e.g. a netboot-gtk image (see wiki for instructions); the resulting mini.iso should be using your local packages (that doesn't work for all udebs since some are loaded way later, over the network, but netcfg might be OK). > In a more general vein, ifupdown creates an appropriate /e/n/i on package > installation already. It would be nicer if netcfg would not completly > overwrite this and instead just add files to /etc/network/interfaces.d/ I don't think I'm going to look into this part since I've little time at the moment. KiBi. signature.asc Description: Digital signature
Bug#836016: netcfg: Drop unnecessary loopback config in /etc/network/interfaces
Package: netcfg
Version: 1.139
Hello,
netcfg still configures a "lo" (loopback) device in
/etc/network/interfaces, although this hasn't been necessary since
ifupdown 0.7.41 in 2013:
https://anonscm.debian.org/cgit/collab-maint/ifupdown.git/commit/?id=2127aa19d9416
Please drop it as this is unnecessary extra parsing work at boot. I
attach an untested (only build-tested) initial patch. The main thing
I'm not sure about is whether this actually needs to clear
/etc/network/interfaces when NetworkManager is installed -- would
anything write "real" interfaces to /e/n/i in this case?
OOI, how can an updated netcfg be tested locally?
In a more general vein, ifupdown creates an appropriate /e/n/i on
package installation already. It would be nicer if netcfg would not
completly overwrite this and instead just add files to
/etc/network/interfaces.d/
Thanks for considering,
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
>From 3f576e1f4f6068712c64988ec7b405cd890f989a Mon Sep 17 00:00:00 2001
From: Martin Pitt
Date: Tue, 30 Aug 2016 10:08:51 +0200
Subject: [PATCH] untested: Remove write_loopback
---
base-installer.d/40netcfg | 4
dhcp.c| 1 -
finish-install.d/55netcfg-copy-config | 6 +-
netcfg-common.c | 17 -
netcfg.h | 1 -
static.c | 1 -
6 files changed, 1 insertion(+), 29 deletions(-)
diff --git a/base-installer.d/40netcfg b/base-installer.d/40netcfg
index f09b20a..da51feb 100755
--- a/base-installer.d/40netcfg
+++ b/base-installer.d/40netcfg
@@ -1,10 +1,6 @@
#!/bin/sh -e
# Copy all relevant networking-related files to /target.
-if [ ! -f /etc/network/interfaces ]; then
- netcfg write_loopback
-fi
-
for file in /etc/network/interfaces /etc/networks /etc/hostname /etc/resolv.conf /etc/hosts; do
if [ -f "$file" ]; then
mkdir /target/$(dirname $file) -p
diff --git a/dhcp.c b/dhcp.c
index fe06950..c3792d8 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -611,7 +611,6 @@ int netcfg_activate_dhcp (struct debconfclient *client, struct netcfg_interface
else {
di_debug("Network config complete");
netcfg_write_common("", hostname, domain);
-netcfg_write_loopback();
netcfg_write_interface(interface);
netcfg_write_resolv(domain, interface);
#if !defined(__FreeBSD_kernel__)
diff --git a/finish-install.d/55netcfg-copy-config b/finish-install.d/55netcfg-copy-config
index 74e96a3..f727150 100755
--- a/finish-install.d/55netcfg-copy-config
+++ b/finish-install.d/55netcfg-copy-config
@@ -58,14 +58,10 @@ case $RET in
# Copy NM config file. First make sure the directory exists
mkdir -p /target/$FILE_PATH_NM_CONFIG
cp /$FILE_PATH_NM_CONFIG/* /target/$FILE_PATH_NM_CONFIG/
-
- # Rewrite /etc/network/interfaces to contain only loopback
- netcfg write_loopback
;;
$CONFIG_LOOPBACK)
- # Rewrite /etc/network/interfaces to contain only loopback
- netcfg write_loopback
+ # no-op
;;
esac
diff --git a/netcfg-common.c b/netcfg-common.c
index c6d1d8d..074c99c 100644
--- a/netcfg-common.c
+++ b/netcfg-common.c
@@ -1108,18 +1108,6 @@ int netcfg_get_domain(struct debconfclient *client, char domain[])
return 0;
}
-void netcfg_write_loopback (void)
-{
-struct netcfg_interface lo;
-
-netcfg_interface_init(&lo);
-lo.name = LO_IF;
-lo.loopback = 1;
-
-netcfg_write_interface(NULL);
-netcfg_write_interface(&lo);
-}
-
/*
* ipaddress.s_addr may be 0
* domain may be null
@@ -1348,11 +1336,6 @@ void parse_args (int argc, char ** argv)
exit(EXIT_SUCCESS);
}
}
-if (!strcmp(argv[1], "write_loopback")) {
-netcfg_write_loopback();
-exit(EXIT_SUCCESS);
-}
-
exit(EXIT_FAILURE);
}
}
diff --git a/netcfg.h b/netcfg.h
index 00a2cea..e8eddda 100644
--- a/netcfg.h
+++ b/netcfg.h
@@ -200,7 +200,6 @@ extern void sigchld_handler (int sig __attribute__ ((unused)));
extern int ask_dhcp_options (struct debconfclient *client, const char *if_name);
-extern void netcfg_write_loopback (void);
extern void netcfg_write_common (const char *ipaddress, const char *hostname, const char *domain);
void netcfg_nameservers_to_array(const char *nameservers, struct netcfg_interface *interface);
diff --git a/static.c b/static.c
index ea12fba..15682b4 100644
--- a/static.c
+++ b/static.c
@@ -679,7 +679,6 @@ int netcfg_get_static(struct debconfclient *client, struct netcfg_interface *ifa
netcfg_write_etc_networks(NULL);
}
netcfg_write_common(iface->ipaddress, hostname, domain);
-netcfg_write_loopback();
netcfg_write_interface(iface);
netcfg_write_resolvconf_options(domain, iface
