The error should be freed by callback functions, but only
_monitor_bridges_cb() actually does it. Simplify this by letting the
caller own the error.
Fixes: 830a5a14cb
(cherry picked from commit 878a3a4125)
When NM quits it destroys all singletons including NMOvsdb, which
invokes callbacks for every pending method call. In the shutdown,
extra care must be taken to not access objects that are already in a
inconsistent state; for example here, the callback changes the device
state, and this causes an access to data that has already been
cleared:
#0 _g_log_abort (breakpoint=breakpoint@entry=1) at gmessages.c:554
#1 g_logv (log_domain=0x5635653b6817 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffb4b2c1e0) at gmessages.c:1362
#2 g_log (log_domain=log_domain@entry=0x5635653b6817 "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7fbb3f58fa4a "%s: assertion '%s' failed") at gmessages.c:1403
#3 g_return_if_fail_warning (log_domain=log_domain@entry=0x5635653b6817 "NetworkManager", pretty_function=pretty_function@entry=0x5635653b6b00 <__func__.34463> "nm_device_factory_manager_find_factory_for_connection", expression=expression@entry=0x5635653b6719 "factories_by_setting") at gmessages.c:2702
#4 nm_device_factory_manager_find_factory_for_connection (connection=connection@entry=0x56356627e0e0) at src/devices/nm-device-factory.c:243
#5 nm_manager_get_connection_iface (self=0x563566241080 [NMManager], connection=connection@entry=0x56356627e0e0, out_parent=out_parent@entry=0x0, error=error@entry=0x0) at src/nm-manager.c:1458
#6 check_connection_compatible (self=<optimized out>, connection=0x56356627e0e0) at src/devices/nm-device.c:4679
#7 check_connection_compatible (device=0x56356647b1b0 [NMDeviceOvsInterface], connection=0x56356627e0e0) at src/devices/ovs/nm-device-ovs-interface.c:95
#8 _nm_device_check_connection_available (self=0x56356647b1b0 [NMDeviceOvsInterface], connection=0x56356627e0e0, flags=NM_DEVICE_CHECK_CON_AVAILABLE_NONE, specific_object=0x0) at src/devices/nm-device.c:12102
#9 nm_device_check_connection_available (self=self@entry=0x56356647b1b0 [NMDeviceOvsInterface], connection=0x56356627e0e0, flags=flags@entry=NM_DEVICE_CHECK_CON_AVAILABLE_NONE, specific_object=specific_object@entry=0x0) at src/devices/nm-device.c:12131
#10 nm_device_recheck_available_connections (self=self@entry=0x56356647b1b0 [NMDeviceOvsInterface]) at src/devices/nm-device.c:12238
#11 _set_state_full (self=self@entry=0x56356647b1b0 [NMDeviceOvsInterface], state=state@entry=NM_DEVICE_STATE_FAILED, reason=reason@entry=NM_DEVICE_STATE_REASON_OVSDB_FAILED, quitting=quitting@entry=0) at src/devices/nm-device.c:13065
#12 nm_device_state_changed (self=self@entry=0x56356647b1b0 [NMDeviceOvsInterface], state=state@entry=NM_DEVICE_STATE_FAILED, reason=reason@entry=NM_DEVICE_STATE_REASON_OVSDB_FAILED) at src/devices/nm-device.c:13328
#13 del_iface_cb (error=<optimized out>, user_data=0x56356647b1b0) at src/devices/ovs/nm-device-ovs-port.c:160
#14 _transact_cb (self=self@entry=0x5635662b9ba0 [NMOvsdb], result=result@entry=0x0, error=0x563566259a10, user_data=user_data@entry=0x5635662ff320) at src/devices/ovs/nm-ovsdb.c:1449
#15 ovsdb_disconnect (self=self@entry=0x5635662b9ba0 [NMOvsdb]) at src/devices/ovs/nm-ovsdb.c:1331
#16 dispose (object=0x5635662b9ba0 [NMOvsdb]) at src/devices/ovs/nm-ovsdb.c:1558
#17 g_object_unref (_object=0x5635662b9ba0) at gobject.c:3293
#18 _nm_singleton_instance_destroy () at src/nm-core-utils.c:138
#19 _dl_fini () at dl-fini.c:253
#20 __run_exit_handlers (status=status@entry=0, listp=0x7fbb3e1ad6c8 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true) at exit.c:77
#21 __GI_exit (status=status@entry=0) at exit.c:99
#22 main (argc=1, argv=0x7fffb4b2cc38) at src/main.c:468
Add a new error code to indicate to callbacks that we are quitting and
no further action must be taken. This is preferable to having
additional references because it allows us to free the resources owned
by callbacks immediately, while references can easily create loops.
https://bugzilla.redhat.com/show_bug.cgi?id=1543871
(cherry picked from commit cf79615169)
Always reschedule a lease renewal attempt: just clear the scheduled
renewal if the connection is really deactivated.
(cherry picked from commit 1a20ff86d5)
Example: when dhcpv4 lease renewal fails, if ipv4.may-fail was "yes",
check also if we have a successful ipv6 conf: if not fail.
Previously we just ignored the other ip family status.
(cherry picked from commit da0fee4d9f)
After writing the connection to disk and rereading it, in addition to
restoring agent-owned secrets in the cache we must also restore
agent-owned secrets from the original connections since they are lost
during the write.
Reported-by: Märt Bakhoff <anon@sigil.red>
https://bugzilla.gnome.org/show_bug.cgi?id=793324
(cherry picked from commit f9c50bf3d3)
The bridge test (and no other either) no longer sets sysfs properties,
so this whole madness is no longer needed. That is good, because Linux
got somewhat stricter (at least in 4.15) about mounting sysfs and the
whole thing wouldn't work with containers where /sys is red-only from
the start.
(cherry picked from commit 6788ced98d)
It's nicer but also doesn't annoy gcc 8: "error: ‘strncpy’ specified bound
depends on the length of the source argument [-Werror=stringop-overflow=]"
(cherry picked from commit 85c0dc4a92)
This is basically the case in the COPR build system where this
(mount -o bind,ro /proc/sys /proc/sys) is the case for reasons unknown.
(cherry picked from commit 984e9d5655)
Previously we always updated resolv.conf on quit. When we are using
systemd-resolved the update is not necessary because the resolver on
127.0.0.53 would still be reachable after NM quits. Also, when NM
manages resolv.conf directly there is no need to update the file
again. Let's rewrite resolv.conf only when using dnsmasq.
https://bugzilla.redhat.com/show_bug.cgi?id=1541031
(cherry picked from commit 37eed6984b)
If IPV6CP terminates before IPCP, pppd enters the RUNNING phase and we
start IP configuration without having an IP interface set, which
triggers assertions. Instead, reimplement stage3_ip6_config_start to
be a no-op. Note that IPv6 configuration on PPP devices has never been
supported by NM.
This is a simpler version of upstream commit dd98ada33f ("ppp:
introduce SetIfindex pppd plugin D-Bus method") that doesn't require
changing the internal plugin API.
https://bugzilla.redhat.com/show_bug.cgi?id=1515829
Setting the state of NMActiveConnection results in invoking callbacks
in NMManager. Hence, it might be far-reaching. Clear
priv->queued_act_request before invoking the callbacks.
(cherry picked from commit ecf3677e57)
Don't just directly switch to DISCONNECTED state. If we are ACTIVATING
or ACTIVATED, first transition to DISCONNECTING state.
(cherry picked from commit 6d623825f6)
There is a small change in behavior:
Previously, the DEACTIVATING/DEACTIVATED states were set if and only if
the previous state was less or equal then ACTIVATED. For example,
if the state was already DEACTIVATING, it would have done nothing.
Now, nm_active_connection_set_state_fail() transitions the states
depending on the previous state. E.g. it would only set DEACTIVATING
state, if the previous state was ACTIVATING/ACTIVATED. On the other hand,
it would always progress the state to DEACTIVATED.
The new behavior makes more sense to me, although I doubt that there is
a visible difference.
(cherry picked from commit c5a97ad265)
unmanaged_to_disconnected() is supposed to mark the device as managed.
However, it may easily be unable to do so, for example if the device
is unmanaged by NM_UNMANAGED_USER_SETTINGS.
Shortly before actually enqueuing the activation request, check and
error out. Otherwise, we might hit an assertion later in
_device_activate().
(cherry picked from commit c6d0fbe7b0)
Note how recheck_assume_connection() called:
nm_exported_object_export (NM_EXPORTED_OBJECT (active));
active_connection_add (self, active);
nm_device_queue_activation (device, NM_ACT_REQUEST (active));
That differs from the order during _internal_activate_generic(), where
we would end up with:
nm_exported_object_export (NM_EXPORTED_OBJECT (active));
nm_device_queue_activation (device, NM_ACT_REQUEST (active));
active_connection_add (self, active);
It makes more sense to me to *first* add the connection, and only then
starting the activation with nm_device_queue_activation().
Also, let active_connection_add() always export the new active
connection object, if it is not already exported. All callers of
active_connection_add() ensured that the new object is already
exported.
(cherry picked from commit 6b08d2dda2)
Also, drop two redundant g_assert(). If we proceed, we will very soon afterwards
hit a SEGFAULT or a g_return_val_if_fail(), which is just as good.
(cherry picked from commit 0df3837656)
NM_DEVICE_OVS_INTERFACE_GET_PRIVATE() is implemented via the _NM_GET_PRIVATE()
macro. This macro uses C11's _Generic() to provide additional compiler checks
when casting from an incompatible pointer type.
As such,
NMDevice *device = ...;
NMDeviceOvsInterfacePrivate *priv;
priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (device);
causes a compilation error:
error: ‘_Generic’ selector of type ‘NMDevice * {aka struct _NMDevice *}’ is not compatible with any association
One workaround would be to cast the pointer first:
priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE ((NMDeviceOvsInterface *) device);
A better fix is to mark NMDevice as a compatible pointer in _NM_GET_PRIVATE(),
which this patch does.
Previously, this went unnoticed, because due to bug "a43bf3388 build: fix configure
check for CC support of _Generic() and __auto_type", we failed to detect support
for _Generic() when compiling with -Werror. That essentially disables this check,
and NM_DEVICE_OVS_INTERFACE_GET_PRIVATE() would do a direct cast.
A workaround for this build failure might be to build with -Werror, which accidentally
results in not using _Generic().
https://bugzilla.gnome.org/show_bug.cgi?id=793183
Fixes: 8ad310f8e3
(cherry picked from commit 782578122c)
OvsInterface can postpone the stage3_ip[46]_config until the link
actually appears. It ought to restart the stage only when the link
appears, not upon further changes to it (which would trip an assertion
when starting the DHCP client while one already exists).
https://bugzilla.redhat.com/show_bug.cgi?id=1540063
(cherry picked from commit 8ad310f8e3)
We already avoid committing the IP configuration for external devices
(see commit 60334a2893). However, we still start DHCP/IPv6-autoconf
and, especially, we change sysctl values of the device.
To be sure that no action is taken on the device, return early from
the IP configuration phase, as in the method=disabled/ignore case.
https://bugzilla.redhat.com/show_bug.cgi?id=1530288
(cherry picked from commit 22f32a16f5)
Kernel (as of 4.14) merely ACKs our RTM_DELQDISC and RTM_DELTFILTER, not
bothering to signal the full RTM_DEL* message unless the removal is
external to NetworkManager.
https://bugzilla.redhat.com/show_bug.cgi?id=1527197
(cherry picked from commit f3b4053a91)
The internal client asserts that the length of the client ID is not more
than MAX_CLIENT_ID_LEN. Avoid that assert by truncating the string.
Also add new nm_dhcp_client_set_client_id_*() setters, that either
set the ID based on a string (in our common dhclient specific
format), or based on the binary data (as obtained from systemd client).
Also, add checks and assertions that the client ID which is
set via nm_dhcp_client_set_client_id() is always of length
of at least 2 (as required by rfc2132, section-9.14).
(cherry picked from commit 686afe531a)
Add a new device state reason code for unsupported IP method. It is
returned, for example, when users select manual IP configuration for
WWAN connections:
# nmcli connection mod Gsm ipv4.method manual ipv4.address 1.2.3.4/32
# nmcli connection up Gsm
Error: Connection activation failed: The selected IP method is not
supported
compared to the old:
Error: Connection activation failed: IP configuration could not be
reserved (no available address, timeout, etc.)
Note that we could instead fail the connection validation if the
method is not supported by the connection type, but adding such
limitation now could make existing connections invalid.
https://bugzilla.redhat.com/show_bug.cgi?id=1459529
(cherry picked from commit aa820e9386)
Don't call nm_utils_parse_inaddr_bin() if the string returned by
mm_bearer_ip_config_get_address() and mm_bearer_ip_config_get_gateway()
is NULL, as the function requires a valid pointer. Throw an error if the
address is NULL, but allow an empty gateway.
Fixes: 7837afe87f
(cherry picked from commit 8ddc6caf98)
When a new settings-connection is populated with the actual settings
read from disk by the plugin, calling nm_settings_connection_update()
with KEEP mode also marks it as unsaved, which should not happen on a
new connection just written to (or read from) disk.
Introduce a new KEEP_SAVED persist mode that is similar to KEEP but
clears the UNSAVED flag.
Fixes: 023ce50d21https://bugzilla.redhat.com/show_bug.cgi?id=1525078
(cherry picked from commit 5fff928a6b)
NMManager tries to assign unique route-metrics in an increasing manner
so that the device which activates first keeps to have the best routes.
This information is also persisted in the device's state file, however
we not only need to persist the effective route-metric which was
eventually chosen by NMManager, but also the aspired metric.
The reason is that when a metric is chosen for a device, the entire
range between aspired and effective route-metric is reserved for that
device. We must remember the entire range so that after restart the
entire range is still considered to be in use.
Fixes: 6a32c64d8f
(cherry picked from commit 4277bc0ee0)
If a volatile connection is deleted by user when it was already being
deleted internally because the device vanished, we may hit the
following failed assertion:
file src/settings/nm-settings-connection.c: line 2196
(nm_settings_connection_signal_remove): should not be reached
The @removed flag keeps track of whether we already signaled the
connection removal. Instead of throwing an assertion if we try to emit
the signal again, just return without action because this can happen
in the situation described above.
While at it, remove the @allow_reuse argument from
nm_settings_connection_signal_remove(): we should never emit the
signal twice. Instead, we should reset the @removed flag when the
connection is added.
Fixes: a9384452edhttps://bugzilla.redhat.com/show_bug.cgi?id=1506552
(cherry picked from commit 98ac0f404e)
First check that the limit of 50 metric points is not surpassed.
Otherwise, if you have an ethernet device (aspired 100, effective
130) and a MACSec devic (aspired 125, effective 155), activating a
new ethernet device would bump it's metric to 155 -- more then
the 50 points limit.
It doesn't matter too much, because the cases where the limit of
50 could have been surpassed were very specific. Still, change
it to ensure that the limit is always honored as one would expect.
Fixes: 6a32c64d8f
(cherry picked from commit 2499d3bdc6)
