Re: [Dnsmasq-discuss] [PATCH] Re: RA-acquired address not marked as 'dynamic' with 2.82
On Mon, Sep 07, 2020 at 07:37:10PM +0200, Geert Stappers wrote:
> I might understand the re-location of CONTEXT_SETLEASE
>
> I don't understand the change from '3 * param->adv_interval'
> to '2 * param->adv_interval'.
>
>
> And my actual message: the patch has been seen ...
How curious. Is there a vim keybinding to decrement a number that I
might have hit by mistake? (searched, yes, it's ctrl-x: TIL)
New one attached, thanks for the review!
Cheers,
--
Iain Lane [ i...@orangesquash.org.uk ]
Debian Developer [ la...@debian.org ]
Ubuntu Developer [ la...@ubuntu.com ]
From 91657a466cdf1d90d3afb64e47803e72cce5678b Mon Sep 17 00:00:00 2001
From: Iain Lane
Date: Mon, 7 Sep 2020 10:20:02 +0100
Subject: [PATCH] Make sure valid and preferred lifetimes always get set
In 4d85e409cd2f4b0935d6ac5e8c72f6a151735d52 we skipped setting the floor
time if we were using the default RA interval. The commit was a bit too
broad; it also caused the valid and preferred lifetimes to be skipped
too, meaning that they were set to infinite.
Adjust the check, so that we only apply the "are we using the default?"
check when calculating the floor; but still set up the `time` variable
because that is used later on as a ceiling for valid_lft and
preferred_lft.
---
src/radv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/radv.c b/src/radv.c
index 41df852..19d7954 100644
--- a/src/radv.c
+++ b/src/radv.c
@@ -629,11 +629,11 @@ static int add_prefixes(struct in6_addr *local, int prefix,
/* find floor time, don't reduce below 3 * RA interval.
If the lease time has been left as default, don't
use that as a floor. */
- if ((context->flags & CONTEXT_SETLEASE) &&
- time > context->lease_time)
+ if (time > context->lease_time)
{
time = context->lease_time;
- if (time < ((unsigned int)(3 * param->adv_interval)))
+ if ((context->flags & CONTEXT_SETLEASE) &&
+ time < ((unsigned int)(3 * param->adv_interval)))
time = 3 * param->adv_interval;
}
--
2.27.0
signature.asc
Description: PGP signature
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
[Dnsmasq-discuss] [PATCH] Re: RA-acquired address not marked as 'dynamic' with 2.82
Hi Dominik,
On Sun, Sep 06, 2020 at 11:30:46PM +0200, Dominik wrote:
> > dnsmasq 2.81:
> > inet6 fd42:d287:488a:d7e8:216:3eff:fecb:d41b/64 scope global dynamic
> > mngtmpaddr noprefixroute
> >
> > dnsmasq 2.82:
> > inet6 fd42:d287:488a:d7e8:216:3eff:fecb:d41b/64 scope global mngtmpaddr
> > noprefixroute
> >
> > Was this intentional and is it actually a problem? i.e. I'm wondering if
> > we should update the tests to not check for 'dynamic', or if a fix in
> > dnsmasq is needed instead.
>
> Hey Iain,
>
> The only related difference I can see between v2.81 and v2.82 seem to be
> this one:
> http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=4d85e409cd2f4b0935d6ac5e8c72f6a151735d52
>
> It's not clear to me when the kernel marks an address as "dynamic".
> Changing the flooring of the lease time may or not have an effect here.
> Would you be able to compile dnsmasq from source and check if this
> behavior you observed can be triggered by going to 4d85e40 and then back
> to its parent (2bd02d2)?
Yeah, thanks, I bisected just now and it is this change:
laney@groovy-vm:~/temp/dnsmasq$ git bisect log
git bisect start
# good: [7ddb99d251c3f5870c8c308a98bb8f283c831872] Debian changelog entry for
CVE-2019-14834
git bisect good 7ddb99d251c3f5870c8c308a98bb8f283c831872
# bad: [f60fea1fb0a288011f57a25dfb653b8f6f8b46b9] CHANGELOG: Fix three typoes.
git bisect bad f60fea1fb0a288011f57a25dfb653b8f6f8b46b9
# good: [49bdf1ead9046c5c554c18ff62fe6e6a9e8a880c] Handle listening on
duplicate addresses
git bisect good 49bdf1ead9046c5c554c18ff62fe6e6a9e8a880c
# good: [837e8f4eb550c688e8a83415c42a99c7bf9a4311] Remove runit support when
building debs for Ubuntu.
git bisect good 837e8f4eb550c688e8a83415c42a99c7bf9a4311
# good: [7e194a0a7d483932eb3f416b8f26131ade588acc] Apply floor of 60s to TTL of
DNSKEY and DS records in cache.
git bisect good 7e194a0a7d483932eb3f416b8f26131ade588acc
# bad: [4d85e409cd2f4b0935d6ac5e8c72f6a151735d52] Change default lease time for
DHCPv6 to one day.
git bisect bad 4d85e409cd2f4b0935d6ac5e8c72f6a151735d52
# good: [2bd02d2f595f1d45a8598a5fce85cfc3d414] Backdated CHANGELOG update.
git bisect good 2bd02d2f595f1d45a8598a5fce85cfc3d414
# first bad commit: [4d85e409cd2f4b0935d6ac5e8c72f6a151735d52] Change default
lease time for DHCPv6 to one day.
Good to know. Actually, I suppose that means in my pasted output I left
out the real bug, which is:
inet6 fd42:d287:488a:d7e8:216:3eff:fecb:d41b/64 scope global mngtmpaddr
noprefixroute
valid_lft forever preferred_lft forever
The lifetimes are *forever* now, but the intention of that commit is
that they were supposed to be one day (86400 seconds). I think maybe the
intention of the commit was this (attached)?
Cheers,
--
Iain Lane [ i...@orangesquash.org.uk ]
Debian Developer [ la...@debian.org ]
Ubuntu Developer [ la...@ubuntu.com ]
From c1183528816f5d9d61a12c05ceeda5975f422b32 Mon Sep 17 00:00:00 2001
From: Iain Lane
Date: Mon, 7 Sep 2020 10:20:02 +0100
Subject: [PATCH] Make sure valid and preferred lifetimes always get set
In 4d85e409cd2f4b0935d6ac5e8c72f6a151735d52 we skipped setting the floor
time if we were using the default RA interval. The commit was a bit too
broad; it also caused the valid and preferred lifetimes to be skipped
too, meaning that they were set to infinite.
Adjust the check, so that we only apply the "are we using the default?"
check when calculating the floor; but still set up the `time` variable
because that is used later on as a ceiling for valid_lft and
preferred_lft.
---
src/radv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/radv.c b/src/radv.c
index 41df852..78edaab 100644
--- a/src/radv.c
+++ b/src/radv.c
@@ -629,11 +629,11 @@ static int add_prefixes(struct in6_addr *local, int prefix,
/* find floor time, don't reduce below 3 * RA interval.
If the lease time has been left as default, don't
use that as a floor. */
- if ((context->flags & CONTEXT_SETLEASE) &&
- time > context->lease_time)
+ if (time > context->lease_time)
{
time = context->lease_time;
- if (time < ((unsigned int)(3 * param->adv_interval)))
+ if ((context->flags & CONTEXT_SETLEASE) &&
+ time < ((unsigned int)(2 * param->adv_interval)))
time = 3 * param->adv_interval;
}
--
2.27.0
signature.asc
Description: PGP signature
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
[Dnsmasq-discuss] RA-acquired address not marked as 'dynamic' with 2.82
Hi, In Ubuntu we started seeing failures of NetworkManager's tests when we updated to dnsmasq 2.82 (from the Debian package; thanks Simon for maintaining this). Search for 'Regex didn't match' in: https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-groovy/groovy/amd64/n/network-manager/20200903_200241_5f5fd@/log.gz and you can work out after a bit of squinting that it's because the kernel is saying the address isn't 'dynamic' any more. I did a bit of manual testing in a VM (spawning LXD containers which get IPs from dnsmasq running on the host) and managed to reproduce this change: dnsmasq 2.81: inet6 fd42:d287:488a:d7e8:216:3eff:fecb:d41b/64 scope global dynamic mngtmpaddr noprefixroute dnsmasq 2.82: inet6 fd42:d287:488a:d7e8:216:3eff:fecb:d41b/64 scope global mngtmpaddr noprefixroute Was this intentional and is it actually a problem? i.e. I'm wondering if we should update the tests to not check for 'dynamic', or if a fix in dnsmasq is needed instead. Cheers, -- Iain Lane [ i...@orangesquash.org.uk ] Debian Developer [ la...@debian.org ] Ubuntu Developer [ la...@ubuntu.com ] signature.asc Description: PGP signature ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [BUG] RA are sent too fast and slows down the machine
On Wed, Aug 21, 2019 at 08:59:07PM +0200, Petr Mensik wrote: > Hi Simon and Maarten, > > we discovered when playing with NetworkManager-ci [1], that lastest > release is somehow broken. Test running dnsmasq are quite slow on latest > release. > > I have created repeatable started script that reproduces it. Then used > git bisect to find when it was broken. It seems fast sending were > intentional in commit 0a496f059c1e9 [2], but maybe way it affects the > system were underestimated. It is significant for systems that hit such > issue. I think it has to be fixed to slow it down to short time > interval, not endless loop. Reported as Fedora bug [3]. Thanks for this Petr. Would you be able to share the script you've used, so that perhaps an upstream developer could recreate the bug? Mainly I wanted to chime in and say that (in addition to the other instance referenced), we found this in the NetworkManager testsuite in Ubuntu. I didn't come up with a nice reproducer at the time, but we did identify the same commit and we've reverted it in Ubuntu. I posted on the ML back then but we didn't get much traction and I didn't follow up very aggressively. http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2018q4/012709.html https://launchpadlibrarian.net/405377161/dnsmasq_2.80-1_2.80-1ubuntu1.diff.gz (the commit ID referenced in the changelog there seems or from somewhere else, it's the same patch) Cheers, -- Iain Lane [ i...@orangesquash.org.uk ] Debian Developer [ la...@debian.org ] Ubuntu Developer [ la...@ubuntu.com ] signature.asc Description: PGP signature ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] Infinite(?) RTR-ADVERTs being sent out [in Ubuntu NetworkManager testuite]
Hi there,
Back at the desk now. All I can do are naive things, so here's some of
those (inline):
On Fri, Dec 21, 2018 at 10:42:10PM +, Iain Lane wrote:
> Thanks Simon. I'm more or less offline for the holidays from now on, but
> I did test this and it didn't resolve the problem, I'm sorry to say.
>
> On Thu, Dec 20, 2018 at 11:36:46PM +, Simon Kelley wrote:
> > /* First time found, do fast RA. */
> > if (template->if_index != if_index ||
> > !IN6_ARE_ADDR_EQUAL(>local6, local))
I added some logging (inside the block, but *before* the new code to
update template->{if_index,local6}) to print out which condition was
failing to cause us to keep entering this loop. It's the second part
(!IN6_ARE_ADDR_EQUAL(...)). Short excerpt at the bottom¹.
Not sure if that output teaches you anything.
I looked at what ra_start_unsolicited() was doing, to see if I could add
an extra condition to prevent entering the branch multiple times for the
same context. This² works for the NetworkManager, but I don't know if
it's a sensible approach.
> > {
> > + template->if_index = if_index;
> > + template->local6 = *local;
> > ra_start_unsolicited(param->now, template);
> > param->newone = 1;
> > }
> > -
> > - template->if_index = if_index;
> > - template->local6 = *local;
(Don't you need to keep doing this for the pre-existing case? Done that in my ²
below.)
Thanks!
--
Iain Lane [ i...@orangesquash.org.uk ]
Debian Developer [ la...@debian.org ]
Ubuntu Developer [ la...@ubuntu.com ]
¹ Jan 7 11:58:40 dnsmasq-dhcp[2867]: RTR-ADVERT(veth42) 2600::
Jan 7 11:58:40 dnsmasq[2867]: Doing the bad thing: template->if_index !=
if_index? no, !IN6_ARE_ADDR_EQUAL(>local6, local)? yes
(2600:::::::0001 !=
2600::::2c9a:36ff:fea9:bc93)
Jan 7 11:58:40 dnsmasq[2867]: Doing the bad thing: template->if_index !=
if_index? no, !IN6_ARE_ADDR_EQUAL(>local6, local)? yes
(2600::::2c9a:36ff:fea9:bc93 !=
2600:::::::0001)
Jan 7 11:58:40 dnsmasq-dhcp[2867]: RTR-ADVERT(veth42) 2600::
Jan 7 11:58:40 dnsmasq[2867]: Doing the bad thing: template->if_index !=
if_index? no, !IN6_ARE_ADDR_EQUAL(>local6, local)? yes
(2600:::::::0001 !=
2600::::2c9a:36ff:fea9:bc93)
Jan 7 11:58:40 dnsmasq[2867]: Doing the bad thing: template->if_index !=
if_index? no, !IN6_ARE_ADDR_EQUAL(>local6, local)? yes
(2600::::2c9a:36ff:fea9:bc93 !=
2600:::::::0001)
Jan 7 11:58:40 dnsmasq-dhcp[2867]: RTR-ADVERT(veth42) 2600::
Jan 7 11:58:40 dnsmasq[2867]: Doing the bad thing: template->if_index !=
if_index? no, !IN6_ARE_ADDR_EQUAL(>local6, local)? yes
(2600:::::::0001 !=
2600::::2c9a:36ff:fea9:bc93)
Jan 7 11:58:40 dnsmasq[2867]: Doing the bad thing: template->if_index !=
if_index? no, !IN6_ARE_ADDR_EQUAL(>local6, local)? yes
(2600::::2c9a:36ff:fea9:bc93 !=
2600:::::::0001)
²
diff --git a/src/dhcp6.c b/src/dhcp6.c
index 56b2532..d8bbf28 100644
--- a/src/dhcp6.c
+++ b/src/dhcp6.c
@@ -655,14 +655,18 @@ static int construct_worker(struct in6_addr *local, int
prefix,
is_same_net6(local, >end6, template->prefix))
{
/* First time found, do fast RA. */
- if (template->if_index != if_index ||
!IN6_ARE_ADDR_EQUAL(>local6, local))
+ if ((template->if_index != if_index ||
!IN6_ARE_ADDR_EQUAL(>local6, local)) &&
template->ra_short_period_start == 0)
{
+ template->if_index = if_index;
+ template->local6 = *local;
ra_start_unsolicited(param->now, template);
param->newone = 1;
}
-
- template->if_index = if_index;
- template->local6 = *local;
+ else
+{
+ template->if_index = if_index;
+ template->local6 = *local;
+}
}
}
signature.asc
Description: PGP signature
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] Infinite(?) RTR-ADVERTs being sent out [in Ubuntu NetworkManager testuite]
Thanks Simon. I'm more or less offline for the holidays from now on, but
I did test this and it didn't resolve the problem, I'm sorry to say.
Cheers,
Iain
On Thu, Dec 20, 2018 at 11:36:46PM +, Simon Kelley wrote:
> Patch below is untested because I'm away from my test rig, but it would
> seem to do the right thing, ie set template->if_index _before_ calling
> ra_start_unsolicited() so that if we re-enter here via an async event it
> doesn't get called again.
>
> If you could test it in your harness, that would be great :)
>
> Cheers,
>
> Simon.
>
>
>
> diff --git a/src/dhcp6.c b/src/dhcp6.c
> index 3932cc7..4082504 100644
> --- a/src/dhcp6.c
> +++ b/src/dhcp6.c
> @@ -650,12 +650,11 @@ static int construct_worker(struct in6_addr
> *local, int prefix,
> /* First time found, do fast RA. */
> if (template->if_index != if_index ||
> !IN6_ARE_ADDR_EQUAL(>local6, local))
> {
> + template->if_index = if_index;
> + template->local6 = *local;
> ra_start_unsolicited(param->now, template);
> param->newone = 1;
> }
> -
> - template->if_index = if_index;
> - template->local6 = *local;
> }
>
>}
>
--
Iain Lane [ i...@orangesquash.org.uk ]
Debian Developer [ la...@debian.org ]
Ubuntu Developer [ la...@ubuntu.com ]
signature.asc
Description: PGP signature
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] Infinite(?) RTR-ADVERTs being sent out [in Ubuntu NetworkManager testuite]
On Thu, Dec 20, 2018 at 01:06:30PM +, Iain Lane wrote: > dnsmasq-dhcp[2010]: RTR-ADVERT(veth42) 2600:: > > repeating over and over. You can view a log file including all the > dnsmasq log entries at [2] - it's huge though, so I suggest downloading > it and using a real text editor instead of your browser. > > This test starts dnsmasq in ra-only mode, and that seems to be the case > that's broken. Since this started happening in a new version, I was able > to bisect 2.79 to 2.80, and I found that commit > 0a496f059c1e9d75c33cce4c1211d58422ba4f62 is the first bad commit. > Indeed, reverting that on master causes the NM testsuite to start > passing again. I attached gdb to dnsmasq while it was doing this. Here's the stack trace of what was going on at the time: (gdb) bt #0 0x7fb1b5a7cfd4 in __GI___libc_write (fd=10, buf=0x55c255462ce8, nbytes=62) at ../sysdeps/unix/sysv/linux/write.c:26 #1 0x55c253c4fb94 in log_write () at log.c:179 #2 0x55c253c502fd in my_syslog (priority=6, format=0x55c253c77e39 "RTR-ADVERT(%s) %s") at log.c:389 #3 0x55c253c5de2e in add_prefixes (local=0x55c25545d65c, prefix=64, scope=0, if_index=13, flags=4, preferred=3600, valid=3600, vparam=0x7ffcd1e77ce0) at radv.c:725 #4 0x55c253c493a0 in iface_enumerate (family=10, parm=0x7ffcd1e77ce0, callback=0x55c253c5d829 ) at netlink.c:268 #5 0x55c253c5cad4 in send_ra_alias (now=1545314006, iface=13, iface_name=0x7ffcd1e77e1c "veth42", dest=0x0, send_iface=13) at radv.c:291 #6 0x55c253c5d826 in send_ra (now=1545314006, iface=13, iface_name=0x7ffcd1e77e1c "veth42", dest=0x0) at radv.c:553 #7 0x55c253c5e0f8 in periodic_ra (now=1545314006) at radv.c:808 #8 0x55c253c3e256 in lease_update_file (now=1545314006) at lease.c:355 #9 0x55c253c52e7e in dhcp_construct_contexts (now=1545314006) at dhcp6.c:789 #10 0x55c253c36812 in newaddress (now=1545314006) at network.c:1721 #11 0x55c253c39902 in async_event (pipe=8, now=1545314006) at dnsmasq.c:1413 #12 0x55c253c38e71 in main (argc=11, argv=0x7ffcd1e78258) at dnsmasq.c:1084 There's a few places you can get an async_event of EVENT_NEWADDR. Some further grubbing around shows that it's coming from iface_enumerate() -> nl_async() -> async_event(EVENT_NEWADDR). You'll see that iface_enumerate() is frame #4 in that trace too, so this smells like it might be a source of infinite recursion to me: every time we call add_prefixes() we also queue another call of the same thing - the new code is to blame via dhcp_construct_contexts() calling construct_worker() to enter the new block setting param->newone = 1, which is checked in dhcp_construct_options(). What's not clear to me is how best to cut this off. If we only called the new code one time that would probably solve it... Cheers, -- Iain Lane [ i...@orangesquash.org.uk ] Debian Developer [ la...@debian.org ] Ubuntu Developer [ la...@ubuntu.com ] signature.asc Description: PGP signature ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
[Dnsmasq-discuss] Infinite(?) RTR-ADVERTs being sent out [in Ubuntu NetworkManager testuite]
Hi there, Forgive my ignorance on all matters dnsmasq/ipv6 - I'm coming to this from the 'desktop' end and I've not got much experience here. We noticed in Ubuntu that our NetworkManager testsuite started failing once we had copied dnsmasq 2.80-1 from unstable. What this testsuite does (you can read the code at [1] if you're interested) is start dnsmasq in the given mode (supplied by the testcase), create some fake interfaces, and verify that the interfaces come up properly. The testcase "test_auto_ip6_raonly_no_pe" started failing. What we see is that the test times out, and when that happens the dnsmasq log file is filled with dnsmasq-dhcp[2010]: RTR-ADVERT(veth42) 2600:: repeating over and over. You can view a log file including all the dnsmasq log entries at [2] - it's huge though, so I suggest downloading it and using a real text editor instead of your browser. This test starts dnsmasq in ra-only mode, and that seems to be the case that's broken. Since this started happening in a new version, I was able to bisect 2.79 to 2.80, and I found that commit 0a496f059c1e9d75c33cce4c1211d58422ba4f62 is the first bad commit. Indeed, reverting that on master causes the NM testsuite to start passing again. Can anyone help shed some light please? Are we looking at a dnsmasq bug, or something that our testsutie is doing wrong? If any more information would be helpful then I'm more than happy to provide it. Cheers all, -- Iain Lane [ i...@orangesquash.org.uk ] Debian Developer [ la...@debian.org ] Ubuntu Developer [ la...@ubuntu.com ] [1] https://git.launchpad.net/network-manager/tree/debian/tests?h=disco - nm.py & network_test_base.py [2] https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-disco/disco/amd64/n/network-manager/20181218_194950_de909@/log.gz signature.asc Description: PGP signature ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
