[2.82 regression] router announcements have 'forever' lifetime by default

[Iain Lane]

The default lifetime was changed to 1 day in 2.82 in the change corresponding to this changelog entry:

 Change default lease time for DHCPv6 to one day.

Fine, but the same commit also did this:

 Alter calculation of preferred and valid times in router
 advertisements, so that these do not have a floor applied
 of the lease time in the dhcp-range if this is not explicitly
 specified and is merely the default.

And that change is buggy and causes advertisements to have infinite lifetime, when you are using the default.

See this thread

  http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q3/014341.html

[Iain Lane (laney)]

I proposed a patch on the thread, waiting for feedback.

[Lucas Kanashiro (lucaskanashiro)]

Thanks for filing this bug Iain.

I am tagging it as server-next and it should be worked on by our team soon. As a source of information for the person who will work on it here is the patch submitted by Iain:

http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q3/014345.html

And in the reply of this message a flaw seems to be found and a new patch was proposed (asked for review):

http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q3/014346.html

[Iain Lane (laney)]

I think either of the patches is probably fine, no idea what any upstream reviewer will prefer.

I guess it would be good to upload one of them sooner rather than later though, to (1) unbreak dnsmasq, (2) unblock network-manager.

[Christian Ehrhardt  (paelzer)]

The patch suggested at http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q3/014346.html isn't applied yet.
I guess this waits for feedback if it worked before they apply it.

To get this going here a PPA and an associated MP:
PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4266
MP: https://code.launchpad.net/~paelzer/ubuntu/+source/dnsmasq/+git/dnsmasq/+merge/390832

[Christian Ehrhardt  (paelzer)]

BTW I randomly picked the latter patch of the two as no one seems to have jumped on the first one and said "yes let us apply it".

[Christian Ehrhardt  (paelzer)]

Arrr 1.0 source, need to apply differently ....

[Christian Ehrhardt  (paelzer)]

Ok changed MP and PPA to work for 1.0 source

[Christian Ehrhardt  (paelzer)]

FYI: builds complete - Running the test builds of network-manager_1.26.2-1ubuntu1.dsc in groovy against the proposed and the PPA version.

[Christian Ehrhardt  (paelzer)]

The tests confirmed the issue that Iain reported and that the fix works.
Also no other issues popped up in the NM test using it.
autopkgtest [08:47:12]: @@@@@@@@@@@@@@@@@@@@ summary
wpa-dhclient PASS
nm.py PASS
killswitches-no-urfkill PASS
urfkill-integration PASS

And while https://bileto.ubuntu.com/excuses/4266/groovy.html looks pretty bad after we skip the known badtests not much is left and of those nothing seems to be due to this change.

I replied upstream hoping they will accept the change so we can then push it to groovy (sooner or later we have to even without upstream saying "yes" but I'd prefer to pick exactly what is committed).
=> http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q3/014387.html

Actually since this holds back other things, let us upload to groovy now (it essentially reverts to old behavior) but come back in a week to potentially update to whatever upstream did by then.

[Launchpad Janitor (janitor)]

This bug was fixed in the package dnsmasq - 2.82-1ubuntu1

---------------
dnsmasq (2.82-1ubuntu1) groovy; urgency=medium

  * src/radv.c: avoid leases to be issued forever when not set (LP: #1894619)

 -- Christian Ehrhardt <email address hidden> Wed, 16 Sep 2020 14:26:58 +0200

[Christian Ehrhardt  (paelzer)]

We finally got an answer to our questions/pings (thanks Simon)
=> https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2021q2/015134.html

TL;DR: this might be wanted new behavior :/ ?!

@Laney - You initially started this when we have faced this bug
I wonder now if we should indeed drop our delta and resolve the fallout in other places like the formerly breaking tests.
You have back then thought deeper into this, while I only carried this delta a few times.
Therefore I'd appreciate if you could let me know what you think of that reply.

P.S. Because if we drop it then I guess we better do that now than e.g. late in 22.04 :-)

[Iain Lane (laney)]

On Mon, Jun 21, 2021 at 05:53:41AM -0000, Christian Ehrhardt  wrote:
> We finally got an answer to our questions/pings (thanks Simon)
> => https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2021q2/015134.html
>
> TL;DR: this might be wanted new behavior :/ ?!
>
> @Laney - You initially started this when we have faced this bug
> I wonder now if we should indeed drop our delta and resolve the fallout in other places like the formerly breaking tests.
> You have back then thought deeper into this, while I only carried this delta a few times.
> Therefore I'd appreciate if you could let me know what you think of that reply.
>
> P.S. Because if we drop it then I guess we better do that now than e.g.
> late in 22.04 :-)

I don't know to be honest. What do you think, does that argument make
sense? I don't really understand why it should be different whether
you're using DHCPv6 or SLAAC, but maybe it's just me not getting
something right now.

If it's fine, then feel free to drop the patch and upload a NM with
adjusted regexes (preferrably committing to the VCS too).

Cheers,

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[Christian Ehrhardt  (paelzer)]

Yeah I'm also not entirely sure.
But I'm assuming that the maintainers insight into all the interactions is better than mine and therefore tend to follow.

About the "why the difference DHCPv6 vs SLAAC" I think that is fine because the code now allows to behave different (reasonable). And the defaults as configured by NM or anything else could configure lifetimes for DHCPv6+SLAAC then no behavior change would happen.

Thereby (with a lot of un-sureness left) I'd intend to drop this change now (somewhat early/mid in 21.10) to identify any worse fallout better now than later.

I've closed my Debian bug that I spawned back in the day.
I will prepare an upload of dnsmasq, then we will see which tests or behavior break on it nowadays.

[Christian Ehrhardt  (paelzer)]

According to the discussion dnsmasq will revert to the upstream intended behavior.
I added an open impish task to reflect that.

At the same time this makes network-mamager expose the test issue we initially found it with.
An example can already be seen in
PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4595/+packages
TestLog: https://autopkgtest.ubuntu.com/results/autopkgtest-impish-ci-train-ppa-service-4595/impish/amd64/n/network-manager/20

Note: I'm not sure if all of the test fails there are due to this change.

[Christian Ehrhardt  (paelzer)]

I have not found any other test issues than the expected NM one.
So I'm uploading since - to some extend - we want to see if anything else breaks.

[Christian Ehrhardt  (paelzer)]

As expected this is now ready in proposed and blocked on the NM test, example:
https://autopkgtest.ubuntu.com/results/autopkgtest-impish/impish/amd64/n/network-manager/20210701_151447_a29ef@/log.gz

[Christian Ehrhardt  (paelzer)]

Ok, as planned I can reproduce this in local autopkgtest.

Multiple hits on:
AssertionError: Regex didn't match: 'inet6 2600::[0-9a-f:]+/64 scope global (?:tentative )?(?:mngtmpaddr )?(?:noprefixroute )?dynamic' not found in '14: eth42@veth42: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000\n link/ether d6:77:5c:e0:6e:89 brd ff:ff:ff:ff:ff:ff\n inet6 2600::19/128 scope global dynamic noprefixroute \n valid_lft 86399sec preferred_lft 86399sec\n inet6 2600::4d67:83f1:60b3:9385/64 scope global noprefixroute \n valid_lft forever preferred_lft forever\n inet6 fe80::892a:6b8:c86a:50a2/64 scope link noprefixroute \n valid_lft forever preferred_lft forever\n'

Due to the changes this is dynamic->forever for some, but not all, of the cases.
An MP has been proposed here [1] which in local VM based autopkgtest already succeeded.
@Iain/Desktop-team - would you have a look and if you agree upload that NM update?

[1]: https://code.launchpad.net/~paelzer/ubuntu/+source/network-manager/+git/network-manager/+merge/405081

[Christian Ehrhardt  (paelzer)]

The new NM tests were fine (even against the old dnsmasq due to the or in the regex).
NM hit a few non related systemd test issues - I re-triggered those.
Finally I did now start the new NM tests against the blocked dnsmasq.

Overall that should hopefully resolve both and let them migrate.

[Christian Ehrhardt  (paelzer)]

dnsmasq tested all fine with the new NM.
Only the odd systemd blocker holds us back now.

[Launchpad Janitor (janitor)]

This bug was fixed in the package network-manager - 1.30.0-1ubuntu4

---------------
network-manager (1.30.0-1ubuntu4) impish; urgency=medium

  * d/t/nm.py: adapt to changes dnsmasq behavior (LP: #1894619)

 -- Christian Ehrhardt <email address hidden> Fri, 02 Jul 2021 11:13:46 +0200

[Christian Ehrhardt  (paelzer)]

The DNSmasq change is in impish as well.
Since we (intentionally) referenced but not closed the bug by the upload I'll have to tag that manually.