| Message ID | 20220812134154.16729-3-kprovost@netgate.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,1/2] ovpn-dco: introduce FreeBSD data-channel offload support | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> Stared-at-code, tested on FreeBSD 14 with and without DCO as client, and with DCO as server, with a number of interesting iroute problems (--route /24, --iroute /32, --route /24 and --iroute same /24, no --route at all, but --iroute /24). See below. The "big packet / fragment" thing still haunts us, but that's not something this patch would address, so out of scope wrt "does iroute work". Some notes - uncrustify had some remarks on #else/#endif with comments -> applied - inet_ntop(3) says one shouldn't use magic numbers, but "INET_ADDRSTRLEN and INET6_ADDRSTRLEN" - this could be addressed in a followup patch - "the rest of the code" uses print_in_addr_t() and print_in6_addr() today, to get a formatted IPv4/IPv6 address out of an in*_addr - but these excel if there is a &gc around, and adding extra gc_new()/gc_free() around the argv_printf() did not really sound like "the resulting code is much nicer". - any particular reason you used argv_printf() + argv_printf_cat(), instead of just putting all into a single argv_printf() call? - the "openvpn_execve_check()" messages state "route *add* command failed", while it could be "del" as well. Making this "proper" might not be worth it, as it would need string manipulation (or "duplicate whole message"). - iroute installation works for the easy cases (--route in server.conf, --iroute with a more-specific of that in ccd/). It does not work for the nasty cases (--route and --iroute with same netbits). I will send a followup e-mail with more details on this soon. - route *deletion* does not work for IPv4 host routes, because of "other code stupidity" - this is not a problem of your code, but of an omission in dco.c (and I do wonder why this works for Linux) 2022-08-20 14:06:56 us=764185 /sbin/route del -net 10.114.200.74/-1 10.114.2.8 - fib 0 route: bad mask length: -1 our internal routes carry a flag structure that says "this is a host route!!" and if it is, "netbits" is not valid and needs to be set to 32 / 128 explicitly. This is pending a larger refactoring, but for the time being, I'll send a patch to fix this in dco.c (interesting enough, my code for IPv6 does this correctly, only the old IPv4 cruft...) - in one case I saw something that looks like a race condition between DCO, "ifconfig route propagation", and "route add" 2022-08-20 15:14:58 us=882323 DCO device tun1 opened 2022-08-20 15:14:58 us=882357 do_ifconfig, ipv4=1, ipv6=1 2022-08-20 15:14:58 us=882393 /sbin/ifconfig tun1 10.114.2.1 10.114.2.2 mtu 1500 netmask 255.255.255.0 up 2022-08-20 15:14:58 us=887080 /sbin/route add -net 10.114.2.0 10.114.2.2 255.255.255.0 add net 10.114.2.0: gateway 10.114.2.2 2022-08-20 15:14:58 us=890690 /sbin/ifconfig tun1 inet6 fd00:abcd:114:2::1/64 mtu 1500 up 2022-08-20 15:14:59 us=902853 /sbin/ifconfig tun1 inet6 -ifdisabled 2022-08-20 15:15:02 us=57619 freebsd-74-amd64/2001:608:0:814::f000:3 /sbin/route add -net 10.114.202.74/32 10.114.2.8 -fib 0 route: writing to routing socket: Network is unreachable add net 10.114.202.74: gateway 10.114.2.8 fib 0: Network is unreachable ... repeating the "route add" call manually (copy-paste) worked without error, so I'm not sure what upset things here. 3 Seconds between interface init and route addition should be more than enough for cooldown as well. Your patch has been applied to the master branch. commit 3433577a99270845cbce7fe738534e0b0ffd5780 Author: Kristof Provost Date: Fri Aug 12 15:41:54 2022 +0200 Support creating iroute route entries on FreeBSD Signed-off-by: Kristof Provost <kprovost@netgate.com> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20220812134154.16729-3-kprovost@netgate.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24895.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
Hi, so, as promised, more details on the route/iroute issues I saw... with a new Subject: line, to keep the threads halfway orderly. On Sat, Aug 20, 2022 at 04:01:02PM +0200, Gert Doering wrote: > - route *deletion* does not work for IPv4 host routes, because of > "other code stupidity" - this is not a problem of your code, but > of an omission in dco.c (and I do wonder why this works for Linux) A patch for *this* has been sent, so this is of no concern anymore (just needs someone bold enough to ACK it). > - in one case I saw something that looks like a race condition between > DCO, "ifconfig route propagation", and "route add" This is something I need to test further, to see if this was a one-off issue, or can be reproduced with good timing. > - iroute installation works for the easy cases (--route in server.conf, > --iroute with a more-specific of that in ccd/). It does not work > for the nasty cases (--route and --iroute with same netbits). > > I will send a followup e-mail with more details on this soon. *This* is the bit I want to cover separately, because this e-mail is going to be long :-) So, here's my test cases, to see if I could get DCO to misbehave. Server config has: # grep ^route server.conf route 10.114.200.0 255.255.255.0 route-ipv6 fd00:abcd:114:200::/64 route 10.114.201.0 255.255.255.0 route-ipv6 fd00:abcd:114:201::/64 ("pull two /24 and two /64 towards the tun interface") There is one particular client which has many addresses configured on its loopback lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> metric 0 mtu 16384 inet 10.114.200.74 netmask 0xffffffff inet 10.114.201.74 netmask 0xffffffff inet 10.114.202.74 netmask 0xffffffff inet6 fd00:abcd:114:200::74 prefixlen 128 inet6 fd00:abcd:114:201::74 prefixlen 128 inet6 fd00:abcd:114:202::74 prefixlen 128 ... and when that client connects, the expectation is that *other* clients can ping these 6 addresses (note it's 200, 201, 202 for v4 and v6). Here's the ccd file for this client: # cat /root/t_server/tun-udp-p2mp/ccd/freebsd-74-amd64 # Host iroute iroute 10.114.200.74 255.255.255.255 iroute-ipv6 fd00:abcd:114:200::74/128 # Subnet-iroute iroute 10.114.201.0 255.255.255.0 iroute-ipv6 fd00:abcd:114:201::/64 # iroute-without-route # 10.114.202, fd00:abcd:114:202::/64 iroute 10.114.202.74 255.255.255.255 iroute-ipv6 fd00:abcd:114:202::74/128 So the "200" routes are "install a host route for IPv4 or IPv6 that is part of an aggregate". This works fine. The "202" routes are "install a host route for IPv4 or IPv6 where there is no covering --route" - this is basically something OpenVPN could not do before DCO (update host routes on-demand on client connect), but is something I've long waited for :-) - this also works fine. Now, the 201 routes are "there whole network specified in --route is routed to a single client". This is not such an unusual setup in cases like "there is a central office OpenVPN server, and one of the branch offices wants it 192.168.77.0/24 reachable from the other clients". This *fails*. For IPv4, we end up with "netstat -rn" looking like this: 10.114.201.0/24 10.114.2.2 UGS tun1 10.114.201.0/24 10.114.2.8 UGS tun1 and "route get" telling us the the first one is preferred $ route get 10.114.201.74 route to: 10.114.201.74 destination: 10.114.201.0 mask: 255.255.255.0 gateway: 10.114.2.2 fib: 0 interface: tun1 so packets loop... (10.114.2.2 is not an address that belongs to an actual client, but "the tun interface peer") the client is 10.114.2.8. $ traceroute 10.114.201.74 traceroute to 10.114.201.74 (10.114.201.74), 64 hops max, 40 byte packets 1 10.114.2.2 (10.114.2.2) 1.706 ms 1.240 ms 0.921 ms 2 localhost (127.0.0.1) 0.865 ms 0.809 ms 0.807 ms 3 10.114.2.2 (10.114.2.2) 1.603 ms 1.605 ms 1.595 ms 4 localhost (127.0.0.1) 1.533 ms 1.383 ms 1.768 ms For IPv6, things look different - route installation fails 2022-08-20 15:47:24 freebsd-74-amd64/2001:608:0:814::f000:3 /sbin/route -6 add -net fd00:abcd:114:201::/64 fd00:abcd:114:2::1006 -fib 0 add net fd00:abcd:114:201::/64: gateway fd00:abcd:114:2::1006 fib 0: route already in table so, the iroute never makes it to kernel, "netstat -rn" remains at fd00:abcd:114:201::/64 link#14 US tun1 and packets loop as well $ traceroute6 fd00:abcd:114:201::74 traceroute6 to fd00:abcd:114:201::74 (fd00:abcd:114:201::74) from fd00:abcd:114:2::1, 64 hops max, 28 byte packets 1 fd00:abcd:114:2::1000 1.749 ms 1.246 ms 0.930 ms 2 fd00:abcd:114:2::1 0.836 ms 1.225 ms 1.067 ms 3 fd00:abcd:114:2::1000 1.555 ms 1.631 ms 1.575 ms 4 fd00:abcd:114:2::1 1.472 ms 1.531 ms 1.677 ms So, what we do on *Linux* to handle this, is to work with route metrics (10.220.* = "same instances, different host, so different address range") linux$ ip route |grep tun1 10.220.200.0/24 via 10.220.2.2 dev tun1 metric 200 10.220.200.74 via 10.220.2.8 dev tun1 metric 100 10.220.201.0/24 via 10.220.2.8 dev tun1 metric 100 10.220.201.0/24 via 10.220.2.2 dev tun1 metric 200 10.220.202.74 via 10.220.2.8 dev tun1 metric 100 linux$ ip -6 route |grep tun1 fd00:abcd:220:200::74 via fd00:abcd:220:2::1006 dev tun1 metric 100 pref medium fd00:abcd:220:200::/64 dev tun1 metric 200 pref medium fd00:abcd:220:201::/64 via fd00:abcd:220:2::1006 dev tun1 metric 100 pref medium fd00:abcd:220:201::/64 dev tun1 metric 200 pref medium fd00:abcd:220:202::74 via fd00:abcd:220:2::1006 dev tun1 metric 100 pref medium so there's always 5 routes in the active table - 2 "--route" routes (according to server.conf) with *metric 200*, and then 3 "--iroute" routes (from ccd/) with metric DCO_IROUTE_METRIC = 100. Linux handles this as would a "Cisco router" do - if there are two identical routes with different metric, the lower-metric route is used, and the other one is ignored. So this works very nicely. Now, back to FreeBSD. - our code does not try to set metrics on FreeBSD - my reading of route(8) does not show me anything in that direction (metric, preference, administrative distance, ...) - I do not understand FreeBSD internals well enough to know whether it can be done at all, or not. If it can not be done, we need to document this as "one of the issues with FreeBSD DCO", and it can of course be easily workarounded in most cases --route 10.114.201.0/24 --iroute 10.114.201.0/25 --iroute 10.114.201.128/25 will get the same thing done "when client is online route the whole /25 down there", but it's harder to get into people's heads :-) (adding code to auto-split such a prefix is technically possible but would be quite a complication, so I'd rather solve this "with documentation"). So, what shall we do? gert
I’ll post a patch. > - any particular reason you used argv_printf() + argv_printf_cat(), > instead of just putting all into a single argv_printf() call? > Mostly that that’s what src/openvpn/networking_iproute2.c does too. I don’t have particularly strong feelings either way, so I’ll add that to the upcoming patch. > - the "openvpn_execve_check()" messages state "route *add* command failed", > while it could be "del" as well. Making this "proper" might not be > worth it, as it would need string manipulation (or "duplicate whole > message"). > It might be nice to turn openvpn_execve_check() into a variadic function so we could do this: status = openvpn_execve_check(&argv, NULL, 0, "ERROR: FreeBSD route %s command failed”, op); But I think I’ll just remove the ‘add’, which makes the error message make sense for both add and del. Best regards, Kristof
On 20 Aug 2022, at 16:33, Gert Doering wrote: >> - iroute installation works for the easy cases (--route in >> server.conf, >> --iroute with a more-specific of that in ccd/). It does not work >> for the nasty cases (--route and --iroute with same netbits). >> >> I will send a followup e-mail with more details on this soon. > > *This* is the bit I want to cover separately, because this e-mail is > going to be long :-) > > So, here's my test cases, to see if I could get DCO to misbehave. > > Server config has: > > # grep ^route server.conf > route 10.114.200.0 255.255.255.0 > route-ipv6 fd00:abcd:114:200::/64 > route 10.114.201.0 255.255.255.0 > route-ipv6 fd00:abcd:114:201::/64 > > ("pull two /24 and two /64 towards the tun interface") > > There is one particular client which has many addresses configured on > its loopback > > lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> metric 0 mtu 16384 > inet 10.114.200.74 netmask 0xffffffff > inet 10.114.201.74 netmask 0xffffffff > inet 10.114.202.74 netmask 0xffffffff > inet6 fd00:abcd:114:200::74 prefixlen 128 > inet6 fd00:abcd:114:201::74 prefixlen 128 > inet6 fd00:abcd:114:202::74 prefixlen 128 > > ... and when that client connects, the expectation is that *other* > clients > can ping these 6 addresses (note it's 200, 201, 202 for v4 and v6). > > Here's the ccd file for this client: > > # cat /root/t_server/tun-udp-p2mp/ccd/freebsd-74-amd64 > # Host iroute > iroute 10.114.200.74 255.255.255.255 > iroute-ipv6 fd00:abcd:114:200::74/128 > # Subnet-iroute > iroute 10.114.201.0 255.255.255.0 > iroute-ipv6 fd00:abcd:114:201::/64 > # iroute-without-route > # 10.114.202, fd00:abcd:114:202::/64 > iroute 10.114.202.74 255.255.255.255 > iroute-ipv6 fd00:abcd:114:202::74/128 > > > So the "200" routes are "install a host route for IPv4 or IPv6 that is > part of an aggregate". This works fine. > > The "202" routes are "install a host route for IPv4 or IPv6 where > there > is no covering --route" - this is basically something OpenVPN could > not > do before DCO (update host routes on-demand on client connect), but is > something I've long waited for :-) - this also works fine. > > Now, the 201 routes are "there whole network specified in --route is > routed to a single client". This is not such an unusual setup in > cases like "there is a central office OpenVPN server, and one of the > branch offices wants it 192.168.77.0/24 reachable from the other > clients". This *fails*. > > For IPv4, we end up with "netstat -rn" looking like this: > > 10.114.201.0/24 10.114.2.2 UGS tun1 > 10.114.201.0/24 10.114.2.8 UGS tun1 > > and "route get" telling us the the first one is preferred > > $ route get 10.114.201.74 > route to: 10.114.201.74 > destination: 10.114.201.0 > mask: 255.255.255.0 > gateway: 10.114.2.2 > fib: 0 > interface: tun1 > > so packets loop... (10.114.2.2 is not an address that belongs to an > actual client, but "the tun interface peer") the client is 10.114.2.8. > > $ traceroute 10.114.201.74 > traceroute to 10.114.201.74 (10.114.201.74), 64 hops max, 40 byte > packets > 1 10.114.2.2 (10.114.2.2) 1.706 ms 1.240 ms 0.921 ms > 2 localhost (127.0.0.1) 0.865 ms 0.809 ms 0.807 ms > 3 10.114.2.2 (10.114.2.2) 1.603 ms 1.605 ms 1.595 ms > 4 localhost (127.0.0.1) 1.533 ms 1.383 ms 1.768 ms > > > For IPv6, things look different - route installation fails > > 2022-08-20 15:47:24 freebsd-74-amd64/2001:608:0:814::f000:3 > /sbin/route -6 add -net fd00:abcd:114:201::/64 fd00:abcd:114:2::1006 > -fib 0 > add net fd00:abcd:114:201::/64: gateway fd00:abcd:114:2::1006 fib 0: > route already in table > > so, the iroute never makes it to kernel, "netstat -rn" remains at > > fd00:abcd:114:201::/64 link#14 US > tun1 > > and packets loop as well > > $ traceroute6 fd00:abcd:114:201::74 > traceroute6 to fd00:abcd:114:201::74 (fd00:abcd:114:201::74) from > fd00:abcd:114:2::1, 64 hops max, 28 byte packets > 1 fd00:abcd:114:2::1000 1.749 ms 1.246 ms 0.930 ms > 2 fd00:abcd:114:2::1 0.836 ms 1.225 ms 1.067 ms > 3 fd00:abcd:114:2::1000 1.555 ms 1.631 ms 1.575 ms > 4 fd00:abcd:114:2::1 1.472 ms 1.531 ms 1.677 ms > > > > So, what we do on *Linux* to handle this, is to work with route > metrics > (10.220.* = "same instances, different host, so different address > range") > > linux$ ip route |grep tun1 > 10.220.200.0/24 via 10.220.2.2 dev tun1 metric 200 > 10.220.200.74 via 10.220.2.8 dev tun1 metric 100 > 10.220.201.0/24 via 10.220.2.8 dev tun1 metric 100 > 10.220.201.0/24 via 10.220.2.2 dev tun1 metric 200 > 10.220.202.74 via 10.220.2.8 dev tun1 metric 100 > linux$ ip -6 route |grep tun1 > fd00:abcd:220:200::74 via fd00:abcd:220:2::1006 dev tun1 metric 100 > pref medium > fd00:abcd:220:200::/64 dev tun1 metric 200 pref medium > fd00:abcd:220:201::/64 via fd00:abcd:220:2::1006 dev tun1 metric 100 > pref medium > fd00:abcd:220:201::/64 dev tun1 metric 200 pref medium > fd00:abcd:220:202::74 via fd00:abcd:220:2::1006 dev tun1 metric 100 > pref medium > > so there's always 5 routes in the active table - 2 "--route" routes > (according to server.conf) with *metric 200*, and then 3 "--iroute" > routes > (from ccd/) with metric DCO_IROUTE_METRIC = 100. > > Linux handles this as would a "Cisco router" do - if there are two > identical routes with different metric, the lower-metric route is > used, and the other one is ignored. So this works very nicely. > > > Now, back to FreeBSD. > > - our code does not try to set metrics on FreeBSD > - my reading of route(8) does not show me anything in that direction > (metric, preference, administrative distance, ...) > - I do not understand FreeBSD internals well enough to know whether > it can be done at all, or not. > It’s rather poorly (i.e. not) documented, but it is possible to set a route metric. It’s called a ‘weight’, and not actually mentioned in the man page. Or shown in netstat output. But: $ sudo route add 172.16.2.0/24 10.0.2.1 add net 172.16.2.0: gateway 10.0.2.1 $ sudo route add 172.16.2.0/24 -weight 2 10.0.2.254 add net 172.16.2.0: gateway 10.0.2.254 So we can add multiple routes for the same network, as long as they have a different gateway. The usual `netstat -rn` doesn’t show the weight. You have to use the json/xml output to get it: $ netstat -rn --libxo json | jq … { "destination": "172.16.2.0/24", "gateway": "10.0.2.1", "flags": "UGS", "flags_pretty": [ "up", "gateway", "static" ], "weight": 1, "interface-name": "bnxt0" }, { "destination": "172.16.2.0/24", "gateway": "10.0.2.254", "flags": "UGS", "flags_pretty": [ "up", "gateway", "static" ], "weight": 2, "interface-name": "bnxt0" }, A higher weight gives a higher priority, so that’s the opposite of what Linux does with metric, I believe. > If it can not be done, we need to document this as "one of the issues > with FreeBSD DCO", and it can of course be easily workarounded in most > cases > > --route 10.114.201.0/24 > --iroute 10.114.201.0/25 > --iroute 10.114.201.128/25 > > will get the same thing done "when client is online route the whole > /25 > down there", but it's harder to get into people's heads :-) (adding > code to auto-split such a prefix is technically possible but would be > quite a complication, so I'd rather solve this "with documentation"). > > So, what shall we do? > If I’m understanding everything correctly it should be relatively simple to extend networking_freebsd.c to also set a weight for each route it installs. We’d have to convert the metric into a weight, which I think we could do as `weight = RT_MAX_WEIGHT - metric` (RT_MAX_WEIGHT is 16777215 /* 3 bytes */). Can you try this? diff --git a/src/openvpn/networking_freebsd.c b/src/openvpn/networking_freebsd.c index 0633dce7..e79af2bf 100644 --- a/src/openvpn/networking_freebsd.c +++ b/src/openvpn/networking_freebsd.c @@ -10,6 +10,8 @@ #if defined(TARGET_FREEBSD) +#include <net/route.h> + static int net_route_v4(const char *op, const in_addr_t *dst, int prefixlen, const in_addr_t *gw, const char *iface, uint32_t table, @@ -23,12 +25,13 @@ net_route_v4(const char *op, const in_addr_t *dst, int prefixlen, _dst = ntohl(*dst); _gw = ntohl(*gw); - argv_printf(&argv, "%s %s -net %s/%d %s -fib %d", + argv_printf(&argv, "%s %s -net %s/%d %s -fib %d -weight %d", ROUTE_PATH, op, inet_ntop(AF_INET, &_dst, buf1, sizeof(buf1)), prefixlen, inet_ntop(AF_INET, &_gw, buf2, sizeof(buf2)), - table); + table, + RT_MAX_WEIGHT - metric); argv_msg(M_INFO, &argv); status = openvpn_execve_check(&argv, NULL, 0, @@ -48,12 +51,13 @@ net_route_v6(const char *op, const struct in6_addr *dst, struct argv argv = argv_new(); bool status; - argv_printf(&argv, "%s -6 %s -net %s/%d %s -fib %d", + argv_printf(&argv, "%s -6 %s -net %s/%d %s -fib %d -weight %d", ROUTE_PATH, op, inet_ntop(AF_INET6, dst, buf1, sizeof(buf1)), prefixlen, inet_ntop(AF_INET6, gw, buf2, sizeof(buf2)), - table); + table, + RT_MAX_WEIGHT - metric); argv_msg(M_INFO, &argv); status = openvpn_execve_check(&argv, NULL, 0, Kristof
Hi, On Mon, Aug 22, 2022 at 02:55:09PM +0200, Kristof Provost wrote: > > Now, back to FreeBSD. > > > > - our code does not try to set metrics on FreeBSD > > - my reading of route(8) does not show me anything in that direction > > (metric, preference, administrative distance, ...) > > - I do not understand FreeBSD internals well enough to know whether > > it can be done at all, or not. > > > It???s rather poorly (i.e. not) documented, but it is possible to set a > route metric. It???s called a ???weight???, and not actually mentioned > in the man page. Or shown in netstat output. > > But: > > $ sudo route add 172.16.2.0/24 10.0.2.1 > add net 172.16.2.0: gateway 10.0.2.1 > $ sudo route add 172.16.2.0/24 -weight 2 10.0.2.254 > add net 172.16.2.0: gateway 10.0.2.254 Oh, this is interesting, and good to know. What is the default weight? "1"? > So we can add multiple routes for the same network, as long as they have > a different gateway. > > The usual `netstat -rn` doesn???t show the weight. You have to use the > json/xml output to get it: > > $ netstat -rn --libxo json | jq > ??? > { > "destination": "172.16.2.0/24", > "gateway": "10.0.2.1", > "flags": "UGS", > "flags_pretty": [ > "up", > "gateway", > "static" > ], > "weight": 1, > "interface-name": "bnxt0" > }, Interesting :-) [..] > A higher weight gives a higher priority, so that???s the opposite of > what Linux does with metric, I believe. Right. Linux metrics are "lower is better", like a routing protocol cost would do. [..] > If I???m understanding everything correctly it should be relatively > simple to extend networking_freebsd.c to also set a weight for each > route it installs. > We???d have to convert the metric into a weight, which I think we could > do as `weight = RT_MAX_WEIGHT - metric` (RT_MAX_WEIGHT is 16777215 /* 3 > bytes */). > > Can you try this? Looks good, might work :-). Will give this a try as soon as time permits - next 2-3 weeks might not work out. gert
On 22 Aug 2022, at 20:16, Gert Doering wrote: > On Mon, Aug 22, 2022 at 02:55:09PM +0200, Kristof Provost wrote: >>> Now, back to FreeBSD. >>> >>> - our code does not try to set metrics on FreeBSD >>> - my reading of route(8) does not show me anything in that direction >>> (metric, preference, administrative distance, ...) >>> - I do not understand FreeBSD internals well enough to know whether >>> it can be done at all, or not. >>> >> It???s rather poorly (i.e. not) documented, but it is possible to set a >> route metric. It???s called a ???weight???, and not actually mentioned >> in the man page. Or shown in netstat output. >> >> But: >> >> $ sudo route add 172.16.2.0/24 10.0.2.1 >> add net 172.16.2.0: gateway 10.0.2.1 >> $ sudo route add 172.16.2.0/24 -weight 2 10.0.2.254 >> add net 172.16.2.0: gateway 10.0.2.254 > > Oh, this is interesting, and good to know. > > What is the default weight? "1"? Yes, the default is RT_DEFAULT_WEIGHT: /usr/include/net/route.h 105:#define RT_DEFAULT_WEIGHT 1 Best regards, Kristof
Hi, FreeBSD DCO has the open issue of "iroute with the same netmask as route, so we want metric/weight/... to differenciate" On Mon, Aug 22, 2022 at 02:55:09PM +0200, Kristof Provost wrote: > But: > > $ sudo route add 172.16.2.0/24 10.0.2.1 > add net 172.16.2.0: gateway 10.0.2.1 > $ sudo route add 172.16.2.0/24 -weight 2 10.0.2.254 > add net 172.16.2.0: gateway 10.0.2.254 I tried patching networking_freebsd.c the way you suggested, and it seems to work "halfway". That is... 2022-10-06 17:26:35 us=684854 freebsd-74-amd64/2001:608:0:814::f000:3 /sbin/route add -net 10.114.201.0/24 10.114.2.2 -fib 0 -weight 16777115 add net 10.114.201.0: gateway 10.114.2.2 fib 0: route already in table 2022-10-06 17:26:35 us=695547 freebsd-74-amd64/2001:608:0:814::f000:3 /sbin/route -6 add -net fd00:abcd:114:201::/64 fd00:abcd:114:2::1000 -fib 0 -weight 16777115 add net fd00:abcd:114:201::/64: gateway fd00:abcd:114:2::1000 fib 0: route already in table ... it takes the command, but still refuses to create the routes. $ netstat -rn --libxo json | jq tells me... { "destination": "10.114.201.0/24", "gateway": "10.114.2.2", "flags": "UGS", "flags_pretty": [ "up", "gateway", "static" ], "weight": 1, "interface-name": "tun1" }, { "destination": "fd00:abcd:114:201::/64", "gateway": "link#5", "flags": "US", "flags_pretty": [ "up", "static" ], "weight": 1, "interface-name": "tun1" }, For IPv6, we definitely have "different weight, different gateway", so I'd expect this to work. For IPv4, this is a bit of a nuisance - the client in question happens to be the *first* client in ipp.txt (= .2), and that's what OpenVPN picks for the "route" gateway too... so, "different weight, same gateway". Now, bumping the "first client" via ipp.txt to the .4 address gives me a whole new sleigh of interesting problems... 2022-10-06 17:36:05 us=447754 freebsd-74-amd64/2001:608:0:814::f000:3 /sbin/route add -net 10.114.200.74/32 10.114.2.4 -fib 0 -weight 16777115 route: writing to routing socket: Network is unreachable add net 10.114.200.74: gateway 10.114.2.4 fib 0: Network is unreachable ... which, I think, is because the interface is flagged as "pointtopoint", and .4 is off-net... $ ifconfig tun1 tun1: flags=8051<UP,POINTOPOINT,RUNNING,MULTICAST> metric 0 mtu 1500 options=80000<LINKSTATE> inet 10.114.2.1 --> 10.114.2.2 netmask 0xffffff00 inet6 fd00:abcd:114:2::1 prefixlen 64 inet6 fe80::250:56ff:fe9c:41b%tun1 prefixlen 64 scopeid 0x5 groups: openvpn So, started out with one solution, now I have two new problems... gert
Hi Gert, On 6 Oct 2022, at 16:39, Gert Doering wrote: > FreeBSD DCO has the open issue of "iroute with the same netmask as > route, so we want metric/weight/... to differenciate" > I’ll try to dig into that, but it’ll be late next week at the earliest. Best regards, Kristof
Hi, On Fri, Oct 07, 2022 at 10:47:08AM +0100, Kristof Provost wrote: > On 6 Oct 2022, at 16:39, Gert Doering wrote: > > FreeBSD DCO has the open issue of "iroute with the same netmask as > > route, so we want metric/weight/... to differenciate" > I???ll try to dig into that, but it???ll be late next week at the earliest. Thanks. I'll dig into the POINTTOPOINT issue... configure a real MULTIPOINT interface for --topology subnet (which is something I've always avoided so far, because "we know the other stuff works") gert
Hi, On Fri, Oct 07, 2022 at 11:56:52AM +0200, Gert Doering wrote: > I'll dig into the POINTTOPOINT issue... configure a real MULTIPOINT > interface for --topology subnet (which is something I've always avoided > so far, because "we know the other stuff works") This is... an interesting problem. Changing the tun(4) stuff to do IFF_BROADCAST for "topology subnet" is straightforward, and makes the whole code much simpler actually (and you can do "ifconfig tun7 1.2.3.4/24" and have it actually do what you expect "here is a subnet, with 255 different next-hop IPs available for routing"). Patch is attached so you can see what I'm talking about :-) Butbut... ovpn(4) does not support the TUNSIFMODE ioctl() to set something that is not IFF_POINTOPOINT, so we fail with 2022-10-11 20:58:28 ioctl(TUNSIFMODE): Can't assign requested address (errno=49) 2022-10-11 20:58:28 DCO device tun14 opened 2022-10-11 20:58:28 /sbin/ifconfig tun14 10.194.3.56/24 mtu 1500 up ifconfig: ioctl (SIOCAIFADDR): Destination address required 2022-10-11 20:58:28 FreeBSD ifconfig failed: external program exited with error status: 1 2022-10-11 20:58:28 Exiting due to fatal error @kp, not sure how to move forward. What we need is a "true on-link subnet", so installation of iroutes to "peer #4" works (which it doesn't today), and I think that means "ovpn(4) needs to learn IFF_BROADCAST". OTOH, in "not peer2peer mode", we only support topology subnet anyway, so maybe it should autoconfigure that, depending on the mode OpenVPN requests (p2p mode -> IFF_POINTOPOINT, p2mp mode -> IFF_BROADCAST), so we don't even need TUNSIFMODE. Looking into if_ovpn.c, this is a bit beyond my understanding of the code, though... gert
Hi, On Tue, Oct 11, 2022 at 09:11:25PM +0200, Gert Doering wrote: > OTOH, in "not peer2peer mode", we only support topology subnet anyway, > so maybe it should autoconfigure that, depending on the mode OpenVPN > requests (p2p mode -> IFF_POINTOPOINT, p2mp mode -> IFF_BROADCAST), > so we don't even need TUNSIFMODE. It's not that easy - there are two factors coming into play here, which are orthogonal. Factor 1: single-peer (client or p2p) vs. multi-peer single-peer -> DCO has only 1 peer, all packets that go into the tun/dco interface are sent out to the single peer ("dumb pipe mode") - exactly like tun(4) behaves If a subnet is configured on the interface, packets to ALL IPs (!= local) in that subnet are sent to the other side. No next-hop lookup is done. multi-peer -> DCO has 0..N peers. Packets that go into the tun/dco interface get mapped according to route next-hop <-> peer[]->vpn_v4 (or v6) address and sent to that peer. If a subnet is configured on the interface (only supported mode :-) ), packets to IP addresses in that subnet that have no mapped-to peer are DROPPED. On Linux this is controlled by openvpn_dco_init() setting dco->ifmode to OVPN_MODE_MP/OVPN_MODE_P2P, and net_iface_new() passing this parameter to kernel DCO. On FreeBSD, OpenVPN does not signal the p2p/p2mp mode to kernel yet (if I am not misreading the code), and if I am reading ovpn_route_peer() in if_ovpn.c correctly, "single-peer mode" will drop packets where a corresponding peer can neither be found by ovpn_find_peer_by_ip, using target IP or Route->Gateway lookup. (*) ovpn_find_peer_by_ip() is using a sequential search, for every single packet, which will perform poorly on servers with 1000s of clients... room for optimization. Factor 2: IFF_POINTOPOINT vs. IFF_BROADCAST This seems to be a *BSD-specific thing, aka "there is nothing in the Linux specific code that seems to bother with this". An Interface can be put to point-to-point mode (by ioctl(TUNSIFMODE) with the IFF_POINTOPOINT flag set). In that case, there is no "on-link" subnet, just "us" and "them", and ifconfig wants two IPv4 addresses as parameter ifconfig tun0 10.2.3.77 10.2.3.1 ... that do not need to be adjacent. It takes a "netmask" argument, but that does not have a real effect. To simulate "subnet" mode, OpenVPN will add a 10.2.3.0/24 -> 10.2.3.1 route. In "dumb pipe mode", this works nicely - the /24 route shoves all packets towards the tun0 interface, and the tun(4) sends them over, without looking. In "openvpn non-dco p2mp server mode" this *also* works nicely, because the tun(4) interface is still in "dumb pipe mode" and OpenVPN will do an iroute lookup in the p2mp server forwarding path, sending to the proper client. In DCO mode with iroutes, this does not work at all anymore. Say, there is a client on 10.2.3.50, using iroute 10.22.33.0/24, so OpenVPN tries to install a route via route add 10.22.33.0/24 10.2.3.50 and FreeBSD *refuses* to install this route, because "10.2.3.50" is not adjacent. In my initial tests I did not notice this, because the only client with iroute was assigned the .2 address, One option that sounds like it would work: always use "remote_addr" (= second parameter to IFF_POINTOPOINT ifconfig) as next-hop for all iroute-originated routes - this would make FreeBSD accept the route, and send packets towards the ovpn(4) interface. Alas, the next-hop would be "remote_addr", not "the right p2mp peer", so the "what peer do I need to send this packet to?" search in ovpn(4) would find the peer on the "remote_addr" IP, not the correct one (= lookup happens in normal routing table). So, it wouldn't work [AND it would add more ugly bits to OpenVPN]. Option #2: set the interface to IFF_BROADCAST, which means "true multi-IP interface, with subnet, and Kernel considers all IPs of that subnet to be adjacent, and acceptable route next-hops". This is what the patch I sent yesterday does to tun(4) interfaces, and the result is amazingly well-behaved - "it just works" and gets *rid* of quite a bit of special-case hack-around code. Want. Unfortunately, this breaks for two reasons with FreeBSD DCO as of today: - no TUNSIFMODE support in ovpn(4), so stuck to IFF_POINTOPOINT mode (I think I can add that, copy-pasting code from if_tuntap.c) - no single-peer mode in ovpn(4) -> this will break "I am on client .5 and want to send packets to client .6" because vpn_v4 peer lookup won't find ".6", and no route towards .1 (server) either, so packet drop. (I'm not sure I can make this happen, but I'll give it a try, following the Linux DCO approach described above) (There is another option here that I think I could mention - we can, of course, do IFF_BROADCAST only on the openvpn p2mp server, and stick to IFF_POINTOPOINT on client/p2p instances. BUT that implies we'd get yet another case for FreeBSD ifconfig - p2p, p2p-simulated subnet, subnet - so the OpenVPN code will get more complex, not simpler - and we still need IFF_BROADCAST support in ovpn(4) - so we can do it properly all the way, I'd say) Note that Aspect 2 is only about IPv4. IPv6 always uses true subnet mode, and all these considerations are of no concern... Phew, what a long mail :-) - hope it clarifies what we're missing today and how to move forward. gert
Hi, people have alreadycomplained at me that I write so long e-mails today, so I can write more... On Wed, Oct 12, 2022 at 08:39:31AM +0200, Gert Doering wrote: > Factor 1: single-peer (client or p2p) vs. multi-peer > > single-peer -> DCO has only 1 peer, all packets that go into the > tun/dco interface are sent out to the single peer > ("dumb pipe mode") - exactly like tun(4) behaves > > If a subnet is configured on the interface, packets to > ALL IPs (!= local) in that subnet are sent to the other > side. No next-hop lookup is done. This is "sort of" handled in if_ovpn.c today ovpn_route_peer(struct ovpn_softc *sc, struct mbuf **m0, const struct sockaddr *dst) { ... /* Shortcut if we're a client (or are a server and have only one client). */ if (sc->peercount == 1) return (ovpn_find_only_peer(sc)); ... so this works for the client, but has one interesting drawback on the server - if there is only a single client connected, the server will send ALL to-be-tunneled packets to that client. As soon as client #2 connects, packets are properly sorted. [..] > Factor 2: IFF_POINTOPOINT vs. IFF_BROADCAST > > This seems to be a *BSD-specific thing, aka "there is nothing in the > Linux specific code that seems to bother with this". I've whacked at if_ovpn.c and dco_freebsd.c a bit now, and I seem to have working code for both ends. I am not a FreeBSD kernel coder, so I have no idea how many behavioural standards I am violating, but it makes "real subnet mode in OpenVPN" work for me, with DCO. Kernel patch attached, OpenVPN patches will follow soonish (outside of this e-mail thread). gert
Hi Gert, I’m still travelling today (and am due for my 5G chip update tomorrow), so it’ll be another day or two before I can look at this in any detail, but at first glance this looks sane. Best regards, Kristof On 12 Oct 2022, at 15:38, Gert Doering wrote: > Hi, > > people have alreadycomplained at me that I write so long e-mails today, > so I can write more... > > On Wed, Oct 12, 2022 at 08:39:31AM +0200, Gert Doering wrote: >> Factor 1: single-peer (client or p2p) vs. multi-peer >> >> single-peer -> DCO has only 1 peer, all packets that go into the >> tun/dco interface are sent out to the single peer >> ("dumb pipe mode") - exactly like tun(4) behaves >> >> If a subnet is configured on the interface, packets to >> ALL IPs (!= local) in that subnet are sent to the other >> side. No next-hop lookup is done. > > This is "sort of" handled in if_ovpn.c today > > ovpn_route_peer(struct ovpn_softc *sc, struct mbuf **m0, > const struct sockaddr *dst) > { > ... > /* Shortcut if we're a client (or are a server and have only one client). */ > if (sc->peercount == 1) > return (ovpn_find_only_peer(sc)); > > > ... so this works for the client, but has one interesting drawback on the > server - if there is only a single client connected, the server will send > ALL to-be-tunneled packets to that client. As soon as client #2 connects, > packets are properly sorted. > > [..] >> Factor 2: IFF_POINTOPOINT vs. IFF_BROADCAST >> >> This seems to be a *BSD-specific thing, aka "there is nothing in the >> Linux specific code that seems to bother with this". > > I've whacked at if_ovpn.c and dco_freebsd.c a bit now, and I seem > to have working code for both ends. I am not a FreeBSD kernel coder, > so I have no idea how many behavioural standards I am violating, > but it makes "real subnet mode in OpenVPN" work for me, with DCO. > > Kernel patch attached, OpenVPN patches will follow soonish (outside > of this e-mail thread). > > 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 gert@greenie.muc.de
On 12 Oct 2022, at 16:38, Gert Doering wrote: > people have alreadycomplained at me that I write so long e-mails today, > so I can write more... > > On Wed, Oct 12, 2022 at 08:39:31AM +0200, Gert Doering wrote: >> Factor 1: single-peer (client or p2p) vs. multi-peer >> >> single-peer -> DCO has only 1 peer, all packets that go into the >> tun/dco interface are sent out to the single peer >> ("dumb pipe mode") - exactly like tun(4) behaves >> >> If a subnet is configured on the interface, packets to >> ALL IPs (!= local) in that subnet are sent to the other >> side. No next-hop lookup is done. > > This is "sort of" handled in if_ovpn.c today > > ovpn_route_peer(struct ovpn_softc *sc, struct mbuf **m0, > const struct sockaddr *dst) > { > ... > /* Shortcut if we're a client (or are a server and have only one client). */ > if (sc->peercount == 1) > return (ovpn_find_only_peer(sc)); > > > ... so this works for the client, but has one interesting drawback on the > server - if there is only a single client connected, the server will send > ALL to-be-tunneled packets to that client. As soon as client #2 connects, > packets are properly sorted. > > [..] >> Factor 2: IFF_POINTOPOINT vs. IFF_BROADCAST >> >> This seems to be a *BSD-specific thing, aka "there is nothing in the >> Linux specific code that seems to bother with this". > > I've whacked at if_ovpn.c and dco_freebsd.c a bit now, and I seem > to have working code for both ends. I am not a FreeBSD kernel coder, > so I have no idea how many behavioural standards I am violating, > but it makes "real subnet mode in OpenVPN" work for me, with DCO. > > Kernel patch attached, OpenVPN patches will follow soonish (outside > of this e-mail thread). > I’m happy with that, and will commit (modulo one small style tweak) that patch in FreeBSD. Best regards, Kristof
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am index 2a139b23..5155a180 100644 --- a/src/openvpn/Makefile.am +++ b/src/openvpn/Makefile.am @@ -88,6 +88,7 @@ openvpn_SOURCES = \ mtu.c mtu.h \ mudp.c mudp.h \ multi.c multi.h \ + networking_freebsd.c \ networking_iproute2.c networking_iproute2.h \ networking_sitnl.c networking_sitnl.h \ networking.h \ diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c index 07dc1087..3ffc56d1 100644 --- a/src/openvpn/dco.c +++ b/src/openvpn/dco.c @@ -599,7 +599,7 @@ void dco_install_iroute(struct multi_context *m, struct multi_instance *mi, struct mroute_addr *addr) { -#if defined(TARGET_LINUX) +#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) if (!dco_enabled(&m->top.options)) { return; @@ -642,13 +642,13 @@ dco_install_iroute(struct multi_context *m, struct multi_instance *mi, &mi->context.c2.push_ifconfig_local, dev, 0, DCO_IROUTE_METRIC); } -#endif /* if defined(TARGET_LINUX) */ +#endif /* if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) */ } void dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi) { -#if defined(TARGET_LINUX) +#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) if (!dco_enabled(&m->top.options)) { return; @@ -681,7 +681,7 @@ dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi) 0, DCO_IROUTE_METRIC); } } -#endif /* if defined(TARGET_LINUX) */ +#endif /* if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) */ } #endif /* defined(ENABLE_DCO) */ diff --git a/src/openvpn/dco_freebsd.h b/src/openvpn/dco_freebsd.h index 3594f229..7de11697 100644 --- a/src/openvpn/dco_freebsd.h +++ b/src/openvpn/dco_freebsd.h @@ -27,6 +27,8 @@ #include "ovpn_dco_freebsd.h" +#define DCO_IROUTE_METRIC 100 + typedef enum ovpn_key_slot dco_key_slot_t; typedef enum ovpn_key_cipher dco_cipher_t; diff --git a/src/openvpn/networking.h b/src/openvpn/networking.h index cf6d39ac..b0b31ea1 100644 --- a/src/openvpn/networking.h +++ b/src/openvpn/networking.h @@ -31,6 +31,9 @@ struct context; #include "networking_sitnl.h" #elif ENABLE_IPROUTE #include "networking_iproute2.h" +#elif defined(TARGET_FREEBSD) +typedef void *openvpn_net_ctx_t; +typedef char openvpn_net_iface_t; #else /* define mock types to ensure code builds on any platform */ typedef void *openvpn_net_ctx_t; @@ -238,7 +241,9 @@ int net_addr_ptp_v4_del(openvpn_net_ctx_t *ctx, const openvpn_net_iface_t *iface, const in_addr_t *local, const in_addr_t *remote); +#endif /* ENABLE_SITNL || ENABLE_IPROUTE */ +#if defined(ENABLE_SITNL) || defined(ENABLE_IPROUTE) || defined(TARGET_FREEBSD) /** * Add a route for an IPv4 address/network * @@ -315,6 +320,10 @@ int net_route_v6_del(openvpn_net_ctx_t *ctx, const struct in6_addr *dst, const openvpn_net_iface_t *iface, uint32_t table, int metric); +#endif /* ENABLE_SITNL || ENABLE_IPROUTE || TARGET_FREEBSD */ + +#if defined(ENABLE_SITNL) || defined(ENABLE_IPROUTE) + /** * Retrieve the gateway and outgoing interface for the specified IPv4 * address/network diff --git a/src/openvpn/networking_freebsd.c b/src/openvpn/networking_freebsd.c new file mode 100644 index 00000000..4e36941e --- /dev/null +++ b/src/openvpn/networking_freebsd.c @@ -0,0 +1,101 @@ +#ifdef HAVE_CONFIG_H +#include "config.h" +#elif defined(_MSC_VER) +#include "config-msvc.h" +#endif +#include "syshead.h" +#include "errlevel.h" +#include "run_command.h" +#include "networking.h" + +#if defined(TARGET_FREEBSD) + +static int +net_route_v4(const char *op, const in_addr_t *dst, int prefixlen, + const in_addr_t *gw, const char *iface, uint32_t table, + int metric) +{ + char buf1[16], buf2[16]; + in_addr_t _dst, _gw; + struct argv argv = argv_new(); + bool status; + + _dst = ntohl(*dst); + _gw = ntohl(*gw); + + argv_printf(&argv, "%s %s", + ROUTE_PATH, op); + argv_printf_cat(&argv, "-net %s/%d %s -fib %d", + inet_ntop(AF_INET, &_dst, buf1, sizeof(buf1)), + prefixlen, + inet_ntop(AF_INET, &_gw, buf2, sizeof(buf2)), + table); + + argv_msg(M_INFO, &argv); + status = openvpn_execve_check(&argv, NULL, 0, + "ERROR: FreeBSD route add command failed"); + + argv_free(&argv); + + return (!status); +} + +static int +net_route_v6(const char *op, const struct in6_addr *dst, + int prefixlen, const struct in6_addr *gw, const char *iface, + uint32_t table, int metric) +{ + char buf1[64], buf2[64]; + struct argv argv = argv_new(); + bool status; + + argv_printf(&argv, "%s -6 %s", + ROUTE_PATH, op); + argv_printf_cat(&argv, "-net %s/%d %s -fib %d", + inet_ntop(AF_INET6, dst, buf1, sizeof(buf1)), + prefixlen, + inet_ntop(AF_INET6, gw, buf2, sizeof(buf2)), + table); + + argv_msg(M_INFO, &argv); + status = openvpn_execve_check(&argv, NULL, 0, + "ERROR: FreeBSD route add command failed"); + + argv_free(&argv); + + return (!status); +} + +int +net_route_v4_add(openvpn_net_ctx_t *ctx, const in_addr_t *dst, int prefixlen, + const in_addr_t *gw, const char *iface, uint32_t table, + int metric) +{ + return net_route_v4("add", dst, prefixlen, gw, iface, table, metric); +} + +int +net_route_v6_add(openvpn_net_ctx_t *ctx, const struct in6_addr *dst, + int prefixlen, const struct in6_addr *gw, const char *iface, + uint32_t table, int metric) +{ + return net_route_v6("add", dst, prefixlen, gw, iface, table, metric); +} + +int +net_route_v4_del(openvpn_net_ctx_t *ctx, const in_addr_t *dst, int prefixlen, + const in_addr_t *gw, const char *iface, uint32_t table, + int metric) +{ + return net_route_v4("del", dst, prefixlen, gw, iface, table, metric); +} + +int +net_route_v6_del(openvpn_net_ctx_t *ctx, const struct in6_addr *dst, + int prefixlen, const struct in6_addr *gw, const char *iface, + uint32_t table, int metric) +{ + return net_route_v6("del", dst, prefixlen, gw, iface, table, metric); +} + +#endif