[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:                    
-------------------------+--------------------------------------------------

Comment(by Flo):

 Replying to [comment:8 nickm]:

 >   * 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)'.

 I definitly see your point. My pragmatic approach to solve this issue
 would be to restrict precision in which users can specify the rate and
 burst parameter to a magnitude of kilobytes. By doing so we avoid round-
 offs in the calculations because in this case rate % 1000 = 0. In my
 opinion a higher precision than kilobyte doesn't make sense to me anyway.
 Does this sound feasible to you?

 >   * 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.

 If I got you right, you would like to see control_event_bandwidth_used()
 and control_event_stream_bandwidth_used() also decoupled from the
 refill_callback. Consider it as done.

 >   * 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.

 I changed it to tor_gettimeofday() as suggested.

 >   * The configuration should probably disallow any refill interval of
 greater than 1000 msec.

 Done.

 >   * 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".

 You are absolutely right. Changed that too.


 I added an revised version of the patch to this ticket, which considers
 your comments. Just let me know about the round-offs and I will change
 that too.

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