[Dnsmasq-discuss] [PATCH] Report filtered A or AAAA records via EDE code

[Simon Kelley]

On 31/03/2023 14:28, Petr Menšík wrote:
> I agree with Dominik.
> 
> --cache-rr=!TXT,!DNSKEY,!DS or maybe just --no-cache-rr=TXT,DNSKEY,DS
> 
> would be more useful.
> 
> For my own preference it would be more useful to cache ANY record by 
> default, maybe with exceptions of explicitly listed records. I think it 
> would be nice to specify maximal record set size instead. On memory 
> constrained devices it might choose to not cache rrset records longer 
> than 32B for example. Addresses might have exception for that. I think 
> usually it does not matter if that is OPENPGPKEY or TXT. I think only 
> what matters is how long the record is and how often it is queried. 
> --no-cache-rrset-over=120 might be an example.
> 
> Larger records should have slightly higher chance to be pushed out from 
> the cache. I think that were not changed yet. Does existing code prefer 
> to release bigger chunks from the cache?

That all sounds really complicated. I wonder if anyone would actually 
use it or find it useful?
> 
> I think also a cache summary should be requestable via dbus or signal. 
> Something like X A records, 256B, Y AAAA records, 600B, Z TXT records, 
> 1400B. Ideally also cache hits number or ratio for those types. 
> Something which could briefly describe what current cache is holding. Do 
> we have something like that already?

sending SIGUSR1 dumps all the records in the cache, but none of the 
stats are kept per-RR type.
> 
> If we want to refuse caching of some records, we should have a better 
> evidence whose tend to occupy memory without being much useful. I think 
> we are making just educated guesses. On some networks it might be 
> significantly different. Gathering a good default needs some evidence, 
> which I think we are missing now. Bare total cache misses and hits are 
> not enough IMO.
> 
> 
> To the original issue with EDE code, thanks for that. The answer from 
> IETF dnsop were indeed filtered might be not a correct one. Code 21 no 
> supported were suggested, unless we define a better one. There is a new 
> type defined Synthetized, but that has also somehow different usage. I 
> have created feature request on bind [1], they have very similar 
> functionality to filter out address records. Do not indicate modified 
> answers with any code at the moment.
> 

Thanks for chasing this. I'll leave the behaviour as-is for now, and aim 
to change things when the standards people come to a conclusion.


Cheers,

Simon.

> [1] https://gitlab.isc.org/isc-projects/bind9/-/issues/3979
> 
> On 3/31/23 09:25, Dominik Derigs via Dnsmasq-discuss wrote:
>> Hey Simon,
>>
>> On Thu, 2023-03-30 at 18:28 +0100, Simon Kelley wrote:
>>> I just merged the branch I've been working on for the last week which
>>> includes this patch, but much modified because the surrounding code has
>>> changed. The function is unaltered.
>>>
>>> The other changes are adding the ability to cache any RR-type, and the
>>> ability to filter any RR-type. There's quite a bit of code cleanup in
>>> the affected code paths too.
>> The new man page says:
>>> By default, dnsmasq caches A, AAAA, CNAME and SRV DNS
>> record types. This option adds other record types to the
>> cache. [...]
>>
>> I wonder how useful this really is. Won't it cause config
>> files to explode with lines like (possibly line-per-line):
>> --cache-
>> rr=NS,MD,MF,SOA,MB,MG,MR,NULL,WKS,PTR,HINFO,MINFO,MX,TXT,RP,
>> AFSDB,X25,ISDN,RT,NSAP,NSAP_PTR,SIG,KEY,PX,GPOS,LOC,NXT,EID,
>> NIMLOC,ATMA,NAPTR,KX,CERT,A6,DNAME,SINK,OPT,APL,DS,SSHFP,IPS
>> ECKEY,RRSIG,NSEC,DNSKEY,DHCID,NSEC3,NSEC3PARAM,TLSA,SMIMEA,H
>> IP,NINFO,RKEY,TALINK,CDS,CDNSKEY,OPENPGPKEY,CSYNC,ZONEMD,SVC
>> B,HTTPS,SPF,UINFO,UID,GID,UNSPEC,NID,L32,L64,LP,EUI48,EUI64,
>> TKEY,TSIG,IXFR,AXFR,MAILB,MAILA,ANY,URI,CAA,AVC,DOA,AMTRELAY
>> ,TA,DLV
>>
>> if I want to cache all types known to dnsmasq by name (yes,
>> this does not include proprietary extensions by numbers). It
>> also seems inefficient to always loop over these 86 RR types
>> when we check if this RR is to be cached.
> 
> Since we have just binary indication for numberic type, I think we need 
> just:
> 
> - default value
> - array of uint8_t for bit indication for each specified type.
> - array size would be max specified type / 8+1.
> - over max value the default value would apply, no need to have full 
> size bitmap.
> 
> This would be able to query type cacheability in linear way. No loops 
> per each query should be necessary, should be quite fast. I think 
> NSEC(3) uses similar kind of bit magic. Not sure that is decoded by 
> dnsmasq anywhere in the code.
> 
> Unless we need dbus API to change cached types runtime, those would be 
> initialized just once. Makes sense to spend time once during init and 
> save it per-query.
> 
>>
>> Looking at this new option, it seems really counter-
>> intuitive to specify "I want to cache ANY but not TXT". Is
>> there a real-world scenario where someone would not like to
>> cache a specific type? I suppose these queries should
>> arguably have a TTL of 0 from upstream to prevent caching.
>>
>> My feeling is that we should really have at least a shortcut
>> to specify "cache everything you can". May this be "--cache-
>> rr" without options, some special "--cache-rr=all" or maybe
>> a dedicated option like "--cache-all".
>>
>> Others than that - thanks for working on this! I already
>> started testing (using the long command above) and will
>> report any oddities I come across.
>>
>> Best,
>> Dominik
>