Bug 40566: Remove unused BDP estimators

src/core/or/congestion_control_common.c

@@ -86,8 +86,7 @@
 
 static bool congestion_control_update_circuit_bdp(congestion_control_t *,
                                                   const circuit_t *,
-                                                  const crypt_path_t *,
+                                                  uint64_t);
-                                                  uint64_t, uint64_t);
 /* Number of times the RTT value was reset. For MetricsPort. */
 static uint64_t num_rtt_reset;
 
@@ -321,12 +320,6 @@ congestion_control_init_params(congestion_control_t *cc,
     cc->cc_alg = cc_alg;
   }
 
-  cc->bdp_alg =
-    networkstatus_get_param(NULL, "cc_bdp_alg",
-        VEGAS_BDP_MIX_ALG,
-        0,
-        NUM_BDP_ALGS-1);
-
   /* Algorithm-specific parameters */
   if (cc->cc_alg == CC_ALG_VEGAS) {
     congestion_control_vegas_set_params(cc, path);
@@ -407,7 +400,6 @@ congestion_control_init(congestion_control_t *cc,
                         cc_path_t path)
 {
   cc->sendme_pending_timestamps = smartlist_new();
-  cc->sendme_arrival_timestamps = smartlist_new();
 
   cc->in_slow_start = 1;
   congestion_control_init_params(cc, params, path);
@@ -438,9 +430,7 @@ congestion_control_free_(congestion_control_t *cc)
     return;
 
   SMARTLIST_FOREACH(cc->sendme_pending_timestamps, uint64_t *, t, tor_free(t));
-  SMARTLIST_FOREACH(cc->sendme_arrival_timestamps, uint64_t *, t, tor_free(t));
   smartlist_free(cc->sendme_pending_timestamps);
-  smartlist_free(cc->sendme_arrival_timestamps);
 
   tor_free(cc);
 }
@@ -457,22 +447,6 @@ enqueue_timestamp(smartlist_t *timestamps_u64, uint64_t timestamp_usec)
   smartlist_add(timestamps_u64, timestamp_ptr);
 }
 
-/**
- * Peek at the head of a smartlist queue of u64 timestamps.
- */
-static inline uint64_t
-peek_timestamp(const smartlist_t *timestamps_u64_usecs)
-{
-  uint64_t *timestamp_ptr = smartlist_get(timestamps_u64_usecs, 0);
-
-  if (BUG(!timestamp_ptr)) {
-    log_err(LD_CIRC, "Congestion control timestamp list became empty!");
-    return 0;
-  }
-
-  return *timestamp_ptr;
-}
-
 /**
  * Dequeue a u64 monotime usec timestamp from the front of a
  * smartlist of pointers to 64.
@@ -676,61 +650,6 @@ congestion_control_note_cell_sent(congestion_control_t *cc,
                     monotime_absolute_usec());
 }
 
-/**
- * Returns true if any edge connections are active.
- *
- * We need to know this so that we can stop computing BDP if the
- * edges are not sending on the circuit.
- */
-static int
-circuit_has_active_streams(const circuit_t *circ,
-                           const crypt_path_t *layer_hint)
-{
-  const edge_connection_t *streams;
-
-  if (CIRCUIT_IS_ORIGIN(circ)) {
-    streams = CONST_TO_ORIGIN_CIRCUIT(circ)->p_streams;
-  } else {
-    streams = CONST_TO_OR_CIRCUIT(circ)->n_streams;
-  }
-
-  /* Check linked list of streams */
-  for (const edge_connection_t *conn = streams; conn != NULL;
-       conn = conn->next_stream) {
-    if (conn->base_.marked_for_close)
-      continue;
-
-    if (edge_uses_cpath(conn, layer_hint)) {
-      if (connection_get_inbuf_len(TO_CONN(conn)) > 0) {
-        log_info(LD_CIRC, "CC: More in edge inbuf...");
-        return 1;
-      }
-
-      /* If we did not reach EOF on this read, there's more */
-      if (!TO_CONN(conn)->inbuf_reached_eof) {
-        log_info(LD_CIRC, "CC: More on edge conn...");
-        return 1;
-      }
-
-      if (TO_CONN(conn)->linked_conn) {
-        if (connection_get_inbuf_len(TO_CONN(conn)->linked_conn) > 0) {
-          log_info(LD_CIRC, "CC: More in linked inbuf...");
-          return 1;
-        }
-
-        /* If there is a linked conn, and *it* did not each EOF,
-         * there's more */
-        if (!TO_CONN(conn)->linked_conn->inbuf_reached_eof) {
-          log_info(LD_CIRC, "CC: More on linked conn...");
-          return 1;
-        }
-      }
-    }
-  }
-
-  return 0;
-}
-
 /**
  * Upon receipt of a SENDME, pop the oldest timestamp off the timestamp
  * list, and use this to update RTT.
@@ -740,15 +659,13 @@ circuit_has_active_streams(const circuit_t *circ,
  */
 bool
 congestion_control_update_circuit_estimates(congestion_control_t *cc,
-                                            const circuit_t *circ,
+                                            const circuit_t *circ)
-                                            const crypt_path_t *layer_hint)
 {
   uint64_t now_usec = monotime_absolute_usec();
 
   /* Update RTT first, then BDP. BDP needs fresh RTT */
   uint64_t curr_rtt_usec = congestion_control_update_circuit_rtt(cc, now_usec);
-  return congestion_control_update_circuit_bdp(cc, circ, layer_hint, now_usec,
+  return congestion_control_update_circuit_bdp(cc, circ, curr_rtt_usec);
-                                               curr_rtt_usec);
 }
 
 /**
@@ -764,13 +681,6 @@ time_delta_should_use_heuristics(const congestion_control_t *cc)
     return true;
   }
 
-  /* If we managed to get enough acks to estimate a SENDME BDP, then
-   * we have enough to estimate clock jumps relative to a baseline,
-   * too. (This is at least 'cc_bwe_min' acks). */
-  if (cc->bdp[BDP_ALG_SENDME_RATE]) {
-    return true;
-  }
-
   /* Not enough data to estimate clock jumps */
   return false;
 }
