February 2022 - tor-commits - lists.torproject.org

[tor/main] Properly compute the number or recv cells from deliver_window
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 86f81abe3043f273d3aa7166c72d100484af9d73 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Wed Dec 1 23:20:17 2021 +0000 Properly compute the number or recv cells from deliver_window Without this conversion, there is an implict 1000-recv_cells, which causes the mod to fail if it is not a factor of 1000. --- src/core/or/sendme.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c index ee670f9d51..9acef1cc20 100644 --- a/src/core/or/sendme.c +++ b/src/core/or/sendme.c @@ -339,7 +339,7 @@ record_cell_digest_on_circ(circuit_t *circ, const uint8_t *sendme_digest) * updated once the cell is actually put in the outbuf. */ static bool -circuit_sendme_cell_is_next(int window, int sendme_inc) +circuit_sendme_cell_is_next(int deliver_window, int sendme_inc) { /* Are we at the limit of the increment and if not, we don't expect next * cell is a SENDME. @@ -348,8 +348,13 @@ circuit_sendme_cell_is_next(int window, int sendme_inc) * next cell is a SENDME, the window (either package or deliver) hasn't been * decremented just yet so when this is called, we are currently processing * the "window - 1" cell. + * + * Because deliver_window starts at CIRCWINDOW_START and counts down, + * to get the actual number of received cells for this check, we must + * first convert to receieved cells, or the modulus operator will fail. */ - if (((window - 1) % sendme_inc) != 0) { + tor_assert(deliver_window <= CIRCWINDOW_START); + if (((CIRCWINDOW_START - (deliver_window - 1)) % sendme_inc) != 0) { return false; }

1 0

[tor/main] Increase RTT ratio used to detect monotime jumps/stalls.
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 8052d0c2c08ac227aaee1134a52d5a3993ba4b10 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Fri Jan 21 18:09:48 2022 +0000 Increase RTT ratio used to detect monotime jumps/stalls. In Shadow, we saw RTT jumps as high as 1000 naturally. So let's set this to 5000, to give us some breathing room. --- src/core/or/congestion_control_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/or/congestion_control_common.c b/src/core/or/congestion_control_common.c index 6fa3bb9a11..fe3228262a 100644 --- a/src/core/or/congestion_control_common.c +++ b/src/core/or/congestion_control_common.c @@ -698,7 +698,7 @@ static bool time_delta_stalled_or_jumped(const congestion_control_t *cc, uint64_t old_delta, uint64_t new_delta) { -#define DELTA_DISCREPENCY_RATIO_MAX 100 +#define DELTA_DISCREPENCY_RATIO_MAX 5000 /* If we have a 0 new_delta, that is definitely a monotime stall */ if (new_delta == 0) { static ratelim_t stall_info_limit = RATELIM_INIT(60);

1 0

[tor/main] Fix NULL pointer deref in logs
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 338d00ba924a3884900bac425b35b6a2320da306 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Wed Oct 13 21:06:33 2021 +0000 Fix NULL pointer deref in logs --- src/core/or/congestion_control_common.c | 5 ++--- src/core/or/congestion_control_nola.c | 3 ++- src/core/or/congestion_control_vegas.c | 3 ++- src/core/or/congestion_control_westwood.c | 3 ++- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/core/or/congestion_control_common.c b/src/core/or/congestion_control_common.c index e316b631d1..6fa3bb9a11 100644 --- a/src/core/or/congestion_control_common.c +++ b/src/core/or/congestion_control_common.c @@ -1040,9 +1040,8 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc, "%"PRIu64", " "%"PRIu64", " "%"PRIu64". ", - // XXX: actually, is this p_chan here? This is - // an or_circuit (exit or onion) - circ->n_chan->global_identifier, circ->n_circ_id, + 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, diff --git a/src/core/or/congestion_control_nola.c b/src/core/or/congestion_control_nola.c index 09f88d4699..52d41157a2 100644 --- a/src/core/or/congestion_control_nola.c +++ b/src/core/or/congestion_control_nola.c @@ -111,7 +111,8 @@ congestion_control_nola_process_sendme(congestion_control_t *cc, "INFL: %"PRIu64", " "NCCE: %"PRIu64", " "SS: %d", - circ->n_chan->global_identifier, circ->n_circ_id, + CONST_TO_OR_CIRCUIT(circ)->p_chan->global_identifier, + CONST_TO_OR_CIRCUIT(circ)->p_circ_id, cc->cwnd, cc->inflight, cc->next_cc_event, diff --git a/src/core/or/congestion_control_vegas.c b/src/core/or/congestion_control_vegas.c index e7ed838478..d823a5068e 100644 --- a/src/core/or/congestion_control_vegas.c +++ b/src/core/or/congestion_control_vegas.c @@ -255,7 +255,8 @@ congestion_control_vegas_process_sendme(congestion_control_t *cc, "QUSE: %"PRIu64", " "NCCE: %"PRIu64", " "SS: %d", - circ->n_chan->global_identifier, circ->n_circ_id, + CONST_TO_OR_CIRCUIT(circ)->p_chan->global_identifier, + CONST_TO_OR_CIRCUIT(circ)->p_circ_id, cc->cwnd, cc->inflight, vegas_bdp_mix(cc), diff --git a/src/core/or/congestion_control_westwood.c b/src/core/or/congestion_control_westwood.c index 4b24234212..357cdeb3b9 100644 --- a/src/core/or/congestion_control_westwood.c +++ b/src/core/or/congestion_control_westwood.c @@ -213,7 +213,8 @@ congestion_control_westwood_process_sendme(congestion_control_t *cc, "WRTT: %"PRIu64", " "WSIG: %"PRIu64", " "SS: %d", - circ->n_chan->global_identifier, circ->n_circ_id, + CONST_TO_OR_CIRCUIT(circ)->p_chan->global_identifier, + CONST_TO_OR_CIRCUIT(circ)->p_circ_id, cc->cwnd, cc->inflight, cc->next_cc_event,

1 0

[tor/main] cc: Change edge_get_ccontrol() to look at both cpath and on_circuit
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit d4cf3fadec94169e4610717beaf4597456cbd961 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jan 12 11:49:04 2022 -0500 cc: Change edge_get_ccontrol() to look at both cpath and on_circuit Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/core/or/congestion_control_flow.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/core/or/congestion_control_flow.c b/src/core/or/congestion_control_flow.c index d61da73627..c8b5ba2473 100644 --- a/src/core/or/congestion_control_flow.c +++ b/src/core/or/congestion_control_flow.c @@ -62,12 +62,15 @@ static uint32_t xon_rate_bytes; static inline const congestion_control_t * edge_get_ccontrol(const edge_connection_t *edge) { - if (edge->cpath_layer) - return edge->cpath_layer->ccontrol; - else if (edge->on_circuit) - return edge->on_circuit->ccontrol; - else - return NULL; + congestion_control_t *ccontrol = NULL; + + if (edge->on_circuit && edge->on_circuit->ccontrol) { + ccontrol = edge->on_circuit->ccontrol; + } else if (edge->cpath_layer && edge->cpath_layer->ccontrol) { + ccontrol = edge->cpath_layer->ccontrol; + } + + return ccontrol; } /**

1 0

[tor/main] hs: Setup congestion control on service rends using intro data
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 6f45ad3771880b9f03bcc0190dce4959df211005 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Feb 3 21:18:52 2022 +0000 hs: Setup congestion control on service rends using intro data Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/feature/hs/hs_circuit.c | 18 ++++++++++++++++++ src/feature/hs/hs_common.c | 13 +------------ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 10a6f51eb3..f2953cfb02 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -17,6 +17,8 @@ #include "core/or/relay.h" #include "core/or/crypt_path.h" #include "core/or/extendinfo.h" +#include "core/or/congestion_control_common.h" +#include "core/crypto/onion_crypto.h" #include "feature/client/circpathbias.h" #include "feature/hs/hs_cell.h" #include "feature/hs/hs_circuit.h" @@ -408,6 +410,14 @@ launch_rendezvous_point_circuit,(const hs_service_t *service, tor_assert(circ->hs_ident); } + if (data->cc_enabled) { + circuit_params_t circ_params = { + .cc_enabled = data->cc_enabled, + .sendme_inc_cells = congestion_control_sendme_inc(), + }; + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params); + } + end: extend_info_free(info); } @@ -504,6 +514,14 @@ retry_service_rendezvous_point(const origin_circuit_t *circ) new_circ->build_state->expiry_time = bstate->expiry_time; new_circ->hs_ident = hs_ident_circuit_dup(circ->hs_ident); + if (TO_CIRCUIT(circ)->ccontrol != NULL) { + circuit_params_t circ_params = { + .cc_enabled = 1, + .sendme_inc_cells = TO_CIRCUIT(circ)->ccontrol->sendme_inc, + }; + TO_CIRCUIT(new_circ)->ccontrol = congestion_control_new(&circ_params); + } + done: return; } diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index 3036ce2710..e326581dd1 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -1687,18 +1687,7 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, /* We do have everything for which we think we can connect successfully. */ info = extend_info_new(NULL, legacy_id, (have_ed25519_id) ? &ed25519_pk : NULL, NULL, - onion_key, &ap.addr, ap.port, - /* TODO-324: oh wow, this is a hard one. - - The protover summary here needs to explain - if we support the newer congestion control or - not. This may require new specification changes. - - Probably there is some analogous service-side - function that needs to initialize congestion - control structures based on what the client says. - */ - NULL, false); + onion_key, &ap.addr, ap.port, NULL, false); done: return info; }

