Hello Pieter,
Thanks for the bug report.
On 07/11/14(Fri) 14:35, Pieter Verberne wrote:
> [...]
>
> `ping6 fe80::200:24ff:fecd:7df8%pppoe0` with pf disabled is no problem.
> ping6, with pf enabled and 'set skip on lo0' does not work very well:
>
> --- fe80::200:24ff:fecd:7df8%pppoe0 ping6 statistics ---
> 58 packets transmitted, 3 packets received, 94.8% packet loss
> round-trip min/avg/max/std-dev = 0.320/0.393/0.491/0.072 ms
>
> pf enabled and 'set skip on lo0' NOT set; works perfectly fine.
This is a funky situation caused by what I call the "KAME loopback
hack". The diff below fixes the problem for me, with a lot of ---,
could you give it a go?
Explanation:
What's happening is that in ip6_output() pf is called on the lo0
interface while in ip6_output() it is called on the pppoe0 interface.
So the diff below matches the IPv4 behavior, and since your packet is
not supposed to be send on the wire, always do the check on lo0. This
is done by removing the horrible "origifp" hack (let's call it hack #1).
Now removing hack #1 is not so easy because when the receiving interface
of a packet (rcvif) is a loopback, ip6_intput() expects that the
destination must be an address configured on this interface.
This check is done because the original checks if the interface attached
to the destination route is a loopback (hack #2). So we can simply get
rid of this by checking for RTF_LOCAL instead.
But in order to have a successful route lookup for local+scoped
addresses we need a destination address containing a scopeid. Since we
can no longer recover this scopeid because we're passing lo0 to
ip6_intput() do not clear it in ip6_output(). This is harmless because
the packets won't be send on the wire.
This has been analysed and discussed at length with mikeb@, ok?
Index: netinet6/ip6_forward.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet6/ip6_forward.c,v
retrieving revision 1.69
diff -u -p -r1.69 ip6_forward.c
--- netinet6/ip6_forward.c 14 Oct 2014 09:52:26 -0000 1.69
+++ netinet6/ip6_forward.c 13 Nov 2014 13:51:38 -0000
@@ -91,7 +91,6 @@ ip6_forward(struct mbuf *m, int srcrt)
struct rtentry *rt;
int error = 0, type = 0, code = 0;
struct mbuf *mcopy = NULL;
- struct ifnet *origifp; /* maybe unnecessary */
#ifdef IPSEC
u_int8_t sproto = 0;
struct m_tag *mtag;
@@ -422,36 +421,6 @@ reroute:
* link identifiers, we can do this stuff after making a copy for
* returning an error.
*/
- if ((rt->rt_ifp->if_flags & IFF_LOOPBACK) != 0) {
- /*
- * See corresponding comments in ip6_output.
- * XXX: but is it possible that ip6_forward() sends a packet
- * to a loopback interface? I don't think so, and thus
- * I bark here. (jin...@kame.net)
- * XXX: it is common to route invalid packets to loopback.
- * also, the codepath will be visited on use of ::1 in
- * rthdr. (itojun)
- */
-#if 1
- if (0)
-#else
- if ((rt->rt_flags & (RTF_BLACKHOLE|RTF_REJECT)) == 0)
-#endif
- {
- inet_ntop(AF_INET6, &ip6->ip6_src, src6, sizeof(src6));
- inet_ntop(AF_INET6, &ip6->ip6_dst, dst6, sizeof(dst6));
- printf("ip6_forward: outgoing interface is loopback. "
- "src %s, dst %s, nxt %d, rcvif %s, outif %s\n",
- src6, dst6,
- ip6->ip6_nxt, m->m_pkthdr.rcvif->if_xname,
- rt->rt_ifp->if_xname);
- }
-
- /* we can just use rcvif in forwarding. */
- origifp = m->m_pkthdr.rcvif;
- }
- else
- origifp = rt->rt_ifp;
if (IN6_IS_SCOPE_EMBED(&ip6->ip6_src))
ip6->ip6_src.s6_addr16[1] = 0;
if (IN6_IS_SCOPE_EMBED(&ip6->ip6_dst))
@@ -492,7 +461,7 @@ reroute:
goto freert;
}
- error = nd6_output(rt->rt_ifp, origifp, m, dst, rt);
+ error = nd6_output(rt->rt_ifp, m, dst, rt);
if (error) {
in6_ifstat_inc(rt->rt_ifp, ifs6_out_discard);
ip6stat.ip6s_cantforward++;
Index: netinet6/ip6_input.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.130
diff -u -p -r1.130 ip6_input.c
--- netinet6/ip6_input.c 14 Oct 2014 09:52:26 -0000 1.130
+++ netinet6/ip6_input.c 13 Nov 2014 13:51:38 -0000
@@ -358,32 +358,16 @@ ip6_input(struct mbuf *m)
}
}
- if (IN6_IS_SCOPE_EMBED(&ip6->ip6_src))
- ip6->ip6_src.s6_addr16[1] = htons(ifp->if_index);
- if (IN6_IS_SCOPE_EMBED(&ip6->ip6_dst))
- ip6->ip6_dst.s6_addr16[1] = htons(ifp->if_index);
-
/*
- * We use rt->rt_ifp to determine if the address is ours or not.
- * If rt_ifp is lo0, the address is ours.
- * The problem here is, rt->rt_ifp for fe80::%lo0/64 is set to lo0,
- * so any address under fe80::%lo0/64 will be mistakenly considered
- * local. The special case is supplied to handle the case properly
- * by actually looking at interface addresses
- * (using in6ifa_ifpwithaddr).
+ * If the packet has been received on a loopback interface it
+ * can be destinated to any local address, not necessarily to
+ * an address configured on `ifp'.
*/
- if ((ifp->if_flags & IFF_LOOPBACK) != 0 &&
- IN6_IS_ADDR_LINKLOCAL(&ip6->ip6_dst)) {
- if (!in6ifa_ifpwithaddr(ifp, &ip6->ip6_dst)) {
- icmp6_error(m, ICMP6_DST_UNREACH,
- ICMP6_DST_UNREACH_ADDR, 0);
- /* m is already freed */
- return;
- }
-
- ours = 1;
- deliverifp = ifp;
- goto hbhcheck;
+ if ((ifp->if_flags & IFF_LOOPBACK) == 0) {
+ if (IN6_IS_SCOPE_EMBED(&ip6->ip6_src))
+ ip6->ip6_src.s6_addr16[1] = htons(ifp->if_index);
+ if (IN6_IS_SCOPE_EMBED(&ip6->ip6_dst))
+ ip6->ip6_dst.s6_addr16[1] = htons(ifp->if_index);
}
if (m->m_pkthdr.pf.flags & PF_TAG_DIVERTED) {
@@ -456,18 +440,11 @@ ip6_input(struct mbuf *m)
}
/*
- * Accept the packet if the forwarding interface to the destination
- * according to the routing table is the loopback interface,
- * unless the associated route has a gateway.
- * Note that this approach causes to accept a packet if there is a
- * route to the loopback interface for the destination of the packet.
- * But we think it's even useful in some situations, e.g. when using
- * a special daemon which wants to intercept the packet.
+ * Accept the packet if the route to the destination is marked
+ * as local.
*/
if (ip6_forward_rt.ro_rt &&
- (ip6_forward_rt.ro_rt->rt_flags &
- (RTF_HOST|RTF_GATEWAY)) == RTF_HOST &&
- ip6_forward_rt.ro_rt->rt_ifp->if_type == IFT_LOOP) {
+ ISSET(ip6_forward_rt.ro_rt->rt_flags, RTF_LOCAL)) {
struct in6_ifaddr *ia6 =
ifatoia6(ip6_forward_rt.ro_rt->rt_ifa);
if (ia6->ia6_flags & IN6_IFF_ANYCAST)
Index: netinet6/ip6_output.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.161
diff -u -p -r1.161 ip6_output.c
--- netinet6/ip6_output.c 1 Nov 2014 21:40:39 -0000 1.161
+++ netinet6/ip6_output.c 13 Nov 2014 13:51:38 -0000
@@ -158,14 +158,13 @@ ip6_output(struct mbuf *m0, struct ip6_p
struct inpcb *inp)
{
struct ip6_hdr *ip6;
- struct ifnet *ifp, *origifp = NULL;
+ struct ifnet *ifp;
struct mbuf *m = m0;
int hlen, tlen;
struct route_in6 ip6route;
struct rtentry *rt = NULL;
struct sockaddr_in6 *dst, dstsock;
int error = 0;
- struct in6_ifaddr *ia6 = NULL;
u_long mtu;
int alwaysfrag, dontfrag;
u_int32_t optlen = 0, plen = 0, unfragpartlen = 0;
@@ -572,26 +571,14 @@ reroute:
/*
* then rt (for unicast) and ifp must be non-NULL valid values.
*/
- if (rt) {
- ia6 = ifatoia6(rt->rt_ifa);
+ if (rt)
rt->rt_use++;
- }
if ((flags & IPV6_FORWARDING) == 0) {
/* XXX: the FORWARDING flag can be set for mrouting. */
in6_ifstat_inc(ifp, ifs6_out_request);
}
- /*
- * The outgoing interface must be in the zone of source and
- * destination addresses. We should use ia_ifp to support the
- * case of sending packets to an address of our own.
- */
- if (ia6 != NULL && ia6->ia_ifp)
- origifp = ia6->ia_ifp;
- else
- origifp = ifp;
-
if (rt && !IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst)) {
if (opt && opt->ip6po_nextroute.ro_rt) {
/*
@@ -708,48 +695,17 @@ reroute:
}
}
- /* Fake scoped addresses */
- if ((ifp->if_flags & IFF_LOOPBACK) != 0) {
- /*
- * If source or destination address is a scoped address, and
- * the packet is going to be sent to a loopback interface,
- * we should keep the original interface.
- */
-
- /*
- * XXX: this is a very experimental and temporary solution.
- * We eventually have sockaddr_in6 and use the sin6_scope_id
- * field of the structure here.
- * We rely on the consistency between two scope zone ids
- * of source add destination, which should already be assured
- * Larger scopes than link will be supported in the near
- * future.
- */
- origifp = NULL;
+ /*
+ * If the packet is not going on the wire it can be destinated
+ * to any local address. In this case do not clear its scopes
+ * to let ip6_input() find a matching local route.
+ */
+ if ((ifp->if_flags & IFF_LOOPBACK) == 0) {
if (IN6_IS_SCOPE_EMBED(&ip6->ip6_src))
- origifp = if_get(ntohs(ip6->ip6_src.s6_addr16[1]));
- else if (IN6_IS_SCOPE_EMBED(&ip6->ip6_dst))
- origifp = if_get(ntohs(ip6->ip6_dst.s6_addr16[1]));
- /*
- * XXX: origifp can be NULL even in those two cases above.
- * For example, if we remove the (only) link-local address
- * from the loopback interface, and try to send a link-local
- * address without link-id information. Then the source
- * address is ::1, and the destination address is the
- * link-local address with its s6_addr16[1] being zero.
- * What is worse, if the packet goes to the loopback interface
- * by a default rejected route, the null pointer would be
- * passed to looutput, and the kernel would hang.
- * The following last resort would prevent such disaster.
- */
- if (origifp == NULL)
- origifp = ifp;
- } else
- origifp = ifp;
- if (IN6_IS_SCOPE_EMBED(&ip6->ip6_src))
- ip6->ip6_src.s6_addr16[1] = 0;
- if (IN6_IS_SCOPE_EMBED(&ip6->ip6_dst))
- ip6->ip6_dst.s6_addr16[1] = 0;
+ ip6->ip6_src.s6_addr16[1] = 0;
+ if (IN6_IS_SCOPE_EMBED(&ip6->ip6_dst))
+ ip6->ip6_dst.s6_addr16[1] = 0;
+ }
/*
* If the outgoing packet contains a hop-by-hop options header,
@@ -861,7 +817,7 @@ reroute:
* transmit packet without fragmentation
*/
if (dontfrag || (!alwaysfrag && tlen <= mtu)) { /* case 1-a and 2-a */
- error = nd6_output(ifp, origifp, m, dst, ro->ro_rt);
+ error = nd6_output(ifp, m, dst, ro->ro_rt);
goto done;
}
@@ -949,7 +905,7 @@ reroute:
if (error == 0) {
ip6stat.ip6s_ofragments++;
in6_ifstat_inc(ifp, ifs6_out_fragcreat);
- error = nd6_output(ifp, origifp, m, dst, ro->ro_rt);
+ error = nd6_output(ifp, m, dst, ro->ro_rt);
} else
m_freem(m);
}
Index: netinet6/nd6.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.124
diff -u -p -r1.124 nd6.c
--- netinet6/nd6.c 1 Nov 2014 21:40:39 -0000 1.124
+++ netinet6/nd6.c 13 Nov 2014 13:51:38 -0000
@@ -1489,8 +1489,7 @@ fail:
* we assume ifp is not a p2p here, so just
* set the 2nd argument as the 1st one.
*/
- nd6_output(ifp, ifp, n,
- satosin6(rt_key(rt)), rt);
+ nd6_output(ifp, n, satosin6(rt_key(rt)), rt);
if (ln->ln_hold == n) {
/* n is back in ln_hold. Discard. */
m_freem(ln->ln_hold);
@@ -1651,8 +1650,8 @@ nd6_rs_output_timo(void *ignored_arg)
#define senderr(e) { error = (e); goto bad;}
int
-nd6_output(struct ifnet *ifp, struct ifnet *origifp, struct mbuf *m0,
- struct sockaddr_in6 *dst, struct rtentry *rt0)
+nd6_output(struct ifnet *ifp, struct mbuf *m0, struct sockaddr_in6 *dst,
+ struct rtentry *rt0)
{
struct mbuf *m = m0;
struct rtentry *rt = rt0;
@@ -1827,17 +1826,6 @@ nd6_output(struct ifnet *ifp, struct ifn
mtag = m_tag_find(m, PACKET_TAG_IPSEC_OUT_CRYPTO_NEEDED, NULL);
#endif /* IPSEC */
- if ((ifp->if_flags & IFF_LOOPBACK) != 0) {
-#ifdef IPSEC
- if (mtag != NULL) {
- /* Tell IPsec to do its own crypto. */
- ipsp_skipcrypto_unmark((struct tdb_ident *)(mtag + 1));
- error = EACCES;
- goto bad;
- }
-#endif /* IPSEC */
- return ((*ifp->if_output)(origifp, m, sin6tosa(dst), rt));
- }
#ifdef IPSEC
if (mtag != NULL) {
/* Tell IPsec to do its own crypto. */
Index: netinet6/nd6.h
===================================================================
RCS file: /home/ncvs/src/sys/netinet6/nd6.h,v
retrieving revision 1.40
diff -u -p -r1.40 nd6.h
--- netinet6/nd6.h 10 Nov 2014 10:46:10 -0000 1.40
+++ netinet6/nd6.h 13 Nov 2014 13:51:38 -0000
@@ -283,8 +283,8 @@ void nd6_rtrequest(int, struct rtentry *
int nd6_ioctl(u_long, caddr_t, struct ifnet *);
struct rtentry *nd6_cache_lladdr(struct ifnet *, struct in6_addr *,
char *, int, int, int);
-int nd6_output(struct ifnet *, struct ifnet *, struct mbuf *,
- struct sockaddr_in6 *, struct rtentry *);
+int nd6_output(struct ifnet *, struct mbuf *, struct sockaddr_in6 *,
+ struct rtentry *);
int nd6_storelladdr(struct ifnet *, struct rtentry *, struct mbuf *,
struct sockaddr *, u_char *);
int nd6_sysctl(int, void *, size_t *, void *, size_t);
Index: netinet6/nd6_nbr.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.83
diff -u -p -r1.83 nd6_nbr.c
--- netinet6/nd6_nbr.c 10 Nov 2014 10:46:10 -0000 1.83
+++ netinet6/nd6_nbr.c 13 Nov 2014 13:51:38 -0000
@@ -872,7 +872,7 @@ nd6_na_input(struct mbuf *m, int off, in
* we assume ifp is not a loopback here, so just set the 2nd
* argument as the 1st one.
*/
- nd6_output(ifp, ifp, n, satosin6(rt_key(rt)), rt);
+ nd6_output(ifp, n, satosin6(rt_key(rt)), rt);
if (ln->ln_hold == n) {
/* n is back in ln_hold. Discard. */
m_freem(ln->ln_hold);
