Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NM renderer: set default IPv6 addr-gen-mode for all interfaces to EUI64 #4291

Merged
merged 1 commit into from
Aug 4, 2023

Conversation

ani-sinha
Copy link
Contributor

@ani-sinha ani-sinha commented Jul 25, 2023

By default, NetworkManager renderer in cloud-init does not set any specific
method for IPV6 addr-gen-mode in the keyfiles it writes. Hence, implicitly the
mode is set to `eui64` in the absence of any global addr-gen-mode option in
NetworkManager configuration.
Later when other interfaces get added via D-Bus API or by using nmcli commands
without explictly setting an addr-gen-mode, NM auto generates new profiles for
those interfaces with addr-gen-mode set to `stable-privacy`. This introduces
inconsistency of configurations between interfaces based on how they were
added. This can cause problems for the customers.

In this change,  cloud-init overrides NetworkManager's preferred default of
`stable-privacy` to use EUI64 using a drop in NetworkManager configuration
file. This setting can be overriden by using global-connection-defaults
setting in /etc/NetworkManager/NetworkManager.conf file.

RHBZ: 2188388
Signed-off-by: Ani Sinha <anisinha@redhat.com>

@ani-sinha
Copy link
Contributor Author

TestNetworkManagerRendering will break because the test compares all generated files in
scripts_dir = "/etc/NetworkManager/system-connections" and the config is created in /etc/NetworkManager/conf.d/ .
Please advice if this is a good direction. The details in Red Hat BZ https://bugzilla.redhat.com/show_bug.cgi?id=2188388 .

@ani-sinha
Copy link
Contributor Author

@blackboxsw any suggestions for this?

@ani-sinha
Copy link
Contributor Author

@TheRealFalcon

@TheRealFalcon TheRealFalcon self-requested a review August 1, 2023 21:33
@TheRealFalcon TheRealFalcon self-assigned this Aug 1, 2023
@TheRealFalcon
Copy link
Member

@ani-sinha The proposal makes sense to me. My only concern at this point would be backwards compatibility with what we have already released.

The BZ says Note that cloud-init does not explicitly configure eui64 in NetworkManager. But it gets it implicitly, because it does not specify the addr-gen-mode in the files that it writes. Is this going to be true everywhere or would it be distro-specific? If addr-gen-mode is not set, will it always be implicitly set to eui64? If so, then I think this change makes sense to apply everywhere upstream as this mode is already used by profiles created by cloud-init. If not, it may be better to keep it as is as to not break other distros using this functionality.

Is this something that can be fixed in NetworkManager? It's kind of strange that ipv6.addr-gen-mode=default and an unspecified addr-gen-mode can mean two different things.

@ani-sinha
Copy link
Contributor Author

The proposal makes sense to me. My only concern at this point would be backwards compatibility with what we have already released.

This request came from our NM team directly so let me reach out to them. They maintain NM and know more about the behavior than me.

@thom311
Copy link

thom311 commented Aug 2, 2023

Is this going to be true everywhere or would it be distro-specific?

It's not disto-specific. NetworkManager avoids doing disto-specific stuff.

If addr-gen-mode is not set, will it always be implicitly set to eui64?

If you write a keyfile (to /{usr/lib,etc,run}/NetworkManager/system-connections) or an ifcfg-rh file (to /etc/sysconfig/network-scripts) and don't explicitly set the address generation mode in the file, then the profile will be interpreted to have ipv6.addr-gen-mode=default-or-eui64. On the other hand, if you add a profile via D-Bus API or via libnm API (for example what you would get with a nmcli connection add command that does not explicitly set ipv6.addr-gen-mode), then the default would be ipv6.addr-gen-mode=default (and that setting would also be explicitly persisted to on disk, otherwise reading it back would change it to "default-or-eui64").

The meaning of "default" is to lookup global connection defaults in NetworkManager.conf, and if unspecified, fallback to "stable-privacy". The meaning of "default-or-eui64" is to lookup global connection defaults and fallback to "eui64".

cloud-init creates profiles by writing them to disk. It does not specifiy the addr-gen-mode in the file, consequently it gets default-or-eui64 , which -- unless the user overwrite the default in NetworkManager.conf -- results in EUI64.

Later, on such a system, if the user types nmcli connection add or merely plugs in a new interface, then a new profile will be created with ipv6.addr-gen-mode=default. Resulting in "stable-privacy".

"stable-privacy" has various benefits, in particular it can generate another address when it detects an address conflict. It works not great for certain users, who expect to anticipate (know in advance) the IPv6 address -- which they could with EUI64. So on a system where a user uses cloud-init, we assume the user would prefer EUI64.

The branch achieves that by changing the default in NetworkManager.

Note that a user can still override this either per-profile (by setting ipv6.addr-gen-mode in the profile) or with a higher-priority file in /etc/NetworkManager/conf.d.

If so, then I think this change makes sense to apply everywhere upstream as this mode is already used by profiles created by cloud-init

Note that the profile explicitly created by cloud-init already get ipv6.addr-gen-mode=default-or-eui64 (thus, usually getting EUI64). This change is for profiles that are created by the user afterwards (either explicitly, or simply by plugging in a new interface).

Is this something that can be fixed in NetworkManager? It's kind of strange that ipv6.addr-gen-mode=default and an unspecified addr-gen-mode can mean two different things.

Although it's odd, there is nothing to fix. It's just, the default differs depending on whether you create files or whether the profiles are created via other means (D-Bus, libnm, nmcli, autogenerated "Wired connection 1"). That is to support backward compatibility, where old files are still treated as in the past, when only EUI64 is used while the rest of NetworkManager prefers to default to "stable-privacy".

In particular with this pull-request, "cloud-init" overrides NetworkManager's preferred default of "stable-privacy" to use EUI64. There simply is no default that suits all cases optimally, so I think cloud-init overriding the default makes sense, as we possibly run in an environment where EUI64 is better (cloud??).

@TheRealFalcon
Copy link
Member

@thom311 , thanks for the additional context. That helps a lot. Given your explanation, I agree that this change makes the most sense. @ani-sinha , feel free to continue with the current PR.

@ani-sinha ani-sinha changed the title [RFC] NM renderer: set default IPv6 addr-gen-mode for all interfaces to eui64 NM renderer: set default IPv6 addr-gen-mode for all interfaces to EUI64 Aug 3, 2023
By default, NetworkManager renderer in cloud-init does not set any specific
method for IPV6 addr-gen-mode in the keyfiles it writes. Hence, implicitly the
mode is set to `eui64` in the absence of any global addr-gen-mode option in
NetworkManager configuration.
Later when other interfaces get added via D-Bus API or by using nmcli commands
without explictly setting an addr-gen-mode, NM auto generates new profiles for
those interfaces with addr-gen-mode set to `stable-privacy`. This introduces
inconsistency of configurations between interfaces based on how they were
added. This can cause problems for the customers.

In this change,  cloud-init overrides NetworkManager's preferred default of
`stable-privacy` to use EUI64 using a drop in NetworkManager configuration
file. This setting can be overriden by using global-connection-defaults
setting in /etc/NetworkManager/NetworkManager.conf file.

RHBZ: 2188388
Signed-off-by: Ani Sinha <anisinha@redhat.com>
@ani-sinha
Copy link
Contributor Author

TestNetworkManagerRendering will break because the test compares all generated files in scripts_dir = "/etc/NetworkManager/system-connections" and the config is created in /etc/NetworkManager/conf.d/

Fixed the unit tests.

Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@TheRealFalcon TheRealFalcon merged commit d41264c into canonical:main Aug 4, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants