cc: Add comments and clean up some syntax

Signed-off-by: David Goulet <dgoulet@torproject.org>

src/core/or/congestion_control_common.c

@@ -26,30 +26,45 @@
 #include "lib/time/compat_time.h"
 #include "feature/nodelist/networkstatus.h"
 
-/* Consensus parameter defaults */
+/* Consensus parameter defaults.
+ *
+ * More details for each of the parameters can be found in proposal 324,
+ * section 6.5 including tuning notes. */
 #define CIRCWINDOW_INIT (500)
-
+#define SENDME_INC_DFLT (50)
-#define CWND_INC_PCT_SS_DFLT (100)
-
-#define SENDME_INC_DFLT  (50)
-#define CWND_MIN_DFLT    (MAX(100, SENDME_INC_DFLT))
-#define CWND_MAX_DFLT    (INT32_MAX)
 
 #define CWND_INC_DFLT (50)
-
+#define CWND_INC_PCT_SS_DFLT (100)
 #define CWND_INC_RATE_DFLT (1)
+#define CWND_MAX_DFLT (INT32_MAX)
+#define CWND_MIN_DFLT (MAX(100, SENDME_INC_DFLT))
 
+#define BWE_SENDME_MIN_DFLT (5)
+#define EWMA_CWND_COUNT_DFLT (2)
+
+/* BDP algorithms for each congestion control algorithms use the piecewise
+ * estimattor. See section 3.1.4 of proposal 324. */
 #define WESTWOOD_BDP_ALG BDP_ALG_PIECEWISE
 #define VEGAS_BDP_MIX_ALG BDP_ALG_PIECEWISE
 #define NOLA_BDP_ALG BDP_ALG_PIECEWISE
 
-#define EWMA_CWND_COUNT_DFLT  2
+/* Indicate OR connection buffer limitations used to stop or start accepting
-
+ * cells in its outbuf.
-#define BWE_SENDME_MIN_DFLT   5
+ *
-
+ * These watermarks are historical to tor in a sense that they've been used
+ * almost from the genesis point. And were likely defined to fit the bounds of
+ * TLS records of 16KB which would be around 32 cells.
+ *
+ * These are defaults of the consensus parameter "orconn_high" and "orconn_low"
+ * values. */
 #define OR_CONN_HIGHWATER_DFLT (32*1024)
 #define OR_CONN_LOWWATER_DFLT (16*1024)
 
+/* Low and high values of circuit cell queue sizes. They are used to tell when
+ * to start or stop reading on the streams attached on the circuit.
+ *
+ * These are defaults of the consensus parameters "cellq_high" and "cellq_low".
+ */
 #define CELL_QUEUE_LOW_DFLT (10)
 #define CELL_QUEUE_HIGH_DFLT (256)
 
@@ -60,12 +75,12 @@ static bool congestion_control_update_circuit_bdp(congestion_control_t *,
                                                   const crypt_path_t *,
                                                   uint64_t, uint64_t);
 
+/* Consensus parameters cached. The non static ones are extern. */
 static uint32_t cwnd_max = CWND_MAX_DFLT;
-uint32_t or_conn_highwater = OR_CONN_HIGHWATER_DFLT;
-uint32_t or_conn_lowwater = OR_CONN_LOWWATER_DFLT;
-
 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;
 
 /**
  * Update global congestion control related consensus parameter values,

src/core/or/congestion_control_flow.c

@@ -32,28 +32,34 @@ static uint32_t xoff_client;
 static uint32_t xoff_exit;
 
 static uint32_t xon_change_pct;
-static uint32_t xon_rate_bytes;
 static uint32_t xon_ewma_cnt;
+static uint32_t xon_rate_bytes;
 
-/** In normal operation, we can get a burst of up to 32 cells before
+/* In normal operation, we can get a burst of up to 32 cells before returning
- * returning to libevent to flush the outbuf. This is a heuristic from
+ * to libevent to flush the outbuf. This is a heuristic from hardcoded values
- * hardcoded values and strange logic in connection_bucket_get_share(). */
+ * and strange logic in connection_bucket_get_share(). */
 #define MAX_EXPECTED_CELL_BURST 32
 
-/**
+/* The following three are for dropmark rate limiting. They define when we
- * The following three are for dropmark rate limiting. They define when
+ * scale down our XON, XOFF, and xmit byte counts. Early scaling is beneficial
- * we scale down our XON, XOFF, and xmit byte counts. Early scaling
+ * because it limits the ability of spurious XON/XOFF to be sent after large
- * is beneficial because it limits the ability of spurious XON/XOFF
+ * amounts of data without XON/XOFF. At these limits, after 10MB of data (or
- * to be sent after large amounts of data without XON/XOFF. At these
+ * more), an adversary can only inject (log2(10MB)-log2(200*500))*100 ~= 1000
- * limits, after 10MB of data (or more), an adversary can only inject
+ * cells of fake XOFF/XON before the xmit byte count will be halved enough to
- * (log2(10MB)-log2(200*500))*100 ~= 1000 cells of fake XOFF/XON before
+ * triggering a limit. */
- * the xmit byte * count will be halved enough to triggering a limit. */
 #define XON_COUNT_SCALE_AT 200
 #define XOFF_COUNT_SCALE_AT 200
 #define ONE_MEGABYTE (UINT64_C(1) << 20)
-#define TOTAL_XMIT_SCALE_AT 10*ONE_MEGABYTE
+#define TOTAL_XMIT_SCALE_AT (10 * ONE_MEGABYTE)
 
-static const congestion_control_t *
+/**
+ * Return the congestion control object of the given edge connection.
+ *
+ * Returns NULL if the edge connection doesn't have a cpath_layer or not
+ * attached to a circuit. But also if the cpath_layer or circuit doesn't have a
+ * congestion control object.
+ */
+static inline const congestion_control_t *
 edge_get_ccontrol(const edge_connection_t *edge)
 {
   if (edge->cpath_layer)
@@ -64,6 +70,13 @@ edge_get_ccontrol(const edge_connection_t *edge)
     return NULL;
 }
 
+/**
+ * Update global congestion control related consensus parameter values, every
+ * consensus update.
+ *
+ * More details for each of the parameters can be found in proposal 324,
+ * section 6.5 including tuning notes.
+ */
 void
 flow_control_new_consensus_params(const networkstatus_t *ns)
 {