[CRIU] [PATCH 2/2 v2] net: restore macvlan by the same way with and without userns
Tycho Andersen
tycho.andersen at canonical.com
Thu Oct 27 10:17:08 PDT 2016
On Thu, Oct 27, 2016 at 07:26:45PM +0300, Andrei Vagin wrote:
> From: Andrei Vagin <avagin at virtuozzo.com>
>
> If userns_restore_one_link() is called outside of usernsd,
> it switches into the criu namespace and switches back before exiting.
>
> v2: rid of the include of linux/net_namespace.h in criu/include/net.h,
> as well as the associated defines and feature checks
>
> Signed-off-by: Andrei Vagin <avagin at virtuozzo.com>
Acked-by: Tycho Andersen <tycho.andersen at canonical.com>
> ---
> Makefile.config | 2 +-
> criu/cr-check.c | 54 --------------
> criu/include/net.h | 7 --
> criu/net.c | 164 +++++-------------------------------------
> scripts/feature-tests.mak | 11 ---
> test/zdtm/static/macvlan.desc | 2 +-
> 6 files changed, 18 insertions(+), 222 deletions(-)
>
> diff --git a/Makefile.config b/Makefile.config
> index 2259ad6..cce32fa 100644
> --- a/Makefile.config
> +++ b/Makefile.config
> @@ -28,7 +28,7 @@ export DEFINES += $(FEATURE_DEFINES)
> export CFLAGS += $(FEATURE_DEFINES)
>
> FEATURES_LIST := TCP_REPAIR STRLCPY STRLCAT PTRACE_PEEKSIGINFO \
> - SETPROCTITLE_INIT MEMFD TCP_REPAIR_WINDOW NET_NAMESPACE_H
> + SETPROCTITLE_INIT MEMFD TCP_REPAIR_WINDOW
>
> # $1 - config name
> define gen-feature-test
> diff --git a/criu/cr-check.c b/criu/cr-check.c
> index 6456486..0422860 100644
> --- a/criu/cr-check.c
> +++ b/criu/cr-check.c
> @@ -938,58 +938,6 @@ static int check_tcp_window(void)
> return 0;
> }
>
> -static int nsid_manip_cb(struct nlmsghdr *hdr, void *arg)
> -{
> - return 0;
> -}
> -
> -static int check_nsid_manip(void)
> -{
> - int parent = -1, ret = -1, nlsk = -1;
> - struct {
> - struct nlmsghdr n;
> - struct rtgenmsg g;
> - char buf[1024];
> - } req;
> -
> - parent = open("/proc/self/ns/net", O_RDONLY);
> - if (parent < 0) {
> - pr_perror("open");
> - return -1;
> - }
> -
> - if (unshare(CLONE_NEWNET) < 0) {
> - pr_perror("unshare");
> - goto out;
> - }
> -
> - nlsk = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
> - if (nlsk < 0) {
> - pr_perror("socket");
> - goto out;
> - }
> -
> - memset(&req, 0, sizeof(req));
> -
> - req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.g));
> - req.n.nlmsg_flags = NLM_F_REQUEST|NLM_F_ACK;
> - req.n.nlmsg_type = RTM_GETNSID;
> - req.n.nlmsg_seq = CR_NLMSG_SEQ;
> -
> - addattr_l(&req.n, sizeof(req), NETNSA_FD, &parent, sizeof(parent));
> - ret = do_rtnl_req(nlsk, &req, req.n.nlmsg_len, nsid_manip_cb, NULL, NULL);
> - if (ret < 0)
> - pr_err("can't manipulate netns ids\n");
> -out:
> - if (nlsk > 0)
> - close(nlsk);
> - if (setns(parent, CLONE_NEWNET) < 0)
> - pr_warn("couldn't setns back to parent");
> - if (parent > 0)
> - close(parent);
> - return ret;
> -}
> -
> static int (*chk_feature)(void);
>
> /*
> @@ -1090,7 +1038,6 @@ int cr_check(void)
> ret |= check_clone_parent_vs_pid();
> ret |= check_cgroupns();
> ret |= check_tcp_window();
> - ret |= check_nsid_manip();
> }
>
> /*
> @@ -1170,7 +1117,6 @@ static struct feature_list feature_list[] = {
> { "loginuid", check_loginuid },
> { "cgroupns", check_cgroupns },
> { "autofs", check_autofs },
> - { "nsid_manip", check_nsid_manip },
> { NULL, NULL },
> };
>
> diff --git a/criu/include/net.h b/criu/include/net.h
> index 2ec3a31..f42a21c 100644
> --- a/criu/include/net.h
> +++ b/criu/include/net.h
> @@ -6,13 +6,6 @@
> #include "common/list.h"
> #include "external.h"
>
> -#ifdef CONFIG_HAS_NET_NAMESPACE_H
> -#include <linux/net_namespace.h>
> -#else
> -#define NETNSA_NSID 1
> -#define NETNSA_FD 3
> -#endif
> -
> #ifndef RTM_GETNSID
> #define RTM_GETNSID 90
> #endif
> diff --git a/criu/net.c b/criu/net.c
> index 35388ee..f563f00 100644
> --- a/criu/net.c
> +++ b/criu/net.c
> @@ -891,7 +891,6 @@ struct newlink_req {
> * request.
> */
> struct newlink_extras {
> - int netns_id; /* IFLA_NET_NS_ID */
> int link; /* IFLA_LINK */
> int target_netns; /* IFLA_NET_NS_FD */
> };
> @@ -916,9 +915,6 @@ static int populate_newlink_req(struct newlink_req *req, int msg_type, NetDevice
> req->i.ifi_flags = nde->flags;
>
> if (extras) {
> - if (extras->netns_id >= 0)
> - addattr_l(&req->h, sizeof(*req), IFLA_LINK_NETNSID, &extras->netns_id, sizeof(extras->netns_id));
> -
> if (extras->link >= 0)
> addattr_l(&req->h, sizeof(*req), IFLA_LINK, &extras->link, sizeof(extras->link));
>
> @@ -1101,108 +1097,39 @@ static int userns_restore_one_link(void *arg, int fd, pid_t pid)
> {
> int nlsk, ret;
> struct newlink_req *req = arg;
> + int ns_fd = get_service_fd(NS_FD_OFF), rst = -1;
> +
> + if (!(root_ns_mask & CLONE_NEWUSER)) {
> + if (switch_ns_by_fd(ns_fd, &net_ns_desc, &rst))
> + return -1;
> + }
>
> nlsk = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
> if (nlsk < 0) {
> pr_perror("Can't create nlk socket");
> - return -1;
> + ret = -1;
> + goto out;
> }
>
> addattr_l(&req->h, sizeof(*req), IFLA_NET_NS_FD, &fd, sizeof(fd));
>
> ret = do_rtnl_req(nlsk, req, req->h.nlmsg_len, restore_link_cb, NULL, NULL);
> close(nlsk);
> - return ret;
> -}
> -
> -static int get_nsid_cb(struct nlmsghdr *nlh, void *arg)
> -{
> - struct rtgenmsg *rthdr;
> - struct rtattr *rta;
> - int len, *netnsid = arg;
> -
> - rthdr = NLMSG_DATA(nlh);
> - len = nlh->nlmsg_len - NLMSG_SPACE(sizeof(*rthdr));
> -
> - if (len < 0)
> - return -1;
> -
> - rta = NETNS_RTA(rthdr);
> -
> - while (RTA_OK(rta, len)) {
> - if (rta->rta_type == NETNSA_NSID)
> - *netnsid = *((int *) RTA_DATA(rta));
> - rta = RTA_NEXT(rta, len);
> - }
> -
> - if (netnsid < 0) {
> - pr_err("Didn't get a netnsid back from netlink?\n");
> - return -1;
> - }
> -
> - return 0;
> -}
> -
> -static int get_criu_netnsid(int nlsk)
> -{
> - static int netnsid = -1;
> - struct {
> - struct nlmsghdr n;
> - struct rtgenmsg g;
> - char buf[1024];
> - } req;
> - int ns_fd = get_service_fd(NS_FD_OFF), i;
> -
> - if (netnsid > 0)
> - return netnsid;
> -
> - for (i = 0; i < 10; i++) {
> - int ret;
> -
> - memset(&req, 0, sizeof(req));
> -
> - req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.g));
> - req.n.nlmsg_flags = NLM_F_REQUEST|NLM_F_ACK;
> - req.n.nlmsg_type = RTM_NEWNSID;
> - req.n.nlmsg_seq = CR_NLMSG_SEQ;
> -
> - addattr_l(&req.n, sizeof(req), NETNSA_FD, &ns_fd, sizeof(ns_fd));
> - addattr_l(&req.n, sizeof(req), NETNSA_NSID, &i, sizeof(i));
> -
> - ret = do_rtnl_req(nlsk, &req, req.n.nlmsg_len, NULL, NULL, NULL);
> - if (ret < 0) {
> - if (ret == -EEXIST) {
> - req.n.nlmsg_type = RTM_GETNSID;
> - ret = do_rtnl_req(nlsk, &req, req.n.nlmsg_len, get_nsid_cb, NULL, &netnsid);
> - if (ret < 0) {
> - pr_err("Couldn't get netnsid: %d\n", ret);
> - return -1;
> - }
> -
> - return netnsid;
> - }
> - errno = -ret;
> - pr_perror("couldn't create new netnsid");
> - return -1;
> - }
>
> - netnsid = i;
> - return netnsid;
> - }
> -
> - pr_err("tried to create too many netnsids\n");
> - return -1;
> +out:
> + if (rst >= 0 && restore_ns(rst, &net_ns_desc) < 0)
> + ret = -1;
> + return ret;
> }
>
> static int restore_one_macvlan(NetDeviceEntry *nde, int nlsk, int criu_nlsk)
> {
> struct newlink_extras extras = {
> - .netns_id = -1,
> .link = -1,
> .target_netns = -1,
> };
> char key[100], *val;
> - int my_netns = -1, ret = -1, s;
> + int my_netns = -1, ret = -1;
>
> snprintf(key, sizeof(key), "macvlan[%s]", nde->name);
> val = external_lookup_data(key);
> @@ -1223,19 +1150,13 @@ static int restore_one_macvlan(NetDeviceEntry *nde, int nlsk, int criu_nlsk)
> */
> extras.link = (int) (unsigned long) val;
>
> - extras.netns_id = get_criu_netnsid(nlsk);
> - if (extras.netns_id < 0) {
> - pr_err("failed to get criu's netnsid\n");
> - return -1;
> - }
> -
> my_netns = open_proc(PROC_SELF, "ns/net");
> if (my_netns < 0) {
> pr_perror("couldn't get my netns");
> return -1;
> }
>
> - if (root_ns_mask & CLONE_NEWUSER) {
> + {
> struct newlink_req req;
>
> if (populate_newlink_req(&req, RTM_NEWLINK, nde, macvlan_link_info, &extras) < 0)
> @@ -1245,27 +1166,9 @@ static int restore_one_macvlan(NetDeviceEntry *nde, int nlsk, int criu_nlsk)
> pr_err("couldn't restore macvlan interface %s via usernsd\n", nde->name);
> goto out;
> }
> - } else {
> - extras.target_netns = my_netns;
> - ret = restore_one_link(nde, criu_nlsk, macvlan_link_info, &extras);
> - if (ret < 0)
> - return -1;
> }
>
> - /* We have to change the flags of the NDE manually here because
> - * we used IFLA_LINK_NETNSID to restore it, which creates the
> - * device and then shuts it down when it changes the device's
> - * namespace, but doesn't start it back up when it goes to the
> - * other namespace. So, we restore its state here.
> - */
> - s = socket(AF_LOCAL, SOCK_STREAM, 0);
> - if (s < 0) {
> - pr_perror("couldn't open socket for flag changing");
> - goto out;
> - }
> - ret = changeflags(s, nde->name, nde->flags);
> - close(s);
> -
> + ret = 0;
> out:
> if (my_netns >= 0)
> close(my_netns);
> @@ -1300,7 +1203,7 @@ static int restore_link(NetDeviceEntry *nde, int nlsk, int criu_nlsk)
>
> static int restore_links(int pid, NetnsEntry **netns)
> {
> - int nlsk, criu_nlsk = -1, ret = -1, my_netns = -1, ns_fd = get_service_fd(NS_FD_OFF);
> + int nlsk, criu_nlsk = -1, ret = -1;
> struct cr_img *img;
> NetDeviceEntry *nde;
>
> @@ -1315,38 +1218,6 @@ static int restore_links(int pid, NetnsEntry **netns)
> return -1;
> }
>
> - if (!(root_ns_mask & CLONE_NEWUSER)) {
> - /* FIXME: this whole dance is so we can have a netlink socket to criu's
> - * netns in case we need it. It should really live on the ns_id struct,
> - * but those aren't generated on restore yet.
> - */
> - my_netns = open_proc(PROC_SELF, "ns/net");
> - if (my_netns < 0) {
> - pr_perror("couldn't open my netns");
> - goto out;
> - }
> -
> - if (setns(ns_fd, CLONE_NEWNET) < 0) {
> - close(my_netns);
> - pr_perror("couldn't setns to parent ns");
> - goto out;
> - }
> -
> - criu_nlsk = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
> - ret = setns(my_netns, CLONE_NEWNET);
> - close(my_netns);
> -
> - if (ret < 0) {
> - pr_perror("Can't setns back my netns");
> - goto out;
> - }
> -
> - if (criu_nlsk < 0) {
> - pr_perror("Can't create nlk socket");
> - goto out;
> - }
> - }
> -
> while (1) {
> NetnsEntry **def_netns = netns;
>
> @@ -1383,9 +1254,6 @@ exit:
> break;
> }
>
> -out:
> - if (criu_nlsk >= 0)
> - close(criu_nlsk);
> close(nlsk);
> close_image(img);
> return ret;
> diff --git a/scripts/feature-tests.mak b/scripts/feature-tests.mak
> index 8812c47..ad50eb4 100644
> --- a/scripts/feature-tests.mak
> +++ b/scripts/feature-tests.mak
> @@ -100,14 +100,3 @@ int main(int argc, char **argv)
> }
>
> endef
> -
> -define FEATURE_TEST_NET_NAMESPACE_H
> -
> -#include <linux/net_namespace.h>
> -
> -int main(int argc, char **argv)
> -{
> - return 0;
> -}
> -
> -endef
> diff --git a/test/zdtm/static/macvlan.desc b/test/zdtm/static/macvlan.desc
> index f7b15e4..49e90d7 100644
> --- a/test/zdtm/static/macvlan.desc
> +++ b/test/zdtm/static/macvlan.desc
> @@ -1 +1 @@
> -{'flavor': 'ns uns', 'deps': [ '/bin/sh', '/usr/bin/sort', '/bin/grep', '/sbin/ip', '/usr/bin/diff'], 'flags': 'suid', 'ropts': '--external macvlan[zdtmmvlan0]:zdtmbr0', 'feature': 'nsid_manip'}
> +{'flavor': 'ns uns', 'deps': [ '/bin/sh', '/usr/bin/sort', '/bin/grep', '/sbin/ip', '/usr/bin/diff'], 'flags': 'suid', 'ropts': '--external macvlan[zdtmmvlan0]:zdtmbr0'}
> --
> 2.7.4
>
More information about the CRIU
mailing list