Later we want to fully support wireguard devices. Also,
possibly activating a generic profile in a wireguard device
would make sense.
Anyway, for the moment, just prevent that from happening
by explicitly marking the device as unmanaged.
(cherry picked from commit e3bd482329)
Currently, NMDeviceWireguard does neither set connection_type_check_compatible
nor implement check_connection_compatible. That means, it appears to be compatible
with every connection profile, which is obviously wrong.
Allow devices not to implement check_connection_compatible() and avoid the issue
by rejecting profiles by default.
(cherry picked from commit baa0008313)
NMConnection is an interface, which is implemented by the types
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).
NMSettingsConnection does a lot of things already:
1) it "is-a" NMDBusObject and exports the API of a connection profile
on D-Bus
2) it interacts with NMSettings and contains functionality
for tracking the profiles.
3) it is the base-class of types like NMSKeyfileConnection and
NMIfcfgConnection. These handle how the profile is persisted
on disk.
4) it implements NMConnection interface, to itself track the
settings of the profile.
3) and 4) would be better implemented via delegation than inheritance.
Address 4) and don't let NMSettingsConnection implemente the NMConnection
interface. Instead, a settings-connection references now a NMSimpleConnection
instance, to which it delegates for keeping the actual profiles.
Advantages:
- by delegating, there is a clearer separation of what
NMSettingsConnection does. For example, in C we often required
casts from NMSettingsConnection to NMConnection. NMConnection
is a very trivial object with very little logic. When we have
a NMConnection instance at hand, it's good to know that it is
*only* that simple instead of also being an entire
NMSettingsConnection instance.
The main purpose of this patch is to simplify the code by separating
the NMConnection from the NMSettingsConnection. We should generally
be aware whether we handle a NMSettingsConnection or a trivial
NMConnection instance. Now, because NMSettingsConnection no longer
"is-a" NMConnection, this distinction is apparent.
- NMConnection is implemented as an interface and we create
NMSimpleConnection instances whenever we need a real instance.
In GLib, interfaces have a performance overhead, that we needlessly
pay all the time. With this change, we no longer require
NMConnection to be an interface. Thus, in the future we could compile
a version of libnm-core for the daemon, where NMConnection is not an
interface but a GObject implementation akin to NMSimpleConnection.
- In the previous implementation, we cannot treat NMConnection immutable
and copy-on-write.
For example, when NMDevice needs a snapshot of the activated
profile as applied-connection, all it can do is clone the entire
NMSettingsConnection as a NMSimpleConnection.
Likewise, when we get a NMConnection instance and want to keep
a reference to it, we cannot do that, because we never know
who also references and modifies the instance.
By separating NMSettingsConnection we could in the future have
NMConnection immutable and copy-on-write, to avoid all unnecessary
clones.
Add a helper function nm_device_parent_find_for_connection() to
unify implementations of setting the parent in update_connection().
There is some change in behavior, in particular for nm-device-vlan.c,
which no longer compares the link information from platform. But
update_connection() is anyway a questionable concept, only used
for external assumed connection (which itself, is questionable). Meaning,
update_connection() is a hack not science, and it's not at all clear
what the correct behavior is.
Also, note how vlan's implementation differs from all others. Why?
Should we always resort to also check the information from platform?
Either way, one of the two approaches should be used consistently and
nm_device_parent_find_for_connection() opts to not consult platform
cache.
These should be logged on DEBUG level:
<warn> platform-linux: do-change-link[2]: failure changing link: failure 97 (Address family not supported by protocol)
<warn> device (wlo1): failed to enable userspace IPv6LL address handling (unspecified)
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/10
For dynamic IP methods (DHCP, IPv4LL, WWAN) the route metric is set at
activation/renewal time using the value from static configuration. To
support runtime change we need to update the dynamic configuration in
place and tell the DHCP client the new value to use for future
renewals.
https://bugzilla.redhat.com/show_bug.cgi?id=1528071
When the IPv4 method is 'auto' and there are static addresses
configured in the connection, start a DAD probe for the static
addresses and apply them immediately on success, without waiting for
DHCP to complete.
Note that if the static address is in the same subnet of the DHCP one,
when we add the DHCP address we want it to be primary and so we will
remove the static address temporarily to achieve the right order of
addresses.
https://bugzilla.redhat.com/show_bug.cgi?id=1369905
Note the special error codes NM_UTILS_ERROR_CONNECTION_AVAILABLE_*.
This will be used to determine, whether the profile is fundamentally
incompatible with the device, or whether just some other properties
mismatch. That information will be importand during a plain `nmcli
connection up`, where NetworkManager searches all devices for a device
to activate. If no device is found (and multiple errors happened),
we want to show the error that is most likely relevant for the user.
Also note, how NMDevice's check_connection_compatible() uses the new
class field "device_class->connection_type_check_compatible" to simplify
checks for compatible profiles.
The error reason is still unused.
We previously kept any acd-manager running if the device was
disconnected. It was possible to trigger a crash by setting a long
dad-timeout and interrupting the activation request:
nmcli con add type ethernet ifname eth0 con-name eth0+ ip4 1.2.3.4/32
nmcli con mod eth0+ ipv4.dad-timeout 10000
nmcli -w 2 con up eth0+
nmcli con down eth0+
After this, the n-acd timer would fire after 10 seconds and try to
disconnect an already disconnected device, throwing the assertion:
NetworkManager:ERROR:src/devices/nm-device.c:9845:
activate_stage5_ip4_config_result: assertion failed: (req)
Fixes: 28f6e8b4d2
We commonly don't use the glib typedefs for char/short/int/long,
but their C types directly.
$ git grep '\<g\(char\|short\|int\|long\|float\|double\)\>' | wc -l
587
$ git grep '\<\(char\|short\|int\|long\|float\|double\)\>' | wc -l
21114
One could argue that using the glib typedefs is preferable in
public API (of our glib based libnm library) or where it clearly
is related to glib, like during
g_object_set (obj, PROPERTY, (gint) value, NULL);
However, that argument does not seem strong, because in practice we don't
follow that argument today, and seldomly use the glib typedefs.
Also, the style guide for this would be hard to formalize, because
"using them where clearly related to a glib" is a very loose suggestion.
Also note that glib typedefs will always just be typedefs of the
underlying C types. There is no danger of glib changing the meaning
of these typedefs (because that would be a major API break of glib).
A simple style guide is instead: don't use these typedefs.
No manual actions, I only ran the bash script:
FILES=($(git ls-files '*.[hc]'))
sed -i \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\>\( [^ ]\)/\1\2/g' \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\> /\1 /g' \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\>/\1/g' \
"${FILES[@]}"
In device_ipx_changed() we only keep track of dad6_failed_addrs
addresses if the device's state is > DISCONNECTED.
For the same reason, we should also do that in queued_ip_config_change().
But it's worse. If the device is in state disconnected, and the user
externally adds IPv6 addresses, we will end up in queued_ip_config_change().
It is easily possible that "need_ipv6ll" ends up being TRUE, which results
in a call to check_and_add_ipv6ll_addr() and later possibly
ip_config_merge_and_apply (self, AF_INET6, TRUE);
This in turn will modify the IP configuration on the device, although
the device may be externally managed and NetworkManager shouldn't touch it.
https://bugzilla.redhat.com/show_bug.cgi?id=1593210
We first iterate over addresses that might have failed IPv6 DAD and
update the state in NMNDisc.
However, while we do that, don't yet invoke the changed signal.
Otherwise, we will invoke it multiple times (in case multiple addresses
failed). Instead, keep track of whether something changed, and handle
it once a bit later.
Whenever we process queued IP changes, we must handle all pending
dad6_failed_addrs. This is, to ensure we don't accumulate more
and more addresses in the list.
Rework the code, by stealing the entire list once at the beginning
dad6_failed_addrs = g_steal_pointer (&priv->dad6_failed_addrs);
and free it at the end:
g_slist_free_full (dad6_failed_addrs, (GDestroyNotify) nmp_object_unref);
This makes it easier to see, that we always process all addresses in
priv->dad6_failed_addrs.
There is no change in behavior, however don't handle dad6_failed_addrs
and dad6_ip6_config in the same block.
While both parts are related to IPv6 DAD, they do something rather
different:
- the first block, checks all candidates from dad6_failed_addrs whether
they actually indicate DAD failed, and handles them by notifying
NMNDisc about failed addresses.
- the second block, checks whether we have now all addresses from
dad6_ip6_config that we are waiting for.
Split the blocks.
We don't need to cancel the current idle-action and schedule a new
one. Just return and wait to be called again.
Also, drop the logging. Similarly, we don't log the postponing for
the previous case either.
We also cancel the idle handler
nm_clear_g_source (&priv->queued_ip_config_id_x[IS_IPv4])
which means, nobody is going to process these addresses (at least
for the moment).
The purpose of "dad6_failed_addrs" is to keep track of addresses that
might be interesting for checking about DAD failures. If we are no
longer reacting on IP changes (because the idle handler was removed),
we also no longer need these addresses.
This simplifies commit 31ca7962f8.
We don't need the boolean flags like "queued_ip4_config_pending" to
track whether we received any platform signals while being not yet
initialized in platform (udev, NM_UNMANAGED_PLATFORM_INIT).
In general, as long as the device is NM_UNMANAGED_PLATFORM_INIT,
all platform signals are ignored. And when the device becomes managed,
we schedule anyway an initial config-change.
"debug" was documentation in `man NetworkManager.conf` as a valid
logging backend. However, it was completely ignored by
nm_logging_syslog_openlog().
In fact, it makes not sense. Passing debug = TRUE to
nm_logging_syslog_openlog(), means that all messages will be
printed to stderr in addition to syslog/journal. However, when
NetworkManager is daemonizing, stderr is closed.
Whether NetworkManager is daemonizing depends entirely on command
line options --no-daemon and --debug. Hence, the logging backend "debug"
from the configuration file either conflicts or is redundant.
Also, adjust logging backend description in `man NetworkManager.conf`.
Also, log a warning about invalid/unsupported logging backend.
If commit_mtu() is called multiple times and dev->get_configured_mtu()
returns @is_user_config=FALSE, only the first call changes the
MTU. So, for example, when the parent MTU of a VLAN changes, we apply
the new MTU only the first time.
Rework the handling of MTU in NMDevice, and store the source of the
configured MTU. When commit_mtu() is called again, we ask the subclass
a MTU to configure and apply it only if the source has higher
priority, or when the parent MTU changed.
Instead of returning a boolean @is_user_config value from
get_configured_mtu(), return an mtu-source enum with possible values
NONE,CONNECTION. This enum will be expanded later; for now there is no
change in behavior.
Rather trivial change. Return-early, to completely handle the simpler
case (the success case) first. In the failure case, we only need
extra effort to generate a nice error message.
