Congestion control usage of negotiated params

3 files changed, 46 insertions, 37 deletions

diff --git a/src/core/or/congestion_control_common.c b/src/core/or/congestion_control_common.c
index b76c0957e4..d4b933a032 100644
--- a/src/core/or/congestion_control_common.c
+++ b/src/core/or/congestion_control_common.c
@@ -33,6 +33,7 @@
  * section 6.5 including tuning notes. */
 #define CIRCWINDOW_INIT (500)
 #define SENDME_INC_DFLT (50)
+#define CC_ALG_DFLT (CC_ALG_SENDME)
 
 #define CWND_INC_DFLT (50)
 #define CWND_INC_PCT_SS_DFLT (100)
@@ -82,6 +83,8 @@ int32_t cell_queue_high = CELL_QUEUE_HIGH_DFLT;
 int32_t cell_queue_low = CELL_QUEUE_LOW_DFLT;
 uint32_t or_conn_highwater = OR_CONN_HIGHWATER_DFLT;
 uint32_t or_conn_lowwater = OR_CONN_LOWWATER_DFLT;
+uint8_t cc_sendme_inc = SENDME_INC_DFLT;
+static cc_alg_t cc_alg = CC_ALG_DFLT;
 
 /**
  * Update global congestion control related consensus parameter values,
@@ -127,6 +130,22 @@ congestion_control_new_consensus_params(const networkstatus_t *ns)
         CWND_MAX_DFLT,
         CWND_MAX_MIN,
         CWND_MAX_MAX);
+
+#define SENDME_INC_MIN 10
+#define SENDME_INC_MAX (1000)
+  cc_sendme_inc =
+    networkstatus_get_param(NULL, "cc_sendme_inc",
+        SENDME_INC_DFLT,
+        SENDME_INC_MIN,
+        SENDME_INC_MAX);
+
+#define CC_ALG_MIN 0
+#define CC_ALG_MAX (NUM_CC_ALGS-1)
+  cc_alg =
+    networkstatus_get_param(NULL, "cc_alg",
+        CC_ALG_DFLT,
+        CC_ALG_MIN,
+        CC_ALG_MAX);
 }
 
 /**
@@ -140,9 +159,10 @@ congestion_control_new_consensus_params(const networkstatus_t *ns)
  */
 static void
 congestion_control_init_params(congestion_control_t *cc,
-                               cc_alg_t cc_alg,
-                               int sendme_inc)
+                               const circuit_params_t *params)
 {
+  cc->sendme_inc = params->sendme_inc_cells;
+
 #define CWND_INIT_MIN 100
 #define CWND_INIT_MAX (10000)
   cc->cwnd =
@@ -175,16 +195,7 @@ congestion_control_init_params(congestion_control_t *cc,
         CWND_INC_RATE_MIN,
         CWND_INC_RATE_MAX);
 
-#define SENDME_INC_MIN 10
-#define SENDME_INC_MAX (1000)
-  cc->sendme_inc =
-    networkstatus_get_param(NULL, "cc_sendme_inc",
-        sendme_inc,
-        SENDME_INC_MIN,
-        SENDME_INC_MAX);
-
-  // XXX: this min needs to abide by sendme_inc range rules somehow
-#define CWND_MIN_MIN sendme_inc
+#define CWND_MIN_MIN 20
 #define CWND_MIN_MAX (1000)
   cc->cwnd_min =
     networkstatus_get_param(NULL, "cc_cwnd_min",
@@ -250,6 +261,14 @@ congestion_control_init_params(congestion_control_t *cc,
   }
 }
 
+/** Returns true if congestion control is enabled in the most recent
+ * consensus */
+bool
+congestion_control_enabled(void)
+{
+  return cc_alg != CC_ALG_SENDME;
+}
+
 /**
  * Allocate and initialize fields in congestion control object.
  *
@@ -259,14 +278,14 @@ congestion_control_init_params(congestion_control_t *cc,
  * acks. This parameter will come from circuit negotiation.
  */
 static void
-congestion_control_init(congestion_control_t *cc, cc_alg_t cc_alg,
-                        int sendme_inc)
+congestion_control_init(congestion_control_t *cc,
+                        const circuit_params_t *params)
 {
   cc->sendme_pending_timestamps = smartlist_new();
   cc->sendme_arrival_timestamps = smartlist_new();
 
   cc->in_slow_start = 1;
-  congestion_control_init_params(cc, cc_alg, sendme_inc);
+  congestion_control_init_params(cc, params);
 
   cc->next_cc_event = CWND_UPDATE_RATE(cc);
 }
@@ -277,12 +296,7 @@ congestion_control_new(const circuit_params_t *params)
 {
   congestion_control_t *cc = tor_malloc_zero(sizeof(congestion_control_t));
 
-  /* TODO-324: Use `params` to pick the algorithm and the window. */
-  (void) params;
-
-  // TODO-324: XXX: the alg and the sendme_inc need to be negotiated during
-  // circuit handshake
-  congestion_control_init(cc, CC_ALG_VEGAS, SENDME_INC_DFLT);
+  congestion_control_init(cc, params);
 
   return cc;
 }
diff --git a/src/core/or/congestion_control_common.h b/src/core/or/congestion_control_common.h
index 81ec79c6e6..c8f6b1c35e 100644
--- a/src/core/or/congestion_control_common.h
+++ b/src/core/or/congestion_control_common.h
@@ -20,11 +20,6 @@ typedef struct congestion_control_t congestion_control_t;
 
 void congestion_control_free_(congestion_control_t *cc);
 
-/* TODO-324: Whisky Tango Foxtot‽  Nothing calls this function anywhere!
- *
- * It needs to be called client-side and relay-side every time we initialize a
- * circuit!
- */
 struct circuit_params_t;
 congestion_control_t *congestion_control_new(
                                     const struct circuit_params_t *params);
@@ -50,12 +45,15 @@ bool is_monotime_clock_reliable(void);
 
 void congestion_control_new_consensus_params(const networkstatus_t *ns);
 
-/* Ugh, C.. these four are private. Use the getter instead, when
+bool congestion_control_enabled(void);
+
+/* Ugh, C.. these are private. Use the getter instead, when
  * external to the congestion control code. */
 extern uint32_t or_conn_highwater;
 extern uint32_t or_conn_lowwater;
 extern int32_t cell_queue_high;
 extern int32_t cell_queue_low;
+extern uint8_t cc_sendme_inc;
 
 /** Stop writing on an orconn when its outbuf is this large */
 static inline uint32_t
@@ -87,6 +85,13 @@ cell_queue_lowwatermark(void)
   return cell_queue_low;
 }
 
