This allows us to reject activation of WEP profiles very early,
also providing a reasonable error code to the client:
$ nmcli d wifi connect test
Error: Failed to add/activate new connection: wpa_supplicant does not support WEP encryption
Since version 2.10, it's possible to build wpa_supplicant without WEP
support. In fact, it's disabled by default. Regrettably, there's no
indication in that version as to whether WEP is enabled or not.
A patch has been sent upstream that exposes the information on D-Bus:
https://patchwork.ozlabs.org/project/hostap/patch/20220307085446.706024-1-lkundrak@v3.sk/
This makes use of the above to indicate presence or absence of WEP
support.
I think we should move away from using the source-ids.
Having a "GSource*" pointer makes it clearer what this is, compared to a
guint source ID. Also, g_source_remove() always needs to first do a hash
lookup (with locking) to resolve the source ID to the GSource. This is
unnecessary.
When a NMDevice is involved in a PPPoE activation, it means that the
connection has connection.interface-name=<ethernet-interface>. In such
case, the ppp ifindex should be set as ip-ifindex of the ethernet
device.
Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
We have nm_device_master_add_slave(). This should be mirrored by
nm_device_master_release_slave() (not release-one-slave).
Thereby, also rename nm_device_master_release_slaves() to
nm_device_master_release_slaves_all() to make it clearer.
I find the two (dependent) booleans "configure" and "force" confusing.
nm_device_master_release_one_slave() has many callers, it's interesting
to be able to grep for the release-type. Add an enum to make this more
readable.
This makes the non-obvious fact clearer, that when you look up an object
by an untrusted, user-provided path, it might not be the object type you
are looking for. In basically all cases, you need to check that the
result is of the expected type. This helper makes that clearer.
We often create the source with default priority, no destroy function and
attach it to the default context (g_main_context_default()). For that
case, we have wrapper functions like nm_g_timeout_add_source()
and nm_g_idle_add_source(). Use those.
There should be no change in behavior.
g_idle_add() uses G_PRIORITY_DEFAULT_IDLE priority. Most of the time we don't
care much about the priority.
But at the places that this patch changes, I think that using
G_PRIORITY_DEFAULT_IDLE (and following g_idle_add()) is more correct. The
reason for this is not very strong, except that it's probably the better
choice. And the old choice was made because I didn't realize that
g_idle_add() uses another default priority. Hence, the old choice was not
for good reasons either.
Pass the full hostname to the DNS manager, so that the domain gets
added to resolv.conf even when the hostname was truncated.
Note that "hostname" argument for plugins's update() function is
currently unused. Don't remove that because it can be potentially
useful to set a global search domain based on the hostname, but change
it to carry the domain directly.
nm_l3_config_data_new_clone() takes non-positive ifindex to use
the ifindex of the l3cd. But it also asserts that the ifindex
is not negative. Fix that assertion failure, by setting the ifindex
to zero.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/907
Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
Currently it is possible to specify a list of default settings plugins
to be used when configuration doesn't contain the main.plugins key.
We want to add a mechanism that allows to automatically load any
plugin found in the plugins directory without needing
configuration. This mechanism is useful when plugins are shipped in a
different, optional subpackage, to automatically use them.
With such mechanism, the actual list of plugins will be determined
(in order of evaluation):
1. via explicit user configuration in /etc, if any
2. via distro configuration in /usr, if any
3. using the build-time default, if any
4. looking for known plugins in /usr/lib
Hope third time is the charm.
The idea here is to remove the OVSDB entry if the device actually went away
violently (like, the it was actually removed from the platform), but keep it if
we're shutting down.
Fixes-test: @ovs_nmstate
Fixes: 966413e78f ('ovs-port: avoid removing the OVSDB entry if we're shutting down')
Fixes: ecc73eb239 ('ovs-port: always remove the OVSDB entry on slave release')
https://bugzilla.redhat.com/show_bug.cgi?id=2055665
When we have a bridge interface with ports attached externally (that is,
not by NetworkManager itself), then it can make sense that during
checkpoint rollback we want to keep those ports attached.
During rollback, we may need to deactivate the bridge device and
re-activate it. Implement this, by setting a flag before deactivating,
which prevents external ports to be detached. The flag gets cleared,
when the device state changes to activated (the following activation)
or unmanaged.
This is an ugly solution, for several reasons.
For one, NMDevice tracks its ports in the "slaves" list. But what
it does is ugly. There is no clear concept to understand what it
actually tacks. For example, it tracks externally added interfaces
(nm_device_sys_iface_state_is_external()) that are attached while
not being connected. But it also tracks interfaces that we want to attach
during activation (but which are not yet actually enslaved). It also tracks
slaves that have no actual netdev device (OVS). So it's not clear what this
list contains and what it should contain at any point in time. When we skip
the change of the slaves states during nm_device_master_release_slaves_all(),
it's not really clear what the effects are. It's ugly, but probably correct
enough. What would be better, if we had a clear purpose of what the
lists (or several lists) mean. E.g. a list of all ports that are
currently, physically attached vs. a list of ports we want to attach vs.
a list of OVS slaves that have no actual netdev device.
Another problem is that we attach state on the device
("activation_state_preserve_external_ports"), which should linger there
during the deactivation and reactivation. How can we be sure that we don't
leave that flag dangling there, and that the desired following activation
is the one we cared about? If the follow-up activation fails short (e.g. an
unmanaged command comes first), will we properly disconnect the slaves?
Should we even? In practice, it might be correct enough.
Also, we only implement this for bridges. I think this is where it makes
the most sense. And after all, it's an odd thing to preserve unknown,
external things during a rollback -- unknown, because we have no knowledge
about why these ports are attached and what to do with them.
Also, the change doesn't remember the ports that were attached when the
checkpoint was created. Instead, we preserve all ports that are attached
during rollback. That seems more useful and easier to implement. So we
don't actually rollback to the configuration when the checkpoint was
created. Instead, we rollback, but keep external devices.
Also, we do this now by default and introduce a flag to get the previous
behavior.
https://bugzilla.redhat.com/show_bug.cgi?id=2035519https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/ # 909
For devices that configure IP by themselves (by returning
"->ready_for_ip_config() = TRUE" and implementing
->act_stage3_ip_config()), we skip manual configuration. Currently,
manual configuration is the only one that sets flag HAS_DNS_PRIORITY
into the resulting l3cd.
So, the merged l3cd for such devices misses a dns-priority and is
ignored by the DNS manager.
Explicitly initialize the priority to 0; in this way, the default
value for the device will be set in the final l3cd during the merge.
Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/931
The kernel may add a reason for hardware rfkill. Make the NetworkManager
able eto fetch it and parse it.
For now, no action will be taken upon the new reasons.
The different reasons that the kernel can expose are either the radio
was switched off by a hardware rfkill switch. This reason is adveritsed
by bit 0 in the bitmap returned by RFKILL_STATE_REASON udev property.
This is the rfkill that existed until now.
The new reason is mapped to bit 1 and teaches the user space that the
wifi device is currently used by the CSME firmware on the platform. In
that case, the NetworkManager can ask CSME (through the iwlmei kernel
module) what BSSID the CSME firmware is associated to. Once the
NetworkManager gets to the conclusion is has the credentials to connect
to that very same AP, it can request the wifi device and the CSME
firmware will allow the host to take the ownership on the device. CSME
will give 3 seconds to the host to get an IP or it'll take the device
back. In order to complete all the process until we get the DHCP ACK
within 3 seconds, the NetworkManager will need to optimize the scan and
limit the scan to that specific BSSID on that specific channel.
All this flow is not implemented yet, but the first step is to identify
that the device is not owned by the host.
The define makes it clearer that there is an important relationship
between the timeout for the async operation, and the wrapup time when
NetworkManager is quitting. Well, not for the time being. But in the future,
when we rework the quitting of NetworkManager.
NM_SHUTDOWN_TIMEOUT_MAX_MSEC is the maximum timeout for how long any
async operation may take. The idea is that during shutdown of NetworkManager
we give that much time to tear down. Then async operations may either implement
cancellation or not bother with that. But in any case, they must complete within
NM_SHUTDOWN_TIMEOUT_MAX_MSEC.
Actually, for the time being, this has no effect at all. I am talking about the
future here. See "Improve Shutdown of NetworkManager" in TODO. This patch
is preparation for that effort.
Anyway. Stopping pppd can take a longer time (5 seconds). That is
currently the (known) longest time how long any of our async operations
is allowed to take.
As all async operations must complete before NM_SHUTDOWN_TIMEOUT_MAX_MSEC,
and we want to wait at least 5 seconds for pppd, we need to increase the
wait time NM_SHUTDOWN_TIMEOUT_MAX_MSEC.
Also add and use NM_SHUTDOWN_TIMEOUT_5000_MSEC, which serves a similar
purpose as NM_SHUTDOWN_TIMEOUT_1500_MSEC.
At some places we scheduled a timeout in NM_SHUTDOWN_TIMEOUT_MAX_MSEC.
There, we want to make sure that we don't take longer than
NM_SHUTDOWN_TIMEOUT_MAX_MSEC. But this leaves the actual wait time
unspecified.
Those callers don't want to wait an undefined time. They really should
be clear about how long they wait. Hence, use NM_SHUTDOWN_TIMEOUT_1500_MSEC
which makes it clear this is 1500 msec but also chosen to be not longer than
NM_SHUTDOWN_TIMEOUT_MAX_MSEC.
When you have an async operation, you must make sure that
it is cancellable or completes in at most NM_SHUTDOWN_TIMEOUT_MAX_MSEC.
But NM_SHUTDOWN_TIMEOUT_MAX_MSEC leaves it undefined how long it is.
If you really want to wait for 1500msec, but also need to ensure
to stay within NM_SHUTDOWN_TIMEOUT_MAX_MSEC, then use
NM_SHUTDOWN_TIMEOUT_1500_MSEC. This has the semantic of guaranteeing
both.
The abbreviations "ms", "us", "ns" don't look good.
Spell out to "msec", "usec", "nsec" as done at other places.
Also, rename NM_SHUTDOWN_TIMEOUT_MS_WATCHDOG to
NM_SHUTDOWN_TIMEOUT_ADDITIONAL_MSEC.
Also, rename NM_SHUTDOWN_TIMEOUT_MS to NM_SHUTDOWN_TIMEOUT_MAX_MSEC.
There are different timeouts, and this is the maximum gracetime we
will give during shutdown to complete async operations.
Naming is hard, but I think these are better names.
