Re: [PATCH] conf: Don't warn if nameservers were found, but won't be advertised

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 8 Mar 2024 12:17:13 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Mar 08, 2024 at 12:25:51AM +0100, Stefano Brivio wrote:
> > Starting from commit 3a2afde87dd1 ("conf, udp: Drop mostly duplicated
> > dns_send arrays, rename related fields"), we won't add to c->ip4.dns
> > and c->ip6.dns nameservers that can't be used by the guest or
> > container, and we won't advertise them.
> > 
> > However, the fact that we don't advertise any nameserver doesn't mean
> > that we didn't find any, and we should warn only if we couldn't find
> > any.
> > 
> > This is particularly relevant in case both --dns-forward and
> > --no-map-gw are passed, and a single loopback address is listed in
> > /etc/resolv.conf: we'll forward queries directed to the address
> > specified by --dns-forward to the loopback address we found, we
> > won't advertise that address, so we shouldn't warn: this is a
> > perfectly legitimate usage.
> > 
> > Reported-by: Paul Holzinger <pholzing@redhat.com>
> > Link: https://github.com/containers/podman/issues/19213
> > Fixes: 3a2afde87dd1 ("conf, udp: Drop mostly duplicated dns_send arrays, rename related fields")
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> 
> I don't think this is quite the right fix.  It makes sense *when*
> --dns-forward is specified.  However if --dns-forward is *not*
> specified, then having only localhost resolvers on the host side means
> we really do have nothing the guest can use.  So I think we need to
> make the behaviour explicitly conditional on the dns_match variable.

I was actually about to do that, then I read the text of the warning
again: "Couldn't get any nameserver address".

If there are just loopback addresses in resolv.conf, and we don't have
--dns-forward, is that claim correct? We could get them, we actually
parse them, we just don't advertise them. At the same time, we show the
user (at least without --quiet) that we don't advertise any server via
DHCP/NDP/DHCPv6: that section will be missing.

On the other hand, I guess there might be some value in giving the user
a hint if they just see name resolution failing. Maybe, if we don't use
any nameserver from resolv.conf (or from the command line), we could
say "Couldn't use any nameserver address"?

> Possibly by making add_dns[46]() accept localhost addresses if
> (dns_match && no_map_gw)?

What do you mean by "accept"? It already sets .dns_host, no matter
what. I don't think we should add loopback addresses to the list we
advertise if c->no_map_gw, because they can't be reached anyway.

Another alternative would be to automatically advertise the address
passed by --dns-forward. But the user can already specify that via
--dns, so we'd be actually losing functionality.

I was rather pondering to set .dns_host from add_dns[46]() iff it's
used (that is, if !IN6_IS_ADDR_UNSPECIFIED(&c->ip[46].dns_match) and
return some value there (maybe that's what you meant by "accept")?

Then, if any call to add_dns[46]() used any address (advertised or
mapped), we wouldn't print any warning.

I'm a bit undecided, because we'd make it more complicated for the sake
of a warning that doesn't really need to be printed anyway. But again,
it might be helpful.

-- 
Stefano