Hi, On Fri, Aug 12, 2016 at 08:24:35PM +0200, David Sommerseth wrote: > On 04/01/16 13:43, Lev Stipakov wrote: > > v2: better method naming > > > > On certain OSes (Windows, OS X) when network adapter is disabled > > (ethernet cable pulled off, Wi-Fi hardware switch disabled), > > operating system starts to use tun as an external interface. > > Outgoing packets are routed to tun, UDP encapsulated, given to > > routing table and sent to.. tun. > > Can this happen on Linux or *BSD? Could this recursive routing > actually happen at all?
Of course :-) - the moment you lose your external interface, for
example, because you unplug the network cable and network manager
deconfigures ip address plus all routes via that interface...
What is left is "routes into the tunnel", so you send the tunneled
packets into the tunnel again. Without that patch, this leads to
"openvpn eats 100% cpu, your VPN is down, and until ping-timeout
fires, it won't recover and the user has no idea why it is broken".
Very typical problem with routing and tunnels :-)
>
> > + + /* skip ipv4 packets for ipv6 tun */ + if
> > (tun_sa.addr.sa.sa_family != AF_INET) + return;
>
> [...snip...]
>
> > + + /* skip ipv6 packets for ipv4 tun */ + if
> > (tun_sa.addr.sa.sa_family != AF_INET6) + return;
>
> How likely is it that these two checks will hit? Do we truly need
> them?
It's quite likely for IPv6 packets to be transported over an IPv4
tunnel, and vice versa, and will be even *more* likely for the next
10 years to come.
So comparing "inside IPv6 address" to "outside IPv4 address" needs
to be avoided (at best, the code might misfire, at worst, you might
overrun comparing an IPv6 address to something that has not allocated
enough bytes) - maybe it can be written more efficient, but in the end
you always have the "is it the same protocol inside and outside?"
check, before you go on comparing addresses.
Right now, the code is nicely readable by using the is_ipv4() macro
etc., but sacrificing this by having a check very early on that
is the unrolled form of the macro, and basically does
if ( outside_address_family != inside_address_family ) { return; }
would indeed be more efficient for the mixed-family case.
OTOH, you need another comparison then for "is it IPv4? is it IPv6?",
so the overall amount of code for the same-family case might be even
higher.
OTOH again, is_ipv4() boils down to an amazing amount of code... so
moving *that* from proto.c to proto.h and make it an inline function
(and hope that the compiler will be smart enough to keep track of all
the then-duplicate checks for is_ipv4() vs. is_ipv6() and just optimize
it properly) might produce more efficient *and* more readable code :-)
(and since it's only called in two places ever, the extra code size
due to inlining isn't big)
> I'm basically concerned of the potential cost an extra check
> adds. As the link speed increases, such checks will have an impact
> later on if now. If truly needed, is it worth looking into providing
> some branch prediction hints to the compiler? (unlikely/likely hints)
For the "protocol variants", you can't, because it depends on what
users do with their tunnels. Today, they might be predominantly
IPv4-over-IPv4, while my personal sessions are mostly IPv6-over-IPv4
already...
> > if (c->c2.buf.len > 0) { + drop_if_recursive_routing (c,
> > &c->c2.buf);
>
> If this is truly only an issue on Windows and OSX, can we consider to
> #ifdef the drop_if_recursive_routing() call?
It can happen on all platforms where "inside" and "outside" routing
(wrt openvpn) happens in the same routing context / namespace / ... and
"some external entity" can modify the "outside" routing table without
OpenVPN knowing. In other words: on all platforms we support today.
gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025 g...@net.informatik.tu-muenchen.de
signature.asc
Description: PGP signature
------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. http://sdm.link/zohodev2dev
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
