On 24/12/16(Sat) 00:23, Alexander Bluhm wrote:
> Hi,
>
> I think it is better to check for a valid route than for an existing
> route in pf route-to. So call rtisvalid() now. I want to have
> pf_route() and pf_route6() as simmilar as possible so I can merge
> them some day. As rtalloc() has to stay after embeding the v6
> scope, I have moved it down in the v4 code. In the v6 code I always
> do the valid route check now. The duplicate route lookup in
> pf_refragment6() will be fixed later.
The reason for not calling rtisvalid() previously was the same as in
ip_output(). This bug was related to stale ifa hanging to static
routes. Since it is now "fixed", this should be safe, ok mpi@.
> Index: net/pf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.1006
> diff -u -p -r1.1006 pf.c
> --- net/pf.c 23 Dec 2016 20:49:41 -0000 1.1006
> +++ net/pf.c 23 Dec 2016 22:53:07 -0000
> @@ -5832,12 +5832,6 @@ pf_route(struct pf_pdesc *pd, struct pf_
> if (ifp == NULL)
> goto bad;
>
> - rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
> - if (rt == NULL) {
> - ipstat_inc(ips_noroute);
> - goto bad;
> - }
> -
> if (pd->kif->pfik_ifp != ifp) {
> if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
> goto bad;
> @@ -5853,6 +5847,12 @@ pf_route(struct pf_pdesc *pd, struct pf_
>
> in_proto_cksum_out(m0, ifp);
>
> + rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
> + if (!rtisvalid(rt)) {
> + ipstat_inc(ips_noroute);
> + goto bad;
> + }
> +
> if (ntohs(ip->ip_len) <= ifp->if_mtu) {
> ip->ip_sum = 0;
> if (ifp->if_capabilities & IFCAP_CSUM_IPv4)
> @@ -5991,6 +5991,12 @@ pf_route6(struct pf_pdesc *pd, struct pf
> if (IN6_IS_SCOPE_EMBED(&dst->sin6_addr))
> dst->sin6_addr.s6_addr16[1] = htons(ifp->if_index);
>
> + rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
> + if (!rtisvalid(rt)) {
> + ip6stat.ip6s_noroute++;
> + goto bad;
> + }
> +
> /*
> * If packet has been reassembled by PF earlier, we have to
> * use pf_refragment6() here to turn it back to fragments.
> @@ -5998,13 +6004,7 @@ pf_route6(struct pf_pdesc *pd, struct pf
> if ((mtag = m_tag_find(m0, PACKET_TAG_PF_REASSEMBLED, NULL))) {
> (void) pf_refragment6(&m0, mtag, dst, ifp);
> } else if ((u_long)m0->m_pkthdr.len <= ifp->if_mtu) {
> - rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
> - if (rt == NULL) {
> - ip6stat.ip6s_noroute++;
> - goto bad;
> - }
> ifp->if_output(ifp, m0, sin6tosa(dst), rt);
> - rtfree(rt);
> } else {
> icmp6_error(m0, ICMP6_PACKET_TOO_BIG, 0, ifp->if_mtu);
> }
> @@ -6012,6 +6012,7 @@ pf_route6(struct pf_pdesc *pd, struct pf
> done:
> if (r->rt != PF_DUPTO)
> pd->m = NULL;
> + rtfree(rt);
> return;
>
> bad:
>
