Am 11.10.18 um 20:41 schrieb Antonio Quartulli: > This patch introduces a tiny netlink interface, optimized > for the openvpn use case. > > It basically exposes all those operations that are currently > handled by directly calling the /sbin/ip command (or even > ifconfig/route, if configured). > > By using netlink, openvpn won't need to spawn new processes > when configuring the tun interface or routes. > This new approach will also allow openvpn to be granted > CAP_NET_ADMIN and be able to properly work even though it > dropped the root privileges (currently handled via workarounds). > > By moving this logic into the sitnl module, tun.c and route.c > also benefit from some code simplification
Just FYI for the rest. The current code still has a bug handling ipv6
and I am waiting for V3 that does not have this bug.
So this is not a full review
> +typedef union {
> + in_addr_t ipv4;
> + struct in6_addr ipv6;
> +} inet_address_t;
> +
> +/**
> + * Link state request message
> + */
> +struct sitnl_link_req {
> + struct nlmsghdr n;
I think one two sentences that netlink requires messages in a certain
format but has no standard header that defines them so we need to define
the message ourselves would be good here.
> +
> +/* if (((int)nladdr.nl_pid != peer) || (h->nlmsg_pid !=
> nladdr.nl_pid)
> + || (h->nlmsg_seq != seq))
> + {
> + rcv_len -= NLMSG_ALIGN(len);
> + h = (struct nlmsghdr *)((char *)h + NLMSG_ALIGN(len));
> + msg(M_DEBUG, "%s: skipping unrelated message. nl_pid:%d
> (peer:%d) nl_msg_pid:%d nl_seq:%d seq:%d",
> + __func__, (int)nladdr.nl_pid, peer, h->nlmsg_pid,
> + h->nlmsg_seq, seq);
> + continue;
> + }
> +*/
The "normal" style for these debug commented out parts is using #ifdef
instead of comments.
> +typedef struct {
> + int addr_size;
> + inet_address_t gw;
> + char iface[IFNAMSIZ];
> +} route_res_t;
> +
This struct is in the middle of the file and I think it should either go
along with the other struct definition in the top of the file or all
structs should be defined before the function that they are used in first.
> +/* used by iproute2 implementation too */
I would not add that comment. Where a function is used can be inferred
pretty easily and we don't have such comments elsewhere. And I fear this
comment might be overlooked when iproute2 is remove in 2.6/2.7 and then
more confusing than helpful.
Arne
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
