From: Christian Brauner <christian.brauner@ubuntu.com>
Date: Fri, 1 Sep 2017 14:44:46 +0000 (+0200)
Subject: network: stop recording saved physical net devices
X-Git-Tag: lxc-2.1.0~9^2~13
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b809f232286d5ad158efab0f15d42e0711c5ae12;p=thirdparty%2Flxc.git

network: stop recording saved physical net devices

liblxc will now correctly log any network device names and ifindeces in their
respective network namespaces. So there's no need to record physical network
devices any more. This spares us heap allocations and memory we need to have
lying around til the container is shutdown.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 211f5d314..8b2be8da0 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -3502,17 +3502,6 @@ int lxc_clear_hooks(struct lxc_conf *c, const char *key)
 	return 0;
 }
 
-static void lxc_clear_saved_nics(struct lxc_conf *conf)
-{
-	int i;
-
-	if (!conf->saved_nics)
-		return;
-	for (i=0; i < conf->num_savednics; i++)
-		free(conf->saved_nics[i].orig_name);
-	free(conf->saved_nics);
-}
-
 static inline void lxc_clear_aliens(struct lxc_conf *conf)
 {
 	struct lxc_list *it,*next;
@@ -3567,7 +3556,6 @@ void lxc_conf_free(struct lxc_conf *conf)
 	lxc_clear_cgroups(conf, "lxc.cgroup");
 	lxc_clear_hooks(conf, "lxc.hook");
 	lxc_clear_mount_entries(conf);
-	lxc_clear_saved_nics(conf);
 	lxc_clear_idmaps(conf);
 	lxc_clear_groups(conf);
 	lxc_clear_includes(conf);
diff --git a/src/lxc/conf.h b/src/lxc/conf.h
index bd525a2be..7c38d93ba 100644
--- a/src/lxc/conf.h
+++ b/src/lxc/conf.h
@@ -244,8 +244,6 @@ struct lxc_conf {
 	struct lxc_list cgroup;
 	struct lxc_list id_map;
 	struct lxc_list network;
-	struct saved_nic *saved_nics;
-	int num_savednics;
 	int auto_mounts;
 	struct lxc_list mount_list;
 	struct lxc_list caps;
diff --git a/src/lxc/confile_utils.c b/src/lxc/confile_utils.c
index b3ae08b68..02924fa93 100644
--- a/src/lxc/confile_utils.c
+++ b/src/lxc/confile_utils.c
@@ -294,6 +294,10 @@ void lxc_log_configured_netdevs(const struct lxc_conf *conf)
 			break;
 		case LXC_NET_PHYS:
 			TRACE("type: phys");
+			if (netdev->priv.phys_attr.ifindex > 0) {
+				TRACE("host side ifindex for phys device: %d",
+				      netdev->priv.phys_attr.ifindex);
+			}
 			break;
 		case LXC_NET_EMPTY:
 			TRACE("type: empty");
diff --git a/src/lxc/network.c b/src/lxc/network.c
index ef3070354..85b25961e 100644
--- a/src/lxc/network.c
+++ b/src/lxc/network.c
@@ -2305,7 +2305,7 @@ bool lxc_delete_network_unpriv(struct lxc_handler *handler)
 	char netns_path[6 + LXC_NUMSTRLEN64 + 4 + LXC_NUMSTRLEN64 + 1];
 	bool deleted_all = true;
 
-	if (!am_unpriv())
+	if (handler->root)
 		return true;
 
 	*netns_path = '\0';
@@ -2374,13 +2374,10 @@ bool lxc_delete_network_unpriv(struct lxc_handler *handler)
 
 int lxc_create_network_priv(struct lxc_handler *handler)
 {
-	bool am_root;
 	struct lxc_list *iterator;
 	struct lxc_list *network = &handler->conf->network;
 
-	/* We need to be root. */
-	am_root = (getuid() == 0);
-	if (!am_root)
+	if (!handler->root)
 		return 0;
 
 	lxc_list_for_each(iterator, network) {
@@ -2482,7 +2479,7 @@ bool lxc_delete_network_priv(struct lxc_handler *handler)
 	struct lxc_list *network = &handler->conf->network;
 	bool deleted_all = true;
 
-	if (am_unpriv())
+	if (!handler->root)
 		return true;
 
 	lxc_list_for_each(iterator, network) {
@@ -2499,12 +2496,12 @@ bool lxc_delete_network_priv(struct lxc_handler *handler)
 			ret = lxc_netdev_rename_by_index(netdev->ifindex, netdev->link);
 			if (ret < 0)
 				WARN("Failed to rename interface with index %d "
-				     "to its initial name \"%s\"",
-				     netdev->ifindex, netdev->link);
+				     "from \"%s\" to its initial name \"%s\"",
+				     netdev->ifindex, netdev->name, netdev->link);
 			else
 				TRACE("Renamed interface with index %d to its "
-				      "initial name \"%s\"",
-				      netdev->ifindex, netdev->link);
+				      "from \"%s\" to its initial name \"%s\"",
+				      netdev->ifindex, netdev->name, netdev->link);
 			continue;
 		}
 
@@ -2600,51 +2597,69 @@ int lxc_requests_empty_network(struct lxc_handler *handler)
 }
 
 /* try to move physical nics to the init netns */
-void lxc_restore_phys_nics_to_netns(int netnsfd, struct lxc_conf *conf)
+int lxc_restore_phys_nics_to_netns(struct lxc_handler *handler)
 {
 	int ret;
-	int i, oldfd;
+	int oldfd;
 	char ifname[IFNAMSIZ];
+	struct lxc_list *iterator;
+	int netnsfd = handler->netnsfd;
+	struct lxc_conf *conf = handler->conf;
 
-	if (netnsfd < 0 || conf->num_savednics == 0)
-		return;
+	/* We need CAP_NET_ADMIN in the parent namespace in order to setns() to
+	 * the parent network namespace. We won't have this capability if we are
+	 * unprivileged.
+	 */
+	if (!handler->root)
+		return 0;
 
-	INFO("Trying to restore network device names in original namespace for "
-	     "%d network devices", conf->num_savednics);
+	TRACE("Moving physical network devices back to parent network namespace");
 
 	oldfd = lxc_preserve_ns(getpid(), "net");
 	if (oldfd < 0) {
 		SYSERROR("Failed to preserve network namespace");
-		return;
+		return -1;
 	}
 
-	ret = setns(netnsfd, 0);
+	ret = setns(netnsfd, CLONE_NEWNET);
 	if (ret < 0) {
 		SYSERROR("Failed to enter network namespace");
 		close(oldfd);
-		return;
+		return -1;
 	}
 
-	for (i = 0; i < conf->num_savednics; i++) {
-		struct saved_nic *s = &conf->saved_nics[i];
+	lxc_list_for_each(iterator, &conf->network) {
+		struct lxc_netdev *netdev = iterator->elem;
 
-		/* retrieve the name of the interface */
-		if (!if_indextoname(s->ifindex, ifname)) {
+		if (netdev->type != LXC_NET_PHYS)
+			continue;
+
+		/* Retrieve the name of the interface in the container's network
+		 * namespace.
+		 */
+		if (!if_indextoname(netdev->ifindex, ifname)) {
 			WARN("No interface corresponding to ifindex %d",
-			     s->ifindex);
+			     netdev->ifindex);
 			continue;
 		}
-		if (lxc_netdev_move_by_name(ifname, 1, s->orig_name))
+
+		ret = lxc_netdev_move_by_name(ifname, 1, netdev->link);
+		if (ret < 0)
 			WARN("Error moving network device \"%s\" back to "
 			     "network namespace", ifname);
-		free(s->orig_name);
+		else
+			TRACE("Moved network device \"%s\" back to network "
+			      "namespace", ifname);
 	}
-	conf->num_savednics = 0;
 
-	ret = setns(oldfd, 0);
-	if (ret < 0)
-		SYSERROR("Failed to enter network namespace");
+	ret = setns(oldfd, CLONE_NEWNET);
 	close(oldfd);
+	if (ret < 0) {
+		SYSERROR("Failed to enter network namespace");
+		return -1;
+	}
+
+	return 0;
 }
 
 static int setup_hw_addr(char *hwaddr, const char *ifname)
diff --git a/src/lxc/network.h b/src/lxc/network.h
index 7c89c5215..5db84058e 100644
--- a/src/lxc/network.h
+++ b/src/lxc/network.h
@@ -175,11 +175,6 @@ struct lxc_netdev {
 	char *downscript;
 };
 
-struct saved_nic {
-	int ifindex;
-	char *orig_name;
-};
-
 /* Convert a string mac address to a socket structure. */
 extern int lxc_convert_mac(char *macaddr, struct sockaddr *sockaddr);
 
@@ -277,7 +272,7 @@ extern int lxc_find_gateway_addresses(struct lxc_handler *handler);
 extern int lxc_create_network_unpriv(const char *lxcpath, char *lxcname,
 				     struct lxc_list *network, pid_t pid);
 extern int lxc_requests_empty_network(struct lxc_handler *handler);
-extern void lxc_restore_phys_nics_to_netns(int netnsfd, struct lxc_conf *conf);
+extern int lxc_restore_phys_nics_to_netns(struct lxc_handler *handler);
 extern int lxc_setup_network_in_child_namespaces(const struct lxc_conf *conf,
 						 struct lxc_list *network);
 
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 131f43c22..d077ff4b0 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -1097,36 +1097,6 @@ out_error:
 	return -1;
 }
 
-static int save_phys_nics(struct lxc_conf *conf)
-{
-	struct lxc_list *iterator;
-	int am_root = (getuid() == 0);
-
-	if (!am_root)
-		return 0;
-
-	lxc_list_for_each(iterator, &conf->network) {
-		struct lxc_netdev *netdev = iterator->elem;
-
-		if (netdev->type != LXC_NET_PHYS)
-			continue;
-		conf->saved_nics = realloc(conf->saved_nics,
-				(conf->num_savednics+1)*sizeof(struct saved_nic));
-		if (!conf->saved_nics)
-			return -1;
-		conf->saved_nics[conf->num_savednics].ifindex = netdev->ifindex;
-		conf->saved_nics[conf->num_savednics].orig_name = strdup(netdev->link);
-		if (!conf->saved_nics[conf->num_savednics].orig_name)
-			return -1;
-		INFO("Stored saved_nic #%d idx %d name %s.", conf->num_savednics,
-			conf->saved_nics[conf->num_savednics].ifindex,
-			conf->saved_nics[conf->num_savednics].orig_name);
-		conf->num_savednics++;
-	}
-
-	return 0;
-}
-
 static int lxc_network_recv_name_and_ifindex_from_child(struct lxc_handler *handler)
 {
 	struct lxc_list *iterator, *network;
@@ -1294,11 +1264,6 @@ static int lxc_spawn(struct lxc_handler *handler)
 				return -1;
 			}
 		}
-
-		if (save_phys_nics(handler->conf)) {
-			ERROR("Failed to save physical nic info.");
-			goto out_abort;
-		}
 	}
 
 	if (!cgroup_init(handler)) {
@@ -1618,8 +1583,10 @@ int __lxc_start(const char *name, struct lxc_handler *handler,
 		}
 	}
 
-	DEBUG("Pushing physical nics back to host namespace");
-	lxc_restore_phys_nics_to_netns(handler->netnsfd, handler->conf);
+	err = lxc_restore_phys_nics_to_netns(handler);
+	if (err < 0)
+		ERROR("Failed to move physical network devices back to parent "
+		      "network namespace");
 
 	if (handler->pinfd >= 0) {
 		close(handler->pinfd);