@@ -937,14 +847,10 @@ congestion_control_update_circuit_rtt(congestion_control_t *cc,
 static bool
 congestion_control_update_circuit_bdp(congestion_control_t *cc,
                                       const circuit_t *circ,
-                                      const crypt_path_t *layer_hint,
-                                      uint64_t now_usec,
                                       uint64_t curr_rtt_usec)
 {
   int chan_q = 0;
   unsigned int blocked_on_chan = 0;
-  uint64_t timestamp_usec;
-  uint64_t sendme_rate_bdp = 0;
 
   tor_assert(cc);
 
@@ -982,10 +888,7 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc,
        cc->blocked_chan = 0;
      }
 
-     cc->bdp[BDP_ALG_CWND_RTT] = cwnd;
+     cc->bdp = cwnd;
-     cc->bdp[BDP_ALG_INFLIGHT_RTT] = cwnd;
-     cc->bdp[BDP_ALG_SENDME_RATE] = cwnd;
-     cc->bdp[BDP_ALG_PIECEWISE] = cwnd;
 
      static ratelim_t dec_notice_limit = RATELIM_INIT(300);
      log_fn_ratelim(&dec_notice_limit, LOG_NOTICE, LD_CIRC,
@@ -1004,84 +907,7 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc,
    * close to ewma RTT. Since all fields are u64, there is plenty of
    * room here to multiply first.
    */
-  cc->bdp[BDP_ALG_CWND_RTT] = cc->cwnd*cc->min_rtt_usec/cc->ewma_rtt_usec;
+  cc->bdp = cc->cwnd*cc->min_rtt_usec/cc->ewma_rtt_usec;
-
-  /*
-   * If we have no pending streams, we do not have enough data to fill
-   * the BDP, so preserve our old estimates but do not make any more.
-   */
-  if (!blocked_on_chan && !circuit_has_active_streams(circ, layer_hint)) {
-    log_info(LD_CIRC,
-               "CC: Streams drained. Spare package window: %"PRIu64
-               ", no BDP update", cc->cwnd - cc->inflight);
-
-    /* Clear SENDME timestamps; they will be wrong with intermittent data */
-    SMARTLIST_FOREACH(cc->sendme_arrival_timestamps, uint64_t *, t,
-                      tor_free(t));
-    smartlist_clear(cc->sendme_arrival_timestamps);
-  } else if (curr_rtt_usec && is_monotime_clock_reliable()) {
-    /* Sendme-based BDP will quickly measure BDP in much less than
-     * a cwnd worth of data when in use (in 2-10 SENDMEs).
-     *
-     * But if the link goes idle, it will be vastly lower than true BDP. Hence
-     * we only compute it if we have either pending stream data, or streams
-     * are still blocked on the channel queued data.
-     *
-     * We also do not compute it if we do not have a current RTT passed in,
-     * because that means that monotime is currently stalled or just jumped.
-     */
-    enqueue_timestamp(cc->sendme_arrival_timestamps, now_usec);
-
-    if (smartlist_len(cc->sendme_arrival_timestamps) >= bwe_sendme_min) {
-      /* If we have more sendmes than fit in a cwnd, trim the list.
-       * Those are not acurrately measuring throughput, if cwnd is
-       * currently smaller than BDP */
-      while (smartlist_len(cc->sendme_arrival_timestamps) >
-             bwe_sendme_min &&
-             (uint64_t)smartlist_len(cc->sendme_arrival_timestamps) >
-                       n_ewma_count(cc)) {
-        (void)dequeue_timestamp(cc->sendme_arrival_timestamps);
-      }
-      int sendme_cnt = smartlist_len(cc->sendme_arrival_timestamps);
-
-      /* Calculate SENDME_BWE_COUNT pure average */
-      timestamp_usec = peek_timestamp(cc->sendme_arrival_timestamps);
-      uint64_t delta = now_usec - timestamp_usec;
-
-      /* In Shadow, the time delta between acks can be 0 if there is no
-       * network activity between them. Only update BDP if the delta is
-       * non-zero. */
-      if (delta > 0) {
-        /* The acked data is in sendme_cnt-1 chunks, because we are counting
-         * the data that is processed by the other endpoint *between* all of
-         * these sendmes. There's one less gap between the sendmes than the
-         * number of sendmes. */
-        uint64_t cells = (sendme_cnt-1)*cc->sendme_inc;
-
-        /* The bandwidth estimate is cells/delta, which when multiplied
-         * by min RTT obtains the BDP. However, we multiply first to
-         * avoid precision issues with the RTT being close to delta in size. */
-        sendme_rate_bdp = cells*cc->min_rtt_usec/delta;
-
-        /* Calculate BDP_EWMA_COUNT N-EWMA */
-        cc->bdp[BDP_ALG_SENDME_RATE] =
-                   n_count_ewma(sendme_rate_bdp, cc->bdp[BDP_ALG_SENDME_RATE],
-                                n_ewma_count(cc));
-      }
-    }
-
-    /* In-flight BDP will cause the cwnd to drift down when underutilized.
-     * It is most useful when the local OR conn is blocked, so we only
-     * compute it if we're utilized. */
-    cc->bdp[BDP_ALG_INFLIGHT_RTT] =
-        (cc->inflight - chan_q)*cc->min_rtt_usec/
-                              MAX(cc->ewma_rtt_usec, curr_rtt_usec);
-  } else {
-    /* We can still update inflight with just an EWMA RTT, but only
-     * if there is data flowing */
-    cc->bdp[BDP_ALG_INFLIGHT_RTT] =
-        (cc->inflight - chan_q)*cc->min_rtt_usec/cc->ewma_rtt_usec;
-  }
 
   /* The orconn is blocked; use smaller of inflight vs SENDME */
   if (blocked_on_chan) {
@@ -1094,13 +920,6 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc,
       cc->next_cc_event = 0;
       cc->blocked_chan = 1;
     }
-
-    if (cc->bdp[BDP_ALG_SENDME_RATE]) {
-      cc->bdp[BDP_ALG_PIECEWISE] = MIN(cc->bdp[BDP_ALG_INFLIGHT_RTT],
-                                      cc->bdp[BDP_ALG_SENDME_RATE]);
-    } else {
-      cc->bdp[BDP_ALG_PIECEWISE] = cc->bdp[BDP_ALG_INFLIGHT_RTT];
-    }
   } else {
     /* If we were previously blocked, emit a new congestion event
      * now that we are unblocked, to re-evaluate cwnd */
@@ -1110,19 +929,6 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc,
       log_info(LD_CIRC, "CC: Streams un-blocked on circ channel. Chanq: %d",
                chan_q);
     }
-
-    cc->bdp[BDP_ALG_PIECEWISE] = MAX(cc->bdp[BDP_ALG_SENDME_RATE],
-                                     cc->bdp[BDP_ALG_CWND_RTT]);
-  }
-
-  /* We can end up with no piecewise value if we didn't have either
-   * a SENDME estimate or enough data for an inflight estimate.
-   * It also happens on the very first sendme, since we need two
-   * to get a BDP. In these cases, use the cwnd method. */
-  if (!cc->bdp[BDP_ALG_PIECEWISE]) {
-    cc->bdp[BDP_ALG_PIECEWISE] = cc->bdp[BDP_ALG_CWND_RTT];
-    log_info(LD_CIRC, "CC: No piecewise BDP. Using %"PRIu64,
-             cc->bdp[BDP_ALG_PIECEWISE]);
   }
 
   if (cc->next_cc_event == 0) {
@@ -1130,44 +936,25 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc,
       log_info(LD_CIRC,
                  "CC: Circuit %d "
                  "SENDME RTT: %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64", "
-                 "BDP estimates: "
+                 "BDP estimate: %"PRIu64,
-                 "%"PRIu64", "
-                 "%"PRIu64", "
-                 "%"PRIu64", "
-                 "%"PRIu64", "
-                 "%"PRIu64". ",
                CONST_TO_ORIGIN_CIRCUIT(circ)->global_identifier,
                cc->min_rtt_usec/1000,
                curr_rtt_usec/1000,
                cc->ewma_rtt_usec/1000,
                cc->max_rtt_usec/1000,
-               cc->bdp[BDP_ALG_INFLIGHT_RTT],
+               cc->bdp);
-               cc->bdp[BDP_ALG_CWND_RTT],
-               sendme_rate_bdp,
-               cc->bdp[BDP_ALG_SENDME_RATE],
-               cc->bdp[BDP_ALG_PIECEWISE]
-               );
     } else {
       log_info(LD_CIRC,
                  "CC: Circuit %"PRIu64":%d "
                  "SENDME RTT: %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64", "
-                 "%"PRIu64", "
+                 "%"PRIu64,
-                 "%"PRIu64", "
-                 "%"PRIu64", "
-                 "%"PRIu64", "
-                 "%"PRIu64". ",
                  CONST_TO_OR_CIRCUIT(circ)->p_chan->global_identifier,
                  CONST_TO_OR_CIRCUIT(circ)->p_circ_id,
                  cc->min_rtt_usec/1000,
                  curr_rtt_usec/1000,
                  cc->ewma_rtt_usec/1000,
                  cc->max_rtt_usec/1000,
-                 cc->bdp[BDP_ALG_INFLIGHT_RTT],
+                 cc->bdp);
-                 cc->bdp[BDP_ALG_CWND_RTT],
-                 sendme_rate_bdp,
-                 cc->bdp[BDP_ALG_SENDME_RATE],
-                 cc->bdp[BDP_ALG_PIECEWISE]
-                 );
     }
   }
 
