Bug 56635

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.

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.

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.

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.

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.

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".

Created attachment 78657 [details] [review]
Add a fake GNetworkMonitor, using a subset of the  ConnMan D-Bus API

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.

(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.

*** Bug 62459 has been marked as a duplicate of this bug. ***

(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 on attachment 78655 [details] [review]
Add a regression test for NetworkManager's "not quite  offline" states

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

++

Comment on attachment 78656 [details] [review]
Split connectivity state into the concepts of "up" and  "stable"

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

++

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 on attachment 78658 [details] [review]
Replace ConnMan connectivity support with  GNetworkMonitor

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

I like this approach. ++

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.