openvswitch: delete ports intelligently

So far, when creating veth devices attached to openvswitch bridges we used to
fork() off a thread on container startup. This thread was kept around until the
container shut down. I have no good explanation why we did it that why but it's
certainly not necessary. Instead, let's fork() off the thread on container
shutdown to delete the veth.

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

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index c5609ec4e5c4f21e8273cf57edd38767beacad22..8b8ae9109792cedd5f1d61d84850909f4f0de458 100644 (file)
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -2757,7 +2757,7 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
        }
 
        if (netdev->link) {
-               err = lxc_bridge_attach(handler->lxcpath, handler->name, netdev->link, veth1);
+               err = lxc_bridge_attach(netdev->link, veth1);
                if (err) {
                        ERROR("failed to attach \"%s\" to bridge \"%s\": %s",
                              veth1, netdev->link, strerror(-err));
@@ -3135,11 +3135,20 @@ bool lxc_delete_network(struct lxc_handler *handler)
                        char *hostveth;
                        if (netdev->priv.veth_attr.pair) {
                                hostveth = netdev->priv.veth_attr.pair;
+
                                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);
+
+                               if (is_ovs_bridge(netdev->link)) {
+                                       ret = lxc_ovs_delete_port(netdev->link, hostveth);
+                                       if (ret < 0)
+                                               WARN("Failed to remove port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
+                                       else
+                                               INFO("Removed port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
+                               }
                        } else if (strlen(netdev->priv.veth_attr.veth1) > 0) {
                                hostveth = netdev->priv.veth_attr.veth1;
                                ret = lxc_netdev_delete_by_name(hostveth);
@@ -3147,7 +3156,16 @@ bool lxc_delete_network(struct lxc_handler *handler)
                                        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));
+
+                                       if (is_ovs_bridge(netdev->link)) {
+                                               ret = lxc_ovs_delete_port(netdev->link, hostveth);
+                                               if (ret < 0) {
+                                                       WARN("Failed to remove port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
+                                               } else {
+                                                       INFO("Removed port \"%s\" from openvswitch bridge \"%s\"", hostveth, netdev->link);
+                                                       memset((void *)&netdev->priv.veth_attr.veth1, 0, sizeof(netdev->priv.veth_attr.veth1));
+                                               }
+                                       }
                                }
                        }
                }

diff --git a/src/lxc/lxc_user_nic.c b/src/lxc/lxc_user_nic.c
index c93b4cc70d1d6ebf13fb5beb0e3188abc1bec1de..96af8363b63b000a9ed6d80cfa33a6aa37c0b67e 100644 (file)
--- a/src/lxc/lxc_user_nic.c
+++ b/src/lxc/lxc_user_nic.c
@@ -483,7 +483,7 @@ static bool create_nic(char *nic, char *br, int pid, char **cnic)
                }
 
                /* attach veth1 to bridge */
-               ret = lxc_bridge_attach(lxcpath, lxcname, br, veth1buf);
+               ret = lxc_bridge_attach(br, veth1buf);
                if (ret < 0) {
                        usernic_error("Error attaching %s to %s.\n", veth1buf, br);
                        goto out_del;

diff --git a/src/lxc/network.c b/src/lxc/network.c
index 84fa9d5a54af86d1d3df95764a28773d10b71d8a..6685792b6304d3f3facb5c1e2ae18f8f37879143 100644 (file)
--- a/src/lxc/network.c
+++ b/src/lxc/network.c
@@ -1395,7 +1395,7 @@ int lxc_ipv6_dest_add(int ifindex, struct in6_addr *dest)
        return ip_route_dest_add(AF_INET6, ifindex, dest);
 }
 
-static bool is_ovs_bridge(const char *bridge)
+bool is_ovs_bridge(const char *bridge)
 {
        char brdirname[22 + IFNAMSIZ + 1] = {0};
        struct stat sb;
@@ -1406,70 +1406,71 @@ static bool is_ovs_bridge(const char *bridge)
        return false;
 }
 
+struct ovs_veth_args {
+       const char *bridge;
+       const char *nic;
+};
+
 /* Called from a background thread - when nic goes away, remove it from the
  * bridge.
  */
-static void ovs_cleanup_nic(const char *lxcpath, const char *name,
-                           const char *bridge, const char *nic)
+static int lxc_ovs_delete_port_exec(void *data)
 {
-       int ret;
+       struct ovs_veth_args *args = data;
 
-       ret = lxc_check_inherited(NULL, true, &(int){-1}, 1);
-       if (ret < 0)
-               return;
+       execlp("ovs-vsctl", "ovs-vsctl", "del-port", args->bridge, args->nic,
+              (char *)NULL);
+       return -1;
+}
 
