- tor-commits - lists.torproject.org

[tor/main] hs: Encode flow control value in the descriptor
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit 95639f35aede81bfde8bd4ee1616e10048176cc7 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Nov 4 10:26:04 2021 -0400 hs: Encode flow control value in the descriptor This simply adds the "flow-control" line, as detailed in prop324, to the descriptor. No decoding is done at this commit. Part of #40506 --- src/feature/dirparse/parsecommon.h | 1 + src/feature/hs/hs_descriptor.c | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/feature/dirparse/parsecommon.h b/src/feature/dirparse/parsecommon.h index 0f343e9c62..675c5f68d5 100644 --- a/src/feature/dirparse/parsecommon.h +++ b/src/feature/dirparse/parsecommon.h @@ -172,6 +172,7 @@ typedef enum { R3_DESC_AUTH_KEY, R3_DESC_AUTH_CLIENT, R3_ENCRYPTED, + R3_FLOW_CONTROL, R_IPO_IDENTIFIER, R_IPO_IP_ADDRESS, diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c index a37eab5b5d..80273c27b1 100644 --- a/src/feature/hs/hs_descriptor.c +++ b/src/feature/hs/hs_descriptor.c @@ -61,6 +61,8 @@ #include "trunnel/ed25519_cert.h" /* Trunnel interface. */ #include "feature/hs/hs_descriptor.h" #include "core/or/circuitbuild.h" +#include "core/or/congestion_control_common.h" +#include "core/or/protover.h" #include "lib/crypt_ops/crypto_rand.h" #include "lib/crypt_ops/crypto_util.h" #include "feature/dirparse/parsecommon.h" @@ -69,6 +71,7 @@ #include "feature/nodelist/torcert.h" /* tor_cert_encode_ed22519() */ #include "lib/memarea/memarea.h" #include "lib/crypt_ops/crypto_format.h" +#include "core/or/versions.h" #include "core/or/extend_info_st.h" @@ -92,6 +95,7 @@ #define str_ip_legacy_key "legacy-key" #define str_ip_legacy_key_cert "legacy-key-cert" #define str_intro_point_start "\n" str_intro_point " " +#define str_flow_control "flow-control" /* Constant string value for the construction to encrypt the encrypted data * section. */ #define str_enc_const_superencryption "hsdir-superencrypted-data" @@ -138,6 +142,7 @@ static token_rule_t hs_desc_encrypted_v3_token_table[] = { T1_START(str_create2_formats, R3_CREATE2_FORMATS, CONCAT_ARGS, NO_OBJ), T01(str_intro_auth_required, R3_INTRO_AUTH_REQUIRED, GE(1), NO_OBJ), T01(str_single_onion, R3_SINGLE_ONION_SERVICE, ARGS, NO_OBJ), + T01(str_flow_control, R3_FLOW_CONTROL, GE(2), NO_OBJ), END_OF_TABLE }; @@ -765,6 +770,13 @@ get_inner_encrypted_layer_plaintext(const hs_descriptor_t *desc) if (desc->encrypted_data.single_onion_service) { smartlist_add_asprintf(lines, "%s\n", str_single_onion); } + + if (congestion_control_enabled()) { + /* Add flow control line into the descriptor. */ + smartlist_add_asprintf(lines, "%s %s %u\n", str_flow_control, + protover_get_supported(PRT_FLOWCTRL), + congestion_control_sendme_inc()); + } } /* Build the introduction point(s) section. */

1 0

[tor/main] Use path type hint for Vegas queue parameters.
by dgoulet＠torproject.org 22 Feb '22

22 Feb '22

