Date: Sun, 21 Aug 2005 00:37:56 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: Hajimu UMEMOTO <ume@FreeBSD.org> Cc: arch@FreeBSD.org Subject: Re: [CFR] reflect resolv.conf update to running application Message-ID: <20050821003536.P14178@fledge.watson.org> In-Reply-To: <ygefyt4yiaz.wl%ume@mahoroba.org> References: <ygefyt4yiaz.wl%ume@mahoroba.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 20 Aug 2005, Hajimu UMEMOTO wrote:
> Our resolver reads resolv.conf once, and never re-read it. Recent
> OpenBSD changed to re-read resolv.conf when it is updated. I believe it
> is useful specially for mobile environment. So, I made a patch for our
> resolver. Please review it.
Two concerns:
(1) Has anyone characterized the significant of the cost of doing a stat()
for every DNS lookup for a significant workload? Does it matter?
Most performance-critical network paths don't do name lookups in order
to prevent indefinite stalls in lookup, but doing, say, 1,000
additional stats a second is not a small issue.
(2) By reading the configuration file more frequently and more quickly
after a change, we increase the chances of a race condition in which
the resolve reads a partially written resolv.conf file during an
update. Does this happen in practice? I've always been very leery of
re-reading configuration files automatically based on a time-stamp, as
updates to files are not atomic at all.
Robert N M Watson
>
> Index: lib/libc/net/getaddrinfo.c
> diff -u -p lib/libc/net/getaddrinfo.c.orig lib/libc/net/getaddrinfo.c
> --- lib/libc/net/getaddrinfo.c.orig Sat May 21 22:46:37 2005
> +++ lib/libc/net/getaddrinfo.c Sat May 21 23:20:27 2005
> @@ -2443,7 +2443,7 @@ res_searchN(name, target)
> int got_nodata = 0, got_servfail = 0, tried_as_is = 0;
> char abuf[MAXDNAME];
>
> - if ((_res.options & RES_INIT) == 0 && res_init() == -1) {
> + if (_res_init() == -1) {
> h_errno = NETDB_INTERNAL;
> return (-1);
> }
> Index: lib/libc/net/gethostbydns.c
> diff -u -p lib/libc/net/gethostbydns.c.orig lib/libc/net/gethostbydns.c
> --- lib/libc/net/gethostbydns.c.orig Sat May 21 22:46:37 2005
> +++ lib/libc/net/gethostbydns.c Sat May 21 23:20:27 2005
> @@ -538,10 +538,6 @@ _dns_gethostbyaddr(void *rval, void *cb_
> he = va_arg(ap, struct hostent *);
> hed = va_arg(ap, struct hostent_data *);
>
> - if ((_res.options & RES_INIT) == 0 && res_init() == -1) {
> - h_errno = NETDB_INTERNAL;
> - return NS_UNAVAIL;
> - }
> switch (af) {
> case AF_INET:
> (void) sprintf(qbuf, "%u.%u.%u.%u.in-addr.arpa",
> Index: lib/libc/net/gethostnamadr.c
> diff -u -p lib/libc/net/gethostnamadr.c.orig lib/libc/net/gethostnamadr.c
> --- lib/libc/net/gethostnamadr.c.orig Sat May 21 22:46:37 2005
> +++ lib/libc/net/gethostnamadr.c Sat May 21 23:20:27 2005
> @@ -44,6 +44,7 @@ __FBSDID("$FreeBSD: src/lib/libc/net/get
> #include <resolv.h> /* XXX hack for _res */
> #include "un-namespace.h"
> #include "netdb_private.h"
> +#include "res_config.h"
>
> extern int _ht_gethostbyname(void *, void *, va_list);
> extern int _dns_gethostbyname(void *, void *, va_list);
> @@ -264,7 +265,7 @@ gethostbyaddr_r(const char *addr, int le
> { 0 }
> };
>
> - if ((_res.options & RES_INIT) == 0 && res_init() == -1) {
> + if (_res_init() == -1) {
> h_errno = NETDB_INTERNAL;
> return -1;
> }
> Index: lib/libc/net/getnetbydns.c
> diff -u -p lib/libc/net/getnetbydns.c.orig lib/libc/net/getnetbydns.c
> --- lib/libc/net/getnetbydns.c.orig Sat May 21 22:46:37 2005
> +++ lib/libc/net/getnetbydns.c Sat May 21 23:20:27 2005
> @@ -304,6 +304,10 @@ _dns_getnetbyaddr(void *rval, void *cb_d
> netbr[1], netbr[0]);
> break;
> }
> + if (_res_init() == -1) {
> + h_errno = NETDB_INTERNAL;
> + return NS_UNAVAIL;
> + }
> if ((buf = malloc(sizeof(*buf))) == NULL) {
> h_errno = NETDB_INTERNAL;
> return NS_NOTFOUND;
> Index: lib/libc/net/name6.c
> diff -u -p lib/libc/net/name6.c.orig lib/libc/net/name6.c
> --- lib/libc/net/name6.c.orig Sat May 21 22:46:38 2005
> +++ lib/libc/net/name6.c Sat May 21 23:20:27 2005
> @@ -121,6 +121,7 @@ __FBSDID("$FreeBSD: src/lib/libc/net/nam
> #include <unistd.h>
> #include "un-namespace.h"
> #include "netdb_private.h"
> +#include "res_config.h"
>
> #ifndef _PATH_HOSTS
> #define _PATH_HOSTS "/etc/hosts"
> @@ -1810,11 +1811,9 @@ _dns_ghbyaddr(void *rval, void *cb_data,
> return NS_NOTFOUND;
> }
>
> - if ((_res.options & RES_INIT) == 0) {
> - if (res_init() < 0) {
> - *errp = h_errno;
> - return NS_UNAVAIL;
> - }
> + if (_res_init() < 0) {
> + *errp = h_errno;
> + return NS_UNAVAIL;
> }
> memset(&hbuf, 0, sizeof(hbuf));
> hbuf.h_name = NULL;
> Index: lib/libc/net/res_config.h
> diff -u lib/libc/net/res_config.h.orig lib/libc/net/res_config.h
> --- lib/libc/net/res_config.h.orig Sat Mar 23 08:41:54 2002
> +++ lib/libc/net/res_config.h Sat May 21 23:20:27 2005
> @@ -8,3 +8,5 @@
> #define MULTI_PTRS_ARE_ALIASES 1 /* fold multiple PTR records into aliases */
> #define CHECK_SRVR_ADDR 1 /* confirm that the server requested sent the reply */
> #define BIND_UPDATE 1 /* update support */
> +
> +int _res_init(void);
> Index: lib/libc/net/res_init.c
> diff -u -p lib/libc/net/res_init.c.orig lib/libc/net/res_init.c
> --- lib/libc/net/res_init.c.orig Thu Feb 26 06:03:45 2004
> +++ lib/libc/net/res_init.c Sat May 21 23:20:27 2005
> @@ -78,11 +78,13 @@ __FBSDID("$FreeBSD: src/lib/libc/net/res
> #include <sys/types.h>
> #include <sys/param.h>
> #include <sys/socket.h>
> +#include <sys/stat.h>
> #include <sys/time.h>
> #include <netinet/in.h>
> #include <arpa/inet.h>
> #include <arpa/nameser.h>
> #include <ctype.h>
> +#include <errno.h>
> #include <limits.h>
> #include <resolv.h>
> #include <stdio.h>
> @@ -100,6 +102,7 @@ __FBSDID("$FreeBSD: src/lib/libc/net/res
> #undef h_errno
> extern int h_errno;
>
> +static void res_readconf(void);
> static void res_setoptions(char *, char *);
>
> #ifdef RESOLVSORT
> @@ -112,6 +115,18 @@ static u_int32_t net_mask(struct in_addr
> # define isascii(c) (!(c & 0200))
> #endif
>
> +#define timespecclear(tvp) ((tvp)->tv_sec = (tvp)->tv_nsec = 0)
> +#define timespeccmp(tvp, uvp, cmp) \
> + (((tvp)->tv_sec == (uvp)->tv_sec) ? \
> + ((tvp)->tv_nsec cmp (uvp)->tv_nsec) : \
> + ((tvp)->tv_sec cmp (uvp)->tv_sec))
> +
> +struct __res_conf_private {
> + struct timespec mtimespec;
> +};
> +
> +static struct __res_conf_private *___res_conf_private(void);
> +
> /*
> * Check structure for failed per-thread allocations.
> */
> @@ -119,6 +134,7 @@ static struct res_per_thread {
> struct __res_state res_state;
> struct __res_state_ext res_state_ext;
> struct __res_send_private res_send_private;
> + struct __res_conf_private res_conf_private;
> int h_errno;
> } _res_per_thread_bogus = { .res_send_private = { .s = -1 } }; /* socket */
>
> @@ -146,21 +162,8 @@ static struct res_per_thread {
> int
> res_init()
> {
> - FILE *fp;
> struct __res_send_private *rsp;
> - char *cp, **pp;
> - int n;
> - char buf[MAXDNAME];
> - int nserv = 0; /* number of nameserver records read from file */
> - int haveenv = 0;
> - int havesearch = 0;
> -#ifdef RESOLVSORT
> - int nsort = 0;
> - char *net;
> -#endif
> -#ifndef RFC1535
> - int dots;
> -#endif
> + char *cp;
>
> /*
> * If allocation of memory for this thread's resolver has failed,
> @@ -208,6 +211,37 @@ res_init()
> if (!_res.id)
> _res.id = res_randomid();
>
> + _res.pfcode = 0;
> + res_readconf();
> +
> + if (issetugid())
> + _res.options |= RES_NOALIASES;
> + else if ((cp = getenv("RES_OPTIONS")) != NULL)
> + res_setoptions(cp, "env");
> + _res.options |= RES_INIT;
> + return (0);
> +}
> +
> +static void
> +res_readconf()
> +{
> + struct __res_conf_private *rcp;
> + FILE *fp;
> + char *cp, **pp;
> + int n;
> + char buf[MAXDNAME];
> + int nserv = 0; /* number of nameserver records read from file */
> + int haveenv = 0;
> + int havesearch = 0;
> +#ifdef RESOLVSORT
> + int nsort = 0;
> + char *net;
> +#endif
> +#ifndef RFC1535
> + int dots;
> +#endif
> + struct stat sb;
> +
> #ifdef USELOOPBACK
> _res.nsaddr.sin_addr = inet_makeaddr(IN_LOOPBACKNET, 1);
> #else
> @@ -220,7 +254,6 @@ res_init()
> memcpy(&_res_ext.nsaddr, &_res.nsaddr, _res.nsaddr.sin_len);
> _res.nscount = 1;
> _res.ndots = 1;
> - _res.pfcode = 0;
>
> /* Allow user to override the local domain definition */
> if (issetugid() == 0 && (cp = getenv("LOCALDOMAIN")) != NULL) {
> @@ -262,7 +295,10 @@ res_init()
> (line[sizeof(name) - 1] == ' ' || \
> line[sizeof(name) - 1] == '\t'))
>
> + rcp = ___res_conf_private();
> if ((fp = fopen(_PATH_RESCONF, "r")) != NULL) {
> + if (fstat(fileno(fp), &sb) == 0)
> + rcp->mtimespec = sb.st_mtimespec;
> /* read the config file */
> while (fgets(buf, sizeof(buf), fp) != NULL) {
> /* skip comments */
> @@ -396,11 +432,11 @@ res_init()
> if (inet_aton(net, &a)) {
> _res.sort_list[nsort].mask = a.s_addr;
> } else {
> - _res.sort_list[nsort].mask =
> + _res.sort_list[nsort].mask =
> net_mask(_res.sort_list[nsort].addr);
> }
> } else {
> - _res.sort_list[nsort].mask =
> + _res.sort_list[nsort].mask =
> net_mask(_res.sort_list[nsort].addr);
> }
> _res_ext.sort_list[nsort].af = AF_INET;
> @@ -465,13 +501,14 @@ res_init()
> continue;
> }
> }
> - if (nserv > 1)
> + if (nserv > 1)
> _res.nscount = nserv;
> #ifdef RESOLVSORT
> _res.nsort = nsort;
> #endif
> (void) fclose(fp);
> - }
> + } else
> + timespecclear(&rcp->mtimespec);
> if (_res.defdname[0] == 0 &&
> gethostname(buf, sizeof(_res.defdname) - 1) == 0 &&
> (cp = strchr(buf, '.')) != NULL)
> @@ -507,12 +544,27 @@ res_init()
> #endif
> #endif /* !RFC1535 */
> }
> +}
>
> - if (issetugid())
> - _res.options |= RES_NOALIASES;
> - else if ((cp = getenv("RES_OPTIONS")) != NULL)
> - res_setoptions(cp, "env");
> - _res.options |= RES_INIT;
> +int
> +_res_init(void)
> +{
> + struct __res_conf_private *rcp;
> + struct stat sb;
> +
> + if ((_res.options & RES_INIT) == 0)
> + return (res_init());
> +
> + if (stat(_PATH_RESCONF, &sb) == -1) {
> + /*
> + * Lost the file, in chroot?
> + * Don' trash settings
> + */
> + return (0);
> + }
> + rcp = ___res_conf_private();
> + if (timespeccmp(&sb.st_mtimespec, &rcp->mtimespec, !=))
> + res_readconf();
> return (0);
> }
>
> @@ -629,6 +681,7 @@ struct __res_state _res;
> #endif
> struct __res_state_ext _res_ext;
> static struct __res_send_private _res_send_private = { .s = -1 }; /* socket */
> +static struct __res_conf_private _res_conf_private;
>
> static thread_key_t res_key;
> static once_t res_init_once = ONCE_INITIALIZER;
> @@ -697,6 +750,14 @@ ___res_send_private(void)
> if (thr_main() != 0)
> return (&_res_send_private);
> return (&allocate_res()->res_send_private);
> +}
> +
> +struct __res_conf_private *
> +___res_conf_private(void)
> +{
> + if (thr_main() != 0)
> + return (&_res_conf_private);
> + return (&allocate_res()->res_conf_private);
> }
>
> int *
> Index: lib/libc/net/res_query.c
> diff -u -p lib/libc/net/res_query.c.orig lib/libc/net/res_query.c
> --- lib/libc/net/res_query.c.orig Sat May 21 22:46:39 2005
> +++ lib/libc/net/res_query.c Sat May 21 23:20:27 2005
> @@ -200,7 +200,7 @@ res_search(name, class, type, answer, an
> int trailing_dot, ret, saved_herrno;
> int got_nodata = 0, got_servfail = 0, tried_as_is = 0;
>
> - if ((_res.options & RES_INIT) == 0 && res_init() == -1) {
> + if (_res_init() == -1) {
> h_errno = NETDB_INTERNAL;
> return (-1);
> }
> Index: lib/libc/net/res_update.c
> diff -u -p lib/libc/net/res_update.c.orig lib/libc/net/res_update.c
> --- lib/libc/net/res_update.c.orig Mon Sep 16 01:51:09 2002
> +++ lib/libc/net/res_update.c Sat May 21 23:20:27 2005
> @@ -36,6 +36,8 @@ __FBSDID("$FreeBSD: src/lib/libc/net/res
> #include <stdlib.h>
> #include <string.h>
>
> +#include "res_config.h"
> +
> /*
> * Separate a linked list of records into groups so that all records
> * in a group will belong to a single zone on the nameserver.
> @@ -84,7 +86,7 @@ res_update(ns_updrec *rrecp_in) {
> u_int16_t dlen, class, qclass, type, qtype;
> u_int32_t ttl;
>
> - if ((_res.options & RES_INIT) == 0 && res_init() == -1) {
> + if (_res_init() == -1) {
> h_errno = NETDB_INTERNAL;
> return (-1);
> }
>
>
> Sincerely,
>
> --
> Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
> ume@mahoroba.org ume@{,jp.}FreeBSD.org
> http://www.imasy.org/~ume/
> _______________________________________________
> freebsd-arch@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050821003536.P14178>