[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/51280: if_srt fails to correctly route IPv6 packets (+ potential fix)
On Mon, Jun 27, 2016 at 10:20 AM, <kre%munnari.oz.au@localhost> wrote:
>>Synopsis: if_srt fails to correctly route IPv6 packets (+ potential fix)
>>Arrival-Date: Mon Jun 27 01:20:01 +0000 2016
>>Originator: Robert Elz
>>Release: NetBSD 7.99.32 (and everything since srt(4) was added)
> This is the system sending the PR ... Not useful info...
> System: NetBSD andromeda.noi.kre.to 7.99.26 NetBSD 7.99.26 (VBOX64-1.1-20160128) #43: Thu Jan 28 16:09:08 ICT 2016 kre%onyx.coe.psu.ac.th@localhost:/usr/obj/current/kernels/amd64/VBOX64 amd64
> Architecture: x86_64
> Machine: amd64
> if_srt (srt(4)) enables routing based upon the source address
> of the packets - when packets are routed to it (via normal routing
> mechanisms, usually a default route aimed at the srt interface)
> it matches the source addr against a configured list of (addr,prefixlen)
> pairs and if a match is found, sends the packet a configured dest
> addr via another interface.
> For ipv4 sending a packet is done by calling the interface
> ip_output routine, and that is what if_srt does. But for
> ipv6 packet output is done by sending the packet via nd6_output()
> which then calls the ip_output routine after naighbour discovery
> is performed. if_srt doesn't do that.
> Once that is fixed, there is another issue ... ipv6 routes are
> generally via link local addresses (for many reasons not relevant
> here). In my case, on a system with 2 interfaces (not a router,
> just a multi-homed host) the router on one of the links has no
> global addr available to use, only link local. The issue
> here is that in the KAME IPv6 stack, inside the kernel, link
> local addresses have the interface-id embedded in them (a truly
> ugly hack) and Neighbour Discovery expects link local addresses
> it sees to contain that embedded scope-id (interface id), and
> fails if it is not present. Since the interface id is not
> really exposed to userland, it would be difficult to have the
> srt config tool (srtconfig(8)) embed that ID in link local
> addresses it configures - and in any case, this is an in-kernel
> hack, and should be confined there.
> Hence we also need a fix to detect when a srt destination address
> is link local, and embed the scope-id in the address when it is.
> Attempt to use srt to source route IPv6 packets (required in the
> setup I have, as without it, packets addressed to the node via
> one of its addresses, coming from random internet sources, all
> get replies sent via the default interface (the interface to
> which the default route is attached). In general this works,
> it produces (slightly) asymmetric routing, but that's not all
> that unusual. Unfortunately, there are firewalls (outside my
> contro) on the links, and they do not at all like seeing just
> half of a TCP connection - and cause such things to fail.
> Watch it fail...
If there is a list of commands to reproduce, that's helpful.
> The patch below is intended for review by those more knowledgeable
> of the IPv6 stack (and NetBSD networking in general) than I am.
> There are 3 segments to the patch.
> The first just adds the necessary include files to make the
> rest of it compile...
> The second runs nd6_output() instead if ifp->if_output (via the
> new locking interface) for IPv6 packets (the locking stuff then
> gets done in nd6_output). This piece I am reasonably confident
> is correct (the XXX comment that just precedes the change applies
> to both output calls now.)
> The third does the link local address embedding stuff. I know
> it is only half done (in the sense, that the embedded scope remains
> visible when the config tool requests the current config and displays
> it - that doesn't stop anything working, and it makes it easier to
> debug the rest of this, though it is not really correct, and is ugly)
> and also is probably not really the right way to do this - rather than
> just using the if_index as the scope id (or zone id, this thing seems
> unsure what it really should be called) it should probably really
> use the data explicitly added to each interface to contain such things,
> but for link locals, the zone id and interface index are always the
> same, and link locals (or multicast link locals, but a multicast
> dest addr makes no sense) are the only address forms that
> in6_setzoneid() modifies, so the shortcut seems reasonable to me.
> Index: if_srt.c
> RCS file: /cvsroot/src/sys/net/if_srt.c,v
> retrieving revision 1.22
> diff -u -r1.22 if_srt.c
> --- if_srt.c 20 Jun 2016 06:46:37 -0000 1.22
> +++ if_srt.c 27 Jun 2016 00:29:36 -0000
> @@ -33,6 +33,9 @@
> #include <sys/ioctl.h>
> #include <netinet/ip.h>
> #include <netinet/ip6.h>
> +#include <netinet6/in6_var.h>
> +#include <netinet6/nd6.h>
> +#include <netinet6/scope6_var.h>
> #include <net/if_types.h>
> #include "if_srt.h"
> @@ -232,6 +235,8 @@
> return 0; /* XXX ENETDOWN? */
> /* XXX is 0 the right last arg here? */
> + if (to->sa_family == AF_INET6)
> + return nd6_output(r->u.dstifp, r->u.dstifp, m, &r->dst.sin6, 0);
I think it's okay. (Though it might be better we make if_output_lock
work for IPv6 as well somehow. Both FreeBSD and OpenBSD do so now.)
> return if_output_lock(r->u.dstifp, r->u.dstifp, m, &r->dst.sa, 0);
> @@ -432,6 +437,8 @@
> scr->srcmask = dr->srcmask;
> scr->u.dstifp = ifp;
> + if (dr->af == AF_INET6)
> + in6_setzoneid(&scr->dst.sin6.sin6_addr, ifp->if_index);
in6_setscope may suit more?
And I think both pieces need #ifdef INET6.
> return 0;
> case SRT_DELRT:
Main Index |
Thread Index |