After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 704841 - API for figuring out "primary" ActiveConnection
API for figuring out "primary" ActiveConnection
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: API
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks: 704670 706098
 
 
Reported: 2013-07-24 22:13 UTC by Dan Winship
Modified: 2013-08-28 15:03 UTC
See Also:
GNOME target: 3.10
GNOME version: ---



Description Dan Winship 2013-07-24 22:13:13 UTC
nm-applet has some complicated logic for figuring out what icon to show, and gnome-shell has some *different* complicated logic, which is now having to change to deal with the new ethernet-free UI. Also, in the case where you have two active devices, and a VPN with the default route, nm-applet just picks randomly between the two devices, rather than showing the icon for the device that the VPN is tunneling over (because there's no easy way to figure that out).

Anyway, the daemon figures all this out in nm-policy every time the routing changes, so we should just expose that information directly.

Something like:

  NMActiveConnection *nm_client_get_default_device_connection ()
  NMActiveConnection *nm_client_get_default6_device_connection ()
  NMVPNConnection    *nm_client_get_default_vpn_connection ()
  NMVPNConnection    *nm_client_get_default6_vpn_connection ()

When no VPN is active, or the VPN doesn't have the default route, get_default_device_connection() returns the NMActiveConnection that has default=TRUE. When a VPN connection has the default route, get_default_device_connection() returns the NMActiveConnection for the device the VPN is tunneling over.

Or maybe the device ones should return NMDevices instead of NMActiveConnections...
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-07-25 05:14:04 UTC
The "default" / "default6" might be confusing in the case where VPN exists. I don't think we were ever too certain, but apparently when VPN exists, the "default" / "default6" properties on the active connections are FALSE. So it might be awkward that get_default_device_connection().default == false.

I was assuming that this would assume multi-VPN in the future, so I was imagining a get_active_vpn_connections() that returned a list of objects. I don't really care whether these are for IPv4 or IPv6 or not. You be the judge of whether that's useful.

For tunnel connections like VPN, perhaps we should have a way of associating what a connection is tunnelling through, and what a connection is being tunnelled by? This would allow drawing the nm-applet icon to be:

    let activeConnection = client.get_default_device_connection();
    if (activeConnection) {
        drawIndicator(activeConnection);
        let vpn = activeConnection.get_tunneled_by();
        if (vpn)
            drawLockIcon();
    }

And the only time we need to schedule a redraw is on 'notify::tunneled-by', and 'notify::default-device-connection' (well, and if/when the signal quality or other properties change on the AC)

The other thing I wonder about is how to do non-success states. If the user failed to connect, we need to make sure we show an error state indicator in the top panel. I'd imagine that a default-routed connection only takes hold after it's transitioned to the ACTIVE state, so maybe returning NMDevices makes more sense, and then we can work out FAILED / DISCONNECTED devices instead of returning null; for the default connection?

I'd like to always have these properties return the "most interesting, most connected device". If we're on wired, and we're connecting to wireless, I don't care, but if we have nothing else, please return the wired device. If we've failed to connect to wired and that's all we have, return that.
Comment 2 Jasper St. Pierre (not reading bugmail) 2013-07-25 05:15:10 UTC
(ccing Giovanni, he might have a few words on this)
Comment 3 Giovanni Campagna 2013-07-25 07:49:00 UTC
(In reply to comment #1)
> The "default" / "default6" might be confusing in the case where VPN exists. I
> don't think we were ever too certain, but apparently when VPN exists, the
> "default" / "default6" properties on the active connections are FALSE. So it
> might be awkward that get_default_device_connection().default == false.

That's true only if the vpn is taking over the default route.

In any case, there is a lot of UI policy here: we want to show wifi + wwan + vpn + connecting + failed, not just the default route (which is what nm-policy is about). Indeed, the default route might be totally uninteresting if you have a wifi hotspot and vpn, while connected through ethernet. Therefore I think it would be wrong to have this code in NetworkManager.

It's possible to have the tracking in NMGtk, so you would feed a list of active connections and get the "most interesting" one, but again, it's not uncommon in the new designs to show more than one icon, or none at all, so it would not be that useful, IMHO.

Finally, I know that technically the default route for ipv4 and ipv6 can be on different interfaces, but I don't think we ever want to even build such a configuration automatically, let alone inform the user of this distinction, so the API, whatever it is, should abstract that.
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-07-25 17:06:30 UTC
(In reply to comment #3)
> That's true only if the vpn is taking over the default route.

I think Dan Winship said that was always the case on IRC.

> In any case, there is a lot of UI policy here: we want to show wifi + wwan +
> vpn + connecting + failed, not just the default route (which is what nm-policy
> is about). Indeed, the default route might be totally uninteresting if you have
> a wifi hotspot and vpn, while connected through ethernet. Therefore I think it
> would be wrong to have this code in NetworkManager.

The designs we have were leaning towards only showing the default icon in the top panel. We discussed this on IRC.

> Finally, I know that technically the default route for ipv4 and ipv6 can be on
> different interfaces, but I don't think we ever want to even build such a
> configuration automatically, let alone inform the user of this distinction, so
> the API, whatever it is, should abstract that.

I agree with this. Is it common to have different devices for default / default6? This just seems like a strange legacy quirk.
Comment 5 Pavel Simerda 2013-07-25 17:28:18 UTC
(In reply to comment #4)
> I agree with this. Is it common to have different devices for default /
> default6? This just seems like a strange legacy quirk.

Not at all. Now it's not very common, when IPv6 is only being used in networking labs. But if/when IPv6 gets out of the labs to everyday use, it will get more common. And it doesn't matter much whether it will be because of IPv6-only VPN (the Microsoft way), IPv4-only VPN (the Cisco way) or (more or less automated) tunneling.

While the user doesn't care about network configuration when it works for him, the distinction is crucial when something breaks. Also various tools need to react separately to the two IP protocols and once again, when it comes to troubleshooting that, this information is crucial.
Comment 6 Jasper St. Pierre (not reading bugmail) 2013-07-25 17:35:39 UTC
(In reply to comment #5)
> Not at all. Now it's not very common, when IPv6 is only being used in
> networking labs. But if/when IPv6 gets out of the labs to everyday use, it will
> get more common. And it doesn't matter much whether it will be because of
> IPv6-only VPN (the Microsoft way), IPv4-only VPN (the Cisco way) or (more or
> less automated) tunneling.

These properties would exclude VPNs from ever becoming the default-device-connection. Are there any use cases that don't involve VPN where you could see the default and default6 devices being different?

From a user perspective, I don't think it makes sense to have default be 3g, and default6 be Wi-Fi, or something like that.
Comment 7 Dan Winship 2013-07-25 17:40:01 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > That's true only if the vpn is taking over the default route.
> 
> I think Dan Winship said that was always the case on IRC.

No, Giovanni has it right. The VPN may or may not take over the default route, and the "default" property is always on the ActiveConnection with the default route, so that may or may not be the VPN.

(In reply to comment #6)
> These properties would exclude VPNs from ever becoming the
> default-device-connection. Are there any use cases that don't involve VPN where
> you could see the default and default6 devices being different?

It is possible for default and default6 to be on different devices. I was assuming you did not want to deal with that in the UI, and so I suggested only a single API. If you wanted to be able to draw multiple icons in the split default/default6 case, we'd need separate default_device_connection and default6_device_connection.
Comment 8 Dan Winship 2013-07-25 17:43:51 UTC
(In reply to comment #6)
> From a user perspective, I don't think it makes sense to have default be 3g,
> and default6 be Wi-Fi, or something like that.

This is definitely a weird area.

One place where this can happen now that's completely wrong is if you're using a VPN to keep your unencrypted traffic off the untrusted wifi network, but the wifi network supports IPv6 and the VPN doesn't. So then your IPv4 traffic would be encrypted, but your IPv6 traffic wouldn't be. In this particular case, it seems like the behavior the user wants would be to disable IPv6 on the wifi connection, but that's not necessarily right in other cases...
Comment 9 Pavel Simerda 2013-07-25 17:53:18 UTC
(In reply to comment #8)
> One place where this can happen now that's completely wrong is if you're using
> a VPN to keep your unencrypted traffic off the untrusted wifi network, but the
> wifi network supports IPv6 and the VPN doesn't. So then your IPv4 traffic would
> be encrypted, but your IPv6 traffic wouldn't be. In this particular case, it
> seems like the behavior the user wants would be to disable IPv6 on the wifi
> connection, but that's not necessarily right in other cases...

But then you should at least have the possibility of setting the IPv6 handling/method on the VPN connection to 'blocked' to explicitly block it. Not that the problem of not blocking IPv6 on IPv4-only equipment is not tied to NetworkManager and it's a real network security challenge.

It might be a good idea to *consider* making VPNs default to block IPv6 if setting IPv4 default route but to make IPv6 connectivity tunnels (nonexistent at the time of writing) default to only take over IPv6 default route and keep IPv4 default route working.
Comment 10 Dan Williams 2013-07-25 18:38:54 UTC
I'm not sure we can safely do a "tunneled-by" property since that might be complicated to actually figure out in the future.  But I think the API danw outlined is probably the simplest thing we can do for now.
Comment 11 Pavel Simerda 2013-07-25 19:13:52 UTC
(In reply to comment #10)
> But I think the API danw
> outlined is probably the simplest thing we can do for now.

+1, except I would use 'default4' and 'default6' to be more explicit.

The API will AFAIK be important for the work Tomáš Hozza is doing for DNSSEC support. It would be nice if he could query ActiveConnection plus its list of nameservers and search domains (1) by connection UUID and (2) by searching for the default ActiveConnection using nmcli.

But that brings another question and that is about the default NMConnection for DNS, as that one is ambiguous when default4 != default6. AFAIK the best way whould be to have 'default', 'default4' and 'default6', where default would be the connection chosen by NetworkManager for protocol independent stuff like DNS, 'default4' would be the connection that provides IPv4 default route and 'default6' would be the connection that provides IPv6 default route.

Then indicators and DNS tools could just use 'default', while tools for which IP protocol versions matter could use 'default4' and 'default6'.
Comment 12 Dan Winship 2013-08-22 17:52:17 UTC
I've pushed danw/defaultapi with a proposal.

I ended up adding only:

  NMActiveConnection *nm_client_get_physical_connection (NMClient *client);
  NMVPNConnection    *nm_client_get_vpn_connection (NMClient *client);

(and the corresponding GObject properties).

I renamed "device_connection" to "physical_connection", because most VPN types involve devices too, but what we're really talking about here is the physical hardware device (or at least, the device in your VM which is *pretending* to be a physical device).

I dropped the separate IPv4/IPv6 APIs because there's really nothing anyone is likely to be able to usefully do with separate IPv4 and IPv6 properties. (I'm thinking we should probably make it so that NM won't put the IPv4 and IPv6 defaults on separate devices unless you explicitly allow it, but that's another bug). Anyway, if someone really wants to show separate ip4 and ip6 physical connections, they can figure it out by hand by analyzing the routing.

The exact behavior of get_physical_connection is:

 * Determines the physical network device that the default route is
 * going over, and returns that device's #NMActiveConnection.
 *
 * In particular, if the default route is directly via a physical
 * device, this returns that device's connection. If the default route
 * is via a VPN, this returns the connection for the physical device
 * that contains the route to the VPN endpoint.
 *
 * (If the IPv4 and IPv6 default routes are not via the same device,
 * this returns the device with the IPv4 route, unless the IPv6
 * default route is via a physical device and the IPv4 default route
 * is not, in which case it returns the device with the IPv6 default
 * route.)
 *
 * If there is no default route, or the default route is over a
 * non-NetworkManager-recognized device, this will return %NULL.

(The IPv6 special case is for if you only have IPv6 connectivity via your local network, but have IPv4 connectivity via a VPN or tunnel (over IPv6). In that case the IPv6 connection needs to be considered primary.)

The properties exist on NMManager as well, both (a) so that they're available directly via D-Bus, and (b) because it's awkward to synthesize them client-side because it involves property changes on multiple objects, and who knows which order you'll get the notifications in.
Comment 13 Dan Williams 2013-08-26 18:06:39 UTC
Though (bikeshedding alert) "physical" means something very specific in networking/hardware, and software/virtual devices aren't physical.  I think we're overloading physical here, which is why "device" was better...  I don't care a lot though.
Comment 14 Dan Winship 2013-08-27 19:10:24 UTC
repushed the branch with :vpn-connection removed (because it turned out not to be useful) and with :activating-connection added. This is "the connection that is currently activating and that is expected to become the new :physical-connection when it completes". So now the shell's logic is something like: show an icon for :physical-connection (unless it's ethernet), unless :activating-connection is non-NULL (and not ethernet), in which case show that.

(In reply to comment #13)
> Though (bikeshedding alert) "physical" means something very specific in
> networking/hardware, and software/virtual devices aren't physical.

Right. I did actually mean "physical", but I wasn't considering the possibility that the default route might go through a vlan/bond/bridge... But I don't like "device". Especially now that we have NMDeviceGenerics for the VPN tunnel, and will eventually have those matched up with their NMVPNConnections.

Maybe "primary connection"?

anyway, ready for review
Comment 15 Dan Williams 2013-08-27 21:08:11 UTC
Yeah, primary sounds better.  Lets do that.

> core: add NMManager:physical-connection and :activating-connection

check_activating_devices() spells out the property names, but we've got #defines for them which are also used elsewhere.  Should switch to the #defines.

device_state_changed() has erroneous whitespace added after the "break;"

The rest looks good.
Comment 16 Dan Winship 2013-08-28 15:03:39 UTC
renamed to "primary connection" and pushed, with the other suggested changes, to master.