[CRIU] [PATCH CRIU 04/14] dump/net/ipv4: add unused bitmask to allow negative sysctl values
Pavel Tikhomirov
ptikhomirov at virtuozzo.com
Mon Apr 4 03:21:47 PDT 2016
On 04/04/2016 01:14 PM, Pavel Emelyanov wrote:
> On 03/30/2016 01:42 PM, Pavel Tikhomirov wrote:
>> Some sysctls can be set with value < 0: ipv4/conf//accept_source_route,
>> ipv4/conf//medium_id, ipv4/conf//tag, ipv6/conf//accept_source_route,
>> ipv6/conf//keep_addr_on_down. So we can not use -1 as unused value.
>
> I don't get the problem we fix with this patch. Why not just saving whatever
> value we have instead of marking it with a special bit?
The problem is on old kernels we might not have all sysctls, so we need
somehow mark those to not to fail on them on restore. Before these
patch, we marked unused sysctls with -1, and did not restore it. But
what if some sysctl is really set to -1?
>
>> So 1) make sysctl_op set CTL_FLAGS_UNUSED flags for sysctl files which
>> are CTL_FLAGS_OPTIONAL and does not exist, 2) write info about unused
>> files into bitmask on image.
>>
>> Leave DEVCONFS_UNUSED for forward/backward compatibility.
>>
>> https://jira.sw.ru/browse/PSBM-30942
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>> ---
>> criu/include/sysctl.h | 1 +
>> criu/net.c | 53 ++++++++++++++++++++++++++++++++++++++-------------
>> criu/sysctl.c | 5 ++++-
>> images/netdev.proto | 2 ++
>> 4 files changed, 47 insertions(+), 14 deletions(-)
>>
>> diff --git a/criu/include/sysctl.h b/criu/include/sysctl.h
>> index b949a40..79a2711 100644
>> --- a/criu/include/sysctl.h
>> +++ b/criu/include/sysctl.h
>> @@ -35,5 +35,6 @@ enum {
>> * Some entries might be missing mark them as optional.
>> */
>> #define CTL_FLAGS_OPTIONAL 1
>> +#define CTL_FLAGS_UNUSED 2
>>
>> #endif /* __CR_SYSCTL_H__ */
>> diff --git a/criu/net.c b/criu/net.c
>> index 096cd25..4a5ae17 100644
>> --- a/criu/net.c
>> +++ b/criu/net.c
>> @@ -95,9 +95,8 @@ static char *devconfs4[] = {
>> };
>>
>> /*
>> - * I case if some entry is missing in
>> - * the kernel, simply write DEVCONFS_UNUSED
>> - * into the image so we would skip it.
>> + * Leave for compatibility if have no mask_unused
>> + * use it to determine missing entries.
>> */
>> #define DEVCONFS_UNUSED (-1u)
>>
>> @@ -106,7 +105,7 @@ static char *devconfs4[] = {
>>
>> static int net_conf_op(char *tgt, int *conf, int n, int op, char *proto,
>> struct sysctl_req *req, char (*path)[MAX_CONF_OPT_PATH], int size,
>> - char **devconfs, int32_t *def_conf)
>> + char **devconfs, int *mask_unused, int32_t *def_conf)
>> {
>> int i, ri;
>> int ret, flags = op == CTL_READ ? CTL_FLAGS_OPTIONAL : 0;
>> @@ -127,9 +126,13 @@ static int net_conf_op(char *tgt, int *conf, int n, int op, char *proto,
>> continue;
>> }
>>
>> - if (op == CTL_WRITE && conf[i] == DEVCONFS_UNUSED)
>> + if (op == CTL_WRITE && (mask_unused
>> + ? mask_unused[i / WORD_BIT] & (1 << (i % WORD_BIT))
>> + /* Forward compatibility */
>> + : conf[i] == DEVCONFS_UNUSED))
>> continue;
>> else if (op == CTL_READ)
>> + /* Backward compatibility */
>> conf[i] = DEVCONFS_UNUSED;
>>
>> snprintf(path[i], MAX_CONF_OPT_PATH, CONF_OPT_PATH, proto, tgt, devconfs[i]);
>> @@ -145,17 +148,27 @@ static int net_conf_op(char *tgt, int *conf, int n, int op, char *proto,
>> pr_err("Failed to %s %s/<confs>\n", (op == CTL_READ)?"read":"write", tgt);
>> return -1;
>> }
>> +
>> + if (op == CTL_READ)
>> + for (i = 0; i < ri; i++) {
>> + if (req[i].flags & CTL_FLAGS_UNUSED) {
>> + int unused = (int *)req[i].arg - conf;
>> + mask_unused[unused / WORD_BIT] |= 1 << (unused % WORD_BIT);
>> + }
>> + }
>> +
>> return 0;
>> }
>>
>> -static int ipv4_conf_op(char *tgt, int *conf, int n, int op, NetnsEntry **netns)
>> +static int ipv4_conf_op(char *tgt, int *conf, int n,
>> + int op, int *mask_unused, NetnsEntry **netns)
>> {
>> struct sysctl_req req[ARRAY_SIZE(devconfs4)];
>> char path[ARRAY_SIZE(devconfs4)][MAX_CONF_OPT_PATH];
>>
>> return net_conf_op(tgt, conf, n, op, "ipv4",
>> req, path, ARRAY_SIZE(devconfs4),
>> - devconfs4, netns ? (*netns)->def_conf4 : NULL);
>> + devconfs4, mask_unused, netns ? (*netns)->def_conf4 : NULL);
>> }
>>
>> int write_netdev_img(NetDeviceEntry *nde, struct cr_imgset *fds)
>> @@ -163,11 +176,14 @@ int write_netdev_img(NetDeviceEntry *nde, struct cr_imgset *fds)
>> return pb_write_one(img_from_set(fds, CR_FD_NETDEV), nde, PB_NETDEV);
>> }
>>
>> +#define CONF_MASK_SIZE 2
>> +
>> static int dump_one_netdev(int type, struct ifinfomsg *ifi,
>> struct nlattr **tb, struct cr_imgset *fds,
>> int (*dump)(NetDeviceEntry *, struct cr_imgset *))
>> {
>> int ret;
>> + int mask_unused4[CONF_MASK_SIZE] = { 0 };
>> NetDeviceEntry netdev = NET_DEVICE_ENTRY__INIT;
>>
>> if (!tb[IFLA_IFNAME]) {
>> @@ -190,12 +206,15 @@ static int dump_one_netdev(int type, struct ifinfomsg *ifi,
>> (int)netdev.address.len, netdev.name);
>> }
>>
>> + netdev.n_mask_unused4 = CONF_MASK_SIZE;
>> + netdev.mask_unused4 = mask_unused4;
>> netdev.n_conf4 = ARRAY_SIZE(devconfs4);
>> netdev.conf4 = xmalloc(sizeof(int) * netdev.n_conf4);
>> if (!netdev.conf4)
>> return -1;
>>
>> - ret = ipv4_conf_op(netdev.name, netdev.conf4, netdev.n_conf4, CTL_READ, NULL);
>> + ret = ipv4_conf_op(netdev.name, netdev.conf4, netdev.n_conf4,
>> + CTL_READ, netdev.mask_unused4, NULL);
>> if (ret < 0)
>> goto err_free;
>>
>> @@ -762,7 +781,8 @@ static int restore_links(int pid, NetnsEntry **netns)
>> */
>> if (nde->type == ND_TYPE__LOOPBACK)
>> def_netns = NULL;
>> - ret = ipv4_conf_op(nde->name, nde->conf4, nde->n_conf4, CTL_WRITE, def_netns);
>> + ret = ipv4_conf_op(nde->name, nde->conf4, nde->n_conf4,
>> + CTL_WRITE, nde->mask_unused4, def_netns);
>> }
>> exit:
>> net_device_entry__free_unpacked(nde, NULL);
>> @@ -879,8 +899,11 @@ static inline int dump_iptables(struct cr_imgset *fds)
>> static int dump_netns_conf(struct cr_imgset *fds)
>> {
>> int ret, n;
>> + int mask_unused4[CONF_MASK_SIZE] = { 0 };
>> NetnsEntry netns = NETNS_ENTRY__INIT;
>>
>> + netns.n_mask_unused4 = CONF_MASK_SIZE;
>> + netns.mask_unused4 = mask_unused4;
>> netns.n_def_conf4 = ARRAY_SIZE(devconfs4);
>> netns.n_all_conf4 = ARRAY_SIZE(devconfs4);
>> netns.def_conf4 = xmalloc(sizeof(int) * netns.n_def_conf4);
>> @@ -893,10 +916,12 @@ static int dump_netns_conf(struct cr_imgset *fds)
>> }
>>
>> n = netns.n_def_conf4;
>> - ret = ipv4_conf_op("default", netns.def_conf4, n, CTL_READ, NULL);
>> + ret = ipv4_conf_op("default", netns.def_conf4, n,
>> + CTL_READ, netns.mask_unused4, NULL);
>> if (ret < 0)
>> goto err_free;
>> - ret = ipv4_conf_op("all", netns.all_conf4, n, CTL_READ, NULL);
>> + ret = ipv4_conf_op("all", netns.all_conf4, n,
>> + CTL_READ, netns.mask_unused4, NULL);
>> if (ret < 0)
>> goto err_free;
>>
>> @@ -1017,10 +1042,12 @@ static int restore_netns_conf(int pid, NetnsEntry **netns)
>> }
>>
>> n = (*netns)->n_def_conf4;
>> - ret = ipv4_conf_op("default", (*netns)->def_conf4, n, CTL_WRITE, NULL);
>> + ret = ipv4_conf_op("default", (*netns)->def_conf4, n,
>> + CTL_WRITE, (*netns)->mask_unused4, NULL);
>> if (ret)
>> goto out;
>> - ret = ipv4_conf_op("all", (*netns)->all_conf4, n, CTL_WRITE, NULL);
>> + ret = ipv4_conf_op("all", (*netns)->all_conf4, n,
>> + CTL_WRITE, (*netns)->mask_unused4, NULL);
>> out:
>> close_image(img);
>> return ret;
>> diff --git a/criu/sysctl.c b/criu/sysctl.c
>> index 21ae4ce..a0e414d 100644
>> --- a/criu/sysctl.c
>> +++ b/criu/sysctl.c
>> @@ -255,8 +255,10 @@ static int __userns_sysctl_op(void *arg, int proc_fd, pid_t pid)
>>
>> fd = openat(dir, req->name, flags);
>> if (fd < 0) {
>> - if (errno == ENOENT && (req->flags & CTL_FLAGS_OPTIONAL))
>> + if (errno == ENOENT && (req->flags & CTL_FLAGS_OPTIONAL)) {
>> + req->flags |= CTL_FLAGS_UNUSED;
>> continue;
>> + }
>> pr_perror("Can't open sysctl %s", req->name);
>> goto out;
>> }
>> @@ -364,6 +366,7 @@ static int __nonuserns_sysctl_op(struct sysctl_req *req, size_t nr_req, int op)
>> fd = openat(dir, req->name, flags);
>> if (fd < 0) {
>> if (errno == ENOENT && (req->flags & CTL_FLAGS_OPTIONAL)) {
>> + req->flags |= CTL_FLAGS_UNUSED;
>> req++;
>> continue;
>> }
>> diff --git a/images/netdev.proto b/images/netdev.proto
>> index 7d7bb26..a2840eb 100644
>> --- a/images/netdev.proto
>> +++ b/images/netdev.proto
>> @@ -31,9 +31,11 @@ message net_device_entry {
>> optional bytes address = 7;
>>
>> repeated int32 conf4 = 8;
>> + repeated int32 mask_unused4 = 9;
>> }
>>
>> message netns_entry {
>> repeated int32 def_conf4 = 1;
>> repeated int32 all_conf4 = 2;
>> + repeated int32 mask_unused4 = 3;
>> }
>>
>
--
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.
More information about the CRIU
mailing list