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.

diff --git a/ntp_core.c b/ntp_core.c
index b73309ed5ff573dd6676147b145e52b27d6a0a95..7a21f5e9ef94efb15797c9eb24749533a36f02f8 100644 (file)
--- 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 bc4c3a7b854f92fa6ec42cfcdbaa48a4955338ee..c19063a5363eb369563872d1141778b3e7d0c0cc 100755 (executable)
--- 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 0d9c3a7d5092cabc6b297db340e2bf2880ed9029..1d92ebf08a0c4e0839e6b5c93eff503ca762ce73 100644 (file)
--- 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);
     }