nm-l2tp / NetworkManager-l2tp

L2TP and L2TP/IPsec support for NetworkManager
GNU General Public License v2.0
488 stars 84 forks source link

Strange compile/runtime bug causing connection failure #44

Closed Ri0n closed 7 years ago

Ri0n commented 7 years ago

Hi guys. Thanks again for the great project and good support.

I've noticed a strange problem recently. My l2tp connection stopped working (does not connect, but at least tries) when I try to connect via NetworkManager on my Gentoo system. Also have a script which starts all the l2tp stuff and it connects fine.

So I compiled a fresh version from git master which is definitely newer then was before. But it fails to connect as well but now even worse (fails immediately after clicking connect)

Logs say it fails on creating RUNDIR (invalid argument). I also noticed I can redefine RUNDIR with runstatedir parameter of configure generated but not yet released version autoconf.

So I tried export runstatedir="${EPREFIX}/run/NetworkManager" in my Gentoo ebuild before build and it worked! My vpn is successfully connected. I rechecked this several times. This export makes the trick.

But it's weird anyway, hard to guess. Can you do anything about it?

Ri0n commented 7 years ago

The ebuild I'm talking about https://github.com/rion-overlay/rion-overlay/blob/master/net-misc/networkmanager-l2tp/networkmanager-l2tp-9999.ebuild

dkosovic commented 7 years ago

What is the version of autoconf you are using? In src/nm-l2tp-service.c I suspect I need to change the following code:

#ifdef RUNSTATEDIR
# define RUNDIR RUNSTATEDIR
#else
# define RUNDIR "/var/run"
#endif

to:

#ifdef LOCALSTATEDIR
# define RUNDIR LOCALSTATEDIR"/run"
#else
# define RUNDIR "/var/run"
#endif

For RUNDIR, as NetworkManager-OpenVPN uses LOCALSTATEDIR instead of RUNSTATEDIR. The reason I started using the RUNDIR macro is because NetworkManager-OpenVPN does:

Unfortunately xl2tpd has a 80 character limit for command-line arguments and because the generated config files now have a UUID instead of a PID for their names, /var/run/NetworkManager/nm-l2tp-xl2tpd-UUID.conf ends up being too long and becomes truncated by one character. So have to use /var/run instead of /var/run/NetworkManager

dkosovic commented 7 years ago

Correction, something like the following filename :

/var/run/NetworkManager/nm-l2tp-xl2tpd-90d35aa0-9b83-4d4e-a76c-3d9407aee522.conf

is 1 character too long. The max strlen for xl2tpd arguments is 79 characters, not 80 as I didn't take into account the null terminator.

Ri0n commented 7 years ago

That's why my /run/NetworkManager/nm-l2tp-xl2tpd-90d35aa0-9b83-4d4e-a76c-3d9407aee522.conf worked.

I also can confirm the code above with LOCALSTATEDIR works.

Ri0n commented 7 years ago

autoconf 2.69

dkosovic commented 7 years ago

Odd, I'm also using autoconf 2.69, but anyway, I've commited the fix

Ri0n commented 7 years ago

Thanks, now my ebuild works w/o this weird line :)

dkosovic commented 7 years ago

Actually I suspect you might need to add --localstatedir=/var like Gentoo's networkmanager-openvpn package does :

Otherwise localstatedir will default to ${prefix}/var/ where prefix is typically set to /usr/, so RUNDIR will end up as /usr/var/run

Ri0n commented 7 years ago

Gentoo passes by default --localstatedir=/var/lib but we have /run and /var/run -> /run. So yes, adding --localstatedir=/var seems to be a good idea. I'll add it.

dkosovic commented 7 years ago

With NetworkManager-l2tp 1.2.8 RUNSTATEDIR was reintroduced, but now includes a proper fix for Autoconf < 2.70 compatibility with commit # https://github.com/nm-l2tp/network-manager-l2tp/commit/9c9d8774abec37f7510924df4b6ff65ac2395550

Ri0n commented 7 years ago

looks nice =) As far as I understand no fixes to my ebuild are required. If Gentoo starts passing runstatedir one day it will start working automagically.