[CRIU] [PATCH] unix: sysctl -- Preserve max_dgram_qlen value

[Pavel Tikhomirov]

Resending due to lost in-reply-to. Please see comments inline.

чт, 31 окт. 2019 г. в 12:20, Alexander Mikhalitsyn 
<alexander.mikhalitsyn at virtuozzo.com>:

>The /proc/sys/net/unix/max_dgram_qlen is a per-net variable and
>we already noticed that systemd inside a container may change its value
>(for example it sets it to 512 by now instead of kernel's default
>value 10), thus we need keep it inside image and restore then.

>Based-on-patch-by: Cyrill Gorcunov <gorcunov at gmail.com>
>Signed-off-by: Alexander Mikhalitsyn <alexander at mihalicyn.com>
>Signed-off-by: Alexander Mikhalitsyn <alexander.mikhalitsyn at virtuozzo.com>
>---
> criu/net.c                             | 102 ++++++++++++++++++++++++-
> images/netdev.proto                    |   1 +
> test/zdtm/static/Makefile              |   1 +
> test/zdtm/static/netns_sub_sysctl.c    |  62 +++++++++++++++
> test/zdtm/static/netns_sub_sysctl.desc |   5 ++
> 5 files changed, 170 insertions(+), 1 deletion(-)
> create mode 100644 test/zdtm/static/netns_sub_sysctl.c
> create mode 100644 test/zdtm/static/netns_sub_sysctl.des

Better split all test/zdtm/... staff to a separate patch. Look on the 
git/mail history, we always send tests as a separate letters(patches). 
In the same series, but separate from criu related letters(patches).

>diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
>index a38482f4..887bf4ac 100644
>--- a/test/zdtm/static/Makefile
>+++ b/test/zdtm/static/Makefile
>@@ -207,6 +207,7 @@ TST_NOFILE  :=                              \
>                pipe03                          \
>                netns_sub                       \
>                netns_sub_veth                  \
>+               netns_sub_sysctl        \
>                unlink_multiple_largefiles      \
>                config_inotify_irmap            \
>                thp_disable                     \
>diff --git a/test/zdtm/static/netns_sub_sysctl.c 
b/test/zdtm/static/netns_sub_sysctl.c
>new file mode 100644
>index 00000000..ea84cd30
>--- /dev/null
>+++ b/test/zdtm/static/netns_sub_sysctl.c
>@@ -0,0 +1,62 @@
>+#include <sched.h>
>+#include <errno.h>
>+#include <stdlib.h>
>+#include <string.h>
>+#include <unistd.h>
>+#include <sys/types.h>
>+#include <sys/wait.h>
>+#include <sys/un.h>
>+#include <sys/socket.h>
>+#include <sys/types.h>
>+#include <sys/stat.h>
>+#include <fcntl.h>
>+#include <stdbool.h>

Looks like not all of them are realy needed, e.g. why you need socket.h?

>+
>+#include "zdtmtst.h"
>+
>+const char *test_doc   = "Check dump and restore a 
net.unix.max_dgram_qlen sysctl parameter in subns";
>+
>+#define CONF_UNIX_PARAM "/proc/sys/net/unix/max_dgram_qlen"
>+
>+int main(int argc, char **argv)
>+{
>+       char cmd[128];
>+       FILE *fp;
>+       int ret, test_max_dgram_qlen = 321, max_dgram_qlen = 0;
>+       test_init(argc, argv);
>+
>+       if (unshare(CLONE_NEWNET)) {
>+               perror("unshare");
>+               return 1;
>+       }
>+
>+       sprintf(cmd, "echo %d > %s", test_max_dgram_qlen, 
CONF_UNIX_PARAM);
>+       if (system(cmd)) {

Do we really need system here, may be just open/write?

>+               pr_perror("Can't change %s", CONF_UNIX_PARAM);
>+               return -1;
>+       }
>+
>+       fp = fopen(CONF_UNIX_PARAM, "r+");
>+       if (fp == NULL) {
>+               pr_perror("fopen");
>+               return -1;
>+       }
>+
>+       ret = fscanf(fp, "%d", &max_dgram_qlen);
>+       if (ret != 1) {
>+               pr_perror("fscanf");
>+               fclose(fp);
>+               return -1;
>+       }
>+
>+       test_daemon();
>+       test_waitsig();
>+
>+       if (test_max_dgram_qlen != max_dgram_qlen) {
>+               fail();
>+               return 1;
>+       }

You are actually not checking that sysctl is persistent after c/r. 
test_waitsig() helper allows you to wait until zdtm makes c/r, so I 
think it is better to read max_dgram_qlen after these. I think these 
test should pass even before your criu-part of the patch.

>+
>+       pass();
>+       return 0;
>+}
>diff --git a/test/zdtm/static/netns_sub_sysctl.desc 
b/test/zdtm/static/netns_sub_sysctl.desc
>new file mode 100644
>index 00000000..e13a65b6
>--- /dev/null
>+++ b/test/zdtm/static/netns_sub_sysctl.desc
>@@ -0,0 +1,5 @@
>+{
>+    'deps': ['/bin/sh', '/sbin/ip|/bin/ip'],

If we don't call system() above we don't need these.

>+    'flavor': 'ns',
>+    'flags': 'suid'
>+}
>--
>2.17.1
>_______________________________________________
>CRIU mailing list
>CRIU at openvz.org
>https://lists.openvz.org/mailman/listinfo/criu

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.