#19877: Implement new guard selection algorithm of prop 271 -------------------------------------------------+------------------------- Reporter: andrea | Owner: nickm Type: task | Status: | needs_revision Priority: Medium | Milestone: Tor: | 0.3.0.x-final Component: Core Tor/Tor | Version: Tor: | unspecified Severity: Normal | Resolution: Keywords: isaremoved, nickwants029, tor- | Actual Points: guards-revamp, TorCoreTeam201611, review- | group-13, actual-review-points=2 | Parent ID: | Points: Reviewer: chelseakomlo, asn | Sponsor: | SponsorU-must -------------------------------------------------+-------------------------
Comment (by chelseakomlo): Hey Nick, Here are some general thoughts, feel free to take/leave what is useful. I added notes to the gitlab review as well. - Naming conventions are a bit confusing, as we use entrynodes, entryguards, and guards. If this naming signifies pre and post prop271, maybe we can add in legacy namespacing to make it more clear (see below). Also, the bridge-specific module looks really great- if there is guard- specific code that is distinct from entrynodes, maybe this belongs in a separate guard module. - Making a clear distinction between pre and post prop271 code would definitely make reviewing easier, as well as eventually migrating away from legacy code... I liked asn's recommendation of namespacing, we could also move legacy code to it's own module, etc. - Some functions are quite large, such as sampled_guards_update_from_consensus and entry_guards_upgrade_waiting_circuits. I see some todos about splitting these up, which is great. - Would it make sense to put parsing code into a parser.c? - I didn't see a lot of tests here: https://gitlab.com/nickm_tor/tor/merge_requests/11 - is there another set of commits for these? - bridges.c doesn't have test coverage :( - It looks like there are several remaining todos that need to be completed. Which of these should be completed as part of this patch? For example, in entrynodes.c, update_guard_selection_choice has a comment about needing to expire existing circuits (line 653). -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19877#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