| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
| |
(It is not yet actually used.)
|
| |
|
|
| |
(The information is not yet recorded.)
|
| |\
| |
| |
| | |
# Conflicts:
# doc/semver_status.md
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The old version of this function was error-prone, and in fact had
errors: it was too easy to forget to add non-persistent fields, and
that's exactly what we forgot in a few cases
(`microdescriptor_missing`, `circ_history`, and
`suspicious_behavior_warned`).
The new version of this function consumes both of the incoming
Guards, and constructs every field explicitly so that we can't
forget to list any.
Closes #429.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously, we treated successfully building a circuit to a guard as
a "success", and any failure, including a directory cache failure,
as a failure. With this change, guards now have separate
success/failure and retry status for circuit usage and directory
usage.
This change is needed for guard-as-directory retry to have
reasonable behavior. Otherwise, when a guard succeeds at building a
circuit, that clears the directory-is-failing status and makes us
retry the guards to quickly.
|
| | |
| |
| |
| | |
This will help when we give Guards a DirStatus as well.
|
| |/
|
|
| |
It's about to be shared between fallbacks and guards.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
| |
The FirstHopId type now records an enum that stores whether the hop
is a guard or a fallback. This change addresses concerns about
remembering to check the type or source of an Id before passing it
down to the FallbackState or GuardSet.
Making this change required an API change, so that dirmgr can
report success/failure status without actually knowing whether it's
using a fallback or a guard.
|
| |
|
|
|
| |
This is preparation for having separate GuardId and FirstHopId types
that distinguish which back-end they index.
|
| |
|
|
|
| |
This replaces a hand-coded replacement that was probably a little
less efficient.
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
| |
The code here uses a new iterator type, since I couldn't find one of
these on crates.io. I tried writing the code without it, but it was
harder to follow and test.
|
| |
|
|
|
|
|
|
|
|
|
| |
We do this by creating a new FallbackSet type that includes status
information, and updating the GuardMgr APIs to record success and
failure about it when appropriate. We can use this to mark
FallbackDirs retriable (or not).
With this change, FallbackDir is now stored internally as a Guard in
the GuardMgr crate. That's fine: the FallbackDir type really only
matters for configuration.
|
| |
|
|
|
|
|
|
|
|
|
| |
We only do this when we fail to get a regular guard (e.g., because
they're all down), and when we have been asked for a guard for a
one-hop directory.
Most of the change in this commit is plumbing to make all of the
types match up.
As before, compilation may still be broken.
|
| | |
|
| |
|
|
|
|
|
|
|
| |
We need to extend our notion of "the origin of a guard" to include
"somewhere outside the guard list"; we need the ability to return a
FallbackDir as a Guard; and we need to remember a few more pieces of
information in each pending request.
As before, this commit may break compilation; it will be restored soon.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
The guard manager is responsible for handing out the first hops of
tor circuits, keeping track of their successes and failures, and
remembering their states. Given that, it makes sense to store this
information here. It is not yet used; I'll be fixing that in
upcoming commits.
Arguably, this information no longer belongs in the directory
manager: I've added a todo about moving it.
This commit will break compilation on its own in a couple of places;
subsequent commits will fix it up.
|
| |
|
|
| |
This is more in keeping with the rest of our code.
|
| |
|
|
|
|
|
| |
This is the logical place for it, I think: the GuardMgr's job is to
pick the first hop for a circuit depending on remembered status for
possible first hops. Making this change will let us streamline the
code that interacts with these objects.
|
| | |
|
| |\
| |
| |
| |
| |
| |
| | |
Don't use SystemTime::now()
Closes #306
See merge request tpo/core/arti!365
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| | |
We'll need this so that we can say "This guard behaved bogusly
as a directory cache; try somebody else."
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When all guards are down, we would previously mark them all as up,
and retry aggressively. But that's far too aggressive: if there's
something wrong with our ability to connect to guards, it makes us
hammer the network over and over, ignoring all the guard retry
timeouts in practice.
Instead,
* We now allow the `pick_guard()` function to fail without
automatically retrying.
* We give different errors in the cases when all our guards are
down, and when all of the guards selected by our active usage
are down.
* Our "guards are down" error includes the time at which a guard
will next be retriable.
This is part of #407.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
C tor used one schedule, and guard-spec specified another. But in
reality we should probably use a randomized schedule to retry
guards, for the reasons explained in the documentation for
RetrySchedule.
I've chosen the minima to be not too far from our previous minima
for primary and non-primary guards.
This is part of #407.
|
| | |
| |
| |
| |
| |
| |
| | |
Code motion and the minimal mechanical changes.
As per
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/375#note_2783078
|
| | | |
|
| | | |
|
| |\ \
| | |
| | |
| | |
| | | |
Disable clippy::clone_on_ref_ptr
See merge request tpo/core/arti!352
|
| | |/
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This lint is IMO inherently ill-conceived.
I have looked for the reasons why this might be thought to be a good
idea and there were basically two (and they are sort of contradictory):
I. "Calling ‘.clone()` on an Rc, Arc, or Weak can obscure the fact
that only the pointer is being cloned, not the underlying data."
This is the wording from
https://rust-lang.github.io/rust-clippy/v0.0.212/#clone_on_ref_ptr
It is a bit terse; we are left to infer why it is a bad idea to
obscure this fact. It seems to me that if it is bad to obscure some
fact, that must be because the fact is a hazard. But why would it be
a hazard to not copy the underlying data ?
In other languages, faliing to copy the underlying data is a serious
correctness hazard. There is a whose class of bugs where things were
not copied, and then mutated and/or reused in multiple places in ways
that were not what the programmer intended. In my experience, this is
a very common bug when writing Python and Javascript. I'm told it's
common in golang too.
But in Rust this bug is much much harder to write. The data inside an
Arc is immutable. To have this bug you'd have use interior mutability
- ie mess around with Mutex or RefCell. That provides a good barrier
to these kind of accidents.
II. "The reason for writing Rc::clone and Arc::clone [is] to make it
clear that only the pointer is being cloned, as opposed to the
underlying data. The former is always fast, while the latter can
be very expensive depending on what is being cloned."
This is the reasoning found here
https://github.com/rust-lang/rust-clippy/issues/2048
This is saying that *not* using Arc::clone is hazardous.
Specifically, that a deep clone is a performance hazard.
But for this argument, the lint is precisely backwards. It's linting
the "good" case and asking for it to be written in a more explicit
way; while the supposedly bad case can be written conveniently.
Also, many objects (in our codebase, and in all the libraries we use)
that are Clone are in fact simply handles. They contain Arc(s) (or
similar) and are cheap to clone. Indeed, that is the usual case.
It does not make sense to distinguish in the syntax we use to clone
such a handle, whether the handle is a transparent Arc, or an opaque
struct containing one or more other handles.
Forcing Arc::clone to be written as such makes for code churn when a
type is changed from Arc<Something> to Something: Clone, or vice
versa.
|
| |/
|
|
|
|
|
|
| |
I wanted this while debugging something.
The ad-hoc impl Debug with f.debug_struct is getting repetitive
and I've already perpetrated one copy-paste mistake.
We should consider using something like the `educe` crate's Clone.
|
| |
|
|
|
| |
This exception is no longer necessary now that the underlying CI bug
is fixed.
|
| |
|
|
| |
Closes #338.
|
| |
|
|
|
|
| |
This edge-case was there even before the migration of
595fe1ab881b94106649, but now it's more explicit and ought to be
revisited.
|
