#8494: Does MaxAdvertisedBandwidth do anything useful and if not, can we deprecate it? -------------------------------------------------+------------------------- Reporter: alphawolf | Owner: juga Type: defect | Status: | assigned Priority: Low | Milestone: Tor: | 0.3.5.x-final Component: Core Tor/Tor | Version: Severity: Normal | Resolution: Keywords: tor-spec, consensus, bandwidth, | Actual Points: MaxAdvertisedBandwidth tor-relay tor-dirauth | needs-insight tor-bwauth | Parent ID: #25925 | Points: Reviewer: | Sponsor: -------------------------------------------------+-------------------------
Comment (by teor): Replying to [comment:16 juga]: > Replying to [comment:15 teor]: > > This spec tells us what a bandwidth generator needs to do to be compatible with the Tor network. > > Sometimes it is useful to give examples of how sbws or torflow implements a feature. > > But first we need to say what *any* generator must do. Then we can give examples. > > ACK > > > > > So let's start with a sentence like this: > > > > If a relay sets MaxAdvertisedBandwidth, generators MUST give it a similar weight to relays with that bandwidth capacity. > > Replying to [comment:13 juga]: > > > Bandwidth terms are different in `dir-spec.txt` and the code. > > > If i'm not mistaken, what is called `bandwidthrate` [0] in the code when creating the descriptor, is called `bandwidth-avg` in `dir-spec.txt` [1]. > > > And it is calculated as the min(BandwidthRate, MaxAdvertisedBandwidth, RelayBandwidthRate) [2]. > > Is this correct or i'm mistaken? > bandwidth-avg is min(BandwidthRate, MaxAdvertisedBandwidth, RelayBandwidthRate, BandwidthBurst, RelayBandwidthBurst) > > > If this is correct, the paragraph to add in `bandwidth-file- spec.txt` could be: > > > > > > sbws limits the relay's measured bandwidth to the bandwidth-avg advertised > > > > Please say "Bandwidth generators MUST limit the relay's weight based on", not "sbws limits the relay's measured bandwidth to". > > > > > in the relay's descriptor, which is the minimum between BandwidthRate, > > > MaxAdvertisedBandwidth and RelayBandwidthRate in the relay's configuration. > > > > > > Though probably the last sentence not needed. > > > > I think we need the last sentence, because we need to explain why bandwidth generators need to implement MaxAdvertisedBandwidth. > > > > Let's also say: > > > > Relays limit their bandwidth when BandwdithRate or RelayBandwidthRate are set. These options reduce a relay's bandwidth capacity. But MaxAdvertisedBandwidth doesn't change the relay's bandwidth capacity. Instead, it asks the bandwidth generator to limit the weight of the relay as if the relay's measured bandwidth was min(measured bandwidth, MaxAdvertisedBandwidth). > > Generators SHOULD NOT limit weights based on bandwidth-observed, because that penalises new relays. > > > > In all your suggestions, i would not use the word `weight` to do not confuse it with the `weight` generated in the consensus. When I say "weight", I mean "the weight generated in the consensus", rather than "the bandwidth measured for the relay". Some generators don't measure relay bandwidth. For example, peerflow aggregates a relay's observed bandwidths from other relays. That's why I said "consensus weight". But we are not making that distinction in the spec, so we can just say "measured bandwidth". > I also would not use the word `capacity` to do not confuse it with the `bandwidth-observed` in the descriptors that it's called `bandwidthcapacity` in the code and comes from relays' self-tests. When I said "capacity", I mean "the total amount of bandwidth the relay can handle". I didn't want to say "measured bandwidth", because that's "the extra amount of bandwidth the relay can handle". But we're not making that distinction in the spec either, so we can just say "measured bandwidth" here as well. > > > Then we can give an example of what sbws does: > > > > sbws measures relay bandwidths, then caps the measured bandwidth using bandwidth-avg (MaxAdvertisedBandwidth). > > as mentioned above, isn't bandwidth-avg = min(MaxAdvertisedBandwidth, MaxAdvertisedBandwidth, RelayBandwidthRate)? No, see above. > > > If Torflow is using also `bandwidth-avg`, then it could also be added in `bandwidth-file-spec.txt`: > > > > > > Torflow does not need to limit the relay's measured bandwidth since it > > > partions relays to be measured by bandwidth-avg > > > > > > Is this correct?. > > > > No, Torflow partitions based on: > > * bandwidth-consensus, if available, or > > * min(bandwidth-avg, bandwidth-burst, bandwidth-observed), or > > * 1, if either bandwidth is zero. > > > > https://gitweb.torproject.org/pytorctl.git/tree/TorCtl.py#n376 > > https://gitweb.torproject.org/pytorctl.git/tree/TorCtl.py#n459 > > > > Torflow also calculates weights based on the descriptor bandwidth to measured bandwidth ratio. > > > > So let's say: > > > > Torflow partitions relays based on their bandwidth. For unmeasured relays, Torflow uses the minimum of all descriptor bandwidths, including bandwidth-avg (MaxAdvertisedBandwidth) > > as mentioned above, isn't bandwidth-avg = min(MaxAdvertisedBandwidth, MaxAdvertisedBandwidth, RelayBandwidthRate)? See above. > > and bandwidth-observed. Then Torflow measures the relays in each partition against each other, which implicitly limits a relay's measured bandwidth to the bandwidths of similar relays. > > > > Torflow also generates consensus weights based on the ratio between the measured bandwidth and the minimum of all descriptor bandwidths (at the time of the measurement). So when an operator reduces the MaxAdvertisedBandwidth for a relay, Torflow reduces that relay's measured bandwidth. > > > > So, sumarizing your 4 sentences suggestions with my comments, plus what you clarify about Torflow: Remember, the spec says *what* a bandwidth generator needs to do. For complicated tasks, we also say *why* and *how*. Because MaxAdvertisedBadwidth is complicated, I don't want to leave these details out. > Bandwidth generators MUST limit the relays' measured bandwidth based on the descriptors' bandwidth-avg. I want to say "MaxAdvertisedBandwidth" in the first sentence, because I want to say *why* we need to take these extra steps to limit the consensus weight. Then I want to give the details of *how* MaxAdvertisedBandwidth works in descriptors right now. If we say "bandwidth-avg", we lose the connection between generator bandwidth limits and MaxAdvertisedBandwidth. Then I want to explain why MaxAdvertisedBandwidth needs to be implemented on bandwidth generators, rather than in Tor itself. > Descriptors' bandwidth-avg is the minimum between BandwidthRate, MaxAdvertisedBandwidth and RelayBandwidthRate in the relays' configuration, when these options are set. I hunk we should rewrite this sentence to say: A relay's MaxAdvertisedBandwidth affects the bandwidth-avg in its descriptor. bandwidth-avg is the minimum of MaxAdvertisedBandwidth, BandwidthRate, RelayBandwidthRate, BandwidthBurst, and RelayBandwidthBurst. > Generators SHOULD NOT limit measured bandwidths based on descriptors' bandwidth-observed, because that penalises new relays. I agree. > When generators does not limit the relays' measured bandwidth on the descriptors' bandwidth-avg, they MUST give a similar measured bandwidth to the consensus bandwidth or the minimum between the descriptor's bandwidth- avg, bandwidth-burst or bandwidth-observed. This sentence allows the Torflow behaviour, which is buggy, because it penalises new relays. And it conflicts with the "SHOULD NOT" in the sentence before. if you delete this sentence, the spec will be less confusing. You can give the sbws and Torflow implementations as examples if you want. > Though i'm still not sure this is correct. I could just copy what you propose, but not sure about `capacity` and `weight` terms, as commented aboved. You're right, we're not being that specific. > As a different issue to solve at some point in the future, i think we should unify terms for bandwidth in Tor code and `dir-spec.txt` to make it less confusing. It'd be also helpful to add formulae or more detailed descriptions on how the different bandwidth terms are generated in `dir- spec.txt` Please open another ticket for changes to other specs. What do you think about these subsections? > > No Zero Bandwidths > > Bandwidth Aggregation > > Bandwidth Scaling > > MaxAdvertisedBandwidth -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/8494#comment:17> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs