This is an automated email from the git hooks/post-receive script. dgoulet pushed a change to branch main in repository tor. from d5306e107f Merge branch 'tor-gitlab/mr/715' new 03d63bc7bd Add a conflux helper to log conflux sets. new 176f0929bb Add conflux logs to diagnose cases where RTTs are absent/zero. new ff59e2f490 Add BUG() macro to marked edge reads new da50d21c42 Bug 40801: Send LINKED_ACK before attaching streams new 6a513e2ff5 Bug 40801: Do not change read state of marked conns new 0149c1ff98 Bug 40801: Add changes file new dbd37c0e7b Bug 40810: Improve validation checks to ignore 0-RTT legs new 5d63842e86 Bug 40810: Avoid using 0 RTT legs new 7ffda7512d Changes file for bug40810 new 44368a727a Merge branch 'tor-gitlab/mr/721' The 10 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: changes/bug40801 | 10 +++++ changes/bug40810 | 4 ++ src/core/or/conflux.c | 21 ++++++--- src/core/or/conflux_pool.c | 106 ++++++++++++++++++++++++++++++++++++++++----- src/core/or/conflux_pool.h | 3 ++ src/core/or/conflux_util.c | 42 +++++++++++++----- src/core/or/relay.c | 8 ++-- 7 files changed, 162 insertions(+), 32 deletions(-) create mode 100644 changes/bug40801 create mode 100644 changes/bug40810 -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit 03d63bc7bdd6913a365b15bbd5ad1da201a8f6f0 Author: Mike Perry <mikeperry-git@torproject.org> AuthorDate: Tue Jun 6 15:14:33 2023 +0000 Add a conflux helper to log conflux sets. --- src/core/or/conflux_pool.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/core/or/conflux_pool.h | 3 +++ 2 files changed, 48 insertions(+) diff --git a/src/core/or/conflux_pool.c b/src/core/or/conflux_pool.c index c84613503f..53985df810 100644 --- a/src/core/or/conflux_pool.c +++ b/src/core/or/conflux_pool.c @@ -2020,6 +2020,51 @@ conflux_pool_init(void) } } +/** + * Return a description of all linked and unlinked circuits associated + * with a conflux set. + * + * For use in rare bug cases that are hard to diagnose. + */ +void +conflux_log_set(const conflux_t *cfx, bool is_client) +{ + tor_assert(cfx); + + log_warn(LD_BUG, "Conflux %s: %d linked, %d launched", + fmt_nonce(cfx->nonce), smartlist_len(cfx->legs), + cfx->num_leg_launch); + + // Log all linked legs + int legs = 0; + CONFLUX_FOR_EACH_LEG_BEGIN(cfx, leg) { + log_warn(LD_BUG, + " - Linked Leg %d purpose=%d; RTT %"PRIu64", sent: %"PRIu64 + " marked: %d", + legs, leg->circ->purpose, leg->circ_rtts_usec, + leg->linked_sent_usec, leg->circ->marked_for_close); + legs++; + } CONFLUX_FOR_EACH_LEG_END(leg); + + // Look up the nonce to see if we have any unlinked circuits. + unlinked_circuits_t *unlinked = unlinked_pool_get(cfx->nonce, is_client); + if (unlinked) { + // Log the number of legs and the is_for_linked_set status + log_warn(LD_BUG, " - Unlinked set: %d legs, for link: %d", + smartlist_len(unlinked->legs), unlinked->is_for_linked_set); + legs = 0; + SMARTLIST_FOREACH_BEGIN(unlinked->legs, leg_t *, leg) { + log_warn(LD_BUG, + " Unlinked Leg: %d purpose=%d; linked: %d, RTT %"PRIu64", " + "sent: %"PRIu64" link ptr %p, marked: %d", + legs, leg->circ->purpose, leg->linked, + leg->rtt_usec, leg->link_sent_usec, + leg->link, leg->circ->marked_for_close); + legs++; + } SMARTLIST_FOREACH_END(leg); + } +} + /** Free and clean up the conflux pool subsystem. This is called by the subsys * manager AFTER all circuits have been freed which implies that all objects in * the pools aren't referenced anymore. */ diff --git a/src/core/or/conflux_pool.h b/src/core/or/conflux_pool.h index 6bb858bb09..9a9701a484 100644 --- a/src/core/or/conflux_pool.h +++ b/src/core/or/conflux_pool.h @@ -36,6 +36,9 @@ void conflux_process_linked(circuit_t *circ, crypt_path_t *layer_hint, const cell_t *cell, const uint16_t cell_len); void conflux_process_linked_ack(circuit_t *circ); +typedef struct conflux_t conflux_t; +void conflux_log_set(const conflux_t *cfx, bool is_client); + #ifdef TOR_UNIT_TESTS bool launch_new_set(int num_legs); digest256map_t *get_linked_pool(bool is_client); -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit 176f0929bb3c8ffe2765fc47a24ee44a66ffd217 Author: Mike Perry <mikeperry-git@torproject.org> AuthorDate: Thu Mar 30 23:43:34 2023 +0000 Add conflux logs to diagnose cases where RTTs are absent/zero. --- src/core/or/conflux.c | 8 +++++++- src/core/or/conflux_pool.c | 50 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/core/or/conflux.c b/src/core/or/conflux.c index 5f6af9268b..3330de81ba 100644 --- a/src/core/or/conflux.c +++ b/src/core/or/conflux.c @@ -21,6 +21,7 @@ #include "core/or/conflux.h" #include "core/or/conflux_params.h" #include "core/or/conflux_util.h" +#include "core/or/conflux_pool.h" #include "core/or/conflux_st.h" #include "core/or/conflux_cell.h" #include "lib/time/compat_time.h" @@ -548,11 +549,16 @@ conflux_pick_first_leg(conflux_t *cfx) } } CONFLUX_FOR_EACH_LEG_END(leg); - if (BUG(!min_leg)) { + if (!min_leg) { // Get the 0th leg; if it does not exist, assert tor_assert(smartlist_len(cfx->legs) > 0); min_leg = smartlist_get(cfx->legs, 0); tor_assert(min_leg); + if (BUG(min_leg->linked_sent_usec == 0)) { + log_warn(LD_BUG, "Conflux has no legs with non-zero RTT. " + "Using first leg."); + conflux_log_set(cfx, CIRCUIT_IS_ORIGIN(min_leg->circ)); + } } // TODO-329-TUNING: We may want to initialize this to a cwnd, to diff --git a/src/core/or/conflux_pool.c b/src/core/or/conflux_pool.c index 53985df810..7e38e151a8 100644 --- a/src/core/or/conflux_pool.c +++ b/src/core/or/conflux_pool.c @@ -822,10 +822,29 @@ record_rtt_client(const circuit_t *circ) tor_assert(CIRCUIT_IS_ORIGIN(circ)); leg_t *leg = unlinked_leg_find(circ, true); - if (leg && leg->link_sent_usec > 0) { - leg->rtt_usec = monotime_absolute_usec() - leg->link_sent_usec; - return leg->rtt_usec; + + if (BUG(!leg || leg->link_sent_usec == 0)) { + log_warn(LD_BUG, + "Conflux: Trying to record client RTT without a timestamp"); + goto err; + } + + uint64_t now = monotime_absolute_usec(); + tor_assert_nonfatal(now >= leg->link_sent_usec); + leg->rtt_usec = now - leg->link_sent_usec; + if (leg->rtt_usec == 0) { + log_warn(LD_CIRC, "Clock appears stalled for conflux."); + // TODO-329-TUNING: For now, let's accept this case. We need to do + // tuning and clean up the tests such that they use RTT in order to + // fail here. + //goto err; } + return leg->rtt_usec; + + err: + // Avoid using this leg until a timestamp comes in + if (leg) + leg->rtt_usec = UINT64_MAX; return UINT64_MAX; } @@ -842,10 +861,26 @@ record_rtt_exit(const circuit_t *circ) tor_assert(CIRCUIT_IS_ORCIRC(circ)); conflux_leg_t *leg = conflux_get_leg(circ->conflux, circ); - if (leg && leg->linked_sent_usec > 0) { - leg->circ_rtts_usec = monotime_absolute_usec() - leg->linked_sent_usec; - return leg->circ_rtts_usec; + + if (BUG(!leg || leg->linked_sent_usec == 0)) { + log_warn(LD_BUG, + "Conflux: Trying to record exit RTT without a timestamp"); + goto err; } + + uint64_t now = monotime_absolute_usec(); + tor_assert_nonfatal(now >= leg->linked_sent_usec); + leg->circ_rtts_usec = now - leg->linked_sent_usec; + + if (leg->circ_rtts_usec == 0) { + log_warn(LD_CIRC, "Clock appears stalled for conflux."); + goto err; + } + return leg->circ_rtts_usec; + + err: + if (leg) + leg->circ_rtts_usec = UINT64_MAX; return UINT64_MAX; } @@ -863,6 +898,9 @@ record_rtt(const circuit_t *circ, bool is_client) if (is_client) { rtt_usec = record_rtt_client(circ); + if (rtt_usec == UINT64_MAX) + return false; + if (rtt_usec >= get_circuit_build_timeout_ms()*1000) { log_info(LD_CIRC, "Conflux leg RTT is above circuit build time out " "currently at %f msec. Relaunching.", -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit ff59e2f490c2ee2f119bfe85e7d95cd7e0bb51fa Author: Mike Perry <mikeperry-git@torproject.org> AuthorDate: Tue Jun 6 16:21:20 2023 +0000 Add BUG() macro to marked edge reads This will give us a full stacktrace. --- src/core/or/relay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 20336dffaf..247024ebc7 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -2333,7 +2333,7 @@ connection_edge_package_raw_inbuf(edge_connection_t *conn, int package_partial, tor_assert(conn); - if (conn->base_.marked_for_close) { + if (BUG(conn->base_.marked_for_close)) { log_warn(LD_BUG, "called on conn that's already marked for close at %s:%d.", conn->base_.marked_for_close_file, conn->base_.marked_for_close); -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit da50d21c42b43ede01bad48d205db67e6eed8bd2 Author: Mike Perry <mikeperry-git@torproject.org> AuthorDate: Thu Jun 8 22:44:26 2023 +0000 Bug 40801: Send LINKED_ACK before attaching streams Otherwise, the BEGIN cell arrives at the exit before it has an RTT, and then it does not know which circuit to prefer in response. --- src/core/or/conflux_pool.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/or/conflux_pool.c b/src/core/or/conflux_pool.c index 7e38e151a8..c8018f45e2 100644 --- a/src/core/or/conflux_pool.c +++ b/src/core/or/conflux_pool.c @@ -797,11 +797,6 @@ try_finalize_set(unlinked_circuits_t *unlinked) unlinked->cfx = NULL; unlinked_free(unlinked); - /* Now that this set is ready to use, try any pending streams again. */ - if (is_client) { - connection_ap_attach_pending(1); - } - log_info(LD_CIRC, "Successfully linked a conflux %s set which is now usable.", is_client ? "client" : "relay"); @@ -1964,6 +1959,12 @@ conflux_process_linked(circuit_t *circ, crypt_path_t *layer_hint, goto end; } + /* If this set is ready to use with a valid conflux set, try any pending + * streams again. */ + if (circ->conflux) { + connection_ap_attach_pending(1); + } + goto end; close: -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit 6a513e2ff57545ad232ff8931b8902421b89c09b Author: Mike Perry <mikeperry-git@torproject.org> AuthorDate: Fri Jun 9 15:52:42 2023 +0000 Bug 40801: Do not change read state of marked conns --- src/core/or/relay.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 247024ebc7..2c722f01cc 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -3081,9 +3081,9 @@ set_block_state_for_streams(circuit_t *circ, edge_connection_t *stream_list, if (stream_id && edge->stream_id != stream_id) continue; - if (!conn->read_event || edge->xoff_received) { - /* This connection is a placeholder for something; probably a DNS - * request. It can't actually stop or start reading.*/ + if (!conn->read_event || edge->xoff_received || + conn->marked_for_close) { + /* This connection should not start or stop reading. */ continue; } -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit 0149c1ff980414fca36c3aa4530ed25daf4e7553 Author: Mike Perry <mikeperry-git@torproject.org> AuthorDate: Fri Jun 9 16:12:51 2023 +0000 Bug 40801: Add changes file --- changes/bug40801 | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/changes/bug40801 b/changes/bug40801 new file mode 100644 index 0000000000..681c10c7d4 --- /dev/null +++ b/changes/bug40801 @@ -0,0 +1,10 @@ + o Minor bugfixes (conflux): + - Fix stream attachment order when creating conflux circuits, so that + stream attachment happens after finishing the full link handshake, + rather than upon set finalization. Fixes bug 40801; bugfix on + 0.4.8.1-alpha. + - Remove a "BUG" warning from conflux_pick_first_leg that can be + triggered by broken or malicious clients. Fixes bug 40801; bugfix + on 0.4.8.1-alpha. + - Fix a case where we were resuming reading on edge connections that + were already marked for close. Fixes bug 40801; bugfix on 0.4.8.1-alpha. -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit dbd37c0e7bb872208d4282d58f5b66d2fced781f Author: Mike Perry <mikeperry-git@torproject.org> AuthorDate: Sat Jun 10 16:00:56 2023 +0000 Bug 40810: Improve validation checks to ignore 0-RTT legs Also add calls to dump the legs of a conflux set if we have too many --- src/core/or/conflux_util.c | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/src/core/or/conflux_util.c b/src/core/or/conflux_util.c index 855d477428..589db41e83 100644 --- a/src/core/or/conflux_util.c +++ b/src/core/or/conflux_util.c @@ -20,6 +20,7 @@ #include "core/or/conflux.h" #include "core/or/conflux_params.h" #include "core/or/conflux_util.h" +#include "core/or/conflux_pool.h" #include "core/or/conflux_st.h" #include "lib/time/compat_time.h" #include "app/config/config.h" @@ -372,22 +373,39 @@ void conflux_validate_legs(const conflux_t *cfx) { tor_assert(cfx); - // TODO-329-UDP: Eventually we want to allow three legs for the - // exit case, to allow reconnection of legs to hit an RTT target. - // For now, this validation helps find bugs. - if (BUG(smartlist_len(cfx->legs) > conflux_params_get_num_legs_set())) { - log_warn(LD_BUG, "Number of legs is above maximum of %d allowed: %d\n", - conflux_params_get_num_legs_set(), smartlist_len(cfx->legs)); - } - + bool is_client = false; + int num_legs = 0; CONFLUX_FOR_EACH_LEG_BEGIN(cfx, leg) { - /* Ensure we have no pending nonce on the circ */ - tor_assert_nonfatal(leg->circ->conflux_pending_nonce == NULL); - tor_assert_nonfatal(leg->circ->conflux != NULL); - if (CIRCUIT_IS_ORIGIN(leg->circ)) { tor_assert_nonfatal(leg->circ->purpose == CIRCUIT_PURPOSE_CONFLUX_LINKED); + is_client = true; + } + + /* Ensure we have no pending nonce on the circ */ + if (BUG(leg->circ->conflux_pending_nonce != NULL)) { + conflux_log_set(cfx, is_client); + continue; + } + + /* Ensure we have a conflux object */ + if (BUG(leg->circ->conflux == NULL)) { + conflux_log_set(cfx, is_client); + continue; + } + + /* Only count legs that have a valid RTT */ + if (leg->circ_rtts_usec > 0) { + num_legs++; } } CONFLUX_FOR_EACH_LEG_END(leg); + + // TODO-329-UDP: Eventually we want to allow three legs for the + // exit case, to allow reconnection of legs to hit an RTT target. + // For now, this validation helps find bugs. + if (BUG(num_legs > conflux_params_get_num_legs_set())) { + log_warn(LD_BUG, "Number of legs is above maximum of %d allowed: %d\n", + conflux_params_get_num_legs_set(), smartlist_len(cfx->legs)); + conflux_log_set(cfx, is_client); + } } -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit 5d63842e86d60e2efdede4dd7afe25666eb86b9d Author: Mike Perry <mikeperry-git@torproject.org> AuthorDate: Tue Jun 13 18:15:07 2023 +0000 Bug 40810: Avoid using 0 RTT legs --- src/core/or/conflux.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/core/or/conflux.c b/src/core/or/conflux.c index 3330de81ba..c979077e1f 100644 --- a/src/core/or/conflux.c +++ b/src/core/or/conflux.c @@ -244,7 +244,9 @@ conflux_decide_circ_minrtt(const conflux_t *cfx) tor_assert(CONFLUX_NUM_LEGS(cfx)); CONFLUX_FOR_EACH_LEG_BEGIN(cfx, leg) { - if (leg->circ_rtts_usec < min_rtt) { + + /* Ignore circuits with no RTT measurement */ + if (leg->circ_rtts_usec && leg->circ_rtts_usec < min_rtt) { circ = leg->circ; min_rtt = leg->circ_rtts_usec; } @@ -279,7 +281,8 @@ conflux_decide_circ_lowrtt(const conflux_t *cfx) continue; } - if (leg->circ_rtts_usec < low_rtt) { + /* Ignore circuits with no RTT */ + if (leg->circ_rtts_usec && leg->circ_rtts_usec < low_rtt) { low_rtt = leg->circ_rtts_usec; circ = leg->circ; } @@ -399,7 +402,8 @@ conflux_decide_circ_cwndrtt(const conflux_t *cfx) /* Find the leg with the minimum RTT.*/ CONFLUX_FOR_EACH_LEG_BEGIN(cfx, l) { - if (l->circ_rtts_usec < min_rtt) { + /* Ignore circuits with invalid RTT */ + if (l->circ_rtts_usec && l->circ_rtts_usec < min_rtt) { min_rtt = l->circ_rtts_usec; leg = l; } @@ -420,7 +424,8 @@ conflux_decide_circ_cwndrtt(const conflux_t *cfx) /* Pick a 'min_leg' with the lowest RTT that still has * room in the congestion window. Note that this works for * min_leg itself, up to inflight. */ - if (cwnd_sendable(l->circ, min_rtt, l->circ_rtts_usec) > 0) { + if (l->circ_rtts_usec && + cwnd_sendable(l->circ, min_rtt, l->circ_rtts_usec) > 0) { leg = l; } } CONFLUX_FOR_EACH_LEG_END(l); -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit 7ffda7512d33b6e0c1417e8eff623b280d1120da Author: Mike Perry <mikeperry-git@torproject.org> AuthorDate: Tue Jun 13 18:13:02 2023 +0000 Changes file for bug40810 --- changes/bug40810 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changes/bug40810 b/changes/bug40810 new file mode 100644 index 0000000000..76859abe0c --- /dev/null +++ b/changes/bug40810 @@ -0,0 +1,4 @@ + o Minor bugfixes (conflux): + - Handle legs being closed or destroyed before computing an RTT + (resulting in warns about too many legs). Fixes bug 40810; bugfix on + 0.4.8.1-alpha. -- To stop receiving notification emails like this one, please contact the administrator of this repository.
This is an automated email from the git hooks/post-receive script. dgoulet pushed a commit to branch main in repository tor. commit 44368a727a85f9f24dc45c11f31958005d158ed6 Merge: d5306e107f 7ffda7512d Author: David Goulet <dgoulet@torproject.org> AuthorDate: Wed Jun 14 09:45:27 2023 -0400 Merge branch 'tor-gitlab/mr/721' changes/bug40801 | 10 +++++ changes/bug40810 | 4 ++ src/core/or/conflux.c | 21 ++++++--- src/core/or/conflux_pool.c | 106 ++++++++++++++++++++++++++++++++++++++++----- src/core/or/conflux_pool.h | 3 ++ src/core/or/conflux_util.c | 42 +++++++++++++----- src/core/or/relay.c | 8 ++-- 7 files changed, 162 insertions(+), 32 deletions(-) -- To stop receiving notification emails like this one, please contact the administrator of this repository.
participants (1)
-
gitolite role