dispatch: Remove 'timeout' callback

- It is no longer necessary to pass a 'timeout' callback to
  dns_dispatch_addresponse(); timeouts are handled directly by the
  'response' callback instead.

- The netmgr handle is no longer passed to dispatch callbacks, since
  they don't (and can't) use it. instead, dispatch_cb_t now takes a
  result code, region, and argument.

- Cleaned up timeout-related tests in dispatch_test.c
This commit is contained in:
Ondřej Surý 2021-08-03 15:27:06 +02:00 committed by Evan Hunt
parent 8b532d2e64
commit e317386090
5 changed files with 122 additions and 116 deletions

View file

@ -741,8 +741,9 @@ doshutdown(void) {
static void
maybeshutdown(void) {
/* when called from getinput, doshutdown might be already finished */
if (requestmgr == NULL)
if (requestmgr == NULL) {
return;
}
ddebug("Shutting down request manager");
dns_requestmgr_shutdown(requestmgr);

View file

@ -61,8 +61,6 @@ struct dns_dispatchmgr {
/* locked by buffer_lock */
dns_qid_t *qid;
isc_mutex_t buffer_lock;
unsigned int buffers;
in_port_t *v4ports; /*%< available ports for IPv4 */
unsigned int nv4ports; /*%< # of available ports for IPv4 */
@ -84,10 +82,9 @@ struct dns_dispentry {
isc_sockaddr_t peer;
in_port_t port;
dns_messageid_t id;
isc_nm_cb_t connected;
isc_nm_cb_t sent;
isc_nm_recv_cb_t response;
isc_nm_cb_t timedout;
dispatch_cb_t connected;
dispatch_cb_t sent;
dispatch_cb_t response;
void *arg;
bool canceled;
ISC_LINK(dns_dispentry_t) link;
@ -433,7 +430,7 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
isc_sockaddr_t peer;
isc_netaddr_t netaddr;
int match;
isc_nm_recv_cb_t response = NULL;
dispatch_cb_t response = NULL;
bool nomore = true;
REQUIRE(VALID_RESPONSE(resp));
@ -444,10 +441,8 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
LOCK(&disp->lock);
if (isc_log_wouldlog(dns_lctx, LVL(90))) {
LOCK(&disp->mgr->buffer_lock);
dispatch_log(disp, LVL(90), "got packet: requests %d",
dispatch_log(disp, LVL(90), "got UDP packet: requests %d",
disp->requests);
UNLOCK(&disp->mgr->buffer_lock);
}
if (eresult == ISC_R_CANCELED) {
@ -466,13 +461,13 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
peer = isc_nmhandle_peeraddr(handle);
isc_netaddr_fromsockaddr(&netaddr, &peer);
if (eresult == ISC_R_TIMEDOUT && resp->timedout != NULL) {
resp->timedout(handle, ISC_R_TIMEDOUT, resp->arg);
if (isc_nmhandle_timer_running(handle)) {
nomore = false;
goto unlock;
}
}
/* if (eresult == ISC_R_TIMEDOUT && resp->timedout != NULL) { */
/* resp->timedout(handle, ISC_R_TIMEDOUT, resp->arg); */
/* if (isc_nmhandle_timer_running(handle)) { */
/* nomore = false; */
/* goto unlock; */
/* } */
/* } */
if (eresult != ISC_R_SUCCESS) {
/*
@ -549,7 +544,7 @@ unlock:
UNLOCK(&disp->lock);
if (response != NULL) {
response(handle, eresult, region, resp->arg);
response(eresult, region, resp->arg);
}
if (nomore) {
@ -703,7 +698,7 @@ unlock:
dispentry_detach(&resp0);
if (resp != NULL) {
resp->response(handle, eresult, region, resp->arg);
resp->response(eresult, region, resp->arg);
}
}
@ -789,7 +784,6 @@ dns_dispatchmgr_create(isc_mem_t *mctx, isc_nm_t *nm,
isc_nm_attach(nm, &mgr->nm);
isc_mutex_init(&mgr->lock);
isc_mutex_init(&mgr->buffer_lock);
ISC_LIST_INIT(mgr->list);
@ -868,8 +862,6 @@ dispatchmgr_destroy(dns_dispatchmgr_t *mgr) {
qid_destroy(mgr->mctx, &mgr->qid);
isc_mutex_destroy(&mgr->buffer_lock);
if (mgr->blackhole != NULL) {
dns_acl_detach(&mgr->blackhole);
}
@ -1271,10 +1263,9 @@ dns_dispatch_detach(dns_dispatch_t **dispp) {
isc_result_t
dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options,
unsigned int timeout, const isc_sockaddr_t *dest,
isc_nm_cb_t connected, isc_nm_cb_t sent,
isc_nm_recv_cb_t response, isc_nm_cb_t timedout,
void *arg, dns_messageid_t *idp,
dns_dispentry_t **resp) {
dispatch_cb_t connected, dispatch_cb_t sent,
dispatch_cb_t response, void *arg,
dns_messageid_t *idp, dns_dispentry_t **resp) {
dns_dispentry_t *res = NULL;
dns_qid_t *qid = NULL;
in_port_t localport = 0;
@ -1282,7 +1273,7 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options,
unsigned int bucket;
bool ok = false;
int i = 0;
isc_nm_recv_cb_t oldest_response = NULL;
dispatch_cb_t oldest_response = NULL;
REQUIRE(VALID_DISPATCH(disp));
REQUIRE(dest != NULL);
@ -1323,7 +1314,6 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options,
.peer = *dest,
.connected = connected,
.sent = sent,
.timedout = timedout,
.response = response,
.arg = arg };
@ -1394,7 +1384,7 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options,
UNLOCK(&disp->lock);
if (oldest_response != NULL) {
oldest_response(res->handle, ISC_R_CANCELED, NULL, res->arg);
oldest_response(ISC_R_CANCELED, NULL, res->arg);
}
*idp = id;
@ -1530,7 +1520,7 @@ disp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
}
if (resp->connected != NULL) {
resp->connected(handle, eresult, resp->arg);
resp->connected(eresult, NULL, resp->arg);
}
detach:
@ -1571,7 +1561,7 @@ send_done(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
REQUIRE(VALID_RESPONSE(resp));
resp->sent(handle, result, resp->arg);
resp->sent(result, NULL, resp->arg);
if (result != ISC_R_SUCCESS) {
isc_nm_cancelread(handle);
@ -1580,6 +1570,34 @@ send_done(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
dispentry_detach(&resp);
}
void
dns_dispatch_read(dns_dispentry_t *resp, uint16_t timeout) {
REQUIRE(resp != NULL);
dns_dispatch_t *disp = resp->disp;
isc_nmhandle_t *handle = NULL;
switch (disp->socktype) {
case isc_socktype_udp:
REQUIRE(resp->handle != NULL);
handle = resp->handle;
break;
case isc_socktype_tcp:
REQUIRE(disp != NULL && disp->handle == NULL);
handle = disp->handle;
break;
default:
INSIST(0);
ISC_UNREACHABLE();
}
isc_nmhandle_settimeout(handle, timeout);
startrecv(disp, resp);
}
void
dns_dispatch_send(dns_dispentry_t *resp, isc_region_t *r, isc_dscp_t dscp) {
isc_nmhandle_t *handle = NULL;

View file

@ -279,6 +279,9 @@ dns_dispatch_send(dns_dispentry_t *resp, isc_region_t *r, isc_dscp_t dscp);
*\li 'resp' is valid.
*/
void
dns_dispatch_read(dns_dispentry_t *resp, uint16_t timeout);
isc_result_t
dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr,
const isc_sockaddr_t *localaddr, bool *connected,
@ -288,13 +291,15 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr,
* if connected == NULL).
*/
typedef void (*dispatch_cb_t)(isc_result_t eresult, isc_region_t *region,
void *cbarg);
isc_result_t
dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options,
unsigned int timeout, const isc_sockaddr_t *dest,
isc_nm_cb_t connected, isc_nm_cb_t sent,
isc_nm_recv_cb_t response, isc_nm_cb_t timedout,
void *arg, dns_messageid_t *idp,
dns_dispentry_t **resp);
dispatch_cb_t connected, dispatch_cb_t sent,
dispatch_cb_t response, void *arg,
dns_messageid_t *idp, dns_dispentry_t **resp);
/*%<
* Add a response entry for this dispatch.
*

View file

@ -108,16 +108,13 @@ static isc_result_t
req_render(dns_message_t *message, isc_buffer_t **buffer, unsigned int options,
isc_mem_t *mctx);
static void
req_response(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region,
void *arg);
req_response(isc_result_t result, isc_region_t *region, void *arg);
static void
req_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg);
req_senddone(isc_result_t eresult, isc_region_t *region, void *arg);
static void
req_sendevent(dns_request_t *request, isc_result_t result);
static void
req_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg);
static void
req_timeout(isc_nmhandle_t *handle, isc_result_t eresult, void *arg);
req_connected(isc_result_t eresult, isc_region_t *region, void *arg);
static void
req_attach(dns_request_t *source, dns_request_t **targetp);
static void
@ -567,8 +564,8 @@ again:
req_attach(request, &rclone);
result = dns_dispatch_addresponse(
request->dispatch, dispopt, request->timeout, destaddr,
req_connected, req_senddone, req_response, req_timeout, request,
&id, &request->dispentry);
req_connected, req_senddone, req_response, request, &id,
&request->dispentry);
if (result != ISC_R_SUCCESS) {
if ((options & DNS_REQUESTOPT_FIXEDID) != 0 && !newtcp) {
newtcp = true;
@ -723,8 +720,7 @@ use_tcp:
req_attach(request, &rclone);
result = dns_dispatch_addresponse(
request->dispatch, 0, request->timeout, destaddr, req_connected,
req_senddone, req_response, req_timeout, request, &id,
&request->dispentry);
req_senddone, req_response, request, &id, &request->dispentry);
if (result != ISC_R_SUCCESS) {
goto cleanup;
}
@ -1007,10 +1003,10 @@ dns_request_destroy(dns_request_t **requestp) {
*** Private: request.
***/
static void
req_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
req_connected(isc_result_t eresult, isc_region_t *region, void *arg) {
dns_request_t *request = (dns_request_t *)arg;
UNUSED(handle);
UNUSED(region);
req_log(ISC_LOG_DEBUG(3), "req_connected: request %p: %s", request,
isc_result_totext(eresult));
@ -1044,13 +1040,13 @@ req_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
}
static void
req_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
req_senddone(isc_result_t eresult, isc_region_t *region, void *arg) {
dns_request_t *request = (dns_request_t *)arg;
REQUIRE(VALID_REQUEST(request));
REQUIRE(DNS_REQUEST_SENDING(request));
UNUSED(handle);
UNUSED(region);
req_log(ISC_LOG_DEBUG(3), "req_senddone: request %p", request);
@ -1072,42 +1068,37 @@ req_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
}
static void
req_timeout(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
req_response(isc_result_t result, isc_region_t *region, void *arg) {
dns_request_t *request = (dns_request_t *)arg;
REQUIRE(VALID_REQUEST(request));
UNUSED(eresult);
req_log(ISC_LOG_DEBUG(3), "req_timeout: request %p", request);
LOCK(&request->requestmgr->locks[request->hash]);
if (--request->udpcount != 0) {
isc_nmhandle_settimeout(handle, request->timeout);
if (!DNS_REQUEST_SENDING(request)) {
req_send(request);
}
}
UNLOCK(&request->requestmgr->locks[request->hash]);
}
static void
req_response(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region,
void *arg) {
dns_request_t *request = (dns_request_t *)arg;
UNUSED(handle);
if (result == ISC_R_CANCELED) {
return;
}
if (result == ISC_R_TIMEDOUT) {
req_log(ISC_LOG_DEBUG(3), "req_timeout: request %p", request);
LOCK(&request->requestmgr->locks[request->hash]);
if (--request->udpcount != 0) {
dns_dispatch_read(request->dispentry, request->timeout);
if (!DNS_REQUEST_SENDING(request)) {
req_send(request);
}
UNLOCK(&request->requestmgr->locks[request->hash]);
return;
}
/* The lock is unlocked below */
goto done;
}
REQUIRE(VALID_REQUEST(request));
req_log(ISC_LOG_DEBUG(3), "req_response: request %p: %s", request,
dns_result_totext(result));
LOCK(&request->requestmgr->locks[request->hash]);
if (result != ISC_R_SUCCESS) {
goto done;
}

View file

@ -609,10 +609,9 @@ empty_bucket(dns_resolver_t *res);
static isc_result_t
resquery_send(resquery_t *query);
static void
resquery_response(isc_nmhandle_t *handle, isc_result_t eresult,
isc_region_t *region, void *arg);
resquery_response(isc_result_t eresult, isc_region_t *region, void *arg);
static void
resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg);
resquery_connected(isc_result_t eresult, isc_region_t *region, void *arg);
static void
fctx_try(fetchctx_t *fctx, bool retrying, bool badcache);
static isc_result_t
@ -1391,11 +1390,8 @@ fctx_cancelquery(resquery_t *query, isc_time_t *finish, bool no_response,
}
/*
* Check for any outstanding socket events. If they exist,
* cancel them and let the event handlers finish the cleanup.
* (XXX: Currently the resolver, rather than dispatch, tracks
* whether it's sending or connecting; this will be moved into
* dispatch later.)
* Check for any outstanding dispatch responses. If they exist,
* cancel them and let their callbacks finish the cleanup.
*/
if (query->dispentry != NULL) {
dns_dispatch_cancel(query->dispentry);
@ -1406,7 +1402,6 @@ fctx_cancelquery(resquery_t *query, isc_time_t *finish, bool no_response,
ISC_LIST_UNLINK(fctx->queries, query, link);
}
/* This is the final detach matching the "init" */
resquery_detach(&query);
}
@ -1481,12 +1476,6 @@ fctx_cleanupaltaddrs(fetchctx_t *fctx) {
}
}
static inline void
fctx_stopqueries(fetchctx_t *fctx, bool no_response, bool age_untried) {
FCTXTRACE("stopqueries");
fctx_cancelqueries(fctx, no_response, age_untried);
}
static inline void
fctx_cleanupall(fetchctx_t *fctx) {
fctx_cleanupfinds(fctx);
@ -1741,7 +1730,7 @@ fctx_done(fetchctx_t *fctx, isc_result_t result, int line) {
fctx->qmin_warning = ISC_R_SUCCESS;
fctx_stopqueries(fctx, no_response, age_untried);
fctx_cancelqueries(fctx, no_response, age_untried);
LOCK(&res->buckets[fctx->bucketnum].lock);
@ -1753,13 +1742,13 @@ fctx_done(fetchctx_t *fctx, isc_result_t result, int line) {
}
static void
resquery_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
resquery_senddone(isc_result_t eresult, isc_region_t *region, void *arg) {
resquery_t *query = (resquery_t *)arg;
fetchctx_t *fctx = NULL;
QTRACE("senddone");
UNUSED(handle);
UNUSED(region);
fctx = query->fctx;
@ -1802,7 +1791,7 @@ resquery_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
}
detach:
resquery_detach(&query); /* Detach dispatch query */
resquery_detach(&query);
}
static inline isc_result_t
@ -1893,25 +1882,20 @@ fctx_setretryinterval(fetchctx_t *fctx, unsigned int rtt) {
isc_time_nowplusinterval(&fctx->next_timeout, &fctx->interval);
}
static void
resquery_timeout(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
resquery_t *query = (resquery_t *)arg;
static isc_result_t
resquery_timeout(resquery_t *query) {
fetchctx_t *fctx = query->fctx;
dns_fetchevent_t *event = NULL, *next = NULL;
uint64_t timeleft;
isc_time_t now;
REQUIRE(VALID_FCTX(fctx));
FCTXTRACE("timeout");
/*
* If not configured for serve-stale, do nothing.
*/
if (eresult == ISC_R_CANCELED ||
(fctx->options & DNS_FETCHOPT_TRYSTALE_ONTIMEOUT) == 0)
{
return;
if ((fctx->options & DNS_FETCHOPT_TRYSTALE_ONTIMEOUT) == 0) {
return (ISC_R_SUCCESS);
}
/*
@ -1922,7 +1906,7 @@ resquery_timeout(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
isc_time_now(&now);
timeleft = isc_time_microdiff(&fctx->expires_try_stale, &now);
if (timeleft >= US_PER_MSEC) {
return;
return (ISC_R_SUCCESS);
}
/*
@ -1950,9 +1934,12 @@ resquery_timeout(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
* resume waiting.
*/
timeleft = isc_time_microdiff(&fctx->next_timeout, &now);
if (timeleft < US_PER_MSEC) {
isc_nmhandle_settimeout(handle, (timeleft / US_PER_MSEC));
if (timeleft >= US_PER_MSEC) {
dns_dispatch_read(query->dispentry, (timeleft / US_PER_MSEC));
return (ISC_R_COMPLETE);
}
return (ISC_R_SUCCESS);
}
static isc_result_t
@ -2154,14 +2141,14 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
result = dns_dispatch_addresponse(
query->dispatch, 0, isc_interval_ms(&fctx->interval),
&query->addrinfo->sockaddr, resquery_connected,
resquery_senddone, resquery_response, resquery_timeout, query,
&query->id, &query->dispentry);
resquery_senddone, resquery_response, query, &query->id,
&query->dispentry);
if (result != ISC_R_SUCCESS) {
goto cleanup_dispatch;
}
/* Connect the socket */
resquery_attach(query, &(resquery_t *){ NULL }); /* dispatch query */
resquery_attach(query, &(resquery_t *){ NULL });
result = dns_dispatch_connect(query->dispentry);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
@ -2772,7 +2759,7 @@ cleanup_temps:
}
static void
resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
resquery_connected(isc_result_t eresult, isc_region_t *region, void *arg) {
resquery_t *query = (resquery_t *)arg;
isc_result_t result;
fetchctx_t *fctx = NULL;
@ -2783,7 +2770,7 @@ resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
QTRACE("connected");
UNUSED(handle);
UNUSED(region);
fctx = query->fctx;
res = fctx->res;
@ -2881,7 +2868,7 @@ resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
}
detach:
resquery_detach(&query); /* Detach dispatch query */
resquery_detach(&query);
}
static void
@ -4405,7 +4392,7 @@ fctx_doshutdown(isc_task_t *task, isc_event_t *event) {
* fetch, and clean up finds and addresses. To avoid deadlock
* with the ADB, we must do this before we lock the bucket lock.
*/
fctx_stopqueries(fctx, false, false);
fctx_cancelqueries(fctx, false, false);
fctx_cleanupall(fctx);
LOCK(&res->buckets[bucketnum].lock);
@ -7206,15 +7193,12 @@ betterreferral(respctx_t *rctx) {
* resquery_send(). Sets up a response context (respctx_t).
*/
static void
resquery_response(isc_nmhandle_t *handle, isc_result_t eresult,
isc_region_t *region, void *arg) {
resquery_response(isc_result_t eresult, isc_region_t *region, void *arg) {
isc_result_t result;
resquery_t *query = (resquery_t *)arg;
fetchctx_t *fctx = NULL;
respctx_t rctx;
UNUSED(handle);
if (eresult == ISC_R_CANCELED) {
return;
}
@ -7225,6 +7209,13 @@ resquery_response(isc_nmhandle_t *handle, isc_result_t eresult,
QTRACE("response");
if (eresult == ISC_R_TIMEDOUT) {
result = resquery_timeout(query);
if (result == ISC_R_COMPLETE) {
return;
}
}
if (isc_sockaddr_pf(&query->addrinfo->sockaddr) == PF_INET) {
inc_stats(fctx->res, dns_resstatscounter_responsev4);
} else {