All callers eventually want a boolean instead of a NMMatchSpecMatchType.
I think the NMMatchSpecMatchType enum still has value at the lower
layers, where the enum values are clearer (when reading the code). So
don't drop NMMatchSpecMatchType entirely.
However, let's add nm_match_spec_match_type_to_bool() to convert the
match-type to a boolean to avoid duplicating the code.
(cherry picked from commit cba8eb9784)
When a port cannot activate because the controller is not ready, it gets
blocked from autoconnect (see commit 725fed01cf ('policy: block
connection from autoconnect in case of failed dependency')).
Later, when the master activates we call activate_slave_connections()
(see commit 32efb87d4d ('core: unblock failed connections when the
master is available')), which unblocks those port profiles so they can
autoconnect.
However, imagine you add a port profile with autoconnect enabled. The
profile tries to autoconnect, finds no master and gets blocked. Then,
add the controller profile with autoconnect disabled. The controller is
not autoactivating, not calling activate_slave_connections() and the
profiles stay down.
Fix that by unblocking autoconnect of the ports when the controller
profile changes.
(cherry picked from commit 645a1bb0ef)
If there are ports that refer the controllers by a device name, and
multiple autoconnectable controller devices of that name, the
situation gets messy. In particular, the autoconnect logic can start
activating a device with a higher autoconnect priority, but then a port
can override it by bringing up another controller of possibly lower
autoconnect priority.
Let's
1.) prefer controller connections with higher autoconnect priority
and
2.) prefer connections that are already active so that we don't
disrupt existing activation.
https://bugzilla.redhat.com/show_bug.cgi?id=2121451
(cherry picked from commit 6adfd60630)
For efficiently and conveniently lookup an NMSetting from the
NMConnection inside the NMSettingsConnection.
Note that this uses the NMMetaSettingType as lookup key. That is a novel
approach, compared to lookup by name (nm_connection_get_setting_by_name())
or GType (nm_connection_get_setting()).
Using the NMMetaSettingType enum is however faster, because it does not
require resolving the name/GType first. This is perfecly fine internal API,
we should use it.
(cherry picked from commit 429cf416fd)
We have several variants to get the NMSetting from an NMConnection. Some
of them are public API (nm_connection_get_setting(), nm_connection_get_setting_by_name()).
The most efficient way is lookup by NMMetaSettingType. Expose that as
internal API, so it can be used. The NMMetaSettingType is internal, but
it exists because it's a very useful enum. Allow others to make use of
it.
Also, add a static assert which prevents various wrong uses at compile
time, for example
_nm_connection_get_setting_by_metatype(connection, NM_TYPE_SETTING_CONNECTION)
(cherry picked from commit db5946ac2f)
Refactor and cleanup the functions to get a setting from a connection.
As the NMConnection tracks the settings in an array indexed by
NMMetaSettingType, the most direct and efficient way is to look up via
that enum.
Previously, nm_connection_get_setting_by_name() would first look up the GType
(which already involved looking up the NMMetaSettingInfo), then based on the
GType it would look up the NMMetaSettingInfo again to get the meta_type. That
is unnecessary. Directly look up the NMMetaSettingInfo, which directly
gives the meta_type.
(cherry picked from commit c60a4649b8)
Before commit a42682d44f ('device: take reference to device object
before 'delete_on_deactivate''), we used a weak pointer to track the
idle action.
As we now use a strong reference, we can store all data about the idle
action in NMDevice itself. Drop DeleteOnDeactivateData.
(cherry picked from commit b48c314328)
NMDevice holds a reference to NMManager, which holds a reference to NMPolicy.
It is not possible that we try to dispose NMPolicy while there are still devices
registered. That would be a bug, that we need to find and solve
differently. Add an assertion instead of trying to handle it.
(cherry picked from commit aede228974)
Add an assertion to nm_policy_device_recheck_auto_activate_schedule(),
that the device is currently registered in NMPolicy. Calling it outside
would be odd, and likely a bug.
But if we only register the auto-activate while being registered, we
don't need to take an additional reference. We know that the object must
be be alive (also, we have assertions that in fact it is still alive).
(cherry picked from commit 0dd4724446)
Hook the information for tracking the activation of a device, to the
NMDevice itself. Sure, that slightly couples the NMPolicy closer to
NMDevice, but the result is still simpler code because we don't need a
separate ActivateData.
It also means we can immediately tell whether the auto activation check
for NMDevice is already scheduled and don't need to search through the
list.
(cherry picked from commit a22e5080a0)
NMPolicy really should be merged into NMManager. It has not a clear responsiblity
so that there are two separate objects only makes things confusing. Anyway. It
is permissible to look up the NMPolicy instance of a NMManager. Add an accessor.
(cherry picked from commit 520fcc8667)
It's the better name. Especially since there is no more signal involved,
the term "emit" doesn't match.
Note also how the previous approach using a signal tried to abstract
what is happening. So we were no longer rechecking-autoconnect, instead,
we were emitting-a-signal-to-recheck-autoconnect. Just be plain about
what it is doing and don't go through a layer of signal.
(cherry picked from commit 751b927cf2)
GObject signals don't make the code easier to understand, on the
contrary. They may have their purpose, when objects truly must/should
not be aware of each other, and need to be composed very loosely. That
is not the case here.
There really is only one subscriber to NM_DEVICE_RECHECK_AUTO_ACTIVATE
signal, and it only makes sense this way. Instead of going through a
signal invocation, just call the well known method directly. It becomes
clearer who calls this code (and it has a lower overhead).
When using cscope/ctags it also is easier to follow the code because the
tools understand function calls.
(cherry picked from commit 3c59c6b393)
The "all" variant is strongly related to nm_policy_device_recheck_auto_activate_schedule().
Rename, so that the names express that better.
(cherry picked from commit 886786ee0b)
The delete_on_deactivate_link_delete() handler may be called after the
device was already removed from NMManager. Don't allow that.
Check whether the device is still exported on D-Bus as indication.
(cherry picked from commit 49c1e01519)
NM_reboot_openvswitch_vlan_configuration_var2 test exposes a race. What
the test does, is to create OVS profiles and repeatedly restart
NetworkManager, checking that those profiles autoconnect and the OVS
configuration gets created.
There is a race, where:
- the OVS interface exists, and an NMDeviceOvsInterface is created
- first ovsdb cleans up old interfaces, sending a json request.
- OVS deletes the interface, and NetworkManager first picks up the
platform signal (there is a race here, usually the ovsdb request
completes first, which will cleanup the NMDeviceOvsInterface in
a different way).
- when the device gets unrealized, we don't schedule a
check-autoactivate, so the device stays down.
See https://bugzilla.redhat.com/show_bug.cgi?id=2152864#c5 for a log
file with more details.
What should instead happen, is to autoactivate the OVS interface, which
then also fully configures the port and bridge interfaces.
Explicitly schedule an autoactivate when unrealizing devices.
Note that there are now several cases, where NetworkManager autoconnects
more eagerly. This even affects some CI tests and user-visible behavior.
But I think relying on "just don't call nm_device_emit_recheck_auto_activate()
to hope that autoconnect doesn't happen is wrong. It must always be
possible to trigger an autoconnect check, and the right thing must
happen. We only don't trigger autoconnect checks *all* the time, because
it would be a waste of CPU resources, but whenever we slightly suspect
that an autoconnect may happen, we must be allowed to trigger a check.
If a device is in a condition where it previously did not autoconnect,
and it also *should* not autoconnect, then we need to fix the code that
evaluates whether an autoconnect may happen (not avoid triggering a
check).
https://bugzilla.redhat.com/show_bug.cgi?id=2152864
Fixes-test: @NM_reboot_openvswitch_vlan_configuration_var2
(cherry picked from commit e699dff46a)
(cherry picked from commit aac8d0cc0cda41da3ddbe952b658ed06a26369e0)
Currently, when we delete a device then autoconnect does not kick in
right away. But that is only, because we happen not to schedule a
"autoactivate" recheck.
What should be happen, is that rechecking whether to autoconnect is
always allowed, and that we have the necessary state to know that
autoconnect currently should not work.
Instead, block autoconnect of the involved profile. That makes sense,
because clearly we don't want to autoconnect right again after `nmcli
device delete $iface`.
(cherry picked from commit 14d429dd17)
(cherry picked from commit 53a3368bd600746c30798fb95113a8ccc5d48de3)
We should avoid using the NM_MANAGER_GET singleton. Everybody already
has a manager instance. Expose it and allow to use it.
(cherry picked from commit 20f791d8fe)
The activation of a connection will clear the block of autoconnect,
we should do the same for reapply.
Signed-off-by: Gris Ge <fge@redhat.com>
(cherry picked from commit 0486efd358)
Between ppp 2.4.8 and 2.4.9, "rp-pppoe.so" was renamed to "pppoe.so" (and a
symlink created). Between 2.4.9 and 2.5.0, the symlink was dropped.
See-also: b2c36e6c0e
I guess, NetworkManager always meant to use ppp's "(rp-)pppoe.so"
plugin, and never the library from the rp-pppoe project.
If a user actually wants to use the plugin from rp-pppoe project, then
this is going to break.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1312
Fixes: afe80171b2 ('ppp: move ppp code to "nm-pppd-compat.c"')
(cherry picked from commit 84e21d8bbc)
Previously, the ppp version was only detected (and used) at one place,
in "nm-pppd-compat.c", after including the ppp headers. That was nice
and easy.
However, with that way, we could only detect it after including ppp
headers, and given the ugliness of ppp headers, we only want to include
them in "nm-pppd-compat.c" (and nowhere else).
In particular, 'nm-pppd-compat.c" uses symbols from the ppp daemon, it
thus can only be linked into a ppp plugin, not in NetworkManager core
itself. But at some places we will need to know the ppp version, outside
of the ppp plugin and "nm-pppd-compat.c".
Additionally, detect it at configure time and place it in "config.h".
There is a static assert that we are in agreement with the two ways of
detection.
(cherry picked from commit 3e66c0bde1)
This is rather bad, because if we reach the "goto again" case,
the error variable is not cleared. Subsequently passing the
error location to nm_device_reapply_finish() will trigger a glib
warning.
Fixes: 29b0420be7 ('nm-cloud-setup: set preserve-external-ip flag during reapply')
(cherry picked from commit c70a5470be)
Once we start reconfiguring the system, we need to finish on all
interfaces. Otherwise, we might reconfigure some interfaces, abort
and leave the network broken. When that happens, a subsequent run
might also be unable to recover, because we are unable to reach the
HTTP meta data service.
https://bugzilla.redhat.com/show_bug.cgi?id=2207812
Fixes: 69f048bf0c ('cloud-setup: add tool for automatic IP configuration in cloud')
(cherry picked from commit dab114f038)
According to systemd, IPv6 forwarding is special anyway, and they only
enable forwarding for "net.ipv6.conf.all.forwarding" ([1]).
Since commit 46e63e03af ('device: announce the managed IPv6
configuration with ipv6.method=shared') we support "ipv6.method=shared"
and enable forwarding for IPv6, on the interface. Whether that makes
sense is questionable, given [1] and the claim that setting it
per-interface is not useful.
Anyway, since that change we always reset the "forwarding" sysctl to
zero, when we don't enable shared mode. That is not right, because the
user didn't explicitly ask for that (and there is no configuration
option like systemd-networkd's "IPForward=" setting to control that).
What we instead should do, not touch/reset the sysctl, unless we really
want to.
No longer set "forwarding" to zero by default. And only restore the
previous value (_dev_sysctl_save_ip6_properties()) if we actually
changed the value to "1".
[1] b8fba0cded/src/network/networkd-sysctl.c (L79)https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/923
Fixes: 46e63e03af ('device: announce the managed IPv6 configuration with ipv6.method=shared')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1616
(cherry picked from commit 4c48301594)
