Bug 56635 - use GNetworkMonitor to monitor connectivity
Summary: use GNetworkMonitor to monitor connectivity
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: unspecified
Hardware: x86-64 (AMD64) Linux (All)
: medium enhancement
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard: review?
Keywords: patch
: 62459 (view as bug list)
Depends on:
Blocks: 68712
  Show dependency treegraph
 
Reported: 2012-11-01 10:54 UTC by Paul Gideon Dann
Modified: 2013-09-03 17:34 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Add a regression test for NetworkManager's "not quite offline" states (4.19 KB, patch)
2013-04-30 16:42 UTC, Simon McVittie
Details | Splinter Review
Split connectivity state into the concepts of "up" and "stable" (6.51 KB, patch)
2013-04-30 16:43 UTC, Simon McVittie
Details | Splinter Review
Add a fake GNetworkMonitor, using a subset of the ConnMan D-Bus API (10.86 KB, patch)
2013-04-30 16:44 UTC, Simon McVittie
Details | Splinter Review
Replace ConnMan connectivity support with GNetworkMonitor (15.73 KB, patch)
2013-04-30 16:45 UTC, Simon McVittie
Details | Splinter Review

Description Paul Gideon Dann 2012-11-01 10:54:48 UTC
I just got version 5.14.0 of telepathy-mission-control in Archlinux to discover that it's pulling in NetworkManager as a hard dependency.  Since I use wicd, it seems a bit silly for NetworkManager to be pulled in by telepathy.  Could this be dynamically loaded, or changed into a plugin, or something else that would make the dependency optional?
Comment 1 Simon McVittie 2012-11-01 12:01:59 UTC
Enabling the NetworkManager integration adds a hard dependency on a couple of (relatively small) libraries supplied by NetworkManager: if Arch's infrastructure turns that into a dependency on the NM daemon itself, I'd consider that to be a problem with that infrastructure.

For instance, in Debian[1], on Linux architectures the telepathy-mission-control-5 binary package has a hard dependency on the libnm-glib4 binary package, but no dependency on the network-manager binary package. The libnm-glib4 and network-manager binary packages are both built by the network-manager source package (similar to "subpackages" in RPM jargon).

[1] <http://packages.debian.org/experimental/telepathy-mission-control-5>

