control: Add a key to GETINFO to get the DoS subsystem stats

The DoS subsystem keeps track of statistics that are logged in the heartbeat with dos_log_heartbeat(). I would like those to be exposed to the control port so it can be collected and graph over time (improve relay monitoring).

(Follows the idea behind #28279 (moved))

changed milestone to %Tor: 0.4.4.x-final

added 043-deferred 044-can actualpoints::0.1 component::core tor/tor milestone::Tor: 0.4.4.x-final needs-spec priority::low reviewer::dgoulet reviewer::teor severity::normal status::needs-revision tor-control tor-spec type::enhancement labels

PR: https://github.com/torproject/tor/pull/1568

I'd like to make changes to control-spec.txt after, not before this PR's review, if that's OK.

Trac:
Username: moonsikpark

Trac:
Status: new to needs_review

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.

Trac:
Milestone: Tor: unspecified to Tor: 0.4.3.x-final
Actualpoints: N/A to 0.1
Reviewer: N/A to teor, dgoulet
Keywords: N/A deleted, needs-spec added

I agree that changing control-spec first is the way to go.

I'll wait for dgoulet's comment on how this should be done.

Trac:
Username: moonsikpark

Replying to moonsikpark:

I agree that changing control-spec first is the way to go.

I'll wait for dgoulet's comment on how this should be done.

Yes, what teor proposed is really the way to go.

@moonsikpark, you can simply make a branch to the tor-spec.git and add the dos category to GETINFO like teor suggested. Do a first pass and we'll go from there :).

Please set this ticket to needs_review when you have done the control spec pull request.

We accept spec pull requests on GitHub here: https://github.com/torproject/torspec

Trac:
Status: needs_review to needs_revision

All 0.4.3.x tickets without 043-must, 043-should, or 043-can are about to be deferred.

Trac:
Keywords: N/A deleted, 043-deferred added

Trac:
Milestone: Tor: 0.4.3.x-final to Tor: 0.4.4.x-final

Trac:
Keywords: 043-deferred deleted, 043-deferred 044-can added

added 48m of time spent

moved to tpo/core/tor#28280 (closed)