From fe9e7b71cf0930db1f8d6872f415466689511885 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 May 2019 12:23:02 +0200 Subject: [PATCH 1/2] lib: split off c-ares code from nhrpd This is useful in other places too, e.g. for BMP outbound connections. Signed-off-by: David Lamparter --- configure.ac | 2 +- debian/frr.install | 1 + lib/command.c | 1 + lib/command.h | 1 + lib/lib_errors.c | 6 +++ lib/lib_errors.h | 1 + {nhrpd => lib}/resolver.c | 102 +++++++++++++++++++++++++------------- lib/resolver.h | 25 ++++++++++ lib/subdir.am | 15 ++++++ nhrpd/nhrp_errors.c | 6 --- nhrpd/nhrp_errors.h | 1 - nhrpd/nhrp_main.c | 2 +- nhrpd/nhrpd.h | 10 +--- nhrpd/subdir.am | 4 +- vtysh/vtysh_config.c | 5 +- 15 files changed, 125 insertions(+), 57 deletions(-) rename {nhrpd => lib}/resolver.c (76%) create mode 100644 lib/resolver.h diff --git a/configure.ac b/configure.ac index 9b57c2ec9b..0911b96900 100755 --- a/configure.ac +++ b/configure.ac @@ -1560,7 +1560,7 @@ if test "${NHRPD}" != ""; then AC_MSG_ERROR([trying to build nhrpd, but libcares not found. install c-ares and its -dev headers.]) ]) fi - +AM_CONDITIONAL([CARES], [test "${NHRPD}" != ""]) dnl ------------------ dnl check Net-SNMP library diff --git a/debian/frr.install b/debian/frr.install index ebb87a0b3e..fe34b23d02 100644 --- a/debian/frr.install +++ b/debian/frr.install @@ -2,6 +2,7 @@ etc/ usr/bin/vtysh usr/bin/mtracebis usr/lib/*/frr/libfrr.* +usr/lib/*/frr/libfrrcares.* usr/lib/*/frr/libfrrospfapiclient.* usr/lib/frr/*.sh usr/lib/frr/*d diff --git a/lib/command.c b/lib/command.c index f257c7d0f9..c8fbf22721 100644 --- a/lib/command.c +++ b/lib/command.c @@ -85,6 +85,7 @@ const char *node_names[] = { "northbound debug", // NORTHBOUND_DEBUG_NODE, "vnc debug", // DEBUG_VNC_NODE, "route-map debug", /* RMAP_DEBUG_NODE */ + "resolver debug", /* RESOLVER_DEBUG_NODE */ "aaa", // AAA_NODE, "keychain", // KEYCHAIN_NODE, "keychain key", // KEYCHAIN_KEY_NODE, diff --git a/lib/command.h b/lib/command.h index de38563f96..08d6128af4 100644 --- a/lib/command.h +++ b/lib/command.h @@ -94,6 +94,7 @@ enum node_type { NORTHBOUND_DEBUG_NODE, /* Northbound Debug node. */ DEBUG_VNC_NODE, /* Debug VNC node. */ RMAP_DEBUG_NODE, /* Route-map debug node */ + RESOLVER_DEBUG_NODE, /* Resolver debug node */ AAA_NODE, /* AAA node. */ KEYCHAIN_NODE, /* Key-chain node. */ KEYCHAIN_KEY_NODE, /* Key-chain key node. */ diff --git a/lib/lib_errors.c b/lib/lib_errors.c index e0559f332d..6e5088142a 100644 --- a/lib/lib_errors.c +++ b/lib/lib_errors.c @@ -356,6 +356,12 @@ static struct log_ref ferr_lib_err[] = { .description = "A callback used to process a configuration change has returned an error while applying the changes", .suggestion = "Gather log data and open an Issue.", }, + { + .code = EC_LIB_RESOLVER, + .title = "DNS Resolution", + .description = "An error was detected while attempting to resolve a hostname", + .suggestion = "Ensure that DNS is working properly and the hostname is configured in dns. If you are still seeing this error, open an issue" + }, { .code = END_FERR, } diff --git a/lib/lib_errors.h b/lib/lib_errors.h index 996a16ba95..4730b6aa33 100644 --- a/lib/lib_errors.h +++ b/lib/lib_errors.h @@ -84,6 +84,7 @@ enum lib_log_refs { EC_LIB_GRPC_INIT, EC_LIB_ID_CONSISTENCY, EC_LIB_ID_EXHAUST, + EC_LIB_RESOLVER, }; extern void lib_error_init(void); diff --git a/nhrpd/resolver.c b/lib/resolver.c similarity index 76% rename from nhrpd/resolver.c rename to lib/resolver.c index 64b16e7ee3..001c293df2 100644 --- a/nhrpd/resolver.c +++ b/lib/resolver.c @@ -17,17 +17,18 @@ #include "vector.h" #include "thread.h" #include "lib_errors.h" - -#include "nhrpd.h" -#include "nhrp_errors.h" +#include "resolver.h" +#include "command.h" struct resolver_state { ares_channel channel; + struct thread_master *master; struct thread *timeout; vector read_threads, write_threads; }; static struct resolver_state state; +static bool resolver_debug; #define THREAD_RUNNING ((struct thread *)-1) @@ -54,7 +55,8 @@ static int resolver_cb_socket_readable(struct thread *t) ares_process_fd(r->channel, fd, ARES_SOCKET_BAD); if (vector_lookup(r->read_threads, fd) == THREAD_RUNNING) { t = NULL; - thread_add_read(master, resolver_cb_socket_readable, r, fd, &t); + thread_add_read(r->master, resolver_cb_socket_readable, r, fd, + &t); vector_set_index(r->read_threads, fd, t); } resolver_update_timeouts(r); @@ -71,7 +73,7 @@ static int resolver_cb_socket_writable(struct thread *t) ares_process_fd(r->channel, ARES_SOCKET_BAD, fd); if (vector_lookup(r->write_threads, fd) == THREAD_RUNNING) { t = NULL; - thread_add_write(master, resolver_cb_socket_writable, r, fd, + thread_add_write(r->master, resolver_cb_socket_writable, r, fd, &t); vector_set_index(r->write_threads, fd, t); } @@ -91,8 +93,8 @@ static void resolver_update_timeouts(struct resolver_state *r) tv = ares_timeout(r->channel, NULL, &tvbuf); if (tv) { unsigned int timeoutms = tv->tv_sec * 1000 + tv->tv_usec / 1000; - thread_add_timer_msec(master, resolver_cb_timeout, r, timeoutms, - &r->timeout); + thread_add_timer_msec(r->master, resolver_cb_timeout, r, + timeoutms, &r->timeout); } } @@ -105,8 +107,8 @@ static void ares_socket_cb(void *data, ares_socket_t fd, int readable, if (readable) { t = vector_lookup_ensure(r->read_threads, fd); if (!t) { - thread_add_read(master, resolver_cb_socket_readable, r, - fd, &t); + thread_add_read(r->master, resolver_cb_socket_readable, + r, fd, &t); vector_set_index(r->read_threads, fd, t); } } else { @@ -122,8 +124,8 @@ static void ares_socket_cb(void *data, ares_socket_t fd, int readable, if (writable) { t = vector_lookup_ensure(r->write_threads, fd); if (!t) { - thread_add_read(master, resolver_cb_socket_writable, r, - fd, &t); + thread_add_read(r->master, resolver_cb_socket_writable, + r, fd, &t); vector_set_index(r->write_threads, fd, t); } } else { @@ -137,25 +139,6 @@ static void ares_socket_cb(void *data, ares_socket_t fd, int readable, } } -void resolver_init(void) -{ - struct ares_options ares_opts; - - state.read_threads = vector_init(1); - state.write_threads = vector_init(1); - - ares_opts = (struct ares_options){ - .sock_state_cb = &ares_socket_cb, - .sock_state_cb_data = &state, - .timeout = 2, - .tries = 3, - }; - - ares_init_options(&state.channel, &ares_opts, - ARES_OPT_SOCK_STATE_CB | ARES_OPT_TIMEOUT - | ARES_OPT_TRIES); -} - static void ares_address_cb(void *arg, int status, int timeouts, struct hostent *he) @@ -165,7 +148,9 @@ static void ares_address_cb(void *arg, int status, int timeouts, size_t i; if (status != ARES_SUCCESS) { - debugf(NHRP_DEBUG_COMMON, "[%p] Resolving failed", query); + if (resolver_debug) + zlog_debug("[%p] Resolving failed", query); + query->callback(query, -1, NULL); query->callback = NULL; return; @@ -186,8 +171,9 @@ static void ares_address_cb(void *arg, int status, int timeouts, } } - debugf(NHRP_DEBUG_COMMON, "[%p] Resolved with %d results", query, - (int)i); + if (resolver_debug) + zlog_debug("[%p] Resolved with %d results", query, (int)i); + query->callback(query, i, &addr[0]); query->callback = NULL; } @@ -199,15 +185,61 @@ void resolver_resolve(struct resolver_query *query, int af, { if (query->callback != NULL) { flog_err( - EC_NHRP_RESOLVER, + EC_LIB_RESOLVER, "Trying to resolve '%s', but previous query was not finished yet", hostname); return; } - debugf(NHRP_DEBUG_COMMON, "[%p] Resolving '%s'", query, hostname); + if (resolver_debug) + zlog_debug("[%p] Resolving '%s'", query, hostname); query->callback = callback; ares_gethostbyname(state.channel, hostname, af, ares_address_cb, query); resolver_update_timeouts(&state); } + +DEFUN(debug_resolver, + debug_resolver_cmd, + "[no] debug resolver", + NO_STR + DEBUG_STR + "Debug DNS resolver actions\n") +{ + resolver_debug = (argc == 2); + return CMD_SUCCESS; +} + +static struct cmd_node resolver_debug_node = {RESOLVER_DEBUG_NODE, "", 1}; + +static int resolver_config_write_debug(struct vty *vty) +{ + if (resolver_debug) + vty_out(vty, "debug resolver\n"); + return 1; +} + + +void resolver_init(struct thread_master *tm) +{ + struct ares_options ares_opts; + + state.master = tm; + state.read_threads = vector_init(1); + state.write_threads = vector_init(1); + + ares_opts = (struct ares_options){ + .sock_state_cb = &ares_socket_cb, + .sock_state_cb_data = &state, + .timeout = 2, + .tries = 3, + }; + + ares_init_options(&state.channel, &ares_opts, + ARES_OPT_SOCK_STATE_CB | ARES_OPT_TIMEOUT + | ARES_OPT_TRIES); + + install_node(&resolver_debug_node, resolver_config_write_debug); + install_element(CONFIG_NODE, &debug_resolver_cmd); + install_element(ENABLE_NODE, &debug_resolver_cmd); +} diff --git a/lib/resolver.h b/lib/resolver.h new file mode 100644 index 0000000000..bc6326edaa --- /dev/null +++ b/lib/resolver.h @@ -0,0 +1,25 @@ +/* C-Ares integration to Quagga mainloop + * Copyright (c) 2014-2015 Timo Teräs + * + * This file is free software: you may copy, redistribute and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + */ + +#ifndef _FRR_RESOLVER_H +#define _FRR_RESOLVER_H + +#include "thread.h" +#include "sockunion.h" + +struct resolver_query { + void (*callback)(struct resolver_query *, int n, union sockunion *); +}; + +void resolver_init(struct thread_master *tm); +void resolver_resolve(struct resolver_query *query, int af, + const char *hostname, void (*cb)(struct resolver_query *, + int, union sockunion *)); + +#endif /* _FRR_RESOLVER_H */ diff --git a/lib/subdir.am b/lib/subdir.am index b0cf77ee92..aa89622028 100644 --- a/lib/subdir.am +++ b/lib/subdir.am @@ -285,6 +285,21 @@ lib_libfrrsnmp_la_SOURCES = \ lib/snmp.c \ # end +# +# c-ares support +# +if CARES +lib_LTLIBRARIES += lib/libfrrcares.la +pkginclude_HEADERS += lib/resolver.h +endif + +lib_libfrrcares_la_CFLAGS = $(WERROR) $(CARES_CFLAGS) +lib_libfrrcares_la_LDFLAGS = -version-info 0:0:0 +lib_libfrrcares_la_LIBADD = $(CARES_LIBS) +lib_libfrrcares_la_SOURCES = \ + lib/resolver.c \ + #end + # # ZeroMQ support # diff --git a/nhrpd/nhrp_errors.c b/nhrpd/nhrp_errors.c index 4c4f55be9e..741e64d8b3 100644 --- a/nhrpd/nhrp_errors.c +++ b/nhrpd/nhrp_errors.c @@ -31,12 +31,6 @@ static struct log_ref ferr_nhrp_err[] = { .description = "NHRP has detected a error with the Strongswan code", .suggestion = "Ensure that StrongSwan is configured correctly. Restart StrongSwan and FRR" }, - { - .code = EC_NHRP_RESOLVER, - .title = "NHRP DNS Resolution", - .description = "NHRP has detected an error in an attempt to resolve a hostname", - .suggestion = "Ensure that DNS is working properly and the hostname is configured in dns. If you are still seeing this error, open an issue" - }, { .code = END_FERR, } diff --git a/nhrpd/nhrp_errors.h b/nhrpd/nhrp_errors.h index 593714786a..d4958358fe 100644 --- a/nhrpd/nhrp_errors.h +++ b/nhrpd/nhrp_errors.h @@ -25,7 +25,6 @@ enum nhrp_log_refs { EC_NHRP_SWAN = NHRP_FERR_START, - EC_NHRP_RESOLVER, }; extern void nhrp_error_init(void); diff --git a/nhrpd/nhrp_main.c b/nhrpd/nhrp_main.c index d7c485f0a0..969638cd77 100644 --- a/nhrpd/nhrp_main.c +++ b/nhrpd/nhrp_main.c @@ -141,7 +141,7 @@ int main(int argc, char **argv) nhrp_error_init(); vrf_init(NULL, NULL, NULL, NULL, NULL); nhrp_interface_init(); - resolver_init(); + resolver_init(master); /* Run with elevated capabilities, as for all netlink activity * we need privileges anyway. */ diff --git a/nhrpd/nhrpd.h b/nhrpd/nhrpd.h index 89de145e65..670c9f4f18 100644 --- a/nhrpd/nhrpd.h +++ b/nhrpd/nhrpd.h @@ -16,6 +16,7 @@ #include "zclient.h" #include "debug.h" #include "memory.h" +#include "resolver.h" DECLARE_MGROUP(NHRPD) @@ -84,15 +85,6 @@ static inline int notifier_active(struct notifier_list *l) return !list_empty(&l->notifier_head); } -struct resolver_query { - void (*callback)(struct resolver_query *, int n, union sockunion *); -}; - -void resolver_init(void); -void resolver_resolve(struct resolver_query *query, int af, - const char *hostname, void (*cb)(struct resolver_query *, - int, union sockunion *)); - void nhrp_zebra_init(void); void nhrp_zebra_terminate(void); diff --git a/nhrpd/subdir.am b/nhrpd/subdir.am index 6e2b91780f..fe76623ac3 100644 --- a/nhrpd/subdir.am +++ b/nhrpd/subdir.am @@ -8,8 +8,7 @@ vtysh_scan += $(top_srcdir)/nhrpd/nhrp_vty.c man8 += $(MANBUILD)/nhrpd.8 endif -nhrpd_nhrpd_LDADD = lib/libfrr.la $(LIBCAP) $(CARES_LIBS) -nhrpd_nhrpd_CFLAGS = $(AM_CFLAGS) $(CARES_CFLAGS) +nhrpd_nhrpd_LDADD = lib/libfrr.la lib/libfrrcares.la $(LIBCAP) nhrpd_nhrpd_SOURCES = \ nhrpd/linux.c \ nhrpd/netlink_arp.c \ @@ -27,7 +26,6 @@ nhrpd_nhrpd_SOURCES = \ nhrpd/nhrp_vc.c \ nhrpd/nhrp_vty.c \ nhrpd/reqid.c \ - nhrpd/resolver.c \ nhrpd/vici.c \ nhrpd/zbuf.c \ nhrpd/znl.c \ diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c index b8957c2b00..4ae1e499ff 100644 --- a/vtysh/vtysh_config.c +++ b/vtysh/vtysh_config.c @@ -377,6 +377,9 @@ void vtysh_config_parse_line(void *arg, const char *line) strlen("debug route-map")) == 0) config = config_get(RMAP_DEBUG_NODE, line); + else if (strncmp(line, "debug resolver", + strlen("debug resolver")) == 0) + config = config_get(RESOLVER_DEBUG_NODE, line); else if (strncmp(line, "debug", strlen("debug")) == 0) config = config_get(DEBUG_NODE, line); else if (strncmp(line, "password", strlen("password")) == 0 @@ -423,7 +426,7 @@ void vtysh_config_parse_line(void *arg, const char *line) || (I) == PREFIX_IPV6_NODE || (I) == FORWARDING_NODE \ || (I) == DEBUG_NODE || (I) == AAA_NODE || (I) == VRF_DEBUG_NODE \ || (I) == NORTHBOUND_DEBUG_NODE || (I) == RMAP_DEBUG_NODE \ - || (I) == MPLS_NODE) + || (I) == RESOLVER_DEBUG_NODE || (I) == MPLS_NODE) /* Display configuration to file pointer. */ void vtysh_config_dump(void) From 50cdb6cf95c5fc7df40e4a7328eb1fc7fcff4a9c Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 23 May 2019 14:25:58 +0200 Subject: [PATCH 2/2] lib/resolver: NULL out callback before call The callback itself might want to reschedule the resolver, so it is useful to clear out the callback field before making the call instead of after. Signed-off-by: David Lamparter --- lib/resolver.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/resolver.c b/lib/resolver.c index 001c293df2..fb8aeed92f 100644 --- a/lib/resolver.c +++ b/lib/resolver.c @@ -145,14 +145,17 @@ static void ares_address_cb(void *arg, int status, int timeouts, { struct resolver_query *query = (struct resolver_query *)arg; union sockunion addr[16]; + void (*callback)(struct resolver_query *, int, union sockunion *); size_t i; + callback = query->callback; + query->callback = NULL; + if (status != ARES_SUCCESS) { if (resolver_debug) zlog_debug("[%p] Resolving failed", query); - query->callback(query, -1, NULL); - query->callback = NULL; + callback(query, -1, NULL); return; } @@ -174,8 +177,7 @@ static void ares_address_cb(void *arg, int status, int timeouts, if (resolver_debug) zlog_debug("[%p] Resolved with %d results", query, (int)i); - query->callback(query, i, &addr[0]); - query->callback = NULL; + callback(query, i, &addr[0]); } void resolver_resolve(struct resolver_query *query, int af,