rx: Remove incorrect backoff code

The ACK packet handling routine contains code which causes the
RTT to backoff if the selective ACK response indicates that there is
a missing packet. The comment justifies this code as being in line
with Phil Karn's work on TCP.

However, the TCP behaviour is that we backoff when we enter resend. Both
TCP and RX have difficulty computing RTTs for resent packets due to the
ambiguous ACK problem. Whilst RX is slightly better than TCP in this
regard, we can't always tell whether an ACK refers to the original, or
resent packet, so resent packets are unable to contribute to the RTT.
This means that if the RTT ends up too low for the connection, and we
start resending every packet, the RTT will never grow to account for
this, as we never feed it any packet samples.

Karn's solution to this was to backoff (double) the RTT value when we
resend a packet, and then to not drop it back down until we receive an
ACK that we can count. This means that we will always get a new sample
for the connection, and the RTT will grow again.

The original author confirms that the current behaviour in RX is
incorrect, so simply remove it with this patchset.

Reviewed-on: http://gerrit.openafs.org/4860
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit b65944973a24e9365dc1ff118ded4c3a1e25f782)

Change-Id: I4d2766d98883dad4f27ff4c52e2a03a49733f89f
Reviewed-on: http://gerrit.openafs.org/4911
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index 1f85946f730c82c9146adb22234462c81b5c5f14..9b5266a003dfbc83ffbda2bb91fe2979e57f84f6 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -4350,21 +4350,6 @@ rxi_ReceiveAckPacket(struct rx_call *call, struct rx_packet *np,
            missing = 1;
        }
 
-        /*
-         * Following the suggestion of Phil Kern, we back off the peer's
-         * timeout value for future packets until a successful response
-         * is received for an initial transmission.
-         */
-        if (missing && !peer->backedOff) {
-            struct clock c = peer->timeout;
-            struct clock max_to = {3, 0};
-
-            clock_Add(&peer->timeout, &c);
-            if (clock_Gt(&peer->timeout, &max_to))
-                peer->timeout = max_to;
-            peer->backedOff = 1;
-        }
-
        /* If packet isn't yet acked, and it has been transmitted at least
         * once, reset retransmit time using latest timeout
         * ie, this should readjust the retransmit timer for all outstanding
@@ -6790,9 +6775,6 @@ rxi_ComputeRoundTripTime(struct rx_packet *p,
     clock_Zero(&(peer->timeout));
     clock_Addmsec(&(peer->timeout), rtt_timeout);
 
-    /* Reset the backedOff flag since we just computed a new timeout value */
-    peer->backedOff = 0;
-
     dpf(("rxi_ComputeRoundTripTime(call=%d packet=%"AFS_PTR_FMT" rtt=%d ms, srtt=%d ms, rtt_dev=%d ms, timeout=%d.%06d sec)\n",
           p->header.callNumber, p, MSEC(&thisRtt), peer->rtt >> 3, peer->rtt_dev >> 2, (peer->timeout.sec), (peer->timeout.usec)));
 }

diff --git a/src/rx/rx.h b/src/rx/rx.h
index 185b4b3863b689fb8d5ea4600b0ffb69ca58d147..d37fcda2d6002a212e1398b6eba30618a3e558d0 100644 (file)
--- a/src/rx/rx.h
+++ b/src/rx/rx.h
@@ -401,7 +401,6 @@ struct rx_peer {
     int rtt;                   /* Smoothed round trip time, measured in milliseconds/8 */
     int rtt_dev;               /* Smoothed rtt mean difference, in milliseconds/4 */
     struct clock timeout;      /* Current retransmission delay */
-    int backedOff;              /* Has the timeout been backed off due to a missing packet? */
     int nSent;                 /* Total number of distinct data packets sent, not including retransmissions */
     int reSends;               /* Total number of retransmissions for this peer, since this structure was created */