GNOME Bugzilla – Bug 694181
Handle GNetworkAddress better in g_network_monitor_base_can_reach()
Last modified: 2013-02-20 13:37:36 UTC
When given a GNetworkAddress, it would make sense to actually try resolving the hostname rather than just checking for a default route. This would help give rudimentary VPN-awareness to applications. Use case: In Evolution-Data-Server I'm giving each backend instance its own GNetworkAddress, created by default from the backend's configuration details. The base backend class automatically listens for "network-changed" signals from the default GNetworkMonitor, and runs g_network_montior_can_reach_async() on the backend's GNetworkAddress to determine the backend's "online" state. The hope is backends that require a VPN connection will automatically stay in "offline" mode until a VPN connection is made (since name resolution will fail), then automatically detect when the host is (potentially) reachable and switch to "online" mode. We're close. We are picking up "network-changed" signal emissions when connecting or disconnecting a VPN, but the can_reach() function is giving false positives. This bug looks to be the last missing piece I need for E-D-S.
Created attachment 236776 [details] [review] Proposed patch Not sure where in the "can_reach" logic this should go, so I just plopped it at the top. Also not sure whether DNS resolution is sufficient to declare the host reachable. The patch returns TRUE if DNS resolution works, but I can remove that if you'd rather the function keep going.
Comment on attachment 236776 [details] [review] Proposed patch > Not sure where in the "can_reach" logic this should go, so I just plopped it at > the top. Also not sure whether DNS resolution is sufficient to declare the > host reachable. So, I had been thinking originally that you don't want to do hostname resolution if you don't have network connectivity, because it might hang for a few seconds waiting for a response. But that doesn't really make sense; if you aren't on the network, you shouldn't have an upstream resolver set in resolv.conf either, so it should just immediately return an error. (For some reason the "host" command does not behave this way, but everything else does.) So doing the lookup first makes sense. However... there's no reason to limit this to GNetworkAddresses... it makes just as much sense for any other GSocketConnectable as well. And actually, we're already doing that a little further down in the function... (I'd forgotten the exact details of what this function did when we were talking earlier.) So really, I think all you need to do is move the "have_ipv4_default_route && have_ipv6_default_route" check to after the first g_socket_address_enumerator_next() call (with appropriate unrefs added to clean up). And then I was going to say "and you have to update can_reach_async too", but apparently GNetworkMonitorBase is inheriting the default can_reach_async(), which just runs can_reach(), which is wrong because it might block. Sigh. We need to fix that at some point too.
Created attachment 236786 [details] [review] Revised patch You're right, this is much cleaner. First attempt felt a little hackish.
(In reply to comment #2) > And then I was going to say "and you have to update can_reach_async too", but > apparently GNetworkMonitorBase is inheriting the default can_reach_async(), > which just runs can_reach(), which is wrong because it might block. Sigh. We > need to fix that at some point too. Don't know GTask yet, but the async function could just do the equivalent of g_simple_async_result_run_in_thread() and have the thread call the sync function, right?
(In reply to comment #3) > Created an attachment (id=236786) [details] [review] > Revised patch > > You're right, this is much cleaner. Actually this turns out not to solve the problem, because GNetworkAddress caches its socket addresses and only does DNS resolution if it has no cached socket addresses. So if I start E-D-S while already on the VPN, then disconnect the VPN, the can_reach() function is back to reporting false positives on my Red Hat calendar because it already has socket addresses cached from when I was on the VPN. First patch -- hackish though it may have been -- worked.
Comment on attachment 236776 [details] [review] Proposed patch Removing obsolete flag on the first patch, since it worked.
(In reply to comment #5) > Actually this turns out not to solve the problem, because GNetworkAddress > caches its socket addresses and only does DNS resolution if it has no cached > socket addresses. Hm... that could cause other problems too; eg, sip.redhat.com resolves to a different address inside the VPN than it does outside, and neither address is reachable from the other side of the VPN. GNetworkAddress and GNetworkService probably need to connect to the resolver's "reload" signal, and uncache their answers in that case. Which requires a bit of thread-safety to be added too... (In reply to comment #4) > Don't know GTask yet, but the async function could just do the equivalent of > g_simple_async_result_run_in_thread() and have the thread call the sync > function, right? it could, but GSocketAddressEnumerator has async methods as well, so it would be better to just use those. (if this is getting bigger than you feel like dealing with, then you can just let me deal with it, and I will make sure to get to it before 2.36 goes out...)
(In reply to comment #7) > (if this is getting bigger than you feel like dealing with, then you can just > let me deal with it, and I will make sure to get to it before 2.36 goes out...) I'll take you up on that. I think you understand my limited use case for E-D-S, so if at least that much could be working by 2.36 I'd really appreciate it.
Created attachment 236837 [details] [review] GNetworkAddress: drop cached addresses on resolver reload If the resolver reloads (ie, if /etc/resolv.conf changes), GNetworkAddress needs to re-resolve its addresses the next time it's enumerated. Otherwise hosts that have different IP addresses inside and outside a VPN won't work correctly if you hold on to a GNetworkAddress for them for a long time.
Created attachment 236838 [details] [review] g_network_monitor_base_can_reach: Check for default route after enumerating. Enumerate the GSocketConnectable before checking for a default route. For some connectable types this will involve a DNS lookup. This will elminate false positives for hosts behind a VPN since DNS lookup will fail if the VPN is not connected.
Created attachment 236839 [details] [review] GNetworkMonitorBase: implement can_reach_async Implement the g_network_monitor_can_reach_async() rather than falling back to the default implementation, which calls the sync version (not in a thread).
(the middle patch is your first patch, unchanged. I just reattached it to keep them in the right order)
Review of attachment 236837 [details] [review]: In g_network_address_address_enumerator_next_async(), I think you need to check the resolver serial outside of the "if (!addr->priv->sockaddrs) { ... }" block. Otherwise you're only dropping cached addresses if you have no cached addresses. Also, I think g_resolver_get_serial() needs to call g_resolver_maybe_reload(). Otherwise it returns a potentially stale serial number, which sort of defeats the purpose of the function.
Created attachment 236867 [details] [review] Suggested changes Patch for my suggested changes in comment #13. With your patches + these changes, E-D-S is working as expected. Testing my calendars, only the Red Hat calendar requires a VPN... 1. Start E-D-S with VPN connected -> all calendars ONLINE 2. Disconnect VPN -> Red Hat calendar OFFLINE, all others ONLINE 3. Disable networking -> all calendars OFFLINE 4. Enable networking -> Red Hat calendar remains OFFLINE, all others ONLINE 5. Connect to VPN -> Red Hat calendar ONLINE
Created attachment 236869 [details] [review] Suggested changes Patch for my suggested changes in comment #13. With your patches + these changes, E-D-S is working as expected. Testing my calendars, only the Red Hat calendar requires a VPN... 1. Start E-D-S with VPN connected -> all calendars ONLINE 2. Disconnect VPN -> Red Hat calendar OFFLINE, all others ONLINE 3. Disable networking -> all calendars OFFLINE 4. Enable networking -> Red Hat calendar remains OFFLINE, all others ONLINE 5. Connect to VPN -> Red Hat calendar ONLINE
(Sorry for the double post... Bugzilla being flakey.)
merged your fixes and pushed. Actually, I made one other change: I made g_resolver_get_serial() be gio-internal, since (a) it's past API freeze, and (b) I didn't really spend a lot of time thinking about it, and if we were going to have this sort of API, we might realize later that something slightly different would be better. Attachment 236837 [details] pushed as c6c1166 - GNetworkAddress: drop cached addresses on resolver reload Attachment 236838 [details] pushed as 4ca3d80 - g_network_monitor_base_can_reach: Check for default route after enumerating. Attachment 236839 [details] pushed as 9670d06 - GNetworkMonitorBase: implement can_reach_async
(In reply to comment #17) > Actually, I made one other change: I made g_resolver_get_serial() be > gio-internal, since (a) it's past API freeze, and (b) I didn't really > spend a lot of time thinking about it, and if we were going to have > this sort of API, we might realize later that something slightly > different would be better. Wise move. Thanks for handling this so swiftly!