Otto Moerbeek [Wed, 14 Dec 2022 13:06:36 +0000 (14:06 +0100)]
Generate EDE in more cases, specifically on unreachable auths or synthesized results.
As there is no specific EDE for synthesised, use noError with a text.
We have to be careful here: a single client query can lead to
multiple beginResolve calls. Some of these are done after the main
result has been looked up, for example to validate the result. These
subsequent calls can generate EDE's but we do not want to copy the
EDE to the main result in those cases. A typical example would be
an absent DS for an Insecure domain. Nothing wrong with these but
we do not want the potential absent DS EDE (which could be synthesize)
to be returned with the main query,
To solve this, mimic the processing of validation state and add
an extra argument to a few methods.
I am not terribly happy with the extra argument. Maybe we should
move to an object holding the parameters and result status of the
nested or subsequent calls. This would also avoid some of the saveX,
setX, beginResolve, restore X sequences.
Remi Gacogne [Mon, 12 Dec 2022 14:42:57 +0000 (15:42 +0100)]
dnsdist: Disable the send wrappers in our CI
The way the send wrappers are implemented, reading the data _after_
it has been sent, cause them to report a data race that does not
exist with existing implementations:
- we call `send()` from thread 1 to send a query to a backend, never
touching the data or associated metadata again from that thread
- we get a response from the backend in a different thread, thread 2,
which will then access the metadata and sometimes (truncated UDP
answers following a DoH query) even modify the data itself
- ASAN and TSAN complain because the wrapper might still be reading
the data after the UDP datagram has been sent, which is effectively
a race, but it does not really make any sense for an actual
implementation of `send()` to do that.
We work around that by disabling the `send()` wrappers in our CI,
for the dnsdist regression tests only, via `intercept_send=0`.
Otto Moerbeek [Tue, 6 Dec 2022 10:18:56 +0000 (11:18 +0100)]
Close a race: some test immediately query metrics when a query result is received.
to avoid ordering issues, update metrics snap before answers are sent out.
Otto Moerbeek [Tue, 8 Nov 2022 10:10:54 +0000 (11:10 +0100)]
Introducing TCounters
This is a mostly lockless (and not using atomics) way to keep track of
counters and other metrics.
Atomic value are more expensive than you would think (especially if
your platform has no native atomic support for your data type), and
using locking all the time for often updated counters is very
expensive as well.
The idea for `TCounters` is based on
https://github.com/ahupowerdns/mcounter
But addresses the issues raised in
https://github.com/ahupowerdns/mcounter/issues/3
Templates are used, the application has to provide a specific class to
hold the values and enums to index these values. The application
specific class also has to provide a `merge()` method to merge two
instances of the application specific data. For counters that is
simple: just add them. Averages (or histogrfam) requires a bit more
work. This is demonstrated in `rec-tcounters.{cc,hh}`
At the end of a body of work the application's threads should call the
`updateSnap()` function. If a certain amount of time has passed since
the last time, a thread local snapshot of the thread local data will
be created in a thread-safe way.
The class that collects the aggregated values reads (also in a thread
safe way) from the snapshot values in each thread.
Updates of individual counters are done on thread-local data,
potentially many times per second. The snaps contain a consistent set
of the values and are taken by default once per 100ms, so reletively
seldom.
By using the snap mnechanism the aggragate values computed are based
on internally consistent counter values (as long as related counters
are updated from the same thread). A (small) drawback is that the
values computed might be a bit out of date.
The snapshot approach was suggested by @wojas.
This PR de demonstrates `TCounters` for a few Recursor metrics: simple
counters and double typed average values. For the latter weights are
kept, so that the average of averages can be computed in a proper way.
Pieter Lexis [Fri, 3 Dec 2021 09:37:46 +0000 (10:37 +0100)]
service files: Add ProtectProc
Another sandboxing option,
[ProtectProc](https://www.freedesktop.org/software/systemd/man/systemd.exec.html#ProtectProc=)
hides all /proc/<pid> that are not owned by the service user and hides
some kernel things from /proc as well.
Short Description:
Raise RLIMIT_MEMLOCK automatically when eBPF is requested.
This PR adds changes to eBPF filter constructor which when invoked automatically raises the RLIMIT_MEMLOCK from 64k to 1024k.
The hard limit for the user needs to be set in `/etc/security/limits.conf`.
Remi Gacogne [Tue, 6 Dec 2022 16:43:12 +0000 (17:43 +0100)]
dnsdist: Fix a warning about long to double conversion
```
dnsdist-backend.cc:601:61: warning: implicit conversion from 'long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Wimplicit-const-int-float-conversion]
if (backOffCoeffTmp != HUGE_VAL && backOffCoeffTmp <= std::numeric_limits<time_t>::max()) {
~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
Remi Gacogne [Fri, 2 Dec 2022 14:57:17 +0000 (15:57 +0100)]
dnsdist: Get rid of TCPCrossProtocolQuerySender
We need this construct to deal with cross-protocol queries, like
queries received over TCP or DoT, but forwarded over DoH, because
the thread dealing with the client and the one dealing with the
backend will not be the same in that case, and we do not want to
have different threads touching the same TCP connections.
So we pass the query and response to the correct thread via pipes.
Until now we were allocating an additional object, TCPCrossProtocolQuerySender,
to deal with that case, but I noticed that the existing IncomingTCPConnectionState
object already does everything we need, except that it needs to
know that the response is a cross-protocol one in order to pass it
via the pipe instead of treating it in a different way. This can be
done by looking if the current thread ID differs from the one that
created this object: if it does, we are dealing with a cross-protocol
response and should pass it via the pipe, and if it does not we
can deal with it directly.
This change saves the need to allocate a new object wrapped in a
shared pointer for each cross-protocol query, which is quite nice.
Remi Gacogne [Tue, 6 Dec 2022 15:23:04 +0000 (16:23 +0100)]
dnsdist: Add a new chain of rules triggered after cache insertion
The general idea is to be able to store the unedited version into
the cache while delivering a different version to the actual client.
This is useful when one is sending different answers to different
clients, like when dealing with abuse traffic, but still want to be
able to cache the initial response from the backend.
We already have a chain of rules that are triggered after a cache-hit,
but until now we lacked the ability to trigger after getting the
response corresponding to a cache-miss.