Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#1937 closed defect (fixed)

circuit_resume_edge_reading_helper is highly unfair

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: #1298 Points:
Reviewer: Sponsor:

Description

See comments on #1298, particularly Roger's:

For example, notice how circuit_resume_edge_reading_helper() always walks the stream list in the same order, and stops when it's exhausted the circuit's package window. Whenever that "stop when package window is empty" case gets triggered, we're being unfair to the streams later in the linked list

And my:

That issue is also relevant, and worth fixing. Here are a couple of ways we could do it:

  • We could only package a limited number of cells per stream at a time, and loop until all input data is packaged, or the queue is full.
  • We could start at a different position in the list of streams each time.
  • Some combination of the above.

I think the answer is "some combination of the above", since the first one still technically favors older streams, and the second one (though technically fair) can produce "fairness" of a really choppy type.

Child Tickets

Change History (6)

comment:1 Changed 9 years ago by nickm

Status: newaccepted

comment:2 Changed 9 years ago by nickm

Status: acceptedneeds_review

Possible fix in branch bug1937.

comment:3 Changed 9 years ago by arma

Looks good to me, modulo the suggestions on irc.

I wonder if there are other cases where we call connection_edge_package_raw_inbuf() without constraining how many cells it can package, and it gobbles up more than it should.

comment:4 Changed 9 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Fix merged to master. Closing; copied arma's comment above to the parent bug #1298.

comment:5 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:6 Changed 7 years ago by nickm

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