Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#1740 closed defect (fixed)

Circuit build timeout code is recording cannibalized circ times

Reported by: mikeperry Owned by: mikeperry
Priority: High Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version: Tor: 0.2.2.x-final
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The circuit build timeout learning code is recording circ build times for 4 hop hidden service and other cannibalized circuits. This can lead to incorrect timeout data and log messages like: "Strange value for circuit build time" and "Extremely large value for circuit build timeout".

Child Tickets

Change History (14)

comment:1 Changed 9 years ago by mikeperry

Component: - Select a componentTor - Tor client

comment:2 Changed 9 years ago by nickm

Milestone: Tor: 0.2.2.x-final

comment:3 Changed 9 years ago by nickm

Owner: set to mikeperry
Status: newassigned

comment:4 Changed 9 years ago by mikeperry

Status: assignedneeds_review

See mikeperry/cbt-hs-and-controlport-fixes.

comment:5 Changed 9 years ago by mikeperry

Status: needs_reviewassigned

Actually, I just saw the update to #1772. I was under the impression that one hop circs did not go through these codepaths. If so, we need to rethink this brancg before merging, I think.

comment:6 Changed 9 years ago by arma

Priority: normalmajor

Triage: this bug is blocking 0.2.2.x-rc. I experience it many times an hour.

comment:7 Changed 9 years ago by mikeperry

Status: assignedneeds_review

Fix should be in mikeperry/bug1740 now. Should be ready for merge, handles the 1 hop case.

comment:8 Changed 9 years ago by arma

arma/bug1740 cleans it up some

comment:9 Changed 9 years ago by Sebastian

I agree that this fixes the case where we count cannibalized circs, but I don't think counting three and four-hop circuits, averaging the result and applying it to both types of circuits is what we want. We should maybe weight it somehow for the circuit length, or not count 4-hop timeouts and allow 4-hops 1 1/3 our timeout to complete

comment:10 Changed 9 years ago by mikeperry

Lets see what nick weighs in on this. Its an easy change to make. We just drop the commit that says "Nick wants us to count all circuits" and then change the ratios in circuit_expire_building().

I do think the 4/3 plan is a pretty good one.

comment:11 Changed 9 years ago by mikeperry

Ok, this should be all updated and merged in mikeperry/bug1740, including arma's changes.

I did a lot of rebasing on that to get it right. Please check it closely for git fail ;)

comment:12 Changed 9 years ago by arma

<nickm> 1740: looks good. Do we care about 5-hop or higher circuits launched by controllers? I say "no".

I agree.

comment:13 Changed 9 years ago by arma

Resolution: fixed
Status: needs_reviewclosed

Merged.

comment:14 Changed 7 years ago by nickm

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