From c9e6918257d983d1e8ed3d69420544b05b13ef85 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 4 Dec 2018 23:18:50 +0100 Subject: [PATCH 1/9] resolved: request incoming ifindex for DNS UDP packets, too This information is useful to pass back to the caller, as it tells us where we found the answer. (While we are at it, fix the socket level for the RECVERR logic) Fixes: #9778 --- src/resolve/resolved-dns-scope.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index ee204435b3..35c3804dba 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -413,9 +413,25 @@ static int dns_scope_socket( if (type == SOCK_DGRAM) { /* Set IP_RECVERR or IPV6_RECVERR to get ICMP error feedback. See discussion in #10345. */ - r = setsockopt_int(fd, SOL_IP, sa.sa.sa_family == AF_INET ? IP_RECVERR : IPV6_RECVERR, true); - if (r < 0) - return r; + + if (sa.sa.sa_family == AF_INET) { + r = setsockopt_int(fd, IPPROTO_IP, IP_RECVERR, true); + if (r < 0) + return r; + + r = setsockopt_int(fd, IPPROTO_IP, IP_PKTINFO, true); + if (r < 0) + return r; + + } else if (sa.sa.sa_family == AF_INET6) { + r = setsockopt_int(fd, IPPROTO_IPV6, IPV6_RECVERR, true); + if (r < 0) + return r; + + r = setsockopt_int(fd, IPPROTO_IPV6, IPV6_RECVPKTINFO, true); + if (r < 0) + return r; + } } if (ret_socket_address) From a77081e30f84bac7ce627d1a760ee6dc6db4354c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Dec 2018 13:33:31 +0100 Subject: [PATCH 2/9] resolvectl: rework ifname_mangle() Let's compare the ifname passed in with what is set already if there is something set already. Complain in that case. This makes commands such as "resolvectl -i foo dns bar" less weird, as we'll refuse the duplicate ifname specifications. Also, free the old arg_ifname right before assigning the new, instead of doing so in advance. --- src/resolve/resolvectl.c | 46 ++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 1e3fabc450..e6d56b0ef4 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -91,36 +91,23 @@ static int parse_ifindex_and_warn(const char *s) { int ifname_mangle(const char *s, bool allow_loopback) { _cleanup_free_ char *iface = NULL; const char *dot; - int r; + int ifi; assert(s); - if (arg_ifname) { - assert(arg_ifindex >= 0); - - if (!allow_loopback && arg_ifindex == LOOPBACK_IFINDEX) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Interface can't be the loopback interface (lo). Sorry."); - - return 1; - } - dot = strchr(s, '.'); if (dot) { - iface = strndup(s, dot - s); - if (!iface) - return log_oom(); - log_debug("Ignoring protocol specifier '%s'.", dot + 1); - } else { - iface = strdup(s); - if (!iface) - return log_oom(); - } + iface = strndup(s, dot - s); - if (parse_ifindex(iface, &r) < 0) { - r = if_nametoindex(iface); - if (r <= 0) { + } else + iface = strdup(s); + if (!iface) + return log_oom(); + + if (parse_ifindex(iface, &ifi) < 0) { + ifi = if_nametoindex(iface); + if (ifi <= 0) { if (errno == ENODEV && arg_ifindex_permissive) { log_debug("Interface '%s' not found, but -f specified, ignoring.", iface); return 0; /* done */ @@ -130,12 +117,17 @@ int ifname_mangle(const char *s, bool allow_loopback) { } } - if (!allow_loopback && r == LOOPBACK_IFINDEX) + if (arg_ifindex > 0 && arg_ifindex != ifi) { + log_error("Specified multiple different interfaces. Refusing."); + return -EINVAL; + } + + if (!allow_loopback && ifi == LOOPBACK_IFINDEX) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Interface can't be the loopback interface (lo). Sorry."); - arg_ifindex = r; - arg_ifname = TAKE_PTR(iface); + arg_ifindex = ifi; + free_and_replace(arg_ifname, iface); return 1; } @@ -2501,7 +2493,6 @@ static int compat_parse_argv(int argc, char *argv[]) { break; case 'i': - arg_ifname = mfree(arg_ifname); r = ifname_mangle(optarg, true); if (r < 0) return r; @@ -2793,7 +2784,6 @@ static int native_parse_argv(int argc, char *argv[]) { break; case 'i': - arg_ifname = mfree(arg_ifname); r = ifname_mangle(optarg, true); if (r < 0) return r; From d129304919e7ece3f5350b9198383b704733eaf1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Dec 2018 13:58:34 +0100 Subject: [PATCH 3/9] resolvectl: for our per-link verbs alternatively accept interface name in -i MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit it's a bit confusing that we take two interfaces for verbs such as "dns" or "domain": once after the verb, and once as --interface=. While there's logic behind it, let's make this least surprising: if either is specified be happy. This means "resolvectl -i foo dns" is now equivalent to "resolvectl dns foo …". Note that this is a tweak only, to minimize surprises. We don't document this alternative syntax, and shouldn't to keep things simple. --- src/resolve/resolvectl.c | 111 +++++++++++++++++++++++---------------- 1 file changed, 65 insertions(+), 46 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index e6d56b0ef4..a468cec72c 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1866,14 +1866,16 @@ static int verb_dns(int argc, char **argv, void *userdata) { assert(bus); - if (argc <= 1) + if (argc >= 2) { + r = ifname_mangle(argv[1], false); + if (r < 0) + return r; + } + + if (arg_ifindex <= 0) return status_all(bus, STATUS_DNS); - r = ifname_mangle(argv[1], false); - if (r < 0) - return r; - - if (argc == 2) + if (argc < 3) return status_ifindex(bus, arg_ifindex, NULL, STATUS_DNS, NULL); r = sd_bus_message_new_method_call( @@ -1950,14 +1952,16 @@ static int verb_domain(int argc, char **argv, void *userdata) { assert(bus); - if (argc <= 1) + if (argc >= 2) { + r = ifname_mangle(argv[1], false); + if (r < 0) + return r; + } + + if (arg_ifindex <= 0) return status_all(bus, STATUS_DOMAIN); - r = ifname_mangle(argv[1], false); - if (r < 0) - return r; - - if (argc == 2) + if (argc < 3) return status_ifindex(bus, arg_ifindex, NULL, STATUS_DOMAIN, NULL); r = sd_bus_message_new_method_call( @@ -2026,14 +2030,16 @@ static int verb_llmnr(int argc, char **argv, void *userdata) { assert(bus); - if (argc <= 1) + if (argc >= 2) { + r = ifname_mangle(argv[1], false); + if (r < 0) + return r; + } + + if (arg_ifindex <= 0) return status_all(bus, STATUS_LLMNR); - r = ifname_mangle(argv[1], false); - if (r < 0) - return r; - - if (argc == 2) + if (argc < 3) return status_ifindex(bus, arg_ifindex, NULL, STATUS_LLMNR, NULL); r = sd_bus_call_method(bus, @@ -2065,14 +2071,16 @@ static int verb_mdns(int argc, char **argv, void *userdata) { assert(bus); - if (argc <= 1) + if (argc >= 2) { + r = ifname_mangle(argv[1], false); + if (r < 0) + return r; + } + + if (arg_ifindex <= 0) return status_all(bus, STATUS_MDNS); - r = ifname_mangle(argv[1], false); - if (r < 0) - return r; - - if (argc == 2) + if (argc < 3) return status_ifindex(bus, arg_ifindex, NULL, STATUS_MDNS, NULL); r = sd_bus_call_method(bus, @@ -2104,14 +2112,16 @@ static int verb_dns_over_tls(int argc, char **argv, void *userdata) { assert(bus); - if (argc <= 1) + if (argc >= 2) { + r = ifname_mangle(argv[1], false); + if (r < 0) + return r; + } + + if (arg_ifindex <= 0) return status_all(bus, STATUS_PRIVATE); - r = ifname_mangle(argv[1], false); - if (r < 0) - return r; - - if (argc == 2) + if (argc < 3) return status_ifindex(bus, arg_ifindex, NULL, STATUS_PRIVATE, NULL); r = sd_bus_call_method(bus, @@ -2143,14 +2153,16 @@ static int verb_dnssec(int argc, char **argv, void *userdata) { assert(bus); - if (argc <= 1) + if (argc >= 2) { + r = ifname_mangle(argv[1], false); + if (r < 0) + return r; + } + + if (arg_ifindex <= 0) return status_all(bus, STATUS_DNSSEC); - r = ifname_mangle(argv[1], false); - if (r < 0) - return r; - - if (argc == 2) + if (argc < 3) return status_ifindex(bus, arg_ifindex, NULL, STATUS_DNSSEC, NULL); r = sd_bus_call_method(bus, @@ -2185,14 +2197,16 @@ static int verb_nta(int argc, char **argv, void *userdata) { assert(bus); - if (argc <= 1) + if (argc >= 2) { + r = ifname_mangle(argv[1], false); + if (r < 0) + return r; + } + + if (arg_ifindex <= 0) return status_all(bus, STATUS_NTA); - r = ifname_mangle(argv[1], false); - if (r < 0) - return r; - - if (argc == 2) + if (argc < 3) return status_ifindex(bus, arg_ifindex, NULL, STATUS_NTA, NULL); /* If only argument is the empty string, then call SetLinkDNSSECNegativeTrustAnchors() @@ -2250,9 +2264,14 @@ static int verb_revert_link(int argc, char **argv, void *userdata) { assert(bus); - r = ifname_mangle(argv[1], false); - if (r < 0) - return r; + if (argc >= 2) { + r = ifname_mangle(argv[1], false); + if (r < 0) + return r; + } + + if (arg_ifindex <= 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Interface argument required."); r = sd_bus_call_method(bus, "org.freedesktop.resolve1", @@ -2943,7 +2962,7 @@ static int native_main(int argc, char *argv[], sd_bus *bus) { { "dnsovertls", VERB_ANY, 3, 0, verb_dns_over_tls }, { "dnssec", VERB_ANY, 3, 0, verb_dnssec }, { "nta", VERB_ANY, VERB_ANY, 0, verb_nta }, - { "revert", 2, 2, 0, verb_revert_link }, + { "revert", VERB_ANY, 2, 0, verb_revert_link }, {} }; From df87a53df0bcf8c2473d0a573bf31857f0402a37 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Dec 2018 14:02:06 +0100 Subject: [PATCH 4/9] resolvectl: remove allow_loopback parameter from ifname_mangle() All our bus calls validate whether the specified device is a loopback device anyway on the server side. Let's hence simplify the client, there's no value in optimizing error paths after all. But there is value in simpler code. --- src/resolve/resolvconf-compat.c | 2 +- src/resolve/resolvectl.c | 30 +++++++++++------------------- src/resolve/resolvectl.h | 2 +- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/resolve/resolvconf-compat.c b/src/resolve/resolvconf-compat.c index 242c26e5ca..383d0b819b 100644 --- a/src/resolve/resolvconf-compat.c +++ b/src/resolve/resolvconf-compat.c @@ -212,7 +212,7 @@ int resolvconf_parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected interface name as argument."); - r = ifname_mangle(argv[optind], false); + r = ifname_mangle(argv[optind]); if (r <= 0) return r; diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index a468cec72c..6c43ffa823 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -88,7 +88,7 @@ static int parse_ifindex_and_warn(const char *s) { return ifi; } -int ifname_mangle(const char *s, bool allow_loopback) { +int ifname_mangle(const char *s) { _cleanup_free_ char *iface = NULL; const char *dot; int ifi; @@ -122,10 +122,6 @@ int ifname_mangle(const char *s, bool allow_loopback) { return -EINVAL; } - if (!allow_loopback && ifi == LOOPBACK_IFINDEX) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Interface can't be the loopback interface (lo). Sorry."); - arg_ifindex = ifi; free_and_replace(arg_ifname, iface); @@ -1867,7 +1863,7 @@ static int verb_dns(int argc, char **argv, void *userdata) { assert(bus); if (argc >= 2) { - r = ifname_mangle(argv[1], false); + r = ifname_mangle(argv[1]); if (r < 0) return r; } @@ -1953,7 +1949,7 @@ static int verb_domain(int argc, char **argv, void *userdata) { assert(bus); if (argc >= 2) { - r = ifname_mangle(argv[1], false); + r = ifname_mangle(argv[1]); if (r < 0) return r; } @@ -2031,7 +2027,7 @@ static int verb_llmnr(int argc, char **argv, void *userdata) { assert(bus); if (argc >= 2) { - r = ifname_mangle(argv[1], false); + r = ifname_mangle(argv[1]); if (r < 0) return r; } @@ -2072,7 +2068,7 @@ static int verb_mdns(int argc, char **argv, void *userdata) { assert(bus); if (argc >= 2) { - r = ifname_mangle(argv[1], false); + r = ifname_mangle(argv[1]); if (r < 0) return r; } @@ -2113,7 +2109,7 @@ static int verb_dns_over_tls(int argc, char **argv, void *userdata) { assert(bus); if (argc >= 2) { - r = ifname_mangle(argv[1], false); + r = ifname_mangle(argv[1]); if (r < 0) return r; } @@ -2154,7 +2150,7 @@ static int verb_dnssec(int argc, char **argv, void *userdata) { assert(bus); if (argc >= 2) { - r = ifname_mangle(argv[1], false); + r = ifname_mangle(argv[1]); if (r < 0) return r; } @@ -2198,7 +2194,7 @@ static int verb_nta(int argc, char **argv, void *userdata) { assert(bus); if (argc >= 2) { - r = ifname_mangle(argv[1], false); + r = ifname_mangle(argv[1]); if (r < 0) return r; } @@ -2265,7 +2261,7 @@ static int verb_revert_link(int argc, char **argv, void *userdata) { assert(bus); if (argc >= 2) { - r = ifname_mangle(argv[1], false); + r = ifname_mangle(argv[1]); if (r < 0) return r; } @@ -2512,7 +2508,7 @@ static int compat_parse_argv(int argc, char *argv[]) { break; case 'i': - r = ifname_mangle(optarg, true); + r = ifname_mangle(optarg); if (r < 0) return r; break; @@ -2742,10 +2738,6 @@ static int compat_parse_argv(int argc, char *argv[]) { if (arg_ifindex <= 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--set-dns=, --set-domain=, --set-llmnr=, --set-mdns=, --set-dnsovertls=, --set-dnssec=, --set-nta= and --revert require --interface=."); - - if (arg_ifindex == LOOPBACK_IFINDEX) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Interface can't be the loopback interface (lo). Sorry."); } return 1 /* work to do */; @@ -2803,7 +2795,7 @@ static int native_parse_argv(int argc, char *argv[]) { break; case 'i': - r = ifname_mangle(optarg, true); + r = ifname_mangle(optarg); if (r < 0) return r; break; diff --git a/src/resolve/resolvectl.h b/src/resolve/resolvectl.h index 7858fcd6b4..6b3a1f8445 100644 --- a/src/resolve/resolvectl.h +++ b/src/resolve/resolvectl.h @@ -26,4 +26,4 @@ extern char **arg_set_dns; extern char **arg_set_domain; extern bool arg_ifindex_permissive; -int ifname_mangle(const char *s, bool allow_loopback); +int ifname_mangle(const char *s); From ff4a77c3ce6af4561bc1af08efd1b16604913904 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Dec 2018 17:31:35 +0100 Subject: [PATCH 5/9] =?UTF-8?q?resolvectl:=20fix=20interface=20output=20wh?= =?UTF-8?q?en=20resolving=20hostnames=E2=86=92addresses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We already used in_addr_ifindex_to_string() which internally appends the ifindex to the address with % if necessary. It's simply wrong to attach the intreface a second time with % then. Also, it breaks stuff that cannot deal with that. Hence, let's reformat this, and add the ifindex as a comment to the output, and drop the second % suffix. --- src/resolve/resolvectl.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 6c43ffa823..371417cdf6 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -157,6 +157,20 @@ static void print_source(uint64_t flags, usec_t rtt) { printf("-- Data is authenticated: %s\n", yes_no(flags & SD_RESOLVED_AUTHENTICATED)); } +static void print_ifindex_comment(int printed_so_far, int ifindex) { + char ifname[IF_NAMESIZE]; + + if (ifindex <= 0) + return; + + if (!if_indextoname(ifindex, ifname)) + log_warning_errno(errno, "Failed to resolve interface name for index %i, ignoring: %m", ifindex); + else + printf("%*s-- link: %s", + 60 > printed_so_far ? 60 - printed_so_far : 0, " ", /* Align comment to the 60th column */ + ifname); +} + static int resolve_host(sd_bus *bus, const char *name) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *req = NULL, *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -198,8 +212,7 @@ static int resolve_host(sd_bus *bus, const char *name) { while ((r = sd_bus_message_enter_container(reply, 'r', "iiay")) > 0) { _cleanup_free_ char *pretty = NULL; - char ifname[IF_NAMESIZE] = ""; - int ifindex, family; + int ifindex, family, k; const void *a; size_t sz; @@ -227,17 +240,16 @@ static int resolve_host(sd_bus *bus, const char *name) { return -EINVAL; } - if (ifindex > 0 && !if_indextoname(ifindex, ifname)) - log_warning_errno(errno, "Failed to resolve interface name for index %i: %m", ifindex); - r = in_addr_ifindex_to_string(family, a, ifindex, &pretty); if (r < 0) return log_error_errno(r, "Failed to print address for %s: %m", name); - printf("%*s%s %s%s%s\n", - (int) strlen(name), c == 0 ? name : "", c == 0 ? ":" : " ", - pretty, - isempty(ifname) ? "" : "%", ifname); + k = printf("%*s%s %s", + (int) strlen(name), c == 0 ? name : "", c == 0 ? ":" : " ", + pretty); + + print_ifindex_comment(k, ifindex); + fputc('\n', stdout); c++; } From 0889b81554e8051f14ed304cdd23f884412cc2e0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Dec 2018 17:42:09 +0100 Subject: [PATCH 6/9] resolvectl: when doing reverse lookups, show ifindex as "comment" too in output Similar as before: don't output ifindex twice on the same address, and show it as comment only. Do this for reverse lookup output and all other output too. --- src/resolve/resolvectl.c | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 371417cdf6..b9f2b9553e 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -283,7 +283,6 @@ static int resolve_address(sd_bus *bus, int family, const union in_addr_union *a _cleanup_(sd_bus_message_unrefp) sd_bus_message *req = NULL, *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *pretty = NULL; - char ifname[IF_NAMESIZE] = ""; uint64_t flags; unsigned c = 0; usec_t ts; @@ -300,10 +299,7 @@ static int resolve_address(sd_bus *bus, int family, const union in_addr_union *a if (r < 0) return log_oom(); - if (ifindex > 0 && !if_indextoname(ifindex, ifname)) - return log_error_errno(errno, "Failed to resolve interface name for index %i: %m", ifindex); - - log_debug("Resolving %s%s%s.", pretty, isempty(ifname) ? "" : "%", ifname); + log_debug("Resolving %s.", pretty); r = sd_bus_message_new_method_call( bus, @@ -341,6 +337,7 @@ static int resolve_address(sd_bus *bus, int family, const union in_addr_union *a while ((r = sd_bus_message_enter_container(reply, 'r', "is")) > 0) { const char *n; + int k; assert_cc(sizeof(int) == sizeof(int32_t)); @@ -352,17 +349,14 @@ static int resolve_address(sd_bus *bus, int family, const union in_addr_union *a if (r < 0) return r; - ifname[0] = 0; - if (ifindex > 0 && !if_indextoname(ifindex, ifname)) - log_warning_errno(errno, "Failed to resolve interface name for index %i: %m", ifindex); - - printf("%*s%*s%*s%s %s\n", + k = printf("%*s%s %s", (int) strlen(pretty), c == 0 ? pretty : "", - isempty(ifname) ? 0 : 1, c > 0 || isempty(ifname) ? "" : "%", - (int) strlen(ifname), c == 0 ? ifname : "", c == 0 ? ":" : " ", n); + print_ifindex_comment(k, ifindex); + fputc('\n', stdout); + c++; } if (r < 0) @@ -390,7 +384,6 @@ static int output_rr_packet(const void *d, size_t l, int ifindex) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr = NULL; _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; int r; - char ifname[IF_NAMESIZE] = ""; r = dns_packet_new(&p, DNS_PROTOCOL_DNS, 0, DNS_PACKET_SIZE_MAX); if (r < 0) @@ -416,15 +409,15 @@ static int output_rr_packet(const void *d, size_t l, int ifindex) { fwrite(data, 1, k, stdout); } else { const char *s; + int k; s = dns_resource_record_to_string(rr); if (!s) return log_oom(); - if (ifindex > 0 && !if_indextoname(ifindex, ifname)) - log_warning_errno(errno, "Failed to resolve interface name for index %i: %m", ifindex); - - printf("%s%s%s\n", s, isempty(ifname) ? "" : " # interface ", ifname); + k = printf("%s", s); + print_ifindex_comment(k, ifindex); + fputc('\n', stdout); } return 0; @@ -768,8 +761,7 @@ static int resolve_service(sd_bus *bus, const char *name, const char *type, cons while ((r = sd_bus_message_enter_container(reply, 'r', "iiay")) > 0) { _cleanup_free_ char *pretty = NULL; - char ifname[IF_NAMESIZE] = ""; - int ifindex, family; + int ifindex, family, k; const void *a; assert_cc(sizeof(int) == sizeof(int32_t)); @@ -796,14 +788,13 @@ static int resolve_service(sd_bus *bus, const char *name, const char *type, cons return -EINVAL; } - if (ifindex > 0 && !if_indextoname(ifindex, ifname)) - log_warning_errno(errno, "Failed to resolve interface name for index %i: %m", ifindex); - - r = in_addr_to_string(family, a, &pretty); + r = in_addr_ifindex_to_string(family, a, ifindex, &pretty); if (r < 0) return log_error_errno(r, "Failed to print address for %s: %m", name); - printf("%*s%s%s%s\n", (int) indent, "", pretty, isempty(ifname) ? "" : "%s", ifname); + k = printf("%*s%s", (int) indent, "", pretty); + print_ifindex_comment(k, ifindex); + fputc('\n', stdout); } if (r < 0) return bus_log_parse_error(r); From 38585af313bb19c516d0a5c5d83857d8c510b0f9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Dec 2018 18:23:48 +0100 Subject: [PATCH 7/9] resolvectl: add some basic highlighting to query output Let's highlight the actual payload the user requested. --- src/resolve/resolvectl.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index b9f2b9553e..b754474191 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -244,9 +244,9 @@ static int resolve_host(sd_bus *bus, const char *name) { if (r < 0) return log_error_errno(r, "Failed to print address for %s: %m", name); - k = printf("%*s%s %s", + k = printf("%*s%s %s%s%s", (int) strlen(name), c == 0 ? name : "", c == 0 ? ":" : " ", - pretty); + ansi_highlight(), pretty, ansi_normal()); print_ifindex_comment(k, ifindex); fputc('\n', stdout); @@ -349,10 +349,10 @@ static int resolve_address(sd_bus *bus, int family, const union in_addr_union *a if (r < 0) return r; - k = printf("%*s%s %s", - (int) strlen(pretty), c == 0 ? pretty : "", - c == 0 ? ":" : " ", - n); + k = printf("%*s%s %s%s%s", + (int) strlen(pretty), c == 0 ? pretty : "", + c == 0 ? ":" : " ", + ansi_highlight(), n, ansi_normal()); print_ifindex_comment(k, ifindex); fputc('\n', stdout); From ec4b9671b31f1f429b9176af26a0a57c437e52b6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Dec 2018 18:38:50 +0100 Subject: [PATCH 8/9] resolvectl: color "comment" output grey Let's de-emphasize the "comment" output a bit, by making it grey. --- src/basic/terminal-util.h | 2 ++ src/resolve/resolvectl.c | 16 +++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h index 86e730028e..c885e0a2d1 100644 --- a/src/basic/terminal-util.h +++ b/src/basic/terminal-util.h @@ -18,6 +18,7 @@ #define ANSI_MAGENTA "\x1B[0;35m" #define ANSI_CYAN "\x1B[0;36m" #define ANSI_WHITE "\x1B[0;37m" +#define ANSI_GREY "\x1B[0;2;37m" /* Bold/highlighted */ #define ANSI_HIGHLIGHT_BLACK "\x1B[0;1;30m" @@ -132,6 +133,7 @@ DEFINE_ANSI_FUNC(highlight_yellow, HIGHLIGHT_YELLOW); DEFINE_ANSI_FUNC(highlight_blue, HIGHLIGHT_BLUE); DEFINE_ANSI_FUNC(highlight_magenta, HIGHLIGHT_MAGENTA); DEFINE_ANSI_FUNC(normal, NORMAL); +DEFINE_ANSI_FUNC(grey, GREY); DEFINE_ANSI_FUNC_UNDERLINE(underline, UNDERLINE, NORMAL); DEFINE_ANSI_FUNC_UNDERLINE(highlight_underline, HIGHLIGHT_UNDERLINE, HIGHLIGHT); diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index b754474191..64abc3c46e 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -137,7 +137,7 @@ static void print_source(uint64_t flags, usec_t rtt) { if (flags == 0) return; - fputs("\n-- Information acquired via", stdout); + printf("\n%s-- Information acquired via", ansi_grey()); if (flags != 0) printf(" protocol%s%s%s%s%s", @@ -149,12 +149,10 @@ static void print_source(uint64_t flags, usec_t rtt) { assert_se(format_timespan(rtt_str, sizeof(rtt_str), rtt, 100)); - printf(" in %s", rtt_str); - - fputc('.', stdout); - fputc('\n', stdout); - - printf("-- Data is authenticated: %s\n", yes_no(flags & SD_RESOLVED_AUTHENTICATED)); + printf(" in %s.%s\n" + "%s-- Data is authenticated: %s%s\n", + rtt_str, ansi_normal(), + ansi_grey(), yes_no(flags & SD_RESOLVED_AUTHENTICATED), ansi_normal()); } static void print_ifindex_comment(int printed_so_far, int ifindex) { @@ -166,9 +164,9 @@ static void print_ifindex_comment(int printed_so_far, int ifindex) { if (!if_indextoname(ifindex, ifname)) log_warning_errno(errno, "Failed to resolve interface name for index %i, ignoring: %m", ifindex); else - printf("%*s-- link: %s", + printf("%*s%s-- link: %s%s", 60 > printed_so_far ? 60 - printed_so_far : 0, " ", /* Align comment to the 60th column */ - ifname); + ansi_grey(), ifname, ansi_normal()); } static int resolve_host(sd_bus *bus, const char *name) { From 67df9b7a06d749fdd84f19f7d75ccf0d743f6d72 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Dec 2018 18:42:32 +0100 Subject: [PATCH 9/9] logs-show: use grey color for de-emphasizing journal log output --- src/shared/logs-show.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index 4076b9e1d5..525a948f36 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -171,6 +171,10 @@ static bool print_multiline( color_on = ANSI_HIGHLIGHT; color_off = ANSI_NORMAL; highlight_on = ANSI_HIGHLIGHT_RED; + } else if (priority >= LOG_DEBUG) { + color_on = ANSI_GREY; + color_off = ANSI_NORMAL; + highlight_on = ANSI_HIGHLIGHT_RED; } }