On 13/11/14(Thu) 15:28, Martin Pieuchot wrote:
> 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?
sthen@ pointed out I forgot a chunk in the previous diff, here's an
updated one:
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 14:55:40 -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. ([email protected])
- * 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 14:55:40 -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 14:55:40 -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 14:55:40 -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 14:55:40 -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 14:55:40 -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);
Index: net/pf.c
===================================================================
RCS file: /home/ncvs/src/sys/net/pf.c,v
retrieving revision 1.891
diff -u -p -r1.891 pf.c
--- net/pf.c 11 Nov 2014 11:37:05 -0000 1.891
+++ net/pf.c 13 Nov 2014 14:55:40 -0000
@@ -5793,7 +5793,7 @@ pf_route6(struct mbuf **m, struct pf_rul
if (IN6_IS_SCOPE_EMBED(&dst->sin6_addr))
dst->sin6_addr.s6_addr16[1] = htons(ifp->if_index);
if ((u_long)m0->m_pkthdr.len <= ifp->if_mtu) {
- nd6_output(ifp, ifp, m0, dst, NULL);
+ nd6_output(ifp, m0, dst, NULL);
} else {
in6_ifstat_inc(ifp, ifs6_in_toobig);
if (r->rt != PF_DUPTO)
