[Nick, can you merge my torspec proposal218 branch, please? Thanks!]
On 2/28/13 2:08 AM, Rob Jansen wrote:
On Mon, Feb 25, 2013 at 10:28 AM, Karsten Loesing <karsten@torproject.org>
wrote:
On 2/23/13 11:20 PM, Rob Jansen wrote:
On Feb 23, 2013 4:22 PM, "Karsten Loesing" <karsten@torproject.org>
wrote:
Your understanding of n_circ_id and p_circ_id matches mine, but are you
sure there's a UID for circuits other than origin circuits? I think you
mean origin_circuit_t->global_identifier. But there's no such field for
or_circuit_t or circuit_t. Or do you mean something else?
Ok. Though I thought my original patch moved the global Id to the base
circuit struct. Perhaps I didn't. Anyway, I'm not sure it matters...
Your original patch did move the ID to circuit_t, but I thought we
wanted to avoid numbering non-origin circuits (mostly because that
affects relays in non-TestingTorNetwork mode and could lead to busy
relays running out of IDs at some point), which is why I took this
change out.
Right, I remember this now. I could imagine ways to get around the 'running
out of ids' problem, like resetting the id counter after a given timeframe.
In fact, we could just let the counter overflow and loop back to 0 on its
own (assuming its a unsigned int type) since we really only need them to be
unique approximately for the life of a circuit. If we see the ID come up
again after an hour, we can assume its a new circuit.
I'd rather not want to change behavior in non-simulation mode. If we
really need a UID for non-origin circuits, we could add a separate
uint64_t global_identifier to circuit_t and only use that in simulating
mode. I'm not sure that we need it though.
Also, even if we moved the field to circuit_t, the new CELL_STATS events
would be the only ones using these UIDs, because all other events are
for origin circuits only. I don't see how these IDs would help us. We
never learn what UID the next or previous node in a circuit picks for a
given circuit.
Not currently, but couldn't we implement the functionality where relays log
or export their circuit UIDs and next/prev IDs over the control port?
Though I'm not sure if you'd want this in mainline Tor if its only useful
in simulation mode...
We could have relays log local circuit UIDs together with p_circ_id,
p_conn_id, n_circ_id, and n_conn_id in simulation mode. But I don't see
how that would facilitate circuit tracking compared to the current
approach using ORCONN and CELL_STATS events.
Here's an example:
- fileclient creates a circuit with
- UID 14,
- n_conn_id 15, and
- n_circ_id 19403.
- tokenglobal is the first hop in this circuit and reports
- (new) UID 12345,
- p_conn_id 32,
- p_circ_id 19403,
- n_conn_id 18, and
- n_circ_id 6710.
- tokenrelay is the second hop and reports
- (new) UID 23456,
- p_conn_id 17,
- p_circ_id 6710,
- n_conn_id 16,
- n_circ_id 34402.
- exit2 is the third and final hop and reports
- (new) UID 34567,
- p_conn_id 15, and
- p_circ_id 34402.
We need ORCONN events to know that fileclient's n_conn_id 15 is the same
connection as tokenglobal's p_conn_id 32. How would the new UIDs make
this easier?
tl;dr: I _think_ it's possible to reconstruct circuits from ORCONN and
CELL_STATS events as they are currently specified in proposal 218.
Great, but do we really expect every Tor controller parser to get this
right? It seems complicated enough that there should be an easier way.
Maybe it's just wishful thinking on my part.
I agree that reconstructing circuits from ORCONN and CELL_STATS events
is far from trivial. I don't really see how to make it simpler though.
What about linking all of the IDs and UIDs as described above?
(See above.)
From an earlier mail in this thread:
Finally, Rob, should I look into CIRC_BW events you suggested a while
ago? If yes, what did you have in mind how that event would look like,
and when/by whom would it be emitted?
If we want to do this, it would likely be an aggregation of all STREAM_BW
events for a circuit, but only during the time when those streams
belonged
to that circuit. I don't think it makes sense to emit it for every
STREAM_BW event though, so what if we aggregate and emit it once per
second? A format similar to the STREAM_BW format should work fine.
Done. I specified and implemented such a CIRC_BW event.
Great!
Here's the updated proposal 218 (Nick, please don't merge this yet):
https://gitweb.torproject.org/user/karsten/torspec.git/blob/refs/heads/propo...
In section 5.3/5.4/5.5, these events are emitted in the
second_elapsed_callback(), right? I wanted to verify that a relay who
hasn't sent anything in a few seconds and then starts sending again will
emit the event at the end of the second after which it resumed sending,
rather than the first bytes after it resumed sending.
Yes, all these events are emitted in second_elapsed_callback().
The last word in 5.4 should be 'reading' instead of 'read'.
Fixed.
Is the specification of 5.5 and 5.6 complex enough to warrant including
example outputs?
Sure, can't hurt. Added a few examples.
In 5.6, were we planning on explaining how buckets can go negative and how
that affects the reporting of the TB_EMPTY events?
I tried a better explanation:
"""
ReadBucketEmpty (WriteBucketEmpty) is the time in millis that the read
(write) bucket was empty since the last refill. LastRefill is the
time in millis since the last refill.
If a bucket went negative and if refilling tokens didn't make it go
positive again, there will be multiple consecutive TB_EMPTY events for
each refill interval during which the bucket contained zero tokens or
less. In such a case, ReadBucketEmpty or WriteBucketEmpty are capped
at LastRefill in order not to report empty times more than once.
"""
Here's the tor branch:
https://gitweb.torproject.org/karsten/tor.git/shortlog/refs/heads/morestats3
Here's a Shadow log file containing all new events:
https://people.torproject.org/~karsten/volatile/scallion.log.gz
Here's the Java program that I used to parse the Shadow log file:
https://people.torproject.org/~karsten/volatile/ParseProposal218Events.java
Ugh. I really don't look forward to writing parsers for this. I guess there
may be few projects that actually require this information, and those that
do can use your code:)
I hope that some of the complexity goes away once we use a parsing
library like Stem. And I think we should provide parsing code to other
people looking into this.
And finally, here's the output, which should be easier to understand now:
https://people.torproject.org/~karsten/volatile/scallion.txt
Search for "Circuit [fileclient-60.1.0.0]:14" to find the circuit I
mentioned earlier in this thread.
Can you review the proposal changes and tell me if they make sense to you?
Best,
Karsten
See comments above. Overall, it looks good. I'm still wondering about the
UIDs / IDs issue. It may be that we don't want to include that for other
reasons, in which case the current implementation is fine.
Okay, in that case let's consider proposal 218 done for the moment,
unless we come up with a better idea to solve circuit-tracking thing.
I asked Nick above to merge my changes, so that I can put proposal 218
on the sponsor F year 3 wiki page as one result of the February 28
milestone. (That doesn't mean we cannot make it even better after
February.)
Thanks!
Karsten
Karsten Loesing
Nick Mathewson