+/** Returns the sendme inc rate cached from the most recent consensus */
+static inline uint8_t
+congestion_control_sendme_inc(void)
+{
+  return cc_sendme_inc;
+}
+
 /**
  * Compute an N-count EWMA, aka N-EWMA. N-EWMA is defined as:
  *  EWMA = alpha*value + (1-alpha)*EWMA_prev
diff --git a/src/core/or/congestion_control_flow.c b/src/core/or/congestion_control_flow.c
index 805654664c..d61da73627 100644
--- a/src/core/or/congestion_control_flow.c
+++ b/src/core/or/congestion_control_flow.c
@@ -275,10 +275,6 @@ circuit_process_stream_xoff(edge_connection_t *conn,
    */
   if (TO_CONN(conn)->type == CONN_TYPE_AP || conn->hs_ident != NULL) {
     uint32_t limit = 0;
-
-    /* TODO: This limit technically needs to come from negotiation,
-     * and be bounds checked for sanity, because the other endpoint
-     * may have a different consensus */
     if (conn->hs_ident)
       limit = xoff_client;
     else
@@ -296,9 +292,6 @@ circuit_process_stream_xoff(edge_connection_t *conn,
     }
   }
 
-  // TODO: Count how many xoffs we have; log if "too many", for shadow
-  // analysis of chatter. Possibly add to extra-info?
-
   log_info(LD_EDGE, "Got XOFF!");
   connection_stop_reading(TO_CONN(conn));
   conn->xoff_received = true;
@@ -371,9 +364,6 @@ circuit_process_stream_xon(edge_connection_t *conn,
   if (TO_CONN(conn)->type == CONN_TYPE_AP || conn->hs_ident != NULL) {
     uint32_t limit = 0;
 
-    /* TODO: This limit technically needs to come from negotiation,
-     * and be bounds checked for sanity, because the other endpoint
-     * may have a different consensus */
     if (conn->hs_ident)
       limit = MIN(xoff_client, xon_rate_bytes);
     else