From f388b71378e6284e592a4f53be12ded516ab80e1 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 13 Jul 2021 12:32:47 +0300 Subject: [PATCH 1/2] Get rid of RW locks in the DoH code This commit gets rid of RW locks in a hot path of the DoH code. In the original design, it was implied that we add new endpoints after the HTTP listener was created. Such a design implies some locking. We do not need such flexibility, though. Instead, we could build a set of endpoints before the HTTP listener gets created. Such a design does not need RW locks at all. --- bin/tests/test_server.c | 12 ++- lib/isc/include/isc/netmgr.h | 51 ++++++++- lib/isc/include/isc/types.h | 5 + lib/isc/netmgr/http.c | 198 ++++++++++++++++++++++------------- lib/isc/netmgr/netmgr-int.h | 15 ++- lib/isc/tests/doh_test.c | 104 ++++++++++-------- lib/ns/interfacemgr.c | 30 +++--- 7 files changed, 277 insertions(+), 138 deletions(-) diff --git a/bin/tests/test_server.c b/bin/tests/test_server.c index 8b0d8ac8a9..b684fdb24d 100644 --- a/bin/tests/test_server.c +++ b/bin/tests/test_server.c @@ -286,15 +286,19 @@ run(void) { case HTTPS: case HTTP: { bool is_https = protocol == HTTPS; + isc_nm_http_endpoints_t *eps = NULL; if (is_https) { isc_tlsctx_createserver(NULL, NULL, &tls_ctx); } - result = isc_nm_listenhttp(netmgr, &sockaddr, 0, NULL, tls_ctx, - 0, &sock); + eps = isc_nm_http_endpoints_new(mctx); + result = isc_nm_http_endpoints_add(eps, DEFAULT_DOH_PATH, + read_cb, NULL, 0); + if (result == ISC_R_SUCCESS) { - result = isc_nm_http_endpoint(sock, DEFAULT_DOH_PATH, - read_cb, NULL, 0); + result = isc_nm_listenhttp(netmgr, &sockaddr, 0, NULL, + tls_ctx, eps, 0, &sock); } + isc_nm_http_endpoints_detach(&eps); } break; #endif default: diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index a7ecf01aba..4b54c4a806 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -499,11 +499,56 @@ isc_nm_httpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, isc_result_t isc_nm_listenhttp(isc_nm_t *mgr, isc_sockaddr_t *iface, int backlog, isc_quota_t *quota, isc_tlsctx_t *ctx, - uint32_t max_concurrent_streams, isc_nmsocket_t **sockp); + isc_nm_http_endpoints_t *eps, uint32_t max_concurrent_streams, + isc_nmsocket_t **sockp); + +isc_nm_http_endpoints_t * +isc_nm_http_endpoints_new(isc_mem_t *mctx); +/*%< + * Create a new, empty HTTP endpoints set object. + * + * Requires: + * \li 'mctx' a valid memory context object. + */ isc_result_t -isc_nm_http_endpoint(isc_nmsocket_t *sock, const char *uri, isc_nm_recv_cb_t cb, - void *cbarg, size_t extrahandlesize); +isc_nm_http_endpoints_add(isc_nm_http_endpoints_t *restrict eps, + const char *uri, const isc_nm_recv_cb_t cb, + void *cbarg, const size_t extrahandlesize); +/*%< Adds a new endpoint to the given HTTP endpoints set object. + * + * NOTE: adding an endpoint is allowed only if the endpoint object has + * not been passed to isc_nm_listenhttp() yet. + * + * Requires: + * \li 'eps' is a valid pointer to a valid isc_nm_http_endpoints_t + * object; + * \li 'uri' is a valid pointer to a string of length > 0; + * \li 'cb' is a valid pointer to a read callback function. + */ + +void +isc_nm_http_endpoints_attach(isc_nm_http_endpoints_t * source, + isc_nm_http_endpoints_t **targetp); +/*%< + * Attaches to an HTTP endpoints set object. + * + * Requires: + * \li 'source' is a non-NULL pointer to a valid + * isc_nm_http_endpoints_t object; + * \li 'target' is a pointer to a pointer, containing NULL. + */ + +void +isc_nm_http_endpoints_detach(isc_nm_http_endpoints_t **restrict epsp); +/*%< + * Detaches from an HTTP endpoints set object. When reference count + * reaches 0, the object get deleted. + * + * Requires: + * \li 'epsp' is a pointer to a pointer to a valid + * isc_nm_http_endpoints_t object. + */ bool isc_nm_is_http_handle(isc_nmhandle_t *handle); diff --git a/lib/isc/include/isc/types.h b/lib/isc/include/isc/types.h index 4581d0bc9d..84f3be07e1 100644 --- a/lib/isc/include/isc/types.h +++ b/lib/isc/include/isc/types.h @@ -92,6 +92,11 @@ typedef struct isc_time isc_time_t; /*%< Time */ typedef struct isc_timer isc_timer_t; /*%< Timer */ typedef struct isc_timermgr isc_timermgr_t; /*%< Timer Manager */ +#if HAVE_LIBNGHTTP2 +typedef struct isc_nm_http_endpoints isc_nm_http_endpoints_t; +/*%< HTTP endpoints set */ +#endif /* HAVE_LIBNGHTTP2 */ + typedef void (*isc_taskaction_t)(isc_task_t *, isc_event_t *); typedef int (*isc_sockfdwatch_t)(isc_task_t *, isc_socket_t *, void *, int); diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index a8132e69a9..4d19593aef 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -213,6 +213,10 @@ static void server_call_cb(isc_nmsocket_t *socket, isc_nm_http_session_t *session, const isc_result_t result, isc_region_t *data); +static isc_nm_httphandler_t * +http_endpoints_find(const char *request_path, + const isc_nm_http_endpoints_t *restrict eps); + static bool http_session_active(isc_nm_http_session_t *session) { REQUIRE(VALID_HTTP2_SESSION(session)); @@ -1653,27 +1657,15 @@ server_on_begin_headers_callback(nghttp2_session *ngsession, static isc_nm_httphandler_t * find_server_request_handler(const char *request_path, - isc_nmsocket_t *serversocket) { + const isc_nmsocket_t *serversocket) { isc_nm_httphandler_t *handler = NULL; REQUIRE(VALID_NMSOCK(serversocket)); - if (request_path == NULL || *request_path == '\0') { - return (NULL); - } - - RWLOCK(&serversocket->h2.lock, isc_rwlocktype_read); if (atomic_load(&serversocket->listening)) { - for (handler = ISC_LIST_HEAD(serversocket->h2.handlers); - handler != NULL; handler = ISC_LIST_NEXT(handler, link)) - { - if (!strcmp(request_path, handler->path)) { - break; - } - } + handler = http_endpoints_find( + request_path, serversocket->h2.listener_endpoints); } - RWUNLOCK(&serversocket->h2.lock, isc_rwlocktype_read); - return (handler); } @@ -2418,10 +2410,15 @@ httplisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { isc_result_t isc_nm_listenhttp(isc_nm_t *mgr, isc_sockaddr_t *iface, int backlog, isc_quota_t *quota, isc_tlsctx_t *ctx, - uint32_t max_concurrent_streams, isc_nmsocket_t **sockp) { + isc_nm_http_endpoints_t *eps, uint32_t max_concurrent_streams, + isc_nmsocket_t **sockp) { isc_nmsocket_t *sock = NULL; isc_result_t result; + REQUIRE(!ISC_LIST_EMPTY(eps->handlers)); + REQUIRE(!ISC_LIST_EMPTY(eps->handler_cbargs)); + REQUIRE(atomic_load(&eps->in_use) == false); + sock = isc_mem_get(mgr->mctx, sizeof(*sock)); isc__nmsocket_init(sock, mgr, isc_nm_httplistener, iface); sock->h2.max_concurrent_streams = @@ -2433,6 +2430,9 @@ isc_nm_listenhttp(isc_nm_t *mgr, isc_sockaddr_t *iface, int backlog, sock->h2.max_concurrent_streams = max_concurrent_streams; } + atomic_store(&eps->in_use, true); + isc_nm_http_endpoints_attach(eps, &sock->h2.listener_endpoints); + if (ctx != NULL) { isc_tlsctx_enable_http2server_alpn(ctx); result = isc_nm_listentls(mgr, iface, httplisten_acceptcb, sock, @@ -2462,6 +2462,96 @@ isc_nm_listenhttp(isc_nm_t *mgr, isc_sockaddr_t *iface, int backlog, return (ISC_R_SUCCESS); } +isc_nm_http_endpoints_t * +isc_nm_http_endpoints_new(isc_mem_t *mctx) { + isc_nm_http_endpoints_t *restrict eps; + REQUIRE(mctx != NULL); + + eps = isc_mem_get(mctx, sizeof(*eps)); + *eps = (isc_nm_http_endpoints_t){ .mctx = NULL }; + + isc_mem_attach(mctx, &eps->mctx); + ISC_LIST_INIT(eps->handler_cbargs); + ISC_LIST_INIT(eps->handlers); + isc_refcount_init(&eps->references, 1); + atomic_init(&eps->in_use, false); + + return eps; +} + +void +isc_nm_http_endpoints_detach(isc_nm_http_endpoints_t **restrict epsp) { + isc_nm_http_endpoints_t *restrict eps; + isc_mem_t *mctx; + isc_nm_httphandler_t *handler = NULL; + isc_nm_httpcbarg_t *httpcbarg = NULL; + + REQUIRE(epsp != NULL); + eps = *epsp; + REQUIRE(eps != NULL); + + if (isc_refcount_decrement(&eps->references) > 1) { + return; + } + + mctx = eps->mctx; + + /* Delete all handlers */ + handler = ISC_LIST_HEAD(eps->handlers); + while (handler != NULL) { + isc_nm_httphandler_t *next = NULL; + + next = ISC_LIST_NEXT(handler, link); + ISC_LIST_DEQUEUE(eps->handlers, handler, link); + isc_mem_free(mctx, handler->path); + isc_mem_put(mctx, handler, sizeof(*handler)); + handler = next; + } + + httpcbarg = ISC_LIST_HEAD(eps->handler_cbargs); + while (httpcbarg != NULL) { + isc_nm_httpcbarg_t *next = NULL; + + next = ISC_LIST_NEXT(httpcbarg, link); + ISC_LIST_DEQUEUE(eps->handler_cbargs, httpcbarg, link); + isc_mem_put(mctx, httpcbarg, sizeof(isc_nm_httpcbarg_t)); + httpcbarg = next; + } + + isc_mem_putanddetach(&mctx, eps, sizeof(*eps)); + *epsp = NULL; +} + +void +isc_nm_http_endpoints_attach(isc_nm_http_endpoints_t *source, + isc_nm_http_endpoints_t **targetp) { + REQUIRE(targetp != NULL && *targetp == NULL); + + isc_refcount_increment(&source->references); + + *targetp = source; +} + +static isc_nm_httphandler_t * +http_endpoints_find(const char *request_path, + const isc_nm_http_endpoints_t *restrict eps) { + isc_nm_httphandler_t *handler = NULL; + + if (request_path == NULL || *request_path == '\0') { + return (NULL); + } + + for (handler = ISC_LIST_HEAD(eps->handlers); handler != NULL; + handler = ISC_LIST_NEXT(handler, link)) + { + if (!strcmp(request_path, handler->path)) { + break; + } + } + + return (handler); +} + /* * In DoH we just need to intercept the request - the response can be sent * to the client code via the nmhandle directly as it's always just the @@ -2484,40 +2574,41 @@ http_callback(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *data, } isc_result_t -isc_nm_http_endpoint(isc_nmsocket_t *sock, const char *uri, isc_nm_recv_cb_t cb, - void *cbarg, size_t extrahandlesize) { - isc_nm_httphandler_t *handler = NULL; - isc_nm_httpcbarg_t *httpcbarg = NULL; +isc_nm_http_endpoints_add(isc_nm_http_endpoints_t *restrict eps, + const char *uri, const isc_nm_recv_cb_t cb, + void *cbarg, const size_t extrahandlesize) { + isc_mem_t *mctx; + isc_nm_httphandler_t *restrict handler = NULL; + isc_nm_httpcbarg_t *restrict httpcbarg = NULL; bool newhandler = false; - REQUIRE(VALID_NMSOCK(sock)); - REQUIRE(sock->type == isc_nm_httplistener); + REQUIRE(eps != NULL); REQUIRE(isc_nm_http_path_isvalid(uri)); + REQUIRE(atomic_load(&eps->in_use) == false); - httpcbarg = isc_mem_get(sock->mgr->mctx, sizeof(isc_nm_httpcbarg_t)); + mctx = eps->mctx; + + httpcbarg = isc_mem_get(mctx, sizeof(isc_nm_httpcbarg_t)); *httpcbarg = (isc_nm_httpcbarg_t){ .cb = cb, .cbarg = cbarg }; ISC_LINK_INIT(httpcbarg, link); - if (find_server_request_handler(uri, sock) == NULL) { - handler = isc_mem_get(sock->mgr->mctx, sizeof(*handler)); + if (http_endpoints_find(uri, eps) == NULL) { + handler = isc_mem_get(mctx, sizeof(*handler)); *handler = (isc_nm_httphandler_t){ .cb = http_callback, .cbarg = httpcbarg, .extrahandlesize = extrahandlesize, - .path = isc_mem_strdup(sock->mgr->mctx, uri) + .path = isc_mem_strdup(mctx, uri) }; ISC_LINK_INIT(handler, link); newhandler = true; } - RWLOCK(&sock->h2.lock, isc_rwlocktype_write); if (newhandler) { - ISC_LIST_APPEND(sock->h2.handlers, handler, link); + ISC_LIST_APPEND(eps->handlers, handler, link); } - ISC_LIST_APPEND(sock->h2.handler_cbargs, httpcbarg, link); - RWUNLOCK(&sock->h2.lock, isc_rwlocktype_write); - + ISC_LIST_APPEND(eps->handler_cbargs, httpcbarg, link); return (ISC_R_SUCCESS); } @@ -2544,37 +2635,6 @@ isc__nm_http_stoplistening(isc_nmsocket_t *sock) { } } -static void -clear_handlers(isc_nmsocket_t *sock) { - isc_nm_httphandler_t *handler = NULL; - isc_nm_httpcbarg_t *httpcbarg = NULL; - - /* Delete all handlers */ - RWLOCK(&sock->h2.lock, isc_rwlocktype_write); - handler = ISC_LIST_HEAD(sock->h2.handlers); - while (handler != NULL) { - isc_nm_httphandler_t *next = NULL; - - next = ISC_LIST_NEXT(handler, link); - ISC_LIST_DEQUEUE(sock->h2.handlers, handler, link); - isc_mem_free(sock->mgr->mctx, handler->path); - isc_mem_put(sock->mgr->mctx, handler, sizeof(*handler)); - handler = next; - } - - httpcbarg = ISC_LIST_HEAD(sock->h2.handler_cbargs); - while (httpcbarg != NULL) { - isc_nm_httpcbarg_t *next = NULL; - - next = ISC_LIST_NEXT(httpcbarg, link); - ISC_LIST_DEQUEUE(sock->h2.handler_cbargs, httpcbarg, link); - isc_mem_put(sock->mgr->mctx, httpcbarg, - sizeof(isc_nm_httpcbarg_t)); - httpcbarg = next; - } - RWUNLOCK(&sock->h2.lock, isc_rwlocktype_write); -} - void isc__nm_async_httpstop(isc__networker_t *worker, isc__netievent_t *ev0) { isc__netievent_httpstop_t *ievent = (isc__netievent_httpstop_t *)ev0; @@ -2903,12 +2963,6 @@ isc__nm_http_initsocket(isc_nmsocket_t *sock) { .request_type = ISC_HTTP_REQ_UNSUPPORTED, .request_scheme = ISC_HTTP_SCHEME_UNSUPPORTED, }; - - if (sock->type == isc_nm_httplistener) { - ISC_LIST_INIT(sock->h2.handlers); - ISC_LIST_INIT(sock->h2.handler_cbargs); - isc_rwlock_init(&sock->h2.lock, 0, 1); - } } void @@ -2922,9 +2976,11 @@ isc__nm_http_cleanup_data(isc_nmsocket_t *sock) { if (sock->type == isc_nm_httplistener || sock->type == isc_nm_httpsocket) { - if (sock->type == isc_nm_httplistener) { - clear_handlers(sock); - isc_rwlock_destroy(&sock->h2.lock); + if (sock->type == isc_nm_httplistener && + sock->h2.listener_endpoints != NULL) { + /* Delete all handlers */ + isc_nm_http_endpoints_detach( + &sock->h2.listener_endpoints); } if (sock->h2.request_path != NULL) { diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 4a14a57a12..f588bc883b 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -31,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -783,6 +782,16 @@ typedef struct isc_nm_httpcbarg { LINK(struct isc_nm_httpcbarg) link; } isc_nm_httpcbarg_t; +struct isc_nm_http_endpoints { + isc_mem_t *mctx; + + ISC_LIST(isc_nm_httphandler_t) handlers; + ISC_LIST(isc_nm_httpcbarg_t) handler_cbargs; + + isc_refcount_t references; + atomic_bool in_use; +}; + typedef struct isc_nmsocket_h2 { isc_nmsocket_t *psock; /* owner of the structure */ char *request_path; @@ -816,9 +825,7 @@ typedef struct isc_nmsocket_h2 { void *cbarg; LINK(struct isc_nmsocket_h2) link; - ISC_LIST(isc_nm_httphandler_t) handlers; - ISC_LIST(isc_nm_httpcbarg_t) handler_cbargs; - isc_rwlock_t lock; + isc_nm_http_endpoints_t *listener_endpoints; bool response_submitted; struct { diff --git a/lib/isc/tests/doh_test.c b/lib/isc/tests/doh_test.c index 2aaa8727f8..04ba182662 100644 --- a/lib/isc/tests/doh_test.c +++ b/lib/isc/tests/doh_test.c @@ -83,6 +83,8 @@ static isc_tlsctx_t *client_tlsctx = NULL; static isc_quota_t listener_quota; static atomic_bool check_listener_quota = ATOMIC_VAR_INIT(false); +static isc_nm_http_endpoints_t *endpoints = NULL; + /* Timeout for soft-timeout tests (0.05 seconds) */ #define T_SOFT 50 @@ -336,6 +338,9 @@ nm_setup(void **state) { isc_quota_init(&listener_quota, 0); atomic_store(&check_listener_quota, false); + INSIST(endpoints == NULL); + endpoints = isc_nm_http_endpoints_new(test_mctx); + *state = nm; return (0); @@ -360,6 +365,8 @@ nm_teardown(void **state) { isc_quota_destroy(&listener_quota); + isc_nm_http_endpoints_detach(&endpoints); + return (0); } @@ -483,8 +490,11 @@ mock_doh_uv_tcp_bind(void **state) { WILL_RETURN(uv_tcp_bind, UV_EADDRINUSE); + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, noop_read_cb, + NULL, 0); + assert_int_equal(result, ISC_R_SUCCESS); result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, NULL, NULL, - 0, &listen_sock); + endpoints, 0, &listen_sock); assert_int_not_equal(result, ISC_R_SUCCESS); assert_null(listen_sock); @@ -500,11 +510,13 @@ doh_noop(void **state) { isc_nmsocket_t *listen_sock = NULL; char req_url[256]; - result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, NULL, NULL, - 0, &listen_sock); + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, noop_read_cb, + NULL, 0); + assert_int_equal(result, ISC_R_SUCCESS); + + result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, NULL, NULL, + endpoints, 0, &listen_sock); assert_int_equal(result, ISC_R_SUCCESS); - result = isc_nm_http_endpoint(listen_sock, DOH_PATH, noop_read_cb, NULL, - 0); isc_nm_stoplistening(listen_sock); isc_nmsocket_close(&listen_sock); @@ -546,12 +558,12 @@ doh_noresponse(void **state) { isc_nmsocket_t *listen_sock = NULL; char req_url[256]; - result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, NULL, NULL, - 0, &listen_sock); + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, noop_read_cb, + NULL, 0); assert_int_equal(result, ISC_R_SUCCESS); - result = isc_nm_http_endpoint(listen_sock, DOH_PATH, noop_read_cb, NULL, - 0); + result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, NULL, NULL, + endpoints, 0, &listen_sock); assert_int_equal(result, ISC_R_SUCCESS); sockaddr_to_url(&tcp_listen_addr, false, req_url, sizeof(req_url), @@ -647,8 +659,12 @@ doh_timeout_recovery(void **state) { isc_tlsctx_t *ctx = atomic_load(&use_TLS) ? server_tlsctx : NULL; char req_url[256]; + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, + doh_receive_request_cb, NULL, 0); + assert_int_equal(result, ISC_R_SUCCESS); + result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, NULL, NULL, - 0, &listen_sock); + endpoints, 0, &listen_sock); assert_int_equal(result, ISC_R_SUCCESS); /* @@ -656,9 +672,6 @@ doh_timeout_recovery(void **state) { * reads to time out. */ noanswer = true; - result = isc_nm_http_endpoint(listen_sock, DOH_PATH, - doh_receive_request_cb, NULL, 0); - assert_int_equal(result, ISC_R_SUCCESS); /* * Shorten all the TCP client timeouts to 0.05 seconds. @@ -777,13 +790,14 @@ doh_recv_one(void **state) { atomic_store(&total_sends, 1); atomic_store(&nsends, atomic_load(&total_sends)); - result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, - atomic_load(&use_TLS) ? server_tlsctx : NULL, - 0, &listen_sock); + + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, + doh_receive_request_cb, NULL, 0); assert_int_equal(result, ISC_R_SUCCESS); - result = isc_nm_http_endpoint(listen_sock, DOH_PATH, - doh_receive_request_cb, NULL, 0); + result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, + atomic_load(&use_TLS) ? server_tlsctx : NULL, + endpoints, 0, &listen_sock); assert_int_equal(result, ISC_R_SUCCESS); sockaddr_to_url(&tcp_listen_addr, atomic_load(&use_TLS), req_url, @@ -927,13 +941,14 @@ doh_recv_two(void **state) { atomic_store(&total_sends, 2); atomic_store(&nsends, atomic_load(&total_sends)); - result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, - atomic_load(&use_TLS) ? server_tlsctx : NULL, - 0, &listen_sock); + + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, + doh_receive_request_cb, NULL, 0); assert_int_equal(result, ISC_R_SUCCESS); - result = isc_nm_http_endpoint(listen_sock, DOH_PATH, - doh_receive_request_cb, NULL, 0); + result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, + atomic_load(&use_TLS) ? server_tlsctx : NULL, + endpoints, 0, &listen_sock); assert_int_equal(result, ISC_R_SUCCESS); sockaddr_to_url(&tcp_listen_addr, atomic_load(&use_TLS), req_url, @@ -1047,13 +1062,13 @@ doh_recv_send(void **state) { isc_thread_t threads[32] = { 0 }; isc_quota_t *quotap = init_listener_quota(workers); - result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, - atomic_load(&use_TLS) ? server_tlsctx : NULL, - 0, &listen_sock); + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, + doh_receive_request_cb, NULL, 0); assert_int_equal(result, ISC_R_SUCCESS); - result = isc_nm_http_endpoint(listen_sock, DOH_PATH, - doh_receive_request_cb, NULL, 0); + result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, + atomic_load(&use_TLS) ? server_tlsctx : NULL, + endpoints, 0, &listen_sock); assert_int_equal(result, ISC_R_SUCCESS); for (size_t i = 0; i < nthreads; i++) { @@ -1151,13 +1166,14 @@ doh_recv_half_send(void **state) { atomic_store(&total_sends, atomic_load(&total_sends) / 2); atomic_store(&nsends, atomic_load(&total_sends)); - result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, - atomic_load(&use_TLS) ? server_tlsctx : NULL, - 0, &listen_sock); + + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, + doh_receive_request_cb, NULL, 0); assert_int_equal(result, ISC_R_SUCCESS); - result = isc_nm_http_endpoint(listen_sock, DOH_PATH, - doh_receive_request_cb, NULL, 0); + result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, + atomic_load(&use_TLS) ? server_tlsctx : NULL, + endpoints, 0, &listen_sock); assert_int_equal(result, ISC_R_SUCCESS); for (size_t i = 0; i < nthreads; i++) { @@ -1260,13 +1276,14 @@ doh_half_recv_send(void **state) { atomic_store(&total_sends, atomic_load(&total_sends) / 2); atomic_store(&nsends, atomic_load(&total_sends)); - result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, - atomic_load(&use_TLS) ? server_tlsctx : NULL, - 0, &listen_sock); + + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, + doh_receive_request_cb, NULL, 0); assert_int_equal(result, ISC_R_SUCCESS); - result = isc_nm_http_endpoint(listen_sock, DOH_PATH, - doh_receive_request_cb, NULL, 0); + result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, + atomic_load(&use_TLS) ? server_tlsctx : NULL, + endpoints, 0, &listen_sock); assert_int_equal(result, ISC_R_SUCCESS); for (size_t i = 0; i < nthreads; i++) { @@ -1369,13 +1386,14 @@ doh_half_recv_half_send(void **state) { atomic_store(&total_sends, atomic_load(&total_sends) / 2); atomic_store(&nsends, atomic_load(&total_sends)); - result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, - atomic_load(&use_TLS) ? server_tlsctx : NULL, - 0, &listen_sock); + + result = isc_nm_http_endpoints_add(endpoints, DOH_PATH, + doh_receive_request_cb, NULL, 0); assert_int_equal(result, ISC_R_SUCCESS); - result = isc_nm_http_endpoint(listen_sock, DOH_PATH, - doh_receive_request_cb, NULL, 0); + result = isc_nm_listenhttp(listen_nm, &tcp_listen_addr, 0, quotap, + atomic_load(&use_TLS) ? server_tlsctx : NULL, + endpoints, 0, &listen_sock); assert_int_equal(result, ISC_R_SUCCESS); for (size_t i = 0; i < nthreads; i++) { diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index ea10b11bde..0d45dde91f 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -542,25 +542,29 @@ ns_interface_listenhttp(ns_interface_t *ifp, isc_tlsctx_t *sslctx, char **eps, size_t neps, isc_quota_t *quota, uint32_t max_concurrent_streams) { #if HAVE_LIBNGHTTP2 - isc_result_t result; + isc_result_t result = ISC_R_FAILURE; isc_nmsocket_t *sock = NULL; + isc_nm_http_endpoints_t *epset = NULL; - result = isc_nm_listenhttp(ifp->mgr->nm, &ifp->addr, ifp->mgr->backlog, - quota, sslctx, max_concurrent_streams, - &sock); + epset = isc_nm_http_endpoints_new(ifp->mgr->mctx); - if (result == ISC_R_SUCCESS) { - for (size_t i = 0; i < neps; i++) { - INSIST(isc_nm_http_path_isvalid(eps[i])); - result = isc_nm_http_endpoint(sock, eps[i], - ns__client_request, ifp, - sizeof(ns_client_t)); - if (result != ISC_R_SUCCESS) { - break; - } + for (size_t i = 0; i < neps; i++) { + result = isc_nm_http_endpoints_add(epset, eps[i], + ns__client_request, ifp, + sizeof(ns_client_t)); + if (result != ISC_R_SUCCESS) { + break; } } + if (result == ISC_R_SUCCESS) { + result = isc_nm_listenhttp( + ifp->mgr->nm, &ifp->addr, ifp->mgr->backlog, quota, + sslctx, epset, max_concurrent_streams, &sock); + } + + isc_nm_http_endpoints_detach(&epset); + if (result != ISC_R_SUCCESS) { isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_ERROR, "creating %s socket: %s", From 170cc41d5c7a25aa0451e74f5ee3fbee29f6885b Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Fri, 16 Jul 2021 14:57:08 +0300 Subject: [PATCH 2/2] Get rid of some HTTP/2 related types when NGHTTP2 is not available This commit removes definitions of some DoH-related types when libnghttp2 is not available. --- lib/isc/netmgr/netmgr-int.h | 21 ++++++++++++--------- lib/isc/netmgr/netmgr.c | 2 ++ lib/isc/tests/doh_test.c | 6 +++++- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index f588bc883b..f46e7fce5a 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -314,15 +314,6 @@ typedef union { isc_nm_accept_cb_t accept; } isc__nm_cb_t; -typedef struct isc_nm_httphandler isc_nm_httphandler_t; -struct isc_nm_httphandler { - char *path; - isc_nm_recv_cb_t cb; - void *cbarg; - size_t extrahandlesize; - LINK(isc_nm_httphandler_t) link; -}; - /* * Wrapper around uv_req_t with 'our' fields in it. req->data should * always point to its parent. Note that we always allocate more than @@ -755,6 +746,7 @@ enum { STATID_ACTIVE = 10 }; +#if HAVE_LIBNGHTTP2 typedef struct isc_nmsocket_tls_send_req { isc_nmsocket_t *tlssock; isc_region_t data; @@ -782,6 +774,14 @@ typedef struct isc_nm_httpcbarg { LINK(struct isc_nm_httpcbarg) link; } isc_nm_httpcbarg_t; +typedef struct isc_nm_httphandler { + char *path; + isc_nm_recv_cb_t cb; + void *cbarg; + size_t extrahandlesize; + LINK(struct isc_nm_httphandler) link; +} isc_nm_httphandler_t; + struct isc_nm_http_endpoints { isc_mem_t *mctx; @@ -836,6 +836,7 @@ typedef struct isc_nmsocket_h2 { void *cstream; } connect; } isc_nmsocket_h2_t; +#endif /* HAVE_LIBNGHTTP2 */ typedef void (*isc_nm_closehandlecb_t)(void *arg); /*%< @@ -882,6 +883,7 @@ struct isc_nmsocket { isc__nm_uvreq_t *pending_req; } tls; +#if HAVE_LIBNGHTTP2 /*% TLS stuff */ struct tlsstream { bool server; @@ -902,6 +904,7 @@ struct isc_nmsocket { } tlsstream; isc_nmsocket_h2_t h2; +#endif /* HAVE_LIBNGHTTP2 */ /*% * quota is the TCP client, attached when a TCP connection * is established. pquota is a non-attached pointer to the diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index ef17cfb234..42a9d681c7 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -1515,7 +1515,9 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type, isc_condition_init(&sock->scond); isc_refcount_init(&sock->references, 1); +#if HAVE_LIBNGHTTP2 memset(&sock->tlsstream, 0, sizeof(sock->tlsstream)); +#endif /* HAVE_LIBNGHTTP2 */ NETMGR_TRACE_LOG("isc__nmsocket_init():%p->references = %" PRIuFAST32 "\n", diff --git a/lib/isc/tests/doh_test.c b/lib/isc/tests/doh_test.c index 04ba182662..198c7bfc23 100644 --- a/lib/isc/tests/doh_test.c +++ b/lib/isc/tests/doh_test.c @@ -9,7 +9,7 @@ * information regarding copyright ownership. */ -#if HAVE_CMOCKA +#if defined(HAVE_CMOCKA) && defined(HAVE_LIBNGHTTP2) #include #include /* IWYU pragma: keep */ #include @@ -2192,7 +2192,11 @@ main(void) { int main(void) { +#if HAVE_LIBNGHTTP2 printf("1..0 # Skipped: cmocka not available\n"); +#else + printf("1..0 # Skipped: libnghttp2 is not available\n"); +#endif return (SKIPPED_TEST_EXIT_CODE); }