[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: : Export END_CIRC_REASON_* to controler



On Thu, Oct 12, 2006 at 09:02:23PM -0500, Mike Perry wrote:
> Thus spake Nick Mathewson (nickm@xxxxxxxxxxxxx):
> 
> > On Mon, Oct 09, 2006 at 04:44:59AM -0500, Mike Perry wrote:
> > > Thus spake Mike Perry (mikepery@xxxxxxxxxx):
> > > 
> > > > Attached is a patch to export circuit failure reasons to the
> > > > controller. I had to convert several END_CIRC_AT_ORIGIN reason
> > > > codes to the real reason codes for calls to circuit_mark_for_close().
> > > 
> > > Woops. In a couple places I forgot to negate the reason codes.
> > 
> > Hi, Mike!  I've applied your patch, and tweaked it to take effect only
> > when extended events are enabled.  Thanks!
> 
> Ok, I've been doing some more testing and I noticed a couple things.
> 
> 1. If the circuit path is empty, there's a double space between
> FAILED/CLOSED and the REASON. Not sure how to best handle this. I
> tweaked my regex to work with single or double space (I think), so
> it doesn't matter to me, but it is sort of contrary to the spec.

Thanks; fixed.  (I hope; please try it out.)

> 2. There's a couple reasons in the source that I missed. New diff
> attached for those.

Thanks, I've (mostly) applied the patch.

I couldn't use all of it, since this isn't right:

> +      /* Prevent arbitrary destroys from going unnoticed by controller */
> +      if(reason == END_CIRC_AT_ORIGIN ||
> +              reason == END_CIRC_REASON_NONE ||
> +              reason == END_CIRC_REASON_REQUESTED) {
> +        reason = END_CIRC_REASON_OR_CONN_CLOSED;

According to tor-spec.txt, OR_CONN_CLOSED means:

     8 -- OR_CONN_CLOSED  (The OR connection that was carrying this
                           circuit died.)

But the code above is called when we get a DESTROY cell, and destroys
only tear down a single circuit, not the whole connection.

Probably, we should define more reasons; see some of the comments I
did for the patch in r8672 for other reasons that don't match up with
what the spec seems to say.

yrs,
-- 
Nick Mathewson

Attachment: pgpiafJmAy0h5.pgp
Description: PGP signature