It includes a reason code that makes it possible for the clients to be
more reasonable about error messages.
The reason code is essentially copied from the VPN, plus three more
reasons that were useful for non-VPN connections.
When deciding whether to touch a device we sometimes look at whether
the active connection is external/assumed. In many cases however,
there is no active connection around (e.g. while moving the device
from state unmanaged to disconnected before assuming).
So in most cases we instead look at the device-state-reason to decide
whether to touch the interface (see nm_device_state_reason_check()).
Often it's desirable to have no state and passing data as function
arguments. However, the state reason has to be passed along several hops
(e.g. a queued state change). Or a change to a master/slave can affect
the slave/master, where we pass on the state reason. Or an intermediate
event might invalidate a previous state reason. Passing the state
whether to touch a device or not as a state-reason is cumbersome
and limited.
Instead, the device should be aware of whats going on. Add a
sys-iface-state with:
- SYS_IFACE_STATE_EXTERNAL: meaning, NM should not touch it
- SYS_IFACE_STATE_ASSUME: meaning, NM is gracefully taking over
- SYS_IFACE_STATE_MANAGED: meaning, the device is managed by NM
- SYS_IFACE_STATE_REMOVED: the device no longer exists
This replaces most checks of nm_device_state_reason_check() and
nm_active_connection_get_activation_type() by instead looking at
the sys-iface-state of the device.
This patch probably has still issues, but the previous behavior was
not very clear either. We will need to identify those issues in future
tests and tweak the behavior. At least, now there is one flag that
describes how to behave.
Now we only search for a candiate with matching UUID. No need to
first lookup all activatable connections, just find the candidate
by UUID and see if it is activatable.
- rename find_ac_for_connection() to
active_connection_find_first_by_connection().
This function has the unexpected(?) behavior to either
search by the @connection using pointer identity, or
by looking up the UUID (depending on whether @connection
is a NMSettingsConnection).
- Split out a active_connection_find_first() part.
The name "find-first" makes it clear that we walk the list until
a matching active-connection is found. That's why I added the
max_state argument. Otherwise, it would have to be called
"find-first-non-disconnected".
- drop nm_manager_get_connection_device(). It only had two callers.
It also used the ambiguity of the @connection argument, but
only one of the two callers cared about that. Meaning,
_internal_activate_device() now explicitly does a lookup by
the NMSettingsConnection.
There is only one caller of assume_connection().
The tasks there are not clearly separate and it is clearer just to
have one large recheck_assume_connection() function which proceeds
step by step, instead of breaking it into separate parts and move
them apart in the source code. The latter implies that -- unless
we forward-declare assume_connection() -- the order of definition
with assume_connection,recheck_assume_connection is contrary the
flow of the code.
Having a separate function in this case is not a simplification
so merge it.
Before, we would have the concept of assumed connections, which is used
for (1) externally configured device that NetworkManager should not
touch and (2) connections that NetworkManager should gracefully take
over after a restart (seamlessly, non-destructively).
The behavior was unclear and mixed. It wasn't clear whether the device
is in no-touch mode (1) or gracefully take-over (2).
Previous commits already introduce separate activation types EXTERNAL (1)
and ASSUME (2).
Also, previously, we would for both (1) and (2) try to find a matching
connection and use it. That doesn't make sense for either.
In the external case (1), we should not pretend that an existing connection
is active. Let's always create a new in-memory connection for these
cases. Note that this means, external devices now will always generate
a connection, instead of pretending an existing one is active.
For the assume case (2), we shall not use nm_utils_match_connection() to
guess which connection might be active. It can only the one that was
active on a previous run of NetworkManager. So, use the information from
the state file and try to activate it. If that fails, it is not an
assume activation type. Note, that this means we now most of the time
don't do ASSUME anymore. Most of the time we do EXTERNAL activation
That is because the state information is only available after restart
of NetworkManager.
We need a distinction between external activations and assuming
connections. The former shall have the meaning of devices that are
*not* managed by NetworkManager, the latter are configurations that
are gracefully taken over after restart (but fully managed).
Express that in the activation-type of the active connection.
Also, no longer use the settings NM_SETTINGS_CONNECTION_FLAGS_VOLATILE
flag to determine whether an assumed connection is "external". These
concepts are entirely orthogonal (although in pratice, external
activations are in-memory and flagged as volatile, but the inverse
is not necessarily true).
Also change match_connection_filter() to consider all connections.
Later, we only call nm_utils_match_connection() for the connection
we want to assume -- which will be a regular settings connection,
not a generated one.
nm_device_uses_assumed_connection() basically called
nm_active_connection_get_assumed() on the device.
Rename those functions to be closer to the activation-type
flags.
The concepts of "assume", "external", and "assume_or_external"
will make sense with the following commits.
The concept of assumed-connection will change. Currently we mark
connections that are generated and assumed as "nm-generated-assumed".
That has several consequences, one of them being that such a settings
connection gets deleted when the device disconnects.
That is, such a settings connection lingers around as long as it's active,
but once it deactivates it gets automatically deleted. As such, it's
a more volatile concept of an in-memory connection.
The concept of such automatically cleaned up connections is useful beyond
generated-assumed. See the related bug rh#1401515.
The devices_inited_cb() callback is really supposed to only run
when there is nothing else left in the mainloop to dispatch.
But as we already schedule the idle action with G_PRIORITY_LOW+10
priority, it is very unlikely that there is anything else ready
to run (unless scheduled with an even lower priority, and then it
wouldn't help either because devices_inited_cb() would win again).
This doesn't really matter, because NMSetting's startup-complete never
switches back to FALSE once reached. Still, cleanup our signal handlers
when no longer needed.
And disconnect the signals before emitting "notify::startup".
Possibly pending messages from the netlink socket were not processed
since the platform instance was created earlier. As nm_manager_start()
may take a long time to run, make sure that there are no pending
messages before querying the devices.
The state-change of a device has a reason argument, which is mostly for information
only.
There are many places in code that are the source of a state-reason.
Mostly these are calls to:
- nm_device_state_changed()
- nm_device_queue_state()
- nm_device_queue_recheck_available()
- nm_device_set_unmanaged_by_*()
- nm_device_master_release_one_slave()
- nm_device_ip_method_failed()
- nm_modem_emit_prepare_result()
- nm_modem_emit_ppp_failed()
- nm_manager_deactivate_connection()
- NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_*);
However, there are a few places in code that look at the reason
to decide how to proceed. I think this is a bad pattern, because
cause and effect are decoupled and it gets hard to understand where
a certain reason is set and what consequences that has.
Add a nop-function nm_device_state_reason_check() to mark all uses
of the device state reason that derive decisions from it. That is,
highlight the "effect" part.
The NMDevice's autoconnect property is settable via D-Bus and is is
also modified by internal decision, like when no PIN is available.
Certain internal actions cause clearing the internal autoconnect flag,
but they should not override the user desicion.
For example, when NM awaks from sleep it would reenable autoconnect,
but it should not reenable it for devices where the user explicitly
said that autoconnect is to be disabled.
Similarly, activating a device alone is not yet an instruction to
re-enable autoconnect. If the user consciously disables autoconnect,
it should stay enabled. On the other hand, activating a device should
reenable autoconnect if it was blocked by internal decision.
We need to track these two flags separately, and set them accordingly.
Update the connectivity state if we go from CONNECTED_GLOBAL to
CONNECTED_LOCAL. It will likely fail immediately (unless there's a default
route we're not aware of or the check URL is routable locally), keeping the
Connectivity property up-to-date.
We want to have some guaranteed order when comparing different connections.
So, in case of equal timestamps, proceed with comparing more properties.
It makes sense to consider the autoconnect-priority next.
This is what get_existing_connection() needs, thus we no longer
need to pre-sort the list.
NMPolicy's auto_activate_device() wants to sort by autoconnect-priority,
nm_utils_cmp_connection_by_autoconnect_priority() but fallback to the default
nm_settings_connection_cmp_default(), which includes the timestamp.
Extend nm_settings_connection_cmp_default() to consider the
autoconnect-priority as well. Thus change behavior so that
nm_settings_connection_cmp_default() is the sort order that
auto_activate_device() wants. That makes sense, as
nm_settings_connection_cmp_default() already considered the
ability to autoconnect as first. Hence, it should also honor
the autoconnect priority.
When doing that, rename nm_settings_connection_cmp_default()
to nm_settings_connection_cmp_autoconnect_priority().
We call these functions a lot. A GSList is just the wrong tool for the
job. Refactor the code to use instead a sorted array everywhere.
This means, we malloc() one array for all connections instead
slice-allocate a GSList item for each. Also, sorting an array
is faster then sorting a GSList.
Technically, the GSList implementation had the same big-O runtime
complexity, but using an array is still faster. That is, sorting
an array and a GSList is both O(n*log(n)).
Actually, nm_settings_get_connections_sorted() used
g_slist_insert_sorted() instead of g_slist_sort(). That results
in O(n^2). That could have been fixed to have O(n*log(n)), but
instead refactor the code to use an array.
nm_settings_get_best_connections() has only one caller: to create
the hidden-SSID list.
Instead of having a highly specialised function (that accepts 3 ways for
filtering -- one of them broken, has one hard-coded way of sorting, and
a @max_requested argument), add a more generic nm_settings_get_connections_clone()
function.
Also invert nm_settings_sort_connections(). The two callers want
to sort descending, not ascending.
I think NM_CACHED_QUARK_FCN() is better because:
- the implementation is in our hand, meaning it is clear that
putting a "static" before NM_CACHED_QUARK_FCN() is guaranteed to
work -- without relying on G_DEFINE_QUARK() to be defined in a way
that this works (in fact, we currently never do that and instead
make all functions non-static).
- it does not construct function names by appending "_quark".
Thus you can grep for the entire function name and finding
the place where it is implemented.
- same with the stings, where the new macro doesn't stringify the
argument, which is less surpising. Again, now you can grep
for the string including the double quoting.
(yes, I really use grep to understand the source-code)
If a slave device activates, we should keep the master up even though it
was taken over before.
This fixes an issue, where a single slave to a master would be
reactivated after a daemon restart. The daemon restart would cause the
master to be treated externally created (would go unmanaged when all the
slaves are gone) while the reactivation would leave the master without
slaves for a while.
(cherry picked from commit b605fb2712)
The only implementations were there for tracking the parent device.
That is now donw via nm_device_parent_*(), parent_changed_notify()
and _parent_notify_changed().
Multiple subclasses have a parent/link interface (NMDeviceIPTunnel,
NMDeviceVlan). Tracking the parent interface properly is midly
complicated to get right. So, instead of repeating it in each
subclass, track it in the parent device.
nm_settings_add_connection_dbus() invokes the activation_add_done()
callback with a NULL @new_connection in case of error: add a check to
prevent a crash.
We used MASTER, BRIDGE and TEAM_MASTER keys for a differnet purpose than the
network.service did, confusing the legacy tooling. Let's do our best to write
compatible configuration files:
* Add *_UUID properties that won't clash with initscripts
* Ignore non-*_UUID keys on read if *_UUID is present
* If the connection.master is an UUID of a connection with a
connection.interface-name, write the uuid into the *_UUID key while setting
the non-*_UUID key to the interface name for compatibility
https://bugzilla.redhat.com/show_bug.cgi?id=1369091
If a connection is ACTIVATED and another one is ACTIVATING but there
is no global connectivity, we currently set the manager state to
CONNECTING and start a connectivity check to verify whether the
manager state can be promoted to CONNECTED_GLOBAL.
If this connectivity check fails, we shouldn't promote a CONNECTING
state to CONNECTED_SITE.
Fixes: 084da69a30
The manager state is already computed every time an active connection
changes state, it is not necessary to call nm_manager_update_state()
also when the property is read.
Moreover, nm_manager_update_state() emits a "notify::state" signal
which causes a re-read of the property by the nm-exported-object,
resulting in a nested execution of nm_manager_update_state().
When going to sleep, we unmanage devices setting the unmanaged flags
immediately but delaying the state transition (because we do it from
another state transition). The signal handler can be executed after
the wake and, especially, after we have already re-managed the device,
making the device unmanaged again.
Detect such situation and force the state to UNMANAGED (which will
also clear any pending state change), so that later we manage the
device again and it will try to activate any available connection.
Fixes: 81ea812362https://bugzilla.redhat.com/show_bug.cgi?id=1382526
This makes it easier to install the files with proper names.
Also, it makes the makefile rules slightly simpler.
Lastly, the documentation is now generated into docs/api, which makes it
possible to get rid of the awkward relative file names in docbook.
Keep the include paths clean and separate. We use directories to group source
files together. That makes sense (I guess), but then we should use this
grouping also when including files. Thus require to #include files with their
path relative to "src/".
Also, we build various artifacts from the "src/" tree. Instead of having
individual CFLAGS for each artifact in Makefile.am, the CFLAGS should be
unified. Previously, the CFLAGS for each artifact differ and are inconsistent
in which paths they add to the search path. Fix the inconsistency by just
don't add the paths at all.
The interaction between the manager state and connectivity check code
is tricky. When there is an active connection with a default route and
NMConnectivity reports full connectivity, we set the CONNECTED_GLOBAL
state. However, if the connectivity check hasn't run yet, we stay in
CONNECTED_SITE state. If there are also other connections that are
activating, the state is set to CONNECTING.
This is a problem, because in CONNECTING we never run the connectivity
check and thus we fail to recognize that there is full connectivity
until a periodic check is run.
To solve this, schedule the connectivity check every time there is an
active connection with default route, even if other connection are
still activating, so that the check result can make the state progress
to CONNECTED_GLOBAL.
Before the link is initialized, that is before UDEV completed
initializing the device, we should not evaluate the user-settings
unmanaged flags.
The reason is, that evaluating it likely involves looking at the
permanent MAC address, which might use the wrong fake MAC address
(before UDEV set the right one). Also, it might use the wrong ifname
to lookup the permanent MAC address via ethtool.
The permanent MAC address of an NMDevice shall not change as
long as the device is realized. That is, we read it only once
and don't change it afterwards.
There are two issues that this commit tries to mitigate:
(1) users are advised to use UDEV to rename interfaces. As we lookup
the permenent MAC address using ethtool (which uses the interface
name), there is a race where we could read the permanent MAC
address using the wrong interface name. We should wait until
UDEV finished initializing the device and until the interface
name is stable (see rh#1388286).
This commit still cannot avoid the race of ethtool entirely. It only
tries to avoid ethtool until UDEV has done its work. That is, until we
expect the interface name no longer to change.
(2) some device types, don't have a permanent MAC address so we fall
back to use the currently set address (fake). Again, users are advised
to use UDEV to configure the MAC addresses on such software devices.
Thus, we should not get the fake MAC address until UDEV initialized
the device.
This patch actually doesn't solve the problem at all yet.
The reason is that a regular caller of nm_device_get_permanent_hw_address() can
not afford to wait until UDEV settled. Thus, any user who requests the
permanent MAC address before the link is initialized, runs into the
problems above.
In a next step, we shall revisit such calls to nm_device_get_permanent_hw_address()
and delay them until the link is initialized.
