| Message ID | 20220819182439.71531-1-gert@greenie.muc.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] FreeBSD-DCO: repair device iteration to find first free interface. | expand |
Acked-by: Kristof Provost <kprovost@netgate.com> Thanks! Kristof On 19 Aug 2022, at 20:24, Gert Doering wrote: > During review/update phase, FreeBSD/DCO's ability to find the first > free tun interface on "--dev tun" got broken, due to two issues: > > - create_interface() called msg(M_ERR|...), which is a fatal error > and aborts OpenVPN, so "no retry with 'tun1' after 'tun0' failed" > > Change to M_WARN|M_ERRNO (= warning level, add strerror(errno), return). > > - open_tun_dco_generic() expects "-errno" as return value of > open_tun_dco(), and breaks the loop on -EPERM. create_interface() > was returning "-1" instead (ioctl() error signalling), which happens > to be "-EPERM" on FreeBSD. > > Change create_interface() to return -errno. > > While at it, remove logging of errors from dco_freebsd.c::open_tun_dco() > (because all errors from create_interface() would be already logged there), > reducing open_tun_dco() to just a wrapper around create_interface(). > > Signed-off-by: Gert Doering <gert@greenie.muc.de> > --- > src/openvpn/dco_freebsd.c | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c > index 06b4d6a9..c6da6ce3 100644 > --- a/src/openvpn/dco_freebsd.c > +++ b/src/openvpn/dco_freebsd.c > @@ -178,7 +178,8 @@ create_interface(struct tuntap *tt, const char *dev) > ret = ioctl(tt->dco.fd, SIOCIFCREATE2, &ifr); > if (ret) > { > - msg(M_ERR | M_ERRNO, "Failed to create interface %s", ifr.ifr_name); > + ret = -errno; > + msg(M_WARN|M_ERRNO, "Failed to create interface %s (SIOCIFCREATE2)", ifr.ifr_name); > return ret; > } > > @@ -194,9 +195,10 @@ create_interface(struct tuntap *tt, const char *dev) > ret = ioctl(tt->dco.fd, SIOCSIFNAME, &ifr); > if (ret) > { > + ret = -errno; > /* Delete the created interface again. */ > (void)ioctl(tt->dco.fd, SIOCIFDESTROY, &ifr); > - msg(M_ERR | M_ERRNO, "Failed to create interface %s", ifr.ifr_data); > + msg(M_WARN|M_ERRNO, "Failed to create interface %s (SIOCSIFNAME)", ifr.ifr_data); > return ret; > } > > @@ -229,16 +231,7 @@ remove_interface(struct tuntap *tt) > int > open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, const char *dev) > { > - int ret; > - > - ret = create_interface(tt, dev); > - > - if (ret < 0) > - { > - msg(M_ERR, "Failed to create interface"); > - } > - > - return ret; > + return create_interface(tt, dev); > } > > void > -- > 2.37.1
To avoid any fat fingers on my side, I've merged this and then subjected
the resulting branch to a round of FreeBSD DCO server tests, which all
behaved quite nicely :-) (iroute missing, because not merged yet, but
the rest works - TCP [no DCO], UDP [DCO, if topology subnet], etc)
Aug 20 11:18:59 fbsd14 tun-udp-p2mp[79256]: GLOBAL_STATS,dco_enabled,1
Aug 20 11:18:59 fbsd14 tun-udp-p2mp-topology-subnet[79278]: GLOBAL_STATS,dco_enabled,1
Patch has been applied to the master branch.
commit efebdfe2de8dd5125fb42d646d03a958b7dc3eea
Author: Gert Doering
Date: Fri Aug 19 20:24:39 2022 +0200
FreeBSD-DCO: repair device iteration to find first free interface.
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Kristof Provost <kprovost@netgate.com>
Message-Id: <20220819182439.71531-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25034.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index 06b4d6a9..c6da6ce3 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -178,7 +178,8 @@ create_interface(struct tuntap *tt, const char *dev) ret = ioctl(tt->dco.fd, SIOCIFCREATE2, &ifr); if (ret) { - msg(M_ERR | M_ERRNO, "Failed to create interface %s", ifr.ifr_name); + ret = -errno; + msg(M_WARN|M_ERRNO, "Failed to create interface %s (SIOCIFCREATE2)", ifr.ifr_name); return ret; } @@ -194,9 +195,10 @@ create_interface(struct tuntap *tt, const char *dev) ret = ioctl(tt->dco.fd, SIOCSIFNAME, &ifr); if (ret) { + ret = -errno; /* Delete the created interface again. */ (void)ioctl(tt->dco.fd, SIOCIFDESTROY, &ifr); - msg(M_ERR | M_ERRNO, "Failed to create interface %s", ifr.ifr_data); + msg(M_WARN|M_ERRNO, "Failed to create interface %s (SIOCSIFNAME)", ifr.ifr_data); return ret; } @@ -229,16 +231,7 @@ remove_interface(struct tuntap *tt) int open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, const char *dev) { - int ret; - - ret = create_interface(tt, dev); - - if (ret < 0) - { - msg(M_ERR, "Failed to create interface"); - } - - return ret; + return create_interface(tt, dev); } void
During review/update phase, FreeBSD/DCO's ability to find the first free tun interface on "--dev tun" got broken, due to two issues: - create_interface() called msg(M_ERR|...), which is a fatal error and aborts OpenVPN, so "no retry with 'tun1' after 'tun0' failed" Change to M_WARN|M_ERRNO (= warning level, add strerror(errno), return). - open_tun_dco_generic() expects "-errno" as return value of open_tun_dco(), and breaks the loop on -EPERM. create_interface() was returning "-1" instead (ioctl() error signalling), which happens to be "-EPERM" on FreeBSD. Change create_interface() to return -errno. While at it, remove logging of errors from dco_freebsd.c::open_tun_dco() (because all errors from create_interface() would be already logged there), reducing open_tun_dco() to just a wrapper around create_interface(). Signed-off-by: Gert Doering <gert@greenie.muc.de> --- src/openvpn/dco_freebsd.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)