From 5167cc5982475513688e28440e410abc112f9498 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 26 Oct 2022 15:17:54 +0300 Subject: [PATCH 1/2] TCP and TLS DNS tests: properly pass connection callback After the loop manager refactoring TCP DNS and TLS DNS unit tests ended up broken. The problem is that in these unit tests the code is written in such a way that for establishing a new connection tcpdns_connect() and tlsdns_connect() functions are used. However, in these tests as a connection callback function connect_connect_cb() is used. The function logic is responsible for determining the function for establishing subsequent connection. To do so, it called get_stream_connect_function() ... which can return only tcp_connect() or tls_connect(), not tcpdns_connect() or tlsdns_connect(). That is definitely *not* what was implied. All this time the unit tests were testing something, but now what was intended. This commit fixes the problem by passing the tcpdns_connect() and tlsdns_connect() function pointers to connect_connect_cb(). --- tests/isc/netmgr_common.c | 5 ++--- tests/isc/tcpdns_test.c | 6 +++--- tests/isc/tlsdns_test.c | 8 ++++---- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/isc/netmgr_common.c b/tests/isc/netmgr_common.c index 3d57d940d0..2324555a6a 100644 --- a/tests/isc/netmgr_common.c +++ b/tests/isc/netmgr_common.c @@ -354,8 +354,6 @@ void connect_connect_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { isc_nmhandle_t *readhandle = NULL; - UNUSED(cbarg); - F(); isc_refcount_decrement(&active_cconnects); @@ -369,7 +367,8 @@ connect_connect_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { do_cconnects_shutdown(loopmgr); } else if (do_send) { isc_job_run(loopmgr, stream_recv_send_connect, - get_stream_connect_function()); + (cbarg == NULL ? get_stream_connect_function() + : (stream_connect_function)cbarg)); } isc_refcount_increment0(&active_creads); diff --git a/tests/isc/tcpdns_test.c b/tests/isc/tcpdns_test.c index 4a66629e0c..43b75a70a3 100644 --- a/tests/isc/tcpdns_test.c +++ b/tests/isc/tcpdns_test.c @@ -61,7 +61,7 @@ start_listening(uint32_t nworkers, isc_nm_accept_cb_t accept_cb, static void tcpdns_connect(isc_nm_t *nm) { isc_nm_tcpdnsconnect(nm, &tcp_connect_addr, &tcp_listen_addr, - connect_connect_cb, NULL, T_CONNECT); + connect_connect_cb, tcpdns_connect, T_CONNECT); } ISC_LOOP_TEST_IMPL(tcpdns_noop) { @@ -70,7 +70,7 @@ ISC_LOOP_TEST_IMPL(tcpdns_noop) { connect_readcb = NULL; isc_refcount_increment0(&active_cconnects); isc_nm_tcpdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, - connect_success_cb, NULL, T_CONNECT); + connect_success_cb, tcpdns_connect, T_CONNECT); } ISC_LOOP_TEST_IMPL(tcpdns_noresponse) { @@ -78,7 +78,7 @@ ISC_LOOP_TEST_IMPL(tcpdns_noresponse) { isc_refcount_increment0(&active_cconnects); isc_nm_tcpdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, - connect_connect_cb, NULL, T_CONNECT); + connect_connect_cb, tcpdns_connect, T_CONNECT); } ISC_LOOP_TEST_IMPL(tcpdns_timeout_recovery) { diff --git a/tests/isc/tlsdns_test.c b/tests/isc/tlsdns_test.c index 2dbc3d532e..61b7fca51f 100644 --- a/tests/isc/tlsdns_test.c +++ b/tests/isc/tlsdns_test.c @@ -60,7 +60,7 @@ start_listening(uint32_t nworkers, isc_nm_accept_cb_t accept_cb, static void tlsdns_connect(isc_nm_t *nm) { isc_nm_tlsdnsconnect(nm, &tcp_connect_addr, &tcp_listen_addr, - connect_connect_cb, NULL, T_CONNECT, + connect_connect_cb, tlsdns_connect, T_CONNECT, tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); } @@ -70,7 +70,7 @@ ISC_LOOP_TEST_IMPL(tlsdns_noop) { connect_readcb = NULL; isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, - connect_success_cb, NULL, T_CONNECT, + connect_success_cb, tlsdns_connect, T_CONNECT, tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); } @@ -79,7 +79,7 @@ ISC_LOOP_TEST_IMPL(tlsdns_noresponse) { isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, - connect_connect_cb, NULL, T_CONNECT, + connect_connect_cb, tlsdns_connect, T_CONNECT, tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); } @@ -100,7 +100,7 @@ ISC_LOOP_TEST_IMPL(tlsdns_timeout_recovery) { isc_nm_settimeouts(connect_nm, T_SOFT, T_SOFT, T_SOFT, T_SOFT); isc_refcount_increment0(&active_cconnects); isc_nm_tlsdnsconnect(connect_nm, &tcp_connect_addr, &tcp_listen_addr, - connect_connect_cb, NULL, T_SOFT, + connect_connect_cb, tlsdns_connect, T_SOFT, tcp_connect_tlsctx, tcp_tlsctx_client_sess_cache); } From cad73b95bff3ee1d3c4301f8fa8a08aa0794d491 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 26 Oct 2022 15:41:30 +0300 Subject: [PATCH 2/2] TLS DNS unit tests: do not share the port with TCP DNS tests TLS DNS unit tests were sharing the port with TCP DNS tests by mistake. That could have caused conflicts between the two, when running the unit tests in parallel. This commit fixes that. --- tests/isc/tlsdns_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/isc/tlsdns_test.c b/tests/isc/tlsdns_test.c index 61b7fca51f..a8de6bce4a 100644 --- a/tests/isc/tlsdns_test.c +++ b/tests/isc/tlsdns_test.c @@ -150,7 +150,7 @@ ISC_TEST_LIST_END static int tlsdns_setup(void **state __attribute__((__unused__))) { - stream_port = TCPDNS_TEST_PORT; + stream_port = TLSDNS_TEST_PORT; return (0); }