fix ack aggregation compensation (holepunchto#259)

* fix ack aggregation estimator

Author: jthomas43

docs/debug_throughput/stevens.py

@@ -88,7 +88,7 @@ def processFile(path_to_file):
 for (x, label) in vlines:
     print("x=", x, "label=", label)
     plot.axvline(x=x, color=colors[color_index], ls="--", label=label)
-    color_index = color_index + 1 % len(colors)
+    color_index = (color_index + 1) % len(colors)
 
 axes.legend()
 d.legend()

include/udx.h

@@ -314,11 +314,14 @@ struct udx_stream_s {
     uint32_t prior_cwnd;
     double full_bw;
 
-    uint64_t ack_epoch_start;
-    uint16_t extra_acked[2];  // volume of data that estimates the degree of aggregation in the network path.
-    uint32_t ack_epoch_acked; // packets (S)ACKed in sampling epoch
-    uint8_t extra_acked_win_rtts;
-    uint8_t extra_acked_win_index;
+    // track windowed maximum of ACK aggregation This enables increasing cwnd
+    // on networks with high ACK aggregation to continue sending during interruptions in the ACK stream.
+
+    uint64_t ack_epoch_start;      // timestamp of current ack epoch. reset when ack rate is <= expected
+    uint16_t extra_acked[2];       // track the windowed maximum degree of ack aggregation
+    uint32_t ack_epoch_acked;      // packets (S)ACKed in current sampling epoch
+    uint8_t extra_acked_win_rtts;  // after extra_acked_win_rtts intervals rotate extra_acked_win_index
+    uint8_t extra_acked_win_index; // position in bbr.extra_acked[]
 
   } bbr;
 

src/debug.h

@@ -34,7 +34,7 @@ static inline void
 open_throughput_file (udx_stream_t *stream) {
   if (!stream->throughput_fd) {
     char filename[100];
-    snprintf(filename, sizeof(filename), "stream.%d.dat", stream->local_id);
+    snprintf(filename, sizeof(filename), "stream.%u.dat", stream->local_id);
     stream->throughput_fd = fopen(filename, "w");
   }
 }

src/udx_bbr.c

@@ -176,12 +176,18 @@ bbr_inflight (udx_stream_t *stream, double bw, double gain) {
   return bbr_bdp(stream, bw, gain);
 }
 
-// unneccessary?
+// Return the highest recently seen degree of ack aggregation. unit is packets.
+// aggregation is the amount acked during an rtt interval greater than
+// the amount predicted by the current pacing rate. (acked - expected_acked)
+// This is used to set a higher cwnd, allowing us to continue sending
+// during interruptions of the ack stream.
+
 static uint32_t
 bbr_ack_aggregation_cwnd (udx_stream_t *stream) {
   uint32_t aggr_cwnd = 0;
   if (bbr_extra_acked_gain && bbr_full_bw_reached(stream)) {
-    uint32_t max_aggr_cwnd = (bbr_bw(stream) * bbr_extra_acked_max_ms) / 1000;
+    // clamps extra aggregation packets to at most extra_acked_max_ms (100ms) of packets at current bw
+    uint32_t max_aggr_cwnd = bbr_bw(stream) * bbr_extra_acked_max_ms;
 
     aggr_cwnd = (bbr_extra_acked_gain * bbr_extra_acked(stream));
     aggr_cwnd = min_uint32(aggr_cwnd, max_aggr_cwnd);
@@ -356,6 +362,7 @@ bbr_update_ack_aggregation (udx_stream_t *stream, udx_rate_sample_t *rs) {
   }
 
   if (stream->bbr.round_start) {
+    // redundant since we clamp to bbr_extra_acked_win_rtts
     stream->bbr.extra_acked_win_rtts = min_uint32(0xff, stream->bbr.extra_acked_win_rtts + 1);
     if (stream->bbr.extra_acked_win_rtts >= bbr_extra_acked_win_rtts) {
       stream->bbr.extra_acked_win_rtts = 0;
@@ -367,7 +374,9 @@ bbr_update_ack_aggregation (udx_stream_t *stream, udx_rate_sample_t *rs) {
   uint32_t epoch_ms = time_delta_ms(stream->delivered_ts, stream->bbr.ack_epoch_start);
   uint32_t expected_acked = bbr_bw(stream) * epoch_ms;
 
-  // reset aggregation epoch if ACK rate is below expected or significanly large no of ack received since epoch
+  // reset aggregation epoch if
+  // 1. ACK rate is below expected or
+  // 2. an (improbably large) # of acks that would overflow ack_epoch_acked arrives
   if (stream->bbr.ack_epoch_acked <= expected_acked || ((uint64_t) stream->bbr.ack_epoch_acked + rs->acked_sacked >= bbr_ack_epoch_acked_reset_thresh)) {
     stream->bbr.ack_epoch_acked = 0;
     stream->bbr.ack_epoch_start = stream->delivered_ts;