Am 11.10.18 um 20:41 schrieb Antonio Quartulli: > iproute2 is the first user of the new networking API and > its one of the two currently supported functionalities on > Linux (the other being net-tools). > > This patch simply copies the current code from tun.c/route.c > to networking_ip.c without introducing any funcional > change to the code. >
I skipped the first patch and added notes about the proposed API in this
patch.
> create mode 100644 src/openvpn/networking_ip.h
>
> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
> index 8afc4146..3caad17d 100644
> --- a/src/openvpn/Makefile.am
> +++ b/src/openvpn/Makefile.am
> @@ -80,7 +80,7 @@ openvpn_SOURCES = \
> mtu.c mtu.h \
> mudp.c mudp.h \
> multi.c multi.h \
> - networking.h \
> + networking_ip.c networking_ip.h networking.h \
As ip is not really but a linux specific tool, I think naming this
networking_linuxip.c is better as networking_ip sounds like something
generic.
> ntlm.c ntlm.h \
> occ.c occ.h \
> openssl_compat.h \
> diff --git a/src/openvpn/networking_ip.c b/src/openvpn/networking_ip.c
> new file mode 100644
> index 00000000..ae667a9c
> --- /dev/null
> +++ b/src/openvpn/networking_ip.c
> @@ -0,0 +1,386 @@
> +/*
> + * Networking API implementation for iproute2
> + *
> + * Copyright (C) 2018 Antonio Quartulli <a...@unstable.cc>
I think since you copied/pasted from other files this should also have
the normal copyrights, right?
> +
> +#if defined(TARGET_LINUX) && defined(ENABLE_IPROUTE)
> +
> +#include "syshead.h"
> +
> +#include "networking.h"
> +#include "networking_ip.h"
> +#include "misc.h"
> +#include "openvpn.h"
> +#include "run_command.h"
> +#include "socket.h"
> +
> +#include <stdbool.h>
> +#include <netinet/in.h>
> +
> +int
> +net_ctx_init(struct context *c, openvpn_net_ctx_t *ctx)
> +{
> + ctx->es = NULL;
> + if (c)
> + ctx->es = c->es;
> +
> + return 0;
> +}
I know that calling our execve with es==NULL is valid but should not we
aim here for consistency and always require c to be non NULL aka ASSERT(c)?
> +int
> +net_iface_up(openvpn_net_ctx_t *ctx, const char *iface, bool up)
> +{
> + struct argv argv = argv_new();
> +
> + argv_printf(&argv, "%s link set dev %s %s", iproute_path, iface,
> + up ? "up" : "down");
> + argv_msg(M_INFO, &argv);
> + openvpn_execve_check(&argv, ctx->es, S_FATAL, "Linux ip link set
> failed");
> +
> + argv_reset(&argv);
> +
> + return 0;
> +}
> +
> +int
> +net_iface_mtu_set(openvpn_net_ctx_t *ctx, const char *iface, uint32_t mtu)
> +{
> + struct argv argv = argv_new();
> +
> + argv_printf(&argv, "%s link set dev %s up mtu %d", iproute_path, iface,
> + mtu);
> + argv_msg(M_INFO, &argv);
> + openvpn_execve_check(&argv, ctx->es, S_FATAL, "Linux ip link set
> failed");
> +
> + return 0;
> +}
> +
> +int
> +net_addr_v4_add(openvpn_net_ctx_t *ctx, const char *iface,
> + const in_addr_t *addr, int prefixlen,
> + const in_addr_t *broadcast)
> +{
> + struct argv argv = argv_new();
> +
> + char *addr_str = (char *)print_in_addr_t(*addr, 0, NULL);
> + char *brd_str = (char *)print_in_addr_t(*broadcast, 0, NULL);
> +
> + argv_printf(&argv, "%s addr add dev %s %s/%d broadcast %s", iproute_path,
> + iface, addr_str, prefixlen, brd_str);
> + argv_msg(M_INFO, &argv);
> + openvpn_execve_check(&argv, ctx->es, S_FATAL, "Linux ip addr add
> failed");
> +
> + free(addr_str);
> + free(brd_str);
> +
> + argv_reset(&argv);
> +
> + return 0;
> +}
> +
I initially written a comment that this method removes the ptp variant
of the method but later saw that there is also a ptp variant of the
method. I would suggest to either ranme this method to
net_addr_bcast_v4_add or have a bool ptp paramter and then call out to
two different methods.
> +net_addr_v6_add(openvpn_net_ctx_t *ctx, const char *iface,
> + const struct in6_addr *addr, int prefixlen)
Same comment as for v4.
> +net_addr_v4_del(openvpn_net_ctx_t *ctx, const char *iface,
> + const in_addr_t *addr, int prefixlen)
> +net_addr_v6_del(openvpn_net_ctx_t *ctx, const char *iface,
> + const struct in6_addr *addr, int prefixlen)
>
These methods assume that removing ip on ptp and broadcast is the same
for ptp and boradcast on all platforms, is that really a sane assumption?
Arne
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
