Proposal: Controller events to better understand connection/circuit usage
Hi everyone, here's a proposal that defines three new controller events for the TestingTorNetwork mode that shall help us better understand connection and circuit usage in private Tor networks with the goal to make simulations more accurate. And there's also code: https://gitweb.torproject.org/karsten/tor.git/shortlog/refs/heads/morestats2 Feedback, to both proposal and code, much appreciated! Thanks, Karsten Filename: xxx-usage-controller-events.txt Title: Controller events to better understand connection/circuit usage Author: Rob Jansen, Karsten Loesing Created: 2013-02-06 Status: Open Target: 0.2.5.x 1. Overview This proposal defines three new controller events that shall help understand connection and circuit usage. These events are designed to be emitted in private Tor networks only. This proposal also defines a tweak to an existing event for the same purpose. 2. Motivation We need to better understand connection and circuit usage in order to better simulate Tor networks. Existing controller events are a fine start, but we need more detailed information about per-connection bandwidth, processed cells by circuit, and token bucket refills. This proposal defines controller events containing the desired information. Most of these usage data are too sensitive to be captured in the public network, unless aggregated sufficiently. That is why we're focusing on private Tor networks first, that is, relays that have TestingTorNetwork set. The new controller events described in this proposal shall all be restricted to private Tor networks. In the next step we might define aggregate statistics to be gathered by public relays, but that will require a new proposal. 3. Design The proposed new event types use Tor's asynchronous event mechanism where a controller registers for events by type and processes events received from the Tor process. Tor controllers can register for any of the new event types, but events will only be emitted if the Tor process is running in TestingTorNetwork mode. 4. Security implications There should be no security implications from the new event types, because they are only emitted in private Tor networks. 5. Specification 5.1. Adding an ID field to ORCONN events The new syntax for ORCONN events is: "650" SP "ORCONN" SP (LongName / Target) SP ORStatus [ SP "ID=" ConnId ] [ SP "REASON=" Reason ] [ SP "NCIRCS=" NumCircuits ] CRLF ConnID is the connection ID which is locally unique among all connection types and which is only included in TestingTorNetwork mode. The remaining specification of that event type stays unchanged. 5.2. Bandwidth used on an OR or DIR or EXIT connection The syntax is: "650" SP "CONN_BW" SP ConnID SP ConnType SP BytesRead SP BytesWritten CRLF BytesRead = 1*DIGIT BytesWritten = 1*DIGIT ConnID is the connection ID which is locally unique among all connection types. ConnType is the connection type, which can be "OR" or "DIR" or "EXIT". BytesWritten and BytesRead are the number of bytes written and read by Tor since the last CONN_BW event on this connection. These events are generated about once per second per connection; no events are generated for connections that have not read or written. These events are only generated if TestingTorNetwork is set. 5.3. Per-circuit cell stats The syntax is: "650" SP "CELL_STATS" SP PCircID SP PConnID SP PAdded SP PRemoved SP PTime SP NCircID SP NConnID SP NAdded SP NRemoved SP NTime CRLF PCircID and NCircID are the locally unique IDs of the app-ward (PCircID) and exit-ward (NCircID) circuit. PConnID and NConnID are the locally unique IDs of the app-ward (PConnID) and exit-ward (NConnID) OR connection. PAdded and NAdded are the total number of cells added to the app-ward (PAdded) and exit-ward (NAdded) queues of this circuit. PRemoved and NRemoved are the total number of cells processed from the app-ward (PRemoved) and exit-ward (NRemoved) queues of this circuit. PTime and NTime are the total waiting times in milliseconds of all processed cells in the app-ward (PTime) and exit-ward (NTime) queues of this circuit. PAdded, NAdded, PRemoved, NRemoved, PTime, and NTime are semicolon-separated key-value lists with keys being lower-case cell types and values being cell numbers or waiting times. These events are generated about once per second per circuit; no events are generated for circuits that have not added or processed any cell. These events are only generated if TestingTorNetwork is set. 5.4. Token buckets refilled The syntax is: "650" SP "TB_EMPTY" SP ["GLOBAL" || "RELAY" || "ORCONN" SP ConnID] SP ReadBucketEmpty SP WriteBucketEmpty SP LastRefill CRLF This event is generated when refilling a previously empty token bucket. The "GLOBAL" and "RELAY" keywords are used for the global or relay token buckets, the "ORCONN" keyword together with a ConnID is used for the token buckets of an OR connection. If both global and relay buckets and/or the buckets of one or more OR connections run out of tokens at the same time, multiple separate events are generated. ReadBucketEmpty (WriteBucketEmpty) is the time in millis that the read (write) bucket was empty. LastRefill is the time in millis since the last refill. ReadBucketEmpty or WriteBucketEmpty are capped at LastRefill in order not to report empty times more than once. These events are only generated if TestingTorNetwork is set. 6. Compatibility There should not be any compatibility issues with other Tor versions. 7. Implementation Most of the implementation should be straight-forward. There's one exception: we pondered adding a unique circuit ID to CELL_STATS events, but so far, only origin circuits have a unique ID. We could move that field from origin_circuit_t to circuit_t and update all references in the code. But this may have undesired side effects which we're not yet aware of. We don't have a good answer yet if we need this ID or not. 8. Performance and scalability notes Most of the new code won't be executed in normal Tor mode. Wherever we needed new fields in existing structs, we tried hard to keep them as small as possible. Still, we should make sure that memory requirements won't grow significantly on busy relays.
Hi Karsten. Looks good to me, just a few comments on the spec parts...
5.1. Adding an ID field to ORCONN events
Please define the format of ConnId. This would belong in section 2.4, expanding... ; Unique identifiers for streams or circuits. Currently, Tor only ; uses digits, but this may change StreamID = 1*16 IDChar CircuitID = 1*16 IDChar IDChar = ALPHA / DIGIT
5.2. Bandwidth used on an OR or DIR or EXIT connection
Minor nit pick, but the normal way of showing an enumeration is... ConnType = "OR" / "DIR" / "EXIT" Please make it clear in the spec if new ConnType are allowed or not.
5.3. Per-circuit cell stats
Again, please specify the format of all of these cells. Most are obviously numeric, but a formal specification helps answer questions like 'can I assume this can't be negative?'. Are you sure that you want to use positional arguments for all of these? If you do then you'll lose a lot of flexibility. Making them positional means... * these values will always be mandatory, even if you want to deprecate them in the future * you'll always need to keep this ordering, with new values appended to the end
"650" SP "TB_EMPTY" SP ["GLOBAL" || "RELAY" || "ORCONN" SP ConnID] SP ReadBucketEmpty SP WriteBucketEmpty SP LastRefill CRLF
Ewww! No, please don't do this. Pretty please with a cherry on top? Having either one *or* two positional arguments for the first slot makes this a special snowflake among tor events (and hence need special handling). ConnID should be an optional keyword argument instead, documented to only exist if the event type is "ORCONN". Same cautioning as before about positional vs keyword arguments, and defining the values. Cheers! -Damian
Hi Damian! Thanks a lot for your feedback! The proposed event formats are not at all set in stone. If we can change formats to make events easier to parse, by all means let's do that. You probably have more experience with the control port than anyone at Tor, so I say let's listen to you. On 2/7/13 5:33 PM, Damian Johnson wrote:
Hi Karsten. Looks good to me, just a few comments on the spec parts...
5.1. Adding an ID field to ORCONN events
Please define the format of ConnId. This would belong in section 2.4, expanding...
; Unique identifiers for streams or circuits. Currently, Tor only ; uses digits, but this may change StreamID = 1*16 IDChar CircuitID = 1*16 IDChar IDChar = ALPHA / DIGIT
Good point, will do.
5.2. Bandwidth used on an OR or DIR or EXIT connection
Minor nit pick, but the normal way of showing an enumeration is...
ConnType = "OR" / "DIR" / "EXIT"
Please make it clear in the spec if new ConnType are allowed or not.
Yes, new types should be allowed, though I think it will be just these three types for now. I wonder if we should avoid restricting connection types in the spec by defining this argument as connection type _string_ as opposed to an enumeration. That would be similar to cell type strings in CELL_STATS events. Has the advantage that we don't have to touch the spec whenever we add another connection type. Does that make sense?
5.3. Per-circuit cell stats
Again, please specify the format of all of these cells. Most are obviously numeric, but a formal specification helps answer questions like 'can I assume this can't be negative?'.
All numbers are non-negative integers. Will specify this more formally.
Are you sure that you want to use positional arguments for all of these? If you do then you'll lose a lot of flexibility. Making them positional means...
* these values will always be mandatory, even if you want to deprecate them in the future * you'll always need to keep this ordering, with new values appended to the end
A fine question. Here are two examples of this event: CELL_STATS 8 47110 15 created=1;relay=1 created=1;relay=1 created=0;relay=0 39943 16 create=1 create=1 create=0 CELL_STATS 8 47110 15 - - - 39943 16 relay_early=1 relay_early=1 relay_early=0 Can you suggest a better format, maybe one without positional arguments?
"650" SP "TB_EMPTY" SP ["GLOBAL" || "RELAY" || "ORCONN" SP ConnID] SP ReadBucketEmpty SP WriteBucketEmpty SP LastRefill CRLF
Ewww! No, please don't do this. Pretty please with a cherry on top?
Ewww, cherries... ;)
Having either one *or* two positional arguments for the first slot makes this a special snowflake among tor events (and hence need special handling). ConnID should be an optional keyword argument instead, documented to only exist if the event type is "ORCONN".
Sure, sounds doable. Will fix that.
Same cautioning as before about positional vs keyword arguments, and defining the values.
I'm fine changing positional arguments to keyword arguments. Maybe we should do that for all three new events. Want to suggest new event formats with keyword arguments to make them easier to parse by Stem and friends? I'll wait with making changes to the proposal until I hear back from you. Thanks again for your very valuable feedback! Cheers! Karsten
I wonder if we should avoid restricting connection types in the spec by defining this argument as connection type _string_ as opposed to an enumeration.
That depends on how volatile you think it'll be. If it's reasonably static then an enumeration would probably be best.
Has the advantage that we don't have to touch the spec whenever we add another connection type. Does that make sense?
True, but that also means that event recipients have no idea what kind of values to expect or what they mean.
A fine question. Here are two examples of this event: ...
Ah, I see. That does make it trickier. I'm not spotting any precedent for doing multiple sub-mappings in an event. Doing this in positional arguments is simple to parse, but beside the lack of flexibility mentioned earlier it's pretty hard to read. I cringe a bit to suggest it, but maybe a mapping in a mapping? CELL_STATS PCircID=8 PConnID=47110 PAdded=created:1,relay:1 PRemoved=created:1,relay:1
Sure, sounds doable. Will fix that.
Thanks!
Want to suggest new event formats with keyword arguments to make them easier to parse by Stem and friends?
Positional arguments aren't harder to parse, just harder to read and more inflexible. I'd be happy to suggest spec alternatives if you want. Mind if we revise the proposal based on the above first? Cheers! -Damian
On Sat, 09 Feb 2013 14:27:33 +0000, Damian Johnson wrote: ...
I cringe a bit to suggest it, but maybe a mapping in a mapping?
CELL_STATS PCircID=8 PConnID=47110 PAdded=created:1,relay:1 PRemoved=created:1,relay:1
You can as well go wild and use recursive syntax: CELL_STATS PCircID=8 PConnID=47110 PAdded=(created=1 relay=1) PRemoved=(created=1 relay=1) (not thinking about the implementation details, like when to match parens). Andreas -- "Totally trivial. Famous last words." From: Linus Torvalds <torvalds@*.org> Date: Fri, 22 Jan 2010 07:29:21 -0800
On Sun, Feb 10, 2013 at 7:08 AM, Andreas Krey <a.krey@gmx.de> wrote:
On Sat, 09 Feb 2013 14:27:33 +0000, Damian Johnson wrote: ...
I cringe a bit to suggest it, but maybe a mapping in a mapping?
CELL_STATS PCircID=8 PConnID=47110 PAdded=created:1,relay:1 PRemoved=created:1,relay:1
You can as well go wild and use recursive syntax:
CELL_STATS PCircID=8 PConnID=47110 PAdded=(created=1 relay=1) PRemoved=(created=1 relay=1)
(not thinking about the implementation details, like when to match parens).
Yeah, especially the latter part sounds painful. I'll leave this as Damian suggested it for now. Thanks, Karsten
On Sat, Feb 9, 2013 at 11:27 PM, Damian Johnson <atagar@torproject.org> wrote:
I wonder if we should avoid restricting connection types in the spec by defining this argument as connection type _string_ as opposed to an enumeration.
That depends on how volatile you think it'll be. If it's reasonably static then an enumeration would probably be best.
I don't know. I think it's somewhat more likely that we'll want to add more connection types than sticking with these three. Changed to a string to be more flexible.
Has the advantage that we don't have to touch the spec whenever we add another connection type. Does that make sense?
True, but that also means that event recipients have no idea what kind of values to expect or what they mean.
Right. Still, I think using a string is better here.
A fine question. Here are two examples of this event: ...
Ah, I see. That does make it trickier.
I'm not spotting any precedent for doing multiple sub-mappings in an event. Doing this in positional arguments is simple to parse, but beside the lack of flexibility mentioned earlier it's pretty hard to read.
I cringe a bit to suggest it, but maybe a mapping in a mapping?
CELL_STATS PCircID=8 PConnID=47110 PAdded=created:1,relay:1 PRemoved=created:1,relay:1
Looks fine.
Sure, sounds doable. Will fix that.
Thanks!
Want to suggest new event formats with keyword arguments to make them easier to parse by Stem and friends?
Positional arguments aren't harder to parse, just harder to read and more inflexible. I'd be happy to suggest spec alternatives if you want. Mind if we revise the proposal based on the above first?
Sure. Please find the revised proposal in branch proposal218 of my public torspec repository: https://gitweb.torproject.org/user/karsten/torspec.git/shortlog/refs/heads/p... Thanks, Karsten
On Mon, Feb 11, 2013 at 9:38 AM, Karsten Loesing <karsten@torproject.org> wrote: [...]
Sure. Please find the revised proposal in branch proposal218 of my public torspec repository:
https://gitweb.torproject.org/user/karsten/torspec.git/shortlog/refs/heads/p...
Merged into torspec master.
Sure. Please find the revised proposal in branch proposal218 of my public torspec repository:
https://gitweb.torproject.org/user/karsten/torspec.git/shortlog/refs/heads/p...
Hi Karsten. Looks good! Pushed some revisions to my proposal218 branch... https://gitweb.torproject.org/user/atagar/torspec.git/shortlog/refs/heads/pr... For the 'Per-circuit cell stats' section it would be nice if there was an explanation about what the 'N' and 'P' prefixes mean (it's described as "app-ward" and "exit-ward" but I haven't a clue what those mean). Cheers! -Damian
On 2/11/13 5:37 PM, Damian Johnson wrote:
Sure. Please find the revised proposal in branch proposal218 of my public torspec repository:
https://gitweb.torproject.org/user/karsten/torspec.git/shortlog/refs/heads/p...
Hi Karsten. Looks good! Pushed some revisions to my proposal218 branch...
https://gitweb.torproject.org/user/atagar/torspec.git/shortlog/refs/heads/pr...
Looks good. I'm fine with making ConnType an enumeration type, especially when controllers must tolerate unrecognized types (which I didn't know was common in control-spec.txt). But I wonder why we would handle CellType differently and not turn that into an enumeration, too. There are 13 connection types and 17 cell types in or.h, so not totally unreasonable to enumerate. Or even better, cell types are specified in Section 3 of tor-spec.txt which we could reference. Also, should we change CellType values to upper-case?
For the 'Per-circuit cell stats' section it would be nice if there was an explanation about what the 'N' and 'P' prefixes mean (it's described as "app-ward" and "exit-ward" but I haven't a clue what those mean).
The P and N come from the Tor code, and I guess they stand for next and previous. We can also call them AppWard and ExitWard, like in AddedAppWar or TimeExitWard, if that makes it easier to understand. Thanks, Karsten
But I wonder why we would handle CellType differently and not turn that into an enumeration, too.
Certainly could. I left the CellType alone (both leaving it as lowercase and a string definition) because it's a sub-mapping and hence we don't have any precedent for those. On reflection I agree with you, an uppercase enum would be more consistent.
The P and N come from the Tor code, and I guess they stand for next and previous. We can also call them AppWard and ExitWard, like in AddedAppWar or TimeExitWard, if that makes it easier to understand.
I'm not sure what 'previous' and 'next' are in reference to, and AppWard and ExitWard aren't any more illuminating. That said, I'm not the target audience for these events so if you think any of those are good descriptions for the users of these events then go for it. :)
I haven't looked at everything in proposal 218, but I would suggest maybe splitting the CELL_STATS into CELL_STATS_LOCAL and CELL_STATS_NETWORK: "650" SP "CELL_STATS_NETWORK" SP CircID SP ConnID SP Added SP Removed SP Time CRLF "650" SP "CELL_STATS_LOCAL" SP CircID SP ConnID SP Added SP Removed SP Time CRLF ...if only to make the definitions easier to read without the N- versus P- prefixed variable names. -- meejah
On Mon, Feb 11, 2013 at 3:35 PM, meejah <meejah@meejah.ca> wrote:
I haven't looked at everything in proposal 218, but I would suggest maybe splitting the CELL_STATS into CELL_STATS_LOCAL and CELL_STATS_NETWORK:
"650" SP "CELL_STATS_NETWORK" SP CircID SP ConnID SP Added SP Removed SP Time CRLF
"650" SP "CELL_STATS_LOCAL" SP CircID SP ConnID SP Added SP Removed SP Time CRLF
...if only to make the definitions easier to read without the N- versus P- prefixed variable names.
Or maybe add a "DIRECTION=" Direction field that can be "app" or "exit". (Controller event types are somewhat expensive, because there's a bitfield in Tor's code soon running out of bits.) The only information that we might miss when splitting events is which (P/N)CircIDs and (P/N)ConnIDs belong to the same circuit. I'll leave it to Rob to say whether we need that information. If not, splitting is an option. Thanks, Karsten
On Wed, Feb 6, 2013 at 11:14 AM, Karsten Loesing <karsten@torproject.org> wrote:
Hi everyone,
here's a proposal that defines three new controller events for the TestingTorNetwork mode that shall help us better understand connection and circuit usage in private Tor networks with the goal to make simulations more accurate.
And there's also code:
https://gitweb.torproject.org/karsten/tor.git/shortlog/refs/heads/morestats2
Feedback, to both proposal and code, much appreciated!
Added as proposal 218. Is there a trac ticket for this? -- Nick
On 2/7/13 5:45 PM, Nick Mathewson wrote:
On Wed, Feb 6, 2013 at 11:14 AM, Karsten Loesing <karsten@torproject.org> wrote:
Hi everyone,
here's a proposal that defines three new controller events for the TestingTorNetwork mode that shall help us better understand connection and circuit usage in private Tor networks with the goal to make simulations more accurate.
And there's also code:
https://gitweb.torproject.org/karsten/tor.git/shortlog/refs/heads/morestats2
Feedback, to both proposal and code, much appreciated!
Added as proposal 218.
Thanks!
Is there a trac ticket for this?
I think that would be either #7358 or #7359. Best, Karsten
participants (5)
-
Andreas Krey -
Damian Johnson -
Karsten Loesing -
meejah -
Nick Mathewson