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 legacy/trac#28279 (moved))

changed milestone to %Tor: 0.4.4.x-final in legacy/trac

added 043-deferred in Legacy / Trac 044-can in Legacy / Trac actualpoints::0.1 in Legacy / Trac component::core tor/tor in Legacy / Trac milestone::Tor: 0.4.4.x-final in Legacy / Trac needs-spec in Legacy / Trac priority::low in Legacy / Trac reviewer::dgoulet in Legacy / Trac reviewer::teor in Legacy / Trac severity::normal in Legacy / Trac status::needs-revision in Legacy / Trac tor-control in Legacy / Trac tor-spec in Legacy / Trac type::enhancement in Legacy / Trac 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 from legacy/trac#28280 (moved)

added Feature label and removed 1 deleted label

added Control API Deferred Specifications + 1 deleted label and removed 1 deleted label

added Backlog label

added Icebox label and removed Backlog label

assigned to @nickm

added Backlog label and removed Icebox label

added Next label and removed Backlog label

added Icebox label and removed Next label

changed milestone to %Tor: 0.4.5.x-freeze

removed milestone %Tor: 0.4.5.x-freeze

added Needs Revision label and removed 1 deleted label

unassigned @nickm

We're not currently planning to add new controller features except in cases of dire need. On the other hand, DoS issues are the sort of thing that we would consider "dire" potentially.

added Triage2022 label

Status: It would be better to use MetricsPort for this.

closed