[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #19877 [Core Tor/Tor]: Implement new guard selection algorithm of prop 271
#19877: Implement new guard selection algorithm of prop 271
-------------------------------------------------+-------------------------
Reporter: andrea | Owner: nickm
Type: task | Status:
| assigned
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 |
Parent ID: | Points:
Reviewer: | Sponsor:
| SponsorU-must
-------------------------------------------------+-------------------------
Comment (by asn):
Hello, I did a very rough initial review based on the WIP branch
`prop271-wip`. Haven't run it yet. Great work so far, very excited about
splitting bridge code to another file as well.
Some of these comments might be too low level for this stage of
implementation, so feel free to ignore them. Also, I have not re-loaded
the whole proposal in my brain yet, so I didn't go too deep in the nitty
gritty details.
- There are a few big scary functions that could be splitted into smaller
functions to make them more manageable. e.g.
`sampled_guards_update_from_consensus()` and
`entry_guards_update_primary`.
Also the fundamental function `select_entry_guard_for_circuit()` could
be split into three smaller functions, each for one spec case.
- Agreed that the `entry_guard_succeeded()` tristate is ugly and makes the
`circuit_send_next_onion_skin()` logic harder to read. Perhaps the retval
could be turned into an enum?
- If `add_bridge_as_entry_guard()` is a pub function of the entrynodes
API, should it have a prefix? Or we are not at that level of tidyness yet?
- Why do we need the spaceless ISO time functions? Can't we use the
spaceful ones for state? Or is it to maintain backwards compat?
- Parsing guards from state seems a bit of a pain now. For example,
`entry_guard_parse_from_state()` does ad-hoc string parsing. Would it be
possible to use the config parsing API for that?
- Could `smartlist_remove_keeporder()` be implemented with
`smartlist_pos()` and `smartlist_del_keeporder()` to avoid writing more
smartlist code?
Finally, how should one robustly test this branch? Do you use iptable
scripts or something?
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19877#comment:9>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs