Re: [PATCH 3/3] device: when IP config is unchanged, only install updated lifetimes

["Ward, David - 0665 - MITLL" <david ward ll mit edu>]

On 05/07/2015 11:20 AM, Thomas Haller wrote:
> On Wed, 2015-05-06 at 15:59 +0000, Ward, David - 0665 - MITLL wrote:
> > On 05/06/2015 03:30 AM, Thomas Haller wrote:
> > > On Tue, 2015-05-05 at 23:53 -0400, David Ward wrote:
> > > > After DHCP RENEW/REBIND events occur (in particular), do not update
> > > > the installed routes if there were no relevant IP config changes.
> > > >
> > > > Aside from being an optimization, this behavior is needed to avoid
> > > > unnecessarily interfering with standalone, commerical VPN clients
> > > > which follow remote security policy settings. These VPN clients may
> > > > force all traffic to go through the tunnel by installing additional
> > > > routes in the kernel for each interface, and they may monitor the
> > > > routing table and terminate the VPN session if any changes occur.
> > > > NetworkManager can coexist with this type of VPN client, as long as
> > > > no IP config changes happen on any devices during the VPN session.
> > >
> > > In nm_device_set_ip4_config(), @old_config and @new_config are both the
> > > representation of "what NM thinks should be configured".
> > >
> > > @has_changes says, that between before and now, whether "what should be"
> > > changed. This allows to safe some internal actions, such as sending
> > > D-Bus notifications about the configuration changes.
> > >
> > >
> > >
> > > That is distinct from what nm_ip4_config_commit() /
> > > nmnm_route_manager_ip6_route_sync() does:
> > > That one takes "what NM thinks should be", and applies it to "what
> > > actually is".
> > >
> > >
> > > The conclusion from @has_changes -> @do_sync_routes is not necessarily
> > > correct... Maybe it is, not sure here...
> > >
> > >
> > >
> > > Could you elaborate a bit more on the actual problem you have? It's
> > > about these VPN clients, right?
> > Thomas, thanks for your comments. Yes, the issue is with using VPN
> > clients targeted for corporate networks.
> >
> >
> > Here's an illustration of how one of these VPN clients works. There are
> > three interfaces on the system:
> >
> >    * wlp12s0, a Wi-Fi interface with an active connection using DHCP
> >    * vmnet1/vmnet8, local VMware interfaces not controlled by
> >      NetworkManager (kept for illustration)
> >
> >
> > This is the initial routing table before the VPN connection. The first
> > two routes are added by NetworkManager.
> >
> > default via 192.168.43.1 dev wlp12s0  proto static  metric 600
> > 192.168.43.0/24 dev wlp12s0  proto kernel  scope link  src
> > 192.168.43.17  metric 600
> > 192.168.251.0/24 dev vmnet1  proto kernel  scope link  src 192.168.251.1
> > 192.168.252.0/24 dev vmnet8  proto kernel  scope link  src 192.168.252.1
> >
> >
> > The VPN client makes a connection to VPN_SERVER_ADDRESS. This creates a
> > fourth interface, tun0, with an IP address of TUN0_ADDRESS. Then the
> > following things happen:
> >
> >    * An explicit route for the next hop to VPN_SERVER_ADDRESS is added
> >      with a metric of 1.
> >    * An explicit route for VPN_SERVER_ADDRESS is added with a metric of 1.
> >    * The routes without a metric get assigned a metric of 10.
> >    * For each existing route, an additional route to the same destination
> >      is added with a metric of 1, sending traffic to TUN0_ADDRESS.
> >
> >
> > So now the routing table looks like this:
> >
> > default via TUN0_ADDRESS dev tun0  scope link  metric 1
> > default via 192.168.43.1 dev wlp12s0  proto static  metric 600
> > VPN_SERVER_ADDRESS via 192.168.43.1 dev wlp12s0  metric 1
> > 192.168.43.0/24 via TUN0_ADDRESS dev tun0  scope link  metric 1
> > 192.168.43.0/24 dev wlp12s0  proto kernel  scope link  src
> > 192.168.43.17  metric 600
> > 192.168.43.1 dev wlp12s0  scope link  metric 1
> > 192.168.251.0/24 via TUN0_ADDRESS dev tun0  scope link  metric 1
> > 192.168.251.0/24 dev vmnet1  scope link  metric 10
> > 192.168.252.0/24 via TUN0_ADDRESS dev tun0  scope link  metric 1
> > 192.168.252.0/24 dev vmnet8  scope link  metric 10
> >
> >
> > The client then monitors the routing table. If any changes happen at
> > all, it assumes the worst and disconnects from the VPN server immediately.
> >
> > With NetworkManager 0.8 (i.e. RHEL 6), this worked without a problem,
> > because address lifetimes from DHCP were not installed in the kernel, so
> > nothing changed once the VPN connection was formed. With NetworkManager
> > 1.0 (i.e. RHEL 7), NetworkManager is now updating the address lifetimes
> > on the Wi-Fi interface when the DHCP lease is renewed. With that update,
> > it is syncing the routing table, and the VPN connection is dropped. My
> > goal with this patch is to cause NetworkManager to detect whether any
> > relevant changes happened in the lease, and if not, just leave
> > everything alone except for the "non-relevant" changes to the lease
> > (address lifetime and MTU).
> >
> > Unfortunately this change isn't even sufficient to address the problem
> > though, because simply updating the address lifetime causes the kernel
> > to reinstall an implicit route on the Wi-Fi interface without a metric,
> > which breaks the connection. (It wouldn't do this if the route didn't
> > have a metric.) That behavior is in the kernel though and not in
> > NetworkManager.
> Hi David,
>
>
> does this also happen on master? On master, NM should see the routes
> that were added externally, and take them into its list of routes.
>
> Later syncs should then leave those externally configured routes
> unmodified.
>
> See
> http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=557667df12fc05b
> (note there is a bug related to this commit
> https://bugzilla.gnome.org/show_bug.cgi?id=749052)

Hi Thomas,

Sorry for the very long delay. I was first able to prevent the kernel 
from reinstalling routes automatically whenever the address lifetime is 
updated, which was part of the problem. I just submitted this as a 
kernel patch: https://patchwork.ozlabs.org/patch/500078/

With this kernel patch installed, the VPN client actually works now with 
the master branch, as well as version 1.0.4 because of your final merge 
commit that fixes the route manager to not delete externally added routes.

Thanks for your help!

David