diff options
author | Thomas Haller <thaller@redhat.com> | 2018-01-05 17:46:49 +0100 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2018-04-10 15:11:23 +0200 |
commit | d8a31794c8b9db243076ba0c24dfe6e496b78697 (patch) | |
tree | bdc2426e90fa820cd777120cb3a86614904c3e20 | |
parent | 4417b8bf3eef8ca7cf45dc973ab107249bd10d67 (diff) | |
download | NetworkManager-d8a31794c8b9db243076ba0c24dfe6e496b78697.tar.gz |
connectivity: rework async connectivity check requests
An asynchronous request should either be cancellable or not keep
the target object alive. Preferably both.
Otherwise, it is impossible to do a controlled shutdown when terminating
NetworkManager. Currently, when NetworkManager is about to terminate,
it just quits the mainloop and essentially leaks everything. That is a
bug. If we ever want to fix that, every asynchronous request must be
cancellable in a controlled way (or it must not prevent objects from
getting disposed, where disposing the object automatically cancels the
callback).
Rework the asynchronous request for connectivity check to
- return a handle that can be used to cancel the operation.
Cancelling is optional. The caller may choose to ignore the handle
because the asynchronous operation does not keep the target object
alive. That means, it is still possible to shutdown, by everybody
giving up their reference to the target object. In which case the
callback will be invoked during dispose() of the target object.
- also, the callback will always be invoked exactly once, and never
synchronously from within the asynchronous start call. But during
cancel(), the callback is invoked synchronously from within cancel().
Note that it's only allowed to cancel an action at most once, and
never after the callback is invoked (also not from within the callback
itself).
- also, NMConnectivity already supports a fake handler, in case
connectivity check is disabled via configuration. Hence, reuse
the same code paths also when compiling without --enable-concheck.
That means, instead of having #if WITH_CONCHECK at various callers,
move them into NMConnectivity. The downside is, that if you build
without concheck, there is a small overhead compared to before. The
upside is, we reuse the same code paths when compiling with or without
concheck.
- also, the patch synchronizes the connecitivty states. For example,
previously `nmcli networking connectivity check` would schedule
requests in parallel, and return the accumulated result of the individual
requests.
However, the global connectivity state of the manager might have have
been the same as the answer to the explicit connecitivity check,
because while the answer for the manual check is waiting for all
pending checks to complete, the global connectivity state could
already change. That is just wrong. There are not multiple global
connectivity states at the same time, there is just one. A manual
connectivity check should have the meaning of ensure that the global
state is up to date, but it still should return the global
connectivity state -- not the answers for several connectivity checks
issued in parallel.
This is related to commit b799de281bc01073c31dd2c86171b29c8132441c
(libnm: update property in the manager after connectivity check),
which tries to address a similar problem client side.
Similarly, each device has a connectivity state. While there might
be several connectivity checks per device pending, whenever a check
completes, it can update the per-device state (and return that device
state as result), but the immediate answer of the individual check
might not matter. This is especially the case, when a later request
returns earlier and obsoletes all earlier requests. In that case,
earlier requests return with the result of the currend devices
connectivity state.
This patch cleans up the internal API and gives a better defined behavior
to the user (thus, the simple API which simplifies implementation for the
caller). However, the implementation of getting this API right and properly
handle cancel and destruction of the target object is more complicated and
complex. But this but is not just for the sake of a nicer API. This fixes
actual issues explained above.
Also, get rid of GAsyncResult to track information about the pending request.
Instead, allocate our own handle structure, which ends up to be nicer
because it's strongly typed and has exactly the properties that are
useful to track the request. Also, it gets rid of the awkward
_finish() API by passing the relevant arguments to the callback
directly.
-rw-r--r-- | src/devices/nm-device.c | 340 | ||||
-rw-r--r-- | src/devices/nm-device.h | 14 | ||||
-rw-r--r-- | src/main.c | 2 | ||||
-rw-r--r-- | src/nm-connectivity.c | 544 | ||||
-rw-r--r-- | src/nm-connectivity.h | 25 | ||||
-rw-r--r-- | src/nm-manager.c | 104 | ||||
-rw-r--r-- | src/tests/config/test-config.c | 8 |
7 files changed, 652 insertions, 385 deletions
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 9e1592a750..6142908a57 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -157,6 +157,15 @@ typedef struct { that the original configuration didn't change. */ } AppliedConfig; +struct _NMDeviceConnectivityHandle { + CList concheck_lst; + NMDevice *self; + NMDeviceConnectivityCallback callback; + gpointer user_data; + NMConnectivityCheckHandle *c_handle; + guint64 seq; +}; + /*****************************************************************************/ enum { @@ -531,9 +540,14 @@ typedef struct _NMDevicePrivate { NMNetns *netns; NMLldpListener *lldp_listener; - NMConnectivityState connectivity_state; + + NMConnectivity *concheck_mgr; + gulong concheck_periodic_id; - guint64 concheck_seq; + + NMConnectivityState connectivity_state; + + CList concheck_lst_head; guint check_delete_unrealized_id; @@ -728,6 +742,16 @@ nm_device_get_platform (NMDevice *self) return nm_netns_get_platform (nm_device_get_netns (self)); } +static NMConnectivity * +concheck_get_mgr (NMDevice *self) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + + if (G_UNLIKELY (!priv->concheck_mgr)) + priv->concheck_mgr = g_object_ref (nm_connectivity_get ()); + return priv->concheck_mgr; +} + static NMIP4Config * _ip4_config_new (NMDevice *self) { @@ -1861,16 +1885,13 @@ nm_device_get_route_metric_default (NMDeviceType device_type) static gboolean default_route_metric_penalty_detect (NMDevice *self) { -#if WITH_CONCHECK NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); /* currently we don't differentiate between IPv4 and IPv6 when detecting * connectivity. */ if ( priv->connectivity_state != NM_CONNECTIVITY_FULL - && nm_connectivity_check_enabled (nm_connectivity_get ())) { + && nm_connectivity_check_enabled (concheck_get_mgr (self))) return TRUE; - } -#endif return FALSE; } @@ -2165,171 +2186,227 @@ nm_device_get_physical_port_id (NMDevice *self) /*****************************************************************************/ static void -update_connectivity_state (NMDevice *self, NMConnectivityState state) +concheck_update_state (NMDevice *self, NMConnectivityState state) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - /* If the connectivity check is disabled, make an optimistic guess. */ - if (state == NM_CONNECTIVITY_UNKNOWN) { + if (state == NM_CONNECTIVITY_ERROR) + return; + + /* If the connectivity check is disabled and we obtain a fake + * result, make an optimistic guess. */ + if (state == NM_CONNECTIVITY_FAKE) { if (priv->state == NM_DEVICE_STATE_ACTIVATED) { if (nm_device_get_best_default_route (self, AF_UNSPEC)) state = NM_CONNECTIVITY_FULL; else state = NM_CONNECTIVITY_LIMITED; - } else { + } else state = NM_CONNECTIVITY_NONE; - } } - if (priv->connectivity_state != state) { -#if WITH_CONCHECK - _LOGD (LOGD_CONCHECK, "state changed from %s to %s", - nm_connectivity_state_to_string (priv->connectivity_state), - nm_connectivity_state_to_string (state)); -#endif - priv->connectivity_state = state; - _notify (self, PROP_CONNECTIVITY); + if (priv->connectivity_state == state) + return; - if ( priv->state == NM_DEVICE_STATE_ACTIVATED - && !nm_device_sys_iface_state_is_external (self)) { - if ( nm_device_get_best_default_route (self, AF_INET) - && !ip_config_merge_and_apply (self, AF_INET, TRUE)) - _LOGW (LOGD_IP4, "Failed to update IPv4 route metric"); - if ( nm_device_get_best_default_route (self, AF_INET6) - && !ip_config_merge_and_apply (self, AF_INET6, TRUE)) - _LOGW (LOGD_IP6, "Failed to update IPv6 route metric"); - } + _LOGD (LOGD_CONCHECK, "state changed from %s to %s", + nm_connectivity_state_to_string (priv->connectivity_state), + nm_connectivity_state_to_string (state)); + priv->connectivity_state = state; + + _notify (self, PROP_CONNECTIVITY); + + if ( priv->state == NM_DEVICE_STATE_ACTIVATED + && !nm_device_sys_iface_state_is_external (self)) { + if ( nm_device_get_best_default_route (self, AF_INET) + && !ip_config_merge_and_apply (self, AF_INET, TRUE)) + _LOGW (LOGD_IP4, "Failed to update IPv4 route metric"); + if ( nm_device_get_best_default_route (self, AF_INET6) + && !ip_config_merge_and_apply (self, AF_INET6, TRUE)) + _LOGW (LOGD_IP6, "Failed to update IPv6 route metric"); } } -typedef struct { - NMDevice *self; - NMDeviceConnectivityCallback callback; - gpointer user_data; - guint64 seq; -} ConnectivityCheckData; +static void +concheck_periodic (NMConnectivity *connectivity, NMDevice *self) +{ + nm_device_check_connectivity (self, NULL, NULL); +} static void -concheck_done (ConnectivityCheckData *data) +concheck_periodic_update (NMDevice *self) { - NMDevice *self = data->self; NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - /* The unsolicited connectivity checks don't hook a callback. */ - if (data->callback) - data->callback (data->self, priv->connectivity_state, data->user_data); - g_object_unref (data->self); - g_slice_free (ConnectivityCheckData, data); + if ( priv->state == NM_DEVICE_STATE_ACTIVATED + && nm_device_get_best_default_route (self, AF_UNSPEC)) { + if (!priv->concheck_periodic_id) { + priv->concheck_periodic_id = g_signal_connect (concheck_get_mgr (self), + NM_CONNECTIVITY_PERIODIC_CHECK, + G_CALLBACK (concheck_periodic), self); + nm_device_check_connectivity (self, NULL, NULL); + } + } else { + if (priv->concheck_periodic_id) { + /* The default route has gone off, trigger a final connectivity check. */ + nm_clear_g_signal_handler (priv->concheck_mgr, &priv->concheck_periodic_id); + nm_device_check_connectivity (self, NULL, NULL); + } + } } -#if WITH_CONCHECK static void -concheck_cb (GObject *source_object, GAsyncResult *result, gpointer user_data) +concheck_handle_complete (NMDeviceConnectivityHandle *handle, + GError *error) { - ConnectivityCheckData *data = user_data; - NMDevice *self = data->self; - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - NMConnectivity *connectivity = NM_CONNECTIVITY (source_object); - NMConnectivityState state; - GError *error = NULL; + /* The moment we invoke the callback, we unlink it. It signals + * that @handle is handled -- as far as the callee of callback + * is concerned. */ + c_list_unlink (&handle->concheck_lst); - state = nm_connectivity_check_finish (connectivity, result, &error); - if (error) { - _LOGW (LOGD_DEVICE, "connectivity checking on '%s' failed: %s", - nm_device_get_iface (self), error->message); - g_error_free (error); + if (handle->c_handle) + nm_connectivity_check_cancel (g_steal_pointer (&handle->c_handle)); + + if (handle->callback) { + handle->callback (handle->self, + handle, + NM_DEVICE_GET_PRIVATE (handle->self)->connectivity_state, + error, + handle->user_data); } - if (data->seq == priv->concheck_seq) - update_connectivity_state (data->self, state); - concheck_done (data); + g_slice_free (NMDeviceConnectivityHandle, handle); } -#endif /* WITH_CONCHECK */ -static gboolean -no_concheck (gpointer user_data) +static void +concheck_cb (NMConnectivity *connectivity, + NMConnectivityCheckHandle *c_handle, + NMConnectivityState state, + GError *error, + gpointer user_data) { - ConnectivityCheckData *data = user_data; + gs_unref_object NMDevice *self = NULL; + NMDevicePrivate *priv; + NMDeviceConnectivityHandle *handle; + NMDeviceConnectivityHandle *other_handle; + gboolean handle_is_alive; + guint64 seq; - concheck_done (data); - return G_SOURCE_REMOVE; + if (nm_utils_error_is_cancelled (error, FALSE)) { + /* the only place where we nm_connectivity_check_cancel(@c_handle), is + * from inside concheck_handle_event(). This is a recursive call, + * nothing to do. */ + return; + } + + /* we keep NMConnectivity instance alive. It cannot be disposing. */ + nm_assert (!nm_utils_error_is_cancelled (error, TRUE)); + + handle = user_data; + nm_assert (handle->c_handle == c_handle); + handle->c_handle = NULL; + + /* keep @self alive, while we invoke callbacks. */ + self = g_object_ref (handle->self); + priv = NM_DEVICE_GET_PRIVATE (self); + + nm_assert (!handle || c_list_contains (&priv->concheck_lst_head, &handle->concheck_lst)); + + seq = handle->seq; + + /* first update the new state, and emit signals. */ + concheck_update_state (self, state); + + handle_is_alive = FALSE; + + /* we might have invoked callbacks during concheck_update_state(). The caller might have + * cancelled and thus destroyed @handle. We have to check whether handle is still alive, + * by searching it in the list of alive handles. + * + * Also, we might want to complete all pending callbacks that were started before + * @handle, as they are automatically obsoleted. */ +check_handles: + c_list_for_each_entry (other_handle, &priv->concheck_lst_head, concheck_lst) { + if (other_handle->seq >= seq) { + /* it's not guaranteed that @handle is still in the list. It might already + * be canceled while invoking callbacks for a previous other_handle. + * If it is already cancelled, @handle is a dangling pointer. + * + * Since @seq is assigned uniquely and increasing, either @other_handle is + * @handle (and thus, handle is alive), or it isn't. */ + if (other_handle == handle) + handle_is_alive = TRUE; + break; + } + + nm_assert (other_handle != handle); + + if (!NM_IN_SET (state, NM_CONNECTIVITY_ERROR)) { + /* we also want to complete handles that were started before the current + * @handle. Their response is out-dated. */ + concheck_handle_complete (other_handle, NULL); + + /* we invoked callbacks, other handles might be cancelled and removed from the list. + * Need to iterate the list from the start. */ + goto check_handles; + } + } + + if (!handle_is_alive) { + /* We didn't find @handle in the list of alive handles. Thus, the handles + * was cancelled while we were invoking events. Nothing to do, and don't + * touch the dangling pointer. */ + return; + } + + concheck_handle_complete (handle, NULL); } -void +NMDeviceConnectivityHandle * nm_device_check_connectivity (NMDevice *self, NMDeviceConnectivityCallback callback, gpointer user_data) { - ConnectivityCheckData *data; -#if WITH_CONCHECK - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); -#endif + static guint64 seq_counter = 0; + NMDevicePrivate *priv; + NMDeviceConnectivityHandle *handle; - data = g_slice_new0 (ConnectivityCheckData); - data->self = g_object_ref (self); - data->callback = callback; - data->user_data = user_data; + g_return_val_if_fail (NM_IS_DEVICE (self), NULL); -#if WITH_CONCHECK - if (priv->concheck_periodic_id) { - data->seq = ++priv->concheck_seq; + priv = NM_DEVICE_GET_PRIVATE (self); - /* Kick off a real connectivity check. */ - nm_connectivity_check_async (nm_connectivity_get (), - nm_device_get_ip_iface (self), - concheck_cb, - data); - return; - } -#endif + handle = g_slice_new0 (NMDeviceConnectivityHandle); + handle->seq = ++seq_counter; + handle->self = self; + handle->callback = callback; + handle->user_data = user_data; + c_list_link_tail (&priv->concheck_lst_head, &handle->concheck_lst); - /* Fake one. */ - g_idle_add (no_concheck, data); + handle->c_handle = nm_connectivity_check_start (concheck_get_mgr (self), + nm_device_get_ip_iface (self), + concheck_cb, + handle); + return handle; } -NMConnectivityState -nm_device_get_connectivity_state (NMDevice *self) +void +nm_device_check_connectivity_cancel (NMDeviceConnectivityHandle *handle) { - g_return_val_if_fail (NM_IS_DEVICE (self), NM_CONNECTIVITY_UNKNOWN); + gs_free_error GError *cancelled_error = NULL; - return NM_DEVICE_GET_PRIVATE (self)->connectivity_state; -} + g_return_if_fail (handle); + g_return_if_fail (NM_IS_DEVICE (handle->self)); + g_return_if_fail (!c_list_is_empty (&handle->concheck_lst)); -#if WITH_CONCHECK -static void -concheck_periodic (NMConnectivity *connectivity, NMDevice *self) -{ - nm_device_check_connectivity (self, NULL, NULL); + nm_utils_error_set_cancelled (&cancelled_error, FALSE, "NMDevice"); + concheck_handle_complete (handle, cancelled_error); } -#endif -static void -concheck_periodic_update (NMDevice *self) +NMConnectivityState +nm_device_get_connectivity_state (NMDevice *self) { -#if WITH_CONCHECK - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - gboolean check_enable; - - check_enable = (priv->state == NM_DEVICE_STATE_ACTIVATED) - && nm_device_get_best_default_route (self, AF_UNSPEC); - - if (check_enable && !priv->concheck_periodic_id) { - /* We just gained a default route. Enable periodic checking. */ - priv->concheck_periodic_id = g_signal_connect (nm_connectivity_get (), - NM_CONNECTIVITY_PERIODIC_CHECK, - G_CALLBACK (concheck_periodic), self); - /* Also kick off a check right away. */ - nm_device_check_connectivity (self, NULL, NULL); - } else if (!check_enable && priv->concheck_periodic_id) { - /* The default route has gone off, and so has connectivity. */ - nm_clear_g_signal_handler (nm_connectivity_get (), &priv->concheck_periodic_id); - update_connectivity_state (self, NM_CONNECTIVITY_NONE); - } -#else - /* update_connectivity_state() figures out how to lie about - * connectivity state if the actual state is not really known. */ - update_connectivity_state (self, NM_CONNECTIVITY_UNKNOWN); -#endif + g_return_val_if_fail (NM_IS_DEVICE (self), NM_CONNECTIVITY_UNKNOWN); + + return NM_DEVICE_GET_PRIVATE (self)->connectivity_state; } /*****************************************************************************/ @@ -14429,10 +14506,12 @@ nm_device_init (NMDevice *self) self->_priv = priv; + c_list_init (&priv->concheck_lst_head); c_list_init (&self->devices_lst); - c_list_init (&priv->slaves); + priv->connectivity_state = NM_CONNECTIVITY_UNKNOWN; + priv->netns = g_object_ref (NM_NETNS_GET); priv->autoconnect_blocked_flags = DEFAULT_AUTOCONNECT @@ -14546,11 +14625,19 @@ dispose (GObject *object) NMDevice *self = NM_DEVICE (object); NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMPlatform *platform; + NMDeviceConnectivityHandle *con_handle; + gs_free_error GError *cancelled_error = NULL; _LOGD (LOGD_DEVICE, "disposing"); nm_assert (c_list_is_empty (&self->devices_lst)); + while ((con_handle = c_list_first_entry (&priv->concheck_lst_head, NMDeviceConnectivityHandle, concheck_lst))) { + if (!cancelled_error) + nm_utils_error_set_cancelled (&cancelled_error, FALSE, "NMDevice"); + concheck_handle_complete (con_handle, cancelled_error); + } + nm_clear_g_cancellable (&priv->deactivating_cancellable); nm_device_assume_state_reset (self); @@ -14624,6 +14711,8 @@ dispose (GObject *object) g_clear_object (&priv->lldp_listener); } + nm_clear_g_signal_handler (priv->concheck_mgr, &priv->concheck_periodic_id); + G_OBJECT_CLASS (nm_device_parent_class)->dispose (object); if (nm_clear_g_source (&priv->queued_state.id)) { @@ -14665,8 +14754,9 @@ finalize (GObject *object) /* for testing, NMDeviceTest does not invoke NMDevice::constructed, * and thus @settings might be unset. */ - if (priv->settings) - g_object_unref (priv->settings); + nm_g_object_unref (priv->settings); + + nm_g_object_unref (priv->concheck_mgr); g_object_unref (priv->netns); } diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index f783cffbf3..24d184657f 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -775,12 +775,20 @@ gboolean nm_device_hw_addr_get_cloned (NMDevice *self, gboolean *preserve, GError **error); +typedef struct _NMDeviceConnectivityHandle NMDeviceConnectivityHandle; + typedef void (*NMDeviceConnectivityCallback) (NMDevice *self, + NMDeviceConnectivityHandle *handle, NMConnectivityState state, + GError *error, gpointer user_data); -void nm_device_check_connectivity (NMDevice *self, - NMDeviceConnectivityCallback callback, - gpointer user_data); + +NMDeviceConnectivityHandle *nm_device_check_connectivity (NMDevice *self, + NMDeviceConnectivityCallback callback, + gpointer user_data); + +void nm_device_check_connectivity_cancel (NMDeviceConnectivityHandle *handle); + NMConnectivityState nm_device_get_connectivity_state (NMDevice *self); typedef struct _NMBtVTableNetworkServer NMBtVTableNetworkServer; diff --git a/src/main.c b/src/main.c index 45704945ff..69902c8437 100644 --- a/src/main.c +++ b/src/main.c @@ -402,9 +402,7 @@ main (int argc, char *argv[]) manager)) goto done; -#if WITH_CONCHECK NM_UTILS_KEEP_ALIVE (manager, nm_connectivity_get (), "NMManager-depends-on-NMConnectivity"); -#endif nm_dispatcher_init (); diff --git a/src/nm-connectivity.c b/src/nm-connectivity.c index 52e40d6031..dd11bdc0f7 100644 --- a/src/nm-connectivity.c +++ b/src/nm-connectivity.c @@ -30,34 +30,55 @@ #include <curl/curl.h> #endif +#include "nm-utils/c-list.h" #include "nm-config.h" #include "NetworkManagerUtils.h" +#define HEADER_STATUS_ONLINE "X-NetworkManager-Status: online\r\n" + /*****************************************************************************/ -NM_UTILS_LOOKUP_STR_DEFINE (nm_connectivity_state_to_string, NMConnectivityState, +NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_state_to_string, int /*NMConnectivityState*/, NM_UTILS_LOOKUP_DEFAULT_WARN ("???"), NM_UTILS_LOOKUP_STR_ITEM (NM_CONNECTIVITY_UNKNOWN, "UNKNOWN"), NM_UTILS_LOOKUP_STR_ITEM (NM_CONNECTIVITY_NONE, "NONE"), NM_UTILS_LOOKUP_STR_ITEM (NM_CONNECTIVITY_LIMITED, "LIMITED"), NM_UTILS_LOOKUP_STR_ITEM (NM_CONNECTIVITY_PORTAL, "PORTAL"), NM_UTILS_LOOKUP_STR_ITEM (NM_CONNECTIVITY_FULL, "FULL"), + + NM_UTILS_LOOKUP_STR_ITEM (NM_CONNECTIVITY_ERROR, "ERROR"), + NM_UTILS_LOOKUP_STR_ITEM (NM_CONNECTIVITY_FAKE, "FAKE"), ); +const char * +nm_connectivity_state_to_string (NMConnectivityState state) +{ + return _state_to_string (state); +} + /*****************************************************************************/ +struct _NMConnectivityCheckHandle { + CList handles_lst; + NMConnectivity *self; + NMConnectivityCheckCallback callback; + gpointer user_data; + + char *ifspec; + #if WITH_CONCHECK + struct { + char *response; + + CURL *curl_ehandle; + struct curl_slist *request_headers; + + GString *recv_msg; + } concheck; +#endif -typedef struct { - GSimpleAsyncResult *simple; - char *response; - CURL *curl_ehandle; - size_t msg_size; - char *msg; - struct curl_slist *request_headers; guint timeout_id; - char *ifspec; -} NMConnectivityCheckHandle; +}; enum { PERIODIC_CHECK, @@ -68,14 +89,19 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { + CList handles_lst_head; char *uri; char *response; gboolean enabled; guint interval; NMConfig *config; - guint periodic_check_id; - CURLM *curl_mhandle; - guint curl_timer; +#if WITH_CONCHECK + struct { + CURLM *curl_mhandle; + guint curl_timer; + guint periodic_check_id; + } concheck; +#endif } NMConnectivityPrivate; struct _NMConnectivity { @@ -105,111 +131,172 @@ NM_DEFINE_SINGLETON_GETTER (NMConnectivity, nm_connectivity_get, NM_TYPE_CONNECT \ if (nm_logging_enabled (__level, _NMLOG2_DOMAIN)) { \ _nm_log (__level, _NMLOG2_DOMAIN, 0, \ - &cb_data->ifspec[3], NULL, \ - "connectivity: (%s) " \ - _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ - &cb_data->ifspec[3] \ - _NM_UTILS_MACRO_REST (__VA_ARGS__)); \ + (cb_data->ifspec ? &cb_data->ifspec[3] : NULL), \ + NULL, \ + "connectivity: (%s) " \ + _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ + (cb_data->ifspec ? &cb_data->ifspec[3] : "") \ + _NM_UTILS_MACRO_REST (__VA_ARGS__)); \ } \ } G_STMT_END /*****************************************************************************/ static void -finish_cb_data (NMConnectivityCheckHandle *cb_data, NMConnectivityState new_state) +cb_data_invoke_callback (NMConnectivityCheckHandle *cb_data, + NMConnectivityState state, + GError *error, + const char *log_message) { - /* Contrary to what cURL manual claim it is *not* safe to remove - * the easy handle "at any moment"; specifically not from the - * write function. Thus here we just dissociate the cb_data from - * the easy handle and the easy handle will be cleaned up when the - * message goes to CURLMSG_DONE in curl_check_connectivity(). */ - curl_easy_setopt (cb_data->curl_ehandle, CURLOPT_PRIVATE, NULL); - - g_simple_async_result_set_op_res_gssize (cb_data->simple, new_state); - g_simple_async_result_complete (cb_data->simple); - g_object_unref (cb_data->simple); - curl_slist_free_all (cb_data->request_headers); - g_free (cb_data->response); - g_free (cb_data->msg); + NMConnectivityCheckCallback callback; + + nm_assert (cb_data); + nm_assert (NM_IS_CONNECTIVITY (cb_data->self)); + + callback = cb_data->callback; + if (!callback) + return; + + cb_data->callback = NULL; + + nm_assert (log_message); + + _LOG2D ("check completed: %s; %s", + nm_connectivity_state_to_string (state), + log_message); + + callback (cb_data->self, + cb_data, + state, + error, + cb_data->user_data); +} + +static void +cb_data_free (NMConnectivityCheckHandle *cb_data, + NMConnectivityState state, + GError *error, + const char *log_message) +{ + NMConnectivity *self; + + nm_assert (cb_data); + + self = cb_data->self; + + nm_assert (NM_IS_CONNECTIVITY (self)); + + c_list_unlink (&cb_data->handles_lst); + +#if WITH_CONCHECK + if (cb_data->concheck.curl_ehandle) { + NMConnectivityPrivate *priv; + + /* Contrary to what cURL manual claim it is *not* safe to remove + * the easy handle "at any moment"; specifically not from the + * write function. Thus here we just dissociate the cb_data from + * the easy handle and the easy handle will be cleaned up when the + * message goes to CURLMSG_DONE in curl_check_connectivity(). */ + curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_WRITEFUNCTION, NULL); + curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_WRITEDATA, NULL); + curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_HEADERFUNCTION, NULL); + curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_HEADERDATA, NULL); + curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_PRIVATE, NULL); + curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_HTTPHEADER, NULL); + + priv = NM_CONNECTIVITY_GET_PRIVATE (self); + + curl_multi_remove_handle (priv->concheck.curl_mhandle, cb_data->concheck.curl_ehandle); + curl_easy_cleanup (cb_data->concheck.curl_ehandle); + + curl_slist_free_all (cb_data->concheck.request_headers); + } +#endif + + nm_clear_g_source (&cb_data->timeout_id); + + cb_data_invoke_callback (cb_data, state, error, log_message); + +#if WITH_CONCHECK + g_free (cb_data->concheck.response); + if (cb_data->concheck.recv_msg) + g_string_free (cb_data->concheck.recv_msg, TRUE); +#endif g_free (cb_data->ifspec); - g_source_remove (cb_data->timeout_id); g_slice_free (NMConnectivityCheckHandle, cb_data); } +/*****************************************************************************/ + +#if WITH_CONCHECK +static const char * +_check_handle_get_response (NMConnectivityCheckHandle *cb_data) +{ + return cb_data->concheck.response ?: NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE; +} + static void -curl_check_connectivity (CURLM *mhandle, CURLMcode ret) +curl_check_connectivity (CURLM *mhandle, int sockfd, int ev_bitmask) { NMConnectivityCheckHandle *cb_data; CURLMsg *msg; CURLcode eret; - CURL *easy_handle; gint m_left; long response_code; + CURLMcode ret; + int running_handles; + ret = curl_multi_socket_action (mhandle, sockfd, ev_bitmask, &running_handles); if (ret != CURLM_OK) - _LOGW ("connectivity check failed"); + _LOGE ("connectivity check failed: %d", ret); while ((msg = curl_multi_info_read (mhandle, &m_left))) { + if (msg->msg != CURLMSG_DONE) continue; /* Here we have completed a session. Check easy session result. */ eret = curl_easy_getinfo (msg->easy_handle, CURLINFO_PRIVATE, (char **) &cb_data); if (eret != CURLE_OK) { - _LOG2E ("curl cannot extract cb_data for easy handle %p, skipping msg", msg->easy_handle); + _LOGE ("curl cannot extract cb_data for easy handle, skipping msg"); continue; } - if (cb_data) { - NMConnectivityState c; - - /* If cb_data is still there this message hasn't been - * taken care of. Do so now. */ - if (msg->data.result != CURLE_OK) { - _LOG2D ("check failed (%d)", msg->data.result); - c = NM_CONNECTIVITY_LIMITED; - } else if ( !cb_data->response[0] - && (curl_easy_getinfo (msg->easy_handle, CURLINFO_RESPONSE_CODE, &response_code) == CURLE_OK) - && response_code == 204) { - /* If we got a 204 response code (no content) and we actually - * requested no content, report full connectivity. */ - _LOG2D ("response with no content received, check successful"); - c = NM_CONNECTIVITY_FULL; - } else { - /* If we get here, it means that easy_write_cb() didn't read enough - * bytes to be able to do a match, or that we were asking for no content - * (204 response code) and we actually got some. Either way, that is - * an indication of a captive portal */ - _LOG2I ("response did not match expected response '%s'; assuming captive portal.", - cb_data->response); - c = NM_CONNECTIVITY_PORTAL; - } - - finish_cb_data (cb_data, c); + if (!cb_data->callback) { + /* callback was already invoked earlier. */ + cb_data_free (cb_data, NM_CONNECTIVITY_UNKNOWN, NULL, NULL); + } else if (msg->data.result != CURLE_OK) { + gs_free char *log_message = NULL; + + log_message = g_strdup_printf ("check failed with curl status %d", msg->data.result); + cb_data_free (cb_data, NM_CONNECTIVITY_LIMITED, NULL, + log_message); + } else if ( !((_check_handle_get_response (cb_data))[0]) + && (curl_easy_getinfo (msg->easy_handle, CURLINFO_RESPONSE_CODE, &response_code) == CURLE_OK) + && response_code == 204) { + /* If we got a 204 response code (no content) and we actually + * requested no content, report full connectivity. */ + cb_data_free (cb_data, NM_CONNECTIVITY_FULL, NULL, + "no content, as expected"); + } else { + /* If we get here, it means that easy_write_cb() didn't read enough + * bytes to be able to do a match, or that we were asking for no content + * (204 response code) and we actually got some. Either way, that is + * an indication of a captive portal */ + cb_data_free (cb_data, NM_CONNECTIVITY_PORTAL, NULL, + "unexpected short response"); } - - /* Do not use message data after calling curl_multi_remove_handle() */ - easy_handle = msg->easy_handle; - curl_multi_remove_handle (mhandle, easy_handle); - curl_easy_cleanup (easy_handle); } } static gboolean curl_timeout_cb (gpointer user_data) { - NMConnectivity *self = NM_CONNECTIVITY (user_data); + gs_unref_object NMConnectivity *self = g_object_ref (NM_CONNECTIVITY (user_data)); NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); - CURLMcode ret; - int pending_conn; - - priv->curl_timer = 0; - - ret = curl_multi_socket_action (priv->curl_mhandle, CURL_SOCKET_TIMEOUT, 0, &pending_conn); - _LOGT ("timeout elapsed - multi_socket_action (%d conn remaining)", pending_conn); - - curl_check_connectivity (priv->curl_mhandle, ret); + priv->concheck.curl_timer = 0; + curl_check_connectivity (priv->concheck.curl_mhandle, CURL_SOCKET_TIMEOUT, 0); return G_SOURCE_REMOVE; } @@ -219,21 +306,17 @@ multi_timer_cb (CURLM *multi, long timeout_ms, void *userdata) NMConnectivity *self = NM_CONNECTIVITY (userdata); NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); - nm_clear_g_source (&priv->curl_timer); + nm_clear_g_source (&priv->concheck.curl_timer); if (timeout_ms != -1) - priv->curl_timer = g_timeout_add (timeout_ms, curl_timeout_cb, self); - + priv->concheck.curl_timer = g_timeout_add (timeout_ms, curl_timeout_cb, self); return 0; } static gboolean -curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer data) +curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer user_data) { - NMConnectivity *self = NM_CONNECTIVITY (data); + gs_unref_object NMConnectivity *self = g_object_ref (NM_CONNECTIVITY (user_data)); NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); - CURLMcode ret; - int pending_conn = 0; - gboolean bret = TRUE; int fd = g_io_channel_unix_get_fd (ch); int action = 0; @@ -241,16 +324,11 @@ curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer data) action |= CURL_CSELECT_IN; if (condition & G_IO_OUT) action |= CURL_CSELECT_OUT; + if (condition & G_IO_ERR) + action |= CURL_CSELECT_ERR; - ret = curl_multi_socket_action (priv->curl_mhandle, fd, 0, &pending_conn); - - curl_check_connectivity (priv->curl_mhandle, ret); - - if (pending_conn == 0) { - nm_clear_g_source (&priv->curl_timer); - bret = FALSE; - } - return bret; + curl_check_connectivity (priv->concheck.curl_mhandle, fd, action); + return G_SOURCE_CONTINUE; } typedef struct { @@ -263,11 +341,12 @@ multi_socket_cb (CURL *e_handle, curl_socket_t s, int what, void *userdata, void { NMConnectivity *self = NM_CONNECTIVITY (userdata); NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); - CurlSockData *fdp = (CurlSockData *) socketp; + CurlSockData *fdp = socketp; GIOCondition condition = 0; if (what == CURL_POLL_REMOVE) { if (fdp) { + curl_multi_assign (priv->concheck.curl_mhandle, s, NULL); nm_clear_g_source (&fdp->ev); g_io_channel_unref (fdp->ch); g_slice_free (CurlSockData, fdp); @@ -276,6 +355,7 @@ multi_socket_cb (CURL *e_handle, curl_socket_t s, int what, void *userdata, void if (!fdp) { fdp = g_slice_new0 (CurlSockData); fdp->ch = g_io_channel_unix_new (s); + curl_multi_assign (priv->concheck.curl_mhandle, s, fdp); } else nm_clear_g_source (&fdp->ev); @@ -283,19 +363,16 @@ multi_socket_cb (CURL *e_handle, curl_socket_t s, int what, void *userdata, void condition = G_IO_IN; else if (what == CURL_POLL_OUT) condition = G_IO_OUT; - else if (condition == CURL_POLL_INOUT) + else if (what == CURL_POLL_INOUT) condition = G_IO_IN | G_IO_OUT; if (condition) fdp->ev = g_io_add_watch (fdp->ch, condition, curl_socketevent_cb, self); - curl_multi_assign (priv->curl_mhandle, s, fdp); } return CURLM_OK; } -#define HEADER_STATUS_ONLINE "X-NetworkManager-Status: online\r\n" - static size_t easy_header_cb (char *buffer, size_t size, size_t nitems, void *userdata) { @@ -304,8 +381,8 @@ easy_header_cb (char *buffer, size_t size, size_t nitems, void *userdata) if ( len >= sizeof (HEADER_STATUS_ONLINE) - 1 && !g_ascii_strncasecmp (buffer, HEADER_STATUS_ONLINE, sizeof (HEADER_STATUS_ONLINE) - 1)) { - _LOG2D ("status header found, check successful"); - finish_cb_data (cb_data, NM_CONNECTIVITY_FULL); + cb_data_invoke_callback (cb_data, NM_CONNECTIVITY_FULL, + NULL, "status header found"); return 0; } @@ -317,23 +394,24 @@ easy_write_cb (void *buffer, size_t size, size_t nmemb, void *userdata) { NMConnectivityCheckHandle *cb_data = userdata; size_t len = size * nmemb; + const char *response = _check_handle_get_response (cb_data);; + + if (!cb_data->concheck.recv_msg) + cb_data->concheck.recv_msg = g_string_sized_new (len + 10); - cb_data->msg = g_realloc (cb_data->msg, cb_data->msg_size + len); - memcpy (cb_data->msg + cb_data->msg_size, buffer, len); - cb_data->msg_size += len; + g_string_append_len (cb_data->concheck.recv_msg, buffer, len); - /* Check matching prefix if a expected response is given */ - if ( cb_data->response[0] - && cb_data->msg_size >= strlen (cb_data->response)) { + if ( response + && cb_data->concheck.recv_msg->len >= strlen (response)) { /* We already have enough data -- check response */ - if (g_str_has_prefix (cb_data->msg, cb_data->response)) { - _LOG2D ("check successful."); - finish_cb_data (cb_data, NM_CONNECTIVITY_FULL); + if (g_str_has_prefix (cb_data->concheck.recv_msg->str, response)) { + cb_data_invoke_callback (cb_data, NM_CONNECTIVITY_FULL, NULL, + "expected response"); } else { - _LOG2I ("response did not match expected response '%s'; assuming captive portal.", - cb_data->response); - finish_cb_data (cb_data, NM_CONNECTIVITY_PORTAL); + cb_data_invoke_callback (cb_data, NM_CONNECTIVITY_PORTAL, NULL, + "unexpected response"); } + return 0; } @@ -341,90 +419,123 @@ easy_write_cb (void *buffer, size_t size, size_t nmemb, void *userdata) } static gboolean -timeout_cb (gpointer user_data) +_timeout_cb (gpointer user_data) { NMConnectivityCheckHandle *cb_data = user_data; - NMConnectivity *self = NM_CONNECTIVITY (g_async_result_get_source_object (G_ASYNC_RESULT (cb_data->simple))); - NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); - CURL *ehandle = cb_data->curl_ehandle; + NMConnectivity *self; - _LOG2I ("timed out"); - finish_cb_data (cb_data, NM_CONNECTIVITY_LIMITED); - curl_multi_remove_handle (priv->curl_mhandle, ehandle); - curl_easy_cleanup (ehandle); + nm_assert (NM_IS_CONNECTIVITY (cb_data->self)); + self = cb_data->self; + + nm_assert (c_list_contains (&NM_CONNECTIVITY_GET_PRIVATE (self)->handles_lst_head, &cb_data->handles_lst)); + + cb_data_free (cb_data, NM_CONNECTIVITY_LIMITED, NULL, "timeout"); return G_SOURCE_REMOVE; } +#endif -void -nm_connectivity_check_async (NMConnectivity *self, - const char *iface, - GAsyncReadyCallback callback, - gpointer user_data) +static gboolean +_idle_cb (gpointer user_data) { - NMConnectivityPrivate *priv; - GSimpleAsyncResult *simple; - CURL *ehandle = NULL; + NMConnectivityCheckHandle *cb_data = user_data; - g_return_if_fail (NM_IS_CONNECTIVITY (self)); - priv = NM_CONNECTIVITY_GET_PRIVATE (self); + nm_assert (NM_IS_CONNECTIVITY (cb_data->self)); + nm_assert (c_list_contains (&NM_CONNECTIVITY_GET_PRIVATE (cb_data->self)->handles_lst_head, &cb_data->handles_lst)); - simple = g_simple_async_result_new (G_OBJECT (self), callback, user_data, - nm_connectivity_check_async); + cb_data->timeout_id = 0; + if (!cb_data->ifspec) { + gs_free_error GError *error = NULL; - if (priv->enabled) - ehandle = curl_easy_init (); + /* the invocation was with an invalid ifname. It is a fail. */ + g_set_error (&error, NM_UTILS_ERROR, NM_UTILS_ERROR_INVALID_ARGUMENT, + "no interface specified for connectivity check"); + cb_data_free (cb_data, NM_CONNECTIVITY_ERROR, NULL, "missing interface"); + } else + cb_data_free (cb_data, NM_CONNECTIVITY_FAKE, NULL, "fake result"); + return G_SOURCE_REMOVE; +} + +NMConnectivityCheckHandle * +nm_connectivity_check_start (NMConnectivity *self, + const char *iface, + NMConnectivityCheckCallback callback, + gpointer user_data) +{ + NMConnectivityPrivate *priv; + NMConnectivityCheckHandle *cb_data; - if (ehandle) { - NMConnectivityCheckHandle *cb_data = g_slice_new0 (NMConnectivityCheckHandle); + g_return_val_if_fail (NM_IS_CONNECTIVITY (self), NULL); + g_return_val_if_fail (!iface || iface[0], NULL); + g_return_val_if_fail (callback, NULL); + + priv = NM_CONNECTIVITY_GET_PRIVATE (self); - cb_data->curl_ehandle = ehandle; - cb_data->request_headers = curl_slist_append (NULL, "Connection: close"); + cb_data = g_slice_new0 (NMConnectivityCheckHandle); + cb_data->self = self; + c_list_link_tail (&priv->handles_lst_head, &cb_data->handles_lst); + cb_data->callback = callback; + cb_data->user_data = user_data; + + if (iface) cb_data->ifspec = g_strdup_printf ("if!%s", iface); - cb_data->simple = simple; - if (priv->response) - cb_data->response = g_strdup (priv->response); - else - cb_data->response = g_strdup (NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE); - - curl_easy_setopt (ehandle, CURLOPT_URL, priv->uri); - curl_easy_setopt (ehandle, CURLOPT_WRITEFUNCTION, easy_write_cb); - curl_easy_setopt (ehandle, CURLOPT_WRITEDATA, cb_data); - curl_easy_setopt (ehandle, CURLOPT_HEADERFUNCTION, easy_header_cb); - curl_easy_setopt (ehandle, CURLOPT_HEADERDATA, cb_data); - curl_easy_setopt (ehandle, CURLOPT_PRIVATE, cb_data); - curl_easy_setopt (ehandle, CURLOPT_HTTPHEADER, cb_data->request_headers); - curl_easy_setopt (ehandle, CURLOPT_INTERFACE, cb_data->ifspec); - curl_multi_add_handle (priv->curl_mhandle, ehandle); - - cb_data->timeout_id = g_timeout_add_seconds (30, timeout_cb, cb_data); - - _LOG2D ("sending request to '%s'", priv->uri); - return; - } else { - _LOGD ("(%s) faking request. Connectivity check disabled", iface); + +#if WITH_CONCHECK + if (iface) { + CURL *ehandle; + + if ( priv->enabled + && (ehandle = curl_easy_init ())) { + + cb_data->concheck.response = g_strdup (priv->response); + cb_data->concheck.curl_ehandle = ehandle; + cb_data->concheck.request_headers = curl_slist_append (NULL, "Connection: close"); + curl_easy_setopt (ehandle, CURLOPT_URL, priv->uri); + curl_easy_setopt (ehandle, CURLOPT_WRITEFUNCTION, easy_write_cb); + curl_easy_setopt (ehandle, CURLOPT_WRITEDATA, cb_data); + curl_easy_setopt (ehandle, CURLOPT_HEADERFUNCTION, easy_header_cb); + curl_easy_setopt (ehandle, CURLOPT_HEADERDATA, cb_data); + curl_easy_setopt (ehandle, CURLOPT_PRIVATE, cb_data); + curl_easy_setopt (ehandle, CURLOPT_HTTPHEADER, cb_data->concheck.request_headers); + curl_easy_setopt (ehandle, CURLOPT_INTERFACE, cb_data->ifspec); + curl_multi_add_handle (priv->concheck.curl_mhandle, ehandle); + + cb_data->timeout_id = g_timeout_add_seconds (30, _timeout_cb, cb_data); + + _LOG2D ("start request to '%s'", priv->uri); + return cb_data; + } } +#endif - g_simple_async_result_set_op_res_gssize (simple, NM_CONNECTIVITY_UNKNOWN); - g_simple_async_result_complete_in_idle (simple); - g_object_unref (simple); + _LOG2D ("start fake request"); + cb_data->timeout_id = g_idle_add (_idle_cb, cb_data); + return cb_data; } -NMConnectivityState -nm_connectivity_check_finish (NMConnectivity *self, - GAsyncResult *result, - GError **error) +void +nm_connectivity_check_cancel (NMConnectivityCheckHandle *cb_data) { - GSimpleAsyncResult *simple; + NMConnectivity *self; + gs_free_error GError *error = NULL; - g_return_val_if_fail (g_simple_async_result_is_valid (result, G_OBJECT (self), nm_connectivity_check_async), NM_CONNECTIVITY_UNKNOWN); + g_return_if_fail (cb_data); - simple = G_SIMPLE_ASYNC_RESULT (result); - if (g_simple_async_result_propagate_error (simple, error)) - return NM_CONNECTIVITY_UNKNOWN; - return (NMConnectivityState) g_simple_async_result_get_op_res_gssize (simple); + self = cb_data->self; + + g_return_if_fail (NM_IS_CONNECTIVITY (self)); + g_return_if_fail (!c_list_is_empty (&cb_data->handles_lst)); + g_return_if_fail (cb_data->callback); + + nm_assert (c_list_contains (&NM_CONNECTIVITY_GET_PRIVATE (self)->handles_lst_head, &cb_data->handles_lst)); + + nm_utils_error_set_cancelled (&error, FALSE, "NMConnectivity"); + + cb_data_free (cb_data, NM_CONNECTIVITY_ERROR, error, "cancelled"); } +/*****************************************************************************/ + gboolean nm_connectivity_check_enabled (NMConnectivity *self) { @@ -435,12 +546,14 @@ nm_connectivity_check_enabled (NMConnectivity *self) /*****************************************************************************/ +#if WITH_CONCHECK static gboolean periodic_check (gpointer user_data) { g_signal_emit (NM_CONNECTIVITY (user_data), signals[PERIODIC_CHECK], 0); return G_SOURCE_CONTINUE; } +#endif static void update_config (NMConnectivity *self, NMConfigData *config_data) @@ -484,13 +597,16 @@ update_config (NMConnectivity *self, NMConfigData *config_data) changed = TRUE; } - /* Set enabled flag. */ - enabled = nm_config_data_get_connectivity_enabled (config_data); + enabled = FALSE; +#if WITH_CONCHECK /* connectivity checking also requires a valid URI, interval and * curl_mhandle */ - if (!(priv->uri && priv->interval && priv->curl_mhandle)) { - enabled = FALSE; - } + if ( priv->uri + && priv->interval + && priv->concheck.curl_mhandle) + enabled = nm_config_data_get_connectivity_enabled (config_data); +#endif + if (priv->enabled != enabled) { priv->enabled = enabled; changed = TRUE; @@ -498,7 +614,7 @@ update_config (NMConnectivity *self, NMConfigData *config_data) /* Set the response. */ response = nm_config_data_get_connectivity_response (config_data); - if (g_strcmp0 (response, priv->response) != 0) { + if (!nm_streq0 (response, priv->response)) { /* a response %NULL means, NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE. Any other response * (including "") is accepted. */ g_free (priv->response); @@ -506,11 +622,13 @@ update_config (NMConnectivity *self, NMConfigData *config_data) changed = TRUE; } +#if WITH_CONCHECK if (changed) { - nm_clear_g_source (&priv->periodic_check_id); + nm_clear_g_source (&priv->concheck.periodic_check_id); if (nm_connectivity_check_enabled (self)) - priv->periodic_check_id = g_timeout_add_seconds (priv->interval, periodic_check, self); + priv->concheck.periodic_check_id = g_timeout_add_seconds (priv->interval, periodic_check, self); } +#endif } static void @@ -523,34 +641,37 @@ config_changed_cb (NMConfig *config, update_config (self, config_data); } +/*****************************************************************************/ + static void nm_connectivity_init (NMConnectivity *self) { NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); - CURLcode retv; - retv = curl_global_init (CURL_GLOBAL_ALL); - if (retv == CURLE_OK) - priv->curl_mhandle = curl_multi_init (); - - if (!priv->curl_mhandle) - _LOGE ("unable to init cURL, connectivity check will not work"); - else { - curl_multi_setopt (priv->curl_mhandle, CURLMOPT_SOCKETFUNCTION, multi_socket_cb); - curl_multi_setopt (priv->curl_mhandle, CURLMOPT_SOCKETDATA, self); - curl_multi_setopt (priv->curl_mhandle, CURLMOPT_TIMERFUNCTION, multi_timer_cb); - curl_multi_setopt (priv->curl_mhandle, CURLMOPT_TIMERDATA, self); - curl_multi_setopt (priv->curl_mhandle, CURLOPT_VERBOSE, 1); - } + c_list_init (&priv->handles_lst_head); priv->config = g_object_ref (nm_config_get ()); - - update_config (self, nm_config_get_data (priv->config)); g_signal_connect (G_OBJECT (priv->config), NM_CONFIG_SIGNAL_CONFIG_CHANGED, G_CALLBACK (config_changed_cb), self); +#if WITH_CONCHECK + if (curl_global_init (CURL_GLOBAL_ALL) == CURLE_OK) + priv->concheck.curl_mhandle = curl_multi_init (); + + if (!priv->concheck.curl_mhandle) + _LOGE ("unable to init cURL, connectivity check will not work"); + else { + curl_multi_setopt (priv->concheck.curl_mhandle, CURLMOPT_SOCKETFUNCTION, multi_socket_cb); + curl_multi_setopt (priv->concheck.curl_mhandle, CURLMOPT_SOCKETDATA, self); + curl_multi_setopt (priv->concheck.curl_mhandle, CURLMOPT_TIMERFUNCTION, multi_timer_cb); + curl_multi_setopt (priv->concheck.curl_mhandle, CURLMOPT_TIMERDATA, self); + curl_multi_setopt (priv->concheck.curl_mhandle, CURLOPT_VERBOSE, 1); + } +#endif + + update_config (self, nm_config_get_data (priv->config)); } static void @@ -558,19 +679,34 @@ dispose (GObject *object) { NMConnectivity *self = NM_CONNECTIVITY (object); NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); + NMConnectivityCheckHandle *cb_data; + GError *error = NULL; + +again: + c_list_for_each_entry (cb_data, &priv->handles_lst_head, handles_lst) { + if (!error) + nm_utils_error_set_cancelled (&error, TRUE, "NMConnectivity"); + cb_data_free (cb_data, NM_CONNECTIVITY_ERROR, error, "shutting down"); + goto again; + } + g_clear_error (&error); g_clear_pointer (&priv->uri, g_free); g_clear_pointer (&priv->response, g_free); +#if WITH_CONCHECK + nm_clear_g_source (&priv->concheck.curl_timer); + + curl_multi_cleanup (priv->concheck.curl_mhandle); + curl_global_cleanup (); + nm_clear_g_source (&priv->concheck.periodic_check_id); +#endif + if (priv->config) { g_signal_handlers_disconnect_by_func (priv->config, config_changed_cb, self); g_clear_object (&priv->config); } - curl_multi_cleanup (priv->curl_mhandle); - curl_global_cleanup (); - nm_clear_g_source (&priv->periodic_check_id); - G_OBJECT_CLASS (nm_connectivity_parent_class)->dispose (object); } @@ -588,5 +724,3 @@ nm_connectivity_class_init (NMConnectivityClass *klass) object_class->dispose = dispose; } - -#endif /* WITH_CONCHECK */ diff --git a/src/nm-connectivity.h b/src/nm-connectivity.h index d9a9f2338b..6c16982307 100644 --- a/src/nm-connectivity.h +++ b/src/nm-connectivity.h @@ -24,6 +24,9 @@ #include "nm-dbus-interface.h" +#define NM_CONNECTIVITY_ERROR ((NMConnectivityState) -1) +#define NM_CONNECTIVITY_FAKE ((NMConnectivityState) -2) + #define NM_TYPE_CONNECTIVITY (nm_connectivity_get_type ()) #define NM_CONNECTIVITY(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_CONNECTIVITY, NMConnectivity)) #define NM_CONNECTIVITY_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_CONNECTIVITY, NMConnectivityClass)) @@ -41,13 +44,21 @@ NMConnectivity *nm_connectivity_get (void); const char *nm_connectivity_state_to_string (NMConnectivityState state); -void nm_connectivity_check_async (NMConnectivity *self, - const char *iface, - GAsyncReadyCallback callback, - gpointer user_data); -NMConnectivityState nm_connectivity_check_finish (NMConnectivity *self, - GAsyncResult *result, - GError **error); gboolean nm_connectivity_check_enabled (NMConnectivity *self); +typedef struct _NMConnectivityCheckHandle NMConnectivityCheckHandle; + +typedef void (*NMConnectivityCheckCallback) (NMConnectivity *self, + NMConnectivityCheckHandle *handle, + NMConnectivityState state, + GError *error, + gpointer user_data); + +NMConnectivityCheckHandle *nm_connectivity_check_start (NMConnectivity *self, + const char *iface, + NMConnectivityCheckCallback callback, + gpointer user_data); + +void nm_connectivity_check_cancel (NMConnectivityCheckHandle *handle); + #endif /* __NETWORKMANAGER_CONNECTIVITY_H__ */ diff --git a/src/nm-manager.c b/src/nm-manager.c index 4bc5c5dd9a..e98b9fe03f 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2450,7 +2450,6 @@ device_realized (NMDevice *device, _emit_device_added_removed (self, device, nm_device_is_real (device)); } -#if WITH_CONCHECK static void device_connectivity_changed (NMDevice *device, GParamSpec *pspec, @@ -2478,7 +2477,6 @@ device_connectivity_changed (NMDevice *device, nm_dispatcher_call_connectivity (priv->connectivity_state, NULL, NULL, NULL); } } -#endif static void _device_realize_finish (NMManager *self, @@ -2588,11 +2586,9 @@ add_device (NMManager *self, NMDevice *device, GError **error) G_CALLBACK (device_realized), self); -#if WITH_CONCHECK g_signal_connect (device, "notify::" NM_DEVICE_CONNECTIVITY, G_CALLBACK (device_connectivity_changed), self); -#endif if (priv->startup) { g_signal_connect (device, "notify::" NM_DEVICE_HAS_PENDING_ACTION, @@ -5441,34 +5437,54 @@ impl_manager_get_logging (NMDBusObject *obj, } typedef struct { - guint remaining; + NMManager *self; GDBusMethodInvocation *context; - NMConnectivityState state; + guint remaining; } ConnectivityCheckData; static void -device_connectivity_done (NMDevice *device, NMConnectivityState state, gpointer user_data) +device_connectivity_done (NMDevice *device, + NMDeviceConnectivityHandle *handle, + NMConnectivityState state, + GError *error, + gpointer user_data) { ConnectivityCheckData *data = user_data; + NMManager *self; + NMManagerPrivate *priv; - data->remaining--; + nm_assert (data); + nm_assert (data->remaining > 0); + nm_assert (NM_IS_MANAGER (data->self)); - /* We check if the state is already FULL so that we can provide the - * response without waiting for slower devices that are not going to - * affect the overall state anyway. */ + data->remaining--; - if (data->state != NM_CONNECTIVITY_FULL) { - if (state > data->state) - data->state = state; + self = data->self; + priv = NM_MANAGER_GET_PRIVATE (self); - if (data->state == NM_CONNECTIVITY_FULL || !data->remaining) { - g_dbus_method_invocation_return_value (data->context, - g_variant_new ("(u)", data->state)); - } + if ( data->context + && ( data->remaining == 0 + || ( state == NM_CONNECTIVITY_FULL + && priv->connectivity_state == NM_CONNECTIVITY_FULL))) { + /* despite having a @handle and @state returned by the requests, we always + * return the current connectivity_state. That is, because the connectivity_state + * and the answer to the connectivity check shall agree. + * + * However, if one of the requests (early) returns full connectivity and agrees with + * the accumulated connectivity state, we no longer have to wait. The result is set. + * + * This also works well, because NMDevice first emits change signals to it's own + * connectivity state, which is then taken into account for the accumulated global + * state. All this happens, before the callback is invoked. */ + g_dbus_method_invocation_return_value (g_steal_pointer (&data->context), + g_variant_new ("(u)", + (guint) priv->connectivity_state)); } - if (!data->remaining) + if (data->remaining == 0) { + g_object_unref (self); g_slice_free (ConnectivityCheckData, data); + } } static void @@ -5482,6 +5498,7 @@ check_connectivity_auth_done_cb (NMAuthChain *chain, GError *error = NULL; NMAuthCallResult result; ConnectivityCheckData *data; + NMDevice *device; priv->auth_chains = g_slist_remove (priv->auth_chains, chain); @@ -5497,23 +5514,37 @@ check_connectivity_auth_done_cb (NMAuthChain *chain, error = g_error_new_literal (NM_MANAGER_ERROR, NM_MANAGER_ERROR_PERMISSION_DENIED, "Not authorized to recheck connectivity"); - } else { - NMDevice *device; + } - /* it's allowed */ - data = g_slice_new0 (ConnectivityCheckData); - data->context = context; + if (error) { + g_dbus_method_invocation_take_error (context, error); + goto out; + } - c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { - data->remaining++; - nm_device_check_connectivity (device, - device_connectivity_done, - data); - } + data = g_slice_new (ConnectivityCheckData); + data->self = g_object_ref (self); + data->context = context; + data->remaining = 0; + + c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { + data->remaining++; + nm_device_check_connectivity (device, + device_connectivity_done, + data); } - if (error) - g_dbus_method_invocation_take_error (context, error); + if (data->remaining == 0) { + /* call the handler at least once. */ + data->remaining = 1; + device_connectivity_done (NULL, + NULL, + NM_CONNECTIVITY_UNKNOWN, + NULL, + data); + /* @data got destroyed. */ + } + +out: nm_auth_chain_unref (chain); } @@ -6559,7 +6590,6 @@ get_property (GObject *object, guint prop_id, const char *path; NMActiveConnection *ac; GPtrArray *ptrarr; - gboolean vbool; switch (prop_id) { case PROP_VERSION: @@ -6616,12 +6646,8 @@ get_property (GObject *object, guint prop_id, g_value_set_boolean (value, nm_config_data_get_connectivity_uri (config_data) != NULL); break; case PROP_CONNECTIVITY_CHECK_ENABLED: -#if WITH_CONCHECK - vbool = nm_connectivity_check_enabled (nm_connectivity_get ()); -#else - vbool = FALSE; -#endif - g_value_set_boolean (value, vbool); + g_value_set_boolean (value, + nm_connectivity_check_enabled (nm_connectivity_get ())); break; case PROP_PRIMARY_CONNECTION: nm_dbus_utils_g_value_set_object_path (value, priv->primary_connection); diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index a6b886ce43..5021575667 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -318,10 +318,10 @@ test_config_global_dns (void) g_object_unref (config); } -#if WITH_CONCHECK static void test_config_connectivity_check (void) { +#if WITH_CONCHECK const char *CONFIG_INTERN = BUILDDIR"/test-connectivity-check-intern.conf"; NMConfig *config; NMConnectivity *connectivity; @@ -351,8 +351,10 @@ test_config_connectivity_check (void) g_object_unref (config); g_assert (remove (CONFIG_INTERN) == 0); -} +#else + g_test_skip ("concheck disabled"); #endif +} static void test_config_no_auto_default (void) @@ -1048,9 +1050,7 @@ main (int argc, char **argv) g_test_add_func ("/config/set-values", test_config_set_values); g_test_add_func ("/config/global-dns", test_config_global_dns); -#if WITH_CONCHECK g_test_add_func ("/config/connectivity-check", test_config_connectivity_check); -#endif g_test_add_func ("/config/signal", test_config_signal); |