From: Alberto Leiva Popper Date: Mon, 30 Oct 2023 20:47:28 +0000 (-0600) Subject: Add minor/stylistic tweaks to main loop X-Git-Tag: 1.6.0~31 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=d48e56b91f13b9111b0135fce574c7ae98c1eae1;p=thirdparty%2FFORT-validator.git Add minor/stylistic tweaks to main loop 1. Merge notify and validation_run into main 2. Remove unused structure fields 3. Rename some symbols --- diff --git a/src/Makefile.am b/src/Makefile.am index 5aae5ae4..55263da1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -21,7 +21,6 @@ fort_SOURCES += json_parser.c json_parser.h fort_SOURCES += line_file.h line_file.c fort_SOURCES += log.h log.c fort_SOURCES += nid.h nid.c -fort_SOURCES += notify.c notify.h fort_SOURCES += output_printer.h output_printer.c fort_SOURCES += resource.h resource.c fort_SOURCES += rpp.h rpp.c @@ -31,7 +30,6 @@ fort_SOURCES += str_token.h str_token.c fort_SOURCES += thread_var.h thread_var.c fort_SOURCES += json_handler.h json_handler.c fort_SOURCES += validation_handler.h validation_handler.c -fort_SOURCES += validation_run.h validation_run.c fort_SOURCES += asn1/content_info.h asn1/content_info.c fort_SOURCES += asn1/decode.h asn1/decode.c diff --git a/src/cache/local_cache.h b/src/cache/local_cache.h index 938e9cba..d826ae75 100644 --- a/src/cache/local_cache.h +++ b/src/cache/local_cache.h @@ -4,7 +4,7 @@ #include "types/uri.h" /* Warms up cache for new validation run */ -int cache_prepare(void); +int cache_prepare(void); /* No revert needed */ /* Downloads @uri into the cache */ int cache_download(struct rpki_uri *uri, bool *); @@ -13,6 +13,6 @@ int cache_download(struct rpki_uri *uri, bool *); /* FIXME call this */ void cache_cleanup(void); -void cache_teardown(void); +void cache_teardown(void); /* No setup needed */ #endif /* SRC_CACHE_LOCAL_CACHE_H_ */ diff --git a/src/common.c b/src/common.c index d83eb399..0aba47b5 100644 --- a/src/common.c +++ b/src/common.c @@ -91,7 +91,7 @@ rwlock_unlock(pthread_rwlock_t *lock) static int process_file(char const *dir_name, char const *file_name, char const *file_ext, - int *fcount, process_file_cb cb, void *arg) + int *fcount, foreach_file_cb cb, void *arg) { char const *ext; char *fullpath; @@ -130,7 +130,7 @@ process_file(char const *dir_name, char const *file_name, char const *file_ext, static int process_dir_files(char const *location, char const *file_ext, bool empty_err, - process_file_cb cb, void *arg) + foreach_file_cb cb, void *arg) { DIR *dir_loc; struct dirent *dir_ent; @@ -172,9 +172,16 @@ end: return error; } +/* + * If @location points to a file, run @cb on it. + * If @location points to a directory, run @cb on every child file suffixed + * @file_ext. + * + * TODO (fine) It's weird that @file_ext only filters in directory mode. + */ int -process_file_or_dir(char const *location, char const *file_ext, bool empty_err, - process_file_cb cb, void *arg) +foreach_file(char const *location, char const *file_ext, bool empty_err, + foreach_file_cb cb, void *arg) { struct stat attr; int error; diff --git a/src/common.h b/src/common.h index d33edbea..15bf84ff 100644 --- a/src/common.h +++ b/src/common.h @@ -58,9 +58,8 @@ int rwlock_read_lock(pthread_rwlock_t *); void rwlock_write_lock(pthread_rwlock_t *); void rwlock_unlock(pthread_rwlock_t *); -typedef int (*process_file_cb)(char const *, void *); -int process_file_or_dir(char const *, char const *, bool, process_file_cb, - void *); +typedef int (*foreach_file_cb)(char const *, void *); +int foreach_file(char const *, char const *, bool, foreach_file_cb, void *); typedef int (*pr_errno_cb)(const char *, ...); bool valid_file_or_dir(char const *, bool, bool, pr_errno_cb); diff --git a/src/config.c b/src/config.c index 18b7bec8..c5387092 100644 --- a/src/config.c +++ b/src/config.c @@ -1080,10 +1080,8 @@ handle_flags_config(int argc, char **argv) } /* - * This triggers when the user runs something like - * `rpki-validator disable-rsync` instead of - * `rpki-validator --disable-rsync`. - * This program does not have unflagged payload. + * This triggers when the user runs something like `fort help` instead + * of `fort --help`. This program does not have unflagged payload. */ if (optind < argc) { error = pr_op_err("I don't know what '%s' is.", argv[optind]); diff --git a/src/main.c b/src/main.c index 2b10c0c7..b6787561 100644 --- a/src/main.c +++ b/src/main.c @@ -1,9 +1,10 @@ +#include + #include "config.h" #include "extension.h" #include "log.h" #include "nid.h" #include "thread_var.h" -#include "validation_run.h" #include "cache/local_cache.h" #include "http/http.h" #include "incidence/incidence.h" @@ -12,17 +13,79 @@ #include "xml/relax_ng.h" static int -run_rtr_server(void) +fort_standalone(void) +{ + int error; + + pr_op_info("Updating cache..."); + + error = vrps_update(NULL); + if (error) { + pr_op_err("Validation unsuccessful; results unusable."); + return error; + } + + pr_op_info("Done."); + return 0; +} + +static int +fort_server(void) { int error; + bool changed; + + pr_op_info("Main loop: Starting..."); error = rtr_start(); if (error) return error; - error = validation_run_first(); - if (!error) - error = validation_run_cycle(); /* Usually loops forever */ + error = vrps_update(NULL); + if (error) { + pr_op_err("Main loop: Validation unsuccessful; results unusable."); + return error; + } + + rtr_notify(); + + /* + * According to some past experience I can't find anymore, there's at + * least one brand of router that misunderstands RTR "No Data Available + * [yet];" it thinks it's a fatal error. + * + * I don't know if the problem persists, I can't find the bug report + * anymore, and given that I've patched several RTR errors since, I'm + * questioning whether it was a problem in the first place. + * + * Tentatively tell the admin to start those routers now. This is + * ridiculous on several levels however, and I'm half a mind to delete + * this notification. + * + * This message was born in 38e256cb, and I've decided to downgrade and + * reword it in the hopes of triggering complaints if someone's still + * using it. + * + * TODO (fine) If nobody complaints, remove in a few months. + */ + pr_op_info("Main loop: Ready for routers."); + + do { + pr_op_info("Main loop: Sleeping."); + sleep(config_get_validation_interval()); + pr_op_info("Main loop: Time to work!"); + + error = vrps_update(&changed); + if (error == -EINTR) + break; + if (error) { + pr_op_debug("Main loop: Error %d (%s)", error, + strerror(abs(error))); + continue; + } + if (changed) + rtr_notify(); + } while (true); rtr_stop(); return error; @@ -98,13 +161,14 @@ main(int argc, char **argv) if (error) goto revert_relax_ng; - /* Do stuff */ + /* Meat */ + switch (config_get_mode()) { case STANDALONE: - error = validation_run_first(); + error = fort_standalone(); break; case SERVER: - error = run_rtr_server(); + error = fort_server(); break; } diff --git a/src/notify.c b/src/notify.c deleted file mode 100644 index b4f71876..00000000 --- a/src/notify.c +++ /dev/null @@ -1,30 +0,0 @@ -#include "notify.h" - -#include "log.h" -#include "rtr/rtr.h" -#include "rtr/pdu_sender.h" -#include "rtr/db/vrps.h" - -static int -send_notify(int fd, int rtr_version, void *arg) -{ - serial_t *serial = arg; - - send_serial_notify_pdu(fd, rtr_version, *serial); - - /* Errors already logged, do not interrupt notify to other clients */ - return 0; -} - -int -notify_clients(void) -{ - serial_t serial; - int error; - - error = get_last_serial_number(&serial); - if (error) - return error; - - return rtr_foreach_client(send_notify, &serial); -} diff --git a/src/notify.h b/src/notify.h deleted file mode 100644 index 44710abf..00000000 --- a/src/notify.h +++ /dev/null @@ -1,6 +0,0 @@ -#ifndef SRC_NOTIFY_H_ -#define SRC_NOTIFY_H_ - -int notify_clients(void); - -#endif /* SRC_NOTIFY_H_ */ diff --git a/src/object/tal.c b/src/object/tal.c index 24a6efa8..221eac9a 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -18,15 +18,12 @@ #include "rtr/db/vrps.h" #include "cache/local_cache.h" -#define TAL_FILE_EXTENSION ".tal" typedef int (*foreach_uri_cb)(struct tal *, struct rpki_uri *, void *); struct uris { struct rpki_uri **array; /* This is an array of rpki URIs. */ unsigned int count; unsigned int size; - unsigned int rsync_count; - unsigned int https_count; }; struct tal { @@ -48,7 +45,7 @@ struct validation_thread { bool retry_local; /* Try to sync the current TA URI? */ bool sync_files; - void *arg; + struct db_table *db; int exit_status; /* This should also only be manipulated by the parent thread. */ SLIST_ENTRY(validation_thread) next; @@ -66,8 +63,6 @@ static void uris_init(struct uris *uris) { uris->count = 0; - uris->rsync_count = 0; - uris->https_count = 0; uris->size = 4; /* Most TALs only define one. */ uris->array = pmalloc(uris->size * sizeof(struct rpki_uri *)); } @@ -96,11 +91,6 @@ uris_add(struct uris *uris, char *uri) if (error) return error; - if (uri_is_rsync(new)) - uris->rsync_count++; - else - uris->https_count++; - if (uris->count + 1 >= uris->size) { uris->size *= 2; uris->array = realloc(uris->array, @@ -453,7 +443,7 @@ handle_tal_uri(struct tal *tal, struct rpki_uri *uri, void *arg) validation_handler.handle_roa_v4 = handle_roa_v4; validation_handler.handle_roa_v6 = handle_roa_v6; validation_handler.handle_router_key = handle_router_key; - validation_handler.arg = thread->arg; + validation_handler.arg = thread->db; error = validation_prepare(&state, tal, &validation_handler); if (error) @@ -595,32 +585,30 @@ thread_destroy(struct validation_thread *thread) /* Creates a thread for the @tal_file */ static int -__do_file_validation(char const *tal_file, void *arg) +spawn_tal_thread(char const *tal_file, void *arg) { - struct tal_param *t_param = arg; + struct tal_param *param = arg; struct validation_thread *thread; int error; thread = pmalloc(sizeof(struct validation_thread)); thread->tal_file = pstrdup(tal_file); - thread->arg = t_param->db; - thread->exit_status = -EINTR; thread->retry_local = true; thread->sync_files = true; + thread->db = param->db; + thread->exit_status = -EINTR; + SLIST_INSERT_HEAD(¶m->threads, thread, next); error = pthread_create(&thread->pid, NULL, do_file_validation, thread); if (error) { - pr_op_err("Could not spawn the file validation thread: %s", - strerror(error)); + pr_op_err("Could not spawn validation thread for %s: %s", + tal_file, strerror(error)); free(thread->tal_file); free(thread); - return error; } - SLIST_INSERT_HEAD(&t_param->threads, thread, next); - - return 0; + return error; } int @@ -633,10 +621,10 @@ perform_standalone_validation(struct db_table *table) param.db = table; SLIST_INIT(¶m.threads); - error = process_file_or_dir(config_get_tal(), TAL_FILE_EXTENSION, true, - __do_file_validation, ¶m); + /* TODO (fine) Maybe don't use threads if there's only one TAL */ + error = foreach_file(config_get_tal(), ".tal", true, spawn_tal_thread, + ¶m); if (error) { - /* End all thread data */ while (!SLIST_EMPTY(¶m.threads)) { thread = SLIST_FIRST(¶m.threads); SLIST_REMOVE_HEAD(¶m.threads, next); @@ -650,13 +638,13 @@ perform_standalone_validation(struct db_table *table) thread = SLIST_FIRST(¶m.threads); tmperr = pthread_join(thread->pid, NULL); if (tmperr) - pr_crit("pthread_join() threw %d on the '%s' thread.", - tmperr, thread->tal_file); + pr_crit("pthread_join() threw %d (%s) on the '%s' thread.", + tmperr, strerror(tmperr), thread->tal_file); SLIST_REMOVE_HEAD(¶m.threads, next); if (thread->exit_status) { error = thread->exit_status; - pr_op_warn("Validation from TAL '%s' yielded error, discarding any other validation results.", - thread->tal_file); + pr_op_warn("Validation from TAL '%s' yielded error %d (%s); discarding all validation results.", + thread->tal_file, error, strerror(abs(error))); } thread_destroy(thread); } diff --git a/src/rtr/db/vrps.c b/src/rtr/db/vrps.c index 65a39f9f..7297dd77 100644 --- a/src/rtr/db/vrps.c +++ b/src/rtr/db/vrps.c @@ -256,9 +256,11 @@ __compute_deltas(struct db_table *old_base, struct db_table *new_base, * - Downloads tree * - Validates tree * - Updates RTR state + * + * If the database changed, @changed will be true. Meant for RTR notificates. */ static int -__vrps_update(bool *notify_clients) +__vrps_update(bool *changed) { /* * This function is the only writer, and it runs once at a time. @@ -271,8 +273,8 @@ __vrps_update(bool *notify_clients) struct deltas *new_deltas; int error; - if (notify_clients) - *notify_clients = false; + if (changed) + *changed = false; old_base = state.base; new_base = NULL; @@ -295,8 +297,7 @@ __vrps_update(bool *notify_clients) */ output_print_data(new_base); - error = __compute_deltas(old_base, new_base, notify_clients, - &new_deltas); + error = __compute_deltas(old_base, new_base, changed, &new_deltas); if (error) { /* * Deltas are nice-to haves. As long as state.base is correct, @@ -371,7 +372,7 @@ vrps_update(bool *changed) if (config_get_mode() == SERVER) pr_op_info("- Serial: %u", serial); if (start != ((time_t) -1) && finish != ((time_t) -1)) - pr_op_info("- Real execution time: %.0lf secs.", difftime(finish, start)); + pr_op_info("- Real execution time: %.0lfs", difftime(finish, start)); return error; } diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c index f339f184..83ae4f0a 100644 --- a/src/rtr/rtr.c +++ b/src/rtr/rtr.c @@ -11,8 +11,11 @@ #include "rtr/err_pdu.h" #include "rtr/pdu.h" #include "rtr/pdu_handler.h" +#include "rtr/pdu_sender.h" #include "rtr/pdu_stream.h" +#include "rtr/db/vrps.h" #include "thread/thread_pool.h" +#include "types/serial.h" struct rtr_server { int fd; @@ -678,25 +681,29 @@ void rtr_stop(void) destroy_db(); } -int -rtr_foreach_client(rtr_foreach_client_cb cb, void *arg) +void +rtr_notify(void) { + serial_t serial; struct pdu_stream **client; int fd; - int error = 0; + int error; + + error = get_last_serial_number(&serial); + if (error) { + pr_op_info("Can't notify RTR clients: %d (%s)", error, + strerror(abs(error))); + return; + } mutex_lock(&lock); ARRAYLIST_FOREACH(&clients, client) { fd = pdustream_fd(*client); - if (fd != -1) { - error = cb(fd, pdustream_version(*client), arg); - if (error) - break; - } + if (fd != -1) + send_serial_notify_pdu(fd, pdustream_version(*client), + serial); } mutex_unlock(&lock); - - return error; } diff --git a/src/rtr/rtr.h b/src/rtr/rtr.h index 6e38187b..f85aa16f 100644 --- a/src/rtr/rtr.h +++ b/src/rtr/rtr.h @@ -4,7 +4,6 @@ int rtr_start(void); void rtr_stop(void); -typedef int (*rtr_foreach_client_cb)(int, int, void *); -int rtr_foreach_client(rtr_foreach_client_cb, void *); +void rtr_notify(void); #endif /* RTR_RTR_H_ */ diff --git a/src/slurm/slurm_loader.c b/src/slurm/slurm_loader.c index 8d40d53b..d90ed757 100644 --- a/src/slurm/slurm_loader.c +++ b/src/slurm/slurm_loader.c @@ -30,7 +30,7 @@ load_slurm_files(struct slurm_csum_list *csums, struct db_slurm **result) if (error) return error; - error = process_file_or_dir(config_get_slurm(), SLURM_FILE_EXTENSION, + error = foreach_file(config_get_slurm(), SLURM_FILE_EXTENSION, false, slurm_parse, db); if (error) goto cancel; @@ -162,7 +162,7 @@ slurm_load_checksums(struct slurm_csum_list *csums) SLIST_INIT(csums); csums->list_size = 0; - error = process_file_or_dir(config_get_slurm(), SLURM_FILE_EXTENSION, + error = foreach_file(config_get_slurm(), SLURM_FILE_EXTENSION, false, __slurm_load_checksums, csums); if (error) destroy_local_csum_list(csums); diff --git a/src/validation_run.c b/src/validation_run.c deleted file mode 100644 index e8221ae9..00000000 --- a/src/validation_run.c +++ /dev/null @@ -1,60 +0,0 @@ -#include "validation_run.h" - -#include - -#include "config.h" -#include "log.h" -#include "notify.h" -#include "config/mode.h" -#include "rtr/db/vrps.h" - -/* Runs a single cycle, use at standalone mode or before running RTR server */ -int -validation_run_first(void) -{ - pr_op_info("Please wait. Validating..."); - - if (vrps_update(NULL) != 0) - return pr_op_err("Validation unsuccessful; results unusable."); - - if (config_get_mode() == SERVER) - pr_op_info("Validation complete; waiting for routers."); - else - pr_op_info("Validation complete."); - - return 0; -} - -/* Run a validation cycle each 'server.interval.validation' secs */ -int -validation_run_cycle(void) -{ - unsigned int validation_interval; - bool changed; - int error; - - validation_interval = config_get_validation_interval(); - do { - sleep(validation_interval); - - error = vrps_update(&changed); - if (error == -EINTR) - break; /* Process interrupted, terminate thread */ - - if (error) { - pr_op_err("Error while trying to update the ROA database. Sleeping..."); - continue; - } - - if (changed) { - error = notify_clients(); - if (error) - pr_op_debug("Couldn't notify clients of the new VRPs. (Error code %d.) Sleeping...", - error); - else - pr_op_debug("Database updated successfully. Sleeping..."); - } - } while (true); - - return error; -} diff --git a/src/validation_run.h b/src/validation_run.h deleted file mode 100644 index 7506cdd9..00000000 --- a/src/validation_run.h +++ /dev/null @@ -1,7 +0,0 @@ -#ifndef SRC_VALIDATION_RUN_H_ -#define SRC_VALIDATION_RUN_H_ - -int validation_run_first(void); -int validation_run_cycle(void); - -#endif /* SRC_VALIDATION_RUN_H_ */