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 cf4cb3c9cc7ed05766bc9636f1e4e6b25278ca87..a3c19fbf722a2b75d461f5c0ca734f713e6d592f 100644 (file)
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -2859,7 +2859,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));
@@ -3239,11 +3239,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);
@@ -3251,7 +3260,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 cff109cb4c7a78c7949732cb18cd2125c239fd65..af44a3210831c15b6d53ca0a67998f7ed21d63c1 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 28aaa95435d5dcfbd12345b7e8c3d2a6828dec11..d1353eafe13caa3f788dda7f1c9116475708378f 100644 (file)
--- a/src/lxc/network.c
+++ b/src/lxc/network.c
@@ -1413,7 +1413,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)
 {
        int ret;
        struct stat sb;
@@ -1431,73 +1431,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;
-
-       TRACE("Registering cleanup thread to remove nic \"%s\" from "
-             "openvswitch bridge \"%s\"", nic, bridge);
-
-       ret = lxc_wait(name, "STOPPED", -1, lxcpath);
-       if (ret < 0) {
-               ERROR("Failed to register cleanup thread to remove nic \"%s\" "
-                     "from  openvswitch bridge \"%s\"", nic, bridge);
-               return;
-       }
-
-       execlp("ovs-vsctl", "ovs-vsctl", "del-port", bridge, nic, (char *)NULL);
-       exit(EXIT_FAILURE);
+       execlp("ovs-vsctl", "ovs-vsctl", "del-port", args->bridge, args->nic,
+              (char *)NULL);
+       return -1;
 }
 
-static int attach_to_ovs_bridge(const char *lxcpath, const char *name,
-                               const char *bridge, const char *nic)
+int lxc_ovs_delete_port(const char *bridge, const char *nic)
 {
-       pid_t pid;
-       char *cmd;
        int ret;
+       char cmd_output[MAXPATHLEN];
+       struct ovs_veth_args args;
 
-       cmd = on_path("ovs-vsctl", NULL);
-       if (!cmd)
+       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 delete \"%s\" from openvswitch bridge \"%s\": "
+                     "%s", bridge, nic, cmd_output);
                return -1;
-       free(cmd);
+       }
 
-       pid = fork();
-       if (pid < 0)
-               return -1;
+       return 0;
+}
 
-       if (pid > 0) {
-               ret = wait_for_pid(pid);
-               if (ret < 0)
-                       return ret;
+static int lxc_ovs_attach_bridge_exec(void *data)
+{
+       struct ovs_veth_args *args = data;
 
-               pid = fork();
-               if (pid < 0)
-                       return -1;
+       execlp("ovs-vsctl", "ovs-vsctl", "add-port", args->bridge, args->nic,
+              (char *)NULL);
+       return -1;
+}
 
-               if (pid > 0)
-                       return 0;
+static int lxc_ovs_attach_bridge(const char *bridge, const char *nic)
+{
+       int ret;
+       char cmd_output[MAXPATHLEN];
+       struct ovs_veth_args args;
 
-               ovs_cleanup_nic(lxcpath, name, bridge, nic);
-               exit(EXIT_SUCCESS);
+       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;
        }
 
-       execlp("ovs-vsctl", "ovs-vsctl", "add-port", bridge, nic, (char *)NULL);
-       exit(EXIT_FAILURE);
+       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 err, fd, index;
        struct ifreq ifr;
@@ -1510,7 +1508,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 f47791b8f350ee437a36f61ac934ac2a80dbec8d..8a79a06228cf3a0f9d1ccd7102adfdbdf3066531 100644 (file)
--- a/src/lxc/network.h
+++ b/src/lxc/network.h
@@ -87,8 +87,10 @@ extern int lxc_ipv4_gateway_add(int ifindex, struct in_addr *gw);
 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. */
 extern int lxc_route_create_default(const char *addr, const char *ifname,