1 0

[tor/main] Set new defaults for congestion control parameters.
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 0a6cde8756423336c8e6901b55c31d67a2a35245 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Fri Jan 21 15:17:20 2022 +0000 Set new defaults for congestion control parameters. Defaults determined from Shadow experimentation. More parameter functionality changes to follow. --- src/core/or/congestion_control_common.c | 20 +++++++++++--------- src/core/or/congestion_control_common.h | 4 ++++ src/core/or/congestion_control_flow.c | 2 +- src/core/or/congestion_control_vegas.c | 10 ++++++---- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/core/or/congestion_control_common.c b/src/core/or/congestion_control_common.c index 6d4f34cff8..09c6c04bf3 100644 --- a/src/core/or/congestion_control_common.c +++ b/src/core/or/congestion_control_common.c @@ -35,16 +35,18 @@ * * 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 SENDME_INC_DFLT (TLS_RECORD_MAX_CELLS) +#define CIRCWINDOW_INIT (4*SENDME_INC_DFLT) + #define CC_ALG_DFLT (CC_ALG_SENDME) #define CC_ALG_DFLT_ALWAYS (CC_ALG_VEGAS) -#define CWND_INC_DFLT (50) -#define CWND_INC_PCT_SS_DFLT (100) +#define CWND_INC_DFLT (TLS_RECORD_MAX_CELLS) +#define CWND_INC_PCT_SS_DFLT (50) #define CWND_INC_RATE_DFLT (1) + +#define CWND_MIN_DFLT (SENDME_INC_DFLT) #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) @@ -138,8 +140,8 @@ congestion_control_new_consensus_params(const networkstatus_t *ns) CWND_MAX_MIN, CWND_MAX_MAX); -#define SENDME_INC_MIN 10 -#define SENDME_INC_MAX (1000) +#define SENDME_INC_MIN 1 +#define SENDME_INC_MAX (255) cc_sendme_inc = networkstatus_get_param(NULL, "cc_sendme_inc", SENDME_INC_DFLT, @@ -171,7 +173,7 @@ congestion_control_init_params(congestion_control_t *cc, const or_options_t *opts = get_options(); cc->sendme_inc = params->sendme_inc_cells; -#define CWND_INIT_MIN 100 +#define CWND_INIT_MIN SENDME_INC_DFLT #define CWND_INIT_MAX (10000) cc->cwnd = networkstatus_get_param(NULL, "cc_cwnd_init", @@ -203,7 +205,7 @@ congestion_control_init_params(congestion_control_t *cc, CWND_INC_RATE_MIN, CWND_INC_RATE_MAX); -#define CWND_MIN_MIN 20 +#define CWND_MIN_MIN SENDME_INC_DFLT #define CWND_MIN_MAX (1000) cc->cwnd_min = networkstatus_get_param(NULL, "cc_cwnd_min", diff --git a/src/core/or/congestion_control_common.h b/src/core/or/congestion_control_common.h index 936cb5887c..1e5a00c942 100644 --- a/src/core/or/congestion_control_common.h +++ b/src/core/or/congestion_control_common.h @@ -13,6 +13,10 @@ #include "core/or/crypt_path_st.h" #include "core/or/circuit_st.h" +/* The maximum whole number of cells that can fit in a + * full TLS record. This is 31. */ +#define TLS_RECORD_MAX_CELLS ((16 * 1024) / CELL_MAX_NETWORK_SIZE) + typedef struct congestion_control_t congestion_control_t; /** Wrapper for the free function, set the CC pointer to NULL after free */ diff --git a/src/core/or/congestion_control_flow.c b/src/core/or/congestion_control_flow.c index c8b5ba2473..3a3a9522fd 100644 --- a/src/core/or/congestion_control_flow.c +++ b/src/core/or/congestion_control_flow.c @@ -116,7 +116,7 @@ flow_control_new_consensus_params(const networkstatus_t *ns) CC_XON_RATE_BYTES_MAX)*RELAY_PAYLOAD_SIZE; #define CC_XON_EWMA_CNT_DFLT (2) -#define CC_XON_EWMA_CNT_MIN (1) +#define CC_XON_EWMA_CNT_MIN (2) #define CC_XON_EWMA_CNT_MAX (100) xon_ewma_cnt = networkstatus_get_param(ns, "cc_xon_ewma_cnt", CC_XON_EWMA_CNT_DFLT, diff --git a/src/core/or/congestion_control_vegas.c b/src/core/or/congestion_control_vegas.c index 3206821f4c..8e13499aff 100644 --- a/src/core/or/congestion_control_vegas.c +++ b/src/core/or/congestion_control_vegas.c @@ -23,11 +23,13 @@ #include "core/or/channel.h" #include "feature/nodelist/networkstatus.h" -#define VEGAS_GAMMA(cc) (6*(cc)->sendme_inc) -#define VEGAS_ALPHA(cc) (3*(cc)->sendme_inc) -#define VEGAS_BETA(cc) (6*(cc)->sendme_inc) +#define OUTBUF_CELLS (2*TLS_RECORD_MAX_CELLS) -#define VEGAS_BDP_MIX_PCT 0 +#define VEGAS_ALPHA(cc) (3*OUTBUF_CELLS-TLS_RECORD_MAX_CELLS) +#define VEGAS_BETA(cc) (3*OUTBUF_CELLS) +#define VEGAS_GAMMA(cc) (3*OUTBUF_CELLS) + +#define VEGAS_BDP_MIX_PCT 100 /** * The original TCP Vegas used only a congestion window BDP estimator. We

