When ModemManager become available, NetworkManager resets
GDBusObjectManagerClient object.
But there is a race condition if object-added is emitted before
modm_ensure_manager(), we need to check existing objects if we want to be
in sync with ModemManager.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1957
Also check for gateway equality when deduplicate routing entries. This
allows to support multiple routes to the same network using different
gateways. This is useful for Thread networks where multiple BRs route
to the same Thread network. If one of these BRs go offline, fallback to
a different router will be much quicker if multiple entries are present.
Note that quick fallback to a different router requires IPv6
reachability probe to be active. Typically Linux disables reachability
probes on Linux machines which act as IPv6 gateway (when forwarding is
enabled).
It might happen that write() returns -1, but the errno is not EINTR.
In that case, the length would be incremented by 1, and the data pointer
to the data being written would be moved back by 1 byte on every error.
Make it so that the function exits with an error if it indicates an error.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1971
Fixes: 3bda3fb60c ('nmtui: initial import of nmtui')
When the lease expires, the DHCP client emits a LEASE_UPDATE event
with a NULL l3cd. After returning from the handler, it sends
immediately a DHCP DISCOVER message to try to get a new lease.
It is important that when the DISCOVER gets sent the address is no
longer configured on the interface. Otherwise, the server could see
that it is already in use and assign a different one. Therefore,
remove the address synchronously when handling the event.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1532
Currently, when the agent manager is sent a registration request
containing UTF-8 characters, it will form an invalid error message
using only one of the bytes from the UTF-8 sequence, which causes
an assertion in glib to fail, which replaces the returned error message
with "[Invalid UTF-8]". It will also print an assertion failure to the
console, or crash NetworkManager on non-release builds.
This commit makes it so that it instead prints out the character in
hexadecimal form if it isn't normally printable, so that it is once
again a valid UTF-8 string.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1965
Fixes: a30cf19858 ('agent: add agent manager and minimal agent class')
A signal handler is not the only place where we need to clean up after
an in-progress readline() on exit; we may do so when erroring out as
well:
Before (not also the missing line break, which is part of the cleanup):
$ (sleep 10; nmcli c del 'Red Hat Wi-Fi')
$ nmcli --ask d wifi connect 'Red Hat Wi-Fi'
Passwords or encryption keys are required to access the wireless network 'Red Hat Wi-Fi'.
Password (802-11-wireless-security.psk): Error: Connection activation failed: The device's active connection disappeared.
$ [terminal messed up, no echo]
After:
$ (sleep 10; nmcli c del 'Red Hat Wi-Fi')
$ nmcli --ask d wifi connect 'Red Hat Wi-Fi'
Passwords or encryption keys are required to access the wireless network 'Red Hat Wi-Fi'.
Password (802-11-wireless-security.psk):
Error: Connection activation failed: The device's active connection disappeared.
$ hello [terminal echo fine, wheee]
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1959
In the future we might want to specify filters when requesting netlink
dumps; this requires that strict check is enabled on the socket.
When enabling strict check, we need to pass a full struct in the
netlink message, otherwise kernel ignores it.
This commit doesn't change behavior.
Adds an option in the connectivity section to change the timeout before
the interface is deemed "limited". Previously, it was hardcoded to
20 seconds, but for our usecase (failing over to cell modem if
hardwired ethernet drops), it's nice to be able to failover to another
interface more quickly.
The OVS interface can be matched via MAC address; in that case, the
"connection.interface-name" property of the connection is empty.
When populating the ovsdb, we need to pass the actual interface name
from the device, not the one from the connection.
Fixes: 830a5a14cb ('device: add support for OpenVSwitch devices')
https://issues.redhat.com/browse/RHEL-34617
The group interface is only used during activation; there is no need
to add a pending action for it, because when the device is in
activating state it already delays "startup-complete" via other
pending actions.
The daemon is now capable of understanding and removing these prefix
tags by itself. It is better than this is not a responsibility of the
secret agent because it requires changes in all secret agents to work
properly (see https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1536).
If the secret agent knows what these prefix tags are, it can remove them
only in the text that is displayed in the UI, but maintaining the
original string as the secret name that is returned to the daemon.
Secret agents that doesn't know what these prefix tags are won't do
anything with them, and they will also return the same string as secret
name, as expected. The only drawback is that they might display the full
string to the user, which is not a nice UX but it will at least work.
Also, allow to translate the secret name for the UI in libnmc.
Commit 345bd1b187 ('libnmc: fix secrets request on 2nd stage of 2FA
authentication') and commit 27c701ebfb ('libnmc: allow user input in
ECHO mode for 2FA challenges') introduced 2 new tags that hints for the
secret agents can have as prefix.
These tags were processed (and removed) in the secret agents, not in the
daemon. This is wrong because a system with an updated VPN plugin but a
not yet updated secret agent (like nm-plasma) will fail: it won't remove
the prefix and the daemon will save the secret with the prefix, i.e.
"x-dynamic-challenge:challenge-response" instead of just
"challenge-response". Then, VPN plugins doesn't recognize it, failing the
profile's activation. This is, in fact, an API break.
Also, if the VPN connection already existed before updating NM and the
VPN plugin, the secret flags are not added to the profile (they are only
added when the profile is created or modified). This causes the user's
first time response is saved to the profile, so the activation fails the
second and next times.
See:
- https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1536
- https://gitlab.gnome.org/GNOME/NetworkManager-openvpn/-/issues/142
Anyway, in a good design the daemon should contain almost all the logic
and the clients should keep as simple as possible. Fix above's problems
by letting the daemon to receive the secret names with the prefix
already included. The daemon will strip it and will know what it means.
Note that this is done only in the functions that saves the secrets from
the data received via D-Bus. For example, nm_setting_vpn_add_secret
doesn't need to do it because this value shouldn't come from VPN
plugin's hints.
Usually, when the method is "auto" we want to avoid configuring routes
until the automatic method completes. To achieve that, we clear the
"allow_routes_without_address" flag of l3cds when the method is "auto".
For VPNs, IP configurations with only routes are perfectly valid,
therefore set the flag.
The name "dhcp_enabled" is misleading because the flag is set for
method=auto, which doesn't necessarily imply DHCP. Also, it doesn't
convey what the flag is used for. Rename it to
"allow_routes_without_address".
An IPv4-over-IPv6 (or vice-versa) IPsec VPN can return IP
configurations with routes and without addresses. For example, in this
scenario:
+---------------+ +---------------+
| fd01::10/64 <-- VPN --> fd02::20/64 |
| host1 | | host2 |
+-------^-------+ +-------^-------+
| |
+-------v-------+ +-------v-------+
| subnet1 | | subnet2 |
| 172.16.1.0/24 | | 172.16.2.0/24 |
+---------------+ +---------------+
host1 and host2 establish a IPv6 tunnel which encapsulates packets
between the two IPv4 subnets. Therefore, in routed mode, host1 will
need to configure a route like "172.16.2.0/24 via ipsec1" even if the
host doesn't have any IPv4 address on the VPN interface.
Accept IP configurations without address from the VPN; only check that
the address and prefix are sane if they are provided.
Connection timestamps are updated (saved to disk) on connection up and
down. This way, the last used connection will take precedence for
autoconnect if they have the same priority.
But as we don't actually do connection down when NM stops, the last
connection timestamp of all active connections is the timestamp of when
they were brought up. Then, the activation order might be wrong on next
start.
One case where timestamps are wrong (although it is not clear how
important it is because the connections are activated on different
interfaces):
1. Activate con1 <- timestamp updated
2. Activate con2 <- timestamp updated
3. Deactivate con2 <- timestamp updated
4. Stop NM <- timestamp of con2 is higher than con1, but con1 was still
active when con2 was brought down.
Other case that is reproducible (from
https://issues.redhat.com/browse/RHEL-35539):
1. Activate con1
2. Activate con2 on same interface:
- As a consequence con1 is deactivated and its timestamp updated
- The timestamp of con2 is also updated
3. Stop NM <- timestamp of con1 and con2 is the same, next activation
order will be undefined.
Fix by saving the timestamps on NM shutdown.
Problem:
Given a OVS port with `autoconnect-ports` set to default or false,
when reactivation required for checkpoint rollback,
previous activated OVS interface will be in deactivate state after
checkpoint rollback.
The root cause:
The `activate_stage1_device_prepare()` will mark the device as
failed when controller is deactivating or deactivated.
In `activate_stage1_device_prepare()`, the controller device is
retrieved from NMActiveConnection, it will be NULL when NMActiveConnection
is in deactivated state. This will cause device been set to
`NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED` which prevent all follow
up `autoconnect` actions.
Fix:
When noticing controller is deactivating or deactivated with reason
`NM_DEVICE_STATE_REASON_NEW_ACTIVATION`, use new function
`nm_active_connection_set_controller_dev()` to wait on controller
device state between NM_DEVICE_STATE_PREPARE and
NM_DEVICE_STATE_ACTIVATED. After that, use existing
`nm_active_connection_set_controller()` to use new
NMActiveConnection of controller to move on.
Resolves: https://issues.redhat.com/browse/RHEL-31972
Signed-off-by: Gris Ge <fge@redhat.com>
Commit 797f3cafee ('device: fall back to saved use_tempaddr value
instead of rereading /proc') changed the behaviour of how to get the
last resort default value for ip6-privacy property.
Previously we read it from /proc/sys/net/ipv6/conf/default, buf after
this commit we started to read /proc/sys/net/ipv6/conf/<iface> instead,
because the user might have set a different value specific for that device.
As NetworkManager changes that value on connection activation, we used
the value read at the time that NetworkManager was started.
Commit 6cb14ae6a6 ('device: introduce ipv6.temp-valid-lifetime and
ipv6.temp-preferred-lifetime properties') introduced 2 new IPv6 privacy
related properties relying on the same mechanism.
However, this new behaviour is problematic because it's not predictable
nor reliable:
- NetworkManager is normally started at boot time. That means that, if a
user wants to set a new value to /proc/sys/net/ipv6/conf/<iface>,
NetworkManager is likely alread running, so the change won't take
effect.
- If NetworkManager is restarted it will read the value again, but this
value can be the one set by NetworkManager itself in the last
activation. This means that different values can be used as default in
the same system boot depending on the restarts of NetworkManager.
Moreover, this weird situation might happen:
- Connection A with ip6-privacy=2 is activated
- NetworkManager is stopped. The value in
/proc/sys/net/ipv6/conf/<iface>/use_tempaddr remains as 2.
- NetworkManager starts. It reads from /proc/sys/... and saves the value
'2' as the default.
- Connection B with no ip6-privacy setting is activated. The '2' saved
as default value is used. The connection didn't specify any value for
it, and the value '2' was set by another connection for that specific
connection only, not manually by a user that wanted '2' to be the
default.
A user shouldn't have to think on when NetworkManager starts or restarts
to known in an easy and predictable way what the default value for
certain property is. It's totally counterintuitive.
Revert back to the old behaviour of reading from
/proc/sys/net/ipv6/conf/default. Although this value is used by the
kernel only for newly created interfaces, and not for already existing
ones, it is reasonable to think on these settings as "systemwide
defaults" that the user has chosen.
Note that setting a different default in NetworkManager.conf still takes
precedence.
If a connection is in-memory (i.e. has flag "unsaved"), after a
checkpoint and rollback it can be wrongly persisted to disk:
- if the connection was modified and written to disk after the
rollback, during the rollback we update it again with persist mode
"keep", which keeps it on disk;
- if the connection was deleted after the rollback, during the
rollback we add it again with persist mode "to-disk".
Instead, remember whether the connection had the "unsaved" flag set
and try to restore the previous state.
However, this is not straightforward as there are 4 different possible
states for the settings connection: persistent; in-memory only;
in-memory shadowing a persistent file; in-memory shadowing a detached
persistent file (i.e. the deletion of the connection doesn't delete
the persistent file). Handle all those cases.
Fixes: 3e09aed2a0 ('checkpoint: add create, rollback and destroy D-Bus API')
When we recibe a Netlink message with a "route change" event, normally
we just ignore it if it's a route that we don't track (i.e. because of
the route protocol).
However, it's not that easy if it has the NLM_F_REPLACE flag because
that means that it might be replacing another route. If the kernel has
similar routes which are candidates for the replacement, it's hard for
NM to guess which one of those is being replaced (as the kernel doesn't
have a "route ID" or similar field to indicate it). Moreover, the kernel
might choose to replace a route that we don't have on cache, so we know
nothing about it.
It is important to note that we cannot just discard Netlink messages of
routes that we don't track if they has the NLM_F_REPLACE. For example,
if we are tracking a route with proto=static, we might receive a replace
message, changing that route to proto=other_proto_that_we_dont_track. We
need to process that message and remove the route from our cache.
As NM doesn't know what route is being replaced, trying to guess will
lead to errors that will leave the cache in an inconsistent state.
Because of that, it just do a cache resync for the routes.
For IPv4 there was an optimization to this: if we don't have in the
cache any route candidate for the replacement there are only 2 possible
options: either add the new route to the cache or discard it if we are
not interested on it. We don't need a resync for that.
This commit is extending that optimization to IPv6 routes. There is no
reason why it shouldn't work in the same way than with IPv4. This
optimization will only work well as long as we find potential candidate
routes in the same way than the kernel (comparing the same fields). NM
calls to this "comparing by WEAK_ID". But this can also happen with IPv4
routes.
It is worth it to enable this optimization because there are routing
daemons using custom routing protocols that makes tens or hundreds of
updates per second. If they use NLM_F_REPLACE, this caused NM to do a
resync hundreds of times per second leading to a 100% CPU usage:
https://issues.redhat.com/browse/RHEL-26195
An additional but smaller optimization is done in this commit: if we
receive a route message for routes that we don't track AND doesn't have
the NLM_F_REPLACE flag, we can ignore the entire message, thus avoiding
the memory allocation of the nmp_object. That nmp_object was going to be
ignored later, anyway, so better to avoid these allocations that, with
the routing daemon of the above's example, can happen hundreds of times
per second.
With this changes, the CPU usage doing `ip route replace` 300 times/s
drops from 100% to 1%. Doing `ip route replace` as fast as possible,
without any rate limitting, still keeps NM with a 3% CPU usage in the
system that I have used to test.
The D-Bus and C APIs admit setting the 802.1X certificates as blobs, as
the documentation of the properties explains. However, this is not
possible from nmcli, where only path to the certificates' files is possible.
This difference in nmcli was explained in the description message that
is shown in nmcli's editor, but this is a documentation that most users
won't ever see, and still the main documentation in nm-settings-nmcli is
missleading.
Add a nmcli specific documentation for the relevant properties and
remove the nmcli's editor descriptions as they are no longer needed.
This allows SLAAC for IPv6 to be performed, even when no IPv6
address was passed by the bearer. The link-local address will be
assigned, because of do_auto = TRUE.
The commit also allows the DNS assignment to be made statically when
no IPv6 address has been statically assigned yet. This is to be able
to receive IPv6 DNS servers via signalling, where host SLAAC still
needs to be performed for some modems (e.g. some huawei modems).
This also changes the logging so that SLAAC usage is logged
on a separate line.
In the gtkdoc comments, the text below tags like `Since: 1.2` is
discarded. In the property `autoconnect-slaves` a line indicating its
deprecation was below one of these tags. As a result, it was missing in
the man page. Fix it.
Fixes: 194455660d ('connection: deprecate NMSettingConnection autoconnect-slaves property')
GCC 14 with LTO complains with:
In function 'nm_team_link_watcher_new_ethtool',
inlined from 'nm_team_link_watcher_new_ethtool' at src/libnm-core-impl/nm-setting-team.c:106:1:
src/libnm-core-impl/nm-setting-team.c:130:33: error: array subscript 'struct NMTeamLinkWatcher[0]' is partly outside array bounds of 'unsigned char[16]' [-Werror=array-bounds=]
130 | watcher->ref_count = 1;
| ^
src/libnm-core-impl/nm-setting-team.c:128:15: note: object of size 16 allocated by 'g_malloc'
128 | watcher = g_malloc(nm_offsetofend(NMTeamLinkWatcher, ethtool));
| ^
even if the warning is disabled via pragma directives in that
code. This looks like the following GCC bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922
saying
We do not track warning options (and thus optimize pragmas /
attributes) across LTO because they are not saved in the function
specific optimization flag section.
We use a (NMTeamLinkWatcher *) to point to a memory area that is
shorter than the struct, because depending on the watcher type we need
to store different parameters; in this way we can save few bytes of
memory for some watcher types. However, this often breaks when
upgrading the compiler; instead just allocate the full struct.
GCC 14 complans with:
src/libnm-glib-aux/nm-uuid.c: In function 'nm_uuid_generate_from_strings_strv':
src/libnm-glib-aux/nm-uuid.c:492:12: error: '_1' may be used uninitialized [-Werror=maybe-uninitialized]
492 | return nm_uuid_generate_from_string_str(s, slen, uuid_type, type_args);
| ^
src/libnm-glib-aux/nm-uuid.c:392:1: note: by argument 1 of type 'const char *' to 'nm_uuid_generate_from_string_str' declared here
392 | nm_uuid_generate_from_string_str(const char *s,
| ^
"-Wmaybe-uninitialized" diagnoses passing pointers or references to
uninitialized memory to functions taking const-qualified arguments.
In this case, nm_uuid_generate_from_string_str()'s first argument is a
"const char *" and so the compiler expects that the string is always
initialized. However, it is not initialized when len is zero.
A non-null zero-length array can be specified in two ways: by setting
len to zero, or by setting len to -1 and having NULL as first
element. Handle both cases in the same way.
NMPowerMonitor emits the "shutdown" signal without arguments; fix the
definition of the signal.
Fixes: bd38a19832 ('connection: add support to down-on-poweroff')
