tor-congestion: WIP implementation of RTT estimation

This tries and fails to implement the round-trip-time estimation algorithm in Prop#324 ("fails" meaning "doesn't pass the test vectors right now"). There's a clock estimate test which seems flawed (since the structure of my code and the C code is different), and there's an RTT test which is broken for reasons I don't understand.

cc @dgoulet: if you could take a quick look through the algorithm and see if there's anything obviously wrong here (or cc someone else who can), that'd be very appreciated! (Otherwise I'll probably just manually instrument and trace through the C-tor implementation when I'm back from leave)

assigned to @eta

requested review from @Diziet

I will take over Reviewer here and once I Approve, I'll set @Diziet back.

requested review from @dgoulet and removed review request for @Diziet

requested review from @Diziet and removed review request for @dgoulet

I had a quick look through this. It didn't seem sensible to do a detailed review of the algorithms against the spec yet? But I did notice a few things that gave me pause. I hope this early feedback is helpful.

mentioned in issue tor#40626 (closed)

added 1 commit

• 5a16971f - fixup! tor-congestion: WIP implementation of RTT estimation

Compare with previous version

added 1 commit

• d5200e23 - fixup! tor-congestion: WIP implementation of RTT estimation

Compare with previous version

Thanks for the comments, everyone! I've pushed a new version that should resolve most of them. @mikeperry, if you could update the set of C-tor test vectors (and spec?), that'd unblock me from finishing this off and implementing more of the specification!

added 518 commits

• d5200e23...23d36aba - 514 commits from branch tpo/core:main
• 092bfe70 - tor-netdir: Add congestion control-related network parameters
• 1077df89 - tor-congestion: WIP implementation of RTT estimation
• e8716044 - fixup! tor-congestion: WIP implementation of RTT estimation
• 861937d4 - fixup! tor-congestion: WIP implementation of RTT estimation

Compare with previous version

marked this merge request as ready

This should now be ready to merge; I've tidied it up and incorporated the new test vectors (and made the changes necessary to get them to work :p)

Currently the MR branch here says // from C-tor src/test/test_congestion_control.c which I would read as a statement that (at the time of writing) these values are the same as the ones in the C Tor file.

So one of the things I intend to do is compare the test vectors in this MR with the ones in C Tor.

made the changes necessary to get them to work :p)

I think what you say here means there will be differences? Is there an MR against C Tor about those?

My intent with this review comment is this: If the C Tor test vectors and the Arti test vectors in this MR are different, I would like some way for me to check that the consensus about the protocol is indeed the version being implemented here, and the Arti code ought to have as good a reference for the test vectors as exists...

I think what you say here means there will be differences? Is there an MR against C Tor about those?

I more meant that I fixed my code so that it passes the new test vectors, not that I changed the test vectors.

(The new test vectors are from this commit, by the way.)