@@ -1175,8 +962,7 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc,
    * the curr_rtt_usec was not 0. */
   bool ret = (blocked_on_chan || curr_rtt_usec != 0);
   if (ret) {
-    tor_trace(TR_SUBSYS(cc), TR_EV(bdp_update), circ, cc, curr_rtt_usec,
+    tor_trace(TR_SUBSYS(cc), TR_EV(bdp_update), circ, cc, curr_rtt_usec);
-              sendme_rate_bdp);
   }
   return ret;
 }
@@ -1186,13 +972,12 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc,
  */
 int
 congestion_control_dispatch_cc_alg(congestion_control_t *cc,
-                                   circuit_t *circ,
+                                   circuit_t *circ)
-                                   const crypt_path_t *layer_hint)
 {
   int ret = -END_CIRC_REASON_INTERNAL;
 
   tor_assert_nonfatal_once(cc->cc_alg == CC_ALG_VEGAS);
-  ret = congestion_control_vegas_process_sendme(cc, circ, layer_hint);
+  ret = congestion_control_vegas_process_sendme(cc, circ);
 
   if (cc->cwnd > cwnd_max) {
     static ratelim_t cwnd_limit = RATELIM_INIT(60);

src/core/or/congestion_control_common.h

@@ -46,16 +46,14 @@ congestion_control_t *congestion_control_new(
                                     cc_path_t path);
 
 int congestion_control_dispatch_cc_alg(congestion_control_t *cc,
-                                       circuit_t *circ,
+                                       circuit_t *circ);
-                                       const crypt_path_t *layer_hint);
 
 void congestion_control_note_cell_sent(congestion_control_t *cc,
                                        const circuit_t *circ,
                                        const crypt_path_t *cpath);
 
 bool congestion_control_update_circuit_estimates(congestion_control_t *,
-                                                 const circuit_t *,
+                                                 const circuit_t *);
-                                                 const crypt_path_t *);
 
 int congestion_control_get_package_window(const circuit_t *,
                                           const crypt_path_t *);

src/core/or/congestion_control_st.h

@@ -106,19 +106,13 @@ struct congestion_control_t {
    * sendme_last_digests. */
   smartlist_t *sendme_pending_timestamps;
 
-  /**
-   * Smartlist of uint64_t monotime timestamp of when sendme's arrived.
-   * FIFO queue that is managed similar to sendme_last_digests.
-   * Used to estimate circuitbandwidth and BDP. */
-  smartlist_t *sendme_arrival_timestamps;
-
   /** RTT time data for congestion control. */
   uint64_t ewma_rtt_usec;
   uint64_t min_rtt_usec;
   uint64_t max_rtt_usec;
 
-  /* BDP estimates by algorithm */
+  /* Vegas BDP estimate */
-  uint64_t bdp[NUM_BDP_ALGS];
+  uint64_t bdp;
 
   /** Congestion window */
   uint64_t cwnd;

src/core/or/congestion_control_vegas.c

@@ -98,7 +98,7 @@ uint64_t cc_stats_vegas_circ_exited_ss = 0;
 static inline uint64_t
 vegas_bdp(const congestion_control_t *cc)
 {
-  return cc->bdp[BDP_ALG_CWND_RTT];
+  return cc->bdp;
 }
 
 /**
@@ -405,8 +405,7 @@ cwnd_full_reset(const congestion_control_t *cc)
  */
 int
 congestion_control_vegas_process_sendme(congestion_control_t *cc,
-                                        const circuit_t *circ,
+                                        const circuit_t *circ)
-                                        const crypt_path_t *layer_hint)
 {
   uint64_t queue_use;
 
@@ -422,7 +421,7 @@ congestion_control_vegas_process_sendme(congestion_control_t *cc,
     cc->next_cwnd_event--;
 
   /* Compute BDP and RTT. If we did not update, don't run the alg */
-  if (!congestion_control_update_circuit_estimates(cc, circ, layer_hint)) {
+  if (!congestion_control_update_circuit_estimates(cc, circ)) {
     cc->inflight = cc->inflight - cc->sendme_inc;
     return 0;
   }

src/core/or/congestion_control_vegas.h

@@ -35,8 +35,7 @@ extern uint64_t cc_stats_vegas_circ_exited_ss;
 
 /* Processing SENDME cell. */
 int congestion_control_vegas_process_sendme(struct congestion_control_t *cc,
-                                            const circuit_t *circ,
+                                            const circuit_t *circ);
-                                            const crypt_path_t *layer_hint);
 void congestion_control_vegas_set_params(struct congestion_control_t *cc,
                                          cc_path_t path);
 

src/core/or/lttng_cc.inc

@@ -142,7 +142,7 @@ TRACEPOINT_EVENT(tor_cc, flow_decide_xoff_sending,
 /* Emitted when the BDP value has been updated. */
 TRACEPOINT_EVENT(tor_cc, bdp_update,
   TP_ARGS(const circuit_t *, circ, const congestion_control_t *, cc,
-          uint64_t, curr_rtt_usec, uint64_t, sendme_rate_bdp),
+          uint64_t, curr_rtt_usec),
   TP_FIELDS(
     ctf_integer(uint64_t, circuit_ptr, circ)
     ctf_integer(uint32_t, n_circ_id, circ->n_circ_id)
@@ -150,11 +150,7 @@ TRACEPOINT_EVENT(tor_cc, bdp_update,
     ctf_integer(uint64_t, curr_rtt_usec, curr_rtt_usec)
     ctf_integer(uint64_t, ewma_rtt_usec, cc->ewma_rtt_usec)
     ctf_integer(uint64_t, max_rtt_usec, cc->max_rtt_usec)
-    ctf_integer(uint64_t, bdp_inflight_rtt, cc->bdp[BDP_ALG_INFLIGHT_RTT])
+    ctf_integer(uint64_t, bdp_cwnd_rtt, cc->bdp)
-    ctf_integer(uint64_t, bdp_cwnd_rtt, cc->bdp[BDP_ALG_CWND_RTT])
-    ctf_integer(uint64_t, bdp_sendme_rate, cc->bdp[BDP_ALG_SENDME_RATE])
-    ctf_integer(uint64_t, bdp_piecewise, cc->bdp[BDP_ALG_PIECEWISE])
-    ctf_integer(uint64_t, sendme_rate_bdp, sendme_rate_bdp)
   )
 )
 

src/core/or/sendme.c

@@ -491,7 +491,7 @@ sendme_process_circuit_level(crypt_path_t *layer_hint,
     return sendme_process_circuit_level_impl(layer_hint, circ);
   }
 
-  return congestion_control_dispatch_cc_alg(cc, circ, layer_hint);
+  return congestion_control_dispatch_cc_alg(cc, circ);
 }
 
 /**

src/test/test_conflux_pool.c

@@ -128,7 +128,6 @@ circuit_establish_circuit_conflux_mock(const uint8_t *conflux_nonce,
   simulate_single_hop_extend(circ, 0);
   simulate_single_hop_extend(circ, 1);
   circ->cpath->prev->ccontrol = tor_malloc_zero(sizeof(congestion_control_t));
-  circ->cpath->prev->ccontrol->sendme_arrival_timestamps = smartlist_new();
   circ->cpath->prev->ccontrol->sendme_pending_timestamps = smartlist_new();
   circ->cpath->prev->ccontrol->sendme_inc = 31;
 
@@ -499,7 +498,6 @@ simulate_circuit_build(circuit_t *client_circ)
   relay_side->purpose = CIRCUIT_PURPOSE_OR;
   relay_side->n_chan = NULL; // No next hop
   relay_side->ccontrol = tor_malloc_zero(sizeof(congestion_control_t));
-  relay_side->ccontrol->sendme_arrival_timestamps = smartlist_new();
   relay_side->ccontrol->sendme_pending_timestamps = smartlist_new();
   relay_side->ccontrol->sendme_inc = 31;
   smartlist_add(exit_circs, relay_side);

src/test/test_congestion_control.c

@@ -216,7 +216,7 @@ run_vegas_cwnd_test_vec(congestion_control_t *cc,
     circ->circuit_blocked_on_p_chan = vec[i].or_conn_blocked_in;
     cc->inflight = vec[i].inflight_in;
 
-    congestion_control_vegas_process_sendme(cc, circ, NULL);
+    congestion_control_vegas_process_sendme(cc, circ);
 
     /* If the or conn was blocked, ensure we updated our
      * CC state */