From: James Jones Date: Fri, 4 Mar 2022 14:09:49 +0000 (-0600) Subject: Add fr_trunk_verify() and FR_TRUNK_VERIFY() to assist with detecting trunk issues... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a8ef88f28614122f8bb1291ea0a0be4e6e6ad000;p=thirdparty%2Ffreeradius-server.git Add fr_trunk_verify() and FR_TRUNK_VERIFY() to assist with detecting trunk issues. (#4379) --- diff --git a/src/lib/server/trunk.c b/src/lib/server/trunk.c index 99b098329bf..187bcd14aa1 100644 --- a/src/lib/server/trunk.c +++ b/src/lib/server/trunk.c @@ -4778,3 +4778,253 @@ fr_trunk_t *fr_trunk_alloc(TALLOC_CTX *ctx, fr_event_list_t *el, return trunk; } + +#ifndef TALLOC_GET_TYPE_ABORT_NOOP +/** Verify a trunk + * + * A trunk has some number of connections, which each have some number of requests. The connections and + * requests are in differing kinds of containers depending on their state and how they are used, and may + * have fields that can only be validated by comparison with a parent. We had planned on passing a "context" + * down with the ancestral values, but that breaks the foo_verify() API. Each foo_verify() will only verify the + * foo's children. + */ +void fr_trunk_verify(char const *file, int line, fr_trunk_t *trunk) +{ + fr_fatal_assert_msg(trunk, "CONSISTENCY CHECK FAILED %s[%i]: fr_trunk_t pointer was NULL", file, line); + (void) talloc_get_type_abort(trunk, fr_trunk_t); + + for (size_t i = 0; i < NUM_ELEMENTS(trunk->watch); i++) { + fr_dlist_verify(file, line, &trunk->watch[i]); + } + +#define IO_FUNC_VERIFY(_func) \ + fr_fatal_assert_msg(trunk->funcs._func, "CONSISTENCY_CHECK_FAILED %s[%i}: " #_func " was NULL", file, line) + + /* + * Only a few of the function pointers *must* be non-NULL.. + */ + IO_FUNC_VERIFY(connection_alloc); + IO_FUNC_VERIFY(connection_prioritise); + IO_FUNC_VERIFY(request_prioritise); + +#define TRUNK_TCONN_CHECKS(_tconn, _state) \ +do { \ + fr_fatal_assert_msg(trunk == _tconn->pub.trunk, \ + "CONSISTENCY_CHECK_FAILED %s[%i}: connection-trunk mismatch", file, line); \ + fr_fatal_assert_msg(_state == _tconn->pub.state, \ + "CONSISTENCY_CHECK_FAILED %s[%i}: connection-state mismatch", file, line); \ +} while (0) + +#define TCONN_DLIST_VERIFY(_dlist, _state) \ +do { \ + fr_dlist_verify(file, line, &(trunk->_dlist)); \ + fr_dlist_foreach(&(trunk->_dlist), fr_trunk_connection_t, tconn) { \ + fr_trunk_connection_verify(file, line, tconn); \ + TRUNK_TCONN_CHECKS(tconn, _state); \ + } \ +} while (0) + +#define TCONN_MINMAX_HEAP_VERIFY(_heap, _state) \ +do {\ + fr_minmax_heap_verify(file, line, trunk->_heap); \ + fr_minmax_heap_foreach(trunk->_heap, fr_trunk_connection_t, tconn) { \ + fr_trunk_connection_verify(file, line, tconn); \ + TRUNK_TCONN_CHECKS(tconn, _state); \ + }} \ +} while (0) + + FR_DLIST_VERIFY(&(trunk->free_requests)); + FR_HEAP_VERIFY(trunk->backlog); + + TCONN_DLIST_VERIFY(init, FR_TRUNK_CONN_INIT); + TCONN_DLIST_VERIFY(connecting, FR_TRUNK_CONN_CONNECTING); + TCONN_MINMAX_HEAP_VERIFY(active, FR_TRUNK_CONN_ACTIVE); + TCONN_DLIST_VERIFY(full, FR_TRUNK_CONN_FULL); + TCONN_DLIST_VERIFY(inactive, FR_TRUNK_CONN_INACTIVE); + TCONN_DLIST_VERIFY(inactive_draining, FR_TRUNK_CONN_INACTIVE_DRAINING); + /* TCONN_DLIST_VERIFY(failed, ???); */ + TCONN_DLIST_VERIFY(closed, FR_TRUNK_CONN_CLOSED); + TCONN_DLIST_VERIFY(draining, FR_TRUNK_CONN_DRAINING); + TCONN_DLIST_VERIFY(draining_to_free, FR_TRUNK_CONN_DRAINING_TO_FREE); + TCONN_DLIST_VERIFY(to_free, FR_TRUNK_CONN_HALTED); +} + +void fr_trunk_connection_verify(char const *file, int line, fr_trunk_connection_t *tconn) +{ + fr_fatal_assert_msg(tconn, "CONSISTENCY CHECK FAILED %s[%i]: fr_trunk_connection_t pointer was NULL", file, line); + (void) talloc_get_type_abort(tconn, fr_trunk_connection_t); + + (void) talloc_get_type_abort(tconn->pub.trunk, fr_trunk_t); + + /* + * shouldn't be both in heap and on list--but it doesn't look like moves + * to active heap wipe the dlist pointers. + */ + +#define TCONN_TREQ_CHECKS(_treq, _state) \ +do { \ + fr_fatal_assert_msg(tconn == _treq->pub.tconn, \ + "CONSISTENCY_CHECK_FAILED %s[%i}: trunk request-tconn mismatch", file, line); \ + fr_fatal_assert_msg(tconn->pub.trunk == _treq->pub.trunk, \ + "CONSISTENCY_CHECK_FAILED %s[%i}: trunk request-trunk mismatch", file, line); \ + fr_fatal_assert_msg(_state == _treq->pub.state, \ + "CONSISTENCY_CHECK_FAILED %s[%i}: trunk request-state mismatch", file, line); \ +} while (0); + +#define TREQ_DLIST_VERIFY(_dlist, _state) \ +do { \ + fr_dlist_verify(file, line, &(tconn->_dlist)); \ + fr_dlist_foreach(&(tconn->_dlist), fr_trunk_request_t, treq) { \ + fr_trunk_request_verify(file, line, treq); \ + TCONN_TREQ_CHECKS(treq, _state); \ + } \ +} while (0); + +#define TREQ_HEAP_VERIFY(_heap, _state) \ +do { \ + fr_heap_iter_t _iter; \ + fr_heap_verify(file, line, tconn->_heap); \ + for (fr_trunk_request_t *treq = fr_heap_iter_init(tconn->_heap, &_iter); \ + treq; \ + treq = fr_heap_iter_next(tconn->_heap, &_iter)) { \ + fr_trunk_request_verify(file, line, treq); \ + TCONN_TREQ_CHECKS(treq, _state); \ + } \ +} while (0); + +#define TREQ_OPTION_VERIFY(_option, _state) \ +do { \ + if (tconn->_option) { \ + fr_trunk_request_verify(file, line, tconn->_option); \ + TCONN_TREQ_CHECKS(tconn->_option, _state); \ + } \ +} while (0); + + /* verify associated requests */ + TREQ_HEAP_VERIFY(pending, FR_TRUNK_REQUEST_STATE_PENDING); + TREQ_DLIST_VERIFY(sent, FR_TRUNK_REQUEST_STATE_SENT); + TREQ_DLIST_VERIFY(cancel, FR_TRUNK_REQUEST_STATE_CANCEL); + TREQ_DLIST_VERIFY(cancel_sent, FR_TRUNK_REQUEST_STATE_CANCEL_SENT); + TREQ_OPTION_VERIFY(partial, FR_TRUNK_REQUEST_STATE_PARTIAL); + TREQ_OPTION_VERIFY(cancel_partial, FR_TRUNK_REQUEST_STATE_CANCEL_PARTIAL); +} + +void fr_trunk_request_verify(char const *file, int line, fr_trunk_request_t *treq) +{ + fr_fatal_assert_msg(treq, "CONSISTENCY CHECK FAILED %s[%i]: fr_trunk_request_t pointer was NULL", file, line); + (void) talloc_get_type_abort(treq, fr_trunk_request_t); + +#ifdef WITH_VERIFY_PTR + if (treq->pub.request) request_verify(file, line, treq->pub.request); +#endif +} + + +bool fr_trunk_search(fr_trunk_t *trunk, void *ptr) +{ +#define TCONN_DLIST_SEARCH(_dlist) \ +do { \ + fr_dlist_foreach(&(trunk->_dlist), fr_trunk_connection_t, tconn) { \ + if (ptr == tconn) { \ + fr_fprintf(stderr, "fr_trunk_search: tconn %p on " #_dlist "\n", ptr); \ + return true; \ + } \ + if (fr_trunk_connection_search(tconn, ptr)) { \ + fr_fprintf(stderr, " in tconn %p on " #_dlist "\n", tconn); \ + return true; \ + } \ + } \ +} while (0) + +#define TCONN_MINMAX_HEAP_SEARCH(_heap) \ +do { \ + fr_minmax_heap_foreach(trunk->_heap, fr_trunk_connection_t, tconn) { \ + if (ptr == tconn) { \ + fr_fprintf(stderr, "fr_trunk_search: tconn %p on " #_heap "\n", ptr); \ + return true; \ + } \ + if (fr_trunk_connection_search(tconn, ptr)) { \ + fr_fprintf(stderr, " on tconn %p on " #_heap "\n", tconn); \ + return true; \ + } \ + }}\ +} while (0) + + TCONN_DLIST_SEARCH(init); + TCONN_DLIST_SEARCH(connecting); + TCONN_MINMAX_HEAP_SEARCH(active); + TCONN_DLIST_SEARCH(full); + TCONN_DLIST_SEARCH(inactive); + TCONN_DLIST_SEARCH(inactive_draining); + TCONN_DLIST_SEARCH(failed); + TCONN_DLIST_SEARCH(closed); + TCONN_DLIST_SEARCH(draining); + TCONN_DLIST_SEARCH(draining_to_free); + TCONN_DLIST_SEARCH(to_free); + + return false; +} + +bool fr_trunk_connection_search(fr_trunk_connection_t *tconn, void *ptr) +{ +#define TREQ_DLIST_SEARCH(_dlist) \ +do { \ + fr_dlist_foreach(&(tconn->_dlist), fr_trunk_request_t, treq) { \ + if (ptr == treq) { \ + fr_fprintf(stderr, "fr_trunk_search: treq %p on " #_dlist "\n", ptr); \ + return true; \ + } \ + if (fr_trunk_request_search(treq, ptr)) { \ + fr_fprintf(stderr, "fr_trunk_search: preq %p found on " #_dlist, ptr); \ + return true; \ + } \ + } \ +} while (0); + +#define TREQ_HEAP_SEARCH(_heap) \ +do { \ + fr_heap_iter_t _iter; \ + for (fr_trunk_request_t *treq = fr_heap_iter_init(tconn->_heap, &_iter); \ + treq; \ + treq = fr_heap_iter_next(tconn->_heap, &_iter)) { \ + if (ptr == treq) { \ + fr_fprintf(stderr, "fr_trunk_search: treq %p in " #_heap "\n", ptr); \ + return true; \ + } \ + if (fr_trunk_request_search(treq, ptr)) { \ + fr_fprintf(stderr, "fr_trunk_search: preq %p found in " #_heap, ptr); \ + return true; \ + } \ + } \ +} while (0); + +#define TREQ_OPTION_SEARCH(_option) \ +do { \ + if (tconn->_option) { \ + if (ptr == tconn->_option) { \ + fr_fprintf(stderr, "fr_trunk_search: treq %p is " #_option "\n", ptr); \ + return true; \ + } \ + if (fr_trunk_request_search(tconn->_option, ptr)) { \ + fr_fprintf(stderr, "fr_trunk_search: preq %p found in " #_option, ptr); \ + return true; \ + } \ + } \ +} while (0); + + /* search associated requests */ + TREQ_HEAP_SEARCH(pending); + TREQ_DLIST_SEARCH(sent); + TREQ_DLIST_SEARCH(cancel); + TREQ_DLIST_SEARCH(cancel_sent); + TREQ_OPTION_SEARCH(partial); + TREQ_OPTION_SEARCH(cancel_partial); + + return false; +} + +bool fr_trunk_request_search(fr_trunk_request_t *treq, void *ptr) +{ + return treq->pub.preq == ptr; +} +#endif diff --git a/src/lib/server/trunk.h b/src/lib/server/trunk.h index 43c5f1f79c6..575a00c8fde 100644 --- a/src/lib/server/trunk.h +++ b/src/lib/server/trunk.h @@ -877,6 +877,28 @@ fr_trunk_watch_entry_t *fr_trunk_add_watch(fr_trunk_t *trunk, fr_trunk_state_t s int fr_trunk_del_watch(fr_trunk_t *trunk, fr_trunk_state_t state, fr_trunk_watch_t watch); /** @} */ +#ifndef TALLOC_GET_TYPE_ABORT_NOOP +void CC_HINT(nonnull(1)) fr_trunk_verify(char const *file, int line, fr_trunk_t *trunk); +void CC_HINT(nonnull(1)) fr_trunk_connection_verify(char const *file, int line, fr_trunk_connection_t *tconn); +void CC_HINT(nonnull(1)) fr_trunk_request_verify(char const *file, int line, fr_trunk_request_t *treq); + +# define FR_TRUNK_VERIFY(_trunk) fr_trunk_verify(__FILE__, __LINE__, _trunk) +# define FR_TRUNK_CONNECTION_VERIFY(_tconn) fr_trunk_connection_verify(__FILE__, __LINE__, _tconn) +# define FR_TRUNK_REQUEST_VERIFY(_treq) fr_trunk_request_verify(__FILE__, __LINE__, _treq) +#elif !defined(NDEBUG) +# define FR_TRUNK_VERIFY(_trunk) fr_assert(_trunk) +# define FR_TRUNK_CONNECTION_VERIFY(_tconn) fr_assert(_tconn) +# define FR_TRUNK_REQUEST_VERIFY(_treq) fr_assert(_treq) +#else +# define FR_TRUNK_VERIFY(_trunk) +# define FR_TRUNK_CONNECTION_VERIFY(_tconn) +# define FR_TRUNK_REQUEST_VERIFY(_treq) +#endif + +bool fr_trunk_search(fr_trunk_t *trunk, void *ptr); +bool fr_trunk_connection_search(fr_trunk_connection_t *tconn, void *ptr); +bool fr_trunk_request_search(fr_trunk_request_t *treq, void *ptr); + #undef _CONST #ifdef __cplusplus diff --git a/src/lib/server/trunk_tests.c b/src/lib/server/trunk_tests.c index 62015f86839..8a9bc4c1b37 100644 --- a/src/lib/server/trunk_tests.c +++ b/src/lib/server/trunk_tests.c @@ -1500,6 +1500,7 @@ static void test_connection_levels_max(void) test_time_base = fr_time_add_time_delta(test_time_base, fr_time_delta_from_nsec(NSEC * 0.5)); trunk = test_setup_trunk(ctx, el, &conf, true, NULL); + FR_TRUNK_VERIFY(trunk); /* * Queuing a request should start a connection. @@ -1507,6 +1508,7 @@ static void test_connection_levels_max(void) TEST_CASE("C0, R1 - Enqueue should spawn"); ALLOC_REQ(a); TEST_CHECK(fr_trunk_request_enqueue(&treq_a, trunk, NULL, preq_a, NULL) == FR_TRUNK_ENQUEUE_IN_BACKLOG); + FR_TRUNK_VERIFY(trunk); /* * Like test_connection_start_on_enqueue(), you have to process the backlog @@ -1516,6 +1518,7 @@ static void test_connection_levels_max(void) fr_event_service(el); TEST_CHECK_LEN(fr_trunk_connection_count_by_state(trunk, FR_TRUNK_CONN_CONNECTING), 1); + FR_TRUNK_VERIFY(trunk); /* * Queuing another request should *NOT* start another connection @@ -1524,21 +1527,25 @@ static void test_connection_levels_max(void) ALLOC_REQ(b); TEST_CHECK(fr_trunk_request_enqueue(&treq_b, trunk, NULL, preq_b, NULL) == FR_TRUNK_ENQUEUE_IN_BACKLOG); TEST_CHECK_LEN(fr_trunk_connection_count_by_state(trunk, FR_TRUNK_CONN_CONNECTING), 1); + FR_TRUNK_VERIFY(trunk); TEST_CASE("C1 connecting, R3 - MUST NOT spawn"); ALLOC_REQ(c); TEST_CHECK(fr_trunk_request_enqueue(&treq_c, trunk, NULL, preq_c, NULL) == FR_TRUNK_ENQUEUE_IN_BACKLOG); TEST_CHECK_LEN(fr_trunk_connection_count_by_state(trunk, FR_TRUNK_CONN_CONNECTING), 1); + FR_TRUNK_VERIFY(trunk); TEST_CASE("C1 connecting, R4 - MUST NOT spawn"); ALLOC_REQ(d); TEST_CHECK(fr_trunk_request_enqueue(&treq_d, trunk, NULL, preq_d, NULL) == FR_TRUNK_ENQUEUE_IN_BACKLOG); TEST_CHECK_LEN(fr_trunk_connection_count_by_state(trunk, FR_TRUNK_CONN_CONNECTING), 1); + FR_TRUNK_VERIFY(trunk); TEST_CASE("C1 connecting, R5 - MUST NOT spawn, NO CAPACITY"); ALLOC_REQ(e); TEST_CHECK(fr_trunk_request_enqueue(&treq_e, trunk, NULL, preq_e, NULL) == FR_TRUNK_ENQUEUE_NO_CAPACITY); TEST_CHECK_LEN(fr_trunk_connection_count_by_state(trunk, FR_TRUNK_CONN_CONNECTING), 1); + FR_TRUNK_VERIFY(trunk); /* * Allowing connection to open @@ -1549,6 +1556,7 @@ static void test_connection_levels_max(void) TEST_CASE("C1 active, R4 - Check pending 2"); TEST_CHECK_LEN(fr_trunk_request_count_by_state(trunk, FR_TRUNK_CONN_ALL, FR_TRUNK_REQUEST_STATE_PENDING), 2); TEST_CHECK_LEN(fr_trunk_request_count_by_state(trunk, FR_TRUNK_CONN_ALL, FR_TRUNK_REQUEST_STATE_BACKLOG), 2); + FR_TRUNK_VERIFY(trunk); /* * Sending requests @@ -1558,6 +1566,7 @@ static void test_connection_levels_max(void) TEST_CASE("C1 active, R4 - Check sent 2"); TEST_CHECK_LEN(fr_trunk_request_count_by_state(trunk, FR_TRUNK_CONN_ALL, FR_TRUNK_REQUEST_STATE_SENT), 2); + FR_TRUNK_VERIFY(trunk); /* * Looping I/O @@ -1583,6 +1592,7 @@ static void test_connection_levels_max(void) TEST_CHECK_LEN(fr_trunk_request_count_by_state(trunk, FR_TRUNK_CONN_ALL, FR_TRUNK_REQUEST_STATE_PENDING), 2); TEST_CHECK_LEN(fr_trunk_request_count_by_state(trunk, FR_TRUNK_CONN_ALL, FR_TRUNK_REQUEST_STATE_BACKLOG), 0); + FR_TRUNK_VERIFY(trunk); TEST_CASE("C1 active, R0 - Check complete 2, pending 0"); @@ -1615,6 +1625,7 @@ static void test_connection_levels_max(void) TEST_CHECK(preq_d->freed == true); TEST_CHECK(fr_trunk_request_count_by_state(trunk, FR_TRUNK_CONN_ALL, FR_TRUNK_REQUEST_STATE_ALL) == 0); + FR_TRUNK_VERIFY(trunk); talloc_free(trunk); talloc_free(ctx); @@ -1832,6 +1843,8 @@ static void test_enqueue_and_io_speed(void) MEM(preq_array = talloc_array(ctx, test_proto_request_t *, requests)); + DEBUG_LVL_SET; + TEST_CASE("Enqueue requests"); enqueue_start = fr_time(); // ProfilerStart(getenv("FR_PROFILE"));