Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-13 Thread Kristof Provost via Openvpn-devel
On 11 Aug 2022, at 23:11, Gert Doering wrote:
> If you're interested, I can unicast you the full file I use for
> my DCO client tests, with different ciphers, some instances with
> compression (= does it properly fall back?), some with http/socks
> proxy, etc., plus a set of client+ca certificates to run against
> our test server.
>
That’d be useful, yes. I’ve not yet been able to get the tests to run the way 
they’re supposed to.

Best regards,
Kristof


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-13 Thread Kristof Provost via Openvpn-devel
On 13 Aug 2022, at 10:10, Gert Doering wrote:
> On Thu, Aug 11, 2022 at 05:25:05PM +0200, Kristof Provost via Openvpn-devel 
> wrote:
>>>  - running openvpn over TCP gives me a kernel panic - this is not so
>>>nice... (see attached .png from the vmware console) - userland seems
>>>to assume "kernel can do TCP", kernel panics on "if !udp, panic()"
>>>(so intentional panic, not corruption panic).
>>>
>>>This is on freebsd git FreeBSD 14.0-CURRENT #1 main-n257130-c0665d5c824
>>>
>> I???ve pushed a fix for this panic in 
>> fd6b3bede5a5c210f327e5c9bd3e415ee905048b.
>> I simply didn???t think that user space might give us a non-UDP
>> socket, so checking for that and rejecting the peer in that case
>> fixes the panic. Thanks for finding that.
>
> JFTR, I have tested "main-n257320-3a3af6b2a16" with the old DCO userland
> patch, and it no longer crashes.  Of course the TCP tests failed, because
> userland only sees "mmmh, it fails!" but has no idea it should fall back
> to non-DCO  (with the new userland patches, this works).
>
Thanks!

> In case you plan to include kernel TCP support, it would be good to
> have this "soonish" - like, before FreeBSD 14 and OpenVPN 2.6.0 release,
> because otherwise this will be a bit painful to synchronize.
>
There’s not plan to add TCP support at the moment.

Best regards,
Kristof


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-13 Thread Gert Doering
Hi,

On Thu, Aug 11, 2022 at 05:25:05PM +0200, Kristof Provost via Openvpn-devel 
wrote:
> >  - running openvpn over TCP gives me a kernel panic - this is not so
> >nice... (see attached .png from the vmware console) - userland seems
> >to assume "kernel can do TCP", kernel panics on "if !udp, panic()"
> >(so intentional panic, not corruption panic).
> >
> >This is on freebsd git FreeBSD 14.0-CURRENT #1 main-n257130-c0665d5c824
> >
> I???ve pushed a fix for this panic in 
> fd6b3bede5a5c210f327e5c9bd3e415ee905048b.
> I simply didn???t think that user space might give us a non-UDP
> socket, so checking for that and rejecting the peer in that case
> fixes the panic. Thanks for finding that.

JFTR, I have tested "main-n257320-3a3af6b2a16" with the old DCO userland
patch, and it no longer crashes.  Of course the TCP tests failed, because
userland only sees "mmmh, it fails!" but has no idea it should fall back
to non-DCO  (with the new userland patches, this works).

In case you plan to include kernel TCP support, it would be good to
have this "soonish" - like, before FreeBSD 14 and OpenVPN 2.6.0 release,
because otherwise this will be a bit painful to synchronize.

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-12 Thread Kristof Provost via Openvpn-devel
Remarks inline. Mostly ACK.