commit b2553bfba28b2e26f09041a3d78fa39c35fd4ac8 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Thu Feb 3 12:01:23 2022 +0000 Use path type hint for Vegas queue parameters. These parameters will vary depending on path length, especially for onions. --- src/app/config/config.c | 1 + src/app/config/or_options_st.h | 3 + src/core/mainloop/cpuworker.c | 8 ++- src/core/or/circuitbuild.c | 16 +++++- src/core/or/congestion_control_common.c | 14 +++-- src/core/or/congestion_control_common.h | 18 +++++- src/core/or/congestion_control_vegas.c | 97 ++++++++++++++++++++++++++++----- src/core/or/congestion_control_vegas.h | 3 +- src/feature/hs/hs_circuit.c | 26 ++++++++- src/feature/hs/hs_client.c | 14 ++++- 10 files changed, 174 insertions(+), 26 deletions(-) diff --git a/src/app/config/config.c b/src/app/config/config.c index 05bd96fc6a..15addd5be4 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -676,6 +676,7 @@ static const config_var_t option_vars_[] = { V(UseMicrodescriptors, AUTOBOOL, "auto"), OBSOLETE("UseNTorHandshake"), VAR("__AlwaysCongestionControl", BOOL, AlwaysCongestionControl, "0"), + VAR("__SbwsExit", BOOL, SbwsExit, "0"), V_IMMUTABLE(User, STRING, NULL), OBSOLETE("UserspaceIOCPBuffers"), OBSOLETE("V1AuthoritativeDirectory"), diff --git a/src/app/config/or_options_st.h b/src/app/config/or_options_st.h index a1ef7a8cf8..290a2bb9b4 100644 --- a/src/app/config/or_options_st.h +++ b/src/app/config/or_options_st.h @@ -604,6 +604,9 @@ struct or_options_t { /** Boolean: Switch to override consensus to enable congestion control */ int AlwaysCongestionControl; + /** Boolean: Switch to specify this is an sbws measurement exit */ + int SbwsExit; + int RephistTrackTime; /**< How many seconds do we keep rephist info? */ /** Should we always fetch our dir info on the mirror schedule (which * means directly from the authorities) no matter our other config? */ diff --git a/src/core/mainloop/cpuworker.c b/src/core/mainloop/cpuworker.c index 2cb667615d..ab970259b5 100644 --- a/src/core/mainloop/cpuworker.c +++ b/src/core/mainloop/cpuworker.c @@ -391,7 +391,13 @@ cpuworker_onion_handshake_replyfn(void *work_) /* If the client asked for congestion control, if our consensus parameter * allowed it to negotiate as enabled, allocate a congestion control obj. */ if (rpl.circ_params.cc_enabled) { - TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&rpl.circ_params); + if (get_options()->SbwsExit) { + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&rpl.circ_params, + CC_PATH_SBWS); + } else { + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&rpl.circ_params, + CC_PATH_EXIT); + } } if (onionskin_answer(circ, diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index 2326dc2a6d..f62a1d93f5 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -1275,7 +1275,21 @@ circuit_finish_handshake(origin_circuit_t *circ, } if (params.cc_enabled) { - hop->ccontrol = congestion_control_new(&params); + int circ_len = circuit_get_cpath_len(circ); + + if (circ_len == DEFAULT_ROUTE_LEN && + circuit_get_cpath_hop(circ, DEFAULT_ROUTE_LEN) == hop) { + hop->ccontrol = congestion_control_new(&params, CC_PATH_EXIT); + } else if (circ_len == SBWS_ROUTE_LEN && + circuit_get_cpath_hop(circ, SBWS_ROUTE_LEN) == hop) { + hop->ccontrol = congestion_control_new(&params, CC_PATH_SBWS); + } else { + static ratelim_t cc_path_limit = RATELIM_INIT(600); + log_fn_ratelim(&cc_path_limit, LOG_WARN, LD_CIRC, + "Unexpected path length %d for circuit", + circ_len); + hop->ccontrol = congestion_control_new(&params, CC_PATH_EXIT); + } } hop->state = CPATH_STATE_OPEN; diff --git a/src/core/or/congestion_control_common.c b/src/core/or/congestion_control_common.c index 09c6c04bf3..e316b631d1 100644 --- a/src/core/or/congestion_control_common.c +++ b/src/core/or/congestion_control_common.c @@ -168,7 +168,8 @@ congestion_control_new_consensus_params(const networkstatus_t *ns) */ static void congestion_control_init_params(congestion_control_t *cc, - const circuit_params_t *params) + const circuit_params_t *params, + cc_path_t path) { const or_options_t *opts = get_options(); cc->sendme_inc = params->sendme_inc_cells; @@ -266,7 +267,7 @@ congestion_control_init_params(congestion_control_t *cc, if (cc->cc_alg == CC_ALG_WESTWOOD) { congestion_control_westwood_set_params(cc); } else if (cc->cc_alg == CC_ALG_VEGAS) { - congestion_control_vegas_set_params(cc); + congestion_control_vegas_set_params(cc, path); } else if (cc->cc_alg == CC_ALG_NOLA) { congestion_control_nola_set_params(cc); } @@ -326,24 +327,25 @@ congestion_control_set_cc_enabled(void) */ static void congestion_control_init(congestion_control_t *cc, - const circuit_params_t *params) + const circuit_params_t *params, + 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); + congestion_control_init_params(cc, params, path); cc->next_cc_event = CWND_UPDATE_RATE(cc); } /** Allocate and initialize a new congestion control object */ congestion_control_t * -congestion_control_new(const circuit_params_t *params) +congestion_control_new(const circuit_params_t *params, cc_path_t path) { congestion_control_t *cc = tor_malloc_zero(sizeof(congestion_control_t)); - congestion_control_init(cc, params); + congestion_control_init(cc, params, path); return cc; } diff --git a/src/core/or/congestion_control_common.h b/src/core/or/congestion_control_common.h index 1e5a00c942..1a57d71331 100644 --- a/src/core/or/congestion_control_common.h +++ b/src/core/or/congestion_control_common.h @@ -19,6 +19,21 @@ typedef struct congestion_control_t congestion_control_t; +/** + * Specifies the path type to help choose congestion control + * parameters. Since these paths are different lengths, they + * will need different queue parameters. */ +typedef enum { + CC_PATH_EXIT = 0, + CC_PATH_ONION = 1, + CC_PATH_ONION_SOS = 2, + CC_PATH_ONION_VG = 3, + CC_PATH_SBWS = 4, +} cc_path_t; + +/** The length of a path for sbws measurement */ +#define SBWS_ROUTE_LEN 2 + /** Wrapper for the free function, set the CC pointer to NULL after free */ #define congestion_control_free(cc) \ FREE_AND_NULL(congestion_control_t, congestion_control_free_, cc) @@ -27,7 +42,8 @@ void congestion_control_free_(congestion_control_t *cc); struct circuit_params_t; congestion_control_t *congestion_control_new( - const struct circuit_params_t *params); + const struct circuit_params_t *params, + cc_path_t path); int congestion_control_dispatch_cc_alg(congestion_control_t *cc, const circuit_t *circ, diff --git a/src/core/or/congestion_control_vegas.c b/src/core/or/congestion_control_vegas.c index 8e13499aff..e7ed838478 100644 --- a/src/core/or/congestion_control_vegas.c +++ b/src/core/or/congestion_control_vegas.c @@ -25,9 +25,31 @@ #define OUTBUF_CELLS (2*TLS_RECORD_MAX_CELLS) -#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) +/* sbws circs are two hops, so params are based on 2 outbufs of cells */ +#define VEGAS_ALPHA_SBWS_DFLT (2*OUTBUF_CELLS-TLS_RECORD_MAX_CELLS) +#define VEGAS_BETA_SBWS_DFLT (2*OUTBUF_CELLS) +#define VEGAS_GAMMA_SBWS_DFLT (2*OUTBUF_CELLS) + +/* Exits are three hops, so params are based on 3 outbufs of cells */ +#define VEGAS_ALPHA_EXIT_DFLT (3*OUTBUF_CELLS-TLS_RECORD_MAX_CELLS) +#define VEGAS_BETA_EXIT_DFLT (3*OUTBUF_CELLS) +#define VEGAS_GAMMA_EXIT_DFLT (3*OUTBUF_CELLS) + +/* Onion rends are six hops, so params are based on 6 outbufs of cells */ +#define VEGAS_ALPHA_ONION_DFLT (6*OUTBUF_CELLS-TLS_RECORD_MAX_CELLS) +#define VEGAS_BETA_ONION_DFLT (6*OUTBUF_CELLS) +#define VEGAS_GAMMA_ONION_DFLT (6*OUTBUF_CELLS) + +/* Single Onions are three hops, so params are based on 3 outbufs of cells */ +#define VEGAS_ALPHA_SOS_DFLT (3*OUTBUF_CELLS-TLS_RECORD_MAX_CELLS) +#define VEGAS_BETA_SOS_DFLT (3*OUTBUF_CELLS) +#define VEGAS_GAMMA_SOS_DFLT (3*OUTBUF_CELLS) + +/* Vanguard Onions are 7 hops (or 8 if both sides use vanguards, but that + * should be rare), so params are based on 7 outbufs of cells */ +#define VEGAS_ALPHA_VG_DFLT (7*OUTBUF_CELLS-TLS_RECORD_MAX_CELLS) +#define VEGAS_BETA_VG_DFLT (7*OUTBUF_CELLS) +#define VEGAS_GAMMA_VG_DFLT (7*OUTBUF_CELLS) #define VEGAS_BDP_MIX_PCT 100 @@ -54,25 +76,74 @@ vegas_bdp_mix(const congestion_control_t *cc) * Cache Vegas consensus parameters. */ void -congestion_control_vegas_set_params(congestion_control_t *cc) +congestion_control_vegas_set_params(congestion_control_t *cc, + cc_path_t path) { tor_assert(cc->cc_alg == CC_ALG_VEGAS); + const char *alpha_str = NULL, *beta_str = NULL, *gamma_str = NULL; + int alpha, beta, gamma; - cc->vegas_params.gamma = - networkstatus_get_param(NULL, "cc_vegas_gamma", - VEGAS_GAMMA(cc), - 0, - 1000); + switch (path) { + case CC_PATH_SBWS: + alpha_str = "cc_vegas_alpha_sbws"; + beta_str = "cc_vegas_beta_sbws"; + gamma_str = "cc_vegas_gamma_sbws"; + alpha = VEGAS_ALPHA_SBWS_DFLT; + beta = VEGAS_BETA_SBWS_DFLT; + gamma = VEGAS_GAMMA_SBWS_DFLT; + break; + case CC_PATH_EXIT: + alpha_str = "cc_vegas_alpha_exit"; + beta_str = "cc_vegas_beta_exit"; + gamma_str = "cc_vegas_gamma_exit"; + alpha = VEGAS_ALPHA_EXIT_DFLT; + beta = VEGAS_BETA_EXIT_DFLT; + gamma = VEGAS_GAMMA_EXIT_DFLT; + break; + case CC_PATH_ONION: + alpha_str = "cc_vegas_alpha_onion"; + beta_str = "cc_vegas_beta_onion"; + gamma_str = "cc_vegas_gamma_onion"; + alpha = VEGAS_ALPHA_ONION_DFLT; + beta = VEGAS_BETA_ONION_DFLT; + gamma = VEGAS_GAMMA_ONION_DFLT; + break; + case CC_PATH_ONION_SOS: + alpha_str = "cc_vegas_alpha_sos"; + beta_str = "cc_vegas_beta_sos"; + gamma_str = "cc_vegas_gamma_sos"; + alpha = VEGAS_ALPHA_SOS_DFLT; + beta = VEGAS_BETA_SOS_DFLT; + gamma = VEGAS_GAMMA_SOS_DFLT; + break; + case CC_PATH_ONION_VG: + alpha_str = "cc_vegas_alpha_vg"; + beta_str = "cc_vegas_beta_vg"; + gamma_str = "cc_vegas_gamma_vg"; + alpha = VEGAS_ALPHA_VG_DFLT; + beta = VEGAS_BETA_VG_DFLT; + gamma = VEGAS_GAMMA_VG_DFLT; + break; + default: + tor_assert(0); + break; + } cc->vegas_params.alpha = - networkstatus_get_param(NULL, "cc_vegas_alpha", - VEGAS_ALPHA(cc), + networkstatus_get_param(NULL, alpha_str, + alpha, 0, 1000); cc->vegas_params.beta = - networkstatus_get_param(NULL, "cc_vegas_beta", - VEGAS_BETA(cc), + networkstatus_get_param(NULL, beta_str, + beta, + 0, + 1000); + + cc->vegas_params.gamma = + networkstatus_get_param(NULL, gamma_str, + gamma, 0, 1000); diff --git a/src/core/or/congestion_control_vegas.h b/src/core/or/congestion_control_vegas.h index 111345081c..95fcea5722 100644 --- a/src/core/or/congestion_control_vegas.h +++ b/src/core/or/congestion_control_vegas.h @@ -16,7 +16,8 @@ int congestion_control_vegas_process_sendme(struct congestion_control_t *cc, const circuit_t *circ, const crypt_path_t *layer_hint); -void congestion_control_vegas_set_params(struct congestion_control_t *cc); +void congestion_control_vegas_set_params(struct congestion_control_t *cc, + cc_path_t path); /* Private section starts. */ #ifdef TOR_CONGESTION_CONTROL_VEGAS_PRIVATE diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index f2953cfb02..271bf652e7 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -415,7 +415,20 @@ launch_rendezvous_point_circuit,(const hs_service_t *service, .cc_enabled = data->cc_enabled, .sendme_inc_cells = congestion_control_sendme_inc(), }; - TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params); + + /* Initialize ccontrol for appropriate path type */ + if (service->config.is_single_onion) { + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, + CC_PATH_ONION_SOS); + } else { + if (get_options()->HSLayer3Nodes) { + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, + CC_PATH_ONION_VG); + } else { + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, + CC_PATH_ONION); + } + } } end: @@ -519,7 +532,16 @@ retry_service_rendezvous_point(const origin_circuit_t *circ) .cc_enabled = 1, .sendme_inc_cells = TO_CIRCUIT(circ)->ccontrol->sendme_inc, }; - TO_CIRCUIT(new_circ)->ccontrol = congestion_control_new(&circ_params); + + /* As per above, in this case, we are a full 3 hop rend, even if we're a + * single-onion service */ + if (get_options()->HSLayer3Nodes) { + TO_CIRCUIT(new_circ)->ccontrol = congestion_control_new(&circ_params, + CC_PATH_ONION_VG); + } else { + TO_CIRCUIT(new_circ)->ccontrol = congestion_control_new(&circ_params, + CC_PATH_ONION_SOS); + } } done: diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 69b071e197..81c0459a86 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -788,7 +788,19 @@ setup_rendezvous_circ_congestion_control(origin_circuit_t *circ) circ_params.cc_enabled = congestion_control_enabled(); if (circ_params.cc_enabled) { circ_params.sendme_inc_cells = desc->encrypted_data.sendme_inc; - TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params); + + if (desc->encrypted_data.single_onion_service) { + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, + CC_PATH_ONION_SOS); + } else { + if (get_options()->HSLayer3Nodes) { + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, + CC_PATH_ONION_VG); + } else { + TO_CIRCUIT(circ)->ccontrol = congestion_control_new(&circ_params, + CC_PATH_ONION); + } + } } }

1 0

[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