From cbd77c9752a9fc65e6fa885e3cf8def72cad09a1 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Thu, 5 Nov 2015 16:43:40 +0100 Subject: [PATCH] ntp: don't keep client sockets open for longer than necessary After sending a client packet, schedule a timeout to close the socket at the time when all server replies would fail the delay test, so the socket is not open for longer than necessary (e.g. when the server is unreachable). With the default maxdelay of 3 seconds the timeout is 7 seconds. --- ntp_core.c | 62 +++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/ntp_core.c b/ntp_core.c index b869e42..a72a01c 100644 --- a/ntp_core.c +++ b/ntp_core.c @@ -71,8 +71,8 @@ struct NCR_Instance_Record { (client/server or symmetric active peer) */ OperatingMode opmode; /* Whether we are sampling this source or not and in what way */ - SCH_TimeoutID timeout_id; /* Scheduler's timeout ID, if we are - running on a timer. */ + SCH_TimeoutID rx_timeout_id; /* Timeout ID for latest received response */ + SCH_TimeoutID tx_timeout_id; /* Timeout ID for next transmission */ int tx_suspended; /* Boolean indicating we can't transmit yet */ int auto_offline; /* If 1, automatically go offline if server/peer @@ -354,18 +354,20 @@ restart_timeout(NCR_Instance inst, double delay) { /* Check if we can transmit */ if (inst->tx_suspended) { - assert(!inst->timeout_id); + assert(!inst->tx_timeout_id); return; } - /* Stop old timer if running */ - SCH_RemoveTimeout(inst->timeout_id); + /* Stop both rx and tx timers if running */ + SCH_RemoveTimeout(inst->rx_timeout_id); + inst->rx_timeout_id = 0; + SCH_RemoveTimeout(inst->tx_timeout_id); /* Start new timer for transmission */ - inst->timeout_id = SCH_AddTimeoutInClass(delay, SAMPLING_SEPARATION, - SAMPLING_RANDOMNESS, - SCH_NtpSamplingClass, - transmit_timeout, (void *)inst); + inst->tx_timeout_id = SCH_AddTimeoutInClass(delay, SAMPLING_SEPARATION, + SAMPLING_RANDOMNESS, + SCH_NtpSamplingClass, + transmit_timeout, (void *)inst); } /* ================================================== */ @@ -373,7 +375,7 @@ restart_timeout(NCR_Instance inst, double delay) static void start_initial_timeout(NCR_Instance inst) { - if (!inst->timeout_id) { + if (!inst->tx_timeout_id) { /* This will be the first transmission after mode change */ /* Mark source active */ @@ -392,6 +394,9 @@ close_client_socket(NCR_Instance inst) NIO_CloseClientSocket(inst->local_addr.sock_fd); inst->local_addr.sock_fd = INVALID_SOCK_FD; } + + SCH_RemoveTimeout(inst->rx_timeout_id); + inst->rx_timeout_id = 0; } /* ================================================== */ @@ -401,8 +406,8 @@ take_offline(NCR_Instance inst) { inst->opmode = MD_OFFLINE; - SCH_RemoveTimeout(inst->timeout_id); - inst->timeout_id = 0; + SCH_RemoveTimeout(inst->tx_timeout_id); + inst->tx_timeout_id = 0; /* Mark source unreachable */ SRC_ResetReachability(inst->source); @@ -489,7 +494,8 @@ NCR_GetInstance(NTP_Remote_Address *remote_addr, NTP_Source_Type type, SourcePar &result->remote_addr.ip_addr, params->min_samples, params->max_samples); - result->timeout_id = 0; + result->rx_timeout_id = 0; + result->tx_timeout_id = 0; result->tx_suspended = 1; result->opmode = params->online ? MD_ONLINE : MD_OFFLINE; result->local_poll = result->minpoll; @@ -559,7 +565,7 @@ NCR_ResetInstance(NCR_Instance instance) instance->local_poll = instance->minpoll; /* The timer was set with a longer poll interval, restart it */ - if (instance->timeout_id) + if (instance->tx_timeout_id) restart_timeout(instance, get_transmit_delay(instance, 0, 0.0)); } } @@ -735,6 +741,22 @@ get_transmit_delay(NCR_Instance inst, int on_tx, double last_tx) return delay_time; } +/* ================================================== */ +/* Timeout handler for closing the client socket when no acceptable + reply can be received from the server */ + +static void +receive_timeout(void *arg) +{ + NCR_Instance inst = (NCR_Instance)arg; + + DEBUG_LOG(LOGF_NtpCore, "Receive timeout for [%s:%d]", + UTI_IPToString(&inst->remote_addr.ip_addr), inst->remote_addr.port); + + inst->rx_timeout_id = 0; + close_client_socket(inst); +} + /* ================================================== */ static int @@ -908,7 +930,7 @@ transmit_timeout(void *arg) NCR_Instance inst = (NCR_Instance) arg; int sent; - inst->timeout_id = 0; + inst->tx_timeout_id = 0; switch (inst->opmode) { case MD_BURST_WAS_ONLINE: @@ -1005,8 +1027,14 @@ transmit_timeout(void *arg) /* Restart timer for this message */ restart_timeout(inst, get_transmit_delay(inst, 1, 0.0)); -} + /* If a client packet was just sent, schedule a timeout to close the socket + at the time when all server replies would fail the delay test, so the + socket is not open for longer than necessary */ + if (inst->mode == MODE_CLIENT) + inst->rx_timeout_id = SCH_AddTimeoutByDelay(inst->max_delay + MAX_SERVER_INTERVAL, + receive_timeout, (void *)inst); +} /* ================================================== */ @@ -1419,7 +1447,7 @@ receive_packet(NTP_Packet *message, struct timeval *now, double now_err, NCR_Ins } /* Get rid of old timeout and start a new one */ - assert(inst->timeout_id); + assert(inst->tx_timeout_id); restart_timeout(inst, delay_time); }