Vladimír Čunát authored 3 years ago and Štěpán Balážik committed 3 years ago

The requests get traced even without verbose logging.
I expect it was a copy&paste mistake from QTRACE docs.

Vladimír Čunát authored 3 years ago and Štěpán Balážik committed 3 years ago

Perhaps this bug was now more pronounced since 5.3.0 changes.
Example problem was disabling minimization or 0x20 (globally or
for some problematic requests); without this change they would get
re-enabled during some fallback actions... which might be exactly
the wrong moment wrt. the motivation to setting these.
https://gitter.im/CZ-NIC/knot-resolver?at=60a221e86a950f3d46ed1cd9

The purpose of this change is to make it harder to accidentally use the
legacy DoH implementation and free up the "doh" kind which may be used
as an alias to a modern implementation in the future.

To (hopefully) improve readability, rename the typical macro usage of:

    if (!kr_assume(x)) y; // to
    if (kr_fails_assert(x)) y;

As a convenience, replace the assert without a return value to a more
simple version:

    (void)!kr_assume(x);  // becomes
    kr_assert(x);

In practice it can easily happen, as loading module and really
configuring it is often done separately.  Then we'd see two fstrm
threads, etc.

Regressed in acd019db.  The intention was clearly to say that
encryption (i.e. DNS-over-TLS) is not supported.

This isn't a regression of 5.3.0 changes.
Layer functions are supposed to return new values for ctx->state,
but here we were sometimes returning kr_error(EFOO) which altered
processing of the request.

Our case: answers directly from policy module would not end up
finishing the request and we'd hit an assert at the end of processing.

Vladimír Čunát authored 4 years ago and Tomas Krizek committed 4 years ago

As first noted in commit d1a229ae, in some cases we do call chains that
are not supported for JIT in LuaJIT.

I'm not 100% sure all of these are needed to comply, but the functions
here are really small and probably not to be that heavily used,
so I don't think it will be costly to interpret them
(and avoiding crashes is more important).

In my tests this fixed occasional crashes when using http://*/trace/*

- return SOA in NODATA answers and allow customizing it
- only call ensure_answer() if really generating an answer
  (otherwise we might e.g. deplete XDP buffers, in extreme cases)

Reported on:
https://gitter.im/CZ-NIC/knot-resolver?at=601ae90e9fa6765ef8f6b408

(and regularize indentation)  Reported on:
https://gitter.im/CZ-NIC/knot-resolver?at=601b36f6aa6a6f319d0235f5

Module would crash due to the change of `request->upstream`
structure.

More idiomatic code seems better:
- for variable initialization we have = { 0 }
- (mm_)calloc for heap allocations
  sizeof: use variable instead of type (where suitable; not sure why)

These do not make the CI job fail, but better avoid them anyway.
I suspect they got added when we updated the CI docker image.

(instead of marking them as TCP)
This includes latest dnstap.proto, except for keeping our local changes
of the licensing comment.
https://github.com/dnstap/dnstap.pb/blob/master/dnstap.proto

Pavel Doležal authored 4 years ago and Vladimír Čunát committed 4 years ago

- log queries and responses as separate dnstap messages
- use "query" instead of "request" to mirror dnstap specification
- don't export "query_zone" field in "CLIENT_*" messages

Vladimír Čunát authored 4 years ago and Tomas Krizek committed 4 years ago

Make it easier to delete all rules specified in daf.

Štěpán Balážik authored 5 years ago and Vladimír Čunát committed 4 years ago

Design discussion: #447
Code discussion: !1030