From b896bb5a783d3fc741b94bf65616b69097b5ecaf Mon Sep 17 00:00:00 2001
From: Miroslav Lichvar <mlichvar@redhat.com>
Date: Wed, 2 Aug 2017 10:54:50 +0200
Subject: [PATCH] ntp: don't send useless requests in interleaved client mode

In interleaved client mode, when so many consecutive requests were lost
that the first valid (interleaved) response would be dropped for being
too old, switch to basic mode so the response can be accepted if it
doesn't fail in the other tests.

This reworks commit 16afa8eb5022792c1b4bf08e3b01095ca5ebd0f5.
---
 ntp_core.c           | 20 +++++++++++---------
 test/unit/ntp_core.c |  3 ++-
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/ntp_core.c b/ntp_core.c
index 3442d14..54cdbe5 100644
--- a/ntp_core.c
+++ b/ntp_core.c
@@ -1119,10 +1119,14 @@ transmit_timeout(void *arg)
      is the first response to the last valid request received from the peer
      and there was just one response to the previous valid request.  This
      prevents the peer from matching the transmit timestamp with an older
-     response if it can't detect missed responses. */
+     response if it can't detect missed responses.  In client mode, which has
+     at most one response per request, check how many responses are missing to
+     prevent the server from responding with a very old transmit timestamp. */
   interleaved = inst->interleaved &&
-                (inst->mode == MODE_CLIENT ||
-                 (inst->prev_tx_count == 1 && inst->tx_count == 0));
+                ((inst->mode == MODE_CLIENT &&
+                  inst->tx_count < MAX_CLIENT_INTERLEAVED_TX) ||
+                 (inst->mode == MODE_ACTIVE &&
+                  inst->prev_tx_count == 1 && inst->tx_count == 0));
 
   /* Check whether we need to 'warm up' the link to the other end by
      sending an NTP exchange to ensure both ends' ARP caches are
@@ -1517,13 +1521,11 @@ receive_packet(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, in interleaved client mode that the timestamps
-       are not too old, and in interleaved symmetric mode that the delay and
-       intervals between remote timestamps don't indicate a missed response */
+       processing time is sane, and in interleaved symmetric mode that the
+       measured delay and intervals between remote timestamps don't indicate
+       a missed response */
     testA = delay - dispersion <= inst->max_delay && precision <= inst->max_delay &&
-            !(inst->mode == MODE_CLIENT &&
-              (response_time > MAX_SERVER_INTERVAL ||
-               (interleaved_packet && inst->tx_count > MAX_CLIENT_INTERLEAVED_TX + 1))) &&
+            !(inst->mode == MODE_CLIENT && response_time > MAX_SERVER_INTERVAL) &&
             !(inst->mode == MODE_ACTIVE && interleaved_packet &&
               (delay > 0.5 * prev_remote_poll_interval ||
                UTI_CompareNtp64(&message->receive_ts, &message->transmit_ts) <= 0 ||
diff --git a/test/unit/ntp_core.c b/test/unit/ntp_core.c
index 6dacd33..a358c2a 100644
--- a/test/unit/ntp_core.c
+++ b/test/unit/ntp_core.c
@@ -251,7 +251,8 @@ test_unit(void)
     for (j = 0; j < 50; j++) {
       DEBUG_LOG("iteration %d, %d", i, j);
 
-      interleaved = random() % 2;
+      interleaved = random() % 2 && (inst->mode != MODE_CLIENT ||
+                                     inst->tx_count <= MAX_CLIENT_INTERLEAVED_TX);
       authenticated = random() % 2;
       valid = (!interleaved || (source.params.interleaved && has_updated)) &&
               (!source.params.authkey || authenticated);