1 0

[tor/main] Guard against 0 time delta in BDP calc.
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 4f3a0e39cf18f3d44dd2f4b469edd7055287a478 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Fri Jan 21 18:40:49 2022 +0000 Guard against 0 time delta in BDP calc. This can only happen in Shadow, but it will cause issues there. --- src/core/or/congestion_control_common.c | 35 +++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/core/or/congestion_control_common.c b/src/core/or/congestion_control_common.c index fe3228262a..4b483a90b0 100644 --- a/src/core/or/congestion_control_common.c +++ b/src/core/or/congestion_control_common.c @@ -937,21 +937,26 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc, timestamp_usec = peek_timestamp(cc->sendme_arrival_timestamps); uint64_t delta = now_usec - timestamp_usec; - /* 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], - cc->ewma_cwnd_cnt*sendme_acks_per_cwnd(cc)); + /* 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], + cc->ewma_cwnd_cnt*sendme_acks_per_cwnd(cc)); + } } /* In-flight BDP will cause the cwnd to drift down when underutilized.

1 0

[tor/main] Relocate two parameters from circuit scope to global scope.
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 3079e2cacc43b17a3a2074a64153cc0f329b5151 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Fri Jan 21 19:47:39 2022 +0000 Relocate two parameters from circuit scope to global scope. Changes in these will not affect in-progress transfers. --- src/core/or/congestion_control_common.c | 51 ++++++++++++++++++++------------- src/core/or/congestion_control_st.h | 10 ------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/core/or/congestion_control_common.c b/src/core/or/congestion_control_common.c index 4b483a90b0..93d3e54fcd 100644 --- a/src/core/or/congestion_control_common.c +++ b/src/core/or/congestion_control_common.c @@ -95,6 +95,16 @@ 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; +/** + * Number of cwnd worth of sendme acks to smooth RTT and BDP with, + * using N_EWMA */ +static uint8_t ewma_cwnd_cnt; + +/** + * Minimum number of sendmes before we begin BDP estimates + */ +static uint8_t bwe_sendme_min; + /** * Update global congestion control related consensus parameter values, * every consensus update. @@ -155,6 +165,23 @@ congestion_control_new_consensus_params(const networkstatus_t *ns) CC_ALG_DFLT, CC_ALG_MIN, CC_ALG_MAX); + +#define BWE_SENDME_MIN_MIN 2 +#define BWE_SENDME_MIN_MAX (20) + bwe_sendme_min = + networkstatus_get_param(NULL, "cc_bwe_min", + BWE_SENDME_MIN_DFLT, + BWE_SENDME_MIN_MIN, + BWE_SENDME_MIN_MAX); + +#define EWMA_CWND_COUNT_MIN 1 +#define EWMA_CWND_COUNT_MAX (100) + ewma_cwnd_cnt = + networkstatus_get_param(NULL, "cc_ewma_cwnd_cnt", + EWMA_CWND_COUNT_DFLT, + EWMA_CWND_COUNT_MIN, + EWMA_CWND_COUNT_MAX); + } /** @@ -214,22 +241,6 @@ congestion_control_init_params(congestion_control_t *cc, CWND_MIN_MIN, CWND_MIN_MAX); -#define EWMA_CWND_COUNT_MIN 1 -#define EWMA_CWND_COUNT_MAX (100) - cc->ewma_cwnd_cnt = - networkstatus_get_param(NULL, "cc_ewma_cwnd_cnt", - EWMA_CWND_COUNT_DFLT, - EWMA_CWND_COUNT_MIN, - EWMA_CWND_COUNT_MAX); - -#define BWE_SENDME_MIN_MIN 2 -#define BWE_SENDME_MIN_MAX (20) - cc->bwe_sendme_min = - networkstatus_get_param(NULL, "cc_bwe_min", - BWE_SENDME_MIN_DFLT, - BWE_SENDME_MIN_MIN, - BWE_SENDME_MIN_MAX); - /* If the consensus says to use OG sendme, but torrc has * always-enabled, use the default "always" alg (vegas), * else use cached conensus alg. */ @@ -804,7 +815,7 @@ congestion_control_update_circuit_rtt(congestion_control_t *cc, return 0; } - ewma_cnt = cc->ewma_cwnd_cnt*sendme_acks_per_cwnd(cc); + ewma_cnt = ewma_cwnd_cnt*sendme_acks_per_cwnd(cc); ewma_cnt = MAX(ewma_cnt, 2); // Use at least 2 cc->ewma_rtt_usec = n_count_ewma(rtt, cc->ewma_rtt_usec, ewma_cnt); @@ -921,12 +932,12 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc, */ enqueue_timestamp(cc->sendme_arrival_timestamps, now_usec); - if (smartlist_len(cc->sendme_arrival_timestamps) >= cc->bwe_sendme_min) { + 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) > - cc->bwe_sendme_min && + bwe_sendme_min && (uint64_t)smartlist_len(cc->sendme_arrival_timestamps) > sendme_acks_per_cwnd(cc)) { (void)dequeue_timestamp(cc->sendme_arrival_timestamps); @@ -955,7 +966,7 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc, /* Calculate BDP_EWMA_COUNT N-EWMA */ cc->bdp[BDP_ALG_SENDME_RATE] = n_count_ewma(sendme_rate_bdp, cc->bdp[BDP_ALG_SENDME_RATE], - cc->ewma_cwnd_cnt*sendme_acks_per_cwnd(cc)); + ewma_cwnd_cnt*sendme_acks_per_cwnd(cc)); } } diff --git a/src/core/or/congestion_control_st.h b/src/core/or/congestion_control_st.h index 6038072568..0d6bf0b662 100644 --- a/src/core/or/congestion_control_st.h +++ b/src/core/or/congestion_control_st.h @@ -177,16 +177,6 @@ struct congestion_control_t { * signals */ uint8_t cwnd_inc_rate; - /** - * Number of cwnd worth of sendme acks to smooth RTT and BDP with, - * using N_EWMA */ - uint8_t ewma_cwnd_cnt; - - /** - * Minimum number of sendmes before we begin BDP estimates - */ - uint8_t bwe_sendme_min; - /** * Number of cells to ack with every sendme. Taken from consensus parameter * and negotiation during circuit setup. */

