<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div></div><div>Hi,</div><div><br></div><div>Thanks for writing this draft spec.</div><div><br></div><div>Please see my suggested changes below:</div><div><br>On 17 Apr 2018, at 21:23, juga <<a href="mailto:juga@riseup.net">juga@riseup.net</a>> wrote:<br></div><blockquote type="cite"><div><span></span><br><span>Hi,</span><br><span></span><br><span>as commented with teor and pastly, i send in-line a draft specification</span><br><span>for the document format that the bandwidth scanner implementations</span><br><span>should produce.</span><br><span></span><br><span>I've left my own questions/notes in square brackets.</span><br><span></span><br><span>Thanks,</span><br><span>juga.</span><br><span></span><br><span>=======================================</span><br><span></span><br><span> Tor Bandwidth Measurements Document Format</span><br><span> [juga: which name should we give to this document?]</span><br></div></blockquote><div><br></div><div>That's a fine name.</div><div>You can leave out the "Document" if you want.</div><br><blockquote type="cite"><div><span>1. Scope and preliminaries</span><br><span></span><br><span> This document describes the format of Tor's bandwidth measurements</span><br><span> document, version X.X.X [juga: which version should be this?]</span><br></div></blockquote><div><br></div><div>It doesn't matter, so let's use semantic versioning:</div><div>* the original torflow format was 1.0.0</div><div>* the format in this spec adds the "version" feature, so it is 1.1.0</div><div> (it is compatible with 1.0.0, as long as parsers ignore unrecognised</div><div> lines)</div><br><blockquote type="cite"><div><span> and later.</span><br><span></span><br><span> Since Tor version X.X.X [juga: which tor version?]</span></div></blockquote><div><br></div><div>It looks like 0.2.4.12-alpha added measured bandwidths</div><div><a href="https://gitweb.torproject.org/tor.git/tree/ChangeLog#n12710">https://gitweb.torproject.org/tor.git/tree/ChangeLog#n12710</a></div><br><blockquote type="cite"><div><span> the directory</span><br><span> authorities use the bandwidth measurements document called</span><br><span> "V3BandwidthsFile" and produced by Torflow [1]</span><br><span> (format described in README.spec.txt [2]).</span><br><span></span><br><span> The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL</span><br><span> NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and</span><br><span> "OPTIONAL" in this document are to be interpreted as described in</span><br><span> RFC 2119.</span><br><span></span><br><span>1.2. Acknowledgements</span><br><span></span><br><span> The original bandwidth measurement scanner (Torflow) and format was</span><br><span> created by mike. Teor suggested to write this specification while</span><br><span> contributing on pastly's new bandwidth scanner implementation.</span><br><span></span><br><span> This specification was revised after feedback from:</span><br><span></span><br><span> XXX</span><br><span></span><br><span>1.3 Outline</span><br><span></span><br><span> The bandwidth measurements mentioned in sections 3.4.1 and 3.4.2</span><br><span> of dir-spec.txt [3] are obtained by bandwidth authorities, which are</span><br><span> either directory authorities or other servers running bandwidth</span><br><span> measurement scanners and sending the results to the former.</span><br><span> [juga: it seems that bandwidth authorities have not been formally</span><br><span> before]</span><br></div></blockquote><div><br></div><div>You could use the definition in the man page:</div><div>"<span style="background-color: rgba(255, 255, 255, 0);">the bandwidth-authority generated file storing information on</span></div><div><span style="background-color: rgba(255, 255, 255, 0);">relays' measured bandwidth capacities"</span></div><div><br></div><blockquote type="cite"><div><span>2. Format details</span><br><span></span><br><span> Bandwidth measurements MUST contain the following sections:</span><br><span> - Header (exactly once)</span><br><span> - Relays measurements (zero or more times)</span><br><span></span><br><span> Each section (or entry) ends with a separator.</span><br></div></blockquote><div><br></div><div>This line is a copy-paste error, it should be deleted.</div><br><blockquote type="cite"><div><span>2.1. Nonterminals</span><br><span></span><br><span> The following nonterminals are defined in the Onionoo details</span><br><span> document specification [4]:</span><br><span></span><br><span> fingerprint</span><br><span> nickname</span><br></div></blockquote><div><br></div><div>This file format gets the fingerprint and nickname from the</div><div>consensus, so you should reference dir-spec.txt.</div><div><br></div><div>(dir-list-spec.txt gets relay fingerprints and nicknames from</div><div>Onionoo. That's why it uses the Onionoo definitions.)</div><div><br></div><div>Here are the definitions of hexdigest (fingerprint) and nickname:</div><div><a href="https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n1268">https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n1268</a></div><br><blockquote type="cite"><div><span> In the bandwidth measurement documents nickname is optional.</span><br></div></blockquote><div><br></div><div>"optional" is not relevant in a definition.</div><div>Let's delete this line, it's already documented as optional later on.</div><div><br></div><blockquote type="cite"><div><span> The following nonterminals are defined in the in dir-spec.txt:</span><br><span></span><br><span> NL (newline)</span><br><span> SP (space)</span><br><span></span><br><span> "bw" = INT, the aggregated measured bandwidth of this relay, in</span><br><span> kilobytes per second.</span><br></div></blockquote><div><br></div><div>bw is not defined in dir-spec.txt. And the formatting is confusing.</div><div>Double quotes are used for ASCII literal strings in dir-spec.txt.</div><div>Can you please follow the format used in dir-spec.txt?</div><div><br></div><div>Here is one example:</div><div><a href="https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n210">https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n210</a></div><div><br></div><div>Here's how you can define bw using the Int definition from</div><div>dir-spec.txt:</div><div><a href="https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n795">https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n795</a></div><div><br></div><div>bw = Int</div><div><span style="background-color: rgba(255, 255, 255, 0);"><br></span></div><div><span style="background-color: rgba(255, 255, 255, 0);">bw is the aggregated measured bandwidth of this relay, in </span><span style="background-color: rgba(255, 255, 255, 0);">kilobytes</span></div><div><span style="background-color: rgba(255, 255, 255, 0);">per second.</span></div><br><blockquote type="cite"><div><span> We introduce the following nonterminals:</span><br><span> [juga: this should probably be defined more formally and should</span><br><span> probably link to other documents, which ones?]</span><br></div></blockquote><div><br></div><div>dir-spec.txt</div><br><blockquote type="cite"><div><span> "version" = The name and the version of the bandwidth scannner</span><br><span> software, such as "sbws 0.1.0".</span><br></div></blockquote><div><br></div><div><div><span style="background-color: rgba(255, 255, 255, 0);">Our newest spec uses "version" for the file format version:</span></div><div><a href="https://gitweb.torproject.org/torspec.git/tree/dir-list-spec.txt#n148" style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);"><font color="#000000">https://gitweb.torproject.org/torspec.git/tree/dir-list-spec.txt#n148</font></a></div></div><div><br></div>So please don't make a field with a different meaning and structure,<div>and call it "version".<div><div><br></div><div>I suggest:</div><div>* use "version" for the file format version (or don't use "version")</div><div>* use "source" for the implementation software name and version</div><div><br></div><div><span style="background-color: rgba(255, 255, 255, 0);">Please fix the formatting of this definition to be like dir-spec.txt.</span></div><div>This definition has two arguments separated by spaces, the name,</div><div>and the version.<br><br><blockquote type="cite"><div><span> The name of the software, if absent, is assumed to be "torflow".</span><br><span> [juga: which should be the version if absent?]</span><br></div></blockquote><div><br></div><div><div><span style="background-color: rgba(255, 255, 255, 0);">"if absent" is not relevant in a definition.</span></div><div><span style="background-color: rgba(255, 255, 255, 0);">Let's move these lines to the header section.</span></div></div><div><br></div><div>The software version should be optional.</div><div>Torflow does not have a version, so we cannot require a version.</div><br><blockquote type="cite"><div><span> "timestamp" = INT, the Unix Epoch time when the file was created.</span><br></div></blockquote><div><br></div><div><span style="background-color: rgba(255, 255, 255, 0);">Please fix the formatting of this definition to be like dir-spec.txt.</span></div><br><blockquote type="cite"><div><span>2.2. Header format</span><br></div></blockquote><div><br></div><div>We should say if order matters.</div><div>We should say how new items get added to the header.</div><div><span style="background-color: rgba(255, 255, 255, 0);">(We could say that parsers MUST ignore unrecognised lines.)</span></div><br><blockquote type="cite"><div><span> It MUST consists of:</span><br><span></span><br><span> "timestamp" timestamp NL</span><br><span> "version" version NL</span></div></blockquote><div><br></div><div>The sbws sample data has:</div><div><div><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">1523911758<br>version=0.1.0</span></font></div></div><div><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);"><br></span></font></div><div><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">The first line does not have a "timestamp" string literal.</span></font></div><div><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">The second line has an equals sign.</span></font></div><div><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">The second line is optional (see the torflow sample data).</span></font></div><div><br></div><div>Does Tor ignore the version line?</div><div>If it does, we should document it.</div><br><blockquote type="cite"><div><span>2.3. Relay measurements format</span></div></blockquote><div><br></div><div>You should say that order on a line doesn't matter, and relay order</div><div>also doesn't matter.</div><br><blockquote type="cite"><div><span> Relays measurements MUST consist of the following items.</span><br><span></span><br><span> "node_id" fingerprint SP</span><br><span> "bw" bandwidth SP</span><br></div></blockquote><div><br></div><div>The format has equals signs, but this definition does not.</div><br><blockquote type="cite"><div><span> When there are no more items, the "bw" item ends with NL instead of</span><br><span> SP.</span><br></div></blockquote><div><br></div><div>It might be easier to say that each line allows extra arguments, and</div><div>reference the dir-spec.txt definition:</div><div><a href="https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n261">https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n261</a></div><div><br></div><div>But does each argument need an equals sign?</div><br><blockquote type="cite"><div><span>2.4. Optional extra items</span><br><span></span><br><span> Different implementations of the bandwidth measurements scanners MAY</span><br><span> include other items per relay.</span><br><span></span><br><span> For instance, sbws includes:</span><br><span></span><br><span> "rtt" = INT, Round Trip Time (to obtain 1B)</span><br></div></blockquote><div><br></div><div>This definition belongs in the definition section.</div><span style="background-color: rgba(255, 255, 255, 0);">Please fix the formatting of this definition to be like dir-spec.txt.</span><div><br><div><blockquote type="cite"><div><span></span><span> Every relay measurement in sbws consists of:</span><br><span></span><br><span> "node_id" fingerprint SP</span><br><span> "bw" bandwidth SP</span><br></div></blockquote><div><br></div><div><span style="background-color: rgba(255, 255, 255, 0);">The format has equals signs, but this definition does not.</span></div><div><span style="background-color: rgba(255, 255, 255, 0);"><br></span></div><div><span style="background-color: rgba(255, 255, 255, 0);">The fingerprints in the sample data have $ signs.</span></div><div>Does Tor require them? Or are they optional?</div><div>We should document it either way.</div><br><blockquote type="cite"><div><span> "nick=" nickname SP</span><br><span> "rtt=" rtt SP</span><br><span> "time=" timestamp NL</span><br></div></blockquote><div><br></div><div>The equals signs are correct here.</div><br><blockquote type="cite"><div><span> Every relay measurement in Torflow consists of:</span><br><span></span><br><span> "node_id" fingerprint SP</span><br><span> "bw" bandwidth SP</span><br></div></blockquote><div><br></div><div><span style="background-color: rgba(255, 255, 255, 0);">The format has equals signs, but this definition does not.</span></div><br><blockquote type="cite"><div><span> "nick=" nickname SP</span><br><span> "measured_at=" slice timestamp NL</span><br></div></blockquote><div><br></div><div><span style="background-color: rgba(255, 255, 255, 0);">slice is not defined, just use "timestamp", then explain using</span></div><div><span style="background-color: rgba(255, 255, 255, 0);">the next line.</span></div><div><br></div><blockquote type="cite"><div><span> The "measured_at" does not correspond to the "time" in sbws.</span><br></div></blockquote><div><br></div><div>Is it worth explaining the difference?</div><br><blockquote type="cite"><div><span> [juga: actually, if bwauths use "measured_at", then the code on them</span><br><span> or sbws should be changed].</span><br></div></blockquote><div><br></div><div>Tor does not contain the string "measured_at":</div><div><a href="https://github.com/torproject/tor/search?q=measured_at">https://github.com/torproject/tor/search?q=measured_at</a></div><div><br></div><div><span style="background-color: rgba(255, 255, 255, 0);">For consistency, please remove "measured_at", or add "updated_at".</span></div><br><blockquote type="cite"><div><span> Torflow includes other items that are out of the scope of this</span><br><span> document.</span><br></div></blockquote><div><br></div><div>We should think about which torflow fields are worth documenting.</div><br><blockquote type="cite"><div><span>References:</span><br><span></span><br><span>1. <a href="https://gitweb.torproject.org/torflow.git">https://gitweb.torproject.org/torflow.git</a></span><br><span>2.</span><br><span><a href="https://gitweb.torproject.org/torflow.git/tree/NetworkScanners/BwAuthority/README.spec.txt#n332">https://gitweb.torproject.org/torflow.git/tree/NetworkScanners/BwAuthority/README.spec.txt#n332</a></span><br><span>3. <a href="https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt">https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt</a></span><br><span>4. <a href="https://metrics.torproject.org/onionoo.html#details">https://metrics.torproject.org/onionoo.html#details</a></span><br><span></span><br><span></span><br><span>A. Sample data</span><br></div></blockquote><div><br></div><div>Maybe the sample data should contain more than one relay?</div><br><blockquote type="cite"><div><span>A.1. Torflow</span><br><span></span><br><span>1523911758</span><br><span>node_id=$68A483E05A2ABDCA6DA5A3EF8DB5177638A27F80 bw=392760 nick=Test</span><br><span>measured_at=1523911725 updated_at=1523911725 pid_error=4.11374090719</span><br><span>pid_error_sum=4.11374090719 pid_bw=57136645 pid_delta=2.12168374577</span><br><span>circ_fail=0.2 scanner=/filepath</span><br><span></span><br><span>A.2. sbws</span><br><span></span><br><span>1523911758</span><br><span>version=0.1.0</span><br><span>node_id=$68A483E05A2ABDCA6DA5A3EF8DB5177638A27F80 bw=392760 nick=Test</span><br><span>rtt=380 time=1523911725</span><br></div></blockquote><br></div></div></div></div></div><div>T</div></body></html>