Bug 1585078

Created attachment 1446696 [details]
[NM PATCH] device: don't try to change MTU on a disconnected device

Patch to fix the assertion in NM.

Created attachment 1446697 [details]
[NM-CI PATCH] ipv6: drop 1.12+ variant of @ipv6_add_static_address_manually_not_active

RFC NetworkManager-ci patch

(In reply to Beniamino Galvani from comment #1)
> Created attachment 1446696 [details]
> [NM PATCH] device: don't try to change MTU on a disconnected device
> 
> Patch to fix the assertion in NM.

lgtm


(In reply to Beniamino Galvani from comment #2)
> Created attachment 1446697 [details]
> [NM-CI PATCH] ipv6: drop 1.12+ variant of
> @ipv6_add_static_address_manually_not_active
> 
> RFC NetworkManager-ci patch


 - in queued_ip_config_change() we call check_and_add_ipv6ll_addr()
   for the reason explained by this code comment:

why is this desired? "external" shall mean, no touching at all.

(In reply to Thomas Haller from comment #3)
>  - in queued_ip_config_change() we call check_and_add_ipv6ll_addr()
>    for the reason explained by this code comment:
> 
> why is this desired? "external" shall mean, no touching at all.

The address gets added when the device is still disconnected and in "managed" sys-iface-state. The in-memory connection is created later.

I think this behavior is reasonable. When NM is not running the kernel adds a link-local address when the interface goes up and so users only have to add other addresses.

When NM is running and an interface is disconnected, there is no address configured. If users manually add an address and NM doesn't automatically add the LL one, I suspect IPv6 will not work or it will break in certain cases. Users will have to know that they need to change the addrgenmode and cycle the interface, or to manually add a LL address. I think this is a usability problem.

Maybe there is a usability problem.

NM initially keeps the device disconnected (and without link-local address).
Then, users start configuring the device outside of NM (without telling NetworkManager). NM should detect that, and from that point on no longer touch the device (at all). Clearly, users are in charge, it's up to them to setup the device to their liking.

If NetworkManager after that point (where it already notices somebody external is messing with the device), still interferes, it's racy. 
Also resetting EUI64 doesn't seem good to do by default. It reveals the MAC address of the user. Is that really what the user wants?


> When NM is not running the kernel adds a link-local address when the interface > goes up and so users only have to add other addresses.

I would rather say: if the link is set as addr-gen-mode=eui64, kernel will add a link-local address. Now, yes, with NetworkManager running, the link might be configured differently. But that is expected.


What we should do, make sure that the user can issue

  nmcli device set $DEV managed no

which should set the device !IFF_UP and re-enable addr-gen-mode=eui64 if the device was disconnected before -- on the other hand, if the device was activated, NM should leave it alone.

Likewise, when somebody stops NetworkManager, we should make sure to set previously disconnected devices !IFF_UP and addr-gen-mode=eui64.


The proper way to take away the device from NetworkManager is to first set it as unmanaged or stopping NetworkManager. I think these are really usability issues we need to fix.

(In reply to Thomas Haller from comment #5)
> Maybe there is a usability problem.
> 
> NM initially keeps the device disconnected (and without link-local address).
> Then, users start configuring the device outside of NM (without telling
> NetworkManager). NM should detect that, and from that point on no longer
> touch the device (at all). Clearly, users are in charge, it's up to them to
> setup the device to their liking.
> 
> If NetworkManager after that point (where it already notices somebody
> external is messing with the device), still interferes, it's racy. 
> Also resetting EUI64 doesn't seem good to do by default. It reveals the MAC
> address of the user. Is that really what the user wants?

In the majority of cases, if think so. It's what kernel does as well.

Regarding the racy behavior, we just add another address and not touch
the address the user added. Sure, the new LL arrives some time later,
but it would take some time even when NM is disabled and one brings the
device up.

> What we should do, make sure that the user can issue
> 
>   nmcli device set $DEV managed no
> 
> which should set the device !IFF_UP and re-enable addr-gen-mode=eui64 if the
> device was disconnected before -- on the other hand, if the device was
> activated, NM should leave it alone.
> 
> Likewise, when somebody stops NetworkManager, we should make sure to set
> previously disconnected devices !IFF_UP and addr-gen-mode=eui64.
> 
> 
> The proper way to take away the device from NetworkManager is to first set
> it as unmanaged or stopping NetworkManager. I think these are really
> usability issues we need to fix.

One of the goals of NM is to interfere as little as possible with
manual changes done externally by users. For IPv4, if you add an
address with iproute2 to a disconnected device, it just
works. Currently, with IPv6 too.

If we do the change you propose, I suspect that people will try to
configure the address manually, see that it doesn't work (like it did
in the past), and blame NetworkManager, possibly disabling it.

Note that the current behavior of adding the IPv6 link-local address
was deliberately chosen back in 2014 (0.995) when NM started to
managed IPv6 in userspace [1]. The commit explicitly mentions this
case:

   "To remain compliant with standards, if a user adds IPv6 addresses
    externally, NetworkManager must also immediately add an IPv6LL address
    for that interface too."

I think this would be a big change in behavior, that we should
document very well in release notes if we decide to adopt it.

After discussion on IRC, we agreed to keep the 1.12+ variant of the test (which checks that no LL address gets added by NM) because it describes a desirable behavior.

But updating NM to the new behavior is not a priority at the moment, and thus the 1.12+ test variant should be disabled; this bug will be kept open to track the issue.

Vladimir, can you please disable the test?

*** Bug 1592795 has been marked as a duplicate of this bug. ***

See also: https://bugzilla.redhat.com/show_bug.cgi?id=1616363#c3

We have now two variants of the test (for pre-1.11.2 and post behavior) and both are passing. 

https://github.com/NetworkManager/NetworkManager-ci/commit/3f7714a9a1bc485cfbbcecbb5f294745d264519a

Closing the bug.