[tor-bugs] #28588 [Core Tor/sbws]: SBWS 'bw_torflow_scale' does not appear to honor relay MaxAdvertisedBandwidth

[Tor Bug Tracker & Wiki]

#28588: SBWS 'bw_torflow_scale' does not appear to honor relay
MaxAdvertisedBandwidth
---------------------------+-------------------------------------
 Reporter:  starlight      |          Owner:  (none)
     Type:  defect         |         Status:  new
 Priority:  Medium         |      Milestone:  sbws 1.0 (MVP must)
Component:  Core Tor/sbws  |        Version:  sbws: 1.0.0
 Severity:  Normal         |     Resolution:
 Keywords:                 |  Actual Points:
Parent ID:                 |         Points:
 Reviewer:                 |        Sponsor:
---------------------------+-------------------------------------
Changes (by teor):

 * version:  sbws: unspecified => sbws: 1.0.0
 * milestone:   => sbws 1.0 (MVP must)


Comment:

 This bug needs to be fixed before we deploy any more bwauths.

 Replying to [comment:2 juga]:
 > Replying to [ticket:28588 starlight]:
 > > Examining the current 1.0.2 branch it does not seem bw_torflow_scale()
 honors MaxAdvertisedBandwidth.
 >
 > If i understood it correctly, torflow does not use the descriptor
 average-bandwidth, but observed-bandwidth. Is that correct?.

 pytorctl's bw_observed is the minimum of all the bandwidths on the
 bandwidth line.
 It is called bw_observed, but that is a confusing name.
 {{{
       elif kw == "bandwidth":
         bws = map(int, g)
         bw_observed = min(bws)
         rate_limited = False
         if bws[0] < bws[1]:
           rate_limited = True
 }}}
 https://gitweb.torproject.org/pytorctl.git/tree/TorCtl.py#n459

 But sbws' bw_observed is just stem's observed_bandwidth:
 {{{
     @property
     def relay_observed_bandwidth(self):
         return self._relay.observed_bandwidth
 }}}
 https://github.com/torproject/sbws/blob/5a88f749e15764f08bf5b40b46db2a655d12d2b5/sbws/lib/resultdump.py#L229

 When sbws should be using min(average_bandwidth, burst_bandwidth,
 observed_bandwidth):
 {{{
 average_bandwidth (int) -- * average rate it's willing to relay in bytes/s
 burst_bandwidth (int) -- * burst rate it's willing to relay in bytes/s
 observed_bandwidth (int) -- * estimated capacity based on usage in bytes/s
 }}}
 https://stem.torproject.org/api/descriptor/server_descriptor.html#stem.descriptor.server_descriptor.ServerDescriptor

 The MaxAdvertisedBandwidth check was implemented in sbws scaling in:
 https://github.com/torproject/sbws/issues/155
 https://github.com/torproject/sbws/pull/191
 https://trac.torproject.org/projects/tor/ticket/8494

 Then removed in commit 514671e:
 https://github.com/torproject/sbws/commit/514671efb4ccc4f9b63dad0f5373891f7f27b990

 The MaxAdvertisedBandwidth check was never implemented in torflow scaling.

 There are no unit tests for MaxAdvertisedBandwidth.
 And the integration test didn't catch this bug:
 https://github.com/torproject/sbws/blob/master/tests/integration/core/test_scanner.py#L21

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