[lxc-devel] [PATCH] fork off a task to delete ovs ports when done
Stéphane Graber
stgraber at ubuntu.com
Thu Jan 28 11:27:45 UTC 2016
On Thu, Jan 14, 2016 at 07:48:57AM +0000, Serge Hallyn wrote:
> The new task waits until the container is STOPPED, then asks
> openvswitch to delete the port.
>
> This requires two new arguements to be sent to lxc-user-nic.
> Since lxc-user-nic ships with lxc, this shouldn't be a problem.
>
> Finally when calling lxc-user-nic, use execlp insteac of execvp
> to preserve lxcpath's const-ness. Technically we are
> guaranteed that execvp won't change the args, but it's worth
> it to silence the warnings (and not hide real errors).
>
> With this patch, container nics are cleaned up from openvswitch
> bridges on shutdown.
>
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
> src/lxc/conf.c | 14 ++++++++------
> src/lxc/conf.h | 3 ++-
> src/lxc/lxc_user_nic.c | 21 +++++++++++++--------
> src/lxc/network.c | 36 +++++++++++++++++++++++++++++++-----
> src/lxc/network.h | 2 +-
> src/lxc/start.c | 3 ++-
> 6 files changed, 57 insertions(+), 22 deletions(-)
>
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index fc31513..fff57b6 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -2621,7 +2621,7 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
> }
>
> if (netdev->link) {
> - err = lxc_bridge_attach(netdev->link, veth1);
> + err = lxc_bridge_attach(handler->lxcpath, handler->name, netdev->link, veth1);
> if (err) {
> ERROR("failed to attach '%s' to the bridge '%s': %s",
> veth1, netdev->link, strerror(-err));
> @@ -2944,7 +2944,8 @@ void lxc_delete_network(struct lxc_handler *handler)
>
> /* lxc-user-nic returns "interface_name:interface_name\n" */
> #define MAX_BUFFER_SIZE IFNAMSIZ*2 + 2
> -static int unpriv_assign_nic(struct lxc_netdev *netdev, pid_t pid)
> +static int unpriv_assign_nic(const char *lxcpath, char *lxcname,
> + struct lxc_netdev *netdev, pid_t pid)
> {
> pid_t child;
> int bytes, pipefd[2];
> @@ -2985,10 +2986,10 @@ static int unpriv_assign_nic(struct lxc_netdev *netdev, pid_t pid)
> } else {
> strncpy(netdev_link, "none", IFNAMSIZ);
> }
> - char *args[] = {LXC_USERNIC_PATH, pidstr, "veth", netdev_link, netdev->name, NULL };
> snprintf(pidstr, 19, "%lu", (unsigned long) pid);
> pidstr[19] = '\0';
> - execvp(args[0], args);
> + execlp(LXC_USERNIC_PATH, LXC_USERNIC_PATH, lxcpath, lxcname,
> + pidstr, "veth", netdev_link, netdev->name, NULL);
> SYSERROR("execvp lxc-user-nic");
> exit(1);
> }
> @@ -3035,7 +3036,8 @@ static int unpriv_assign_nic(struct lxc_netdev *netdev, pid_t pid)
> return 0;
> }
>
> -int lxc_assign_network(struct lxc_list *network, pid_t pid)
> +int lxc_assign_network(const char *lxcpath, char *lxcname,
> + struct lxc_list *network, pid_t pid)
> {
> struct lxc_list *iterator;
> struct lxc_netdev *netdev;
> @@ -3048,7 +3050,7 @@ int lxc_assign_network(struct lxc_list *network, pid_t pid)
> netdev = iterator->elem;
>
> if (netdev->type == LXC_NET_VETH && !am_root) {
> - if (unpriv_assign_nic(netdev, pid))
> + if (unpriv_assign_nic(lxcpath, lxcname, netdev, pid))
> return -1;
> // lxc-user-nic has moved the nic to the new ns.
> // unpriv_assign_nic() fills in netdev->name.
> diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> index b0274ec..f2323f6 100644
> --- a/src/lxc/conf.h
> +++ b/src/lxc/conf.h
> @@ -400,7 +400,8 @@ 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 int lxc_assign_network(struct lxc_list *networks, pid_t pid);
> +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);
> extern int lxc_find_gateway_addresses(struct lxc_handler *handler);
>
> diff --git a/src/lxc/lxc_user_nic.c b/src/lxc/lxc_user_nic.c
> index af7f8b4..87780ca 100644
> --- a/src/lxc/lxc_user_nic.c
> +++ b/src/lxc/lxc_user_nic.c
> @@ -53,11 +53,13 @@
>
> static void usage(char *me, bool fail)
> {
> - fprintf(stderr, "Usage: %s pid type bridge nicname\n", me);
> + fprintf(stderr, "Usage: %s lxcpath name pid type bridge nicname\n", me);
> fprintf(stderr, " nicname is the name to use inside the container\n");
> exit(fail ? 1 : 0);
> }
>
> +static char *lxcpath, *lxcname;
> +
> static int open_and_lock(char *path)
> {
> int fd;
> @@ -444,7 +446,7 @@ static bool create_nic(char *nic, char *br, int pid, char **cnic)
> }
>
> /* attach veth1 to bridge */
> - if (lxc_bridge_attach(br, veth1buf) < 0) {
> + if (lxc_bridge_attach(lxcpath, lxcname, br, veth1buf) < 0) {
> fprintf(stderr, "Error attaching %s to %s\n", veth1buf, br);
> goto out_del;
> }
> @@ -803,13 +805,16 @@ int main(int argc, char *argv[])
> exit(1);
> }
>
> - if (argc < 4)
> + if (argc < 6)
> usage(argv[0], true);
> - if (argc >= 5)
> - vethname = argv[4];
> + if (argc >= 7)
> + vethname = argv[6];
> +
> + lxcpath = argv[1];
> + lxcname = argv[2];
>
> errno = 0;
> - pid = (int) strtol(argv[1], NULL, 10);
> + pid = (int) strtol(argv[3], NULL, 10);
> if (errno) {
> fprintf(stderr, "Could not read pid: %s\n", argv[1]);
> exit(1);
> @@ -831,9 +836,9 @@ int main(int argc, char *argv[])
> exit(1);
> }
>
> - n = get_alloted(me, argv[2], argv[3], &alloted);
> + n = get_alloted(me, argv[4], argv[5], &alloted);
> if (n > 0)
> - gotone = get_nic_if_avail(fd, alloted, pid, argv[2], argv[3], n, &nicname, &cnic);
> + gotone = get_nic_if_avail(fd, alloted, pid, argv[4], argv[5], n, &nicname, &cnic);
>
> close(fd);
> free_alloted(&alloted);
> diff --git a/src/lxc/network.c b/src/lxc/network.c
> index 3417928..49633ab 100644
> --- a/src/lxc/network.c
> +++ b/src/lxc/network.c
> @@ -36,6 +36,7 @@
> #include <sys/socket.h>
> #include <sys/param.h>
> #include <sys/ioctl.h>
> +#include <sys/inotify.h>
Where is that include used?
> #include <arpa/inet.h>
> #include <net/if.h>
> #include <net/if_arp.h>
> @@ -1403,10 +1404,25 @@ static bool is_ovs_bridge(const char *bridge)
> return false;
> }
>
> -static int attach_to_ovs_bridge(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)
> +{
> + if (lxc_check_inherited(NULL, true, -1) < 0)
> + return;
> + if (lxc_wait(name, "STOPPED", -1, lxcpath) < 0)
> + return;
> + execlp("ovs-vsctl", "ovs-vsctl", "del-port", bridge, nic, NULL);
> + exit(1); /* not reached */
> +}
> +
> +static int attach_to_ovs_bridge(const char *lxcpath, const char *name, const char *bridge, const char *nic)
> {
> pid_t pid;
> char *cmd;
> + int ret;
>
> cmd = on_path("ovs-vsctl", NULL);
> if (!cmd)
> @@ -1416,8 +1432,18 @@ static int attach_to_ovs_bridge(const char *bridge, const char *nic)
> pid = fork();
> if (pid < 0)
> return -1;
> - if (pid > 0)
> - return wait_for_pid(pid);
> + 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, NULL))
> exit(1);
> @@ -1429,7 +1455,7 @@ static int attach_to_ovs_bridge(const char *bridge, const char *nic)
> * 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 *bridge, const char *ifname)
> +int lxc_bridge_attach(const char *lxcpath, const char *name, const char *bridge, const char *ifname)
> {
> int fd, index, err;
> struct ifreq ifr;
> @@ -1442,7 +1468,7 @@ int lxc_bridge_attach(const char *bridge, const char *ifname)
> return -EINVAL;
>
> if (is_ovs_bridge(bridge))
> - return attach_to_ovs_bridge(bridge, ifname);
> + return attach_to_ovs_bridge(lxcpath, name, 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 cd42c8f..aa19dae 100644
> --- a/src/lxc/network.h
> +++ b/src/lxc/network.h
> @@ -109,7 +109,7 @@ 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 *bridge, const char *ifname);
> +extern int lxc_bridge_attach(const char *lxcpath, const char *name, const char *bridge, const char *ifname);
>
> /*
> * Create default gateway
> diff --git a/src/lxc/start.c b/src/lxc/start.c
> index 06bfd4b..06ca8a4 100644
> --- a/src/lxc/start.c
> +++ b/src/lxc/start.c
> @@ -1102,7 +1102,8 @@ static int lxc_spawn(struct lxc_handler *handler)
>
> /* Create the network configuration */
> if (handler->clone_flags & CLONE_NEWNET) {
> - if (lxc_assign_network(&handler->conf->network, handler->pid)) {
> + if (lxc_assign_network(handler->lxcpath, handler->name,
> + &handler->conf->network, handler->pid)) {
> ERROR("failed to create the configured network");
> goto out_delete_net;
> }
> --
> 2.5.0
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160128/c2d60da3/attachment-0001.sig>
More information about the lxc-devel
mailing list