I’ll post an updated version soon. (I’ve also added a check for UDP in 
dco_check_option_conflict_ce().

On 10 Aug 2022, at 18:32, Gert Doering wrote:
> On Mon, Aug 08, 2022 at 04:34:23PM +0200, Kristof Provost via Openvpn-devel 
> wrote:
>> diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
>> new file mode 100644
>> index ..a8a53fe3
>> --- /dev/null
>> +++ b/src/openvpn/dco_freebsd.c
>> @@ -0,0 +1,636 @@
> [..]
>> +static nvlist_t *
>> +sockaddr_to_nvlist(const struct sockaddr *sa)
>> +{
> [..]
>> +default:
>> +abort();
>
> please use "ASSERT(0);" here.  It will do the same thing, but if ever
> hit, it will print a __FILE__, __LINE__ message to the log, so it's
> easier for us to see *where* it triggered a "this must never happen"
> condition.
>
Fixed.

>> +int
>> +dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd,
>> + struct sockaddr *localaddr, struct sockaddr *remoteaddr,
>> + struct in_addr *remote_in4, struct in6_addr *remote_in6)
>> +{
>> +struct ifdrv drv;
>> +nvlist_t *nvl;
>> +int ret;
>> +
>> +nvl = nvlist_create(0);
>
> We use C99 these days, so this could be written as
>
>> +nvlist_t *nvl = nvlist_create(0);
>
> but this is a matter of personal preference.  Both are fine, just wanted
> to point out that the option exists.
>
That’s FreeBSD’s style(9) showing here. If it’s acceptable to OpenVPN I’m just 
going to keep it as-is.

>> +
>> +nvlist_add_number(nvl, "fd", sd);
>> +nvlist_add_number(nvl, "peerid", peerid);
>> +
>> +bzero(, sizeof(drv));
>> +snprintf(drv.ifd_name, IFNAMSIZ, "%s", dco->ifname);
>> +drv.ifd_cmd = OVPN_NEW_PEER;
>> +drv.ifd_data = nvlist_pack(nvl, _len);
>> +
>> +ret = ioctl(dco->fd, SIOCSDRVSPEC, );
>> +free(drv.ifd_data);
>
> What happens should nvlist_pack() fail here?  Manpage says it returns
> NULL, will the ioctl() handle this gracefully, or will something crash?
>
That’d really only happen if a memory allocation fails, and given modern OS’s 
overcommit behaviour that’s also not going to happen. (If you manage to run the 
system out of memory malloc() will still give you virtual memory, but the OOM 
killer will come to visit when you get around to actually using it.)

> (If the ioctl() returns something like EINVAL in this case, perfectly fine)
>
The copyin() in the kernel will fail, and that’d return an error, without 
crashing things.
So, yes, that’s what’d happen.

>> +nvlist_destroy(nvl);
>> +if (ret)
>> +{
>> +msg(D_DCO, "Failed to create new peer %d", errno);
>> +return ret;
>> +}
>> +
>> +return 0;
>> +}
>
> Most functions in OpenVPN that do use a "ret" variable would look
> more like this:
>
>> +if (ret)
>> +{
>> +msg(D_DCO, "Failed to create new peer %d", errno);
>> +}
>> +
>> +return ret;
>> +}
>
> we do have all sorts of variants (*sigh*), but since you also used both
> versions, let's go for this one.
>
Fixed.

>> +msg(D_DCO, "Failed to create new peer %d", errno);
>
> this can be written as
>
>> +msg(D_DCO|M_ERRNO, "Failed to create new peer");
>
> M_ERRNO will automatically append the strerror(errno) stuff.
>
Oh neat. I’ve made that change.

