From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 4 Jan 2023 15:36:15 +0000 (+0100)
Subject: resolvectl: don't filter loopback DNS server from global DNS server list
X-Git-Tag: v253-rc1~187^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=889a1b9f4e799b31f1be06db74708aa8beb70829;p=thirdparty%2Fsystemd.git

resolvectl: don't filter loopback DNS server from global DNS server list

"resolvectl status" shows per-link DNS servers separately from global
ones. When querying the global list, it will contain both per-link and
global servers however. Thus, to not show duplicate info we filter all
entries that actually have a non-zero ifindex set (under the assumption
that that's a per-link server).

This doesn't work if people configured 127.0.0.1 as global server
though, as we'll add ifindex 1 to it since
6e32414a66ff8dbcef233981a7066684d903ee9f unconditionally even for global
servers.

Let's address that by excluding entries with ifindex 1 from suppression.
This is safe as resolved ignores loopback ifaces, hence never will have
per-link servers on ifindex 1.

Note that this splits up the "with_ifindex" parameter into a second
parameter "only_global", since they semantically do two different
things. One controls whether we shall expect/parse an ifindex dbus
field. The other controls whether we shall filter all ifindex values set
!= 0. These are effectively always used in conjunction hence making them
the same actually worked. However this is utterly confusing I think,
which as I guess is resulting in the confusion around #25796 (which
removes the whole check)

Replaces: #25796
---

diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c
index d92ef1646fc..682fe87ba92 100644
--- a/src/resolve/resolvectl.c
+++ b/src/resolve/resolvectl.c
@@ -1206,7 +1206,13 @@ static int reset_server_features(int argc, char **argv, void *userdata) {
         return 0;
 }
 
-static int read_dns_server_one(sd_bus_message *m, bool with_ifindex, bool extended, char **ret) {
+static int read_dns_server_one(
+                sd_bus_message *m,
+                bool with_ifindex,  /* read "ifindex" reply that also carries an interface index */
+                bool extended,      /* read "extended" reply, i.e. with port number and server name */
+                bool only_global,   /* suppress entries with an (non-loopback) ifindex set (i.e. which are specific to some interface) */
+                char **ret) {
+
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         _cleanup_free_ char *pretty = NULL;
         union in_addr_union a;
@@ -1256,8 +1262,8 @@ static int read_dns_server_one(sd_bus_message *m, bool with_ifindex, bool extend
                 return 1;
         }
 
-        if (with_ifindex && ifindex != 0) {
-                /* only show the global ones here */
+        if (only_global && ifindex > 0 && ifindex != LOOPBACK_IFINDEX) {
+                /* This one has an (non-loopback) ifindex set, and we were told to suppress those. Hence do so. */
                 *ret = NULL;
                 return 1;
         }
@@ -1285,7 +1291,7 @@ static int map_link_dns_servers_internal(sd_bus *bus, const char *member, sd_bus
         for (;;) {
                 _cleanup_free_ char *pretty = NULL;
 
-                r = read_dns_server_one(m, false, extended, &pretty);
+                r = read_dns_server_one(m, /* with_ifindex= */ false, extended, /* only_global= */ false, &pretty);
                 if (r < 0)
                         return r;
                 if (r == 0)
@@ -1318,14 +1324,14 @@ static int map_link_current_dns_server(sd_bus *bus, const char *member, sd_bus_m
         assert(m);
         assert(userdata);
 
-        return read_dns_server_one(m, false, false, userdata);
+        return read_dns_server_one(m, /* with_ifindex= */ false, /* extended= */ false, /* only_global= */ false, userdata);
 }
 
 static int map_link_current_dns_server_ex(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) {
         assert(m);
         assert(userdata);
 
-        return read_dns_server_one(m, false, true, userdata);
+        return read_dns_server_one(m, /* with_ifindex= */ false, /* extended= */ true, /* only_global= */ false, userdata);
 }
 
 static int read_domain_one(sd_bus_message *m, bool with_ifindex, char **ret) {
@@ -1755,7 +1761,7 @@ static int map_global_dns_servers_internal(
         for (;;) {
                 _cleanup_free_ char *pretty = NULL;
 
-                r = read_dns_server_one(m, true, extended, &pretty);
+                r = read_dns_server_one(m, /* with_ifindex= */ true, extended, /* only_global= */ true, &pretty);
                 if (r < 0)
                         return r;
                 if (r == 0)
@@ -1785,17 +1791,11 @@ static int map_global_dns_servers_ex(sd_bus *bus, const char *member, sd_bus_mes
 }
 
 static int map_global_current_dns_server(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) {
-        assert(m);
-        assert(userdata);
-
-        return read_dns_server_one(m, true, false, userdata);
+        return read_dns_server_one(m, /* with_ifindex= */ true, /* extended= */ false, /* only_global= */ true, userdata);
 }
 
 static int map_global_current_dns_server_ex(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) {
-        assert(m);
-        assert(userdata);
-
-        return read_dns_server_one(m, true, true, userdata);
+        return read_dns_server_one(m, /* with_ifindex= */ true, /* extended= */ true, /* only_global= */ true, userdata);
 }
 
 static int map_global_domains(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) {