[tor-bugs] #3630 [Tor Relay]: Reduce token bucket refill interval

[Tor Bug Tracker & Wiki]

#3630: Reduce token bucket refill interval
-------------------------+--------------------------------------------------
 Reporter:  Flo          |          Owner:                    
     Type:  enhancement  |         Status:  needs_review      
 Priority:  major        |      Milestone:  Tor: 0.2.3.x-final
Component:  Tor Relay    |        Version:                    
 Keywords:               |         Parent:                    
   Points:               |   Actualpoints:                    
-------------------------+--------------------------------------------------
Changes (by nickm):

  * priority:  normal => major


Comment:

 Reading through the patch, it looks mostly good.  There's some stuff that
 should get fixed up (either you could do that or I could get to it
 eventually), and a few bigger issues to think about.

 Big stuff the needs to get fixed pre-merge:
   * The round-offs with the division when doing refills seem pretty
 broken.  Consider that most of of the calculations for "bandwidthrate"
 make a new "rate per millisecond" by calculating rate / 1000.   This means
 that the real rate of bytes per second you'll see is not the rate you
 configured, but 'rate - (rate % 1000)'.
   * By specification, controller bandwidth events must be sent at about a
 once/second rate.  We shouldn't be flooding them at the refill interval
 rate, as this patch makes us do when running in non-bufferevent mode.

 Answers to questions:
   * Libevent doesn't currently launch any callbacks in parallel.  If it
 gains the ability to do so in the future, it won't do so by default unless
 it is told to do so.
   * The right way to find out the time in a timer callback is not
 gettimeofday() [windows doesn't have that].  Nor does it involve looking
 at periodic_timer_t.tv: that's only present when we're building with
 Libevent versions before 2.0.  Instead, either use tor_gettimeofday() or
 event_base_gettimeofday_cached(), depending on how much accuracy you need.

 Smaller stuff, easier to fix:
   * The configuration should probably disallow any refill interval of
 greater than 1000 msec.

 Stuff I don't understand:
   * What is going on with the revised burst calculations?  It seems to be
 that the burst should remain unchanged: it is the maximum number of bytes
 in the bucked, regardless of how often the bucked refills.  Yet lots of
 places in the new connection.c do something like "burst = configured_burst
 / 1000 * RefillInterval".

 Stuff that can get fixed after the merge:
   * Whatever this option is, it should apply to bufferevents as well as
 older token buckets.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/3630#comment:8>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online