-       TRACE("Registering cleanup thread to remove nic \"%s\" from "
-             "openvswitch bridge \"%s\"", nic, bridge);
+int lxc_ovs_delete_port(const char *bridge, const char *nic)
+{
+       int ret;
+       char cmd_output[MAXPATHLEN];
+       struct ovs_veth_args args;
 
-       ret = lxc_wait(name, "STOPPED", -1, lxcpath);
+       args.bridge = bridge;
+       args.nic = nic;
+       ret = run_command(cmd_output, sizeof(cmd_output),
+                         lxc_ovs_delete_port_exec, (void *)&args);
        if (ret < 0) {
-               ERROR("Failed to register cleanup thread to remove nic \"%s\" "
-                     "from  openvswitch bridge \"%s\"", nic, bridge);
-               return;
+               ERROR("Failed to delete \"%s\" from openvswitch bridge \"%s\": "
+                     "%s", bridge, nic, cmd_output);
+               return -1;
        }
 
-       execlp("ovs-vsctl", "ovs-vsctl", "del-port", bridge, nic, (char *)NULL);
-       exit(EXIT_FAILURE);
+       return 0;
 }
 
-static int attach_to_ovs_bridge(const char *lxcpath, const char *name, const char *bridge, const char *nic)
+static int lxc_ovs_attach_bridge_exec(void *data)
 {
-       pid_t pid;
-       char *cmd;
-       int ret;
+       struct ovs_veth_args *args = data;
 
-       cmd = on_path("ovs-vsctl", NULL);
-       if (!cmd)
-               return -1;
-       free(cmd);
+       execlp("ovs-vsctl", "ovs-vsctl", "add-port", args->bridge, args->nic,
+              (char *)NULL);
+       return -1;
+}
 
-       pid = fork();
-       if (pid < 0)
+static int lxc_ovs_attach_bridge(const char *bridge, const char *nic)
+{
+       int ret;
+       char cmd_output[MAXPATHLEN];
+       struct ovs_veth_args args;
+
+       args.bridge = bridge;
+       args.nic = nic;
+       ret = run_command(cmd_output, sizeof(cmd_output),
+                         lxc_ovs_attach_bridge_exec, (void *)&args);
+       if (ret < 0) {
+               ERROR("Failed to attach \"%s\" to openvswitch bridge \"%s\": %s",
+                     bridge, nic, cmd_output);
                return -1;
-       if (pid > 0) {
-               ret = wait_for_pid(pid);
-               if (ret < 0)
-                       return ret;
-               pid = fork();
-               if (pid < 0)
-                       return -1;  // how to properly recover?
-               if (pid > 0)
-                       return 0;
-               ovs_cleanup_nic(lxcpath, name, bridge, nic);
-               exit(0);
        }
 
-       if (execlp("ovs-vsctl", "ovs-vsctl", "add-port", bridge, nic, (char *)NULL))
-               exit(1);
-       // not reached
-       exit(1);
+       return 0;
 }
 
-/*
- * There is a lxc_bridge_attach, but no need of a bridge detach
- * as automatically done by kernel when a netdev is deleted.
- */
-int lxc_bridge_attach(const char *lxcpath, const char *name, const char *bridge, const char *ifname)
+int lxc_bridge_attach(const char *bridge, const char *ifname)
 {
        int fd, index, err;
        struct ifreq ifr;
@@ -1482,7 +1483,7 @@ int lxc_bridge_attach(const char *lxcpath, const char *name, const char *bridge,
                return -EINVAL;
 
        if (is_ovs_bridge(bridge))
-               return attach_to_ovs_bridge(lxcpath, name, bridge, ifname);
+               return lxc_ovs_attach_bridge(bridge, ifname);
 
        fd = socket(AF_INET, SOCK_STREAM, 0);
        if (fd < 0)

diff --git a/src/lxc/network.h b/src/lxc/network.h
index aa19dae33c35fffab498344583f32d31bbf9d836..b9dec641b6b415620fa723096ec26422625b2bab 100644 (file)
--- a/src/lxc/network.h
+++ b/src/lxc/network.h
@@ -109,7 +109,10 @@ extern int lxc_ipv6_gateway_add(int ifindex, struct in6_addr *gw);
 /*
  * Attach an interface to the bridge
  */
-extern int lxc_bridge_attach(const char *lxcpath, const char *name, const char *bridge, const char *ifname);
+extern int lxc_bridge_attach(const char *bridge, const char *ifname);
+extern int lxc_ovs_delete_port(const char *bridge, const char *nic);
+
+extern bool is_ovs_bridge(const char *bridge);
 
 /*
  * Create default gateway