Opened 4 months ago

Last modified 4 months ago

#33097 assigned defect

outbuf_flushlen seems to serve no purpose

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

Description

The "outbuf_flushlen" field in connection_t seems to not actually do anything that's distinct from buf_datalen(conn->outbuf)... except possibly, to get out of sync with it under rare conditions? (eg #32472).

Flushlen once existed to implement rate limiting: it was introduced in 117cbeeaaf30cdb before we even had round robining. Later there was some logic involved with f5ebf4c712d693c to try to flush full TLS records. And controller connections used ab838bddb89f to force early flushing there... but right now, we don't flush according to the same logic that we used to flush, and I think outbuf_flushlen is now obsolete.

I think there's a case to be made for removing this field in 0.4.3, but I'd like to be cautious.

Adding arma to cc in case he can remember what outbuf_flushlen is for.

Child Tickets

Change History (1)

comment:1 Changed 4 months ago by arma

outbuf_flushlen is for exactly what you described: "how much of the outbuf should we attempt to flush now?" The outbuf could be arbitrary-sized, for example if a pile of cells came in but the corresponding place to send them to is blocked. And so when that destination unblocks, we don't just want to write the entire outbuf to the network right then, because it could be unfair to the other places that also want to write, especially if it uses up our write tokens and we stop wanting to flush anything else.

If we now do all of that using other logic, sounds great.

Note: See TracTickets for help on using tickets.