From 0ae6f2485b9784d3d2881d31372831128a7781b1 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Thu, 30 Jun 2022 10:18:48 +0200 Subject: [PATCH] ntp: don't use first response in interleaved mode With the first interleaved response coming after a basic response the client is forced to select the four timestamps covering most of the last polling interval, which makes measured delay very sensitive to the frequency offset between server and client. To avoid corrupting the minimum delay held in sourcestats (which can cause testC failures), reject the first interleaved response in the client/server mode as failing the test A. This does not change anything for the symmetric mode, where both sets of the four timestamps generally cover a significant part of the polling interval. --- ntp_core.c | 7 ++++++- test/simulation/122-xleave | 4 +++- test/unit/ntp_core.c | 5 ++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/ntp_core.c b/ntp_core.c index b73309e..7a21f5e 100644 --- a/ntp_core.c +++ b/ntp_core.c @@ -1907,12 +1907,17 @@ process_response(NCR_Instance inst, NTP_Local_Address *local_addr, /* Test A requires that the minimum estimate of the peer delay is not larger than the configured maximum, in both client modes that the server - processing time is sane, and in interleaved symmetric mode that the + processing time is sane, in interleaved client/server mode that the + previous response was not in basic mode (which prevents using timestamps + that minimise delay error), and in interleaved symmetric mode that the measured delay and intervals between remote timestamps don't indicate a missed response */ testA = sample.peer_delay - sample.peer_dispersion <= inst->max_delay && precision <= inst->max_delay && !(inst->mode == MODE_CLIENT && response_time > MAX_SERVER_INTERVAL) && + !(inst->mode == MODE_CLIENT && interleaved_packet && + UTI_IsZeroTimespec(&inst->prev_local_tx.ts) && + UTI_CompareTimespecs(&local_transmit.ts, &inst->local_tx.ts) == 0) && !(inst->mode == MODE_ACTIVE && interleaved_packet && (sample.peer_delay > 0.5 * prev_remote_poll_interval || UTI_CompareNtp64(&message->receive_ts, &message->transmit_ts) <= 0 || diff --git a/test/simulation/122-xleave b/test/simulation/122-xleave index bc4c3a7..c19063a 100755 --- a/test/simulation/122-xleave +++ b/test/simulation/122-xleave @@ -15,10 +15,11 @@ check_chronyd_exit || test_fail check_source_selection || test_fail check_sync || test_fail -check_file_messages "111 111 1111.* 4I [DKH] [DKH]\$" 0 0 measurements.log || test_fail +check_file_messages "111 111 .111.* 4I [DKH] [DKH]\$" 0 0 measurements.log || test_fail rm -f tmp/measurements.log server_conf="" +max_sync_time=270 run_test || test_fail check_chronyd_exit || test_fail @@ -28,6 +29,7 @@ check_sync || test_fail check_file_messages "111 111 1111.* 4B [DKH] [DKH]\$" 2 2 measurements.log || test_fail check_file_messages "111 111 1111.* 4I [DKH] [DKH]\$" 30 200 measurements.log || test_fail +check_file_messages "111 111 0111.* 4I [DKH] [DKH]\$" 1 1 measurements.log || test_fail rm -f tmp/measurements.log clients=2 diff --git a/test/unit/ntp_core.c b/test/unit/ntp_core.c index 0d9c3a7..1d92ebf 100644 --- a/test/unit/ntp_core.c +++ b/test/unit/ntp_core.c @@ -496,7 +496,10 @@ test_unit(void) send_request(inst1); process_request(&remote_addr); - proc_response(inst1, 1, 1, 1, 0); + proc_response(inst1, + !source.params.interleaved || source.params.version != 4 || + inst1->mode == MODE_ACTIVE || j != 2, + 1, 1, 0); advance_time(1 << inst1->local_poll); }