Fixes a crash where the default DNS domain to be announced together with the
prefixes to be delegated is updated at the same time the device is being
unrealized.
https://bugzilla.redhat.com/show_bug.cgi?id=1425818
NMDeviceGeneric:check_connection_compatible() doesn't check for a
matching interface name. It relies on the parent implementation to
do that.
The parent implementation calls nm_manager_get_connection_iface().
That fails for NM_SETTING_GENERIC_SETTING_NAME, because that one has
no factory. Maybe this imbalance of having no factory for the Generic device
is wrong, but usually factories only match a distinct set of device
types, while the generic factory would handle them all (as last resort).
Without this, activating a generic connection might activate the
wrong interface.
We have unit tests for writing and re-reading ifcfg file. Those
tests compare whether a file can be successfully read and is
semantically identical.
However, there were no tests that a certain output is written in
a stable format. We aim not to change the output of what we write.
For that, add tests to not only check the semantic of the written
ifcfg file, but their bits and bytes.
Some future changes may well intentionally change the current
output. That will require to update the expected result files
and can be done via
NMTST_IFCFG_RH_UPDATE_EXPECTED=yes src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh
Note that alias, route, and key files are not checked.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1445414
As NMDevice now creates the NMPacrunnerManager instance
as needed, it is even more likely that the initial call
to nm_pacrunner_manager_send() will only queue (but not yet
send) the new config.
Later, when the D-Bus proxy is created, we will not get a
name-owner changed signal. We instead have to push the configuration
right away.
nm_pacrunner_manager_remove() required a "tag" argument. It was a
bug for callers trying to remove a configuration for a non-existing
tag.
That effectively means, the caller must keep track of whether a certain
"tag" is pending. The caller also must remember the tag -- a tag that he
must choose uniquely in the first place.
Turn that around and have nm_pacrunner_manager_send() return a (non
NULL) call-id. This call-id may later be used to remove the
configuration.
Apparently, previously the tracking of the "tag" was not always correct
and we hit the assertion in nm_pacrunner_manager_remove().
https://bugzilla.redhat.com/show_bug.cgi?id=1444374
Usually, this "<allow send_destination="..."/>" part is shipped
by firewalld's D-Bus policy. However, if firewalld is initially
not installed with NetworkManager already running, dbus-daemon
seems to cache the missing permission for the D-Bus connection.
As a result, when installing and starting firewalld, NetworkManager
requests fail until restart:
firewall: [0x7f4b83643890,change:"eth1"]: complete: request failed (Rejected send message, 1 matched rules; type="method_call", sender=":1.3" (uid=0 pid=715 comm="/usr/sbin/NetworkManager --no-daemon ") interface="org.fedoraproject.FirewallD1.zone" member="changeZone" error name="(unset)" requested_reply="0" destination=":1.25" (uid=0 pid=1243 comm="/usr/bin/python -Es /usr/sbin/firewalld --nofork -"))
https://bugzilla.redhat.com/show_bug.cgi?id=1436770
We want to ignore certain errors from firewalld. In the past,
the error message contained only the error code.
Since recently ([1], [2]), the error message contains a longer text:
NetworkManager[647]: <debug> [1492768494.7475] device[0x7f7f21e78f50] (eth0): Activation: setting firewall zone 'default'
NetworkManager[647]: <debug> [1492768494.7475] firewall: [0x7f7f21ed8900,change:"eth0"]: firewall zone change eth0:default
...
firewalld[2342]: ERROR: UNKNOWN_INTERFACE: 'eth0' is not in any zone
NetworkManager[647]: <warn> [1492768494.7832] firewall: [0x7f7f0400c780,remove:"eth0"]: complete: request failed (UNKNOWN_INTERFACE: 'eth0' is not in any zone)
[1] c77156d7f6
[2] 7c6ab456c5
We now initialize the NMFirewallManager asynchronously. That means, at
first firewalld appears as "not running", for which we usually would
fake-success right away.
It would be complex for callers to wait for firewall-manager to be
ready. So instead, have the asynchronous requests be queued and
complete them once the D-Bus proxy is initialized.
Next we will get another mode, so an is-idle doesn't cut it.
It can be confusing where the mode is set and where it is only
accessed read-only. For that, add mode_mutable.
Creating it asynchronously changes that on the first call to
nm_firewall_manager_get() the instance is not yet running.
Note that NMPolicy already connects to the "STARTED" signal and
reapplies the zones when firewalld appears. So, this delayed
change of the running state is handled mostly fine already.
One part is still missing, it's to queue add_or_change/remove calls
while the firewall manager is initializing. That follows next.
Since commit 2d1b85f (th/assume-vs-unmanaged-bgo746440), we clearly
distinguish between two modes when encountering devices with external
IP configuration:
a) external devices. For those devices we generate a volatile in-memory
connection and pretend it's active. However, the device must not be
touched by NetworkManager in any way.
b) assume, seamless take over. Mostly for restart of NetworkManager,
we activate a connection gracefully without going through an down-up
cycle. After the device reaches activated state, the device is
considered fully managed. For this only an existing, non volatile
connection can be used.
Before 'th/assume-vs-unmanaged-bgo746440', the behaviors were not
clearly separated.
Since then, we only choose to assume a connection (b) when the state
file indicates a matching connection. Now, extend this to also assume
connections when:
- during first-start (not after a restart) when there is no
state file yet.
- and, if we have an existing, non volatile, connection which
matches the device's configuration.
This patch lets NetworkManager assume connection also on first start.
That is for example useful when handing over network configuration from
initrd.
This only applies to existing, permanent, matching(!) connections, so it is a
good guess that the user wants NM to take over this interface. This brings us
closer to the previous behavior before 'th/assume-vs-unmanaged-bgo746440'.
https://bugzilla.redhat.com/show_bug.cgi?id=1439220
nm_config_device_state_*() always access the file system directly,
they don't cache data in NMConfig. Hence, they don't use the
@self argument.
Maybe those functions don't belong to nm-config.h, anyway. For lack
of a better place they are there.
Before, setting a device to unmanaged causes it to go down and clear
the interface state.
It may be useful to instruct NetworkManager not to touch the device
anymore but leave the current state up. Changing behavior for
nmcli device set "$DEV" managed no
To get the previous behavior, one has to first disconnect the interface
via
nmcli device disconnect "$DEV"
nmcli device set "$DEV" managed no
Note that non-permanent addresses like from DHCP will eventually time
out because NetworkManager stops the DHCP client. When instructing
NetworkManager to let go of the device, you have to take it over in
any way you see fit.
https://bugzilla.redhat.com/show_bug.cgi?id=1371433
Arguably, we currently only have one instance of NMPlatform,
NMRouteManager, NMDefaultRouteManager -- the one owned by the
NMNetns singleton.
Hence, all these instances we create with "log-with-ptr" set explicitly
to false.
In the future we want to support namespaces, and it will be be common to
have multiple instances. For that we have "log-with-ptr" so we are able
to disambiguiate the logging.
Change the default to TRUE because it makes more sense. It has currently
no effect as the default is never used.
Reduce the use of NM_PLATFORM_GET / nm_platform_get() to get
the platform singleton instance.
For one, this is a step towards supporting namespaces, where we need
to use different NMNetns/NMPlatform instances depending on in which
namespace the device lives.
Also, we should reduce our use of singletons. They are difficult to
coordinate on shutdown. Instead there should be a clear order of
dependencies, expressed by owning a reference to those singelton
instances. We already own a reference to the platform singelton,
so use it and avoid NM_PLATFORM_GET.
This also ensures that we own a reference to the
NMPlatform, NMRouteManager and NMDefaultRouteManager
instances. See bug rh#1440089 where we might access
the singleton getter after destroing the singleton
instance of NMRouteManager. This is prevented by
keeping a reference to those instances -- indirectly
via the netns instance.
Later, we may add support for multiple namespaces. Then it might
make sense to swap the NMNetns instance of a device when moving
the device between namespaces.
Also, drop the use of singelton instances.
https://bugzilla.redhat.com/show_bug.cgi?id=1440089
NMPlatform, NMRouteManager and NMDefaultRouteManager are singletons
instances. Users of those are for example NMDevice, which registers
to GObject signals of both NMPlatform and NMRouteManager.
Hence, as NMDevice:dispose() disconnects the signal handlers, it must
ensure that those singleton instances live longer then the NMDevice
instance. That is usually accomplished by having users of singleton
instances own a reference to those instances.
For NMDevice that effectively means that it shall own a reference to
several singletons.
NMPlatform, NMRouteManager, and NMDefaultRouteManager are all
per-namespace. In general it doesn't make sense to have more then
one instances of these per name space. Nnote that currently we don't
support multiple namespaces yet. If we will ever support multiple
namespaces, then a NMDevice would have a reference to all of these
manager instances. Hence, introduce a new class NMNetns which bundles
them together.
Set the device state as removed when the link disappears, so that in
the call to unrealize() when the device is unmanaged we also perform a
cleanup of it and especially, we terminate any DHCP client instances
running on the device.
If we keep DHCP clients running, we can hit assertions later when we
start another instance on the same interface, because we kill the old
dhclient from the pidfile, and the g_child_watch_add() done by the
first client instance is not able to waitpid() it, complaining with:
GChildWatchSource: Exit status of a child process was requested but
ECHILD was received by waitpid(). Most likely the process is
ignoring SIGCHLD, or some other thread is invoking waitpid() with a
nonpositive first argument; either behavior can break applications
that use g_child_watch_add()/g_spawn_sync() either directly or
indirectly.
https://bugzilla.redhat.com/show_bug.cgi?id=1436602
The input list of routes is allowed to contain non-normalized routes,
that is, routes which host part is non-zero. Such routes are rejected
by kernel, but NM should transparently allow them (by normalizing
the host part).
The ID comparison function route_id_cmp() already properly ignored
the (possibly non-zero) host part. However, in the internal list we
also should make sure not to track such routes. We achive that by
normalizing the host part to zero.
Note that below we check whether the tracked route is idential to
the route configured at platform. If we don't normalize the host part,
the comparison will always indicate that the route is not yet
configured, and thus we will re-sync the route every time.
Kernel requires that routes have a host part of zero. For NetworkManager
configuration we allow non-zero host parts (but ignore them). Fix
route_compare() to ignore the host part.
This has only effect during assuming connections. That means, on
restart NM would fail to match a connection with static routes
if it has a non-zero host part. So, the impact is rather small.
Routes with a non-zero host part are not allowed by kernel and
don't really exist. We didn't reject such routes in users configuration,
so various part of NM allow such routes. NM should silently strip
the host part.
Extend the cache's route ID to clear the host part too.
Note that NM's handling of routes is fundamentally flawed, as
for kernels routes don't have an "id" (or rather: all properties
of a route are part of it's ID, not only the family,ifindex,
network/plen and metric tuple (see related bug rh#1337855).
Platform's add/remove operations accept a "network" argument.
Kernel requires that the host part (based on plen) is all zero.
For NetworkManager we are more resilient to user configuration.
Cleanup the input argument already before calling _nl_msg_new_route().
Note that we use the same "network" argument to construct a obj_id
instance and to find the route in the cache (do_add_addrroute()).
Without cleaning the host part, the added object cannot be found
and the add-route command seemingly fails.
Got an assertion due to priv-proxy unset.
NMDevice:
- _platform_link_cb_idle()
- nm_device_unrealize() [NMDeviceTun]
- nm_device_state_changed()
- _set_state_full()
NMVpnConnection:
- _set_vpn_state()
- call_plugin_disconnect()
It seam to me, that can only happen if the NMVpnConnection never
completed on_proxy_acquired() and is still in preparing state when
being disconnected.
Avoid that be checking whether we have a proxy.
https://bugzilla.redhat.com/show_bug.cgi?id=1442064
When a VPN connection can't be activated we have to unexport and
dispose it. Commit f2182fbf9b ("core: don't emit double
PropertiesChanged signal for new active connections") removed the call
to nm_exported_object_unexport() in case of failure because the active
connection already gets unreferenced on failure.
However, an exported object can't be disposed until it's explicitly
unexported because GDBus code keeps a reference to it. The result was
that the active connection was kept alive and exported, but without
explicit references to it. As soon as the connection was unexported,
it was also automatically disposed, causing issues like:
(src/nm-exported-object.c:1025):dispose: code should not be reached
#0 _g_log_abort () at /lib64/libglib-2.0.so.0
#1 g_logv () at /lib64/libglib-2.0.so.0
#2 g_log () at /lib64/libglib-2.0.so.0
#3 g_warn_message () at /lib64/libglib-2.0.so.0
#4 dispose (object=0xaaf110) at src/nm-exported-object.c:1025
#5 dispose (object=0xaaf110) at src/nm-active-connection.c:1246
#6 dispose (object=0xaaf110) at src/vpn/nm-vpn-connection.c:2642
#7 g_object_unref () at /lib64/libgobject-2.0.so.0
#8 registration_data_free () at /lib64/libgio-2.0.so.0
#9 g_hash_table_remove_internal () at /lib64/libglib-2.0.so.0
#10 g_dbus_object_manager_server_unexport_unlocked () at /lib64/libgio-2.0.so.0
#11 g_dbus_object_manager_server_unexport () at /lib64/libgio-2.0.so.0
#12 nm_bus_manager_unregister_object (self=0x9069e0, object=object@entry=0xaaf110) at src/nm-bus-manager.c:858
#13 nm_exported_object_unexport (self=0xaaf110) at src/nm-exported-object.c:714
#14 _settings_connection_removed (connection=<optimized out>, user_data=0xaaf110) at src/nm-active-connection.c:184
#15 g_closure_invoke () at /lib64/libgobject-2.0.so.0
#16 signal_emit_unlocked_R () at /lib64/libgobject-2.0.so.0
#17 g_signal_emit_valist () at /lib64/libgobject-2.0.so.0
#18 g_signal_emit_by_name () at /lib64/libgobject-2.0.so.0
#19 nm_settings_connection_signal_remove (self=self@entry=0x9e4a80, allow_reuse=allow_reuse@entry=0) at src/settings/nm-settings-connection.c:2085
#20 do_delete (self=0x9e4a80, callback=0x58106a <con_delete_cb>, user_data=0xa84fa0) at src/settings/nm-settings-connection.c:768
#21 do_delete (connection=0x9e4a80, callback=0x58106a <con_delete_cb>, user_data=0xa84fa0) at src/settings/plugins/keyfile/nms-keyfile-connection.c:127
#22 nm_settings_connection_delete (self=self@entry=0x9e4a80, callback=callback@entry=0x58106a <con_delete_cb>, user_data=0xa84fa0) at src/settings/nm-settings-connection.c:694
#23 delete_auth_cb (self=self@entry=0x9e4a80, context=context@entry=0x7fffd80131e0, subject=0x91fb40, error=<optimized out>, data=data@entry=0x0) at src/settings/nm-settings-connection.c:1879
#24 pk_auth_cb (chain=0x7fffd00024a0, chain_error=<optimized out>, context=0x7fffd80131e0, user_data=<optimized out>) at src/settings/nm-settings-connection.c:1351
#25 auth_chain_finish (user_data=0x7fffd00024a0) at src/nm-auth-utils.c:92
#26 g_idle_dispatch () at /lib64/libglib-2.0.so.0
Restore the unexport upon failure to fix this.
Fixes: f2182fbf9bhttps://bugzilla.redhat.com/show_bug.cgi?id=1440077
The address change involves setting the link down which causes the supplicant
interface to change state and in turn another scan attempt. This could lead to
a loop in case of broken drivers that are not able to change the MAC address
iff the MAC address is attempted at each scan request.
https://bugzilla.redhat.com/show_bug.cgi?id=1382741
