Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#2756 closed defect (fixed)

Send stream-level sendme when outbuf is low: don't wait for it to empty!

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


Right now we consider sending an stream-level SENDME under two conditions:

  • We get a data cell on that stream.
  • We have completely flushed the outbuf for that stream.

But suppose that the sender has exhausted its quota of cells. We actually are _willing_ to send a SENDME cell before we've flushed everything down to 10*RELAY_PAYLOAD_SIZE (and we should, for throughput). So we should add another check that happens when we flush data, to see if we have drained enough to send a stream-level relay cell.

Child Tickets

Change History (7)

comment:1 Changed 8 years ago by nickm

Status: newneeds_review

Please review branch bug2756_relay in my public repository.

comment:2 Changed 8 years ago by arma

The function header for connection_edge_consider_sending_sendme() needs to be edited too.

I notice that connection_edge_flushed_some() and connection_edge_finished_flushing() both check about sending a sendme now. And they're called in basically the same places. The connection_edge_consider_sending_sendme() logic looks pretty light though, so I don't mind.

Is the whole switch infrastructure needed in connection_edge_flushed_some()? We want to check about a sendme for all types of edge conns. I notice in connection_or_flushed_some() we don't check whether it's an OR conn.

comment:3 Changed 8 years ago by nickm

Edited the doxygen comment (assuming that's what you mean by "header")

I'd like to make finished_flushing and flushed_some less redundant, but that seems to me like an orthogonal thing to do in 0.2.3.x.

We aren't in fact switching based on the *type* of the connection in connection_edge_flushed_some(); we're switching on its state. If the connection isn't open, we don't want to send sendmes (because if it's an an AP conn that isn't open, we don't have anywhere to send them, and if it's an exit connection that isn't open, we shouldn't be sending sendme cells before we send the connected cell).

Good to merge now?

comment:4 Changed 8 years ago by arma

Looks fine.

comment:5 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

merged it. thanks!

comment:6 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:7 Changed 7 years ago by nickm

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