Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)
On 10/09/2021 11:27, Dominik DL6ER wrote: > Hey Joerg, > > On Fri, 2021-09-10 at 10:13 +0200, jo...@schuetter.org wrote: >> rev-server=fe80::/10,192.168.178.1 > > dnsmasq always only accepted IPv6 prefixes that are a multiple of > 4. There was just no enforcing of this before v2.86. > > Looking at the v2.85 code, dnsmasq will have added >> fe80::/8 > for you in version v2.85 without any warnings. > > I submitted patches for supporting arbitrary prefix lengths (IPv4 > is even more restrictive (only /8, /16, /24 and /32 are allowed) > last year. I already rebased my patches on the current master and > will submit them in the next few days. This means support for /10 > might come in the next release (if the patches are accepted). > For IPv6, this seems like an obviously good thing to do, since one --rev-server can create as most eight .ip6.arpa, but for IPv4, that goes up to 128 since for instance 192.168.1.0/25 expands to 0.1.168.192.in-addr.arpa 1.1.168.192.in-addr.arpa . . 127.1.168.192.in-addr.arpa I guess that now we have efficient searches over domains, that's more likely to be OK, but it needs some consideration. Cheers, Simon. ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH]
On 10/09/2021 09:37, Dominik DL6ER wrote: > 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). > Ah, thank you! I combined that with the one that started this thread and committed them. I have a tool which builds with all the possible single compile-time options, and a good selection of combinations (every possible one would take WAY to long) and it passes fine. > 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. Ack. > > > 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 ( 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. Agreed. It's pretty difficult to get into trouble with O(N) where N is constant and magnitude three. Cheers, Simon. tagged 2.87-test1 just to get 2.87 into the version string. Petr - I shall work though your coverity-originated patches as I get time in the next day or two. ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH]
On 9/10/21 10:37 AM, Dominik DL6ER wrote:
> 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).
Builds fine with DNSSEC, LIBIDN2 and DBUS. I failed to notice querystr()
were called always and unconditionally, I thought I were optimizing just
--log-queries scenario.
>
> 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 ( 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
I admit moving querystr into log_query is much more important
optimization than my table lookup change. But it can still be applied
with minor tweak and would save some loops. In patch #1. If HTTPS record
is logged, it would have to pass 64 rows before finding correct one.
With runtime dynamic data, we have not much better choices. But for
built-in static data, we can switch linear lookup to constant. I admit
64 iterations is not too much to save, but at least something. Often it
means lookup both for query and response.
I think dest = arg; should be called after querystr(), just like it was
before. It works with common addresses, but there might be some corner
cases broken. Change in patch #2.
--
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
>From 281e9cc7fff0e067cb3b590f6f4e837ea80cae60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?=
Date: Fri, 10 Sep 2021 16:18:08 +0200
Subject: [PATCH 2/2] Assign dest after arg is set
---
src/cache.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/cache.c b/src/cache.c
index ab8f9d9..f43c34b 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -1970,7 +1970,7 @@ static char *edestr(int ede)
void log_query(unsigned int flags, char *name, union all_addr *addr, char *arg, unsigned short type)
{
- char *source, *dest = arg;
+ char *source, *dest;
char *verb = "is";
char *extra = "";
@@ -1980,6 +1980,7 @@ void log_query(unsigned int flags, char *name, union all_addr *addr, char *arg,
/* build query type string if requested */
if(type > 0)
arg = querystr(arg, type);
+ dest = arg;
#ifdef HAVE_DNSSEC
if ((flags & F_DNSSECOK) && option_bool(OPT_EXTRALOG))
--
2.31.1
>From 95a7dba6d43a29bf0562c04262edf65db5bdc513 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?=
Date: Thu, 9 Sep 2021 21:42:10 +0200
Subject: [PATCH 1/2] Include all DNS types and speed up lookups
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Reworked proposal made by Dominik DL6ER, add all types registered by
IANA registry. Replace sequential walking through single table by
walking through set of arrays with offsets for their values. Makes
it more efficient with multiple values, while it omits gaps with
undefined types.
Signed-off-by: Petr Menšík
---
src/cache.c | 173 --
src/dnsmasq.h | 2 +-
2 files changed, 125 insertions(+), 50 deletions(-)
diff --git a/src/cache.c b/src/cache.c
index 5be73cf..ab8f9d9 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -29,52 +29,125 @@ static void make_non_terminals(struct crec *source);
static struct crec *really_insert(char *name, union all_addr *addr, unsigned short class,
time_t now, unsigned long ttl, unsigned int flags);
+/* taken from https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml */
/*
Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)
Hello Dominik On 2021-09-10 12:27, Dominik DL6ER wrote: Hey Joerg, On Fri, 2021-09-10 at 10:13 +0200, jo...@schuetter.org wrote: rev-server=fe80::/10,192.168.178.1 dnsmasq always only accepted IPv6 prefixes that are a multiple of 4. There was just no enforcing of this before v2.86. From a DNS perspective the block size of 4 (or 8 for IPv4) absolutely makes sense, but having the option to provide the CIDR with any figure (as comon on routing) would be great. Thanks for pointing out the issue. I have added the address with four /12 networks: rev-server=fe80::/12,192.168.178.1 rev-server=fe90::/12,192.168.178.1 rev-server=fea0::/12,192.168.178.1 rev-server=feb0::/12,192.168.178.1 Regards Joerg___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)
Hey Joerg, On Fri, 2021-09-10 at 10:13 +0200, jo...@schuetter.org wrote: > rev-server=fe80::/10,192.168.178.1 dnsmasq always only accepted IPv6 prefixes that are a multiple of 4. There was just no enforcing of this before v2.86. Looking at the v2.85 code, dnsmasq will have added > fe80::/8 for you in version v2.85 without any warnings. I submitted patches for supporting arbitrary prefix lengths (IPv4 is even more restrictive (only /8, /16, /24 and /32 are allowed) last year. I already rebased my patches on the current master and will submit them in the next few days. This means support for /10 might come in the next release (if the patches are accepted). Best, Dominik ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH]
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 ( 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
From e0d56e962e058add871c2d49c0e224d41157057a Mon Sep 17 00:00:00 2001
From: Dominik DL6ER
Date: Fri, 10 Sep 2021 10:00:56 +0200
Subject: [PATCH] Move call to querystr() inside log_query() to ensure it is
only called when logging is enabled.
Signed-off-by: DL6ER
---
src/auth.c | 40 +-
src/cache.c| 10 +--
src/dnsmasq.h | 3 +-
src/dnssec.c | 16 +--
src/domain-match.c | 6 ++--
src/forward.c | 40 +-
src/rfc1035.c | 70 +++---
7 files changed, 93 insertions(+), 92 deletions(-)
diff --git a/src/auth.c b/src/auth.c
index 172a4b2..7be1613 100644
--- a/src/auth.c
+++ b/src/auth.c
@@ -210,7 +210,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
if (local_query || in_zone(zone, intr->name, NULL))
{
found = 1;
- log_query(flag | F_REVERSE | F_CONFIG, intr->name, , NULL);
+ log_query(flag | F_REVERSE | F_CONFIG, intr->name, , NULL, 0);
if (add_resource_record(header, limit, , nameoffset, ,
daemon->auth_ttl, NULL,
T_PTR, C_IN, "d", intr->name))
@@ -234,7 +234,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
strcat(name, ".");
strcat(name, zone->domain);
}
- log_query(flag | F_DHCP | F_REVERSE, name, , record_source(crecp->uid));
+ log_query(flag | F_DHCP | F_REVERSE, name, , record_source(crecp->uid), 0);
found = 1;
if (add_resource_record(header, limit, , nameoffset, ,
daemon->auth_ttl, NULL,
@@ -243,7 +243,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
}
else if (crecp->flags & (F_DHCP | F_HOSTS) && (local_query || in_zone(zone, name, NULL)))
{
- log_query(crecp->flags & ~F_FORWARD, name, , record_source(crecp->uid));
+ log_query(crecp->flags & ~F_FORWARD, name, , record_source(crecp->uid), 0);
found = 1;
if (add_resource_record(header, limit, , nameoffset, ,
daemon->auth_ttl, NULL,
@@ -257,7 +257,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
if (!found && is_rev_synth(flag, , name) && (local_query || in_zone(zone, name, NULL)))
{
- log_query(F_CONFIG | F_REVERSE | flag, name, , NULL);
+ log_query(F_CONFIG | F_REVERSE | flag, name, , NULL, 0);
found = 1;
if (add_resource_record(header, limit, , nameoffset, ,
@@ -269,7 +269,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
if (found)
nxdomain = 0;
else
- log_query(flag | F_NEG | F_NXDOMAIN | F_REVERSE | (auth ? F_AUTH : 0), NULL, , NULL);
+ log_query(flag | F_NEG | F_NXDOMAIN | F_REVERSE | (auth ? F_AUTH : 0), NULL, , NULL, 0);
continue;
}
@@ -300,7 +300,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
if (rc == 2 && qtype == T_MX)
{
found = 1;
- log_query(F_CONFIG | F_RRNAME, name, NULL, "");
+ log_query(F_CONFIG | F_RRNAME, name, NULL, "", 0);
if (add_resource_record(header, limit, , nameoffset, , daemon->auth_ttl,
NULL, T_MX, C_IN, "sd", rec->weight, rec->target))
[Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)
Hello After upgrading dnsmasq from 2.85 to 2.86 (debian sid) the rev-server lines regarding IPv6 are no longer accepted error: bad IPv6 prefix at line 26 of /etc/dnsmasq.d/05-local.conf config line: rev-server=fe80::/10,192.168.178.1 Best Regards Joerg___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] ipv6 - How to add static route with metric
Hello Simon Le 10/09/2021 à 00:52, Simon Kelley a écrit : On 09/09/2021 10:22, Daniel via Dnsmasq-discuss wrote: Hello, using dnsmasq for office network, I would like to add an ipv6 static route. My configuration looks like dhcp-range=set:computer6, fd00:5678:beef:cafe::ff:0, fd00:5678:beef:cafe::ff:, slaac, ra-names, 1h dhcp-option=tag:computer6,option6:3,fd00:5678:beef:cafe::2 dhcp-option=tag:computer6,option6:6,fd00:5678:beef:cafe::1 dhcp-option=tag:computer6,option6:ntp-server,fd00:5678:beef:cafe::2 Now I want to add route like dhcp-option=tag:computer6,option6:3,fd00:5678:beef:cafe:fade::/96,192,fd00:5678:beef:cafe::1 but this does not work (also tried What am I doing wrong ? Thanks for your support The space of option numbers in DHCPv6 is not the same as the space of option numbers in DHCPv4. Just because option 3 in DHCP4 sets the default route, doesn't mean that option 3 sets the default route on DHCPv6 - option 3 does something completely different in DHCPv6. The same is true for option 6 - the option NUMBER for the DNS server in DHCPv6 is 23. The function of option 3 in DHCPv4 is done using router advertisements in IPv6 land - there's no direct equivalent of the DHCP option at all, and for a generalised static route, I don't believe that a DHCPv6 option has been defined. There's an expired draft proposal which is insanely complex, https://datatracker.ietf.org/doc/html/draft-ietf-mif-dhcpv6-route-option-05 But it has never been ratified as an RFC. You confirm what Petr wrote, will find another way to archieve my goal. Thanks for your help -- Daniel ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] ipv6 - How to add static route with metric
Hello Petr Le 10/09/2021 à 00:10, Petr Menšík a écrit : Hello Daniel, dnsmasq --help dhcp6 does not list options 3 and 6. According to Iana list [1], those are OPTION_IA_NA and OPTION_ORO. It does not seem they are correct for IPv6. I can see there plenty of options, but did not find way to specify IPv6 route. Are you sure the target router for the route should not use router advertisement to propagate its routes instead? Would you describe, what are you trying to archieve? Why should have clients have some route instead of router for the network? I'm in a phase of replacing OpenVPN with wireguard as well as replacing ipv4 with ipv6 in the network. I have 2 routers in the office network: UTM Sophos as firewall in a VM as default GW, another VM running Debian11 which do the routing job (VPNs, DNS, aso) usind dnsmasq & bind as GW for VPNs (I have 3 servers in DC which are connected through VPN to this network, each of them having VMs). I setup some only ipv6 desktops, the Debian VM acting for NAT64. Everything works perfect except that only ipv6 desktops can't reach what is in DC behind VPNs as there default route point to UTM Sophos. If I manually ip -6 r add via dev lan it works. I could add manually this command for one desktop, but for many ... that's why I was looking for a solution with dnsmasq. Could --ra-param help instead? ra is enabled on both routers. I will modify the dnsmasq setup so the default route would be to set to DebianVM for those machines. Thanks for your help. On 9/9/21 11:22 AM, Daniel via Dnsmasq-discuss wrote: Hello, using dnsmasq for office network, I would like to add an ipv6 static route. My configuration looks like dhcp-range=set:computer6, fd00:5678:beef:cafe::ff:0, fd00:5678:beef:cafe::ff:, slaac, ra-names, 1h dhcp-option=tag:computer6,option6:3,fd00:5678:beef:cafe::2 dhcp-option=tag:computer6,option6:6,fd00:5678:beef:cafe::1 dhcp-option=tag:computer6,option6:ntp-server,fd00:5678:beef:cafe::2 Now I want to add route like dhcp-option=tag:computer6,option6:3,fd00:5678:beef:cafe:fade::/96,192,fd00:5678:beef:cafe::1 but this does not work (also tried What am I doing wrong ? Thanks for your support -- Daniel Huhardeaux +33.368460...@tootai.net sip:8...@sip.tootai.net +41.445532...@swiss-itech.chtootaiNET ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
