| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It is unused but most importantly it allows any RELAY cell to be sent
from anywhere in the code which is really not desirable because it is
skipping congestion control.
It also allows us to remove the `control_tx` from the reactor which is
one less channel to track/understand/think about.
This opens up the door to all sorts of problems especially side channel
that can be exploited if we are not careful.
We can always bring this back if we need it but for now, it is unused
and allows us to remove the `CtrlMsg::SendRelayCell` control message.
No code behavior change.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This object only mission is to handle a control message (CtrlMsg). It
encapsulates a reference to the reactor and its lifetime is only the
time it takes to handle the message.
Main goal of this is to try to reduce reactor.rs size into logical
containers.
No code behavior change.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This hopefully fixes #1832.
I haven't been able to repro the failure from #1832 locally, but judging
from the logs, I suspect it's triggered by interleavings like:
- `extend_fut` executes up until `circ.extend_ntor(..).await`, where
it's blocked `await`ing a completion notification from the circuit
reactor
- `reply_fut` starts running:
* `CircuitExtender` installs a meta-handler in the circuit reactor,
and waits for an `EXTENDED2` cell
* `test_extend()` reads a cell from the receiving end of the fake
channel's output sink (`rx`)
* `test_extend()` sends the `EXTENDED2` cell to the circuit reactor
over `CircuitRxSender`
* `rx` is dropped
- `reply_fut` completes
- in the channel reactor's `run_once()`, `select_biased!` resolves
to an error, because the receiving end of the `output` sink (`rx`) was
dropped. The reactor logs `got sink error: SendError { kind:
Disconnected }` (see [`SendError`]), and exits
- the circuit reactor exits too, because the receiving end of its
`chan_sender` (which was in the channel reactor) was dropped
Returning the `rx` of the fake channel's output sink from `reply_fut`
*should* stop it from being dropped, and fix #1832.
[`SendError`]: https://docs.rs/futures/latest/futures/channel/mpsc/struct.SendError.html#method.is_disconnected
|
| |\
| |
| |
| |
| | |
tor-proto: Tweak StreamCtrl APIs for RPC.
See merge request tpo/core/arti!2755
|
| | |
| |
| |
| |
| |
| |
| |
| | |
This method is experimental, so no semver note is needed.
It is redundant with `client_stream_ctrl()?.circuit()?`.
(The name and the unconditional return type of this method
are probably an error.)
|
| | |
| |
| |
| |
| | |
Since these return a client-specific type,
they need a client-specific name before we can stabilize them for RPC.
|
| | |
| |
| |
| |
| |
| |
| | |
Semantically, the new name matches the behavior much better.
(A stream could well count as open if we had sent a RESOLVE but not
received a RESOLVED, so we might someday want to have an `is_open`
defined for _all_ streams, not just data streams.)
|
| | |
| |
| |
| |
| |
| | |
The API for this type, and the fact that it implements
ClientStreamCtrl unconditionally, means that it is only for client
DataStreams.
|
| | | |
|
| |\ \
| |/
|/|
| |
| | |
tor-proto: Rewrite circuit reactor run_once() loop to use select!.
See merge request tpo/core/arti!2747
|
| | | |
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| |
| | |
It's unclear to me why this warning didn't fire before! The the circuit
reactor had this exact same `debug_assert` prior to the refactoring,
taking a mutable reference to the `StreamMap`.
|
| | | |
|
| | |
| |
| |
| | |
This is now handled in `Reactor::ready_streams_iterator()`.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| | |
This is replaced by the new `Reactor::run_once()` impl.
Note: this removal is in a separate commit to make the diff of the
refactoring commits easier to review (the diff between the old and the
new impl is not meaningful because the new impl is practically a
rewrite).
|
| | |
| |
| |
| | |
This is replaced by `Option<RunOnceCmdInner>`.
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This rewrites the circuit reactor main loop to use `select_biased!` to poll
multiple futures simultaneously.
The new `run_once()`, like the old, first waits for an initial
`CtrlMsg::Create`. Then, it uses a `select_biased!` to poll the
`chan_sender` sink and shutdown channel for readiness.
When the channel sink is ready, we poll the `control` and `input`
channels like before, as well as the new `ready_streams` `Stream`
(`ready_streams` is a `futures::Stream` that replaces the previous
`send_outbound()` function).
Most of the implementation remains unchanged, except the `handle_input`,
`handle_cell` and `handle_control` functions no longer send anything on
the `chan_sender` channel. Instead, they may do some (synchronous)
processing, and send instructions for the remaining work that needs to
be done (for example, for writing the cell to the `chan_sender`
channel). These instructions are handled at the end of `run_once()`,
and are encoded in the `RunOnceCmdInner` enum.
What this change does **not** do:
* the control channel *still* bypasses congestion control. We could
fix this by making the various reactor functions send the
`RunOnceCmdInner` commands to `run_once()` via a channel
(instead of returning them). This would enable the reactor to stop
reading the commands (except for handle `Sendme`, which would be
handled separately) if it's blocked on congestion control.
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | |
| |
| |
| | |
No functional changes, just code motion.
|
| | |
| |
| |
| |
| | |
We are about to use this in the circuit reactor (instead of
`pollish_send_unbounded`).
|
| |/ |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
| |
This is mostly code motion (I recommend reviewing with `--color-moved`).
The code is intentionally misindented to make diffing easier. A future
commit will fix the indentation.
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
| |
For readability.
|
| |
|
|
|
| |
This enables us to get rid of two lines from the circuit reactor
constructor.
|
| |\
| |
| |
| |
| | |
tor-proto: Replace sleep() in test with advance_until_stalled().
See merge request tpo/core/arti!2721
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This test spuriously failed on my !2720 branch
(https://gitlab.torproject.org/gabi-250/arti/-/jobs/811495):
```
failures:
---- circuit::test::invalid_circ_sendme stdout ----
E tor_proto::channel::reactor: UniqId(71): Running reactor
E tor_proto::circuit::reactor: Circ 23.17: Running circuit reactor
E tor_proto::circuit::reactor: Circ 23.17: reactor received AddFakeHop { relay_cell_format: V0, fwd_lasthop: false, rev_lasthop: false, params: CircParameters { extend_by_ed25519_id: true, ccontrol: CongestionControlParams { alg: FixedWindow(FixedWindowParams { circ_window_start: 1000, circ_window_min: 100, circ_window_max: 1000 }), cwnd_params: CongestionWindowParams { cwnd_init: 124, cwnd_inc_pct_ss: Percentage { value: 100 }, cwnd_inc: 1, cwnd_inc_rate: 31, cwnd_min: 124, cwnd_max: 4294967295, sendme_inc: 31 }, rtt_params: RoundTripEstimatorParams { ewma_cwnd_pct: Percentage { value: 50 }, ewma_max: 10, ewma_ss_max: 2, rtt_reset_pct: Percentage { value: 100 } } } }, done: Sender { complete: false } }
E tor_proto::circuit::reactor: Circ 23.17: reactor received AddFakeHop { relay_cell_format: V0, fwd_lasthop: true, rev_lasthop: true, params: CircParameters { extend_by_ed25519_id: true, ccontrol: CongestionControlParams { alg: FixedWindow(FixedWindowParams { circ_window_start: 1000, circ_window_min: 100, circ_window_max: 1000 }), cwnd_params: CongestionWindowParams { cwnd_init: 124, cwnd_inc_pct_ss: Percentage { value: 100 }, cwnd_inc: 1, cwnd_inc_rate: 31, cwnd_min: 124, cwnd_max: 4294967295, sendme_inc: 31 }, rtt_params: RoundTripEstimatorParams { ewma_cwnd_pct: Percentage { value: 50 }, ewma_max: 10, ewma_ss_max: 2, rtt_reset_pct: Percentage { value: 100 } } } }, done: Sender { complete: false } }
E tor_proto::circuit::reactor: Circ 23.17: reactor received BeginStream { hop_num: HopNum(2), message: Begin(Begin { addr: [119, 119, 119, 46, 101, 120, 97, 109, 112, 108, 101, 46, 99, 111, 109], port: 443, flags: BeginFlags(IPV6_OKAY) }), sender: Sender { tx: Sender { closed: false }, mq: TypedParticipation(Participation(Noop)) }, rx: Receiver { inner: ReceiverInner { state: Mutex { data: Ok(ReceiverState { rx: StreamUnobtrusivePeeker { buffered: None, poll_waker: None, inner: Some(Receiver { closed: false }) }, mq: TypedParticipation(Participation(Noop)), collapse_callbacks: 0 }), poisoned: false, .. } } }, done: Sender { complete: false }, cmd_checker: DataCmdChecker { expecting_connected: true } }
Using RNG seed ARTI_TEST_PRNG=62d085c0fb1213c4f41d1a0a5c3f92dd10223a27c42bfda3caedfe60e011e95d
E tor_proto::circuit::reactor: Circ 23.17: handling cell: Relay(Relay { body: .. })
Using RNG seed ARTI_TEST_PRNG=fd2f2f045a7340f1189972b13645346943efa2c42f43afd2161420c692bb4ff0
E tor_proto::circuit::reactor: Circ 23.17: handling cell: Relay(Relay { body: .. })
E tor_proto::circuit::reactor: Circ 23.17: Circuit reactor stopped: Err(CircProto("Mismatched tag on circuit SENDME"))
E tor_proto::channel::reactor: UniqId(71): reactor received CloseCircuit(CircId(128))
E tor_proto::channel::reactor: UniqId(71): Circuit 128 is gone; sending DESTROY
thread 'circuit::test::invalid_circ_sendme' panicked at crates/tor-proto/src/circuit.rs:2262:21:
reactor continued running after invalid sendme
failures:
circuit::test::invalid_circ_sendme
test result: FAILED. 142 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out; finished in 4.93s
```
This is probably because of the `sleep()`-based check. This branch
changes the test to use `MockRuntime`, replacing the `sleep()` with
`advance_until_stalled()`.
|
| | |
| |
| |
| |
| |
| | |
The more concise name makes the code a bit more readable, IMO (and it's
consistent with the `Result` type-aliasing style we've been using in the
rest of the codebase).
|
| | |
| |
| |
| | |
It's not needed because `CircHop` is defined in the same file.
|
| | |
| |
| |
| | |
No functional changes, just `cargo fmt`.
|
| |/
|
|
|
|
|
|
|
| |
This is mostly code motion in preparation for refactoring `run_once()`
to use `select!` instead of the hand-rolled `poll_fn` impl.
Note: the code here is intentionally mis-indented, to simplify diffing
(I recommend using `git diff --color-moved`). A future commit will fix
the indentation.
|
| |
|
|
|
|
| |
Both in tor-proto and tor-circmgr.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
| |
Allows us to remove the pub.
Signed-off-by: David Goulet <[email protected]>
|
| |
|
|
|
|
|
| |
Important to enforce that every field is explicitely set so we avoid
forgetting fields.
Signed-off-by: David Goulet <[email protected]>
|
