Skip to content

Commit

Permalink
openvswitch: delete ports intelligently
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Christian Brauner committed Aug 25, 2017
1 parent 1efc106 commit 581c75e
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 54 deletions.
22 changes: 20 additions & 2 deletions src/lxc/conf.c
Expand Up @@ -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));
Expand Down Expand Up @@ -3239,19 +3239,37 @@ 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);
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));

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));
}
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/lxc/lxc_user_nic.c
Expand Up @@ -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;
Expand Down
96 changes: 47 additions & 49 deletions src/lxc/network.c
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions src/lxc/network.h
Expand Up @@ -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,
Expand Down

0 comments on commit 581c75e

Please sign in to comment.