Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#1793 closed defect (fixed)

Exit nodes send END_STREAM_REASON_MISC

Reported by: Sebastian Owned by:
Priority: Medium Milestone:
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Some digging showed that we're missing (WSA)EHOSTUNREACH. We might be missing more, but I didn't find anything else.

Child Tickets

Change History (11)

comment:1 Changed 9 years ago by Sebastian

Status: newneeds_review

A proposed fix for that is in misc-reason in my repository. Is END_STREAM_REASON_INTERNAL the right thing to send?

comment:2 Changed 9 years ago by Sebastian

Component: - Select a componentTor - Relay

comment:3 Changed 9 years ago by nickm

I don't think INTERNAL is the right reason; INTERNAL is for "something went wrong with the OR." But EHOSTUNREACH happens when IP packets can't get routed to to the host in question, which is probably not the OR's fault.

comment:4 Changed 9 years ago by Sebastian

So what is your suggestion? I considered the others, but not sure what we should pick. Should we make a new reason?

comment:5 Changed 9 years ago by nickm

Adding a new reason would probably make the most sense, unless you want to change the semantics of CONNRESET or something.

comment:6 Changed 9 years ago by Sebastian

Added a new commit to misc-reason. Would that work better?

comment:7 Changed 9 years ago by nickm

Looks good. A documentation issue:

  • control-spec.txt needs to document controller-visible stream reasons.

Grepping for things that look at reasons...

  • Should this reason be listed in edge_reason_is_retriable? [and should that function be in reasons.c?]
  • Should this reason be one of the cases in connection_ap_processs_end_not_open() ?

comment:8 Changed 9 years ago by Sebastian

Fixed the control-spec.txt.

I think we don't want to move edge_reason_is_retriable() into reasons.c, because it really is just a quick and short helper function for connection_ap_process_end_not_open() and that one needs to handle all the reasons that are specified in edge_reason_is_retriable() anyways.

I've added the END_STREAM_REASON_NOROUTE to the END_STREAM_REASON_MISC in connection_ap_process_end_not_open(). I think it's fine to piggyback there the same way as END_STREAM_REASON_TIMEOUT does.

comment:9 Changed 9 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Looks better to me.

it doesn't compile, though:
+ case END_STREAM_REASON_NOROUTE: "return "NOROUTE";

Fixing that and merging.

comment:10 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:11 Changed 7 years ago by nickm

Component: Tor RelayTor
Note: See TracTickets for help on using tickets.