Attention is currently required from: Arne Schwabe.
Hello Arne Schwabe,
I'd like you to do a code review.
Please visit
https://gerrit.openvpn.net/c/openvpn/+/2154?usp=email
to review the following change.
Change subject: route: use the net_ API for routes on FreeBSD and DragonFly
......................................................................
route: use the net_ API for routes on FreeBSD and DragonFly
Add and delete IPv4 and IPv6 routes through net_route_v4/v6_add/del()
on FreeBSD and DragonFly, as on Linux, instead of running route(8)
from route.c. Build networking_freebsd.c for DragonFly as well; -fib
is only passed on FreeBSD.
--route-table stays Linux only. A non-contiguous IPv4 netmask now gets
an error instead of an invalid prefix length. networking_freebsd.c
logs route commands at D_ROUTE, as route.c did. The shared error
messages lose their "Linux" prefix.
Change-Id: I601454a511c9aac4f37d8629b1692266cdbd6885
Signed-off-by: Razvan Cojocaru <[email protected]>
---
M src/openvpn/networking.h
M src/openvpn/networking_freebsd.c
M src/openvpn/route.c
3 files changed, 73 insertions(+), 128 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/54/2154/1
diff --git a/src/openvpn/networking.h b/src/openvpn/networking.h
index 7ad6e2e..4b30092 100644
--- a/src/openvpn/networking.h
+++ b/src/openvpn/networking.h
@@ -30,7 +30,7 @@
#include "networking_sitnl.h"
#elif ENABLE_IPROUTE
#include "networking_iproute2.h"
-#elif defined(TARGET_FREEBSD)
+#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY)
typedef void *openvpn_net_ctx_t;
typedef char openvpn_net_iface_t;
#else /* ifdef ENABLE_SITNL */
@@ -236,7 +236,8 @@
#endif /* ENABLE_SITNL || ENABLE_IPROUTE */
-#if defined(ENABLE_SITNL) || defined(ENABLE_IPROUTE) || defined(TARGET_FREEBSD)
+#if defined(ENABLE_SITNL) || defined(ENABLE_IPROUTE) ||
defined(TARGET_FREEBSD) \
+ || defined(TARGET_DRAGONFLY)
/**
* Add a route for an IPv4 address/network
*
@@ -309,7 +310,7 @@
const struct in6_addr *gw, const openvpn_net_iface_t
*iface, uint32_t table,
int metric);
-#endif /* ENABLE_SITNL || ENABLE_IPROUTE || TARGET_FREEBSD */
+#endif /* ENABLE_SITNL || ENABLE_IPROUTE || TARGET_FREEBSD || TARGET_DRAGONFLY
*/
#if defined(ENABLE_SITNL) || defined(ENABLE_IPROUTE)
diff --git a/src/openvpn/networking_freebsd.c b/src/openvpn/networking_freebsd.c
index e3697d4..0b2b225 100644
--- a/src/openvpn/networking_freebsd.c
+++ b/src/openvpn/networking_freebsd.c
@@ -6,7 +6,7 @@
#include "run_command.h"
#include "networking.h"
-#if defined(TARGET_FREEBSD)
+#if defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY)
static int
net_route_v4(const char *op, const in_addr_t *dst, int prefixlen, const
in_addr_t *gw,
@@ -26,6 +26,7 @@
*/
argv_printf(&argv, "%s %s -net %s/%d", ROUTE_PATH, op,
inet_ntop(AF_INET, &_dst, buf1, sizeof(buf1)), prefixlen);
+ /* FIXME -- add on-link support: iface is ignored when gw is set */
if (gw)
{
_gw = ntohl(*gw);
@@ -35,14 +36,22 @@
{
argv_printf_cat(&argv, "-iface %s", iface);
}
+#if 0
+ if (metric >= 0)
+ {
+ argv_printf_cat(&argv, "-rtt %d", metric);
+ }
+#endif
+#if defined(TARGET_FREEBSD)
/* without -fib, route(8) uses the FIB of the process */
if (table)
{
argv_printf_cat(&argv, "-fib %u", table);
}
+#endif
- argv_msg(M_INFO, &argv);
- bool status = openvpn_execve_check(&argv, NULL, 0, "ERROR: FreeBSD route
command failed");
+ argv_msg(D_ROUTE, &argv);
+ bool status = openvpn_execve_check(&argv, NULL, 0, "ERROR: route command
failed");
argv_free(&argv);
@@ -77,13 +86,15 @@
{
argv_printf_cat(&argv, "-iface %s", iface);
}
+#if defined(TARGET_FREEBSD)
if (table)
{
argv_printf_cat(&argv, "-fib %u", table);
}
+#endif
- argv_msg(M_INFO, &argv);
- bool status = openvpn_execve_check(&argv, NULL, 0, "ERROR: FreeBSD route
command failed");
+ argv_msg(D_ROUTE, &argv);
+ bool status = openvpn_execve_check(&argv, NULL, 0, "ERROR: route command
failed");
argv_free(&argv);
@@ -118,4 +129,4 @@
return net_route_v6("delete", dst, prefixlen, gw, iface, table, metric);
}
-#endif /* if defined(TARGET_FREEBSD) */
+#endif /* if defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) */
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 53550c4..abf6152 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -103,6 +103,18 @@
#define RTA_SUCCESS 1 /* route addition succeeded */
#define RTA_EEXIST 2 /* route not added as it already exists */
+/* Platforms that add and delete routes through the net_ API (networking.h) */
+#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) ||
defined(TARGET_DRAGONFLY)
+#define ROUTE_NET_API
+#endif
+
+/* --route-table is supported only on Linux */
+#if defined(TARGET_LINUX)
+#define ROUTE_TABLE_ID(id) (id)
+#else
+#define ROUTE_TABLE_ID(id) 0
+#endif
+
#ifndef TARGET_ANDROID
static bool
add_bypass_address(struct route_bypass *rb, const in_addr_t a)
@@ -1436,7 +1448,7 @@
/* Return true if the "on-link" form of the route should be used. This is
when the gateway for
* a route is specified as an interface rather than an address. */
-#if defined(TARGET_LINUX) || defined(_WIN32) || defined(TARGET_DARWIN)
+#if defined(ROUTE_NET_API) || defined(_WIN32) || defined(TARGET_DARWIN)
static inline bool
is_on_link(const int is_local_route, const unsigned int flags, const struct
route_gateway_info *rgi)
{
@@ -1462,7 +1474,7 @@
struct argv argv = argv_new();
struct gc_arena gc = gc_new();
-#if !defined(TARGET_LINUX)
+#if !defined(ROUTE_NET_API)
const char *network = print_in_addr_t(r->network, 0, &gc);
#if !defined(TARGET_AIX)
const char *netmask = print_in_addr_t(r->netmask, 0, &gc);
@@ -1476,7 +1488,7 @@
goto done;
}
-#if defined(TARGET_LINUX)
+#if defined(ROUTE_NET_API)
const char *iface = NULL;
int metric = -1;
@@ -1490,18 +1502,26 @@
metric = r->metric;
}
+ const int netbits = netmask_to_netbits2(r->netmask);
+ if (netbits < 0)
+ {
+ msg(M_WARN, "ERROR: cannot add route with non-contiguous netmask %s",
+ print_in_addr_t(r->netmask, 0, &gc));
+ status = RTA_ERROR;
+ goto done;
+ }
status = RTA_SUCCESS;
- int ret = net_route_v4_add(ctx, &r->network,
netmask_to_netbits2(r->netmask), &r->gateway,
- iface, r->table_id, metric);
+ int ret = net_route_v4_add(ctx, &r->network, netbits, &r->gateway, iface,
+ ROUTE_TABLE_ID(r->table_id), metric);
if (ret == -EEXIST)
{
- msg(D_ROUTE, "NOTE: Linux route add command failed because route
exists");
+ msg(D_ROUTE, "NOTE: route add command failed because route exists");
status = RTA_EEXIST;
}
else if (ret < 0)
{
- msg(M_WARN, "ERROR: Linux route add command failed");
+ msg(M_WARN, "ERROR: route add command failed");
status = RTA_ERROR;
}
@@ -1605,44 +1625,6 @@
bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route add
command failed");
status = ret ? RTA_SUCCESS : RTA_ERROR;
-#elif defined(TARGET_FREEBSD)
-
- argv_printf(&argv, "%s add", ROUTE_PATH);
-
-#if 0
- if (r->flags & RT_METRIC_DEFINED)
- {
- argv_printf_cat(&argv, "-rtt %d", r->metric);
- }
-#endif
-
- argv_printf_cat(&argv, "-net %s %s %s", network, gateway, netmask);
-
- /* FIXME -- add on-link support for FreeBSD */
-
- argv_msg(D_ROUTE, &argv);
- bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: FreeBSD route add
command failed");
- status = ret ? RTA_SUCCESS : RTA_ERROR;
-
-#elif defined(TARGET_DRAGONFLY)
-
- argv_printf(&argv, "%s add", ROUTE_PATH);
-
-#if 0
- if (r->flags & RT_METRIC_DEFINED)
- {
- argv_printf_cat(&argv, "-rtt %d", r->metric);
- }
-#endif
-
- argv_printf_cat(&argv, "-net %s %s %s", network, gateway, netmask);
-
- /* FIXME -- add on-link support for Dragonfly */
-
- argv_msg(D_ROUTE, &argv);
- bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: DragonFly route add
command failed");
- status = ret ? RTA_SUCCESS : RTA_ERROR;
-
#elif defined(TARGET_DARWIN)
argv_printf(&argv, "%s add", ROUTE_PATH);
@@ -1708,10 +1690,10 @@
bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: Haiku inet route add
command failed");
status = ret ? RTA_SUCCESS : RTA_ERROR;
-#else /* if defined(TARGET_LINUX) */
+#else /* if defined(ROUTE_NET_API) */
msg(M_FATAL,
"Sorry, but I don't know how to do 'route' commands on this operating
system. Try putting your routes in a --route-up script");
-#endif /* if defined(TARGET_LINUX) */
+#endif /* if defined(ROUTE_NET_API) */
done:
if (status == RTA_SUCCESS)
@@ -1786,8 +1768,7 @@
const char *network = print_in6_addr(r6->network, 0, &gc);
const char *gateway = print_in6_addr(r6->gateway, 0, &gc);
-#if defined(TARGET_DARWIN) || defined(TARGET_FREEBSD) ||
defined(TARGET_DRAGONFLY) \
- || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD)
+#if defined(TARGET_DARWIN) || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD)
/* the BSD platforms cannot specify gateway and interface independently,
* but for link-local destinations, we MUST specify the interface, so
@@ -1839,7 +1820,7 @@
goto done;
}
-#if defined(TARGET_LINUX)
+#if defined(ROUTE_NET_API)
int metric = -1;
if ((r6->flags & RT_METRIC_DEFINED) && (r6->metric > 0))
{
@@ -1848,15 +1829,15 @@
status = RTA_SUCCESS;
int ret = net_route_v6_add(ctx, &r6->network, r6->netbits, gateway_needed
? &r6->gateway : NULL,
- device, r6->table_id, metric);
+ device, ROUTE_TABLE_ID(r6->table_id), metric);
if (ret == -EEXIST)
{
- msg(D_ROUTE, "NOTE: Linux route add command failed because route
exists");
+ msg(D_ROUTE, "NOTE: route add command failed because route exists");
status = RTA_EEXIST;
}
else if (ret < 0)
{
- msg(M_WARN, "ERROR: Linux route add command failed");
+ msg(M_WARN, "ERROR: route add command failed");
status = RTA_ERROR;
}
@@ -1902,23 +1883,6 @@
bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route add
-inet6 command failed");
status = ret ? RTA_SUCCESS : RTA_ERROR;
-#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY)
-
- argv_printf(&argv, "%s add -inet6 %s/%d", ROUTE_PATH, network,
r6->netbits);
-
- if (gateway_needed)
- {
- argv_printf_cat(&argv, "%s", gateway);
- }
- else
- {
- argv_printf_cat(&argv, "-iface %s", device);
- }
-
- argv_msg(D_ROUTE, &argv);
- bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: *BSD route add
-inet6 command failed");
- status = ret ? RTA_SUCCESS : RTA_ERROR;
-
#elif defined(TARGET_DARWIN)
argv_printf(&argv, "%s add -inet6 %s -prefixlen %d", ROUTE_PATH, network,
r6->netbits);
@@ -1969,10 +1933,10 @@
bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: Haiku inet6 route
add command failed");
status = ret ? RTA_SUCCESS : RTA_ERROR;
-#else /* if defined(TARGET_LINUX) */
+#else /* if defined(ROUTE_NET_API) */
msg(M_FATAL,
"Sorry, but I don't know how to do 'route ipv6' commands on this
operating system. Try putting your routes in a --route-up script");
-#endif /* if defined(TARGET_LINUX) */
+#endif /* if defined(ROUTE_NET_API) */
done:
if (status == RTA_SUCCESS)
@@ -1995,13 +1959,13 @@
delete_route(struct route_ipv4 *r, const struct tuntap *tt, unsigned int flags,
const struct route_gateway_info *rgi, const struct env_set *es,
openvpn_net_ctx_t *ctx)
{
-#if !defined(TARGET_LINUX)
+#if !defined(ROUTE_NET_API)
const char *network;
#if !defined(TARGET_AIX)
const char *netmask;
#endif
const char *gateway;
-#else /* if !defined(TARGET_LINUX) */
+#else /* if !defined(ROUTE_NET_API) */
int metric;
#endif
int is_local_route;
@@ -2014,7 +1978,7 @@
struct gc_arena gc = gc_new();
struct argv argv = argv_new();
-#if !defined(TARGET_LINUX)
+#if !defined(ROUTE_NET_API)
network = print_in_addr_t(r->network, 0, &gc);
#if !defined(TARGET_AIX)
netmask = print_in_addr_t(r->netmask, 0, &gc);
@@ -2028,7 +1992,7 @@
goto done;
}
-#if defined(TARGET_LINUX)
+#if defined(ROUTE_NET_API)
metric = -1;
if (r->flags & RT_METRIC_DEFINED)
{
@@ -2036,10 +2000,10 @@
}
if (net_route_v4_del(ctx, &r->network, netmask_to_netbits2(r->netmask),
&r->gateway, NULL,
- r->table_id, metric)
+ ROUTE_TABLE_ID(r->table_id), metric)
< 0)
{
- msg(M_WARN, "ERROR: Linux route delete command failed");
+ msg(M_WARN, "ERROR: route delete command failed");
}
#elif defined(_WIN32)
@@ -2089,20 +2053,6 @@
argv_msg(D_ROUTE, &argv);
openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route delete command
failed");
-#elif defined(TARGET_FREEBSD)
-
- argv_printf(&argv, "%s delete -net %s %s %s", ROUTE_PATH, network,
gateway, netmask);
-
- argv_msg(D_ROUTE, &argv);
- openvpn_execve_check(&argv, es, 0, "ERROR: FreeBSD route delete command
failed");
-
-#elif defined(TARGET_DRAGONFLY)
-
- argv_printf(&argv, "%s delete -net %s %s %s", ROUTE_PATH, network,
gateway, netmask);
-
- argv_msg(D_ROUTE, &argv);
- openvpn_execve_check(&argv, es, 0, "ERROR: DragonFly route delete command
failed");
-
#elif defined(TARGET_DARWIN)
if (is_on_link(is_local_route, flags, rgi))
@@ -2150,10 +2100,10 @@
argv_msg(D_ROUTE, &argv);
openvpn_execve_check(&argv, es, 0, "ERROR: Haiku inet route delete command
failed");
-#else /* if defined(TARGET_LINUX) */
+#else /* if defined(ROUTE_NET_API) */
msg(M_FATAL,
"Sorry, but I don't know how to do 'route' commands on this operating
system. Try putting your routes in a --route-up script");
-#endif /* if defined(TARGET_LINUX) */
+#endif /* if defined(ROUTE_NET_API) */
done:
r->flags &= ~RT_ADDED;
@@ -2175,7 +2125,7 @@
}
#if !defined(_WIN32)
-#if !defined(TARGET_LINUX)
+#if !defined(ROUTE_NET_API)
const char *gateway;
#endif
#if !defined(TARGET_SOLARIS)
@@ -2202,12 +2152,11 @@
struct argv argv = argv_new();
network = print_in6_addr(r6->network, 0, &gc);
-#if !defined(TARGET_LINUX) && !defined(_WIN32)
+#if !defined(ROUTE_NET_API) && !defined(_WIN32)
gateway = print_in6_addr(r6->gateway, 0, &gc);
#endif
-#if defined(TARGET_DARWIN) || defined(TARGET_FREEBSD) ||
defined(TARGET_DRAGONFLY) \
- || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD)
+#if defined(TARGET_DARWIN) || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD)
/* the BSD platforms cannot specify gateway and interface independently,
* but for link-local destinations, we MUST specify the interface, so
@@ -2225,7 +2174,7 @@
msg(D_ROUTE, "delete_route_ipv6(%s/%u)", network, r6->netbits);
-#if defined(TARGET_LINUX)
+#if defined(ROUTE_NET_API)
int metric = -1;
if ((r6->flags & RT_METRIC_DEFINED) && (r6->metric > 0))
{
@@ -2233,10 +2182,10 @@
}
if (net_route_v6_del(ctx, &r6->network, r6->netbits, gateway_needed ?
&r6->gateway : NULL,
- device, r6->table_id, metric)
+ device, ROUTE_TABLE_ID(r6->table_id), metric)
< 0)
{
- msg(M_WARN, "ERROR: Linux route v6 delete command failed");
+ msg(M_WARN, "ERROR: route v6 delete command failed");
}
#elif defined(_WIN32)
@@ -2258,22 +2207,6 @@
argv_msg(D_ROUTE, &argv);
openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route delete -inet6
command failed");
-#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY)
-
- argv_printf(&argv, "%s delete -inet6 %s/%d", ROUTE_PATH, network,
r6->netbits);
-
- if (gateway_needed)
- {
- argv_printf_cat(&argv, "%s", gateway);
- }
- else
- {
- argv_printf_cat(&argv, "-iface %s", device);
- }
-
- argv_msg(D_ROUTE, &argv);
- openvpn_execve_check(&argv, es, 0, "ERROR: *BSD route delete -inet6
command failed");
-
#elif defined(TARGET_DARWIN)
argv_printf(&argv, "%s delete -inet6 %s -prefixlen %d", ROUTE_PATH,
network, r6->netbits);
@@ -2326,10 +2259,10 @@
argv_msg(D_ROUTE, &argv);
openvpn_execve_check(&argv, es, 0, "ERROR: Haiku inet6 route delete
command failed");
-#else /* if defined(TARGET_LINUX) */
+#else /* if defined(ROUTE_NET_API) */
msg(M_FATAL,
"Sorry, but I don't know how to do 'route ipv6' commands on this
operating system. Try putting your routes in a --route-down script");
-#endif /* if defined(TARGET_LINUX) */
+#endif /* if defined(ROUTE_NET_API) */
argv_free(&argv);
gc_free(&gc);
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/2154?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I601454a511c9aac4f37d8629b1692266cdbd6885
Gerrit-Change-Number: 2154
Gerrit-PatchSet: 1
Gerrit-Owner: Răzvan Cojocaru <[email protected]>
Gerrit-Reviewer: Arne Schwabe <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
