On Sun, 2014-02-16 at 11:15 +0000, David Woodhouse wrote: > On Sun, 2014-02-16 at 12:48 +0800, Antonio Borneo wrote: > > Hi David, > > > > I'm reworking a patch I have received for vpnc > > http://lists.unix-ag.uni-kl.de/pipermail/vpnc-devel/2013-December/004037.html > > in a set of new patches that I'm going to apply soon. > > > > The key point is the split, in vpnc-script, of do_disconnect() > > by creating a new do_destroy(). > > I have isolated this part in the patch in attachment. > > > > For backward compatibility I kept the original behavior for > > reason=disconnect, but I consider this name as incorrect and > > obsolete so added synonym reason=disconnect_destroy. > > For me the most interesting case of backward compatibility has been > using an old script with newer VPN client. At least in the past with > various systems shipping vpnc-script from the vpnc distribution while > OpenConnect required various new features, that's the one I've had to > deal with more. > > So I'm wondering if a better approach would be to continue to use > 'disconnect', but add a new environment variable which indicates that > this is a new VPN client that supports a separate 'destroy' invocation. +1 from me; that's cleaner to deal with for NetworkManager-vpnc too, I think. Dan > So instead of the plethora of > 'disconnect/disconnect_only/disconnect_destroy' methods, we just have > 'disconnect' with a modifier in a separate variable. And 'destroy'. > > That way, all forms of compatibility should work. New clients invoking > an old script will call 'disconnect', which will do the destroy > operation too, and then call the unsupported 'destroy' method. > > Old clients invoking a new script will calls 'disconnect' but without > that extra 'disconnect=disconnect_only' environment variable (or > whatever we choose to call it), and thus the new script will do what > they expect. > > > Please notice that destroy_tun_device() has code for *BSD only. > > If we suppose that on these OS the packages are built with proper > > dependencies, we can be more aggressive with a cleaner patch > > that introduces only the reason destroy. > > We'd have to look again at that. In the past they've mostly been > shipping an old version from vpnc which has been problematic for > openconnect. I think FreeBSD at least did switch to a separate > vpnc-script package which both their vpnc and openconnect packages > depend on. And it seems to be up to date as of about two days ago. > Despite that change being Windows-only :) > > The OpenConnect configure script does check for existence of vpnc-script > in the default location. It could be augmented to check whether the > script supports the 'destroy' method, perhaps. > > But in practice I think it's easier and safer to just ensure > compatibility in both directions, as described above. > > > In this case reason destroy have to be called independently by > > openconnect and vpnc. > > Right. > > Did I ever give you access to the vpnc-scripts repository, btw? If you > mail me a SSH public key (and preferred username), I can. > > I have changes to vpnc-script-win.js in that repository, which probably > don't affect you yet since you don't support IPv6 yet. But there may be > more changes coming (it doesn't support full-tunnel mode in Legacy IP, > and should probably *remove* all IP addresses before setting up the > tunnel and in 'disconnect', etc.). > > (Btw, once vpnc is in git as its primary version control, it should be > simple enough just to do 'git pull' to pull in vpnc-script changes.) > > _______________________________________________ > vpnc-devel mailing list > vpnc-devel at unix-ag.uni-kl.de > https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel > http://www.unix-ag.uni-kl.de/~massar/vpnc/
