From: Alessio Podda Date: Fri, 10 Jul 2026 13:13:18 +0000 (+0200) Subject: Make isc_nm_udplistener into its own type X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=87f51862fb043b54dda94b4de817d5cd2699be2a;p=thirdparty%2Fbind9.git Make isc_nm_udplistener into its own type The udplistener socket type wraps a group of SO_REUSEPORT udp sockets bound to the same port and address. Since it is implemented as an instance of isc_nmsocket_t, clients would need to attach to the full udplistener instead of the individual sockets. This is a great cause of contention on machines with high core count. With this commit, we split isc_nm_udplistener out of isc_nmsocket_t, and let clients attach to the underlying sockets, which are sharded by thread. --- diff --git a/bin/tests/test_server.c b/bin/tests/test_server.c index 13596fc22f6..6483d8f3dc0 100644 --- a/bin/tests/test_server.c +++ b/bin/tests/test_server.c @@ -238,11 +238,12 @@ static void run(void) { isc_result_t result; isc_nmsocket_t *sock = NULL; + isc_nm_udplistener_t *udp_listener = NULL; switch (protocol) { case UDP: result = isc_nm_listenudp(ISC_NM_LISTEN_ALL, &sockaddr, read_cb, - NULL, &sock); + NULL, &udp_listener); break; case TCP: result = isc_nm_listenstreamdns( @@ -284,8 +285,13 @@ run(void) { test_server_yield(); - isc_nm_stoplistening(sock); - isc_nmsocket_close(&sock); + if (udp_listener != NULL) { + isc_nm_udplistener_stop(udp_listener); + isc_nm_udplistener_detach(&udp_listener); + } else { + isc_nm_stoplistening(sock); + isc_nmsocket_close(&sock); + } } int diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 99aed3dd916..283606d6d72 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -130,9 +130,8 @@ void isc_nmsocket_close(isc_nmsocket_t **sockp); /*%< * isc_nmsocket_close() detaches a listening socket that was - * created by isc_nm_listenudp(), isc_nm_listentcp(), or - * isc_nm_listentcpdns(). Once there are no remaining child - * sockets with active handles, the socket will be closed. + * created by isc_nm_listentcp() or isc_nm_listentcpdns(). Once there are no + * remaining child sockets with active handles, the socket will be closed. */ void @@ -272,17 +271,29 @@ isc_nmhandle_real_localaddr(isc_nmhandle_t *handle); isc_result_t isc_nm_listenudp(uint32_t workers, isc_sockaddr_t *iface, isc_nm_recv_cb_t cb, - void *cbarg, isc_nmsocket_t **sockp); + void *cbarg, isc_nm_udplistener_t **listenerp); /*%< * Start listening for UDP packets on interface 'iface' using net manager * 'mgr'. * - * On success, 'sockp' will be updated to contain a new listening UDP socket. + * On success, 'listenerp' will be updated to contain a new UDP listener. * * When a packet is received on the socket, 'cb' will be called with 'cbarg' * as its argument. */ +void +isc_nm_udplistener_stop(isc_nm_udplistener_t *listener); +/*%< + * Stop a UDP listener and close its per-worker sockets. This must be called + * before detaching the final listener reference. + */ + +ISC_REFCOUNT_DECL(isc_nm_udplistener); +/*%< + * Increment or decrement a UDP listener reference. + */ + void isc_nm_udpconnect(isc_sockaddr_t *local, isc_sockaddr_t *peer, isc_nm_cb_t cb, void *cbarg, unsigned int timeout); diff --git a/lib/isc/include/isc/types.h b/lib/isc/include/isc/types.h index 1a5b9d86ac2..e268a2807f7 100644 --- a/lib/isc/include/isc/types.h +++ b/lib/isc/include/isc/types.h @@ -55,16 +55,17 @@ typedef struct isc_mempool isc_mempool_t; /*%< Memory Pool */ typedef struct isc_netaddr isc_netaddr_t; /*%< Net Address */ typedef struct isc_netaddrlink isc_netaddrlink_t; /*%< Linkable Net Address */ typedef ISC_LIST(isc_netaddrlink_t) isc_netaddrlist_t; /*%< Net Address List */ -typedef struct isc_netprefix isc_netprefix_t; /*%< Net Prefix */ -typedef struct isc_nmsocket isc_nmsocket_t; /*%< Network manager socket */ -typedef struct isc_nmhandle isc_nmhandle_t; /*%< Network manager handle */ -typedef struct isc_portset isc_portset_t; /*%< Port Set */ -typedef struct isc_quota isc_quota_t; /*%< Quota */ -typedef struct isc_ratelimiter isc_ratelimiter_t; /*%< Rate Limiter */ -typedef struct isc_region isc_region_t; /*%< Region */ -typedef struct isc_rlevent isc_rlevent_t; /*%< Rate Limiter Event */ -typedef struct isc_signal isc_signal_t; /*%< Signal handler */ -typedef struct isc_sockaddr isc_sockaddr_t; /*%< Socket Address */ +typedef struct isc_netprefix isc_netprefix_t; /*%< Net Prefix */ +typedef struct isc_nmsocket isc_nmsocket_t; /*%< Network manager socket */ +typedef struct isc_nmhandle isc_nmhandle_t; /*%< Network manager handle */ +typedef struct isc_nm_udplistener isc_nm_udplistener_t; +typedef struct isc_portset isc_portset_t; /*%< Port Set */ +typedef struct isc_quota isc_quota_t; /*%< Quota */ +typedef struct isc_ratelimiter isc_ratelimiter_t; /*%< Rate Limiter */ +typedef struct isc_region isc_region_t; /*%< Region */ +typedef struct isc_rlevent isc_rlevent_t; /*%< Rate Limiter Event */ +typedef struct isc_signal isc_signal_t; /*%< Signal handler */ +typedef struct isc_sockaddr isc_sockaddr_t; /*%< Socket Address */ typedef ISC_LIST(isc_sockaddr_t) isc_sockaddrlist_t; /*%< Socket Address List * */ typedef struct isc_stats isc_stats_t; /*%< Statistics */ @@ -106,7 +107,6 @@ typedef enum isc_nmsocket_type { isc_nm_proxyudpsocket = 1 << 7, isc_nm_maxsocket, - isc_nm_udplistener, /* Aggregate of nm_udpsocks */ isc_nm_tcplistener, isc_nm_tlslistener, isc_nm_httplistener, diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index cce8b690295..2137ec8123a 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -571,6 +571,7 @@ struct isc_nmsocket { struct { isc_nmsocket_t *sock; + isc_nm_udplistener_t *udp_listener; bool reading; size_t nsending; void *send_req; @@ -825,8 +826,7 @@ isc__nmsocket_reset(isc_nmsocket_t *sock); bool isc__nmsocket_active(isc_nmsocket_t *sock); /*%< - * Determine whether 'sock' is active by checking 'sock->active' - * or, for child sockets, 'sock->parent->active'. + * Determine whether 'sock' is active by checking 'sock->active'. */ void @@ -894,11 +894,8 @@ isc__nm_udp_shutdown(isc_nmsocket_t *sock); * sockets. */ -void -isc__nm_udp_stoplistening(isc_nmsocket_t *sock); -/*%< - * Stop listening on 'sock'. - */ +uint32_t +isc__nm_udplistener_nchildren(const isc_nm_udplistener_t *listener); void isc__nm_udp_settimeout(isc_nmhandle_t *handle, uint32_t timeout); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index a106df9613d..4fe1cd61441 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -689,8 +689,7 @@ void isc_nmsocket_close(isc_nmsocket_t **sockp) { REQUIRE(sockp != NULL); REQUIRE(VALID_NMSOCK(*sockp)); - REQUIRE((*sockp)->type == isc_nm_udplistener || - (*sockp)->type == isc_nm_tcplistener || + REQUIRE((*sockp)->type == isc_nm_tcplistener || (*sockp)->type == isc_nm_streamdnslistener || (*sockp)->type == isc_nm_tlslistener || (*sockp)->type == isc_nm_httplistener || @@ -743,7 +742,6 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc__networker_t *worker, switch (type) { case isc_nm_udpsocket: - case isc_nm_udplistener: switch (family) { case AF_INET: sock->statsindex = udp4statsindex; @@ -1661,7 +1659,6 @@ isc_nm_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, switch (handle->sock->type) { case isc_nm_udpsocket: - case isc_nm_udplistener: isc__nm_udp_send(handle, region, cb, cbarg); break; case isc_nm_tcpsocket: @@ -1811,9 +1808,6 @@ isc_nm_stoplistening(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); switch (sock->type) { - case isc_nm_udplistener: - isc__nm_udp_stoplistening(sock); - break; case isc_nm_tcplistener: isc__nm_tcp_stoplistening(sock); break; @@ -2019,7 +2013,6 @@ isc__nmsocket_shutdown(isc_nmsocket_t *sock) { case isc_nm_tcpsocket: isc__nm_tcp_shutdown(sock); break; - case isc_nm_udplistener: case isc_nm_tcplistener: return; default: @@ -2941,8 +2934,6 @@ nmsocket_type_totext(isc_nmsocket_type type) { switch (type) { case isc_nm_udpsocket: return "isc_nm_udpsocket"; - case isc_nm_udplistener: - return "isc_nm_udplistener"; case isc_nm_tcpsocket: return "isc_nm_tcpsocket"; case isc_nm_tcplistener: diff --git a/lib/isc/netmgr/proxyudp.c b/lib/isc/netmgr/proxyudp.c index 20d44a0ebd8..1c4749f899d 100644 --- a/lib/isc/netmgr/proxyudp.c +++ b/lib/isc/netmgr/proxyudp.c @@ -351,12 +351,13 @@ isc_nm_listenproxyudp(uint32_t workers, isc_sockaddr_t *iface, } result = isc_nm_listenudp(workers, iface, proxyudp_read_cb, listener, - &listener->outer); + &listener->proxy.udp_listener); if (result == ISC_R_SUCCESS) { listener->active = true; listener->result = result; - listener->nchildren = listener->outer->nchildren; + listener->nchildren = isc__nm_udplistener_nchildren( + listener->proxy.udp_listener); *sockp = listener; } else { for (size_t i = 0; i < listener->proxy.udp_server_socks_num; @@ -530,10 +531,15 @@ isc__nm_proxyudp_stoplistening(isc_nmsocket_t *listener) { REQUIRE(VALID_NMSOCK(listener)); REQUIRE(listener->type == isc_nm_proxyudplistener); REQUIRE(listener->proxy.sock == NULL); + REQUIRE(!listener->closing); - isc__nmsocket_stop(listener); - + listener->closing = true; listener->active = false; + isc_nm_udplistener_stop(listener->proxy.udp_listener); + isc_nm_udplistener_detach(&listener->proxy.udp_listener); + listener->recv_cb = NULL; + listener->recv_cbarg = NULL; + listener->closed = true; for (size_t i = 1; i < listener->proxy.udp_server_socks_num; i++) { stop_proxyudp_child(listener->proxy.udp_server_socks[i]); @@ -561,6 +567,7 @@ isc__nm_proxyudp_cleanup_data(isc_nmsocket_t *sock) { proxyudp_clear_proxy_header_data(sock); break; case isc_nm_proxyudplistener: + INSIST(sock->proxy.udp_listener == NULL); isc_mem_cput(sock->worker->mctx, sock->proxy.udp_server_socks, sock->proxy.udp_server_socks_num, sizeof(isc_nmsocket_t *)); diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index d595af99e57..65ee0267408 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -65,6 +65,90 @@ udp_send_cb(uv_udp_send_t *req, int status); static void udp_close_cb(uv_handle_t *handle); +static void +udp_timer_close_cb(uv_handle_t *handle); + +#define UDP_LISTENER_MAGIC ISC_MAGIC('U', 'D', 'P', 'L') +#define VALID_UDP_LISTENER(listener) \ + (ISC_MAGIC_VALID(listener, UDP_LISTENER_MAGIC) && \ + isc_refcount_current(&(listener)->references) > 0) + +struct isc_nm_udplistener { + int magic; + isc_refcount_t references; + isc_mem_t *mctx; + isc_sockaddr_t iface; + isc_nm_recv_cb_t recv_cb; + void *recv_cbarg; + isc_barrier_t listen_barrier; + isc_barrier_t stop_barrier; + unsigned int uv_flags; + bool closing; + uint32_t nchildren; + isc_nmsocket_t *children[] ISC_ATTR_COUNTED_BY(nchildren); +}; + +typedef struct udp_child_job { + isc_nm_udplistener_t *listener; + isc_tid_t tid; + uv_os_sock_t fd; +} udp_child_job_t; + +static void +udp_uv_handle_attach(uv_handle_t *handle, isc_nmsocket_t *sock) { + isc_nmsocket_t *attached = NULL; + void *data = uv_handle_get_data(handle); + + REQUIRE(VALID_NMSOCK(sock)); + REQUIRE(data == NULL || data == sock); + + isc__nmsocket_attach(sock, &attached); + uv_handle_set_data(handle, attached); +} + +static isc_nmsocket_t * +udp_uv_handle_get(uv_handle_t *handle) { + isc_nmsocket_t *sock = uv_handle_get_data(handle); + + REQUIRE(VALID_NMSOCK(sock)); + return sock; +} + +static void +udp_uv_handle_detach(uv_handle_t *handle) { + isc_nmsocket_t *sock = udp_uv_handle_get(handle); + + uv_handle_set_data(handle, NULL); + isc__nmsocket_detach(&sock); +} + +static void +udp_listener_destroy(isc_nm_udplistener_t *listener) { + isc_mem_t *mctx = listener->mctx; + size_t size = sizeof(*listener) + + ISC_CHECKED_MUL(listener->nchildren, + sizeof(listener->children[0])); + + REQUIRE(listener->closing); + for (size_t i = 0; i < listener->nchildren; i++) { + INSIST(listener->children[i] == NULL); + } + + isc_barrier_destroy(&listener->listen_barrier); + isc_barrier_destroy(&listener->stop_barrier); + isc_refcount_destroy(&listener->references); + listener->magic = 0; + isc_mem_putanddetach(&mctx, listener, size); +} + +ISC_REFCOUNT_IMPL(isc_nm_udplistener, udp_listener_destroy); + +uint32_t +isc__nm_udplistener_nchildren(const isc_nm_udplistener_t *listener) { + REQUIRE(VALID_UDP_LISTENER(listener)); + + return listener->nchildren; +} static uv_os_sock_t isc__nm_udp_lb_socket(sa_family_t sa_family) { @@ -93,12 +177,30 @@ isc__nm_udp_lb_socket(sa_family_t sa_family) { */ static void start_udp_child_job(void *arg) { - isc_nmsocket_t *sock = arg; + udp_child_job_t *job = arg; + isc_nm_udplistener_t *listener = job->listener; + isc__networker_t *worker = isc__networker_current(); + isc_nmsocket_t *sock = NULL; + isc_mem_t *mctx = listener->mctx; - REQUIRE(VALID_NMSOCK(sock)); - REQUIRE(VALID_NMSOCK(sock->parent)); - REQUIRE(sock->type == isc_nm_udpsocket); - REQUIRE(sock->tid == isc_tid()); + REQUIRE(VALID_UDP_LISTENER(listener)); + REQUIRE(job->tid == isc_tid()); + + sock = isc_mempool_get(worker->nmsocket_pool); + isc__nmsocket_init(sock, worker, isc_nm_udpsocket, &listener->iface, + NULL); + listener->children[job->tid] = sock; + sock->recv_cb = listener->recv_cb; + sock->recv_cbarg = listener->recv_cbarg; + sock->inactive_handles_max = ISC_NM_NMHANDLES_MAX; + if (isc__netmgr->load_balance_sockets) { + sock->fd = isc__nm_udp_lb_socket( + listener->iface.type.sa.sa_family); + } else { + INSIST(job->fd >= 0); + sock->fd = dup(job->fd); + } + INSIST(sock->fd >= 0); int r, uv_bind_flags = 0; int uv_init_flags = 0; @@ -113,13 +215,11 @@ start_udp_child_job(void *arg) { #endif r = uv_udp_init_ex(&loop->loop, &sock->uv_handle.udp, uv_init_flags); UV_RUNTIME_CHECK(uv_udp_init_ex, r); - uv_handle_set_data(&sock->uv_handle.handle, sock); - /* This keeps the socket alive after everything else is gone */ - isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); + udp_uv_handle_attach(&sock->uv_handle.handle, sock); r = uv_timer_init(&loop->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + udp_uv_handle_attach((uv_handle_t *)&sock->read_timer, sock); r = uv_udp_open(&sock->uv_handle.udp, sock->fd); if (r < 0) { @@ -135,7 +235,7 @@ start_udp_child_job(void *arg) { if (isc__netmgr->load_balance_sockets) { r = isc__nm_udp_freebind(&sock->uv_handle.udp, - &sock->parent->iface.type.sa, + &listener->iface.type.sa, uv_bind_flags); if (r < 0) { isc__nm_incstats(sock, STATID_BINDFAIL); @@ -144,16 +244,16 @@ start_udp_child_job(void *arg) { } else if (sock->tid == 0) { /* This thread is first, bind the socket */ r = isc__nm_udp_freebind(&sock->uv_handle.udp, - &sock->parent->iface.type.sa, + &listener->iface.type.sa, uv_bind_flags); if (r < 0) { isc__nm_incstats(sock, STATID_BINDFAIL); goto done; } - sock->parent->uv_handle.udp.flags = sock->uv_handle.udp.flags; + listener->uv_flags = sock->uv_handle.udp.flags; } else { /* The socket is already bound, just copy the flags */ - sock->uv_handle.udp.flags = sock->parent->uv_handle.udp.flags; + sock->uv_handle.udp.flags = listener->uv_flags; } isc__nm_set_network_buffers(&sock->uv_handle.handle); @@ -173,81 +273,79 @@ done: REQUIRE(!loop->paused); if (sock->tid != 0) { - isc_barrier_wait(&sock->parent->listen_barrier); + isc_barrier_wait(&listener->listen_barrier); } + + isc_mem_put(mctx, job, sizeof(*job)); + isc_nm_udplistener_detach(&listener); } static void -start_udp_child(isc_sockaddr_t *iface, isc_nmsocket_t *sock, uv_os_sock_t fd, +start_udp_child(isc_nm_udplistener_t *listener, uv_os_sock_t fd, isc_tid_t tid) { isc__networker_t *worker = isc__networker_get(tid); - isc_nmsocket_t *csock = &sock->children[tid]; + udp_child_job_t *job = isc_mem_get(listener->mctx, sizeof(*job)); - isc__nmsocket_init(csock, worker, isc_nm_udpsocket, iface, sock); - csock->recv_cb = sock->recv_cb; - csock->recv_cbarg = sock->recv_cbarg; - csock->inactive_handles_max = ISC_NM_NMHANDLES_MAX; - - if (isc__netmgr->load_balance_sockets) { - csock->fd = isc__nm_udp_lb_socket(iface->type.sa.sa_family); - } else { - INSIST(fd >= 0); - csock->fd = dup(fd); - } - INSIST(csock->fd >= 0); + *job = (udp_child_job_t){ .tid = tid, .fd = fd }; + isc_nm_udplistener_attach(listener, &job->listener); if (tid == 0) { - start_udp_child_job(csock); + start_udp_child_job(job); } else { - isc_async_run(worker->loop, start_udp_child_job, csock); + isc_async_run(worker->loop, start_udp_child_job, job); } } isc_result_t isc_nm_listenudp(uint32_t workers, isc_sockaddr_t *iface, isc_nm_recv_cb_t cb, - void *cbarg, isc_nmsocket_t **sockp) { + void *cbarg, isc_nm_udplistener_t **listenerp) { isc_result_t result = ISC_R_UNSET; - isc_nmsocket_t *sock = NULL; + isc_nm_udplistener_t *listener = NULL; uv_os_sock_t fd = -1; isc__networker_t *worker = isc__networker_get(0); + uint32_t nchildren = (workers == ISC_NM_LISTEN_ALL) + ? (uint32_t)isc__netmgr->nloops + : workers; + size_t size = sizeof(*listener) + + ISC_CHECKED_MUL(nchildren, sizeof(listener->children[0])); REQUIRE(isc_tid() == 0); + REQUIRE(listenerp != NULL && *listenerp == NULL); if (isc__nm_closing(worker)) { return ISC_R_SHUTTINGDOWN; } - sock = isc_mempool_get(worker->nmsocket_pool); - isc__nmsocket_init(sock, worker, isc_nm_udplistener, iface, NULL); - - if (workers == ISC_NM_LISTEN_ALL) { - sock->nchildren = (uint32_t)isc__netmgr->nloops; - } else { - sock->nchildren = workers; - } - REQUIRE(sock->nchildren <= isc__netmgr->nloops); - - sock->children = isc_mem_cget(worker->mctx, sock->nchildren, - sizeof(sock->children[0])); - - isc__nmsocket_barrier_init(sock); - - sock->recv_cb = cb; - sock->recv_cbarg = cbarg; + listener = isc_mem_get(worker->mctx, size); + *listener = (isc_nm_udplistener_t){ + .magic = UDP_LISTENER_MAGIC, + .references = ISC_REFCOUNT_INITIALIZER(1), + .mctx = isc_mem_ref(worker->mctx), + .nchildren = nchildren, + .iface = *iface, + .recv_cb = cb, + .recv_cbarg = cbarg, + }; + REQUIRE(listener->nchildren > 0); + REQUIRE(listener->nchildren <= isc__netmgr->nloops); + isc_barrier_init(&listener->listen_barrier, listener->nchildren); + isc_barrier_init(&listener->stop_barrier, listener->nchildren); if (!isc__netmgr->load_balance_sockets) { fd = isc__nm_udp_lb_socket(iface->type.sa.sa_family); } - start_udp_child(iface, sock, fd, 0); - result = sock->children[0].result; + listener->children[0] = NULL; + start_udp_child(listener, fd, 0); + result = listener->children[0]->result; INSIST(result != ISC_R_UNSET); - for (size_t i = 1; i < sock->nchildren; i++) { - start_udp_child(iface, sock, fd, i); + for (size_t i = 1; i < listener->nchildren; i++) { + listener->children[i] = NULL; + start_udp_child(listener, fd, i); } - isc_barrier_wait(&sock->listen_barrier); + isc_barrier_wait(&listener->listen_barrier); if (!isc__netmgr->load_balance_sockets) { isc__nm_closesocket(fd); @@ -257,25 +355,22 @@ isc_nm_listenudp(uint32_t workers, isc_sockaddr_t *iface, isc_nm_recv_cb_t cb, * If any of the child sockets have failed then isc_nm_listenudp * fails. */ - for (size_t i = 1; i < sock->nchildren; i++) { + for (size_t i = 1; i < listener->nchildren; i++) { if (result == ISC_R_SUCCESS && - sock->children[i].result != ISC_R_SUCCESS) + listener->children[i]->result != ISC_R_SUCCESS) { - result = sock->children[i].result; + result = listener->children[i]->result; } } if (result != ISC_R_SUCCESS) { - sock->active = false; - isc__nm_udp_stoplistening(sock); - isc_nmsocket_close(&sock); + isc_nm_udplistener_stop(listener); + isc_nm_udplistener_detach(&listener); return result; } - sock->active = true; - - *sockp = sock; + *listenerp = listener; return ISC_R_SUCCESS; } @@ -399,54 +494,63 @@ isc_nm_routeconnect(isc_nm_cb_t cb, void *cbarg) { */ static void stop_udp_child_job(void *arg) { - isc_nmsocket_t *sock = arg; + udp_child_job_t *job = arg; + isc_nm_udplistener_t *listener = job->listener; + isc_nmsocket_t *sock = listener->children[job->tid]; + isc_mem_t *mctx = listener->mctx; + + REQUIRE(VALID_UDP_LISTENER(listener)); REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_tid()); - REQUIRE(sock->parent != NULL); sock->active = false; - isc__nm_udp_close(sock); + if (!sock->closing && !sock->closed) { + isc__nm_udp_close(sock); + } REQUIRE(!sock->worker->loop->paused); - isc_barrier_wait(&sock->parent->stop_barrier); + isc_barrier_wait(&listener->stop_barrier); + + isc_mem_put(mctx, job, sizeof(*job)); + isc_nm_udplistener_detach(&listener); } static void -stop_udp_child(isc_nmsocket_t *sock) { +stop_udp_child(isc_nm_udplistener_t *listener, isc_tid_t tid) { + isc_nmsocket_t *sock = listener->children[tid]; + udp_child_job_t *job = isc_mem_get(listener->mctx, sizeof(*job)); + REQUIRE(VALID_NMSOCK(sock)); + *job = (udp_child_job_t){ .tid = tid }; + isc_nm_udplistener_attach(listener, &job->listener); if (sock->tid == 0) { - stop_udp_child_job(sock); + stop_udp_child_job(job); } else { - isc_async_run(sock->worker->loop, stop_udp_child_job, sock); + isc_async_run(sock->worker->loop, stop_udp_child_job, job); } } void -isc__nm_udp_stoplistening(isc_nmsocket_t *sock) { - REQUIRE(VALID_NMSOCK(sock)); - REQUIRE(sock->type == isc_nm_udplistener); - REQUIRE(sock->tid == isc_tid()); - REQUIRE(sock->tid == 0); - REQUIRE(!sock->closing); - - sock->closing = true; +isc_nm_udplistener_stop(isc_nm_udplistener_t *listener) { + REQUIRE(VALID_UDP_LISTENER(listener)); + REQUIRE(isc_tid() == 0); + REQUIRE(!listener->closing); - /* Mark the parent socket inactive */ - sock->active = false; + listener->closing = true; /* Stop all the other threads' children */ - for (size_t i = 1; i < sock->nchildren; i++) { - stop_udp_child(&sock->children[i]); + for (size_t i = 1; i < listener->nchildren; i++) { + stop_udp_child(listener, i); } /* Stop the child for the main thread */ - stop_udp_child(&sock->children[0]); + stop_udp_child(listener, 0); - /* Stop the parent */ - sock->closed = true; - isc__nmsocket_prep_destroy(sock); + for (size_t i = 0; i < listener->nchildren; i++) { + isc__nmsocket_detach(&listener->children[i]); + } } /* @@ -946,10 +1050,8 @@ fail: static void udp_close_cb(uv_handle_t *handle) { - isc_nmsocket_t *sock = uv_handle_get_data(handle); - uv_handle_set_data(handle, NULL); + isc_nmsocket_t *sock = udp_uv_handle_get(handle); - REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_tid()); REQUIRE(sock->closing); REQUIRE(!sock->closed); @@ -958,14 +1060,14 @@ udp_close_cb(uv_handle_t *handle) { isc__nm_incstats(sock, STATID_CLOSE); - if (sock->parent != NULL) { - /* listening socket (listen) */ - isc__nmsocket_detach(&sock); - } else { - /* client and server sockets */ - sock->connected = false; - isc__nmsocket_prep_destroy(sock); - } + sock->connected = false; + + udp_uv_handle_detach(handle); +} + +static void +udp_timer_close_cb(uv_handle_t *handle) { + udp_uv_handle_detach(handle); } void @@ -990,11 +1092,15 @@ isc__nm_udp_close(isc_nmsocket_t *sock) { /* 2. close the listening socket */ isc__nmsocket_clearcb(sock); isc__nm_stop_reading(sock); + if (sock->client) { + udp_uv_handle_attach(&sock->uv_handle.handle, sock); + } uv_close(&sock->uv_handle.handle, udp_close_cb); /* 1. close the read timer */ isc__nmsocket_timer_stop(sock); - uv_close((uv_handle_t *)&sock->read_timer, NULL); + uv_close((uv_handle_t *)&sock->read_timer, + sock->client ? NULL : udp_timer_close_cb); } void @@ -1025,14 +1131,5 @@ isc__nm_udp_shutdown(isc_nmsocket_t *sock) { return; } - /* Destroy the non-listening socket */ - if (sock->parent == NULL) { - isc__nmsocket_prep_destroy(sock); - return; - } - - /* Destroy the listening socket if on the same loop */ - if (sock->tid == sock->parent->tid) { - isc__nmsocket_prep_destroy(sock->parent); - } + isc__nmsocket_prep_destroy(sock); } diff --git a/lib/ns/client.c b/lib/ns/client.c index 1025d3dbe21..ee1c37e5a2b 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -148,7 +148,6 @@ ns_client_transport_type(const ns_client_t *client) { switch (isc_nm_socket_type(client->inner.handle)) { case isc_nm_udpsocket: - case isc_nm_udplistener: case isc_nm_proxyudpsocket: case isc_nm_proxyudplistener: return DNS_TRANSPORT_UDP; diff --git a/lib/ns/include/ns/interfacemgr.h b/lib/ns/include/ns/interfacemgr.h index c338563b1f8..9c4a33c5d7d 100644 --- a/lib/ns/include/ns/interfacemgr.h +++ b/lib/ns/include/ns/interfacemgr.h @@ -67,28 +67,29 @@ #define NS_INTERFACEFLAG_LISTENING 0x02U /*%< listening */ /*% The nameserver interface structure */ struct ns_interface { - unsigned int magic; /*%< Magic number. */ - ns_interfacemgr_t *mgr; /*%< Interface manager. */ - isc_mutex_t lock; - unsigned int generation; /*%< Generation number. */ - isc_sockaddr_t addr; /*%< Address and port. */ - unsigned int flags; /*%< Interface flags */ - char name[32]; /*%< Null terminated. */ - isc_nmsocket_t *udplistensocket; - isc_nmsocket_t *tcplistensocket; - isc_nmsocket_t *tlslistensocket; - isc_nmsocket_t *http_listensocket; - isc_nmsocket_t *http_secure_listensocket; - isc_quota_t *http_quota; - isc_refcount_t ntcpaccepting; /*%< Number of clients - * ready to accept new - * TCP connections on this - * interface */ - isc_refcount_t ntcpactive; /*%< Number of clients - * servicing TCP queries - * (whether accepting or - * connected) */ - ns_clientmgr_t *clientmgr; /*%< Client manager. */ + unsigned int magic; /*%< Magic number. */ + ns_interfacemgr_t *mgr; /*%< Interface manager. */ + isc_mutex_t lock; + unsigned int generation; /*%< Generation number. */ + isc_sockaddr_t addr; /*%< Address and port. */ + unsigned int flags; /*%< Interface flags */ + char name[32]; /*%< Null terminated. */ + isc_nm_udplistener_t *udplistener; + isc_nmsocket_t *proxyudplistensocket; + isc_nmsocket_t *tcplistensocket; + isc_nmsocket_t *tlslistensocket; + isc_nmsocket_t *http_listensocket; + isc_nmsocket_t *http_secure_listensocket; + isc_quota_t *http_quota; + isc_refcount_t ntcpaccepting; /*%< Number of clients + * ready to accept new + * TCP connections on this + * interface */ + isc_refcount_t ntcpactive; /*%< Number of clients + * servicing TCP queries + * (whether accepting or + * connected) */ + ns_clientmgr_t *clientmgr; /*%< Client manager. */ isc_nm_proxy_type_t proxy_type; ISC_LINK(ns_interface_t) link; }; diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index 07b5a641217..e439de9dce9 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -480,12 +480,12 @@ ns_interface_listenudp(ns_interface_t *ifp, isc_nm_proxy_type_t proxy) { if (proxy == ISC_NM_PROXY_NONE) { result = isc_nm_listenudp(ISC_NM_LISTEN_ALL, &ifp->addr, ns_client_request, ifp, - &ifp->udplistensocket); + &ifp->udplistener); } else { INSIST(proxy == ISC_NM_PROXY_PLAIN); result = isc_nm_listenproxyudp(ISC_NM_LISTEN_ALL, &ifp->addr, ns_client_request, ifp, - &ifp->udplistensocket); + &ifp->proxyudplistensocket); } return result; } @@ -730,9 +730,13 @@ void ns_interface_shutdown(ns_interface_t *ifp) { ifp->flags &= ~NS_INTERFACEFLAG_LISTENING; - if (ifp->udplistensocket != NULL) { - isc_nm_stoplistening(ifp->udplistensocket); - isc_nmsocket_close(&ifp->udplistensocket); + if (ifp->udplistener != NULL) { + isc_nm_udplistener_stop(ifp->udplistener); + isc_nm_udplistener_detach(&ifp->udplistener); + } + if (ifp->proxyudplistensocket != NULL) { + isc_nm_stoplistening(ifp->proxyudplistensocket); + isc_nmsocket_close(&ifp->proxyudplistensocket); } if (ifp->tcplistensocket != NULL) { isc_nm_stoplistening(ifp->tcplistensocket); @@ -1009,8 +1013,10 @@ same_listener_type(ns_interface_t *ifp, ns_listenelt_t *new_le) { } else if (new_le->sslctx != NULL && ifp->tlslistensocket != NULL) { /* TLS/DoT */ same_transport_type = true; - } else if (new_le->sslctx == NULL && (ifp->udplistensocket != NULL || - ifp->tcplistensocket != NULL)) + } else if (new_le->sslctx == NULL && + (ifp->udplistener != NULL || + ifp->proxyudplistensocket != NULL || + ifp->tcplistensocket != NULL)) { /* "plain" DNS/Do53 */ same_transport_type = true; diff --git a/tests/dns/dispatch_test.c b/tests/dns/dispatch_test.c index 91e7bb5e1ed..5f580b044e5 100644 --- a/tests/dns/dispatch_test.c +++ b/tests/dns/dispatch_test.c @@ -74,6 +74,7 @@ static dns_transport_t *tls_transport = NULL; static dns_transport_list_t *transport_list = NULL; static isc_nmsocket_t *sock = NULL; +static isc_nm_udplistener_t *udp_listener = NULL; const struct in6_addr in6addr_blackhole = { { { 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1 } } }; @@ -622,6 +623,15 @@ stop_listening(void *arg) { assert_null(sock); } +static void +stop_udp_listening(void *arg) { + UNUSED(arg); + + isc_nm_udplistener_stop(udp_listener); + isc_nm_udplistener_detach(&udp_listener); + assert_null(udp_listener); +} + ISC_LOOP_TEST_IMPL(dispatch_timeout_tcp_response) { isc_result_t result; test_dispatch_t *test = isc_mem_get(isc_g_mctx, sizeof(*test)); @@ -739,11 +749,11 @@ ISC_LOOP_TEST_IMPL(dispatch_timeout_udp_response) { /* Server */ result = isc_nm_listenudp(ISC_NM_LISTEN_ONE, &udp_server_addr, - noop_nameserver, NULL, &sock); + noop_nameserver, NULL, &udp_listener); assert_int_equal(result, ISC_R_SUCCESS); /* ensure we stop listening after the test is done */ - isc_loop_teardown(isc_loop_main(), stop_listening, sock); + isc_loop_teardown(isc_loop_main(), stop_udp_listening, udp_listener); /* Client */ result = dns_dispatchmgr_create(isc_g_mctx, &test->dispatchmgr); @@ -771,10 +781,10 @@ ISC_LOOP_TEST_IMPL(dispatch_getnext) { /* Server */ result = isc_nm_listenudp(ISC_NM_LISTEN_ONE, &udp_server_addr, - nameserver, NULL, &sock); + nameserver, NULL, &udp_listener); assert_int_equal(result, ISC_R_SUCCESS); - isc_loop_teardown(isc_loop_main(), stop_listening, sock); + isc_loop_teardown(isc_loop_main(), stop_udp_listening, udp_listener); /* Client */ testdata.region.base = testdata.message; @@ -812,10 +822,10 @@ ISC_LOOP_TEST_IMPL(dispatch_mismatch_tcp) { /* Server: replies with a single wrong-id response. */ result = isc_nm_listenudp(ISC_NM_LISTEN_ONE, &udp_server_addr, - nameserver_mismatch, NULL, &sock); + nameserver_mismatch, NULL, &udp_listener); assert_int_equal(result, ISC_R_SUCCESS); - isc_loop_teardown(isc_loop_main(), stop_listening, sock); + isc_loop_teardown(isc_loop_main(), stop_udp_listening, udp_listener); /* Client */ testdata.region.base = testdata.message; diff --git a/tests/isc/netmgr_common.c b/tests/isc/netmgr_common.c index 72237ef37e0..cd7d600ed46 100644 --- a/tests/isc/netmgr_common.c +++ b/tests/isc/netmgr_common.c @@ -97,6 +97,7 @@ isc_refcount_t active_ssends = 0; isc_refcount_t active_sreads = 0; isc_nmsocket_t *listen_sock = NULL; +isc_nm_udplistener_t *udp_listen_sock = NULL; isc_quota_t listener_quota; atomic_bool check_listener_quota = false; @@ -238,6 +239,13 @@ stop_listening(void *arg ISC_ATTR_UNUSED) { assert_null(listen_sock); } +static void +stop_udp_listening(void *arg ISC_ATTR_UNUSED) { + isc_nm_udplistener_stop(udp_listen_sock); + isc_nm_udplistener_detach(&udp_listen_sock); + assert_null(udp_listen_sock); +} + /* Callbacks */ void @@ -1347,12 +1355,17 @@ udp_start_listening(uint32_t nworkers, isc_nm_recv_cb_t cb) { NULL, &listen_sock); } else { result = isc_nm_listenudp(nworkers, &udp_listen_addr, cb, NULL, - &listen_sock); + &udp_listen_sock); } assert_int_equal(result, ISC_R_SUCCESS); - isc_loop_teardown(isc_loop_main(), stop_listening, listen_sock); + if (udp_use_PROXY) { + isc_loop_teardown(isc_loop_main(), stop_listening, listen_sock); + } else { + isc_loop_teardown(isc_loop_main(), stop_udp_listening, + udp_listen_sock); + } } static void diff --git a/tests/isc/netmgr_common.h b/tests/isc/netmgr_common.h index 0a1fd73fa3f..0cb072510dc 100644 --- a/tests/isc/netmgr_common.h +++ b/tests/isc/netmgr_common.h @@ -124,6 +124,7 @@ extern isc_refcount_t active_ssends; extern isc_refcount_t active_sreads; extern isc_nmsocket_t *listen_sock; +extern isc_nm_udplistener_t *udp_listen_sock; extern isc_quota_t listener_quota; extern atomic_bool check_listener_quota; diff --git a/tests/isc/udp_test.c b/tests/isc/udp_test.c index 23557a988fe..430074cfd3f 100644 --- a/tests/isc/udp_test.c +++ b/tests/isc/udp_test.c @@ -81,9 +81,9 @@ ISC_LOOP_TEST_IMPL(mock_listenudp_uv_udp_open) { WILL_RETURN(uv_udp_open, UV_ENOMEM); result = isc_nm_listenudp(ISC_NM_LISTEN_ALL, &udp_listen_addr, - mock_recv_cb, NULL, &listen_sock); + mock_recv_cb, NULL, &udp_listen_sock); assert_int_not_equal(result, ISC_R_SUCCESS); - assert_null(listen_sock); + assert_null(udp_listen_sock); RESET_RETURN; @@ -96,9 +96,9 @@ ISC_LOOP_TEST_IMPL(mock_listenudp_uv_udp_bind) { WILL_RETURN(uv_udp_bind, UV_EADDRINUSE); result = isc_nm_listenudp(ISC_NM_LISTEN_ALL, &udp_listen_addr, - mock_recv_cb, NULL, &listen_sock); + mock_recv_cb, NULL, &udp_listen_sock); assert_int_not_equal(result, ISC_R_SUCCESS); - assert_null(listen_sock); + assert_null(udp_listen_sock); RESET_RETURN; @@ -111,9 +111,9 @@ ISC_LOOP_TEST_IMPL(mock_listenudp_uv_udp_recv_start) { WILL_RETURN(uv_udp_recv_start, UV_EADDRINUSE); result = isc_nm_listenudp(ISC_NM_LISTEN_ALL, &udp_listen_addr, - mock_recv_cb, NULL, &listen_sock); + mock_recv_cb, NULL, &udp_listen_sock); assert_int_not_equal(result, ISC_R_SUCCESS); - assert_null(listen_sock); + assert_null(udp_listen_sock); RESET_RETURN; @@ -175,6 +175,44 @@ ISC_LOOP_TEST_IMPL(mock_udpconnect_uv_send_buffer_size) { RESET_RETURN; } +ISC_LOOP_TEST_IMPL(udp_listener_child_ref) { + isc_result_t result = ISC_R_SUCCESS; + isc_nmsocket_t *child = NULL; + isc_nmsocket_t *hold = NULL; + uint_fast32_t listener_refs; + uint_fast32_t child_refs; + + result = isc_nm_listenudp(ISC_NM_LISTEN_ONE, &udp_listen_addr, + mock_recv_cb, NULL, &udp_listen_sock); + assert_int_equal(result, ISC_R_SUCCESS); + assert_non_null(udp_listen_sock); + assert_int_equal(udp_listen_sock->nchildren, 1); + + child = udp_listen_sock->children[0]; + assert_non_null(child); + assert_int_equal(child->type, isc_nm_udpsocket); + assert_null(child->parent); + + listener_refs = isc_refcount_current(&udp_listen_sock->references); + child_refs = isc_refcount_current(&child->references); + + isc__nmsocket_attach(child, &hold); + assert_ptr_equal(hold, child); + assert_int_equal(isc_refcount_current(&child->references), + child_refs + 1); + assert_int_equal(isc_refcount_current(&udp_listen_sock->references), + listener_refs); + + isc__nmsocket_detach(&hold); + assert_null(hold); + + isc_nm_udplistener_stop(udp_listen_sock); + isc_nm_udplistener_detach(&udp_listen_sock); + assert_null(udp_listen_sock); + + isc_loopmgr_shutdown(); +} + ISC_LOOP_TEST_IMPL(udp_noop) { udp_noop(arg); } ISC_LOOP_TEST_IMPL(udp_noresponse) { udp_noresponse(arg); } @@ -213,6 +251,7 @@ ISC_TEST_ENTRY_CUSTOM(mock_udpconnect_uv_recv_buffer_size, setup_udp_test, teardown_udp_test) ISC_TEST_ENTRY_CUSTOM(mock_udpconnect_uv_send_buffer_size, setup_udp_test, teardown_udp_test) +ISC_TEST_ENTRY_CUSTOM(udp_listener_child_ref, setup_udp_test, teardown_udp_test) ISC_TEST_ENTRY_CUSTOM(udp_noop, udp_noop_setup, udp_noop_teardown) ISC_TEST_ENTRY_CUSTOM(udp_noresponse, udp_noresponse_setup,