[tor-bugs] #28280 [Core Tor/Tor]: control: Add a key to GETINFO to get the DoS subsystem stats

Tor Bug Tracker & Wiki blackhole at torproject.org
Wed Nov 27 02:18:04 UTC 2019


#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


More information about the tor-bugs mailing list