Expose more circuit state-change events to controllers
We currently send CIRC events to controllers that request them when a circuit is created, extended by one hop, or finished. Now that we send detailed information about circuits' purposes and states in those events, we should allow controllers to ask for notification events when circuits' purposes or states change, too.
Activity
changed milestone to %Tor: 0.2.3.x-final
See feature3457-v2 (
git://git.torproject.org/rransom/tor.git feature3457-v2) for an implementation. This branch contains my feature2411-v3 (see #2411 (moved)) and bug3465-023-v2 (see #3465 (moved)) branches.Trac:
Status: new to needs_reviewSee feature3457-v2 (
git://git.torproject.org/rransom/torspec.git feature3457-v2) for documentation for the changes in my feature3457-v2 tor branch. This branch contains my bug2411a-v2 torspec (see #2411 (moved)) branch.
FYI: The PyTorCtl changes to support #2411 (moved) (in #3679 (moved)) appeared to mysteriously break the bw auths somehow (#4015 (moved))
See feature3457-v3 (
https://git.torproject.org/rransom/tor.git feature3457-v3) for a branch rebased onto current master. This branch contains my feature2411-v4 branch (see #2411 (moved)); bug3465-023-v2 was merged long ago.Replying to nickm:
Why do we need a new event type for this? It seems like something we could do just fine with the current CIRC events. We can't add new CircStatus entries (I think), but we can just send back the previous CircStatus with the new purpose.
I want to indicate to the controller what caused the event to be sent. CIRC2 events are also a bit ‘louder’ than CIRC events, and will become even louderer if we add an INTRODUCE CIRC2 event type to tell controllers of a hidden service when an INTRODUCE2 cell has been received.
We can't add new CircStatus values for CIRC events because (a) the spec doesn't specifically allow it, and (b) Vidalia won't handle them sensibly (and I don't know how any controller could). Adding a new event type seemed much better.
notes to myself for revisions to make:
- rename circuit_change_purpose to circuit_set_purpose for consistency
- Use smartlist_add_asprintf() in circuit_describe_status_for_controller
- Come up with a better name for circ2 if possible
- nuke the explicit enumeration-value setting in circuit_status_2_event_t
- For formatting time, "sec,usec" is pretty horrible and not what we do elsewhere in tor or in control-spec. Either ISOTime or some spaceless variant thereupon would be better. (in circuit_describe_status_for_controller()). If for some reason we can't use ISOTime (or some spaceless variant etc), then let's use an actual decimal fraction.
- void* arg2 is making me uncomfortable. If that's only a timeval for now, let's make it a timeval. If it really has to be lots of other things, let's make typechecking wrapper functions as needed.
rransom, do you have any suggestions for a better name for CIRC2?
Replying to nickm:
notes to myself for revisions to make:
- rename circuit_change_purpose to circuit_set_purpose for consistency
circuit_change_purposeisn't called (and shouldn't be) the first time a circuit's purpose is set.- Use smartlist_add_asprintf() in circuit_describe_status_for_controller
- Come up with a better name for circ2 if possible
- nuke the explicit enumeration-value setting in circuit_status_2_event_t
- For formatting time, "sec,usec" is pretty horrible and not what we do elsewhere in tor or in control-spec. Either ISOTime or some spaceless variant thereupon would be better. (in circuit_describe_status_for_controller()). If for some reason we can't use ISOTime (or some spaceless variant etc), then let's use an actual decimal fraction.
Torperf uses that format. I agree that it sucks, but I didn't know of any formatting functions already in Tor that would emit all the information in that time struct.
- void* arg2 is making me uncomfortable. If that's only a timeval for now, let's make it a timeval. If it really has to be lots of other things, let's make typechecking wrapper functions as needed.
rransom, do you have any suggestions for a better name for CIRC2?
CIRC_MINOR?CIRC_MINOR_EVENT?-
Replying to rransom:
Torperf uses that format. I agree that it sucks, but I didn't know of any formatting functions already in Tor that would emit all the information in that time struct.
How about ISOTime with the space replaced by a slash? eg, 2011-01-11/00:02:15.324128
- void* arg2 is making me uncomfortable. If that's only a timeval for now, let's make it a timeval. If it really has to be lots of other things, let's make typechecking wrapper functions as needed.
rransom, do you have any suggestions for a better name for CIRC2?
CIRC_MINOR?CIRC_MINOR_EVENT?CIRC_MINOR seems fine to me.
I'll do these changes tonight or tomorrow unless this is something you'd rather do.
Replying to nickm:
Replying to rransom:
Torperf uses that format. I agree that it sucks, but I didn't know of any formatting functions already in Tor that would emit all the information in that time struct.
How about ISOTime with the space replaced by a slash? eg, 2011-01-11/00:02:15.324128
The standard spaceless format uses a “T” there instead.
- void* arg2 is making me uncomfortable. If that's only a timeval for now, let's make it a timeval. If it really has to be lots of other things, let's make typechecking wrapper functions as needed.
rransom, do you have any suggestions for a better name for CIRC2?
CIRC_MINOR?CIRC_MINOR_EVENT?CIRC_MINOR seems fine to me.
I'll do these changes tonight or tomorrow unless this is something you'd rather do.
I'm not up to writing Tor patches tonight.
mentioned in issue #4003 (moved)
moved to tpo/core/tor#3457 (closed)