* [PATCH] Tests for res_init
@ 2017-05-09 12:04 Florian Weimer
2017-05-18 18:33 ` Siddhesh Poyarekar
2017-05-18 20:05 ` Siddhesh Poyarekar
0 siblings, 2 replies; 20+ messages in thread
From: Florian Weimer @ 2017-05-09 12:04 UTC (permalink / raw)
To: GNU C Library
[-- Attachment #1: Type: text/plain, Size: 912 bytes --]
The attached patch adds various tests for res_init and other
initialization functions. Bugs 21474 (already fixed) and 21475 were
discovered while writing these tests. I did not bother to fix bug 21475
(search path truncation) because it will be gone with a larger
resolv.conf parser rewrite anyway.
The tests use various namespace kludges to be able to test unmodified
binaries, which have a hardcoded /etc/resolv.conf path. On current
Fedora, no root privileges are required, which is why didn't make this
an xtest.
The patch as written depends on the subprocess capture functionality in
the dynarray patch
https://sourceware.org/ml/libc-alpha/2017-04/msg00498.html
but I could cherry-pick the support functionality from there before
committing (or use a different mechanism to capture the deserialized
resolv.conf data, such as the new shared memory allocator).
Comments?
Thanks,
Florian
[-- Attachment #2: res_init.patch --]
[-- Type: text/x-patch, Size: 46414 bytes --]
resolv: Tests for various versions of res_init
2017-05-09 Florian Weimer <fweimer@redhat.com>
Tests for various versions of res_init.
* resolv/Makefile [build-shared] (tests): Add tst-resolv-res_init,
tst-resolv-res_init-thread.
(tst-resolv-res_init): Link against libdl, libresolv.
(tst-resolv-res_init-thread): Link against libdl, libresolv,
libpthread.
* resolv/tst-resolv-res_init.c: New file.
* resolv/tst-resolv-res_init-skeleton.c: Likewise.
* resolv/tst-resolv-res_init-thread.c: Likewise.
* support/Makefile (libsupport-routines): Add support-xstat,
support_can_chroot, support_capture_subprocess_check,
support_isolate_in_subprocess, support_shared_allocate,
support_write_file_string, xchroot, xmkdir, xopen.
* support/capture_subprocess.h (enum support_capture_allow): Define.
(support_capture_subprocess_check): Declare.
* support/namespace.h (support_can_chroot)
(support_isolate_in_subprocess): Declare.
* support/support-xstat.c: New file.
* support/support.h (support_shared_allocate, support_shared_free)
(support_write_file_string): Declare.
* support/support_can_chroot.c: New file.
* support/support_capture_subprocess_check.c: Likewise.
* support/support_isolate_in_subprocess.c: Likewise.
* support/support_shared_allocate.c: Likewise.
* support/support_write_file_string.c: Likewise.
* support/xchroot.c: Likwise.
* support/xmkdir.c: Likwise.
* support/xopen.c: Likwise.
* support/xunistd.h (xopen, xstat, xmkdir, xchroot): Declare.
diff --git a/resolv/Makefile b/resolv/Makefile
index d41fd46..bb9346f 100644
--- a/resolv/Makefile
+++ b/resolv/Makefile
@@ -56,6 +56,8 @@ tests += \
ifeq (yes,$(build-shared))
tests += \
tst-resolv-canonname \
+ tst-resolv-res_init \
+ tst-resolv-res_init-thread \
endif
@@ -136,6 +138,9 @@ $(objpfx)tst-res_use_inet6: $(objpfx)libresolv.so $(shared-thread-library)
$(objpfx)tst-resolv-basic: $(objpfx)libresolv.so $(shared-thread-library)
$(objpfx)tst-resolv-edns: $(objpfx)libresolv.so $(shared-thread-library)
$(objpfx)tst-resolv-network: $(objpfx)libresolv.so $(shared-thread-library)
+$(objpfx)tst-resolv-res_init: $(libdl) $(objpfx)libresolv.so
+$(objpfx)tst-resolv-res_init-thread: $(libdl) $(objpfx)libresolv.so \
+ $(shared-thread-library)
$(objpfx)tst-resolv-qtypes: $(objpfx)libresolv.so $(shared-thread-library)
$(objpfx)tst-resolv-search: $(objpfx)libresolv.so $(shared-thread-library)
$(objpfx)tst-resolv-canonname: \
diff --git a/resolv/tst-resolv-res_init-skeleton.c b/resolv/tst-resolv-res_init-skeleton.c
new file mode 100644
index 0000000..9824bd9
--- /dev/null
+++ b/resolv/tst-resolv-res_init-skeleton.c
@@ -0,0 +1,643 @@
+/* Test parsing of /etc/resolv.conf. Genric version.
+ Copyright (C) 2016-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* Before including this file, TEST_THREAD has to be defined to 0 or
+ 1, depending on whether the threading tests should be compiled
+ in. */
+
+#include <arpa/inet.h>
+#include <gnu/lib-names.h>
+#include <netdb.h>
+#include <resolv/resolv-internal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/namespace.h>
+#include <support/run_diff.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/test-driver.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
+
+#if TEST_THREAD
+# include <support/xthread.h>
+#endif
+
+/* This is the host name used to ensure predictable behavior of
+ res_init. */
+static const char *const test_hostname = "www.example.com";
+
+/* Path to the test root directory. */
+static char *path_chroot;
+
+/* Path to resolv.conf under path_chroot (outside the chroot). */
+static char *path_resolv_conf;
+
+static void
+prepare (int argc, char **argv)
+{
+ path_chroot = xasprintf ("%s/tst-resolv-res_init-XXXXXX", test_dir);
+ if (mkdtemp (path_chroot) == NULL)
+ FAIL_EXIT1 ("mkdtemp (\"%s\"): %m", path_chroot);
+ add_temp_file (path_chroot);
+
+ /* Create the /etc directory in the chroot environment. */
+ char *path_etc = xasprintf ("%s/etc", path_chroot);
+ xmkdir (path_etc, 0777);
+ add_temp_file (path_etc);
+
+ /* Create an empty resolv.conf file. */
+ path_resolv_conf = xasprintf ("%s/resolv.conf", path_etc);
+ add_temp_file (path_resolv_conf);
+ support_write_file_string (path_resolv_conf, "");
+
+ free (path_etc);
+
+ /* valgrind needs a temporary directory in the chroot. */
+ {
+ char *path_tmp = xasprintf ("%s/tmp", path_chroot);
+ xmkdir (path_tmp, 0777);
+ add_temp_file (path_tmp);
+ free (path_tmp);
+ }
+}
+
+/* Verify that the chroot environment has been set up. */
+static void
+check_chroot_working (void *closure)
+{
+ xchroot (path_chroot);
+ FILE *fp = xfopen (_PATH_RESCONF, "r");
+ xfclose (fp);
+
+ TEST_VERIFY_EXIT (res_init () == 0);
+ TEST_VERIFY (_res.options & RES_INIT);
+
+ char buf[100];
+ if (gethostname (buf, sizeof (buf)) < 0)
+ FAIL_EXIT1 ("gethostname: %m");
+ if (strcmp (buf, test_hostname) != 0)
+ FAIL_EXIT1 ("unexpected host name: %s", buf);
+}
+
+/* If FLAG is set in *OPTIONS, write NAME to FP, and clear it in
+ *OPTIONS. */
+static void
+print_option_flag (FILE *fp, int *options, int flag, const char *name)
+{
+ if (*options & flag)
+ {
+ fprintf (fp, " %s", name);
+ *options &= ~flag;
+ }
+}
+
+/* Write a decoded version of the resolver configuration *RESP to the
+ stream FP. */
+static void
+print_resp (FILE *fp, res_state resp)
+{
+ /* The options directive. */
+ {
+ /* RES_INIT is used internally for tracking initialization. */
+ TEST_VERIFY (resp->options & RES_INIT);
+ /* Also mask out other default flags which cannot be set through
+ the options directive. */
+ int options
+ = resp->options & ~(RES_INIT | RES_RECURSE | RES_DEFNAMES | RES_DNSRCH);
+ if (options != 0
+ || resp->ndots != 1
+ || resp->retrans != RES_TIMEOUT
+ || resp->retry != RES_DFLRETRY)
+ {
+ fputs ("options", fp);
+ if (resp->ndots != 1)
+ fprintf (fp, " ndots:%d", resp->ndots);
+ if (resp->retrans != RES_TIMEOUT)
+ fprintf (fp, " timeout:%d", resp->retrans);
+ if (resp->retry != RES_DFLRETRY)
+ fprintf (fp, " attempts:%d", resp->retry);
+ print_option_flag (fp, &options, RES_USEVC, "use-vc");
+ print_option_flag (fp, &options, DEPRECATED_RES_USE_INET6, "inet6");
+ print_option_flag (fp, &options, RES_ROTATE, "rotate");
+ print_option_flag (fp, &options, RES_USE_EDNS0, "edns0");
+ print_option_flag (fp, &options, RES_SNGLKUP,
+ "single-request");
+ print_option_flag (fp, &options, RES_SNGLKUPREOP,
+ "single-request-reopen");
+ print_option_flag (fp, &options, RES_NOTLDQUERY, "no-tld-query");
+ fputc ('\n', fp);
+ if (options != 0)
+ fprintf (fp, "; error: unresolved option bits: 0x%x\n", options);
+ }
+ }
+
+ /* The search and domain directives. */
+ if (resp->dnsrch[0] != NULL)
+ {
+ fputs ("search", fp);
+ for (int i = 0; i < MAXDNSRCH && resp->dnsrch[i] != NULL; ++i)
+ {
+ fputc (' ', fp);
+ fputs (resp->dnsrch[i], fp);
+ }
+ fputc ('\n', fp);
+ }
+ else if (resp->defdname[0] != '\0')
+ fprintf (fp, "domain %s\n", resp->defdname);
+
+ /* The sortlist directive. */
+ if (resp->nsort > 0)
+ {
+ fputs ("sortlist", fp);
+ for (int i = 0; i < resp->nsort && i < MAXRESOLVSORT; ++i)
+ {
+ char net[20];
+ if (inet_ntop (AF_INET, &resp->sort_list[i].addr,
+ net, sizeof (net)) == NULL)
+ FAIL_EXIT1 ("inet_ntop: %m\n");
+ char mask[20];
+ if (inet_ntop (AF_INET, &resp->sort_list[i].mask,
+ mask, sizeof (mask)) == NULL)
+ FAIL_EXIT1 ("inet_ntop: %m\n");
+ fprintf (fp, " %s/%s", net, mask);
+ }
+ fputc ('\n', fp);
+ }
+
+ /* The nameserver directives. */
+ for (size_t i = 0; i < resp->nscount; ++i)
+ {
+ char host[NI_MAXHOST];
+ char service[NI_MAXSERV];
+
+ /* See get_nsaddr in res_send.c. */
+ void *addr;
+ size_t addrlen;
+ if (resp->nsaddr_list[i].sin_family == 0
+ && resp->_u._ext.nsaddrs[i] != NULL)
+ {
+ addr = resp->_u._ext.nsaddrs[i];
+ addrlen = sizeof (*resp->_u._ext.nsaddrs[i]);
+ }
+ else
+ {
+ addr = &resp->nsaddr_list[i];
+ addrlen = sizeof (resp->nsaddr_list[i]);
+ }
+
+ int ret = getnameinfo (addr, addrlen,
+ host, sizeof (host), service, sizeof (service),
+ NI_NUMERICHOST | NI_NUMERICSERV);
+ if (ret != 0)
+ {
+ if (ret == EAI_SYSTEM)
+ fprintf (fp, "; error: getnameinfo: %m\n");
+ else
+ fprintf (fp, "; error: getnameinfo: %s\n", gai_strerror (ret));
+ }
+ else
+ {
+ fprintf (fp, "nameserver %s\n", host);
+ if (strcmp (service, "53") != 0)
+ fprintf (fp, "; unrepresentable port number %s\n\n", service);
+ }
+ }
+
+ TEST_VERIFY (!ferror (fp));
+}
+
+/* Parameters of one test case. */
+struct test_case
+{
+ /* A short, descriptive name of the test. */
+ const char *name;
+
+ /* The contents of the /etc/resolv.conf file. */
+ const char *conf;
+
+ /* The expected output from print_resp. */
+ const char *expected;
+
+ /* Setting for the LOCALDOMAIN environment variable. NULL if the
+ variable is not to be set. */
+ const char *localdomain;
+
+ /* Setting for the RES_OPTIONS environment variable. NULL if the
+ variable is not to be set. */
+ const char *res_options;
+};
+
+enum test_init
+{
+ test_init,
+ test_ninit,
+ test_mkquery,
+ test_gethostbyname,
+ test_getaddrinfo,
+ test_init_method_last = test_getaddrinfo
+};
+
+/* Closure argument for run_res_init. */
+struct test_context
+{
+ enum test_init init;
+ const struct test_case *t;
+};
+
+static void
+setup_nss_dns_and_chroot (void)
+{
+ /* Load nss_dns outside of the chroot. */
+ if (dlopen (LIBNSS_DNS_SO, RTLD_LAZY) == NULL)
+ FAIL_EXIT1 ("could not load " LIBNSS_DNS_SO ": %s", dlerror ());
+ xchroot (path_chroot);
+ /* Force the use of nss_dns. */
+ __nss_configure_lookup ("hosts", "dns");
+}
+
+/* Run res_ninit or res_init in a subprocess and dump the parsed
+ resolver state to standard output. */
+static void
+run_res_init (void *closure)
+{
+ struct test_context *ctx = closure;
+ TEST_VERIFY (getenv ("LOCALDOMAIN") == NULL);
+ TEST_VERIFY (getenv ("RES_OPTIONS") == NULL);
+ if (ctx->t->localdomain != NULL)
+ setenv ("LOCALDOMAIN", ctx->t->localdomain, 1);
+ if (ctx->t->res_options != NULL)
+ setenv ("RES_OPTIONS", ctx->t->res_options, 1);
+
+ switch (ctx->init)
+ {
+ case test_init:
+ xchroot (path_chroot);
+ TEST_VERIFY (res_init () == 0);
+ print_resp (stdout, &_res);
+ return;
+
+ case test_ninit:
+ xchroot (path_chroot);
+ res_state resp = xmalloc (sizeof (*resp));
+ memset (resp, 0, sizeof (*resp));
+ TEST_VERIFY (res_ninit (resp) == 0);
+ print_resp (stdout, resp);
+ res_nclose (resp);
+ free (resp);
+ return;
+
+ case test_mkquery:
+ xchroot (path_chroot);
+ unsigned char buf[512];
+ TEST_VERIFY (res_mkquery (QUERY, "www.example",
+ C_IN, ns_t_a, NULL, 0,
+ NULL, buf, sizeof (buf)) > 0);
+ print_resp (stdout, &_res);
+ return;
+
+ case test_gethostbyname:
+ setup_nss_dns_and_chroot ();
+ /* Trigger implicit initialization of the _res structure. The
+ actual lookup result is immaterial. */
+ (void )gethostbyname ("www.example");
+ print_resp (stdout, &_res);
+ return;
+
+ case test_getaddrinfo:
+ setup_nss_dns_and_chroot ();
+ /* Trigger implicit initialization of the _res structure. The
+ actual lookup result is immaterial. */
+ struct addrinfo *ai;
+ (void) getaddrinfo ("www.example", NULL, NULL, &ai);
+ print_resp (stdout, &_res);
+ return;
+ }
+
+ FAIL_EXIT1 ("invalid init method %d", ctx->init);
+}
+
+#if TEST_THREAD
+/* Helper function which calls run_res_init from a thread. */
+static void *
+run_res_init_thread_func (void *closure)
+{
+ run_res_init (closure);
+ return NULL;
+}
+
+/* Variant of res_run_init which runs the function on a non-main
+ thread. */
+static void
+run_res_init_on_thread (void *closure)
+{
+ xpthread_join (xpthread_create (NULL, run_res_init_thread_func, closure));
+}
+#endif /* TEST_THREAD */
+
+struct test_case test_cases[] =
+ {
+ {.name = "empty file",
+ .conf = "",
+ .expected = "search example.com\n"
+ "nameserver 127.0.0.1\n"
+ },
+ {.name = "empty file with LOCALDOMAIN",
+ .conf = "",
+ .expected = "search example.net\n"
+ "nameserver 127.0.0.1\n",
+ .localdomain = "example.net",
+ },
+ {.name = "empty file with RES_OPTIONS",
+ .conf = "",
+ .expected = "options attempts:5 edns0\n"
+ "search example.com\n"
+ "nameserver 127.0.0.1\n",
+ .res_options = "edns0 attempts:5",
+ },
+ {.name = "empty file with RES_OPTIONS and LOCALDOMAIN",
+ .conf = "",
+ .expected = "options attempts:5 edns0\n"
+ "search example.org\n"
+ "nameserver 127.0.0.1\n",
+ .localdomain = "example.org",
+ .res_options = "edns0 attempts:5",
+ },
+ {.name = "basic",
+ .conf = "domain example.net\n"
+ "search corp.example.com example.com\n"
+ "nameserver 192.0.2.1\n",
+ .expected = "search corp.example.com example.com\n"
+ "nameserver 192.0.2.1\n"
+ },
+ {.name = "whitespace",
+ .conf = "# This test covers comment and whitespace processing "
+ " (trailing whitespace,\n"
+ "# missing newline at end of file).\n"
+ "\n"
+ "domain example.net\n"
+ ";search commented out\n"
+ "search corp.example.com\texample.com\n"
+ "#nameserver 192.0.2.3\n"
+ "nameserver 192.0.2.1 \n"
+ "nameserver 192.0.2.2", /* No \n at end of file. */
+ .expected = "search corp.example.com example.com\n"
+ "nameserver 192.0.2.1\n"
+ "nameserver 192.0.2.2\n"
+ },
+ {.name = "option values, multiple servers",
+ .conf = "options\tinet6\tndots:3 edns0\tattempts:5\ttimeout:19\n"
+ "domain example.net\n"
+ ";domain comment\n"
+ "search corp.example.com\texample.com\n"
+ "nameserver 192.0.2.1\n"
+ "nameserver ::1\n"
+ "nameserver 192.0.2.2\n",
+ .expected = "options ndots:3 timeout:19 attempts:5 inet6 edns0\n"
+ "search corp.example.com example.com\n"
+ "nameserver 192.0.2.1\n"
+ "nameserver ::1\n"
+ "nameserver 192.0.2.2\n"
+ },
+ {.name = "out-of-range option vales",
+ .conf = "options use-vc timeout:999 attempts:999 ndots:99\n"
+ "search example.com\n",
+ .expected = "options ndots:15 timeout:30 attempts:5 use-vc\n"
+ "search example.com\n"
+ "nameserver 127.0.0.1\n"
+ },
+ {.name = "repeated directives",
+ .conf = "options ndots:3 use-vc\n"
+ "options edns0 ndots:2\n"
+ "domain corp.example\n"
+ "search example.net corp.example.com example.com\n"
+ "search example.org\n"
+ "search\n",
+ .expected = "options ndots:2 use-vc edns0\n"
+ "search example.org\n"
+ "nameserver 127.0.0.1\n"
+ },
+ {.name = "many name servers, sortlist",
+ .conf = "options single-request\n"
+ "search example.org example.com example.net corp.example.com\n"
+ "sortlist 192.0.2.0/255.255.255.0\n"
+ "nameserver 192.0.2.1\n"
+ "nameserver 192.0.2.2\n"
+ "nameserver 192.0.2.3\n"
+ "nameserver 192.0.2.4\n"
+ "nameserver 192.0.2.5\n"
+ "nameserver 192.0.2.6\n"
+ "nameserver 192.0.2.7\n"
+ "nameserver 192.0.2.8\n",
+ .expected = "options single-request\n"
+ "search example.org example.com example.net corp.example.com\n"
+ "sortlist 192.0.2.0/255.255.255.0\n"
+ "nameserver 192.0.2.1\n"
+ "nameserver 192.0.2.2\n"
+ "nameserver 192.0.2.3\n"
+ },
+ {.name = "IPv4 and IPv6 nameservers",
+ .conf = "options single-request\n"
+ "search example.org example.com example.net corp.example.com"
+ " legacy.example.com\n"
+ "sortlist 192.0.2.0\n"
+ "nameserver 192.0.2.1\n"
+ "nameserver 2001:db8::2\n"
+ "nameserver 192.0.2.3\n"
+ "nameserver 2001:db8::4\n"
+ "nameserver 192.0.2.5\n"
+ "nameserver 2001:db8::6\n"
+ "nameserver 192.0.2.7\n"
+ "nameserver 2001:db8::8\n",
+ .expected = "options single-request\n"
+ "search example.org example.com example.net corp.example.com"
+ " legacy.example.com\n"
+ "sortlist 192.0.2.0/255.255.255.0\n"
+ "nameserver 192.0.2.1\n"
+ "nameserver 2001:db8::2\n"
+ "nameserver 192.0.2.3\n"
+ },
+ {.name = "many search entries",
+ .conf = "options single-request-reopen\n"
+ "search 1.example 2.example 3.example 4.example 5.example 6.example"
+ " 7.example\n"
+ "nameserver 192.0.2.1\n",
+ .expected = "options single-request-reopen\n"
+ "search 1.example 2.example 3.example 4.example 5.example 6.example\n"
+ "nameserver 192.0.2.1\n"
+ },
+ {.name = "overlong search entries",
+ .conf = "options dnssec\n"
+ "search aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+ ".bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+ ".example"
+ " ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"
+ ".ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
+ ".example"
+ " eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"
+ ".fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
+ ".example\n"
+ "nameserver 192.0.2.1\n",
+ .expected =
+ "search aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+ ".bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+ ".example"
+ /* This search entry is truncated. See bug 21475. */
+ " ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"
+ ".ddddddddddddddddddddddddddddddddddddddddddddddddddddddd\n"
+ "nameserver 192.0.2.1\n"
+ },
+ {.name = "many search entries via LOCALDOMAIN",
+ .conf = "options single-request-reopen\n"
+ "search 1.example 2.example 3.example 4.example 5.example 6.example"
+ " 7.example\n"
+ "nameserver 192.0.2.1\n",
+ .expected = "options single-request-reopen\n"
+ "search 1.example.net 2.example.net 3.example.net 4.example.net"
+ " 5.example.net 6.example.net\n"
+ "nameserver 192.0.2.1\n",
+ .localdomain = "1.example.net 2.example.net 3.example.net 4.example.net"
+ " 5.example.net 6.example.net 7.example.net\n"
+ },
+ {.name = "garbage after nameserver",
+ .conf = "nameserver 192.0.2.1 garbage\n"
+ "nameserver 192.0.2.2:5353\n"
+ "nameserver 192.0.2.3 5353\n",
+ .expected = "search example.com\n"
+ "nameserver 192.0.2.1\n"
+ "nameserver 192.0.2.3\n"
+ },
+ {.name = "RES_OPTIONS is cummulative",
+ .conf = "options timeout:7 ndots:2 use-vc\n"
+ "nameserver 192.0.2.1\n",
+ .expected = "options ndots:3 timeout:7 attempts:5 use-vc edns0\n"
+ "search example.com\n"
+ "nameserver 192.0.2.1\n",
+ .res_options = "attempts:5 ndots:3 edns0 ",
+ },
+ { NULL }
+ };
+
+/* Run the indicated test case. This function assumes that the chroot
+ contents has already been set up. */
+static void
+test_file_contents (const struct test_case *t)
+{
+#if TEST_THREAD
+ for (int do_thread = 0; do_thread < 2; ++do_thread)
+#endif
+ for (int init_method = 0; init_method <= test_init_method_last;
+ ++init_method)
+ {
+ if (test_verbose > 0)
+ printf ("info: testing init method %d\n", init_method);
+ struct test_context ctx = { .init = init_method, .t = t };
+ void (*func) (void *) = run_res_init;
+#if TEST_THREAD
+ if (do_thread)
+ func = run_res_init_on_thread;
+#endif
+ struct support_capture_subprocess proc
+ = support_capture_subprocess (func, &ctx);
+ if (strcmp (proc.out.buffer, t->expected) != 0)
+ {
+ support_record_failure ();
+ printf ("error: output mismatch for %s\n", t->name);
+ support_run_diff ("expected", t->expected,
+ "actual", proc.out.buffer);
+ }
+ support_capture_subprocess_check (&proc, t->name, 0,
+ sc_allow_stdout);
+ support_capture_subprocess_free (&proc);
+ }
+}
+
+static int
+do_test (void)
+{
+ support_become_root ();
+ support_enter_network_namespace ();
+ if (!support_in_uts_namespace () || !support_can_chroot ())
+ return EXIT_UNSUPPORTED;
+
+ /* We are in an UTS namespace, so we can set the host name without
+ altering the state of the entire system. */
+ if (sethostname (test_hostname, strlen (test_hostname)) != 0)
+ FAIL_EXIT1 ("sethostname: %m");
+
+ /* These environment variables affect resolv.conf parsing. */
+ unsetenv ("LOCALDOMAIN");
+ unsetenv ("RES_OPTIONS");
+
+ /* Ensure that the chroot setup worked. */
+ {
+ struct support_capture_subprocess proc
+ = support_capture_subprocess (check_chroot_working, NULL);
+ support_capture_subprocess_check (&proc, "chroot", 0, sc_allow_none);
+ support_capture_subprocess_free (&proc);
+ }
+
+ for (size_t i = 0; test_cases[i].name != NULL; ++i)
+ {
+ if (test_verbose > 0)
+ printf ("info: running test: %s\n", test_cases[i].name);
+ TEST_VERIFY (test_cases[i].conf != NULL);
+ TEST_VERIFY (test_cases[i].expected != NULL);
+
+ support_write_file_string (path_resolv_conf, test_cases[i].conf);
+
+ test_file_contents (&test_cases[i]);
+
+ /* The expected output from the empty file test is used for
+ further tests. */
+ if (test_cases[i].conf[0] == '\0')
+ {
+ if (test_verbose > 0)
+ printf ("info: special test: missing file\n");
+ TEST_VERIFY (unlink (path_resolv_conf) == 0);
+ test_file_contents (&test_cases[i]);
+
+ if (test_verbose > 0)
+ printf ("info: special test: dangling symbolic link\n");
+ TEST_VERIFY (symlink ("does-not-exist", path_resolv_conf) == 0);
+ test_file_contents (&test_cases[i]);
+ TEST_VERIFY (unlink (path_resolv_conf) == 0);
+
+ if (test_verbose > 0)
+ printf ("info: special test: unreadable file\n");
+ support_write_file_string (path_resolv_conf, "");
+ TEST_VERIFY (chmod (path_resolv_conf, 0) == 0);
+ test_file_contents (&test_cases[i]);
+
+ /* Restore the empty file. */
+ TEST_VERIFY (unlink (path_resolv_conf) == 0);
+ support_write_file_string (path_resolv_conf, "");
+ }
+ }
+
+ free (path_chroot);
+ path_chroot = NULL;
+ free (path_resolv_conf);
+ path_resolv_conf = NULL;
+ return 0;
+}
+
+#define PREPARE prepare
+#include <support/test-driver.c>
diff --git a/resolv/tst-resolv-res_init-thread.c b/resolv/tst-resolv-res_init-thread.c
new file mode 100644
index 0000000..1dfc771
--- /dev/null
+++ b/resolv/tst-resolv-res_init-thread.c
@@ -0,0 +1,20 @@
+/* Test parsing of /etc/resolv.conf, threading version.
+ Copyright (C) 2016-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#define TEST_THREAD 1
+#include "tst-resolv-res_init-skeleton.c"
diff --git a/resolv/tst-resolv-res_init.c b/resolv/tst-resolv-res_init.c
new file mode 100644
index 0000000..b4b2563
--- /dev/null
+++ b/resolv/tst-resolv-res_init.c
@@ -0,0 +1,20 @@
+/* Test parsing of /etc/resolv.conf, non-threading version.
+ Copyright (C) 2016-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#define TEST_THREAD 0
+#include "tst-resolv-res_init-skeleton.c"
diff --git a/support/Makefile b/support/Makefile
index 3ce73a6..20b0343 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -35,8 +35,11 @@ libsupport-routines = \
oom_error \
resolv_test \
set_fortify_handler \
+ support-xstat \
support_become_root \
+ support_can_chroot \
support_capture_subprocess \
+ support_capture_subprocess_check \
support_enter_network_namespace \
support_format_address_family \
support_format_addrinfo \
@@ -44,8 +47,11 @@ libsupport-routines = \
support_format_herrno \
support_format_hostent \
support_format_netent \
+ support_isolate_in_subprocess \
support_record_failure \
support_run_diff \
+ support_shared_allocate \
+ support_write_file_string \
support_test_main \
support_test_verify_impl \
temp_file \
@@ -55,6 +61,7 @@ libsupport-routines = \
xasprintf \
xbind \
xcalloc \
+ xchroot \
xclose \
xconnect \
xdup2 \
@@ -65,8 +72,10 @@ libsupport-routines = \
xlisten \
xmalloc \
xmemstream \
+ xmkdir \
xmmap \
xmunmap \
+ xopen \
xpipe \
xpoll \
xpthread_attr_destroy \
diff --git a/support/capture_subprocess.h b/support/capture_subprocess.h
index 3265d63..510c15f 100644
--- a/support/capture_subprocess.h
+++ b/support/capture_subprocess.h
@@ -39,4 +39,23 @@ struct support_capture_subprocess support_capture_subprocess
support_capture_subprocess. */
void support_capture_subprocess_free (struct support_capture_subprocess *);
+enum support_capture_allow
+{
+ /* No output is allowed. */
+ sc_allow_none = 0x01,
+ /* Output to stdout is permitted. */
+ sc_allow_stdout = 0x02,
+ /* Output to standard error is permitted. */
+ sc_allow_stderr = 0x04,
+};
+
+/* Check that the subprocess exited with STATUS and that only the
+ allowed outputs happened. ALLOWED is a combination of
+ support_capture_allow flags. Report errors under the CONTEXT
+ message. */
+void support_capture_subprocess_check (struct support_capture_subprocess *,
+ const char *context, int status,
+ int allowed)
+ __attribute__ ((nonnull (1, 2)));
+
#endif /* SUPPORT_CAPTURE_SUBPROCESS_H */
diff --git a/support/namespace.h b/support/namespace.h
index 6bc82d6..e1ccaa1 100644
--- a/support/namespace.h
+++ b/support/namespace.h
@@ -35,6 +35,13 @@ __BEGIN_DECLS
single-threaded processes. */
bool support_become_root (void);
+/* Return true if this process can perform a chroot operation. In
+ general, this is only possible if support_become_root has been
+ called. Note that the actual test is performed in a subprocess,
+ after fork, so that the file system root of the original process is
+ not changed. */
+bool support_can_chroot (void);
+
/* Enter a network namespace (and a UTS namespace if possible) and
configure the loopback interface. Return true if a network
namespace could be created. Print diagnostics to standard output.
@@ -48,6 +55,11 @@ bool support_enter_network_namespace (void);
UTS namespace. */
bool support_in_uts_namespace (void);
+/* Invoke CALLBACK (CLOSURE) in a subprocess created using fork.
+ Terminate the calling process if the subprocess exits with a
+ non-zero exit status. */
+void support_isolate_in_subprocess (void (*callback) (void *), void *closure);
+
__END_DECLS
#endif
diff --git a/support/support-xstat.c b/support/support-xstat.c
new file mode 100644
index 0000000..0b02a1f
--- /dev/null
+++ b/support/support-xstat.c
@@ -0,0 +1,30 @@
+/* stat64 with error checking.
+ Copyright (C) 2016-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* NB: Non-standard file name to avoid sysdeps override for xstat. */
+
+#include <support/check.h>
+#include <support/xunistd.h>
+#include <sys/stat.h>
+
+void
+xstat (const char *path, struct stat64 *result)
+{
+ if (stat64 (path, result) != 0)
+ FAIL_EXIT1 ("stat64 (\"%s\"): %m", path);
+}
diff --git a/support/support.h b/support/support.h
index 7292e2a..4b5f04c 100644
--- a/support/support.h
+++ b/support/support.h
@@ -44,6 +44,21 @@ void set_fortify_handler (void (*handler) (int sig));
void oom_error (const char *function, size_t size)
__attribute__ ((nonnull (1)));
+/* Return a pointer to a memory region of SIZE bytes. The memory is
+ initialized to zero and will be shared with subprocesses (across
+ fork). The returned pointer must be freed using
+ support_shared_free; it is not compatible with the malloc
+ functions. */
+void *support_shared_allocate (size_t size);
+
+/* Deallocate a pointer returned by support_shared_allocate. */
+void support_shared_free (void *);
+
+/* Write CONTENTS to the file PATH. Create or truncate the file as
+ needed. The file mode is 0666 masked by the umask. Terminate the
+ process on error. */
+void support_write_file_string (const char *path, const char *contents);
+
/* Error-checking wrapper functions which terminate the process on
error. */
diff --git a/support/support_can_chroot.c b/support/support_can_chroot.c
new file mode 100644
index 0000000..0dfd2de
--- /dev/null
+++ b/support/support_can_chroot.c
@@ -0,0 +1,65 @@
+/* Return true if the process can perform a chroot operation.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/namespace.h>
+#include <support/support.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <xunistd.h>
+
+static void
+callback (void *closure)
+{
+ int *result = closure;
+ struct stat64 before;
+ xstat ("/dev", &before);
+ if (chroot ("/dev") != 0)
+ {
+ *result = errno;
+ return;
+ }
+ struct stat64 after;
+ xstat ("/", &after);
+ TEST_VERIFY (before.st_dev == after.st_dev);
+ TEST_VERIFY (before.st_ino == after.st_ino);
+ *result = 0;
+}
+
+bool
+support_can_chroot (void)
+{
+ int *result = support_shared_allocate (sizeof (*result));
+ *result = 0;
+ support_isolate_in_subprocess (callback, result);
+ bool ok = *result == 0;
+ if (!ok)
+ {
+ static bool already_warned;
+ if (!already_warned)
+ {
+ already_warned = true;
+ errno = *result;
+ printf ("warning: this process does not support chroot: %m\n");
+ }
+ }
+ support_shared_free (result);
+ return ok;
+}
diff --git a/support/support_capture_subprocess_check.c b/support/support_capture_subprocess_check.c
new file mode 100644
index 0000000..708c89f
--- /dev/null
+++ b/support/support_capture_subprocess_check.c
@@ -0,0 +1,67 @@
+/* Verify capture output from a subprocess.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+
+static void
+print_context (const char *context, bool *failed)
+{
+ if (*failed)
+ /* Do not duplicate message. */
+ return;
+ support_record_failure ();
+ printf ("error: subprocess failed: %s\n", context);
+}
+
+void
+support_capture_subprocess_check (struct support_capture_subprocess *proc,
+ const char *context, int status,
+ int allowed)
+{
+ TEST_VERIFY ((allowed & sc_allow_none)
+ || (allowed & sc_allow_stdout)
+ || (allowed & sc_allow_stderr));
+ TEST_VERIFY (!((allowed & sc_allow_none)
+ && ((allowed & sc_allow_stdout)
+ || (allowed & sc_allow_stderr))));
+
+ bool failed = false;
+ if (proc->status != status)
+ {
+ print_context (context, &failed);
+ printf ("error: expected exit status: %d\n", status);
+ printf ("error: actual exit status: %d\n", status);
+ }
+ if (!(allowed & sc_allow_stdout) && proc->out.length != 0)
+ {
+ print_context (context, &failed);
+ printf ("error: unexpected output from subprocess\n");
+ fwrite (proc->out.buffer, proc->out.length, 1, stdout);
+ puts ("\n");
+ }
+ if (!(allowed & sc_allow_stderr) && proc->err.length != 0)
+ {
+ print_context (context, &failed);
+ printf ("error: unexpected error output from subprocess\n");
+ fwrite (proc->err.buffer, proc->err.length, 1, stdout);
+ puts ("\n");
+ }
+}
diff --git a/support/support_isolate_in_subprocess.c b/support/support_isolate_in_subprocess.c
new file mode 100644
index 0000000..cf48614
--- /dev/null
+++ b/support/support_isolate_in_subprocess.c
@@ -0,0 +1,38 @@
+/* Run a function in a subprocess.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <support/check.h>
+#include <support/xunistd.h>
+
+void
+support_isolate_in_subprocess (void (*callback) (void *), void *closure)
+{
+ pid_t pid = xfork ();
+ if (pid == 0)
+ {
+ /* Child process. */
+ callback (closure);
+ _exit (0);
+ }
+
+ /* Parent process. */
+ int status;
+ xwaitpid (pid, &status, 0);
+ if (status != 0)
+ FAIL_EXIT1 ("child process exited with status %d", status);
+}
diff --git a/support/support_shared_allocate.c b/support/support_shared_allocate.c
new file mode 100644
index 0000000..61d088e
--- /dev/null
+++ b/support/support_shared_allocate.c
@@ -0,0 +1,57 @@
+/* Allocate a memory region shared across processes.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <stddef.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <sys/mman.h>
+
+/* Header for the allocation. It contains the size of the allocation
+ for subsequent unmapping. */
+struct header
+{
+ size_t total_size;
+ char data[] __attribute__ ((aligned (__alignof__ (max_align_t))));
+};
+
+void *
+support_shared_allocate (size_t size)
+{
+ size_t total_size = size + offsetof (struct header, data);
+ if (total_size < size)
+ {
+ errno = ENOMEM;
+ oom_error (__func__, size);
+ return NULL;
+ }
+ else
+ {
+ struct header *result = xmmap (NULL, total_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_SHARED, -1);
+ result->total_size = total_size;
+ return &result->data;
+ }
+}
+
+void
+support_shared_free (void *data)
+{
+ struct header *header = data - offsetof (struct header, data);
+ xmunmap (header, header->total_size);
+}
diff --git a/support/support_write_file_string.c b/support/support_write_file_string.c
new file mode 100644
index 0000000..48e8959
--- /dev/null
+++ b/support/support_write_file_string.c
@@ -0,0 +1,39 @@
+/* Write a string to a file.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <fcntl.h>
+#include <string.h>
+#include <support/check.h>
+#include <xunistd.h>
+
+void
+support_write_file_string (const char *path, const char *contents)
+{
+ int fd = xopen (path, O_CREAT | O_TRUNC | O_WRONLY, 0666);
+ const char *end = contents + strlen (contents);
+ for (const char *p = contents; p < end; )
+ {
+ ssize_t ret = write (fd, p, end - p);
+ if (ret < 0)
+ FAIL_EXIT1 ("cannot write to \"%s\": %m", path);
+ if (ret == 0)
+ FAIL_EXIT1 ("zero-length write to \"%s\"", path);
+ p += ret;
+ }
+ xclose (fd);
+}
diff --git a/support/xchroot.c b/support/xchroot.c
new file mode 100644
index 0000000..af99aa2
--- /dev/null
+++ b/support/xchroot.c
@@ -0,0 +1,28 @@
+/* chroot with error checking.
+ Copyright (C) 2016-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <support/check.h>
+#include <support/xunistd.h>
+#include <sys/stat.h>
+
+void
+xchroot (const char *path)
+{
+ if (chroot (path) != 0)
+ FAIL_EXIT1 ("chroot (\"%s\"): %m", path);
+}
diff --git a/support/xmkdir.c b/support/xmkdir.c
new file mode 100644
index 0000000..96aa0e1
--- /dev/null
+++ b/support/xmkdir.c
@@ -0,0 +1,28 @@
+/* mkdir with error checking.
+ Copyright (C) 2016-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <support/check.h>
+#include <support/xunistd.h>
+#include <sys/stat.h>
+
+void
+xmkdir (const char *path, mode_t mode)
+{
+ if (mkdir (path, mode) != 0)
+ FAIL_EXIT1 ("mkdir (\"%s\", 0%o): %m", path, mode);
+}
diff --git a/support/xopen.c b/support/xopen.c
new file mode 100644
index 0000000..91aa99d
--- /dev/null
+++ b/support/xopen.c
@@ -0,0 +1,31 @@
+/* open64 with error checking.
+ Copyright (C) 2016-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <support/check.h>
+#include <support/xunistd.h>
+#include <fcntl.h>
+
+int
+xopen (const char *path, int flags, mode_t mode)
+{
+ int ret = open64 (path, flags, mode);
+ if (ret < 0)
+ FAIL_EXIT1 ("open64 (\"%s\", 0x%x, 0%o): %m", path, flags, mode);
+ return ret;
+}
+
diff --git a/support/xunistd.h b/support/xunistd.h
index 7c14bda..151d743 100644
--- a/support/xunistd.h
+++ b/support/xunistd.h
@@ -22,15 +22,22 @@
#ifndef SUPPORT_XUNISTD_H
#define SUPPORT_XUNISTD_H
-#include <unistd.h>
#include <sys/cdefs.h>
+#include <sys/types.h>
+#include <unistd.h>
__BEGIN_DECLS
+struct stat64;
+
pid_t xfork (void);
pid_t xwaitpid (pid_t, int *status, int flags);
void xpipe (int[2]);
void xdup2 (int, int);
+int xopen (const char *path, int flags, mode_t);
+void xstat (const char *path, struct stat64 *);
+void xmkdir (const char *path, mode_t);
+void xchroot (const char *path);
/* Close the file descriptor. Ignore EINTR errors, but terminate the
process on other errors. */
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-05-09 12:04 [PATCH] Tests for res_init Florian Weimer
@ 2017-05-18 18:33 ` Siddhesh Poyarekar
2017-05-18 18:37 ` Florian Weimer
2017-05-18 20:05 ` Siddhesh Poyarekar
1 sibling, 1 reply; 20+ messages in thread
From: Siddhesh Poyarekar @ 2017-05-18 18:33 UTC (permalink / raw)
To: Florian Weimer, GNU C Library
On Tuesday 09 May 2017 05:34 PM, Florian Weimer wrote:
> The attached patch adds various tests for res_init and other
> initialization functions. Bugs 21474 (already fixed) and 21475 were
> discovered while writing these tests. I did not bother to fix bug 21475
> (search path truncation) because it will be gone with a larger
> resolv.conf parser rewrite anyway.
Could you get the test for 21475 out of this patch? You could add that
in with the parser rewrite later. That way this patch won't result in
failures in the testsuite.
Siddhesh
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-05-18 18:33 ` Siddhesh Poyarekar
@ 2017-05-18 18:37 ` Florian Weimer
2017-05-18 19:09 ` Siddhesh Poyarekar
0 siblings, 1 reply; 20+ messages in thread
From: Florian Weimer @ 2017-05-18 18:37 UTC (permalink / raw)
To: Siddhesh Poyarekar, GNU C Library
On 05/18/2017 08:33 PM, Siddhesh Poyarekar wrote:
> On Tuesday 09 May 2017 05:34 PM, Florian Weimer wrote:
>> The attached patch adds various tests for res_init and other
>> initialization functions. Bugs 21474 (already fixed) and 21475 were
>> discovered while writing these tests. I did not bother to fix bug 21475
>> (search path truncation) because it will be gone with a larger
>> resolv.conf parser rewrite anyway.
>
> Could you get the test for 21475 out of this patch? You could add that
> in with the parser rewrite later. That way this patch won't result in
> failures in the testsuite.
I can make this clearer in the comment. The test currently contains the
wrong expected output, as a remainder to update the bug when this is fixed.
Thanks,
Florian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-05-18 18:37 ` Florian Weimer
@ 2017-05-18 19:09 ` Siddhesh Poyarekar
2017-05-18 19:15 ` Florian Weimer
0 siblings, 1 reply; 20+ messages in thread
From: Siddhesh Poyarekar @ 2017-05-18 19:09 UTC (permalink / raw)
To: Florian Weimer, GNU C Library
On Friday 19 May 2017 12:07 AM, Florian Weimer wrote:
> I can make this clearer in the comment. The test currently contains the
> wrong expected output, as a remainder to update the bug when this is fixed.
Oh no, that would be worse I think, much better to not have it in there
and add it with the fix than have it in there with the wrong expected
output.
Siddhesh
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-05-18 19:09 ` Siddhesh Poyarekar
@ 2017-05-18 19:15 ` Florian Weimer
0 siblings, 0 replies; 20+ messages in thread
From: Florian Weimer @ 2017-05-18 19:15 UTC (permalink / raw)
To: Siddhesh Poyarekar, GNU C Library
On 05/18/2017 09:09 PM, Siddhesh Poyarekar wrote:
> On Friday 19 May 2017 12:07 AM, Florian Weimer wrote:
>> I can make this clearer in the comment. The test currently contains the
>> wrong expected output, as a remainder to update the bug when this is fixed.
>
> Oh no, that would be worse I think, much better to not have it in there
> and add it with the fix than have it in there with the wrong expected
> output.
I don't have a strong opinion on this matter. I'll remove the test.
Do you have any comments on the rest of the patch?
Thanks,
Florian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-05-09 12:04 [PATCH] Tests for res_init Florian Weimer
2017-05-18 18:33 ` Siddhesh Poyarekar
@ 2017-05-18 20:05 ` Siddhesh Poyarekar
2017-06-02 13:49 ` Florian Weimer
1 sibling, 1 reply; 20+ messages in thread
From: Siddhesh Poyarekar @ 2017-05-18 20:05 UTC (permalink / raw)
To: Florian Weimer, GNU C Library
On Tuesday 09 May 2017 05:34 PM, Florian Weimer wrote:
> The attached patch adds various tests for res_init and other
> initialization functions. Bugs 21474 (already fixed) and 21475 were
> discovered while writing these tests. I did not bother to fix bug 21475
> (search path truncation) because it will be gone with a larger
> resolv.conf parser rewrite anyway.
>
> The tests use various namespace kludges to be able to test unmodified
> binaries, which have a hardcoded /etc/resolv.conf path. On current
> Fedora, no root privileges are required, which is why didn't make this
> an xtest.
>
> The patch as written depends on the subprocess capture functionality in
> the dynarray patch
>
> https://sourceware.org/ml/libc-alpha/2017-04/msg00498.html
>
> but I could cherry-pick the support functionality from there before
> committing (or use a different mechanism to capture the deserialized
> resolv.conf data, such as the new shared memory allocator).
>
> Comments?
Looks fine to me with a few nits inline, mostly incorrect copyright
year. It would be really nice if such large patches were split into
smaller contained patches. In this case for example, the support code
could have gone into a separate patch because it makes for much easier
review.
Thanks,
Siddhesh
>
> Thanks,
> Florian
>
> res_init.patch
>
>
> resolv: Tests for various versions of res_init
>
> 2017-05-09 Florian Weimer <fweimer@redhat.com>
>
> Tests for various versions of res_init.
> * resolv/Makefile [build-shared] (tests): Add tst-resolv-res_init,
> tst-resolv-res_init-thread.
> (tst-resolv-res_init): Link against libdl, libresolv.
> (tst-resolv-res_init-thread): Link against libdl, libresolv,
> libpthread.
> * resolv/tst-resolv-res_init.c: New file.
> * resolv/tst-resolv-res_init-skeleton.c: Likewise.
> * resolv/tst-resolv-res_init-thread.c: Likewise.
> * support/Makefile (libsupport-routines): Add support-xstat,
> support_can_chroot, support_capture_subprocess_check,
> support_isolate_in_subprocess, support_shared_allocate,
> support_write_file_string, xchroot, xmkdir, xopen.
> * support/capture_subprocess.h (enum support_capture_allow): Define.
> (support_capture_subprocess_check): Declare.
> * support/namespace.h (support_can_chroot)
> (support_isolate_in_subprocess): Declare.
> * support/support-xstat.c: New file.
> * support/support.h (support_shared_allocate, support_shared_free)
> (support_write_file_string): Declare.
> * support/support_can_chroot.c: New file.
> * support/support_capture_subprocess_check.c: Likewise.
> * support/support_isolate_in_subprocess.c: Likewise.
> * support/support_shared_allocate.c: Likewise.
> * support/support_write_file_string.c: Likewise.
> * support/xchroot.c: Likwise.
> * support/xmkdir.c: Likwise.
> * support/xopen.c: Likwise.
> * support/xunistd.h (xopen, xstat, xmkdir, xchroot): Declare.
>
> diff --git a/resolv/Makefile b/resolv/Makefile
> index d41fd46..bb9346f 100644
> --- a/resolv/Makefile
> +++ b/resolv/Makefile
> @@ -56,6 +56,8 @@ tests += \
> ifeq (yes,$(build-shared))
> tests += \
> tst-resolv-canonname \
> + tst-resolv-res_init \
> + tst-resolv-res_init-thread \
>
> endif
>
> @@ -136,6 +138,9 @@ $(objpfx)tst-res_use_inet6: $(objpfx)libresolv.so $(shared-thread-library)
> $(objpfx)tst-resolv-basic: $(objpfx)libresolv.so $(shared-thread-library)
> $(objpfx)tst-resolv-edns: $(objpfx)libresolv.so $(shared-thread-library)
> $(objpfx)tst-resolv-network: $(objpfx)libresolv.so $(shared-thread-library)
> +$(objpfx)tst-resolv-res_init: $(libdl) $(objpfx)libresolv.so
> +$(objpfx)tst-resolv-res_init-thread: $(libdl) $(objpfx)libresolv.so \
> + $(shared-thread-library)
> $(objpfx)tst-resolv-qtypes: $(objpfx)libresolv.so $(shared-thread-library)
> $(objpfx)tst-resolv-search: $(objpfx)libresolv.so $(shared-thread-library)
> $(objpfx)tst-resolv-canonname: \
> diff --git a/resolv/tst-resolv-res_init-skeleton.c b/resolv/tst-resolv-res_init-skeleton.c
> new file mode 100644
> index 0000000..9824bd9
> --- /dev/null
> +++ b/resolv/tst-resolv-res_init-skeleton.c
> @@ -0,0 +1,643 @@
> +/* Test parsing of /etc/resolv.conf. Genric version.
> + Copyright (C) 2016-2017 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +/* Before including this file, TEST_THREAD has to be defined to 0 or
> + 1, depending on whether the threading tests should be compiled
> + in. */
> +
> +#include <arpa/inet.h>
> +#include <gnu/lib-names.h>
> +#include <netdb.h>
> +#include <resolv/resolv-internal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <support/capture_subprocess.h>
> +#include <support/check.h>
> +#include <support/namespace.h>
> +#include <support/run_diff.h>
> +#include <support/support.h>
> +#include <support/temp_file.h>
> +#include <support/test-driver.h>
> +#include <support/xstdio.h>
> +#include <support/xunistd.h>
> +
> +#if TEST_THREAD
> +# include <support/xthread.h>
> +#endif
> +
> +/* This is the host name used to ensure predictable behavior of
> + res_init. */
> +static const char *const test_hostname = "www.example.com";
> +
> +/* Path to the test root directory. */
> +static char *path_chroot;
> +
> +/* Path to resolv.conf under path_chroot (outside the chroot). */
> +static char *path_resolv_conf;
> +
> +static void
> +prepare (int argc, char **argv)
> +{
> + path_chroot = xasprintf ("%s/tst-resolv-res_init-XXXXXX", test_dir);
> + if (mkdtemp (path_chroot) == NULL)
> + FAIL_EXIT1 ("mkdtemp (\"%s\"): %m", path_chroot);
> + add_temp_file (path_chroot);
> +
> + /* Create the /etc directory in the chroot environment. */
> + char *path_etc = xasprintf ("%s/etc", path_chroot);
> + xmkdir (path_etc, 0777);
> + add_temp_file (path_etc);
> +
> + /* Create an empty resolv.conf file. */
> + path_resolv_conf = xasprintf ("%s/resolv.conf", path_etc);
> + add_temp_file (path_resolv_conf);
> + support_write_file_string (path_resolv_conf, "");
> +
> + free (path_etc);
> +
> + /* valgrind needs a temporary directory in the chroot. */
> + {
> + char *path_tmp = xasprintf ("%s/tmp", path_chroot);
> + xmkdir (path_tmp, 0777);
> + add_temp_file (path_tmp);
> + free (path_tmp);
> + }
> +}
> +
> +/* Verify that the chroot environment has been set up. */
> +static void
> +check_chroot_working (void *closure)
> +{
> + xchroot (path_chroot);
> + FILE *fp = xfopen (_PATH_RESCONF, "r");
> + xfclose (fp);
> +
> + TEST_VERIFY_EXIT (res_init () == 0);
> + TEST_VERIFY (_res.options & RES_INIT);
> +
> + char buf[100];
> + if (gethostname (buf, sizeof (buf)) < 0)
> + FAIL_EXIT1 ("gethostname: %m");
> + if (strcmp (buf, test_hostname) != 0)
> + FAIL_EXIT1 ("unexpected host name: %s", buf);
> +}
> +
> +/* If FLAG is set in *OPTIONS, write NAME to FP, and clear it in
> + *OPTIONS. */
> +static void
> +print_option_flag (FILE *fp, int *options, int flag, const char *name)
> +{
> + if (*options & flag)
> + {
> + fprintf (fp, " %s", name);
> + *options &= ~flag;
> + }
> +}
> +
> +/* Write a decoded version of the resolver configuration *RESP to the
> + stream FP. */
> +static void
> +print_resp (FILE *fp, res_state resp)
> +{
> + /* The options directive. */
> + {
> + /* RES_INIT is used internally for tracking initialization. */
> + TEST_VERIFY (resp->options & RES_INIT);
> + /* Also mask out other default flags which cannot be set through
> + the options directive. */
> + int options
> + = resp->options & ~(RES_INIT | RES_RECURSE | RES_DEFNAMES | RES_DNSRCH);
> + if (options != 0
> + || resp->ndots != 1
> + || resp->retrans != RES_TIMEOUT
> + || resp->retry != RES_DFLRETRY)
> + {
> + fputs ("options", fp);
> + if (resp->ndots != 1)
> + fprintf (fp, " ndots:%d", resp->ndots);
> + if (resp->retrans != RES_TIMEOUT)
> + fprintf (fp, " timeout:%d", resp->retrans);
> + if (resp->retry != RES_DFLRETRY)
> + fprintf (fp, " attempts:%d", resp->retry);
> + print_option_flag (fp, &options, RES_USEVC, "use-vc");
> + print_option_flag (fp, &options, DEPRECATED_RES_USE_INET6, "inet6");
> + print_option_flag (fp, &options, RES_ROTATE, "rotate");
> + print_option_flag (fp, &options, RES_USE_EDNS0, "edns0");
> + print_option_flag (fp, &options, RES_SNGLKUP,
> + "single-request");
> + print_option_flag (fp, &options, RES_SNGLKUPREOP,
> + "single-request-reopen");
> + print_option_flag (fp, &options, RES_NOTLDQUERY, "no-tld-query");
> + fputc ('\n', fp);
> + if (options != 0)
> + fprintf (fp, "; error: unresolved option bits: 0x%x\n", options);
> + }
> + }
> +
> + /* The search and domain directives. */
> + if (resp->dnsrch[0] != NULL)
> + {
> + fputs ("search", fp);
> + for (int i = 0; i < MAXDNSRCH && resp->dnsrch[i] != NULL; ++i)
> + {
> + fputc (' ', fp);
> + fputs (resp->dnsrch[i], fp);
> + }
> + fputc ('\n', fp);
> + }
> + else if (resp->defdname[0] != '\0')
> + fprintf (fp, "domain %s\n", resp->defdname);
> +
> + /* The sortlist directive. */
> + if (resp->nsort > 0)
> + {
> + fputs ("sortlist", fp);
> + for (int i = 0; i < resp->nsort && i < MAXRESOLVSORT; ++i)
> + {
> + char net[20];
> + if (inet_ntop (AF_INET, &resp->sort_list[i].addr,
> + net, sizeof (net)) == NULL)
> + FAIL_EXIT1 ("inet_ntop: %m\n");
> + char mask[20];
> + if (inet_ntop (AF_INET, &resp->sort_list[i].mask,
> + mask, sizeof (mask)) == NULL)
> + FAIL_EXIT1 ("inet_ntop: %m\n");
> + fprintf (fp, " %s/%s", net, mask);
> + }
> + fputc ('\n', fp);
> + }
> +
> + /* The nameserver directives. */
> + for (size_t i = 0; i < resp->nscount; ++i)
> + {
> + char host[NI_MAXHOST];
> + char service[NI_MAXSERV];
> +
> + /* See get_nsaddr in res_send.c. */
> + void *addr;
> + size_t addrlen;
> + if (resp->nsaddr_list[i].sin_family == 0
> + && resp->_u._ext.nsaddrs[i] != NULL)
> + {
> + addr = resp->_u._ext.nsaddrs[i];
> + addrlen = sizeof (*resp->_u._ext.nsaddrs[i]);
> + }
> + else
> + {
> + addr = &resp->nsaddr_list[i];
> + addrlen = sizeof (resp->nsaddr_list[i]);
> + }
> +
> + int ret = getnameinfo (addr, addrlen,
> + host, sizeof (host), service, sizeof (service),
> + NI_NUMERICHOST | NI_NUMERICSERV);
> + if (ret != 0)
> + {
> + if (ret == EAI_SYSTEM)
> + fprintf (fp, "; error: getnameinfo: %m\n");
> + else
> + fprintf (fp, "; error: getnameinfo: %s\n", gai_strerror (ret));
> + }
> + else
> + {
> + fprintf (fp, "nameserver %s\n", host);
> + if (strcmp (service, "53") != 0)
> + fprintf (fp, "; unrepresentable port number %s\n\n", service);
> + }
> + }
> +
> + TEST_VERIFY (!ferror (fp));
> +}
> +
> +/* Parameters of one test case. */
> +struct test_case
> +{
> + /* A short, descriptive name of the test. */
> + const char *name;
> +
> + /* The contents of the /etc/resolv.conf file. */
> + const char *conf;
> +
> + /* The expected output from print_resp. */
> + const char *expected;
> +
> + /* Setting for the LOCALDOMAIN environment variable. NULL if the
> + variable is not to be set. */
> + const char *localdomain;
> +
> + /* Setting for the RES_OPTIONS environment variable. NULL if the
> + variable is not to be set. */
> + const char *res_options;
> +};
> +
> +enum test_init
> +{
> + test_init,
> + test_ninit,
> + test_mkquery,
> + test_gethostbyname,
> + test_getaddrinfo,
> + test_init_method_last = test_getaddrinfo
> +};
> +
> +/* Closure argument for run_res_init. */
> +struct test_context
> +{
> + enum test_init init;
> + const struct test_case *t;
> +};
> +
> +static void
> +setup_nss_dns_and_chroot (void)
> +{
> + /* Load nss_dns outside of the chroot. */
> + if (dlopen (LIBNSS_DNS_SO, RTLD_LAZY) == NULL)
> + FAIL_EXIT1 ("could not load " LIBNSS_DNS_SO ": %s", dlerror ());
> + xchroot (path_chroot);
> + /* Force the use of nss_dns. */
> + __nss_configure_lookup ("hosts", "dns");
> +}
> +
> +/* Run res_ninit or res_init in a subprocess and dump the parsed
> + resolver state to standard output. */
> +static void
> +run_res_init (void *closure)
> +{
> + struct test_context *ctx = closure;
> + TEST_VERIFY (getenv ("LOCALDOMAIN") == NULL);
> + TEST_VERIFY (getenv ("RES_OPTIONS") == NULL);
> + if (ctx->t->localdomain != NULL)
> + setenv ("LOCALDOMAIN", ctx->t->localdomain, 1);
> + if (ctx->t->res_options != NULL)
> + setenv ("RES_OPTIONS", ctx->t->res_options, 1);
> +
> + switch (ctx->init)
> + {
> + case test_init:
> + xchroot (path_chroot);
> + TEST_VERIFY (res_init () == 0);
> + print_resp (stdout, &_res);
> + return;
> +
> + case test_ninit:
> + xchroot (path_chroot);
> + res_state resp = xmalloc (sizeof (*resp));
> + memset (resp, 0, sizeof (*resp));
> + TEST_VERIFY (res_ninit (resp) == 0);
> + print_resp (stdout, resp);
> + res_nclose (resp);
> + free (resp);
> + return;
> +
> + case test_mkquery:
> + xchroot (path_chroot);
> + unsigned char buf[512];
> + TEST_VERIFY (res_mkquery (QUERY, "www.example",
> + C_IN, ns_t_a, NULL, 0,
> + NULL, buf, sizeof (buf)) > 0);
> + print_resp (stdout, &_res);
> + return;
> +
> + case test_gethostbyname:
> + setup_nss_dns_and_chroot ();
> + /* Trigger implicit initialization of the _res structure. The
> + actual lookup result is immaterial. */
> + (void )gethostbyname ("www.example");
> + print_resp (stdout, &_res);
> + return;
> +
> + case test_getaddrinfo:
> + setup_nss_dns_and_chroot ();
> + /* Trigger implicit initialization of the _res structure. The
> + actual lookup result is immaterial. */
> + struct addrinfo *ai;
> + (void) getaddrinfo ("www.example", NULL, NULL, &ai);
> + print_resp (stdout, &_res);
> + return;
> + }
> +
> + FAIL_EXIT1 ("invalid init method %d", ctx->init);
> +}
> +
> +#if TEST_THREAD
> +/* Helper function which calls run_res_init from a thread. */
> +static void *
> +run_res_init_thread_func (void *closure)
> +{
> + run_res_init (closure);
> + return NULL;
> +}
> +
> +/* Variant of res_run_init which runs the function on a non-main
> + thread. */
> +static void
> +run_res_init_on_thread (void *closure)
> +{
> + xpthread_join (xpthread_create (NULL, run_res_init_thread_func, closure));
> +}
> +#endif /* TEST_THREAD */
> +
> +struct test_case test_cases[] =
> + {
> + {.name = "empty file",
> + .conf = "",
> + .expected = "search example.com\n"
> + "nameserver 127.0.0.1\n"
> + },
> + {.name = "empty file with LOCALDOMAIN",
> + .conf = "",
> + .expected = "search example.net\n"
> + "nameserver 127.0.0.1\n",
> + .localdomain = "example.net",
> + },
> + {.name = "empty file with RES_OPTIONS",
> + .conf = "",
> + .expected = "options attempts:5 edns0\n"
> + "search example.com\n"
> + "nameserver 127.0.0.1\n",
> + .res_options = "edns0 attempts:5",
> + },
> + {.name = "empty file with RES_OPTIONS and LOCALDOMAIN",
> + .conf = "",
> + .expected = "options attempts:5 edns0\n"
> + "search example.org\n"
> + "nameserver 127.0.0.1\n",
> + .localdomain = "example.org",
> + .res_options = "edns0 attempts:5",
> + },
> + {.name = "basic",
> + .conf = "domain example.net\n"
> + "search corp.example.com example.com\n"
> + "nameserver 192.0.2.1\n",
> + .expected = "search corp.example.com example.com\n"
> + "nameserver 192.0.2.1\n"
> + },
> + {.name = "whitespace",
> + .conf = "# This test covers comment and whitespace processing "
> + " (trailing whitespace,\n"
> + "# missing newline at end of file).\n"
> + "\n"
> + "domain example.net\n"
> + ";search commented out\n"
> + "search corp.example.com\texample.com\n"
> + "#nameserver 192.0.2.3\n"
> + "nameserver 192.0.2.1 \n"
> + "nameserver 192.0.2.2", /* No \n at end of file. */
> + .expected = "search corp.example.com example.com\n"
> + "nameserver 192.0.2.1\n"
> + "nameserver 192.0.2.2\n"
> + },
> + {.name = "option values, multiple servers",
> + .conf = "options\tinet6\tndots:3 edns0\tattempts:5\ttimeout:19\n"
> + "domain example.net\n"
> + ";domain comment\n"
> + "search corp.example.com\texample.com\n"
> + "nameserver 192.0.2.1\n"
> + "nameserver ::1\n"
> + "nameserver 192.0.2.2\n",
> + .expected = "options ndots:3 timeout:19 attempts:5 inet6 edns0\n"
> + "search corp.example.com example.com\n"
> + "nameserver 192.0.2.1\n"
> + "nameserver ::1\n"
> + "nameserver 192.0.2.2\n"
> + },
> + {.name = "out-of-range option vales",
> + .conf = "options use-vc timeout:999 attempts:999 ndots:99\n"
> + "search example.com\n",
> + .expected = "options ndots:15 timeout:30 attempts:5 use-vc\n"
> + "search example.com\n"
> + "nameserver 127.0.0.1\n"
> + },
> + {.name = "repeated directives",
> + .conf = "options ndots:3 use-vc\n"
> + "options edns0 ndots:2\n"
> + "domain corp.example\n"
> + "search example.net corp.example.com example.com\n"
> + "search example.org\n"
> + "search\n",
> + .expected = "options ndots:2 use-vc edns0\n"
> + "search example.org\n"
> + "nameserver 127.0.0.1\n"
> + },
> + {.name = "many name servers, sortlist",
> + .conf = "options single-request\n"
> + "search example.org example.com example.net corp.example.com\n"
> + "sortlist 192.0.2.0/255.255.255.0\n"
> + "nameserver 192.0.2.1\n"
> + "nameserver 192.0.2.2\n"
> + "nameserver 192.0.2.3\n"
> + "nameserver 192.0.2.4\n"
> + "nameserver 192.0.2.5\n"
> + "nameserver 192.0.2.6\n"
> + "nameserver 192.0.2.7\n"
> + "nameserver 192.0.2.8\n",
> + .expected = "options single-request\n"
> + "search example.org example.com example.net corp.example.com\n"
> + "sortlist 192.0.2.0/255.255.255.0\n"
> + "nameserver 192.0.2.1\n"
> + "nameserver 192.0.2.2\n"
> + "nameserver 192.0.2.3\n"
> + },
> + {.name = "IPv4 and IPv6 nameservers",
> + .conf = "options single-request\n"
> + "search example.org example.com example.net corp.example.com"
> + " legacy.example.com\n"
> + "sortlist 192.0.2.0\n"
> + "nameserver 192.0.2.1\n"
> + "nameserver 2001:db8::2\n"
> + "nameserver 192.0.2.3\n"
> + "nameserver 2001:db8::4\n"
> + "nameserver 192.0.2.5\n"
> + "nameserver 2001:db8::6\n"
> + "nameserver 192.0.2.7\n"
> + "nameserver 2001:db8::8\n",
> + .expected = "options single-request\n"
> + "search example.org example.com example.net corp.example.com"
> + " legacy.example.com\n"
> + "sortlist 192.0.2.0/255.255.255.0\n"
> + "nameserver 192.0.2.1\n"
> + "nameserver 2001:db8::2\n"
> + "nameserver 192.0.2.3\n"
> + },
> + {.name = "many search entries",
> + .conf = "options single-request-reopen\n"
> + "search 1.example 2.example 3.example 4.example 5.example 6.example"
> + " 7.example\n"
> + "nameserver 192.0.2.1\n",
> + .expected = "options single-request-reopen\n"
> + "search 1.example 2.example 3.example 4.example 5.example 6.example\n"
> + "nameserver 192.0.2.1\n"
> + },
> + {.name = "overlong search entries",
> + .conf = "options dnssec\n"
> + "search aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
> + ".bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
> + ".example"
> + " ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"
> + ".ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
> + ".example"
> + " eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"
> + ".fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
> + ".example\n"
> + "nameserver 192.0.2.1\n",
> + .expected =
> + "search aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
> + ".bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
> + ".example"
> + /* This search entry is truncated. See bug 21475. */
> + " ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"
> + ".ddddddddddddddddddddddddddddddddddddddddddddddddddddddd\n"
> + "nameserver 192.0.2.1\n"
> + },
> + {.name = "many search entries via LOCALDOMAIN",
> + .conf = "options single-request-reopen\n"
> + "search 1.example 2.example 3.example 4.example 5.example 6.example"
> + " 7.example\n"
> + "nameserver 192.0.2.1\n",
> + .expected = "options single-request-reopen\n"
> + "search 1.example.net 2.example.net 3.example.net 4.example.net"
> + " 5.example.net 6.example.net\n"
> + "nameserver 192.0.2.1\n",
> + .localdomain = "1.example.net 2.example.net 3.example.net 4.example.net"
> + " 5.example.net 6.example.net 7.example.net\n"
> + },
> + {.name = "garbage after nameserver",
> + .conf = "nameserver 192.0.2.1 garbage\n"
> + "nameserver 192.0.2.2:5353\n"
> + "nameserver 192.0.2.3 5353\n",
> + .expected = "search example.com\n"
> + "nameserver 192.0.2.1\n"
> + "nameserver 192.0.2.3\n"
> + },
> + {.name = "RES_OPTIONS is cummulative",
> + .conf = "options timeout:7 ndots:2 use-vc\n"
> + "nameserver 192.0.2.1\n",
> + .expected = "options ndots:3 timeout:7 attempts:5 use-vc edns0\n"
> + "search example.com\n"
> + "nameserver 192.0.2.1\n",
> + .res_options = "attempts:5 ndots:3 edns0 ",
> + },
> + { NULL }
> + };
> +
> +/* Run the indicated test case. This function assumes that the chroot
> + contents has already been set up. */
> +static void
> +test_file_contents (const struct test_case *t)
> +{
> +#if TEST_THREAD
> + for (int do_thread = 0; do_thread < 2; ++do_thread)
> +#endif
> + for (int init_method = 0; init_method <= test_init_method_last;
> + ++init_method)
> + {
> + if (test_verbose > 0)
> + printf ("info: testing init method %d\n", init_method);
> + struct test_context ctx = { .init = init_method, .t = t };
> + void (*func) (void *) = run_res_init;
> +#if TEST_THREAD
> + if (do_thread)
> + func = run_res_init_on_thread;
> +#endif
> + struct support_capture_subprocess proc
> + = support_capture_subprocess (func, &ctx);
> + if (strcmp (proc.out.buffer, t->expected) != 0)
> + {
> + support_record_failure ();
> + printf ("error: output mismatch for %s\n", t->name);
> + support_run_diff ("expected", t->expected,
> + "actual", proc.out.buffer);
> + }
> + support_capture_subprocess_check (&proc, t->name, 0,
> + sc_allow_stdout);
> + support_capture_subprocess_free (&proc);
> + }
> +}
> +
> +static int
> +do_test (void)
> +{
> + support_become_root ();
> + support_enter_network_namespace ();
> + if (!support_in_uts_namespace () || !support_can_chroot ())
> + return EXIT_UNSUPPORTED;
> +
> + /* We are in an UTS namespace, so we can set the host name without
> + altering the state of the entire system. */
> + if (sethostname (test_hostname, strlen (test_hostname)) != 0)
> + FAIL_EXIT1 ("sethostname: %m");
> +
> + /* These environment variables affect resolv.conf parsing. */
> + unsetenv ("LOCALDOMAIN");
> + unsetenv ("RES_OPTIONS");
> +
> + /* Ensure that the chroot setup worked. */
> + {
> + struct support_capture_subprocess proc
> + = support_capture_subprocess (check_chroot_working, NULL);
> + support_capture_subprocess_check (&proc, "chroot", 0, sc_allow_none);
> + support_capture_subprocess_free (&proc);
> + }
> +
> + for (size_t i = 0; test_cases[i].name != NULL; ++i)
> + {
> + if (test_verbose > 0)
> + printf ("info: running test: %s\n", test_cases[i].name);
> + TEST_VERIFY (test_cases[i].conf != NULL);
> + TEST_VERIFY (test_cases[i].expected != NULL);
> +
> + support_write_file_string (path_resolv_conf, test_cases[i].conf);
> +
> + test_file_contents (&test_cases[i]);
> +
> + /* The expected output from the empty file test is used for
> + further tests. */
> + if (test_cases[i].conf[0] == '\0')
> + {
> + if (test_verbose > 0)
> + printf ("info: special test: missing file\n");
> + TEST_VERIFY (unlink (path_resolv_conf) == 0);
> + test_file_contents (&test_cases[i]);
> +
> + if (test_verbose > 0)
> + printf ("info: special test: dangling symbolic link\n");
> + TEST_VERIFY (symlink ("does-not-exist", path_resolv_conf) == 0);
> + test_file_contents (&test_cases[i]);
> + TEST_VERIFY (unlink (path_resolv_conf) == 0);
> +
> + if (test_verbose > 0)
> + printf ("info: special test: unreadable file\n");
> + support_write_file_string (path_resolv_conf, "");
> + TEST_VERIFY (chmod (path_resolv_conf, 0) == 0);
> + test_file_contents (&test_cases[i]);
> +
> + /* Restore the empty file. */
> + TEST_VERIFY (unlink (path_resolv_conf) == 0);
> + support_write_file_string (path_resolv_conf, "");
> + }
> + }
> +
> + free (path_chroot);
> + path_chroot = NULL;
> + free (path_resolv_conf);
> + path_resolv_conf = NULL;
> + return 0;
> +}
> +
> +#define PREPARE prepare
> +#include <support/test-driver.c>
> diff --git a/resolv/tst-resolv-res_init-thread.c b/resolv/tst-resolv-res_init-thread.c
> new file mode 100644
> index 0000000..1dfc771
> --- /dev/null
> +++ b/resolv/tst-resolv-res_init-thread.c
> @@ -0,0 +1,20 @@
> +/* Test parsing of /etc/resolv.conf, threading version.
> + Copyright (C) 2016-2017 Free Software Foundation, Inc.
Fix copyright date.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#define TEST_THREAD 1
> +#include "tst-resolv-res_init-skeleton.c"
> diff --git a/resolv/tst-resolv-res_init.c b/resolv/tst-resolv-res_init.c
> new file mode 100644
> index 0000000..b4b2563
> --- /dev/null
> +++ b/resolv/tst-resolv-res_init.c
> @@ -0,0 +1,20 @@
> +/* Test parsing of /etc/resolv.conf, non-threading version.
> + Copyright (C) 2016-2017 Free Software Foundation, Inc.
Fix copyright date.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#define TEST_THREAD 0
> +#include "tst-resolv-res_init-skeleton.c"
Why not just have a tst-resolv-res_init.c and
tst-resolv-res_init-thread.c? That's how a lot of the other similar
kinds of tests are rewritten. I don't have a very strong opinion on
this though, you can choose the color of your shed :)
> diff --git a/support/Makefile b/support/Makefile
> index 3ce73a6..20b0343 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -35,8 +35,11 @@ libsupport-routines = \
> oom_error \
> resolv_test \
> set_fortify_handler \
> + support-xstat \
> support_become_root \
> + support_can_chroot \
> support_capture_subprocess \
> + support_capture_subprocess_check \
> support_enter_network_namespace \
> support_format_address_family \
> support_format_addrinfo \
> @@ -44,8 +47,11 @@ libsupport-routines = \
> support_format_herrno \
> support_format_hostent \
> support_format_netent \
> + support_isolate_in_subprocess \
> support_record_failure \
> support_run_diff \
> + support_shared_allocate \
> + support_write_file_string \
> support_test_main \
> support_test_verify_impl \
> temp_file \
> @@ -55,6 +61,7 @@ libsupport-routines = \
> xasprintf \
> xbind \
> xcalloc \
> + xchroot \
> xclose \
> xconnect \
> xdup2 \
> @@ -65,8 +72,10 @@ libsupport-routines = \
> xlisten \
> xmalloc \
> xmemstream \
> + xmkdir \
> xmmap \
> xmunmap \
> + xopen \
> xpipe \
> xpoll \
> xpthread_attr_destroy \
> diff --git a/support/capture_subprocess.h b/support/capture_subprocess.h
> index 3265d63..510c15f 100644
> --- a/support/capture_subprocess.h
> +++ b/support/capture_subprocess.h
> @@ -39,4 +39,23 @@ struct support_capture_subprocess support_capture_subprocess
> support_capture_subprocess. */
> void support_capture_subprocess_free (struct support_capture_subprocess *);
>
> +enum support_capture_allow
> +{
> + /* No output is allowed. */
> + sc_allow_none = 0x01,
> + /* Output to stdout is permitted. */
> + sc_allow_stdout = 0x02,
> + /* Output to standard error is permitted. */
> + sc_allow_stderr = 0x04,
> +};
> +
> +/* Check that the subprocess exited with STATUS and that only the
> + allowed outputs happened. ALLOWED is a combination of
> + support_capture_allow flags. Report errors under the CONTEXT
> + message. */
> +void support_capture_subprocess_check (struct support_capture_subprocess *,
> + const char *context, int status,
> + int allowed)
> + __attribute__ ((nonnull (1, 2)));
> +
> #endif /* SUPPORT_CAPTURE_SUBPROCESS_H */
> diff --git a/support/namespace.h b/support/namespace.h
> index 6bc82d6..e1ccaa1 100644
> --- a/support/namespace.h
> +++ b/support/namespace.h
> @@ -35,6 +35,13 @@ __BEGIN_DECLS
> single-threaded processes. */
> bool support_become_root (void);
>
> +/* Return true if this process can perform a chroot operation. In
> + general, this is only possible if support_become_root has been
> + called. Note that the actual test is performed in a subprocess,
> + after fork, so that the file system root of the original process is
> + not changed. */
> +bool support_can_chroot (void);
> +
> /* Enter a network namespace (and a UTS namespace if possible) and
> configure the loopback interface. Return true if a network
> namespace could be created. Print diagnostics to standard output.
> @@ -48,6 +55,11 @@ bool support_enter_network_namespace (void);
> UTS namespace. */
> bool support_in_uts_namespace (void);
>
> +/* Invoke CALLBACK (CLOSURE) in a subprocess created using fork.
> + Terminate the calling process if the subprocess exits with a
> + non-zero exit status. */
> +void support_isolate_in_subprocess (void (*callback) (void *), void *closure);
> +
> __END_DECLS
>
> #endif
> diff --git a/support/support-xstat.c b/support/support-xstat.c
> new file mode 100644
> index 0000000..0b02a1f
> --- /dev/null
> +++ b/support/support-xstat.c
> @@ -0,0 +1,30 @@
> +/* stat64 with error checking.
> + Copyright (C) 2016-2017 Free Software Foundation, Inc.
Fix copyright year.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +/* NB: Non-standard file name to avoid sysdeps override for xstat. */
> +
> +#include <support/check.h>
> +#include <support/xunistd.h>
> +#include <sys/stat.h>
> +
> +void
> +xstat (const char *path, struct stat64 *result)
> +{
> + if (stat64 (path, result) != 0)
> + FAIL_EXIT1 ("stat64 (\"%s\"): %m", path);
> +}
> diff --git a/support/support.h b/support/support.h
> index 7292e2a..4b5f04c 100644
> --- a/support/support.h
> +++ b/support/support.h
> @@ -44,6 +44,21 @@ void set_fortify_handler (void (*handler) (int sig));
> void oom_error (const char *function, size_t size)
> __attribute__ ((nonnull (1)));
>
> +/* Return a pointer to a memory region of SIZE bytes. The memory is
> + initialized to zero and will be shared with subprocesses (across
> + fork). The returned pointer must be freed using
> + support_shared_free; it is not compatible with the malloc
> + functions. */
> +void *support_shared_allocate (size_t size);
> +
> +/* Deallocate a pointer returned by support_shared_allocate. */
> +void support_shared_free (void *);
> +
> +/* Write CONTENTS to the file PATH. Create or truncate the file as
> + needed. The file mode is 0666 masked by the umask. Terminate the
> + process on error. */
> +void support_write_file_string (const char *path, const char *contents);
> +
> /* Error-checking wrapper functions which terminate the process on
> error. */
>
> diff --git a/support/support_can_chroot.c b/support/support_can_chroot.c
> new file mode 100644
> index 0000000..0dfd2de
> --- /dev/null
> +++ b/support/support_can_chroot.c
> @@ -0,0 +1,65 @@
> +/* Return true if the process can perform a chroot operation.
> + Copyright (C) 2017 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <errno.h>
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/namespace.h>
> +#include <support/support.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <xunistd.h>
> +
> +static void
> +callback (void *closure)
> +{
> + int *result = closure;
> + struct stat64 before;
> + xstat ("/dev", &before);
> + if (chroot ("/dev") != 0)
> + {
> + *result = errno;
> + return;
> + }
> + struct stat64 after;
> + xstat ("/", &after);
> + TEST_VERIFY (before.st_dev == after.st_dev);
> + TEST_VERIFY (before.st_ino == after.st_ino);
> + *result = 0;
> +}
> +
> +bool
> +support_can_chroot (void)
> +{
> + int *result = support_shared_allocate (sizeof (*result));
> + *result = 0;
> + support_isolate_in_subprocess (callback, result);
> + bool ok = *result == 0;
> + if (!ok)
> + {
> + static bool already_warned;
> + if (!already_warned)
> + {
> + already_warned = true;
> + errno = *result;
> + printf ("warning: this process does not support chroot: %m\n");
> + }
> + }
> + support_shared_free (result);
> + return ok;
> +}
> diff --git a/support/support_capture_subprocess_check.c b/support/support_capture_subprocess_check.c
> new file mode 100644
> index 0000000..708c89f
> --- /dev/null
> +++ b/support/support_capture_subprocess_check.c
> @@ -0,0 +1,67 @@
> +/* Verify capture output from a subprocess.
> + Copyright (C) 2017 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <support/capture_subprocess.h>
> +#include <support/check.h>
> +
> +static void
> +print_context (const char *context, bool *failed)
> +{
> + if (*failed)
> + /* Do not duplicate message. */
> + return;
> + support_record_failure ();
> + printf ("error: subprocess failed: %s\n", context);
> +}
> +
> +void
> +support_capture_subprocess_check (struct support_capture_subprocess *proc,
> + const char *context, int status,
> + int allowed)
> +{
> + TEST_VERIFY ((allowed & sc_allow_none)
> + || (allowed & sc_allow_stdout)
> + || (allowed & sc_allow_stderr));
> + TEST_VERIFY (!((allowed & sc_allow_none)
> + && ((allowed & sc_allow_stdout)
> + || (allowed & sc_allow_stderr))));
> +
> + bool failed = false;
> + if (proc->status != status)
> + {
> + print_context (context, &failed);
> + printf ("error: expected exit status: %d\n", status);
> + printf ("error: actual exit status: %d\n", status);
> + }
> + if (!(allowed & sc_allow_stdout) && proc->out.length != 0)
> + {
> + print_context (context, &failed);
> + printf ("error: unexpected output from subprocess\n");
> + fwrite (proc->out.buffer, proc->out.length, 1, stdout);
> + puts ("\n");
> + }
> + if (!(allowed & sc_allow_stderr) && proc->err.length != 0)
> + {
> + print_context (context, &failed);
> + printf ("error: unexpected error output from subprocess\n");
> + fwrite (proc->err.buffer, proc->err.length, 1, stdout);
> + puts ("\n");
> + }
> +}
> diff --git a/support/support_isolate_in_subprocess.c b/support/support_isolate_in_subprocess.c
> new file mode 100644
> index 0000000..cf48614
> --- /dev/null
> +++ b/support/support_isolate_in_subprocess.c
> @@ -0,0 +1,38 @@
> +/* Run a function in a subprocess.
> + Copyright (C) 2017 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/check.h>
> +#include <support/xunistd.h>
> +
> +void
> +support_isolate_in_subprocess (void (*callback) (void *), void *closure)
> +{
> + pid_t pid = xfork ();
> + if (pid == 0)
> + {
> + /* Child process. */
> + callback (closure);
> + _exit (0);
> + }
> +
> + /* Parent process. */
> + int status;
> + xwaitpid (pid, &status, 0);
> + if (status != 0)
> + FAIL_EXIT1 ("child process exited with status %d", status);
> +}
> diff --git a/support/support_shared_allocate.c b/support/support_shared_allocate.c
> new file mode 100644
> index 0000000..61d088e
> --- /dev/null
> +++ b/support/support_shared_allocate.c
> @@ -0,0 +1,57 @@
> +/* Allocate a memory region shared across processes.
> + Copyright (C) 2017 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <errno.h>
> +#include <stddef.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <sys/mman.h>
> +
> +/* Header for the allocation. It contains the size of the allocation
> + for subsequent unmapping. */
> +struct header
> +{
> + size_t total_size;
> + char data[] __attribute__ ((aligned (__alignof__ (max_align_t))));
> +};
> +
> +void *
> +support_shared_allocate (size_t size)
> +{
> + size_t total_size = size + offsetof (struct header, data);
> + if (total_size < size)
> + {
> + errno = ENOMEM;
> + oom_error (__func__, size);
> + return NULL;
> + }
> + else
> + {
> + struct header *result = xmmap (NULL, total_size, PROT_READ | PROT_WRITE,
> + MAP_ANONYMOUS | MAP_SHARED, -1);
> + result->total_size = total_size;
> + return &result->data;
> + }
> +}
> +
> +void
> +support_shared_free (void *data)
> +{
> + struct header *header = data - offsetof (struct header, data);
> + xmunmap (header, header->total_size);
> +}
> diff --git a/support/support_write_file_string.c b/support/support_write_file_string.c
> new file mode 100644
> index 0000000..48e8959
> --- /dev/null
> +++ b/support/support_write_file_string.c
> @@ -0,0 +1,39 @@
> +/* Write a string to a file.
> + Copyright (C) 2017 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <fcntl.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <xunistd.h>
> +
> +void
> +support_write_file_string (const char *path, const char *contents)
> +{
> + int fd = xopen (path, O_CREAT | O_TRUNC | O_WRONLY, 0666);
> + const char *end = contents + strlen (contents);
> + for (const char *p = contents; p < end; )
> + {
> + ssize_t ret = write (fd, p, end - p);
> + if (ret < 0)
> + FAIL_EXIT1 ("cannot write to \"%s\": %m", path);
> + if (ret == 0)
> + FAIL_EXIT1 ("zero-length write to \"%s\"", path);
> + p += ret;
> + }
> + xclose (fd);
> +}
> diff --git a/support/xchroot.c b/support/xchroot.c
> new file mode 100644
> index 0000000..af99aa2
> --- /dev/null
> +++ b/support/xchroot.c
> @@ -0,0 +1,28 @@
> +/* chroot with error checking.
> + Copyright (C) 2016-2017 Free Software Foundation, Inc.
Fix copyright year.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/check.h>
> +#include <support/xunistd.h>
> +#include <sys/stat.h>
> +
> +void
> +xchroot (const char *path)
> +{
> + if (chroot (path) != 0)
> + FAIL_EXIT1 ("chroot (\"%s\"): %m", path);
> +}
> diff --git a/support/xmkdir.c b/support/xmkdir.c
> new file mode 100644
> index 0000000..96aa0e1
> --- /dev/null
> +++ b/support/xmkdir.c
> @@ -0,0 +1,28 @@
> +/* mkdir with error checking.
> + Copyright (C) 2016-2017 Free Software Foundation, Inc.
Fix copyright year.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/check.h>
> +#include <support/xunistd.h>
> +#include <sys/stat.h>
> +
> +void
> +xmkdir (const char *path, mode_t mode)
> +{
> + if (mkdir (path, mode) != 0)
> + FAIL_EXIT1 ("mkdir (\"%s\", 0%o): %m", path, mode);
> +}
> diff --git a/support/xopen.c b/support/xopen.c
> new file mode 100644
> index 0000000..91aa99d
> --- /dev/null
> +++ b/support/xopen.c
> @@ -0,0 +1,31 @@
> +/* open64 with error checking.
> + Copyright (C) 2016-2017 Free Software Foundation, Inc.
Fix copyright year.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/check.h>
> +#include <support/xunistd.h>
> +#include <fcntl.h>
> +
> +int
> +xopen (const char *path, int flags, mode_t mode)
> +{
> + int ret = open64 (path, flags, mode);
> + if (ret < 0)
> + FAIL_EXIT1 ("open64 (\"%s\", 0x%x, 0%o): %m", path, flags, mode);
> + return ret;
> +}
> +
> diff --git a/support/xunistd.h b/support/xunistd.h
> index 7c14bda..151d743 100644
> --- a/support/xunistd.h
> +++ b/support/xunistd.h
> @@ -22,15 +22,22 @@
> #ifndef SUPPORT_XUNISTD_H
> #define SUPPORT_XUNISTD_H
>
> -#include <unistd.h>
> #include <sys/cdefs.h>
> +#include <sys/types.h>
> +#include <unistd.h>
>
> __BEGIN_DECLS
>
> +struct stat64;
> +
> pid_t xfork (void);
> pid_t xwaitpid (pid_t, int *status, int flags);
> void xpipe (int[2]);
> void xdup2 (int, int);
> +int xopen (const char *path, int flags, mode_t);
> +void xstat (const char *path, struct stat64 *);
> +void xmkdir (const char *path, mode_t);
> +void xchroot (const char *path);
>
> /* Close the file descriptor. Ignore EINTR errors, but terminate the
> process on other errors. */
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-05-18 20:05 ` Siddhesh Poyarekar
@ 2017-06-02 13:49 ` Florian Weimer
2017-06-02 20:19 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: Florian Weimer @ 2017-06-02 13:49 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: GNU C Library
On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
> Why not just have a tst-resolv-res_init.c and
> tst-resolv-res_init-thread.c? That's how a lot of the other similar
> kinds of tests are rewritten. I don't have a very strong opinion on
> this though, you can choose the color of your shed :)
I do it this way so that I can use #if instead of #ifdef, following the
current guidelines to trigger -Wundef warnings on typos.
I'm going to push this without the tests expecting incorrect results.
Thanks,
Florian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-02 13:49 ` Florian Weimer
@ 2017-06-02 20:19 ` H.J. Lu
2017-06-02 20:40 ` Florian Weimer
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-06-02 20:19 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, GNU C Library
On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>> Why not just have a tst-resolv-res_init.c and
>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>> kinds of tests are rewritten. I don't have a very strong opinion on
>> this though, you can choose the color of your shed :)
>
> I do it this way so that I can use #if instead of #ifdef, following the
> current guidelines to trigger -Wundef warnings on typos.
>
> I'm going to push this without the tests expecting incorrect results.
>
On Fedora 25/x86-64, I got
[hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
Timed out: killed the child process
[hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
Timed out: killed the child process
[hjl@gnu-6 build-x86_64-linux]$
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-02 20:19 ` H.J. Lu
@ 2017-06-02 20:40 ` Florian Weimer
2017-06-02 20:43 ` H.J. Lu
2017-06-03 15:38 ` Andreas Schwab
0 siblings, 2 replies; 20+ messages in thread
From: Florian Weimer @ 2017-06-02 20:40 UTC (permalink / raw)
To: H.J. Lu; +Cc: Siddhesh Poyarekar, GNU C Library
On 06/02/2017 10:19 PM, H.J. Lu wrote:
> On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>>> Why not just have a tst-resolv-res_init.c and
>>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>>> kinds of tests are rewritten. I don't have a very strong opinion on
>>> this though, you can choose the color of your shed :)
>>
>> I do it this way so that I can use #if instead of #ifdef, following the
>> current guidelines to trigger -Wundef warnings on typos.
>>
>> I'm going to push this without the tests expecting incorrect results.
>>
>
> On Fedora 25/x86-64, I got
>
> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
> Timed out: killed the child process
> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
> Timed out: killed the child process
> [hjl@gnu-6 build-x86_64-linux]$
I see that too, with 4.11.3-200.fc25.x86_64. What's your kernel version?
I don't see the delay with 4.10.17-100.fc24.x86_64. There, the poll
system calls return immediately. I wonder if it's some sort of
regression in network namespaces.
Florian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-02 20:40 ` Florian Weimer
@ 2017-06-02 20:43 ` H.J. Lu
2017-06-03 2:06 ` H.J. Lu
2017-06-03 15:38 ` Andreas Schwab
1 sibling, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-06-02 20:43 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, GNU C Library
On Fri, Jun 2, 2017 at 1:40 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/02/2017 10:19 PM, H.J. Lu wrote:
>> On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>>>> Why not just have a tst-resolv-res_init.c and
>>>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>>>> kinds of tests are rewritten. I don't have a very strong opinion on
>>>> this though, you can choose the color of your shed :)
>>>
>>> I do it this way so that I can use #if instead of #ifdef, following the
>>> current guidelines to trigger -Wundef warnings on typos.
>>>
>>> I'm going to push this without the tests expecting incorrect results.
>>>
>>
>> On Fedora 25/x86-64, I got
>>
>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
>> Timed out: killed the child process
>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
>> Timed out: killed the child process
>> [hjl@gnu-6 build-x86_64-linux]$
>
> I see that too, with 4.11.3-200.fc25.x86_64. What's your kernel version?
>
> I don't see the delay with 4.10.17-100.fc24.x86_64. There, the poll
> system calls return immediately. I wonder if it's some sort of
> regression in network namespaces.
Yes, I am also running 4.11.3-200.fc25.x86_64.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-02 20:43 ` H.J. Lu
@ 2017-06-03 2:06 ` H.J. Lu
2017-06-03 6:43 ` Florian Weimer
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-06-03 2:06 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, GNU C Library
On Fri, Jun 2, 2017 at 1:43 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jun 2, 2017 at 1:40 PM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 06/02/2017 10:19 PM, H.J. Lu wrote:
>>> On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>>>>> Why not just have a tst-resolv-res_init.c and
>>>>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>>>>> kinds of tests are rewritten. I don't have a very strong opinion on
>>>>> this though, you can choose the color of your shed :)
>>>>
>>>> I do it this way so that I can use #if instead of #ifdef, following the
>>>> current guidelines to trigger -Wundef warnings on typos.
>>>>
>>>> I'm going to push this without the tests expecting incorrect results.
>>>>
>>>
>>> On Fedora 25/x86-64, I got
>>>
>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
>>> Timed out: killed the child process
>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
>>> Timed out: killed the child process
>>> [hjl@gnu-6 build-x86_64-linux]$
>>
>> I see that too, with 4.11.3-200.fc25.x86_64. What's your kernel version?
>>
>> I don't see the delay with 4.10.17-100.fc24.x86_64. There, the poll
>> system calls return immediately. I wonder if it's some sort of
>> regression in network namespaces.
>
> Yes, I am also running 4.11.3-200.fc25.x86_64.
>
I booted 4.10.16-200.fc25.x86_64 and the test passed. Any ideas?
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-03 2:06 ` H.J. Lu
@ 2017-06-03 6:43 ` Florian Weimer
2017-06-03 15:00 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: Florian Weimer @ 2017-06-03 6:43 UTC (permalink / raw)
To: H.J. Lu; +Cc: Siddhesh Poyarekar, GNU C Library
On 06/03/2017 04:06 AM, H.J. Lu wrote:
> On Fri, Jun 2, 2017 at 1:43 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Jun 2, 2017 at 1:40 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 06/02/2017 10:19 PM, H.J. Lu wrote:
>>>> On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>>>>>> Why not just have a tst-resolv-res_init.c and
>>>>>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>>>>>> kinds of tests are rewritten. I don't have a very strong opinion on
>>>>>> this though, you can choose the color of your shed :)
>>>>>
>>>>> I do it this way so that I can use #if instead of #ifdef, following the
>>>>> current guidelines to trigger -Wundef warnings on typos.
>>>>>
>>>>> I'm going to push this without the tests expecting incorrect results.
>>>>>
>>>>
>>>> On Fedora 25/x86-64, I got
>>>>
>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
>>>> Timed out: killed the child process
>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
>>>> Timed out: killed the child process
>>>> [hjl@gnu-6 build-x86_64-linux]$
>>>
>>> I see that too, with 4.11.3-200.fc25.x86_64. What's your kernel version?
>>>
>>> I don't see the delay with 4.10.17-100.fc24.x86_64. There, the poll
>>> system calls return immediately. I wonder if it's some sort of
>>> regression in network namespaces.
>>
>> Yes, I am also running 4.11.3-200.fc25.x86_64.
>>
>
> I booted 4.10.16-200.fc25.x86_64 and the test passed. Any ideas?
I'm going to try to come up with a self-contained reproducer, and then
approach the kernel people if it still looks like a kernel bug.
Florian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-03 6:43 ` Florian Weimer
@ 2017-06-03 15:00 ` H.J. Lu
2017-06-04 0:56 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-06-03 15:00 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, GNU C Library
On Fri, Jun 2, 2017 at 11:43 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/03/2017 04:06 AM, H.J. Lu wrote:
>> On Fri, Jun 2, 2017 at 1:43 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Jun 2, 2017 at 1:40 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>>> On 06/02/2017 10:19 PM, H.J. Lu wrote:
>>>>> On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>>>>>>> Why not just have a tst-resolv-res_init.c and
>>>>>>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>>>>>>> kinds of tests are rewritten. I don't have a very strong opinion on
>>>>>>> this though, you can choose the color of your shed :)
>>>>>>
>>>>>> I do it this way so that I can use #if instead of #ifdef, following the
>>>>>> current guidelines to trigger -Wundef warnings on typos.
>>>>>>
>>>>>> I'm going to push this without the tests expecting incorrect results.
>>>>>>
>>>>>
>>>>> On Fedora 25/x86-64, I got
>>>>>
>>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
>>>>> Timed out: killed the child process
>>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
>>>>> Timed out: killed the child process
>>>>> [hjl@gnu-6 build-x86_64-linux]$
>>>>
>>>> I see that too, with 4.11.3-200.fc25.x86_64. What's your kernel version?
>>>>
>>>> I don't see the delay with 4.10.17-100.fc24.x86_64. There, the poll
>>>> system calls return immediately. I wonder if it's some sort of
>>>> regression in network namespaces.
>>>
>>> Yes, I am also running 4.11.3-200.fc25.x86_64.
>>>
>>
>> I booted 4.10.16-200.fc25.x86_64 and the test passed. Any ideas?
>
> I'm going to try to come up with a self-contained reproducer, and then
> approach the kernel people if it still looks like a kernel bug.
>
> Florian
Kernel regression was introduced by
commit 580bdf5650fff8f66468ce491f8308f1117b7074
Merge: e60a426 a249708
Author: David S. Miller <davem@davemloft.net>
Date: Tue Jan 17 15:19:37 2017 -0500
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
But I don't know exactly which commit caused it.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-02 20:40 ` Florian Weimer
2017-06-02 20:43 ` H.J. Lu
@ 2017-06-03 15:38 ` Andreas Schwab
1 sibling, 0 replies; 20+ messages in thread
From: Andreas Schwab @ 2017-06-03 15:38 UTC (permalink / raw)
To: Florian Weimer; +Cc: H.J. Lu, Siddhesh Poyarekar, GNU C Library
On Jun 02 2017, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/02/2017 10:19 PM, H.J. Lu wrote:
>> On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>>>> Why not just have a tst-resolv-res_init.c and
>>>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>>>> kinds of tests are rewritten. I don't have a very strong opinion on
>>>> this though, you can choose the color of your shed :)
>>>
>>> I do it this way so that I can use #if instead of #ifdef, following the
>>> current guidelines to trigger -Wundef warnings on typos.
>>>
>>> I'm going to push this without the tests expecting incorrect results.
>>>
>>
>> On Fedora 25/x86-64, I got
>>
>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
>> Timed out: killed the child process
>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
>> Timed out: killed the child process
>> [hjl@gnu-6 build-x86_64-linux]$
>
> I see that too, with 4.11.3-200.fc25.x86_64. What's your kernel version?
I don't see that with 4.11.3-1-default.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-03 15:00 ` H.J. Lu
@ 2017-06-04 0:56 ` H.J. Lu
2017-06-04 1:46 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-06-04 0:56 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, GNU C Library
On Sat, Jun 3, 2017 at 8:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jun 2, 2017 at 11:43 PM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 06/03/2017 04:06 AM, H.J. Lu wrote:
>>> On Fri, Jun 2, 2017 at 1:43 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Fri, Jun 2, 2017 at 1:40 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>> On 06/02/2017 10:19 PM, H.J. Lu wrote:
>>>>>> On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>>> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>>>>>>>> Why not just have a tst-resolv-res_init.c and
>>>>>>>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>>>>>>>> kinds of tests are rewritten. I don't have a very strong opinion on
>>>>>>>> this though, you can choose the color of your shed :)
>>>>>>>
>>>>>>> I do it this way so that I can use #if instead of #ifdef, following the
>>>>>>> current guidelines to trigger -Wundef warnings on typos.
>>>>>>>
>>>>>>> I'm going to push this without the tests expecting incorrect results.
>>>>>>>
>>>>>>
>>>>>> On Fedora 25/x86-64, I got
>>>>>>
>>>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
>>>>>> Timed out: killed the child process
>>>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
>>>>>> Timed out: killed the child process
>>>>>> [hjl@gnu-6 build-x86_64-linux]$
>>>>>
>>>>> I see that too, with 4.11.3-200.fc25.x86_64. What's your kernel version?
>>>>>
>>>>> I don't see the delay with 4.10.17-100.fc24.x86_64. There, the poll
>>>>> system calls return immediately. I wonder if it's some sort of
>>>>> regression in network namespaces.
>>>>
>>>> Yes, I am also running 4.11.3-200.fc25.x86_64.
>>>>
>>>
>>> I booted 4.10.16-200.fc25.x86_64 and the test passed. Any ideas?
>>
>> I'm going to try to come up with a self-contained reproducer, and then
>> approach the kernel people if it still looks like a kernel bug.
>>
>> Florian
>
> Kernel regression was introduced by
>
> commit 580bdf5650fff8f66468ce491f8308f1117b7074
> Merge: e60a426 a249708
> Author: David S. Miller <davem@davemloft.net>
> Date: Tue Jan 17 15:19:37 2017 -0500
>
> Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
>
> But I don't know exactly which commit caused it.
>
It was caused by:
commit 44bb765cf07ab6622e6fdf4bce546b43bd20faee
Author: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue Jan 10 12:32:36 2017 -0800
net: dsa: Implement ndo_get_phys_port_name()
Return the physical port number of a DSA created network device using
ndo_get_phys_port_name().
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-04 0:56 ` H.J. Lu
@ 2017-06-04 1:46 ` H.J. Lu
2017-06-04 5:10 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-06-04 1:46 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, GNU C Library
On Sat, Jun 3, 2017 at 5:56 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Jun 3, 2017 at 8:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Jun 2, 2017 at 11:43 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 06/03/2017 04:06 AM, H.J. Lu wrote:
>>>> On Fri, Jun 2, 2017 at 1:43 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Fri, Jun 2, 2017 at 1:40 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>> On 06/02/2017 10:19 PM, H.J. Lu wrote:
>>>>>>> On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>>>> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>>>>>>>>> Why not just have a tst-resolv-res_init.c and
>>>>>>>>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>>>>>>>>> kinds of tests are rewritten. I don't have a very strong opinion on
>>>>>>>>> this though, you can choose the color of your shed :)
>>>>>>>>
>>>>>>>> I do it this way so that I can use #if instead of #ifdef, following the
>>>>>>>> current guidelines to trigger -Wundef warnings on typos.
>>>>>>>>
>>>>>>>> I'm going to push this without the tests expecting incorrect results.
>>>>>>>>
>>>>>>>
>>>>>>> On Fedora 25/x86-64, I got
>>>>>>>
>>>>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
>>>>>>> Timed out: killed the child process
>>>>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
>>>>>>> Timed out: killed the child process
>>>>>>> [hjl@gnu-6 build-x86_64-linux]$
>>>>>>
>>>>>> I see that too, with 4.11.3-200.fc25.x86_64. What's your kernel version?
>>>>>>
>>>>>> I don't see the delay with 4.10.17-100.fc24.x86_64. There, the poll
>>>>>> system calls return immediately. I wonder if it's some sort of
>>>>>> regression in network namespaces.
>>>>>
>>>>> Yes, I am also running 4.11.3-200.fc25.x86_64.
>>>>>
>>>>
>>>> I booted 4.10.16-200.fc25.x86_64 and the test passed. Any ideas?
>>>
>>> I'm going to try to come up with a self-contained reproducer, and then
>>> approach the kernel people if it still looks like a kernel bug.
>>>
>>> Florian
>>
>> Kernel regression was introduced by
>>
>> commit 580bdf5650fff8f66468ce491f8308f1117b7074
>> Merge: e60a426 a249708
>> Author: David S. Miller <davem@davemloft.net>
>> Date: Tue Jan 17 15:19:37 2017 -0500
>>
>> Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
>>
>> But I don't know exactly which commit caused it.
>>
>
> It was caused by:
>
> commit 44bb765cf07ab6622e6fdf4bce546b43bd20faee
> Author: Florian Fainelli <f.fainelli@gmail.com>
> Date: Tue Jan 10 12:32:36 2017 -0800
>
> net: dsa: Implement ndo_get_phys_port_name()
>
> Return the physical port number of a DSA created network device using
> ndo_get_phys_port_name().
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
>
I was wrong. It isn't the bad one.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-04 1:46 ` H.J. Lu
@ 2017-06-04 5:10 ` H.J. Lu
2017-06-04 5:39 ` Florian Weimer
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-06-04 5:10 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, GNU C Library
[-- Attachment #1: Type: text/plain, Size: 4593 bytes --]
On Sat, Jun 3, 2017 at 6:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Jun 3, 2017 at 5:56 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Sat, Jun 3, 2017 at 8:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Jun 2, 2017 at 11:43 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>>> On 06/03/2017 04:06 AM, H.J. Lu wrote:
>>>>> On Fri, Jun 2, 2017 at 1:43 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Fri, Jun 2, 2017 at 1:40 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>>> On 06/02/2017 10:19 PM, H.J. Lu wrote:
>>>>>>>> On Fri, Jun 2, 2017 at 6:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>>>>> On 05/18/2017 10:05 PM, Siddhesh Poyarekar wrote:
>>>>>>>>>> Why not just have a tst-resolv-res_init.c and
>>>>>>>>>> tst-resolv-res_init-thread.c? That's how a lot of the other similar
>>>>>>>>>> kinds of tests are rewritten. I don't have a very strong opinion on
>>>>>>>>>> this though, you can choose the color of your shed :)
>>>>>>>>>
>>>>>>>>> I do it this way so that I can use #if instead of #ifdef, following the
>>>>>>>>> current guidelines to trigger -Wundef warnings on typos.
>>>>>>>>>
>>>>>>>>> I'm going to push this without the tests expecting incorrect results.
>>>>>>>>>
>>>>>>>>
>>>>>>>> On Fedora 25/x86-64, I got
>>>>>>>>
>>>>>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init.out
>>>>>>>> Timed out: killed the child process
>>>>>>>> [hjl@gnu-6 build-x86_64-linux]$ cat resolv/tst-resolv-res_init-thread.out
>>>>>>>> Timed out: killed the child process
>>>>>>>> [hjl@gnu-6 build-x86_64-linux]$
>>>>>>>
>>>>>>> I see that too, with 4.11.3-200.fc25.x86_64. What's your kernel version?
>>>>>>>
>>>>>>> I don't see the delay with 4.10.17-100.fc24.x86_64. There, the poll
>>>>>>> system calls return immediately. I wonder if it's some sort of
>>>>>>> regression in network namespaces.
>>>>>>
>>>>>> Yes, I am also running 4.11.3-200.fc25.x86_64.
>>>>>>
>>>>>
>>>>> I booted 4.10.16-200.fc25.x86_64 and the test passed. Any ideas?
>>>>
>>>> I'm going to try to come up with a self-contained reproducer, and then
>>>> approach the kernel people if it still looks like a kernel bug.
>>>>
>>>> Florian
>>>
>>> Kernel regression was introduced by
>>>
>>> commit 580bdf5650fff8f66468ce491f8308f1117b7074
>>> Merge: e60a426 a249708
>>> Author: David S. Miller <davem@davemloft.net>
>>> Date: Tue Jan 17 15:19:37 2017 -0500
>>>
>>> Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
>>>
>>> But I don't know exactly which commit caused it.
>>>
>>
>> It was caused by:
>>
>> commit 44bb765cf07ab6622e6fdf4bce546b43bd20faee
>> Author: Florian Fainelli <f.fainelli@gmail.com>
>> Date: Tue Jan 10 12:32:36 2017 -0800
>>
>> net: dsa: Implement ndo_get_phys_port_name()
>>
>> Return the physical port number of a DSA created network device using
>> ndo_get_phys_port_name().
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>
>>
>
> I was wrong. It isn't the bad one.
>
This commit:
commit c0303efeab7391ec51c337e0ac5740860ad01fe7
Author: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Mon Jan 9 16:04:09 2017 +0100
net: reduce cycles spend on ICMP replies that gets rate limited
This patch split the global and per (inet)peer ICMP-reply limiter
code, and moves the global limit check to earlier in the packet
processing path. Thus, avoid spending cycles on ICMP replies that
gets limited/suppressed anyhow.
The global ICMP rate limiter icmp_global_allow() is a good solution,
it just happens too late in the process. The kernel goes through the
full route lookup (return path) for the ICMP message, before taking
the rate limit decision of not sending the ICMP reply.
Details: The kernels global rate limiter for ICMP messages got added
in commit 4cdf507d5452 ("icmp: add a global rate limitation"). It is
a token bucket limiter with a global lock. It brilliantly avoids
locking congestion by only updating when 20ms (HZ/50) were elapsed. It
can then avoids taking lock when credit is exhausted (when under
pressure) and time constraint for refill is not yet meet.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
is the culprit. This patch for kernel 4.11.3 reverts it and fixes
my problems.
--
H.J.
[-- Attachment #2: 0001-Revert-net-reduce-cycles-spend-on-ICMP-replies-that-.patch --]
[-- Type: text/x-patch, Size: 6476 bytes --]
From 4f92e60679454c35dbbd2df844ab38184baa62e9 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 3 Jun 2017 22:02:26 -0700
Subject: [PATCH] Revert "net: reduce cycles spend on ICMP replies that gets
rate limited"
This reverts commit c0303efeab7391ec51c337e0ac5740860ad01fe7, which
caused resolv/tst-resolv-res_init and resolv/tst-resolv-res_init-thread
on glibc master branch to timeout.
---
net/ipv4/icmp.c | 69 ++++++++++++++++++---------------------------------------
net/ipv6/icmp.c | 49 +++++++++++++---------------------------
2 files changed, 37 insertions(+), 81 deletions(-)
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index fc310db..d58a1bc 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -280,33 +280,6 @@ bool icmp_global_allow(void)
}
EXPORT_SYMBOL(icmp_global_allow);
-static bool icmpv4_mask_allow(struct net *net, int type, int code)
-{
- if (type > NR_ICMP_TYPES)
- return true;
-
- /* Don't limit PMTU discovery. */
- if (type == ICMP_DEST_UNREACH && code == ICMP_FRAG_NEEDED)
- return true;
-
- /* Limit if icmp type is enabled in ratemask. */
- if (!((1 << type) & net->ipv4.sysctl_icmp_ratemask))
- return true;
-
- return false;
-}
-
-static bool icmpv4_global_allow(struct net *net, int type, int code)
-{
- if (icmpv4_mask_allow(net, type, code))
- return true;
-
- if (icmp_global_allow())
- return true;
-
- return false;
-}
-
/*
* Send an ICMP frame.
*/
@@ -315,22 +288,34 @@ static bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,
struct flowi4 *fl4, int type, int code)
{
struct dst_entry *dst = &rt->dst;
- struct inet_peer *peer;
bool rc = true;
- int vif;
- if (icmpv4_mask_allow(net, type, code))
+ if (type > NR_ICMP_TYPES)
+ goto out;
+
+ /* Don't limit PMTU discovery. */
+ if (type == ICMP_DEST_UNREACH && code == ICMP_FRAG_NEEDED)
goto out;
/* No rate limit on loopback */
if (dst->dev && (dst->dev->flags&IFF_LOOPBACK))
goto out;
- vif = l3mdev_master_ifindex(dst->dev);
- peer = inet_getpeer_v4(net->ipv4.peers, fl4->daddr, vif, 1);
- rc = inet_peer_xrlim_allow(peer, net->ipv4.sysctl_icmp_ratelimit);
- if (peer)
- inet_putpeer(peer);
+ /* Limit if icmp type is enabled in ratemask. */
+ if (!((1 << type) & net->ipv4.sysctl_icmp_ratemask))
+ goto out;
+
+ rc = false;
+ if (icmp_global_allow()) {
+ int vif = l3mdev_master_ifindex(dst->dev);
+ struct inet_peer *peer;
+
+ peer = inet_getpeer_v4(net->ipv4.peers, fl4->daddr, vif, 1);
+ rc = inet_peer_xrlim_allow(peer,
+ net->ipv4.sysctl_icmp_ratelimit);
+ if (peer)
+ inet_putpeer(peer);
+ }
out:
return rc;
}
@@ -409,8 +394,6 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
struct inet_sock *inet;
__be32 daddr, saddr;
u32 mark = IP4_REPLY_MARK(net, skb->mark);
- int type = icmp_param->data.icmph.type;
- int code = icmp_param->data.icmph.code;
if (ip_options_echo(&icmp_param->replyopts.opt.opt, skb))
return;
@@ -418,10 +401,6 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
/* Needed by both icmp_global_allow and icmp_xmit_lock */
local_bh_disable();
- /* global icmp_msgs_per_sec */
- if (!icmpv4_global_allow(net, type, code))
- goto out_bh_enable;
-
sk = icmp_xmit_lock(net);
if (!sk)
goto out_bh_enable;
@@ -455,7 +434,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
rt = ip_route_output_key(net, &fl4);
if (IS_ERR(rt))
goto out_unlock;
- if (icmpv4_xrlim_allow(net, rt, &fl4, type, code))
+ if (icmpv4_xrlim_allow(net, rt, &fl4, icmp_param->data.icmph.type,
+ icmp_param->data.icmph.code))
icmp_push_reply(icmp_param, &fl4, &ipc, &rt);
ip_rt_put(rt);
out_unlock:
@@ -674,10 +654,6 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
/* Needed by both icmp_global_allow and icmp_xmit_lock */
local_bh_disable();
- /* Check global sysctl_icmp_msgs_per_sec ratelimit */
- if (!icmpv4_global_allow(net, type, code))
- goto out_bh_enable;
-
sk = icmp_xmit_lock(net);
if (!sk)
goto out_bh_enable;
@@ -734,7 +710,6 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
if (IS_ERR(rt))
goto out_unlock;
- /* peer icmp_ratelimit */
if (!icmpv4_xrlim_allow(net, rt, &fl4, type, code))
goto ende;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 230b5aa..f480c85 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -166,30 +166,6 @@ static bool is_ineligible(const struct sk_buff *skb)
return false;
}
-static bool icmpv6_mask_allow(int type)
-{
- /* Informational messages are not limited. */
- if (type & ICMPV6_INFOMSG_MASK)
- return true;
-
- /* Do not limit pmtu discovery, it would break it. */
- if (type == ICMPV6_PKT_TOOBIG)
- return true;
-
- return false;
-}
-
-static bool icmpv6_global_allow(int type)
-{
- if (icmpv6_mask_allow(type))
- return true;
-
- if (icmp_global_allow())
- return true;
-
- return false;
-}
-
/*
* Check the ICMP output rate limit
*/
@@ -200,7 +176,12 @@ static bool icmpv6_xrlim_allow(struct sock *sk, u8 type,
struct dst_entry *dst;
bool res = false;
- if (icmpv6_mask_allow(type))
+ /* Informational messages are not limited. */
+ if (type & ICMPV6_INFOMSG_MASK)
+ return true;
+
+ /* Do not limit pmtu discovery, it would break it. */
+ if (type == ICMPV6_PKT_TOOBIG)
return true;
/*
@@ -217,16 +198,20 @@ static bool icmpv6_xrlim_allow(struct sock *sk, u8 type,
} else {
struct rt6_info *rt = (struct rt6_info *)dst;
int tmo = net->ipv6.sysctl.icmpv6_time;
- struct inet_peer *peer;
/* Give more bandwidth to wider prefixes. */
if (rt->rt6i_dst.plen < 128)
tmo >>= ((128 - rt->rt6i_dst.plen)>>5);
- peer = inet_getpeer_v6(net->ipv6.peers, &fl6->daddr, 1);
- res = inet_peer_xrlim_allow(peer, tmo);
- if (peer)
- inet_putpeer(peer);
+ if (icmp_global_allow()) {
+ struct inet_peer *peer;
+
+ peer = inet_getpeer_v6(net->ipv6.peers,
+ &fl6->daddr, 1);
+ res = inet_peer_xrlim_allow(peer, tmo);
+ if (peer)
+ inet_putpeer(peer);
+ }
}
dst_release(dst);
return res;
@@ -490,10 +475,6 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
/* Needed by both icmp_global_allow and icmpv6_xmit_lock */
local_bh_disable();
- /* Check global sysctl_icmp_msgs_per_sec ratelimit */
- if (!icmpv6_global_allow(type))
- goto out_bh_enable;
-
mip6_addr_swap(skb);
memset(&fl6, 0, sizeof(fl6));
--
2.9.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-04 5:10 ` H.J. Lu
@ 2017-06-04 5:39 ` Florian Weimer
2017-06-04 15:21 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: Florian Weimer @ 2017-06-04 5:39 UTC (permalink / raw)
To: H.J. Lu; +Cc: Siddhesh Poyarekar, GNU C Library
On 06/04/2017 07:10 AM, H.J. Lu wrote:
> This commit:
>
> commit c0303efeab7391ec51c337e0ac5740860ad01fe7
> Author: Jesper Dangaard Brouer <brouer@redhat.com>
> Date: Mon Jan 9 16:04:09 2017 +0100
>
> net: reduce cycles spend on ICMP replies that gets rate limited
>
> This patch split the global and per (inet)peer ICMP-reply limiter
> code, and moves the global limit check to earlier in the packet
> processing path. Thus, avoid spending cycles on ICMP replies that
> gets limited/suppressed anyhow.
>
> The global ICMP rate limiter icmp_global_allow() is a good solution,
> it just happens too late in the process. The kernel goes through the
> full route lookup (return path) for the ICMP message, before taking
> the rate limit decision of not sending the ICMP reply.
>
> Details: The kernels global rate limiter for ICMP messages got added
> in commit 4cdf507d5452 ("icmp: add a global rate limitation"). It is
> a token bucket limiter with a global lock. It brilliantly avoids
> locking congestion by only updating when 20ms (HZ/50) were elapsed. It
> can then avoids taking lock when credit is exhausted (when under
> pressure) and time constraint for refill is not yet meet.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Acked-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> is the culprit. This patch for kernel 4.11.3 reverts it and fixes
> my problems.
Makes sense. There does not seem to be any check left that localhost
responses aren't rate-limited. Furthermore, the rate limit is not
namespace-specific, so the network namespace used in the test doesn't
provide any isolation.
I think I'll have enough information to write a reduced test case, and
I'll approach the kernel developers to get this fixed upstream.
Thanks,
Florian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-04 5:39 ` Florian Weimer
@ 2017-06-04 15:21 ` H.J. Lu
2017-06-06 11:50 ` Florian Weimer
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-06-04 15:21 UTC (permalink / raw)
To: Florian Weimer; +Cc: Siddhesh Poyarekar, GNU C Library
On Sat, Jun 3, 2017 at 10:39 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/04/2017 07:10 AM, H.J. Lu wrote:
>
>> This commit:
>>
>> commit c0303efeab7391ec51c337e0ac5740860ad01fe7
>> Author: Jesper Dangaard Brouer <brouer@redhat.com>
>> Date: Mon Jan 9 16:04:09 2017 +0100
>>
>> net: reduce cycles spend on ICMP replies that gets rate limited
>>
>> This patch split the global and per (inet)peer ICMP-reply limiter
>> code, and moves the global limit check to earlier in the packet
>> processing path. Thus, avoid spending cycles on ICMP replies that
>> gets limited/suppressed anyhow.
>>
>> The global ICMP rate limiter icmp_global_allow() is a good solution,
>> it just happens too late in the process. The kernel goes through the
>> full route lookup (return path) for the ICMP message, before taking
>> the rate limit decision of not sending the ICMP reply.
>>
>> Details: The kernels global rate limiter for ICMP messages got added
>> in commit 4cdf507d5452 ("icmp: add a global rate limitation"). It is
>> a token bucket limiter with a global lock. It brilliantly avoids
>> locking congestion by only updating when 20ms (HZ/50) were elapsed. It
>> can then avoids taking lock when credit is exhausted (when under
>> pressure) and time constraint for refill is not yet meet.
>>
>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>> Acked-by: Eric Dumazet <edumazet@google.com>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>
>> is the culprit. This patch for kernel 4.11.3 reverts it and fixes
>> my problems.
>
> Makes sense. There does not seem to be any check left that localhost
> responses aren't rate-limited. Furthermore, the rate limit is not
> namespace-specific, so the network namespace used in the test doesn't
> provide any isolation.
>
> I think I'll have enough information to write a reduced test case, and
> I'll approach the kernel developers to get this fixed upstream.
>
I opened a Fedora 25 bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1458586
Hopefully, I will be notified when it is fixed in Fedora 25.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Tests for res_init
2017-06-04 15:21 ` H.J. Lu
@ 2017-06-06 11:50 ` Florian Weimer
0 siblings, 0 replies; 20+ messages in thread
From: Florian Weimer @ 2017-06-06 11:50 UTC (permalink / raw)
To: H.J. Lu; +Cc: Siddhesh Poyarekar, GNU C Library
On 06/04/2017 05:21 PM, H.J. Lu wrote:
> I opened a Fedora 25 bug:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1458586
I had already filed:
https://bugzilla.redhat.com/show_bug.cgi?id=1458542
Upstream discussion continues there:
http://marc.info/?l=linux-netdev&m=149656032817085
We'll see how it goes. I'm fine with the source interface hack from a
purely behavioral POV.
Thanks,
Florian
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-06-06 11:50 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-09 12:04 [PATCH] Tests for res_init Florian Weimer
2017-05-18 18:33 ` Siddhesh Poyarekar
2017-05-18 18:37 ` Florian Weimer
2017-05-18 19:09 ` Siddhesh Poyarekar
2017-05-18 19:15 ` Florian Weimer
2017-05-18 20:05 ` Siddhesh Poyarekar
2017-06-02 13:49 ` Florian Weimer
2017-06-02 20:19 ` H.J. Lu
2017-06-02 20:40 ` Florian Weimer
2017-06-02 20:43 ` H.J. Lu
2017-06-03 2:06 ` H.J. Lu
2017-06-03 6:43 ` Florian Weimer
2017-06-03 15:00 ` H.J. Lu
2017-06-04 0:56 ` H.J. Lu
2017-06-04 1:46 ` H.J. Lu
2017-06-04 5:10 ` H.J. Lu
2017-06-04 5:39 ` Florian Weimer
2017-06-04 15:21 ` H.J. Lu
2017-06-06 11:50 ` Florian Weimer
2017-06-03 15:38 ` Andreas Schwab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).