From 358daf49c0627d382a21fc03464464520218c8fa Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Fri, 28 Oct 2016 13:40:44 +0200 Subject: [PATCH] conf, start: be smarter when deleting networks - So far we blindly called lxc_delete_network() to make sure that we deleted all network interfaces. This resulted in pointless netlink calls, especially when a container had multiple networks defined. Let's be smarter and have lxc_delete_network() return a boolean that indicates whether *all* configured networks have been deleted. If so, don't needlessly try to delete them again in start.c. This also decreases confusing error messages a user might see. - When we receive -ENODEV from one of our lxc_netdev_delete_*() functions, let's assume that either the network device already got deleted or that it got moved to a different network namespace. Inform the user about this but do not report an error in this case. - When we have explicitly deleted the host side of a veth pair let's immediately free(priv.veth_attr.pair) and NULL it, or memset(priv.veth_attr.pair, ...) the corresponding member so we don't needlessly try to destroy them again when we have to call lxc_delete_network() again in start.c Signed-off-by: Christian Brauner --- src/lxc/conf.c | 61 +++++++++++++++++++++++++++++++++---------------- src/lxc/conf.h | 2 +- src/lxc/start.c | 10 ++++++-- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 651f1f33cb..6bf41013c2 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -2880,20 +2880,22 @@ int lxc_create_network(struct lxc_handler *handler) return 0; } -void lxc_delete_network(struct lxc_handler *handler) +bool lxc_delete_network(struct lxc_handler *handler) { int ret; struct lxc_list *network = &handler->conf->network; struct lxc_list *iterator; struct lxc_netdev *netdev; + bool deleted_all = true; lxc_list_for_each(iterator, network) { netdev = iterator->elem; if (netdev->ifindex != 0 && netdev->type == LXC_NET_PHYS) { if (lxc_netdev_rename_by_index(netdev->ifindex, netdev->link)) - WARN("Failed to rename to the initial name the " \ - "netdev '%s'", netdev->link); + WARN("Failed to rename interface with index %d " + "to its initial name \"%s\".", + netdev->ifindex, netdev->link); continue; } @@ -2907,35 +2909,54 @@ void lxc_delete_network(struct lxc_handler *handler) */ if (netdev->ifindex != 0) { ret = lxc_netdev_delete_by_index(netdev->ifindex); - if (ret < 0) - WARN("Failed to remove interface %d '%s': %s.", - netdev->ifindex, - netdev->name ? netdev->name : "(null)", strerror(-ret)); - else - INFO("Removed interface %d '%s'.", - netdev->ifindex, - netdev->name ? netdev->name : "(null)"); + if (-ret == ENODEV) { + INFO("Interface \"%s\" with index %d already " + "deleted or existing in different network " + "namespace.", + netdev->name ? netdev->name : "(null)", + netdev->ifindex); + } else if (ret < 0) { + deleted_all = false; + WARN("Failed to remove interface \"%s\" with " + "index %d: %s.", + netdev->name ? netdev->name : "(null)", + netdev->ifindex, strerror(-ret)); + } else { + INFO("Removed interface \"%s\" with index %d.", + netdev->name ? netdev->name : "(null)", + netdev->ifindex); + } } /* Explicitly delete host veth device to prevent lingering * devices. We had issues in LXD around this. */ if (netdev->type == LXC_NET_VETH) { - char *hostveth = NULL; - if (netdev->priv.veth_attr.pair) + char *hostveth; + if (netdev->priv.veth_attr.pair) { hostveth = netdev->priv.veth_attr.pair; - else if (strlen(netdev->priv.veth_attr.veth1) > 0) + ret = lxc_netdev_delete_by_name(hostveth); + if (ret < 0) { + WARN("Failed to remove interface \"%s\" from host: %s.", hostveth, strerror(-ret)); + } else { + INFO("Removed interface \"%s\" from host.", hostveth); + free(netdev->priv.veth_attr.pair); + netdev->priv.veth_attr.pair = NULL; + } + } else if (strlen(netdev->priv.veth_attr.veth1) > 0) { hostveth = netdev->priv.veth_attr.veth1; - - if (hostveth) { ret = lxc_netdev_delete_by_name(hostveth); - if (ret < 0) - WARN("Failed to remove '%s' from host.", hostveth); - else - INFO("Deleted '%s' from host.", hostveth); + if (ret < 0) { + WARN("Failed to remove \"%s\" from host: %s.", hostveth, strerror(-ret)); + } else { + INFO("Removed interface \"%s\" from host.", hostveth); + memset((void *)&netdev->priv.veth_attr.veth1, 0, sizeof(netdev->priv.veth_attr.veth1)); + } } } } + + return deleted_all; } #define LXC_USERNIC_PATH LIBEXECDIR "/lxc/lxc-user-nic" diff --git a/src/lxc/conf.h b/src/lxc/conf.h index 842e4dcff7..a0f0c3ebc4 100644 --- a/src/lxc/conf.h +++ b/src/lxc/conf.h @@ -408,7 +408,7 @@ extern int pin_rootfs(const char *rootfs); extern int lxc_requests_empty_network(struct lxc_handler *handler); extern int lxc_create_network(struct lxc_handler *handler); -extern void lxc_delete_network(struct lxc_handler *handler); +extern bool lxc_delete_network(struct lxc_handler *handler); extern int lxc_assign_network(const char *lxcpath, char *lxcname, struct lxc_list *networks, pid_t pid); extern int lxc_map_ids(struct lxc_list *idmap, pid_t pid); diff --git a/src/lxc/start.c b/src/lxc/start.c index cc48962449..9144050dbb 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -1343,6 +1343,7 @@ int __lxc_start(const char *name, struct lxc_conf *conf, struct lxc_handler *handler; int err = -1; int status; + bool removed_all_netdevs = true; handler = lxc_init(name, conf, lxcpath); if (!handler) { @@ -1437,7 +1438,7 @@ int __lxc_start(const char *name, struct lxc_conf *conf, lxc_restore_phys_nics_to_netns(handler->netnsfd, handler->conf); DEBUG("Tearing down virtual network devices used by container"); - lxc_delete_network(handler); + removed_all_netdevs = lxc_delete_network(handler); if (handler->pinfd >= 0) { close(handler->pinfd); @@ -1447,7 +1448,12 @@ int __lxc_start(const char *name, struct lxc_conf *conf, lxc_monitor_send_exit_code(name, status, handler->lxcpath); err = lxc_error_set_and_log(handler->pid, status); out_fini: - lxc_delete_network(handler); + if (!removed_all_netdevs) { + DEBUG("Failed tearing down all network devices used by container. Trying again!"); + removed_all_netdevs = lxc_delete_network(handler); + if (!removed_all_netdevs) + DEBUG("Failed tearing down network devices used by container. Not trying again!"); + } out_detach_blockdev: detach_block_device(handler->conf);