> D_DCO is "--verb 3", so that message won't be visible in normal operation
> - if we consider this an error, then it should be
>
>> +msg(M_WARN|M_ERRNO, "Failed to create new peer");
>
> instead (or if it's "fatal error, give up" -> M_ERR).
>
These are generally fatal errors, yes.
There are follow-up error messages from the calling code that do get logged, 
but they’re usually less clear about what the actual problem is. I’ll see about 
changing D_DCO -> M_ERR or M_WARN.

> Now, I'm not sure if accessing errno after the free() call is guaranteed
> to be save - so maybe move the msg() call up?
>
I’m pretty sure free() doesn’t mess with errno (as in, I’ve looked at the 
jemalloc code), but it’s better to not have to check.
Happily, thanks to your earlier suggestion it’s much easier to just move the 
free after the log, so I’ll do that.

>> +bool
>> +ovpn_dco_init(int mode, dco_context_t *dco)
>> +{
>> +if (open_fd(dco) < 0)
>> +{
>> +msg(D_DCO, "Failed to open socket");
>
> Same here - if you want this to be heard, D_DCO should be M_WARN|M_ERRNO.
>
> If this is just informational, because the caller will make sufficient
> noise, D_DCO|M_ERRNO is good (but you might want to point out that this
> is the "DCO socket" that couldn't be opened).
>
Yeah, Iv’e gone through and made them all a bit louder. It’s all failure 
conditions, so being overly verbose there isn’t really an issue.

>> +static int
>> +create_interface(struct tuntap *tt, const char *dev)
>> +{
>> +int ret;
>> +struct ifreq ifr;
>> +
>> +bzero(, sizeof(ifr));
>
> There is a convenience macro here, CLEAR(ifr)  (which does exactly
> this - #define CLEAR(x) memset(&(x), 0, sizeof(x)).
>

[Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-12 Thread Kristof Provost via Openvpn-devel
From: Kristof Provost 

Implement data-channel offload for FreeBSD. The implementation and flow
is very similar to that of the Linux DCO support.

Signed-off-by: Kristof Provost 
---
 configure.ac   |   5 +
 src/openvpn/Makefile.am|   1 +
 src/openvpn/dco.c  |   8 +
 src/openvpn/dco_freebsd.c  | 645 +
 src/openvpn/dco_freebsd.h  |  59 +++
 src/openvpn/dco_internal.h |   1 +
 src/openvpn/forward.c  |   8 +-
 src/openvpn/mtcp.c |   4 +-
 src/openvpn/mudp.c |   2 +-
 src/openvpn/multi.c|   4 +-
 src/openvpn/options.c  |   8 +-
 src/openvpn/options.h  |   4 +-
 src/openvpn/ovpn_dco_freebsd.h |  64 
 src/openvpn/tun.c  |  39 +-
 src/openvpn/tun.h  |   6 +
 15 files changed, 827 insertions(+), 31 deletions(-)
 create mode 100644 src/openvpn/dco_freebsd.c
 create mode 100644 src/openvpn/dco_freebsd.h
 create mode 100644 src/openvpn/ovpn_dco_freebsd.h

diff --git a/configure.ac b/configure.ac
index 9466fe15..f715b404 100644
--- a/configure.ac
+++ b/configure.ac
@@ -787,6 +787,11 @@ dnl
AC_DEFINE(ENABLE_DCO, 1, [Enable shared data channel 
offload])
AC_MSG_NOTICE([Enabled ovpn-dco support for Linux])
;;
+   *-*-freebsd*)
+   LIBS="${LIBS} -lnv"
+   AC_DEFINE(ENABLE_DCO, 1, [Enable data channel offload 
for FreeBSD])
+   AC_MSG_NOTICE([Enabled ovpn-dco support for FreeBSD])
+   ;;
*)
AC_MSG_NOTICE([Ignoring --enable-dco on non Linux 
platform])
;;
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index aaa1dbce..2a139b23 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -54,6 +54,7 @@ openvpn_SOURCES = \
crypto_openssl.c crypto_openssl.h \
crypto_mbedtls.c crypto_mbedtls.h \
dco.c dco.h dco_internal.h \
+   dco_freebsd.c dco_freebsd.h \
dco_linux.c dco_linux.h \
dhcp.c dhcp.h \
dns.c dns.h \
diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 4f40255e..07dc1087 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -271,6 +271,14 @@ dco_check_option_conflict_ce(const struct connection_entry 
*ce, int msglevel)
 return false;
 }
 
+#if defined(TARGET_FREEBSD)
+if (! proto_is_udp(ce->proto))
+{
+msg(msglevel, "TCP is not supported.");
+return false;
+}
+#endif
+
 return true;
 }
 
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
new file mode 100644
index ..06b4d6a9
--- /dev/null
+++ b/src/openvpn/dco_freebsd.c
@@ -0,0 +1,645 @@
+/*
+ *  Interface to FreeBSD dco networking code
+ *
+ *  Copyright (C) 2022 Rubicon Communications, LLC (Netgate). All Rights 
Reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program (see the file COPYING included with this
+ *  distribution); if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#elif defined(_MSC_VER)
+#include "config-msvc.h"
+#endif
+
+#if defined(ENABLE_DCO) && defined(TARGET_FREEBSD)
+
+#include "syshead.h"
+
+#include 
+#include 
+#include 
+#include 
+
+#include "dco_freebsd.h"
+#include "dco.h"
+#include "tun.h"
+#include "crypto.h"
+#include "ssl_common.h"
+
+static nvlist_t *
+sockaddr_to_nvlist(const struct sockaddr *sa)
+{
+nvlist_t *nvl = nvlist_create(0);
+
+nvlist_add_number(nvl, "af", sa->sa_family);
+
+switch (sa->sa_family)
+{
+case AF_INET:
+{
+const struct sockaddr_in *in = (const struct sockaddr_in *)sa;
+nvlist_add_binary(nvl, "address", >sin_addr, 
sizeof(in->sin_addr));
+nvlist_add_number(nvl, "port", in->sin_port);
+break;
+}
+
+case AF_INET6:
+{
+const struct sockaddr_in6 *in6 = (const struct sockaddr_in6 *)sa;
+nvlist_add_binary(nvl, "address", >sin6_addr, 
sizeof(in6->sin6_addr));
+nvlist_add_number(nvl, "port", in6->sin6_port);
+break;
+}
+
+default:
+ASSERT(0);
+}
+
+return (nvl);
+}
+
+int
+dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd,
+ struct sockaddr *localaddr, 

Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-11 Thread Gert Doering
Hi,

On Thu, Aug 11, 2022 at 05:25:05PM +0200, Kristof Provost via Openvpn-devel 
wrote:
> On 10 Aug 2022, at 18:32, Gert Doering wrote:
> > as promised, here's test results and code review.
> >
> > Test results:
> >
> >  - running openvpn over TCP gives me a kernel panic - this is not so
> >nice... (see attached .png from the vmware console) - userland seems
> >to assume "kernel can do TCP", kernel panics on "if !udp, panic()"
> >(so intentional panic, not corruption panic).
> >
> >This is on freebsd git FreeBSD 14.0-CURRENT #1 main-n257130-c0665d5c824
>
> I???ve pushed a fix for this panic in 
> fd6b3bede5a5c210f327e5c9bd3e415ee905048b.
> I simply didn???t think that user space might give us a non-UDP
> socket, so checking for that and rejecting the peer in that case
> fixes the panic. Thanks for finding that.

Always happy to crash other people's software :-)

That said, this seems to imply that FreeBSD DCO is UDP-only today - do
you have plans for TCP as well, or is this something "someone else needs
to contribute"?

For the time being we should add an #ifdef TARGET_FREEBSD check to
dco.c:dco_check_option_conflict_platform() and disable DCO for
--proto tcp.

I thought the function would only get introduced in patch 20/25 of
the big DCO series, but that patch is a bit out of sync - the function
is already there, with Linux specific checks.


> >  - running openvpn over UDP has issues with fragmentation - almost all
> >t_client tests that *do* use DCO fail the "big ping" test
> >
> > FAIL: IPv4 ping test (3000 bytes) failed, but should succeed.
> > FAIL: IPv6 ping test (3000 bytes) failed, but should succeed.
> >
> >this is "with no special mtu related options to OpenVPN", so the
> >tun interface is MTU 1500, and the pings are created by
> >
> > fping -b 3000 -C 20 -p 250 -q -C 5 -p 400 10.194.5.1 10.194.0.1
> > fping6 -b 3000 -C 20 -p 250 -q -C 5 -p 400 fd00:abcd:194:5::1 
> > fd00:abcd:194:0::1
> >
> >(testing the host "on the p2p link" and "something behind a --route
> > pushed by the server")
>
> I???m failing to reproduce this in my own test setup. With a
> simple ???ping -c 2000??? on a 1500 MTU tunnel I see two layers of
> fragmentation (the ICMP packet gets fragmented before it goes through
> the tunnel, and then the outer UDP packet gets fragmented as well),
> as I???d expect. Traffic flows in both directions. (i.e. the ping
> succeeds).

OK.  This is generally good, so I need to find out who is eating my
packets.  In my case, I have 3 fragments, which might or might not
cause a difference - and there's stuff in between (FreeBSD 14 client
via FreeBSD 12 pf(4) firewall to FreeBSD 11 [or so] server).

> Is there any documentation on how t_client.sh works? It???s not at all 
> clear to me what goes in t_client.in (e.g. in OPENVPN_BASE_P2P).

Basically, t_client is a very basic vehicle to fire up certain
OpenVPN configurations, verify that the expect IPv4 and IPv6 addresses
show up on "a new interface", and that pings to certain destinations
work, with varying packet sizes.  It's a bit of a sledge hammer, but
it's good at finding stuff.

My configs have something like this:

CA_CERT="$KEYBASE/ca.crt"
CLIENT_KEY="$KEYBASE/cron2-gentoo-i386.key"
CLIENT_CERT="$KEYBASE/cron2-gentoo-i386.crt"

REMOTE=conn-test-server.openvpn.org

# this is the basic "TLS client" stuff with ca/cert/key
# remove the comp-lzo for DCO tests
OPENVPN_BASE_P2MP="--client --ca $CA_CERT \
--cert $CLIENT_CERT --key $CLIENT_KEY \
--remote-cert-tls server --comp-lzo --verb 3 $OPENVPN_EXTRA_CF"

#
# Test 2: UDP / p2mp tun
#   specify IPv4+IPv6 addresses expected from server and ping targets
#
RUN_TITLE_2="udp / p2pm / top net30"
OPENVPN_CONF_2="$OPENVPN_BASE_P2MP --dev tun --proto udp4 --remote $REMOTE 
--port 51194"
EXPECT_IFCONFIG4_2="10.194.2.54"
EXPECT_IFCONFIG6_2="fd00:abcd:194:2::100c"
PING4_HOSTS_2="10.194.2.1 10.194.0.1"
PING6_HOSTS_2="fd00:abcd:194:2::1 fd00:abcd:194:0::1"

... so this would connect to a specific UDP instance on the test server,
expect to be assigned these addresses (= this needs adjustment after
the first run, when you know what addresses will be assigned, and it
assumes --ip-pool-persist on the server), and then pings things.

I usually ping "the server IP on that tun" and "a loopback IP on the
server, which is only reachable by means of 'push route...'", so I
can see if push/pull and route installation works.


If you're interested, I can unicast you the full file I use for
my DCO client tests, with different ciphers, some instances with
compression (= does it properly fall back?), some with http/socks
proxy, etc., plus a set of client+ca certificates to run against
our test server.


[..]
> >... this *does* look correct, but there is never a reply from the other
> >end, so something is not right.  Either inside or outside.
>
> Is your server in this scenario a FreeBSD DCO machine, or Linux DCO or 
> something else?
> In 

Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-11 Thread Gert Doering
Hi,

On Thu, Aug 11, 2022 at 07:00:25PM +0200, Arne Schwabe wrote:
> > I???ve pushed a fix for this panic in 
> > fd6b3bede5a5c210f327e5c9bd3e415ee905048b.
> > I simply didn???t think that user space might give us a non-UDP socket, so 
> > checking for that and rejecting the peer in that case fixes the panic. 
> > Thanks for finding that.
> 
> You should probably also modify the check for dco incompatible in 
> OpenVPN so that using TCP disables DCO on FreeBSD.

Indeed.  One of the next patches will bring "per platform options check",
which would be a good place for the #ifdef TARGET_FREEBSD bits.

I'll see that I can pick part of that patch and apply it tomorrow
(the patch as posted refers to symbols that are brought in by the
"main" win-dco patch, but half of it should apply & make sense as is)

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-11 Thread Arne Schwabe

Am 11.08.22 um 17:25 schrieb Kristof Provost via Openvpn-devel:

On 10 Aug 2022, at 18:32, Gert Doering wrote:

as promised, here's test results and code review.

Test results:

  - running openvpn over TCP gives me a kernel panic - this is not so
nice... (see attached .png from the vmware console) - userland seems
to assume "kernel can do TCP", kernel panics on "if !udp, panic()"
(so intentional panic, not corruption panic).

This is on freebsd git FreeBSD 14.0-CURRENT #1 main-n257130-c0665d5c824


I’ve pushed a fix for this panic in fd6b3bede5a5c210f327e5c9bd3e415ee905048b.
I simply didn’t think that user space might give us a non-UDP socket, so 
checking for that and rejecting the peer in that case fixes the panic. Thanks 
for finding that.


You should probably also modify the check for dco incompatible in 
OpenVPN so that using TCP disables DCO on FreeBSD.


Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-11 Thread Kristof Provost via Openvpn-devel
On 10 Aug 2022, at 18:32, Gert Doering wrote:
> as promised, here's test results and code review.
>
> Test results:
>
>  - running openvpn over TCP gives me a kernel panic - this is not so
>nice... (see attached .png from the vmware console) - userland seems
>to assume "kernel can do TCP", kernel panics on "if !udp, panic()"
>(so intentional panic, not corruption panic).
>
>This is on freebsd git FreeBSD 14.0-CURRENT #1 main-n257130-c0665d5c824
>
I’ve pushed a fix for this panic in fd6b3bede5a5c210f327e5c9bd3e415ee905048b.
I simply didn’t think that user space might give us a non-UDP socket, so 
checking for that and rejecting the peer in that case fixes the panic. Thanks 
for finding that.

>  - running openvpn over UDP has issues with fragmentation - almost all
>t_client tests that *do* use DCO fail the "big ping" test
>
> run IPv4 ping tests (want_ok), 64 byte packets...
> run IPv4 ping tests (want_ok), 1440 byte packets...
> run IPv4 ping tests (want_ok), 3000 byte packets...
>
> FAIL: IPv4 ping test (3000 bytes) failed, but should succeed.
> run IPv6 ping tests (want_ok), 64 byte packets...
> run IPv6 ping tests (want_ok), 1440 byte packets...
> run IPv6 ping tests (want_ok), 3000 byte packets...
>
> FAIL: IPv6 ping test (3000 bytes) failed, but should succeed.
>
>this is "with no special mtu related options to OpenVPN", so the
>tun interface is MTU 1500, and the pings are created by
>
> fping -b 3000 -C 20 -p 250 -q -C 5 -p 400 10.194.5.1 10.194.0.1
> fping6 -b 3000 -C 20 -p 250 -q -C 5 -p 400 fd00:abcd:194:5::1 
> fd00:abcd:194:0::1
>
>(testing the host "on the p2p link" and "something behind a --route
> pushed by the server")
>
I’m failing to reproduce this in my own test setup. With a simple ‘ping -c 
2000’ on a 1500 MTU tunnel I see two layers of fragmentation (the ICMP packet 
gets fragmented before it goes through the tunnel, and then the outer UDP 
packet gets fragmented as well), as I’d expect. Traffic flows in both 
directions. (i.e. the ping succeeds).

Is there any documentation on how t_client.sh works? It’s not at all clear to 
me what goes in t_client.in (e.g. in OPENVPN_BASE_P2P).

>I have not investigated exactly what happens at which point yet, but
>with Linux DCO, we had a similar effect where a fragmented *inside*
>packet ("ICMP") carried over a flag in the sk_buff that resulted in
>the kernel refusing to fragment the resulting *outside* packet.
>
>(Inside MTU 1500 -> fping -s 3000 -> 3 "inside" fragments,
> 1500+1500+68, adding openvpn overhead -> packet > 1500 ->
> external fragmentation needed for the first two)
>
>Inside tcpdump (tun0):
>
> 17:28:37.342050 IP (tos 0x0, ttl 64, id 50758, offset 0, flags [+], proto 
> ICMP (1), length 1500)
> 10.194.5.222 > 10.194.5.1: ICMP echo request, id 2433, seq 0, length 1480
> 17:28:37.342142 IP (tos 0x0, ttl 64, id 50758, offset 1480, flags [+], proto 
> ICMP (1), length 1500)
> 10.194.5.222 > 10.194.5.1: ip-proto-1
> 17:28:37.342270 IP (tos 0x0, ttl 64, id 50758, offset 2960, flags [none], 
> proto ICMP (1), length 68)
> 10.194.5.222 > 10.194.5.1: ip-proto-1
>
>Outside tcpdump (em0):
>
> 17:28:37.342103 IP (tos 0x0, ttl 64, id 50759, offset 0, flags [+], proto UDP 
> (17), length 1500)
> 194.97.140.5.15738 > 199.102.77.82.51197: UDP, bad length 1528 > 1472
> 17:28:37.342123 IP (tos 0x0, ttl 64, id 50759, offset 1480, flags [none], 
> proto UDP (17), length 76)
> 194.97.140.5 > 199.102.77.82: ip-proto-17
> 17:28:37.342185 IP (tos 0x0, ttl 64, id 50760, offset 0, flags [+], proto UDP 
> (17), length 1500)
> 194.97.140.5.15738 > 199.102.77.82.51197: UDP, bad length 1528 > 1472
> 17:28:37.342228 IP (tos 0x0, ttl 64, id 50760, offset 1480, flags [none], 
> proto UDP (17), length 76)
> 194.97.140.5 > 199.102.77.82: ip-proto-17
> 17:28:37.342300 IP (tos 0x0, ttl 64, id 50761, offset 0, flags [none], proto 
> UDP (17), length 132)
> 194.97.140.5.15738 > 199.102.77.82.51197: UDP, length 104
>
>
>... this *does* look correct, but there is never a reply from the other
>end, so something is not right.  Either inside or outside.
>
Is your server in this scenario a FreeBSD DCO machine, or Linux DCO or 
something else?
In my setup the server is non-DCO FreeBSD.

Best regards,
Kristof


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-10 Thread Kristof Provost via Openvpn-devel
Thanks!

On 10 Aug 2022, at 18:32, Gert Doering wrote:
> Test results:
>
>  - running openvpn over TCP gives me a kernel panic - this is not so
>nice... (see attached .png from the vmware console) - userland seems
>to assume "kernel can do TCP", kernel panics on "if !udp, panic()"
>(so intentional panic, not corruption panic).
>
>This is on freebsd git FreeBSD 14.0-CURRENT #1 main-n257130-c0665d5c824
>
I think I see what the problem is. I’m writing up a test case to reproduce it 
and will commit a fix soon. Hopefully tomorrow.

>  - running openvpn over UDP has issues with fragmentation - almost all
>t_client tests that *do* use DCO fail the "big ping" test
>
I’m going to have to do some digging here.

> - tcpdump'ing on the DCO interface gave me complains from the kernel
>   about locking on ctrl-c'ing
>
> Aug 10 17:28:41 fbsd14 kernel: lock order bpf global lock -> iflib ctx 
> lock attempted at:
> Aug 10 17:28:41 fbsd14 kernel: #0 0x80c5c3dd at 
> witness_checkorder+0xbfd
> Aug 10 17:28:41 fbsd14 kernel: #1 0x80bf5303 at _sx_xlock+0x63
> Aug 10 17:28:41 fbsd14 kernel: #2 0x80d3874f at 
> iflib_if_ioctl+0x2df
> Aug 10 17:28:41 fbsd14 kernel: #3 0x80d19b5e at if_setflag+0xde
> Aug 10 17:28:41 fbsd14 kernel: #4 0x80d19a2a at ifpromisc+0x2a
> Aug 10 17:28:41 fbsd14 kernel: #5 0x80d0e72b at 
> bpf_detachd_locked+0x27b
> Aug 10 17:28:41 fbsd14 kernel: #6 0x80d111f7 at bpf_dtor+0x87
> Aug 10 17:28:41 fbsd14 kernel: #7 0x80a7818b at 
> devfs_destroy_cdevpriv+0xab
> Aug 10 17:28:41 fbsd14 kernel: #8 0x80a7bda4 at devfs_close_f+0x64
> Aug 10 17:28:41 fbsd14 kernel: #9 0x80b876eb at _fdrop+0x1b
> Aug 10 17:28:41 fbsd14 kernel: #10 0x80b8af3b at closef+0x1db
> Aug 10 17:28:41 fbsd14 kernel: #11 0x80b8ec97 at closefp_impl+0x77
> Aug 10 17:28:41 fbsd14 kernel: #12 0x810c733e at 
> amd64_syscall+0x12e
> Aug 10 17:28:41 fbsd14 kernel: #13 0x8109ae0b at 
> fast_syscall_common+0xf8
>
>... so while this is outside "openvpn source code patches", it's
>still something that smells like it needs to be addressed.
>
That appears to be unrelated to DCO. It’s a problem with iflib, and I think 
I’ve seen it before.
Out of scope for DCO, at least.

> Now, coding style ;-) - as promised, I went through the code for things
> that need to be done in a certain way in OpenVPN land, due to agreed
> convention... inline (things I do not comment could go in "as is").
>
I’ll fix those once I’ve dealt with the panic and fragmentation issues.

Thanks for the review.

Kristof


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-10 Thread Gert Doering
(Re-sending, as the first one had a .png attached which exceeded what
sourceforge is willing to forward)

Hi,

as promised, here's test results and code review.

Test results:

 - running openvpn over TCP gives me a kernel panic - this is not so
   nice... (see attached .png from the vmware console) - userland seems
   to assume "kernel can do TCP", kernel panics on "if !udp, panic()"
   (so intentional panic, not corruption panic).

   This is on freebsd git FreeBSD 14.0-CURRENT #1 main-n257130-c0665d5c824

 - running openvpn over UDP has issues with fragmentation - almost all
   t_client tests that *do* use DCO fail the "big ping" test

run IPv4 ping tests (want_ok), 64 byte packets...
run IPv4 ping tests (want_ok), 1440 byte packets...
run IPv4 ping tests (want_ok), 3000 byte packets...

FAIL: IPv4 ping test (3000 bytes) failed, but should succeed.
run IPv6 ping tests (want_ok), 64 byte packets...
run IPv6 ping tests (want_ok), 1440 byte packets...
run IPv6 ping tests (want_ok), 3000 byte packets...

FAIL: IPv6 ping test (3000 bytes) failed, but should succeed.

   this is "with no special mtu related options to OpenVPN", so the
   tun interface is MTU 1500, and the pings are created by

fping -b 3000 -C 20 -p 250 -q -C 5 -p 400 10.194.5.1 10.194.0.1
fping6 -b 3000 -C 20 -p 250 -q -C 5 -p 400 fd00:abcd:194:5::1 
fd00:abcd:194:0::1

   (testing the host "on the p2p link" and "something behind a --route
pushed by the server")

   I have not investigated exactly what happens at which point yet, but
   with Linux DCO, we had a similar effect where a fragmented *inside*
   packet ("ICMP") carried over a flag in the sk_buff that resulted in
   the kernel refusing to fragment the resulting *outside* packet.

   (Inside MTU 1500 -> fping -s 3000 -> 3 "inside" fragments, 
1500+1500+68, adding openvpn overhead -> packet > 1500 -> 
external fragmentation needed for the first two)

   Inside tcpdump (tun0):

17:28:37.342050 IP (tos 0x0, ttl 64, id 50758, offset 0, flags [+], proto ICMP 
(1), length 1500)
10.194.5.222 > 10.194.5.1: ICMP echo request, id 2433, seq 0, length 1480
17:28:37.342142 IP (tos 0x0, ttl 64, id 50758, offset 1480, flags [+], proto 
ICMP (1), length 1500)
10.194.5.222 > 10.194.5.1: ip-proto-1
17:28:37.342270 IP (tos 0x0, ttl 64, id 50758, offset 2960, flags [none], proto 
ICMP (1), length 68)
10.194.5.222 > 10.194.5.1: ip-proto-1

   Outside tcpdump (em0):

17:28:37.342103 IP (tos 0x0, ttl 64, id 50759, offset 0, flags [+], proto UDP 
(17), length 1500)
194.97.140.5.15738 > 199.102.77.82.51197: UDP, bad length 1528 > 1472
17:28:37.342123 IP (tos 0x0, ttl 64, id 50759, offset 1480, flags [none], proto 
UDP (17), length 76)
194.97.140.5 > 199.102.77.82: ip-proto-17
17:28:37.342185 IP (tos 0x0, ttl 64, id 50760, offset 0, flags [+], proto UDP 
(17), length 1500)
194.97.140.5.15738 > 199.102.77.82.51197: UDP, bad length 1528 > 1472
17:28:37.342228 IP (tos 0x0, ttl 64, id 50760, offset 1480, flags [none], proto 
UDP (17), length 76)
194.97.140.5 > 199.102.77.82: ip-proto-17
17:28:37.342300 IP (tos 0x0, ttl 64, id 50761, offset 0, flags [none], proto 
UDP (17), length 132)
194.97.140.5.15738 > 199.102.77.82.51197: UDP, length 104


   ... this *does* look correct, but there is never a reply from the other
   end, so something is not right.  Either inside or outside.

   This happens with IPv4 or IPv6 UDP outside, and IPv4 or IPv6 inside.


- tcpdump'ing on the DCO interface gave me complains from the kernel
  about locking on ctrl-c'ing

Aug 10 17:28:41 fbsd14 kernel: lock order bpf global lock -> iflib ctx lock 
attempted at:
Aug 10 17:28:41 fbsd14 kernel: #0 0x80c5c3dd at 
witness_checkorder+0xbfd
Aug 10 17:28:41 fbsd14 kernel: #1 0x80bf5303 at _sx_xlock+0x63
Aug 10 17:28:41 fbsd14 kernel: #2 0x80d3874f at iflib_if_ioctl+0x2df
Aug 10 17:28:41 fbsd14 kernel: #3 0x80d19b5e at if_setflag+0xde
Aug 10 17:28:41 fbsd14 kernel: #4 0x80d19a2a at ifpromisc+0x2a
Aug 10 17:28:41 fbsd14 kernel: #5 0x80d0e72b at 
bpf_detachd_locked+0x27b
Aug 10 17:28:41 fbsd14 kernel: #6 0x80d111f7 at bpf_dtor+0x87
Aug 10 17:28:41 fbsd14 kernel: #7 0x80a7818b at 
devfs_destroy_cdevpriv+0xab
Aug 10 17:28:41 fbsd14 kernel: #8 0x80a7bda4 at devfs_close_f+0x64
Aug 10 17:28:41 fbsd14 kernel: #9 0x80b876eb at _fdrop+0x1b
Aug 10 17:28:41 fbsd14 kernel: #10 0x80b8af3b at closef+0x1db
Aug 10 17:28:41 fbsd14 kernel: #11 0x80b8ec97 at closefp_impl+0x77
Aug 10 17:28:41 fbsd14 kernel: #12 0x810c733e at amd64_syscall+0x12e
Aug 10 17:28:41 fbsd14 kernel: #13 0x8109ae0b at 
fast_syscall_common+0xf8

   ... so while this is outside "openvpn source code patches", it's 
   still something that smells like it needs to be addressed.


Now, coding style ;-) - 

[Openvpn-devel] [PATCH 1/2] ovpn-dco: introduce FreeBSD data-channel offload support

2022-08-08 Thread Kristof Provost via Openvpn-devel
From: Kristof Provost 

Implement data-channel offload for FreeBSD. The implementation and flow
is very similar to that of the Linux DCO support.

Signed-off-by: Kristof Provost 
---
 configure.ac   |   5 +
 src/openvpn/Makefile.am|   1 +
 src/openvpn/dco_freebsd.c  | 636 +
 src/openvpn/dco_freebsd.h  |  59 +++
 src/openvpn/dco_internal.h |   1 +
 src/openvpn/forward.c  |   8 +-
 src/openvpn/mtcp.c |   4 +-
 src/openvpn/mudp.c |   2 +-
 src/openvpn/multi.c|   4 +-
 src/openvpn/options.c  |   8 +-
 src/openvpn/options.h  |   4 +-
 src/openvpn/ovpn_dco_freebsd.h |  64 
 src/openvpn/tun.c  |  38 +-
 src/openvpn/tun.h  |   6 +
 14 files changed, 808 insertions(+), 32 deletions(-)
 create mode 100644 src/openvpn/dco_freebsd.c
 create mode 100644 src/openvpn/dco_freebsd.h
 create mode 100644 src/openvpn/ovpn_dco_freebsd.h

diff --git a/configure.ac b/configure.ac
index 353da08c..ed1332da 100644
--- a/configure.ac
+++ b/configure.ac
@@ -787,6 +787,11 @@ dnl
AC_DEFINE(ENABLE_DCO, 1, [Enable shared data channel 
offload])
AC_MSG_NOTICE([Enabled ovpn-dco support for Linux])
;;
+   *-*-freebsd*)
+   LIBS="${LIBS} -lnv"
+   AC_DEFINE(ENABLE_DCO, 1, [Enable data channel offload 
for FreeBSD])
+   AC_MSG_NOTICE([Enabled ovpn-dco support for FreeBSD])
+   ;;
*)
AC_MSG_NOTICE([Ignoring --enable-dco on non Linux 
platform])
;;
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index aaa1dbce..2a139b23 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -54,6 +54,7 @@ openvpn_SOURCES = \
crypto_openssl.c crypto_openssl.h \
crypto_mbedtls.c crypto_mbedtls.h \
dco.c dco.h dco_internal.h \
+   dco_freebsd.c dco_freebsd.h \
dco_linux.c dco_linux.h \
dhcp.c dhcp.h \
dns.c dns.h \
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
new file mode 100644
index ..a8a53fe3
--- /dev/null
+++ b/src/openvpn/dco_freebsd.c
@@ -0,0 +1,636 @@
+/*
+ *  Interface to FreeBSD dco networking code
+ *
+ *  Copyright (C) 2022 Rubicon Communications, LLC (Netgate). All Rights 
Reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program (see the file COPYING included with this
+ *  distribution); if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#elif defined(_MSC_VER)
+#include "config-msvc.h"
+#endif
+
+#if defined(ENABLE_DCO) && defined(TARGET_FREEBSD)
+
+#include "syshead.h"
+
+#include 
+#include 
+#include 
+#include 
+
+#include "dco_freebsd.h"
+#include "dco.h"
+#include "tun.h"
+#include "crypto.h"
+#include "ssl_common.h"
+
+static nvlist_t *
+sockaddr_to_nvlist(const struct sockaddr *sa)
+{
+nvlist_t *nvl = nvlist_create(0);
+
+nvlist_add_number(nvl, "af", sa->sa_family);
+
+switch (sa->sa_family)
+{
+case AF_INET:
+{
+const struct sockaddr_in *in = (const struct sockaddr_in *)sa;
+nvlist_add_binary(nvl, "address", >sin_addr, 
sizeof(in->sin_addr));
+nvlist_add_number(nvl, "port", in->sin_port);
+break;
+}
+
+case AF_INET6:
+{
+const struct sockaddr_in6 *in6 = (const struct sockaddr_in6 *)sa;
+nvlist_add_binary(nvl, "address", >sin6_addr, 
sizeof(in6->sin6_addr));
+nvlist_add_number(nvl, "port", in6->sin6_port);
+break;
+}
+
+default:
+abort();
+}
+
+return (nvl);
+}
+
+int
+dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd,
+ struct sockaddr *localaddr, struct sockaddr *remoteaddr,
+ struct in_addr *remote_in4, struct in6_addr *remote_in6)
+{
+struct ifdrv drv;
+nvlist_t *nvl;
+int ret;
+
+nvl = nvlist_create(0);
+
+msg(D_DCO_DEBUG, "%s: peer-id %d, fd %d", __func__, peerid, sd);
+
+if (localaddr)
+{
+nvlist_add_nvlist(nvl, "local", sockaddr_to_nvlist(localaddr));
+}
+
+if (remoteaddr)
+{
+nvlist_add_nvlist(nvl, "remote", sockaddr_to_nvlist(remoteaddr));
+}
+
+if