[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #28280 [Core Tor/Tor]: control: Add a key to GETINFO to get the DoS subsystem stats
#28280: control: Add a key to GETINFO to get the DoS subsystem stats
-----------------------------------------------+---------------------------
Reporter: dgoulet | Owner: (none)
Type: enhancement | Status:
| needs_review
Priority: Low | Milestone: Tor:
| 0.4.3.x-final
Component: Core Tor/Tor | Version:
Severity: Normal | Resolution:
Keywords: tor-control, tor-spec, needs-spec | Actual Points: 0.1
Parent ID: | Points:
Reviewer: teor, dgoulet | Sponsor:
-----------------------------------------------+---------------------------
Changes (by teor):
* keywords: tor-control, tor-spec => tor-control, tor-spec, needs-spec
* reviewer: => teor, dgoulet
* milestone: Tor: unspecified => Tor: 0.4.3.x-final
* actualpoints: => 0.1
Comment:
Thanks for this code!
> I'd like to make changes to control-spec.txt after, not before this PR's
review, if that's OK.
I think it would be best if we agreed on a design first? Having a spec is
a good way to agree on a design.
A few design notes:
1. We might want a `dos` GETINFO category, with subcategories for each
type of DoS.
2. I don't know if we will want individual keys in each subcategory.
3. We can do rollups of each sub-category and category, if we want.
4. We should list all DoS variables as keys, including the `*_enabled`
variables
A few formatting notes:
1. All the names should be in a consistent format.
2. We probably want to use the existing lowercase-hyphen format.
3. Most of our controller names aren't plural.
4. All values should be numeric (not `False`)
5. We probably want values separated by newlines, to match existing
GETINFOs. See `downloads/` for an example.
https://github.com/torproject/torspec/blob/master/control-spec.txt#L1008
A few implementation notes:
1. The code will be easier to maintain if you use smartlist_add_asprintf()
for each key=value, and smartlist_join_strings() at the end.
2. The code will be even easier to maintain if you write a function that
checks `*_enabled`, formats the key=value, and returns a newly allocated
string. Then you can just pass the key name, the `*_enabled` variable, and
the actual variable to the function.
There are also some minor issues with the code, I added comments on the
pull request.
I'd like to check this advice with the developer who wrote the DoS code,
so I'm assigning this ticket to them for review.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/28280#comment:3>
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