GNOME Bugzilla – Bug 780448
[review] lr/device-conn: Make connectivity checking per-device
Last modified: 2017-03-28 14:04:23 UTC
https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=lr/device-conn
+ static void _update_default_route (NMDevice *self, int addr_family, gboolean has, gboolean is_assumed) spurious newline + g_dbus_method_invocation_return_value (data->context, + g_variant_new ("(u)", data->state)); indention. + priv->concheck_periodic_id = g_signal_connect (nm_connectivity_get (), + NM_CONNECTIVITY_PERIODIC_CHECK, indention. I think the global connectivity check should not consider all devices. In device_connectivity_changed(), the connectivity is not what is the connectivity state of the best-device (the one with the default route). For the same reason, as the global metered state is not the best state of all devices, but the state of the best-device. On the other hand, check_connectivity_auth_done_cb() should trigger a recheck for all devices, but it should only return when all pending requests are complete (not the shortcut via "if (data->state != NM_CONNECTIVITY_FULL)". + g_signal_handler_disconnect (nm_connectivity_get (), priv->concheck_periodic_id); I would prefer to take a reference to singletons for which we subscribe. But arguably, NMDevice doesn't do that for NMConfig or NMPlatform either... which is fine, as long as *somebody* keeps the singleton alive at least as long as the NMDevice is alive. In short: have NMManager own a reference to NMConnectivity so that the lifetime is NMDevice < NMManager < NMConnectivity. Alternatively, NM_UTILS_KEEP_ALIVE (manager, nm_connectivity_get (), "NMManager-depends-on-NMConnectivity"); in main(). + const gchar *connectivity_state_string I think char,int,float,double is always preferred over gchar,gint,gfloat,gdouble.
Pushed an updated branch. Added a commit that adjusts metrics according to the connectivity check results.
> core: make connectivity checking per-device +#if WITH_CONCHECK +static void +concheck_cb (GObject *source_object, + GAsyncResult *result, + gpointer user_data) Indentation. +static void +concheck_periodic_update (NMDevice *self) + [...] + } else if (!check_enable && priv->concheck_periodic_id) { + /* The default route has gone off, and so has connectivity. */ + update_connectivity_state (self, NM_CONNECTIVITY_NONE); + g_signal_handler_disconnect (nm_connectivity_get (), priv->concheck_periodic_id); + priv->concheck_periodic_id = 0; + } update_connectivity_state() triggers a reconfiguration of IPv4/IPv6, which in turn calls again into concheck_periodic_update(). I think we should move the clearing of priv->concheck_periodic_id before update_connectivity_state() here, so that no action is taken in the nested execution of concheck_periodic_update(). The rest LGTM.
29af644f9ad92e7ee7b602875a3b1afbbd4f3e8e