On Sun, Jan 24, 2021 at 07:24:07PM +0100, Florian Obser wrote:
> On Sun, Jan 24, 2021 at 01:06:31PM +0100, Klemens Nanni wrote:
> > On Sun, Jan 24, 2021 at 12:52:50PM +0100, Theo Buehler wrote:
> > > Probably better to sync first with the corresponding unbound commit
> > > https://cvsweb.openbsd.org/src/usr.sbin/unbound/services/outside_network.c#rev1.21
> > > then adjust udp_connect_needs_log() as needed.
> > Good call, thanks.
> >
> > Here's the combined diff that syncs with unbound and adds EADDRNOTAVAIL
> > in the same fashion.
> >
> > In case that is OK, I'd commit sync and addition separately.
> >
> > Feedback? OK?
>
> I consider the libunbound directory off-limits, it is not a fork and
> it should not be a fork.
>
> We currently carry two diffs in there and I want to get rid of them
> (see at the end).
>
> Especialy the local syslog one is stupid. I think we are doing
> something wrong or libunbound does something wrong, not sure.
>
> Anyway, I think what we should actually do is disable logging in
> libunbound unless we crank logging to debug in unwind. There is
> nothing interesting comming out of libunbound for us.
>
> This leaves the 2nd problem, why is it going off the rails so badly if
> there is no v4? Is it also doing this for v6 when that's not present?
> Try resolving dnssec-failed.org, it's delegated to comcast which uses
> 5 nameservers all with v4 and v6 addresses. Since dnssec validation is
> intentionally broken unwind tries to talk to *all* nameservers, the
> log does not stop scrolling. It's so bad it actually times out on IPv6
> only.
(lib)unbound is very aggressive in finding a answer that validates. If
a reply fails validation, it will continue to recurse to see if it can
find a valid one. Other recursors use different apporoaches, e.g.
PowerDNS recursor cuts of the recursion if it finds an answer, even
if fails validation. I'd say if some of your nameservers provide
non-validating answers, fix them.
-Otto
>
> Is this a bug in (lib)unbound or is it doing what it's supposed to do?
>
> It is possible that from the point of view of (lib)unbound it is
> behaving perfectly fine, if you don't have an address family you are
> supposed to hit the available button in unbound.conf.
>
> If that is the case we should do the same, automatically. I mean
> that's the whole point of unwind, figure out what works and use that.
> Meaning if we detect that we don't have working IPv4 set use-ipv4: no.
>
> commit d273f78b8643bdb01f621260eb323123b774e431
> Author: florian <florian>
> Date: Fri Dec 6 13:08:48 2019 +0000
>
> Stop fiddling with openlog / closelog in libunbound. unwind handles
> this. We need to find a way to properly upstream this.
> OK otto
>
> diff --git libunbound/util/log.c libunbound/util/log.c
> index dfbb2334994..e8e987963c5 100644
> --- libunbound/util/log.c
> +++ libunbound/util/log.c
> @@ -109,16 +109,20 @@ log_init(const char* filename, int use_syslog, const
> char* chrootdir)
> fclose(cl);
> }
> #ifdef HAVE_SYSLOG_H
> +#if 0 /* unwind handles syslog for us */
> if(logging_to_syslog) {
> closelog();
> logging_to_syslog = 0;
> }
> +#endif
> if(use_syslog) {
> /* do not delay opening until first write, because we may
> * chroot and no longer be able to access dev/log and so on */
> /* the facility is LOG_DAEMON by default, but
> * --with-syslog-facility=LOCAL[0-7] can override it */
> +#if 0 /* unwind handles syslog for us */
> openlog(ident, LOG_NDELAY, UB_SYSLOG_FACILITY);
> +#endif
> logging_to_syslog = 1;
> lock_basic_unlock(&log_lock);
> return;
>
> commit 81b0d744ff77e26ea69cee28aed10081d3973fe8
> Author: otto <otto>
> Date: Sat Dec 14 19:56:26 2019 +0000
>
> Be less aggressive pre-allocating memory; ok florian@
>
> diff --git libunbound/util/alloc.c libunbound/util/alloc.c
> index 7e9618931ca..e9613b10dcd 100644
> --- libunbound/util/alloc.c
> +++ libunbound/util/alloc.c
> @@ -113,7 +113,7 @@ alloc_init(struct alloc_cache* alloc, struct alloc_cache*
> super,
> alloc->last_id -= 1; /* for compiler portability. */
> alloc->last_id |= alloc->next_id;
> alloc->next_id += 1; /* because id=0 is special. */
> - alloc->max_reg_blocks = 100;
> + alloc->max_reg_blocks = 10;
> alloc->num_reg_blocks = 0;
> alloc->reg_list = NULL;
> alloc->cleanup = NULL;
>
>
> >
> >
> > Index: libunbound/services/outside_network.c
> > ===================================================================
> > RCS file: /cvs/src/sbin/unwind/libunbound/services/outside_network.c,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 outside_network.c
> > --- libunbound/services/outside_network.c 11 Dec 2020 12:21:40 -0000
> > 1.9
> > +++ libunbound/services/outside_network.c 24 Jan 2021 12:03:38 -0000
> > @@ -1745,6 +1745,36 @@ select_id(struct outside_network* outnet
> > return 1;
> > }
> >
> > +/** return true is UDP connect error needs to be logged */
> > +static int udp_connect_needs_log(int err)
> > +{
> > + switch(err) {
> > + case ECONNREFUSED:
> > +# ifdef ENETUNREACH
> > + case ENETUNREACH:
> > +# endif
> > +# ifdef EHOSTDOWN
> > + case EHOSTDOWN:
> > +# endif
> > +# ifdef EHOSTUNREACH
> > + case EHOSTUNREACH:
> > +# endif
> > +# ifdef ENETDOWN
> > + case ENETDOWN:
> > +# endif
> > +# ifdef EADDRNOTAVAIL
> > + case EADDRNOTAVAIL:
> > +# endif
> > + if(verbosity >= VERB_ALGO)
> > + return 1;
> > + return 0;
> > + default:
> > + break;
> > + }
> > + return 1;
> > +}
> > +
> > +
> > /** Select random interface and port */
> > static int
> > select_ifport(struct outside_network* outnet, struct pending* pend,
> > @@ -1804,9 +1834,11 @@ select_ifport(struct outside_network* ou
> > /* connect() to the destination */
> > if(connect(fd, (struct sockaddr*)&pend->addr,
> > pend->addrlen) < 0) {
> > - log_err_addr("udp connect failed",
> > - strerror(errno), &pend->addr,
> > - pend->addrlen);
> > + if(udp_connect_needs_log(errno)) {
> > + log_err_addr("udp connect
> > failed",
> > + strerror(errno),
> > &pend->addr,
> > + pend->addrlen);
> > + }
> > sock_close(fd);
> > return 0;
> > }
> >
>
> --
> I'm not entirely sure you are real.
>
