These properties were never implemented. Also, they were not settable
via nmcli. Drop them from being shown. This is an API break, but
hopefully something that does not affect anybody in a bad way.
The sys-iface-state "assume" means to gracefully take over a device (for
example, after a restart). The end result is a fully managed interface.
The flag only has meaning while activating, and for most practical
purposes, such devices should be treated the same as fully activated
ones.
Without this, the MTU is not reset until the device reaches fully
activated state, at which point the sys-iface-state switches from
"assume" to "managed". With the previous commit, at that point we also
schedule an idle commit, which ends up also setting the MTU. Before
that, the MTU was only reset some undefined time later, when we happened
to do another NML3Cfg commit. Nonetheless, even waiting until we reach
fully activated state is wrong. Also during activation, commit the MTU.
I guess, what theoretically could happen is that we get our MTU via
ip-config (like DHCP). Then, during assuming we hit _commit_mtu()
without having the DHCP lease yet. This happens after a restart, so it
would be wrong to first reset the MTU, before we re-receive the DHCP
lease. However, if the MTU is really to be set due via
NM_DEVICE_MTU_SOURCE_IP_CONFIG, then all other MTU sources are also not
in effect (because ip-config has a low priority). In that case, we would
not have an MTU to reset and the code would not commit a new MTU. Thus
this should still be fine, also during activation when we didn't yet get
the DHCP lease (or other information to dynamically set the MTU).
When assuming a device, the NMActiveConnection switches the
sys-iface-state from "assume" to "managed" when the device reaches the
activated state.
<debug> [1679353062.8884] active-connection[000055bd310b92e0]: set state activated (was activating)
<debug> [1679353062.8885] active-connection[000055bd310b92e0]: update activation type from assume to managed
Note that the "assume" state is probably a misfeature, and should be
dropped in favor of more appropriate flags. Meaning, "assume" state for
the most part is very similar to sys-iface-state "managed", and the
cases where (during activation) we need to be graceful, may be better
covered with other (more specialized) state flags. Regardless, for most
practical purposes, sys-iface-state "assume" should be treated similar
to "managed" state.
When we fully activated, we should be sure to do yet another idle
commit. Note that scheduling an idle-commit is something that must
always be allowed to any users of NML3Cfg. The users have no knowledge
about each other and coordinate by registering their commit type
handles. Issuing an idle "auto" commit must be therefore allowed to
them at any time. If that were not the case, then there would be a bug
to fix. The only reason to maybe not do it, is when we are sure there is
nothing to commit and we would want to avoid unnecessary work.
You can easily reproduce this and see that we don't in fact schedule a
commit after becoming managed. A commit usually only happens later, for
example when we receive an autoconf6 update.
This affects for example setting the MTU. Currently, _commit_mtu() bails
out for nm_device_sys_iface_state_is_external_or_assume() and thus
during activation the MTU will not be set. Later, once we reach
activated state, due to this it still is not set right away. This patch
fixes that, although we should also change _commit_mtu() to not bail out
for sys-iface-state "assume".
NetworkManager.service is "Type=dbus". Systemd takes that as indication
for declaring the service as started when the D-Bus name is acquired.
Currently, we acquire the name very early. The benefit is, that the
service appears to start very fast. However, most the D-Bus API is not
yet populated or ready to use. So if you order your service
`After=NetworkManager.service`, then there is a race that NetworkManager
might not yet be fully usable.
Another benefit was that requesting a D-Bus name is atomic. That means,
we could take that to ensure only one NetworkManager daemon was running.
If we noticed that NetworkManager is already running, we would quit
without doing anything. In practice, systemd already ensures that the
daemon is not running in parallel. This was still useful for catching
misuse when testing manually. This is now no longer done. We will notice
a concurrent NetworkManager only very late, at which point we might have
already broken things (e.g. rewrite wrong state files).
Fix the race with `After=` by acquiring the name much later.
Note that NetworkManager is pretty slow during initialization. This
easily adds several hundreds of milliseconds to the startup.
Since l3cfg rework, NetworkManager tracks IP routes early, not not only
when IP configuration is ready. That means, with `ipv4.method=auto` and
static `ipv4.routes`, then routes are most likely already configured
before the IP address is obtained via DHCP.
That may be desirable in some cases, but for many cases it's probably
wrong.
Instead, only configure the routes (with an ifindex) when we also have
an IP address.
https://bugzilla.redhat.com/show_bug.cgi?id=2102212https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1574
"wifi.seen-bssids" looks like a regular property, but it is not. Unlike
almost all other properties, it does not contain user configuration,
rather it gets filled by the daemon.
The values are thus stored in "/var/lib/NetworkManager/seen-bssids"
file, and the daemon maintains the values separately from the profile.
Only before exporting the profile on D-Bus, the value gets merged (see
NM_SETTINGS_CONNECTION_GET_PRIVATE(self)->>getsettings_cached and
nm_connection_to_dbus_full().
Hence, looking at nm_setting_wireless_get_num_seen_bssids() is not
working. Fix that.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1253
Fixes: 0f3203338c ('wifi: roam aggressively if we on a multi-AP network')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1577
The mock service is more widely useful -- in particular for testing
nm-cloud-setup in a following commit.
Split the commonly useful parts into TestNmClient class.
The activation_state_change_delay_ms was not too useful, since it could
be changed only after the AC started activating. Not a big deal, since
it was actually unused.
Apart from that, the SetActiveConnectionStateChangedDelay() didn't make
a whole lot of sense either: it accepted a Device path, but actually
was looking up an AC.
Let's move the property to the Device, so that 1.) it can be adjusted
before the AC is constructed (the AC will inherit it from the Device)
and 2.) SetActiveConnectionStateChangedDelay() does no longer hurt my
feelings.
I don't know what's going on:
======================================================================
ERROR: test_ec2 (__main__.TestNmCloudSetup.test_ec2)
----------------------------------------------------------------------
Traceback (most recent call last):
File "NetworkManager/src/tests/client/test-client.py", line 2169, in f
func(self)
File "NetworkManager/src/tests/client/test-client.py", line 2194, in test_ec2
conn = self.srv.op_AddAndActivateConnection(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "NetworkManager/src/tests/client/test-client.py", line 748, in __call__
return method(*args)
^^^^^^^^^^^^^
File "/usr/lib64/python3.11/site-packages/dbus/proxies.py", line 72, in __call__
return self._proxy_method(*args, **keywords)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.11/site-packages/dbus/proxies.py", line 141, in __call__
return self._connection.call_blocking(self._named_service,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.11/site-packages/dbus/connection.py", line 634, in call_blocking
reply_message = self.send_message_with_reply_and_block(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
dbus.exceptions.DBusException: org.freedesktop.DBus.Python.Exception: Traceback (most recent call last):
File "/usr/lib64/python3.11/site-packages/dbus/service.py", line 712, in _message_cb
retval = candidate_method(self, *args, **keywords)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "NetworkManager/tools/test-networkmanager-service.py", line 1693, in AddAndActivateConnection
conpath, acpath, result = self.AddAndActivateConnection2(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "NetworkManager/tools/test-networkmanager-service.py", line 1707, in AddAndActivateConnection2
conpath = gl.settings.AddConnection(con_hash)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "NetworkManager/tools/test-networkmanager-service.py", line 2198, in AddConnection
return self.add_connection(con_hash)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "NetworkManager/tools/test-networkmanager-service.py", line 2208, in add_connection
con_inst = Connection(self.c_counter, con_hash, do_verify_strict)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "NetworkManager/tools/test-networkmanager-service.py", line 2045, in __init__
NmUtil.con_hash_verify(con_hash, do_verify_strict=do_verify_strict)
File "NetworkManager/tools/test-networkmanager-service.py", line 594, in con_hash_verify
BusErr.raise_nmerror(e)
File "NetworkManager/tools/test-networkmanager-service.py", line 497, in raise_nmerror
raise e
File "NetworkManager/tools/test-networkmanager-service.py", line 590, in con_hash_verify
con_nm = NmUtil.con_hash_to_connection(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "NetworkManager/tools/test-networkmanager-service.py", line 537, in con_hash_to_connection
assert GLib.Variant.equal(x_con, Util.variant_from_dbus(con_hash))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "NetworkManager/tools/test-networkmanager-service.py", line 378, in variant_from_dbus
raise Exception("Unsupported type for value '%s'" % (repr(val)))
Exception:
Unsupported type for value 'dbus.Dictionary({
dbus.String('connection'): dbus.Dictionary({
dbus.String('type'): dbus.String('802-3-ethernet'),
dbus.String('id'): dbus.String('con-eth0'),
'uuid': '5fcfd6d7-1e63-3332-8826-a7eda103792d'
}, signature=dbus.Signature('ss')),
dbus.String('ipv4'): dbus.Dictionary({
dbus.String('method'): dbus.String('auto')
}, signature=dbus.Signature('ss'))
}, signature=dbus.Signature('sa{ss}'))'
Previously, there was "temporary-not-available" mechanism in NML3Cfg,
which aimed to handle IPv6 routes with prefsrc. Theoretically, that
mechanism may have been extended to other use-cases, like IPv4 routes
with prefsrc. What it attempted to handle, is the inability to configure
such routes, unless the respective prefsrc address is configured and
non-tentative. However, the address that we are waiting for, could also
be on another interface, so that mechanism wasn't applicable. This is
now replaced by _routes_watch_ip_addrs(). It seems there isn't anything
useful left for the "temporary-not-available" mechanism and it can go,
except...
We want to log a warning when we are unable to configure a route. Also,
in the future we might want to know when the IP configuration is
degradated due to inability to configure the desired routes (a condition
that we might want to expose to the user, not only via logging; or we
may want to react on that).
However, with prefsrc routes we don't know right away whether the
inability to configure the route right away indicates an actual problem,
or whether that will resolve itself (e.g. after the address passes
DAD/ACD, after we received an DHCP lease or after the address was
configured on another interface). Consequently, to know whether the
current inability to configure such a route is a problem, we need to
know the larger context. nm_platform_ip_route_sync() does not have that
context.
Instead, nm_platform_ip_route_sync() needs only do debug log about
failure to configure routes. It will now also return all the failed
routes to NML3Cfg, which can decide whether that is a problem.
This reworks the previous "temporary-not-available" mechanism to track
the state of the failed routes, to eventually decide whether there is an
actual problem (and log about it).
Another problem this solves is that since commit ('platform: always
reconfigure IP routes even if removed externally'), we will eagerly
re-try to configure the same route over and over. We cannot just spam
the log with warnings about the same failure on every commit. We need to
remember that we already logged about the problem and rate limit
warnings otherwise. This is what the new mechanism also achieves.
Indeed, all this is mostly for the sole benefit of logging better
warnings (and not duplicated).
It was unused anyway.
But also, what would we do with this? We are in the middle of a commit,
if something goes wrong, we cannot just abort but need to continue on
and make the best of it.
Maybe there are very specific error cases that we need to handle, but
those are not covered by a boolean return value. Instead, we might need
to take specific action.
The boolean success variable was meaningless. Drop it.
Routes with pref_src (RTA_PREFSRC) can only be added when the
corresponding IP address is configured (and non-tentative, in case of
IPv6). Additionally, that address may be on any interface, not only on
the one we want to configure the route on. This means, when we first
activate a profile with a route that has a src attrbute, then that src
address might only be configured later. For example, with IPv6, it takes
a while for the address to become non-tentative. Or the address might
come from DHCP, and not be present initially. Or the address might even
be configured on another interface/profile. That means, while we might
be unable to configure the route now, we may become able any time later.
Solve that by subscribing to NMNetns to get notifications whenever such
an address gets added. In that case, schedule an idle commit, which may
then succeed.
The implementation came with two flavors, where watcher could either
specify a tag or no tag. That resulted in different usage patterns and
behavior.
Handles with tag are indexed by a dictionary and de-duplicated. Also the intended
pattern is to delete them with nm_netns_watcher_remove_all(),
Currently, nm_netns_watcher_remove_handle() was not permissible to tag-full handles,
because of the de-duplication and because handles had no ref-counting
implemented (the latter would be fixable, so
nm_netns_watcher_remove_handle() would be made to work).
On the other hand, handles without tag are never de-duplicated. They are
also not indexed, so nm_netns_watcher_remove_all() doesn't work for
them. They could only be removed via nm_netns_watcher_remove_handle().
Currently, the only user of the API will use tag-full handles. Drop the
unused API. This is done as a separate commit, to potentially revert and
restore tag-less handles (after they were already implemented).
NML3Cfg will want to know when an address changes -- on any interface.
We want to support gazillion of interfaces, a naive approach is not
going to scale. Instead, NMNetns already subscribes to all platform
signals, it should dispatch events for address changes.
Add a mechanism how users (NML3Cfg) can register watches, and get called
back when the event happens.
Kernel rejects adding routes that have a gateway, if there is no direct
(onlink) route to that gateway. The exact conditions are non-trivial due
to the complexities of routing, but that's it basically.
Anyway. In NetworkManager we don't want to have such non-obvious
interdependencies. If the user configures a route with a gateway, but
"forgets" to configure a direct route to the gateway, we don't assume
that the user configured the wrong route. Instead, we assume the user
forgot to configure the additional route and add it automatically. That
is for convenience, but also because (as said) the rules for this are
non-trivial. Moreover, it's problematic to report an error in routing
during activation. Should we fail activation altogether? Should we just
log an error and otherwise silently proceed? Logging is not a sensible
behavior that the (possibly non-human) user can meaningfully handle. So
we instead try to make it work.
Previously, nm_platform_ip_route_sync() had the workaround of when we
failed to configure a route and it looked like it might be due to the
missing onlink route, we would add a suitable /32 / /128 route. The
problem is that we want that NML3Cfg is aware of what routes we want to
configure. The lower layer nm_platform_ip_route_sync() adding additional
routes makes that difficult (maybe nm_platform_ip_route_sync() could
return the additional routes that it added, but it doesn't).
The better solution seems to be that
nm_l3_config_data_add_dependent_onlink_routes() adds the required routes
in NML3Cfg during commit. This is done since commit 4073211595
('Revert "l3cfg: do not add dependent routes for non-default routes"').
Further, since commit ('platform: always reconfigure IP routes even if
removed externally') we also always try to re-add the routes we want,
regardless of whether they appear to be deleted by the user.
So a suitable onlink route really should be always there, and there is
no more need for this workaround.
NML3Cfg is stateful, that means it remembers which address/route it
configured earlier. That is important because the API users of NML3Cfg
only say what the want to configure now, and NML3Cfg needs to remove
addresses/routes that it configured earlier but are no longer to be
present. Also, NetworkManager wants to allow the user to add
addresses/routes externally with `ip addr|route add` and NetworkManager
not removing it. This is a common use case for dispatcher scripts, but
in general, we want to allow other components to add addresses/routes.
We try something similar with the removal of routes/addresses managed by
NetworkManager. When NetworkManager adds a route/address, which later
disappears, then we assume that the user intentionally removed the
address/route and take the hint to not re-add it.
However, it doesn't work. It is problematic for two reasons:
- kernel can automatically remove routes. For example, deleting an IPv4
address that is the prefsrc of a route, will cause kernel to delete
that route. Sure, we may be unable to re-configure the route at this
moment, but we shouldn't remember indefinitely that the route is
supposed to be absent. Rather, we should re-add it when possible.
- kernel is a pain with validating consistencies of routes. For example,
when a route has a nexthop gateway, then the gateway must be onlink
(directly reachable), or kernel refuses to add it with "Nexthop has
invalid gateway". Of course, when removing the onlink route kernel is
fine leaving the gateway route behind, which it would otherwise refuse
to add.
Anyway. Such interdependencies for when kernel rejects adding a route
with "Nexthop has invalid gateway" are non-trivial. We try to work
around that by always adding the necessary onlink routes. See
nm_l3_config_data_add_dependent_onlink_routes(). But if the user
externally removed the dependent onlink route, and NetworkManager
remembers to not re-adding it, then the efforts from
nm_l3_config_data_add_dependent_onlink_routes() are ignored. This
causes ripple effects and NetworkManager will also be unable to add the
nexthop route.
Trying to preserve absence of routes that NetworkManager would like to
configure is not tenable. Don't do it anymore. There was anyway no
guarantee that on the next update NetworkManager wouldn't try to re-add
the route in question. For example, if the route came from DHCP, and the
lease temporarily went away and came back, then NetworkManager probably
would have (correctly) forgotten that the user wished that the route be
absent. This did not work reliably and it just causes problems.
NMPrioq is taken from systemd's "prioq.c". It is a nice data structure,
that accepts and an index pointer, to directly access elements inside
the heap.
Previously, the API didn't require a consistent index, while the data is
not inside the heap. nm_prioq_{update,shuffle,remove}()) call find_item(),
which silently accepts wrong indexes and assumes the element is not in
the heap.
Keeping the index in sync with the data seems error prone. Accepting any
index without asserting may be convenient for the user (as the user is
not required to pre-initialize the index with NM_PRIOQ_IDX_NULL).
However, it also misses to catch potential bugs.
Now the index must be kept consistent, in particular also if the element
is not enqueued. This means, you must initialize them with
NM_PRIOQ_IDX_NULL.
