From: Steffan Karger <steffan.karger@fox-it.com>
Date: Wed, 31 Mar 2021 18:03:23 +0000 (+0200)
Subject: reliable: retransmit if 3 follow-up ACKs are received
X-Git-Tag: v2.6_beta1~565
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=203afbe95ee324ec941e35399f6655dc4f669f12;p=thirdparty%2Fopenvpn.git

reliable: retransmit if 3 follow-up ACKs are received

To improve the control channel performance under packet loss conditions,
add a more aggressive retransmit policy similar to what many TCP
implementations do: retransmit a packet if the ACK timeout expires (like
we already do), *or* if three ACKs for follow-up packets are received.

The rationale behind this is that if follow-up packets *are* received, the
connection is apparently functional and we should be able to retransmit
immediately. This significantly improves performance for connections with
low (up to a few percent) packet loss.
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <E1lRfW3-0001sy-VM@sfs-ml-4.v29.lw.sourceforge.com>
URL: https://www.mail-archive.com/search?l=mid&q=E1lRfW3-0001sy-VM@sfs-ml-4.v29.lw.sourceforge.com

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---

diff --git a/src/openvpn/reliable.c b/src/openvpn/reliable.c
index 6c1f2da15..15b90fbe4 100644
--- a/src/openvpn/reliable.c
+++ b/src/openvpn/reliable.c
@@ -382,7 +382,14 @@ reliable_send_purge(struct reliable *rel, const struct reliable_ack *ack)
                 }
 #endif
                 e->active = false;
-                break;
+            }
+            else if (e->active && e->packet_id < pid)
+            {
+                /* We have received an ACK for a packet with a higher PID. Either
+                 * we have received ACKs out of or order or the packet has been
+                 * lost. We count the number of ACKs to determine if we should
+                 * resend it early. */
+                e->n_acks++;
             }
         }
     }
@@ -555,7 +562,7 @@ reliable_can_send(const struct reliable *rel)
         if (e->active)
         {
             ++n_active;
-            if (now >= e->next_try)
+            if (now >= e->next_try || e->n_acks >= N_ACK_RETRANSMIT)
             {
                 ++n_current;
             }
@@ -581,7 +588,12 @@ reliable_send(struct reliable *rel, int *opcode)
     for (i = 0; i < rel->size; ++i)
     {
         struct reliable_entry *e = &rel->array[i];
-        if (e->active && local_now >= e->next_try)
+
+        /* If N_ACK_RETRANSMIT later packets have received ACKs, we assume
+         * that the packet was lost and resend it even if the timeout has
+         * not expired yet. */
+        if (e->active
+            && (e->n_acks >= N_ACK_RETRANSMIT || local_now >= e->next_try))
         {
             if (!best || reliable_pid_min(e->packet_id, best->packet_id))
             {
@@ -599,6 +611,7 @@ reliable_send(struct reliable *rel, int *opcode)
         /* constant timeout, no backoff */
         best->next_try = local_now + best->timeout;
 #endif
+        best->n_acks = 0;
         *opcode = best->opcode;
         dmsg(D_REL_DEBUG, "ACK reliable_send ID " packet_id_format " (size=%d to=%d)",
              (packet_id_print_type)best->packet_id, best->buf.len,
@@ -686,6 +699,7 @@ reliable_mark_active_incoming(struct reliable *rel, struct buffer *buf,
             e->opcode = opcode;
             e->next_try = 0;
             e->timeout = 0;
+            e->n_acks = 0;
             dmsg(D_REL_DEBUG, "ACK mark active incoming ID " packet_id_format, (packet_id_print_type)e->packet_id);
             return;
         }
diff --git a/src/openvpn/reliable.h b/src/openvpn/reliable.h
index a84d42907..97e6dce7e 100644
--- a/src/openvpn/reliable.h
+++ b/src/openvpn/reliable.h
@@ -52,6 +52,10 @@
                                  *   the reliability layer for one VPN
                                  *   tunnel in one direction can store. */
 
+#define N_ACK_RETRANSMIT 3      /**< We retry sending a packet early if
+                                 *   this many later packets have been
+                                 *   ACKed. */
+
 /**
  * The acknowledgment structure in which packet IDs are stored for later
  * acknowledgment.
@@ -72,6 +76,9 @@ struct reliable_entry
     interval_t timeout;
     time_t next_try;
     packet_id_type packet_id;
+    size_t n_acks;  /* Number of acks received for packets with higher PID.
+                     * Used for fast retransmission when there were at least
+                     * N_ACK_RETRANSMIT. */
     int opcode;
     struct buffer buf;
 };