From d18d292b69069a17597989251c05606cbe50a0eb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Apr 2018 14:48:52 +0200 Subject: Revert "core: merge branch 'bg/restart-assume-rh1551958'" This reverts commit cc1920d71470042c4e0837848da9183526b663d0, reversing changes made to eb8257dea5802a004af9cccacb30af98440e2172. This breaks restart, at least for Wi-Fi devices: #0 0x00007ffff5ee8771 in _g_log_abort (breakpoint=breakpoint@entry=1) at gmessages.c:554 #1 0x00007ffff5ee9a5b in g_logv (log_domain=0x7ffff671a738 "GLib-GIO", log_level=G_LOG_LEVEL_CRITICAL, format=, args=args@entry=0x7fffffffd720) at gmessages.c:1362 #2 0x00007ffff5ee9baf in g_log (log_domain=log_domain@entry=0x7ffff671a738 "GLib-GIO", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7ffff5f347ea "%s: assertion '%s' failed") at gmessages.c:1403 #3 0x00007ffff5eea0f9 in g_return_if_fail_warning (log_domain=log_domain@entry=0x7ffff671a738 "GLib-GIO", pretty_function=pretty_function@entry=0x7ffff673fc10 <__func__.25628> "g_dbus_proxy_call_internal", expression=expression@entry=0x7ffff673fb1c "G_IS_DBUS_PROXY (proxy)") at gmessages.c:2702 #4 0x00007ffff66cdc5f in g_dbus_proxy_call_internal (proxy=0x0, method_name=method_name@entry=0x555555810510 "Scan", parameters=0x555555c7a530, flags=flags@entry=G_DBUS_CALL_FLAGS_NONE, timeout_msec=timeout_msec@entry=-1, fd_list=fd_list@entry=0x0, cancellable=0x0, callback=0x55555574cb96 , user_data=0x555555ac2220) at gdbusproxy.c:2664 #5 0x00007ffff66cf686 in g_dbus_proxy_call (proxy=, method_name=method_name@entry=0x555555810510 "Scan", parameters=, flags=flags@entry=G_DBUS_CALL_FLAGS_NONE, timeout_msec=timeout_msec@entry=-1, cancellable=cancellable@entry=0x0, callback=0x55555574cb96 , user_data=0x555555ac2220) at gdbusproxy.c:2970 #6 0x000055555574e026 in nm_supplicant_interface_request_scan (self=0x555555ac2220 [NMSupplicantInterface], ssids=ssids@entry=0x0) at src/supplicant/nm-supplicant-interface.c:1821 #7 0x00007fffe1038276 in request_wireless_scan (self=self@entry=0x555555c6ee60 [NMDeviceWifi], periodic=periodic@entry=0, force_if_scanning=force_if_scanning@entry=0, ssids=, ssids@entry=0x0) at src/devices/wifi/nm-device-wifi.c:1347 #8 0x00007fffe1039011 in device_state_changed (device=0x555555c6ee60 [NMDeviceWifi], new_state=NM_DEVICE_STATE_DISCONNECTED, old_state=, reason=) at src/devices/wifi/nm-device-wifi.c:2998 #9 0x00007ffff432ed1e in ffi_call_unix64 () at ../src/x86/unix64.S:76 #10 0x00007ffff432e68f in ffi_call (cif=cif@entry=0x7fffffffdc70, fn=fn@entry=0x7fffe1038e1e , rvalue=, avalue=avalue@entry=0x7fffffffdb60) at ../src/x86/ffi64.c:525 #15 0x00007ffff63db66f in (instance=instance@entry=0x555555c6ee60, signal_id=, detail=detail@entry=0) at gsignal.c:3447 #11 0x00007ffff63bff39 in g_cclosure_marshal_generic (closure=0x555555c22ea0, return_gvalue=0x0, n_param_values=, param_values=, invocation_hint=, marshal_data=) at gclosure.c:1490 #12 0x00007ffff63bf73d in g_closure_invoke (closure=0x555555c22ea0, return_value=0x0, n_param_values=4, param_values=0x7fffffffdea0, invocation_hint=0x7fffffffde20) at gclosure.c:804 #13 0x00007ffff63d1f30 in signal_emit_unlocked_R (node=node@entry=0x555555c22750, detail=detail@entry=0, instance=instance@entry=0x555555c6ee60, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffdea0) at gsignal.c:3673 #14 0x00007ffff63dad05 in g_signal_emit_valist (instance=0x555555c6ee60, signal_id=, detail=0, var_args=var_args@entry=0x7fffffffe0b0) at gsignal.c:3391 #16 0x00005555556f0f18 in _set_state_full (self=self@entry=0x555555c6ee60 [NMDeviceWifi], state=state@entry=NM_DEVICE_STATE_DISCONNECTED, reason=reason@entry=NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED, quitting=quitting@entry=0) at src/devices/nm-device.c:13268 #17 0x00005555556f1774 in nm_device_state_changed (self=self@entry=0x555555c6ee60 [NMDeviceWifi], state=state@entry=NM_DEVICE_STATE_DISCONNECTED, reason=reason@entry=NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) at src/devices/nm-device.c:13435 #18 0x00005555555bcf95 in recheck_assume_connection (self=self@entry=0x555555b09140 [NMManager], device=device@entry=0x555555c6ee60 [NMDeviceWifi]) at src/nm-manager.c:2297 #19 0x00005555555bd53e in _device_realize_finish (self=self@entry=0x555555b09140 [NMManager], device=device@entry=0x555555c6ee60 [NMDeviceWifi], plink=plink@entry=0x555555ae43d8) at src/nm-manager.c:2473 #20 0x00005555555c01d0 in platform_link_added (self=self@entry=0x555555b09140 [NMManager], ifindex=, plink=plink@entry=0x555555ae43d8, guess_assume=, dev_state=) at src/nm-manager.c:2789 #21 0x00005555555c0cec in platform_query_devices (self=self@entry=0x555555b09140 [NMManager]) at src/nm-manager.c:2901 #22 0x00005555555c439e in nm_manager_start (self=0x555555b09140 [NMManager], error=) at src/nm-manager.c:5632 #23 0x000055555558498e in main (argc=, argv=) at src/main.c:413 --- src/NetworkManagerUtils.c | 24 ++++++++++++++++- src/NetworkManagerUtils.h | 1 + src/nm-manager.c | 66 ++++++++++++++++++++++++++++++++--------------- src/tests/test-general.c | 2 +- 4 files changed, 70 insertions(+), 23 deletions(-) diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index 72f2cbd568..12d46697ae 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -767,6 +767,10 @@ check_possible_match (NMConnection *orig, * @connections: a (optionally pre-sorted) list of connections from which to * find a matching connection to @original based on "inferrable" properties * @original: the #NMConnection to find a match for from @connections + * @indicated: whether the match is already hinted/indicated. That is the + * case when we found the connection in the state file from a previous run. + * In this case, we perform a relexed check, as we have a good hint + * that the connection actually matches. * @device_has_carrier: pass %TRUE if the device that generated @original has * a carrier, %FALSE if not * @match_filter_func: a function to check whether each connection from @connections @@ -786,6 +790,7 @@ check_possible_match (NMConnection *orig, NMConnection * nm_utils_match_connection (NMConnection *const*connections, NMConnection *original, + gboolean indicated, gboolean device_has_carrier, gint64 default_v4_metric, gint64 default_v6_metric, @@ -806,7 +811,24 @@ nm_utils_match_connection (NMConnection *const*connections, continue; } - if (!nm_connection_diff (original, candidate, NM_SETTING_COMPARE_FLAG_INFERRABLE, &diffs)) { + if (indicated) { + NMSettingConnection *s_orig, *s_cand; + + s_orig = nm_connection_get_setting_connection (original); + s_cand = nm_connection_get_setting_connection (candidate); + + /* It is indicated that this connection matches. Assume we have + * a match, but check for particular differences that let us + * reject the candidate. */ + if (!nm_streq0 (nm_setting_connection_get_connection_type (s_orig), + nm_setting_connection_get_connection_type (s_cand))) + continue; + if (!nm_streq0 (nm_setting_connection_get_slave_type (s_orig), + nm_setting_connection_get_slave_type (s_cand))) + continue; + + /* this is good enough for a match */ + } else if (!nm_connection_diff (original, candidate, NM_SETTING_COMPARE_FLAG_INFERRABLE, &diffs)) { if (!best_match) { best_match = check_possible_match (original, candidate, diffs, device_has_carrier, default_v4_metric, default_v6_metric); diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index 54ba10fe28..13bdb67e85 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -41,6 +41,7 @@ typedef gboolean (NMUtilsMatchFilterFunc) (NMConnection *connection, gpointer us NMConnection *nm_utils_match_connection (NMConnection *const*connections, NMConnection *original, + gboolean indicated, gboolean device_has_carrier, gint64 default_v4_metric, gint64 default_v6_metric, diff --git a/src/nm-manager.c b/src/nm-manager.c index b64cb39aa4..c82347e27e 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2125,12 +2125,12 @@ get_existing_connection (NMManager *self, gboolean *out_generated) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - NMConnection *connection = NULL; + gs_unref_object NMConnection *connection = NULL; NMSettingsConnection *added = NULL; GError *error = NULL; NMDevice *master = NULL; int ifindex = nm_device_get_ifindex (device); - NMSettingsConnection *matched = NULL; + NMSettingsConnection *matched; NMSettingsConnection *connection_checked = NULL; gboolean assume_state_guess_assume = FALSE; const char *assume_state_connection_uuid = NULL; @@ -2161,31 +2161,54 @@ get_existing_connection (NMManager *self, } } + /* The core of the API is nm_device_generate_connection() function and + * update_connection() virtual method and the convenient connection_type + * class attribute. Subclasses supporting the new API must have + * update_connection() implemented, otherwise nm_device_generate_connection() + * returns NULL. + */ + connection = nm_device_generate_connection (device, master, &maybe_later, &error); + if (!connection) { + if (!maybe_later) + nm_device_assume_state_reset (device); + _LOG2D (LOGD_DEVICE, device, "assume: cannot generate connection: %s", + error->message); + g_error_free (error); + return NULL; + } + nm_device_assume_state_get (device, &assume_state_guess_assume, &assume_state_connection_uuid); + /* Now we need to compare the generated connection to each configured + * connection. The comparison function is the heart of the connection + * assumption implementation and it must compare the connections very + * carefully to sort out various corner cases. Also, the comparison is + * not entirely symmetric. + * + * When no configured connection matches the generated connection, we keep + * the generated connection instead. + */ if ( assume_state_connection_uuid && (connection_checked = nm_settings_get_connection_by_uuid (priv->settings, assume_state_connection_uuid)) - && nm_device_check_connection_compatible (device, NM_CONNECTION (connection_checked))) - matched = connection_checked; - - if (!matched) { - /* The core of the API is nm_device_generate_connection() function and - * update_connection() virtual method and the convenient connection_type - * class attribute. Subclasses supporting the new API must have - * update_connection() implemented, otherwise nm_device_generate_connection() - * returns NULL. - */ - connection = nm_device_generate_connection (device, master, &maybe_later, &error); - if (!connection) { - if (!maybe_later) - nm_device_assume_state_reset (device); - _LOG2D (LOGD_DEVICE, device, "assume: cannot generate connection: %s", error->message); - g_error_free (error); - return NULL; - } - } + && !active_connection_find_first (self, connection_checked, NULL, + NM_ACTIVE_CONNECTION_STATE_DEACTIVATING) + && nm_device_check_connection_compatible (device, NM_CONNECTION (connection_checked))) { + NMConnection *const connections[] = { + NM_CONNECTION (connection_checked), + NULL, + }; + + matched = NM_SETTINGS_CONNECTION (nm_utils_match_connection (connections, + connection, + TRUE, + nm_device_has_carrier (device), + nm_device_get_route_metric (device, AF_INET), + nm_device_get_route_metric (device, AF_INET6), + NULL, NULL)); + } else + matched = NULL; if (!matched && assume_state_guess_assume) { gs_free NMSettingsConnection **connections = NULL; @@ -2209,6 +2232,7 @@ get_existing_connection (NMManager *self, matched = NM_SETTINGS_CONNECTION (nm_utils_match_connection ((NMConnection *const*) connections, connection, + FALSE, nm_device_has_carrier (device), nm_device_get_route_metric (device, AF_INET), nm_device_get_route_metric (device, AF_INET6), diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 5276d9b0e3..632f2c33cc 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -359,7 +359,7 @@ _match_connection (GSList *connections, } list[i] = NULL; - return nm_utils_match_connection (list, original, device_has_carrier, default_v4_metric, default_v6_metric, NULL, NULL); + return nm_utils_match_connection (list, original, FALSE, device_has_carrier, default_v4_metric, default_v6_metric, NULL, NULL); } static void -- cgit v1.2.1