Opened 7 months ago

Closed 4 months ago

#24989 closed defect (fixed)

Count hsdir requests against maxcircuitspending

Reported by: mikeperry Owned by: mikeperry
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: review-group-34, 034-triage-20180328
Cc: Actual Points:
Parent ID: Points:
Reviewer: mikeperry Sponsor:

Description

In #13837, we split off hsdir purposes from general. In the process, it means that we stopped counting hsdir circuits towards the global pending rate limit in count_pending_general_client_circuits(). That function was already trying to avoid counting hs circuits, but since nothing else globally limits the number of pending hs circuits, we probably should keep rate limiting hsdirs there, too.

Child Tickets

Change History (12)

comment:1 Changed 7 months ago by arma

See also the discussion in #24973

comment:2 Changed 7 months ago by mikeperry

Status: assignedneeds_review

https://oniongit.eu/mikeperry/tor/commits/bug24989. Note that I only added the client-side hsdir fetch purpose to this check. I figure dealing with the service side holistically in #24973 is the right way to deal with the service rate limiting.

comment:3 Changed 6 months ago by teor

Milestone: Tor: 0.3.4.x-final

Move all needs_review tickets without a release to 0.3.4

comment:4 Changed 6 months ago by nickm

Keywords: review-group-34 added

comment:5 Changed 5 months ago by dgoulet

Reviewer: asn

Reviewer week 03/19th

comment:6 Changed 5 months ago by asn

Patch LGTM. I pushed a branch bug24989 in my github repo (!): https://github.com/asn-d6/tor which is rebased to latest master, and also fixes the change entry a bit.

Mike please check it out, and let's put this in merge_ready.

comment:7 Changed 5 months ago by nickm

Keywords: 034-triage-20180328 added

comment:8 Changed 5 months ago by asn

Reviewer: asnmikeperry

comment:9 Changed 4 months ago by mikeperry

Status: needs_reviewmerge_ready

Ok asn's changes are good. (They are just changelog + doc fixes). Sorry for the delay!

comment:10 Changed 4 months ago by nickm

Should this go into 0.3.3 or 0.3.4?

comment:11 Changed 4 months ago by mikeperry

I think this should go in 0.3.3 since it restores behavior that #13837 disabled. I rebased it on to maint-0.3.3 in mikeperry/bug24989_033

comment:12 Changed 4 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Squashed and merging to 0.3.3 and forward!

Note: See TracTickets for help on using tickets.