1 0

[tor/main] Increment by at least 2 sendme_incs in slow start
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 1960bf09f3e3fb234678247f1f78d806bb9066ed Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Sat Jan 22 19:22:40 2022 +0000 Increment by at least 2 sendme_incs in slow start --- src/core/or/congestion_control_vegas.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/or/congestion_control_vegas.c b/src/core/or/congestion_control_vegas.c index d823a5068e..0ebff17faf 100644 --- a/src/core/or/congestion_control_vegas.c +++ b/src/core/or/congestion_control_vegas.c @@ -206,8 +206,8 @@ congestion_control_vegas_process_sendme(congestion_control_t *cc, if (cc->in_slow_start) { if (queue_use < cc->vegas_params.gamma && !cc->blocked_chan) { /* Grow to BDP immediately, then exponential growth until - * congestion signal */ - cc->cwnd = MAX(cc->cwnd + CWND_INC_SS(cc), + * congestion signal. Increment by at least 2 sendme's worth. */ + cc->cwnd = MAX(cc->cwnd + MAX(CWND_INC_SS(cc), 2*cc->sendme_inc), vegas_bdp_mix(cc)); } else { /* Congestion signal: Fall back to Vegas equilibrium (BDP) */

1 0

[tor/main] Make N_EWMA a percentage of CWND update rate, capped by a max value.
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 43f6f3fd3a85047fc7fc41e6ce8f924d007b98ee Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Fri Jan 21 23:00:35 2022 +0000 Make N_EWMA a percentage of CWND update rate, capped by a max value. This proved better in Shadow sims than just a multiple of CWND. --- src/core/or/congestion_control_common.c | 56 ++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/src/core/or/congestion_control_common.c b/src/core/or/congestion_control_common.c index 93d3e54fcd..fb56014315 100644 --- a/src/core/or/congestion_control_common.c +++ b/src/core/or/congestion_control_common.c @@ -49,7 +49,9 @@ #define CWND_MAX_DFLT (INT32_MAX) #define BWE_SENDME_MIN_DFLT (5) -#define EWMA_CWND_COUNT_DFLT (2) + +#define N_EWMA_CWND_PCT_DFLT (50) +#define N_EWMA_MAX_DFLT (10) /* BDP algorithms for each congestion control algorithms use the piecewise * estimattor. See section 3.1.4 of proposal 324. */ @@ -98,7 +100,12 @@ static cc_alg_t cc_alg = CC_ALG_DFLT; /** * Number of cwnd worth of sendme acks to smooth RTT and BDP with, * using N_EWMA */ -static uint8_t ewma_cwnd_cnt; +static uint8_t n_ewma_cwnd_pct; + +/** + * Maximum number N for the N-count EWMA averaging of RTT and BDP. + */ +static uint8_t n_ewma_max; /** * Minimum number of sendmes before we begin BDP estimates @@ -174,14 +181,21 @@ congestion_control_new_consensus_params(const networkstatus_t *ns) BWE_SENDME_MIN_MIN, BWE_SENDME_MIN_MAX); -#define EWMA_CWND_COUNT_MIN 1 -#define EWMA_CWND_COUNT_MAX (100) - ewma_cwnd_cnt = - networkstatus_get_param(NULL, "cc_ewma_cwnd_cnt", - EWMA_CWND_COUNT_DFLT, - EWMA_CWND_COUNT_MIN, - EWMA_CWND_COUNT_MAX); - +#define N_EWMA_CWND_PCT_MIN 1 +#define N_EWMA_CWND_PCT_MAX (255) + n_ewma_cwnd_pct = + networkstatus_get_param(NULL, "cc_ewma_cwnd_pct", + N_EWMA_CWND_PCT_DFLT, + N_EWMA_CWND_PCT_MIN, + N_EWMA_CWND_PCT_MAX); + +#define N_EWMA_MAX_MIN 2 +#define N_EWMA_MAX_MAX (INT32_MAX) + n_ewma_max = + networkstatus_get_param(NULL, "cc_ewma_max", + N_EWMA_MAX_DFLT, + N_EWMA_MAX_MIN, + N_EWMA_MAX_MAX); } /** @@ -429,14 +443,19 @@ dequeue_timestamp(smartlist_t *timestamps_u64_usecs) } /** - * Returns the number of sendme acks that will be recieved in the - * current congestion window size, rounded to nearest int. + * Returns the number N of N-count EWMA, for averaging RTT and BDP over + * N SENDME acks. + * + * This N is bracketed between a divisor of the number of acks in a CWND + * and a max value. It is always at least 2. */ static inline uint64_t -sendme_acks_per_cwnd(const congestion_control_t *cc) +n_ewma_count(const congestion_control_t *cc) { - /* We add half a sendme_inc to cwnd to round to the nearest int */ - return ((cc->cwnd + cc->sendme_inc/2)/cc->sendme_inc); + uint64_t ewma_cnt = MIN(CWND_UPDATE_RATE(cc)*n_ewma_cwnd_pct/100, + n_ewma_max); + ewma_cnt = MAX(ewma_cnt, 2); + return ewma_cnt; } /** @@ -815,8 +834,7 @@ congestion_control_update_circuit_rtt(congestion_control_t *cc, return 0; } - ewma_cnt = ewma_cwnd_cnt*sendme_acks_per_cwnd(cc); - ewma_cnt = MAX(ewma_cnt, 2); // Use at least 2 + ewma_cnt = n_ewma_count(cc); cc->ewma_rtt_usec = n_count_ewma(rtt, cc->ewma_rtt_usec, ewma_cnt); @@ -939,7 +957,7 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc, while (smartlist_len(cc->sendme_arrival_timestamps) > bwe_sendme_min && (uint64_t)smartlist_len(cc->sendme_arrival_timestamps) > - sendme_acks_per_cwnd(cc)) { + n_ewma_count(cc)) { (void)dequeue_timestamp(cc->sendme_arrival_timestamps); } int sendme_cnt = smartlist_len(cc->sendme_arrival_timestamps); @@ -966,7 +984,7 @@ congestion_control_update_circuit_bdp(congestion_control_t *cc, /* Calculate BDP_EWMA_COUNT N-EWMA */ cc->bdp[BDP_ALG_SENDME_RATE] = n_count_ewma(sendme_rate_bdp, cc->bdp[BDP_ALG_SENDME_RATE], - ewma_cwnd_cnt*sendme_acks_per_cwnd(cc)); + n_ewma_count(cc)); } }

1 0