Having said that, it would be nice if the NM and ConnMan integration could be done either as a plugin (with a sensible, stable API via libmissioncontrol-plugins, that doesn't expose MC internals like the old McdTransport API did), or by using GNetworkMonitor. Severity -> enhancement for that.
Comment 2 Paul Gideon Dann 2012-11-05 09:59:07 UTC
The plugin route seems like a winner to me.  Archlinux likes to keep things simple, and splitting up packages is not the done thing.  The issue has been rejected on the Archlinux bugtracker for this reason: https://bugs.archlinux.org/task/32355#history.
Comment 3 Will Thompson 2012-11-07 14:19:13 UTC
I think basically replacing McdConnectivityMonitor with GNetworkMonitor would be sensible: I think the latter provides everything that the former does, without depending on connman/nm/wicd/whatever.
Comment 4 Simon McVittie 2013-04-30 16:42:14 UTC
I have a patch-set that works for the regression tests (but possibly not in real life). I'm going to test it in a few real configurations (NM with netlink GNetworkMonitor, ConnMan with netlink GNetworkMonitor, ConnMan with <https://github.com/jukkar/connman-network-monitor>), but I think it's ready for review while I do that.

(In reply to comment #2)
> Archlinux likes to keep things
> simple, and splitting up packages is not the done thing.

I still think this is a packaging bug - distributions that separate library and daemon packages don't do it for fun, they do have valid reasons, and this is one of them. However, with this patch-set, building with "--with-connectivity=no" will avoid the dependency on network-manager libraries, and the only feature that'll be lost on NM systems is that MC won't automatically disconnect everything (in the hope that the network is still briefly usable) when NM reports CONNECTING, DISCONNECTING or ASLEEP status.

If Telepathy reviewers' opinion is that special handling of the CONNECTING, DISCONNECTING and ASLEEP statuses is not worth bothering with at all, then we can cut out the HAVE_NM code path and just use GNetworkMonitor everywhere. In the longer term, Bug #41148 is probably a better approach to that anyway.
Comment 5 Simon McVittie 2013-04-30 16:42:58 UTC
Created attachment 78655 [details] [review]
Add a regression test for NetworkManager's "not quite  offline" states

We were only testing the intersection of ConnMan and NM functionality,
but NM actually has a bit more.
Comment 6 Simon McVittie 2013-04-30 16:43:35 UTC
Created attachment 78656 [details] [review]
Split connectivity state into the concepts of "up" and  "stable"

ConnMan and GNetworkMonitor only give us two states: either we have
some level of Internet connectivity, or we don't. NetworkManager adds
a third category of states (CONNECTING, DISCONNECTING, ASLEEP):
"you might have connectivity, but things are changing, so you might
want to disconnect now". I've represented this as "not stable".

The flag is deliberately "stable", not "unstable", so that all the
boolean flags have the same sense: if they are TRUE, we're
"more connected".
Comment 7 Simon McVittie 2013-04-30 16:44:22 UTC
Created attachment 78657 [details] [review]
Add a fake GNetworkMonitor, using a subset of the  ConnMan D-Bus API
Comment 8 Simon McVittie 2013-04-30 16:45:40 UTC
Created attachment 78658 [details] [review]
Replace ConnMan connectivity support with  GNetworkMonitor

The information we're getting from ConnMan is no more informative
than GNetworkMonitor, and a ConnMan GNetworkMonitor implementation
exists (e.g. <https://github.com/jukkar/connman-network-monitor>),
so we might as well go via that.

On NetworkManager systems, we now use NetworkManager to detect
transient states like DISCONNECTING, but use the GNetworkMonitor
to detect whether the interface is up. If NetworkManager is the only
thing managing an interface, that makes no difference, but if there is
an interface not managed by NetworkManager, the default netlink
GNetworkMonitor will say we're online even while NM is DISCONNECTED,
which is what we want.

On non-NetworkManager, non-ConnMan systems where netlink works,
netlink will tell us at least the basics of what's going on, and we
can avoid trying to connect while there is no Internet route.

On systems without a working GNetworkMonitor, GIO's default
implementation will say "yes, you are online" at all times, which is
no worse than we had previously.
Comment 9 Simon McVittie 2013-04-30 16:50:50 UTC
(In reply to comment #7)
> Add a fake GNetworkMonitor, using a subset of the  ConnMan D-Bus API

... or we could submodule in <https://github.com/jukkar/connman-network-monitor> if preferred, but that seems like more code than we really need.

This one matches the subset/version of ConnMan's D-Bus API that we were previously using and testing; we could also change the bus name, break compatibility with genuine ConnMan and slim it down to "whatever makes it least code" (e.g. using o.fd.DBus.Properties) if desired.
Comment 10 Simon McVittie 2013-05-03 11:03:45 UTC
*** Bug 62459 has been marked as a duplicate of this bug. ***
Comment 11 Simon McVittie 2013-08-23 18:21:07 UTC
(In reply to comment #4)
> I have a patch-set that works for the regression tests (but possibly not in
> real life). I'm going to test it in a few real configurations (NM with
> netlink GNetworkMonitor, ConnMan with netlink GNetworkMonitor, ConnMan with
> <https://github.com/jukkar/connman-network-monitor>)

It seems to work OK in those configurations. Reviewers?
Comment 12 Guillaume Desmottes 2013-08-30 08:42:21 UTC
Comment on attachment 78655 [details] [review]
Add a regression test for NetworkManager's "not quite  offline" states

Review of attachment 78655 [details] [review]:
-----------------------------------------------------------------

++
Comment 13 Guillaume Desmottes 2013-08-30 08:43:41 UTC
Comment on attachment 78656 [details] [review]
Split connectivity state into the concepts of "up" and  "stable"

Review of attachment 78656 [details] [review]:
-----------------------------------------------------------------

++
Comment 14 Guillaume Desmottes 2013-08-30 08:46:28 UTC
Comment on attachment 78657 [details] [review]
Add a fake GNetworkMonitor, using a subset of the  ConnMan D-Bus API

Review of attachment 78657 [details] [review]:
-----------------------------------------------------------------

++
Comment 15 Guillaume Desmottes 2013-08-30 08:51:41 UTC
Comment on attachment 78658 [details] [review]
Replace ConnMan connectivity support with  GNetworkMonitor

Review of attachment 78658 [details] [review]:
-----------------------------------------------------------------

I like this approach. ++
Comment 16 Simon McVittie 2013-09-03 17:34:23 UTC
Fixed in git for 5.15.1.

It's the Arch maintainer's decision whether they continue to compile against a NM library or not - if daemon+library packages in Arch aren't split, then I personally think that's an infrastructure bug, but not compiling against NM and relying on the Netlink GNetworkMonitor might be a good workaround for it.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.