[Dnsmasq-discuss] [PATCH]

Dominik DL6ER dl6er at dl6er.de
Fri Sep 10 08:37:53 UTC 2021


Hey Petr and Simon,

On Thu, 2021-09-09 at 23:35 +0100, Simon Kelley wrote:
> Much tidier is to simplify things and add an extra parameter to
> log_query which is the type, if type is non-zero, log_query calls
> querystr(), so
> 
> log_query(flags, name, addr, querystr("description", type));
> 
> becomes
> 
> log_query(flags, name, addr, "description", type);
> 

I agree this is a good solution, patch attached. I hope to have caught
all places (I don't have the dependencies to build all possible
combinations myself).

On Thu, 2021-09-09 at 23:35 +0100, Simon Kelley wrote:
> querystr becomes a local function to cache.c or just gets rolled into
> log_query().

I did the former. Merging it into log_query() would make an already
complex routine even longer.


Concerning performance: The loop can at most iterate over 89 entries
before it says: "Didn't find". However, for the vast vast majority of
cases, the match will be in the first 30ish (AAAA and SRV) as query
types behind those are not very likely to be seen. The loop breaks as
soon as the match is found. Looking at the gcc11 x86_64 -O2 assembly,
there are no surprises, only very few assembler instructions per
iteration are needed. The next type integers in the table can always be
found 16 bytes after the former. Hence, we just need

.FOR_LOOP
        add     rax, 1
        cmp     rax, 89
        je      .NOT_FOUND
        mov     rdx, rax
        movsx   rcx, eax
        sal     rdx, 4
        cmp     DWORD PTR typestr[rdx], r1d
        jne     .FOR_LOOP
	[... stuff if found ...]

Given we call library functions like strlen() and sprintf(), our loop
here is surely not any kind of bottleneck. Even if it'd be even larger.

Best,
Dominik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Move-call-to-querystr-inside-log_query-to-ensure-it-.patch
Type: text/x-patch
Size: 34581 bytes
Desc: not available
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20210910/4c10f13d/attachment-0001.bin>


More information about the Dnsmasq-discuss mailing list