Re: pf af-to route output

[Alexandr Nedvedicky]

On Wed, Nov 23, 2016 at 01:43:59AM +0100, Mike Belopuhov wrote:
> On 23 November 2016 at 01:42, Alexander Bluhm  wrote:
> > On Tue, Nov 22, 2016 at 01:44:09PM +0100, Mike Belopuhov wrote:
> >> OK, all I wanted to know was if this is know to work and if it has
> >> been tested.  I'd argue that we don't put the code that doesn't work
> >> or not tested or we don't know what it does :)
> >
> > After looking at all the cases, it will be hard to test the at-to
> > with route-to combinations.  As the feature never worked, let's
> > disable it.  If someone has a usecase, he can put it back.
> >
> > ok?
> >
> > bluhm
> >
> 
> I agree.  If someone makes it work and gets it tested,
> we can consider including the code.

no objection here too. I like bluhm's patch.

Re: pf af-to route output

[Mike Belopuhov]

On 23 November 2016 at 01:42, Alexander Bluhm  wrote:
> On Tue, Nov 22, 2016 at 01:44:09PM +0100, Mike Belopuhov wrote:
>> OK, all I wanted to know was if this is know to work and if it has
>> been tested.  I'd argue that we don't put the code that doesn't work
>> or not tested or we don't know what it does :)
>
> After looking at all the cases, it will be hard to test the at-to
> with route-to combinations.  As the feature never worked, let's
> disable it.  If someone has a usecase, he can put it back.
>
> ok?
>
> bluhm
>

I agree.  If someone makes it work and gets it tested,
we can consider including the code.

Re: pf af-to route output

[Alexander Bluhm]

On Tue, Nov 22, 2016 at 01:44:09PM +0100, Mike Belopuhov wrote:
> OK, all I wanted to know was if this is know to work and if it has
> been tested.  I'd argue that we don't put the code that doesn't work
> or not tested or we don't know what it does :)

After looking at all the cases, it will be hard to test the at-to
with route-to combinations.  As the feature never worked, let's
disable it.  If someone has a usecase, he can put it back.

ok?

bluhm

Index: sys/net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1001
diff -u -p -r1.1001 pf.c
--- sys/net/pf.c22 Nov 2016 19:29:54 -  1.1001
+++ sys/net/pf.c23 Nov 2016 00:00:30 -
@@ -6878,28 +6878,16 @@ done:
action = PF_DROP;
break;
}
-   if (r->rt) {
-   switch (pd.naf) {
-   case AF_INET:
-   pf_route(, r, s);
-   break;
-   case AF_INET6:
-   pf_route6(, r, s);
-   break;
-   }
-   }
-   if (pd.m) {
-   pd.m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
-   switch (pd.naf) {
-   case AF_INET:
-   ip_output(pd.m, NULL, NULL, 0, NULL, NULL, 0);
-   break;
-   case AF_INET6:
-   ip6_output(pd.m, NULL, NULL, 0, NULL, NULL);
-   break;
-   }
-   pd.m = NULL;
+   pd.m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
+   switch (pd.naf) {
+   case AF_INET:
+   ip_output(pd.m, NULL, NULL, 0, NULL, NULL, 0);
+   break;
+   case AF_INET6:
+   ip6_output(pd.m, NULL, NULL, 0, NULL, NULL);
+   break;
}
+   pd.m = NULL;
action = PF_PASS;
break;
 #endif /* INET6 */
Index: sbin/pfctl/parse.y
===
RCS file: /data/mirror/openbsd/cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.655
diff -u -p -r1.655 parse.y
--- sbin/pfctl/parse.y  26 Aug 2016 06:06:58 -  1.655
+++ sbin/pfctl/parse.y  23 Nov 2016 00:07:42 -
@@ -1530,6 +1530,11 @@ pfrule   : action dir logquick interface 
yyerror("af-to can only be used with direction 
in");
YYERROR;
}
+   if (($8.marker & FOM_AFTO) && $8.route.rt) {
+   yyerror("af-to cannot be used together with "
+   "route-to, reply-to, dup-to");
+   YYERROR;
+   }
r.af = $5;
 
if ($8.tag)

Re: pf af-to route output

[Mike Belopuhov]

On 22 November 2016 at 13:35, Alexander Bluhm  wrote:
> On Mon, Nov 21, 2016 at 10:47:34PM +0100, Mike Belopuhov wrote:
>> On 21 November 2016 at 22:38, Alexandr Nedvedicky
>> > The bluhm's change should not alter behavior of older code.
>> Yes, it just adds something new.
>
> I did not try to add something new, I have preserved what was there
> in pf_route().  I have moved the "if (!r->rt)" from pf_route() to
> the "case PF_AFRT" in pf_test().  Now it is more obvious what is
> happening and we ask ourselves "does it work?".  I have not tested
> it.
>

Not exactly.  You're now performing two output actions, while only
one was done before.  If r->rt was specified it was no longer a valid
af-to usage.

Re: pf af-to route output

[Mike Belopuhov]

On 22 November 2016 at 13:35, Alexander Bluhm  wrote:
> On Mon, Nov 21, 2016 at 10:47:34PM +0100, Mike Belopuhov wrote:
>> On 21 November 2016 at 22:38, Alexandr Nedvedicky
>> > The bluhm's change should not alter behavior of older code.
>> Yes, it just adds something new.
>
> I did not try to add something new, I have preserved what was there
> in pf_route().  I have moved the "if (!r->rt)" from pf_route() to
> the "case PF_AFRT" in pf_test().  Now it is more obvious what is
> happening and we ask ourselves "does it work?".  I have not tested
> it.
>
> The parser does not accpet the obvious thing:
> pass in on net1 inet af-to inet6 from 2001:db8::1 to 2001:db8::/96 route-to 
> 2001:db8::1@net0
>
> This might actually work:
> pass in inet all flags S/SA af-to inet6 from 0.0.0.0 dup-to em0
> Although pfctl prints the from 0.0.0.0 in the wrong af.
>
> The parser accepts this, but I doubt that pf will create a valid
> IPv6 packet with the 1.2.3.4 address.
> pass in inet all flags S/SA af-to inet6 from 0.0.0.0 dup-to 1.2.3.4@em0
>
> So we have a kernel implementation that might work for a feature
> that makes sense.  Especially the reply-to could be useful.  But
> the parser is too dumb.  I think we should fix the parser and then
> test the kernel.
>
> bluhm

OK, all I wanted to know was if this is know to work and if it has
been tested.  I'd argue that we don't put the code that doesn't work
or not tested or we don't know what it does :)

When af-to was devised, it wasn't meant to be compatible with route-to
because they do the same thing: policy routing.  I didn't think about
dup-to back then, but this one makes sense to me.  However dup-to is
now operating on the translated packet and pfctl parser might not
expect that if destination address is provided.  Should it be fixed or
simultaneous specification of dup-to/route-to/reply-to and af-to
should be prohibited?

Re: pf af-to route output

[Alexandr Nedvedicky]

> So we have a kernel implementation that might work for a feature
> that makes sense.  Especially the reply-to could be useful.  But
> the parser is too dumb.  I think we should fix the parser and then
> test the kernel.

I absolutely agree.

regards
sasha

Re: pf af-to route output

[Alexander Bluhm]

On Mon, Nov 21, 2016 at 10:47:34PM +0100, Mike Belopuhov wrote:
> On 21 November 2016 at 22:38, Alexandr Nedvedicky
> > The bluhm's change should not alter behavior of older code.
> Yes, it just adds something new.

I did not try to add something new, I have preserved what was there
in pf_route().  I have moved the "if (!r->rt)" from pf_route() to
the "case PF_AFRT" in pf_test().  Now it is more obvious what is
happening and we ask ourselves "does it work?".  I have not tested
it.

The parser does not accpet the obvious thing:
pass in on net1 inet af-to inet6 from 2001:db8::1 to 2001:db8::/96 route-to 
2001:db8::1@net0

This might actually work:
pass in inet all flags S/SA af-to inet6 from 0.0.0.0 dup-to em0
Although pfctl prints the from 0.0.0.0 in the wrong af.

The parser accepts this, but I doubt that pf will create a valid
IPv6 packet with the 1.2.3.4 address.
pass in inet all flags S/SA af-to inet6 from 0.0.0.0 dup-to 1.2.3.4@em0

So we have a kernel implementation that might work for a feature
that makes sense.  Especially the reply-to could be useful.  But
the parser is too dumb.  I think we should fix the parser and then
test the kernel.

bluhm

Re: pf af-to route output

[Mike Belopuhov]

On 21 November 2016 at 22:38, Alexandr Nedvedicky
 wrote:
> I don't have my test bed ready to play with NAT-64 + PBR. The only think 
> I've
> tried is the parser and it seems to me NAT-64 + PBR is problematic:
>
> echo 'pass in on net1 inet af-to inet6 from 2001:db8::1 to 
> 2001:db8::/96 route-to 2001:db8::1@net0' | pfctl -n -f -
> stdin:1: af mismatch in routing spec
> stdin:1: skipping rule due to errors
> stdin:1: rule expands to no valid combination
>

% echo "pass in inet af-to inet6 from ::1 dup-to em0" | pfctl -vnf -
pass in inet all flags S/SA af-to inet6 from 0.0.0.0 dup-to em0
% echo "pass in inet af-to inet6 from ::1 dup-to (em0 1.2.3.4)" | pfctl -vnf -
pass in inet all flags S/SA af-to inet6 from 0.0.0.0 dup-to 1.2.3.4@em0

> parser does not allow me to use IPv6 address as a next hop. As soon as I 
> remove
> next-hop, the parser accepts the rule:
>
> echo 'pass in on net1 inet af-to inet6 from 2001:db8::1 to 
> 2001:db8::/96 route-to net0' | pfctl -n -f -
>
> The bluhm's change should not alter behavior of older code.
>

Yes, it just adds something new.

Re: pf af-to route output

[Alexandr Nedvedicky]

On Mon, Nov 21, 2016 at 07:11:23PM +0100, Mike Belopuhov wrote:
> On Mon, Nov 14, 2016 at 16:38 +0100, Alexander Bluhm wrote:
> > Hi,
> > 
> > The !r->rt case is only used by af-to.  pf_route6() calls ip6_output()
> > to do the work while pf_route() has some custom implementation for
> > that.  It is simpler to call ip_output() or ip6_output() from
> > pf_test() directly.
> > 
> > ok?
> > 
> > bluhm
> > 
> > Index: net/pf.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> > retrieving revision 1.998
> > diff -u -p -r1.998 pf.c
> > --- net/pf.c14 Nov 2016 13:25:00 -  1.998
> > +++ net/pf.c14 Nov 2016 14:08:57 -
> > @@ -6908,10 +6884,28 @@ done:
> > action = PF_DROP;
> > break;
> > }
> > -   if (pd.naf == AF_INET)
> > -   pf_route(, r, s);
> > -   if (pd.naf == AF_INET6)
> > -   pf_route6(, r, s);
> > +   if (r->rt) {
> > +   switch (pd.naf) {
> > +   case AF_INET:
> > +   pf_route(, r, s);
> > +   break;
> > +   case AF_INET6:
> > +   pf_route6(, r, s);
> > +   break;
> > +   }
> > +   }
> 
> Is the r->rt check there to catch additional dup-to/route-to actions
> hooked on to the af-to rule?  Does it actually work?

I don't have my test bed ready to play with NAT-64 + PBR. The only think 
I've
tried is the parser and it seems to me NAT-64 + PBR is problematic:

echo 'pass in on net1 inet af-to inet6 from 2001:db8::1 to 
2001:db8::/96 route-to 2001:db8::1@net0' | pfctl -n -f -
stdin:1: af mismatch in routing spec
stdin:1: skipping rule due to errors
stdin:1: rule expands to no valid combination

parser does not allow me to use IPv6 address as a next hop. As soon as I 
remove
next-hop, the parser accepts the rule:

echo 'pass in on net1 inet af-to inet6 from 2001:db8::1 to 
2001:db8::/96 route-to net0' | pfctl -n -f -

The bluhm's change should not alter behavior of older code.

regards
sasha

Re: pf af-to route output

[Mike Belopuhov]

On Mon, Nov 14, 2016 at 16:38 +0100, Alexander Bluhm wrote:
> Hi,
> 
> The !r->rt case is only used by af-to.  pf_route6() calls ip6_output()
> to do the work while pf_route() has some custom implementation for
> that.  It is simpler to call ip_output() or ip6_output() from
> pf_test() directly.
> 
> ok?
> 
> bluhm
> 
> Index: net/pf.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.998
> diff -u -p -r1.998 pf.c
> --- net/pf.c  14 Nov 2016 13:25:00 -  1.998
> +++ net/pf.c  14 Nov 2016 14:08:57 -
> @@ -6908,10 +6884,28 @@ done:
>   action = PF_DROP;
>   break;
>   }
> - if (pd.naf == AF_INET)
> - pf_route(, r, s);
> - if (pd.naf == AF_INET6)
> - pf_route6(, r, s);
> + if (r->rt) {
> + switch (pd.naf) {
> + case AF_INET:
> + pf_route(, r, s);
> + break;
> + case AF_INET6:
> + pf_route6(, r, s);
> + break;
> + }
> + }

Is the r->rt check there to catch additional dup-to/route-to actions
hooked on to the af-to rule?  Does it actually work?

> + if (pd.m) {
> + pd.m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
> + switch (pd.naf) {
> + case AF_INET:
> + ip_output(pd.m, NULL, NULL, 0, NULL, NULL, 0);
> + break;
> + case AF_INET6:
> + ip6_output(pd.m, NULL, NULL, 0, NULL, NULL);
> + break;
> + }
> + pd.m = NULL;
> + }
>   action = PF_PASS;
>   break;
>  #endif /* INET6 */
> 

Re: pf af-to route output

[Alexander Bluhm]

On Sat, Nov 19, 2016 at 09:07:11PM +1300, Richard Procter wrote:
> Note, pf_route() calls pf_test() only if (pd->kif->pfik_ifp != ifp).
> (I read this as 'pf changed the packet's interface'.) 

This check was added in the commit:


revision 1.218
date: 2002/06/07 21:46:08;  author: jasoni;  state: Exp;  lines: +27 -21;
in pf_route{6}, do not pass thru pf_test again if the outgoing
interface has not changed
- ok dhartmei@


I guess this a protection against looping in pf with outgoing
route-to rules.  This problem cannot happen with af-to.  The af-to
rule is always an incoming rule.  The pf_test() in ip_output() is
done in outgoing direction.  So the af-to code is never called
recursively.

bluhm

Re: pf af-to route output

[Richard Procter]

On Mon, 14 Nov 2016, Alexander Bluhm wrote:

> Hi,
> 
> The !r->rt case is only used by af-to.  pf_route6() calls ip6_output()
> to do the work while pf_route() has some custom implementation for
> that.  It is simpler to call ip_output() or ip6_output() from
> pf_test() directly.
> 
> ok?

Note, pf_route() calls pf_test() only if (pd->kif->pfik_ifp != ifp).
(I read this as 'pf changed the packet's interface'.) 

Using ip_output() avoids this guard. If it's an optimisation, no problem, 
but that's unclear to me. 

(I suspect it's ok, as af-to is invalid in out-bound rules: so the guard 
is always true and pf_test() is always called, unless the af-to packet is 
being sent out the interface it arrived on. But pf_test() should be called 
for all output packets, so this patch would improve the situation.)

With that proviso, ok procter@ 

best, 
Richard. 

Re: pf af-to route output

[Alexandr Nedvedicky]

Hello,

> The !r->rt case is only used by af-to.  pf_route6() calls ip6_output()
> to do the work while pf_route() has some custom implementation for
> that.  It is simpler to call ip_output() or ip6_output() from
> pf_test() directly.

It looks good to me. I'm O.K. with change.

regards
sasha

Re: pf af-to route output

[Alexander Bluhm]

On Mon, Nov 14, 2016 at 04:38:07PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> The !r->rt case is only used by af-to.  pf_route6() calls ip6_output()
> to do the work while pf_route() has some custom implementation for
> that.  It is simpler to call ip_output() or ip6_output() from
> pf_test() directly.
> 
> ok?

anyone?

> 
> bluhm
> 
> Index: net/pf.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.998
> diff -u -p -r1.998 pf.c
> --- net/pf.c  14 Nov 2016 13:25:00 -  1.998
> +++ net/pf.c  14 Nov 2016 14:08:57 -
> @@ -5820,50 +5820,34 @@ pf_route(struct pf_pdesc *pd, struct pf_
>   dst->sin_addr = ip->ip_dst;
>   rtableid = m0->m_pkthdr.ph_rtableid;
>  
> - if (!r->rt) {
> - rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
> - if (rt == NULL) {
> - ipstat_inc(ips_noroute);
> + if (s == NULL) {
> + bzero(sns, sizeof(sns));
> + if (pf_map_addr(AF_INET, r,
> + (struct pf_addr *)>ip_src,
> + , NULL, sns, >route, PF_SN_ROUTE)) {
> + DPFPRINTF(LOG_ERR,
> + "pf_route: pf_map_addr() failed.");
>   goto bad;
>   }
>  
> - ifp = if_get(rt->rt_ifidx);
> -
> - if (rt->rt_flags & RTF_GATEWAY)
> - dst = satosin(rt->rt_gateway);
> -
> - m0->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
> + if (!PF_AZERO(, AF_INET))
> + dst->sin_addr.s_addr = naddr.v4.s_addr;
> + ifp = r->route.kif ?
> + r->route.kif->pfik_ifp : NULL;
>   } else {
> - if (s == NULL) {
> - bzero(sns, sizeof(sns));
> - if (pf_map_addr(AF_INET, r,
> - (struct pf_addr *)>ip_src,
> - , NULL, sns, >route, PF_SN_ROUTE)) {
> - DPFPRINTF(LOG_ERR,
> - "pf_route: pf_map_addr() failed.");
> - goto bad;
> - }
> -
> - if (!PF_AZERO(, AF_INET))
> - dst->sin_addr.s_addr = naddr.v4.s_addr;
> - ifp = r->route.kif ?
> - r->route.kif->pfik_ifp : NULL;
> - } else {
> - if (!PF_AZERO(>rt_addr, AF_INET))
> - dst->sin_addr.s_addr =
> - s->rt_addr.v4.s_addr;
> - ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
> - }
> -
> - rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
> - if (rt == NULL) {
> - ipstat_inc(ips_noroute);
> - goto bad;
> - }
> + if (!PF_AZERO(>rt_addr, AF_INET))
> + dst->sin_addr.s_addr =
> + s->rt_addr.v4.s_addr;
> + ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
>   }
>   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, ) != PF_PASS)
> @@ -5928,8 +5912,6 @@ pf_route(struct pf_pdesc *pd, struct pf_
>  done:
>   if (r->rt != PF_DUPTO)
>   pd->m = NULL;
> - if (!r->rt)
> - if_put(ifp);
>   rtfree(rt);
>   return;
>  
> @@ -5982,12 +5964,6 @@ pf_route6(struct pf_pdesc *pd, struct pf
>   dst->sin6_addr = ip6->ip6_dst;
>   rtableid = m0->m_pkthdr.ph_rtableid;
>  
> - if (!r->rt) {
> - m0->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
> - ip6_output(m0, NULL, NULL, 0, NULL, NULL);
> - goto done;
> - }
> -
>   if (s == NULL) {
>   bzero(sns, sizeof(sns));
>   if (pf_map_addr(AF_INET6, r, (struct pf_addr *)>ip6_src,
> @@ -6908,10 +6884,28 @@ done:
>   action = PF_DROP;
>   break;
>   }
> - if (pd.naf == AF_INET)
> - pf_route(, r, s);
> - if (pd.naf == AF_INET6)
> - pf_route6(, r, s);
> + if (r->rt) {
> + switch (pd.naf) {
> + case AF_INET:
> + pf_route(, r, s);
> + break;
> + case AF_INET6:
> + pf_route6(, r, s);
> + break;
> + }
> + }
> + if (pd.m) {
> + pd.m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
> + switch (pd.naf) {
> + case AF_INET:
> + 

pf af-to route output

[Alexander Bluhm]

Hi,

The !r->rt case is only used by af-to.  pf_route6() calls ip6_output()
to do the work while pf_route() has some custom implementation for
that.  It is simpler to call ip_output() or ip6_output() from
pf_test() directly.

ok?

bluhm

Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.998
diff -u -p -r1.998 pf.c
--- net/pf.c14 Nov 2016 13:25:00 -  1.998
+++ net/pf.c14 Nov 2016 14:08:57 -
@@ -5820,50 +5820,34 @@ pf_route(struct pf_pdesc *pd, struct pf_
dst->sin_addr = ip->ip_dst;
rtableid = m0->m_pkthdr.ph_rtableid;
 
-   if (!r->rt) {
-   rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
-   if (rt == NULL) {
-   ipstat_inc(ips_noroute);
+   if (s == NULL) {
+   bzero(sns, sizeof(sns));
+   if (pf_map_addr(AF_INET, r,
+   (struct pf_addr *)>ip_src,
+   , NULL, sns, >route, PF_SN_ROUTE)) {
+   DPFPRINTF(LOG_ERR,
+   "pf_route: pf_map_addr() failed.");
goto bad;
}
 
-   ifp = if_get(rt->rt_ifidx);
-
-   if (rt->rt_flags & RTF_GATEWAY)
-   dst = satosin(rt->rt_gateway);
-
-   m0->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
+   if (!PF_AZERO(, AF_INET))
+   dst->sin_addr.s_addr = naddr.v4.s_addr;
+   ifp = r->route.kif ?
+   r->route.kif->pfik_ifp : NULL;
} else {
-   if (s == NULL) {
-   bzero(sns, sizeof(sns));
-   if (pf_map_addr(AF_INET, r,
-   (struct pf_addr *)>ip_src,
-   , NULL, sns, >route, PF_SN_ROUTE)) {
-   DPFPRINTF(LOG_ERR,
-   "pf_route: pf_map_addr() failed.");
-   goto bad;
-   }
-
-   if (!PF_AZERO(, AF_INET))
-   dst->sin_addr.s_addr = naddr.v4.s_addr;
-   ifp = r->route.kif ?
-   r->route.kif->pfik_ifp : NULL;
-   } else {
-   if (!PF_AZERO(>rt_addr, AF_INET))
-   dst->sin_addr.s_addr =
-   s->rt_addr.v4.s_addr;
-   ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
-   }
-
-   rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
-   if (rt == NULL) {
-   ipstat_inc(ips_noroute);
-   goto bad;
-   }
+   if (!PF_AZERO(>rt_addr, AF_INET))
+   dst->sin_addr.s_addr =
+   s->rt_addr.v4.s_addr;
+   ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
}
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, ) != PF_PASS)
@@ -5928,8 +5912,6 @@ pf_route(struct pf_pdesc *pd, struct pf_
 done:
if (r->rt != PF_DUPTO)
pd->m = NULL;
-   if (!r->rt)
-   if_put(ifp);
rtfree(rt);
return;
 
@@ -5982,12 +5964,6 @@ pf_route6(struct pf_pdesc *pd, struct pf
dst->sin6_addr = ip6->ip6_dst;
rtableid = m0->m_pkthdr.ph_rtableid;
 
-   if (!r->rt) {
-   m0->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
-   ip6_output(m0, NULL, NULL, 0, NULL, NULL);
-   goto done;
-   }
-
if (s == NULL) {
bzero(sns, sizeof(sns));
if (pf_map_addr(AF_INET6, r, (struct pf_addr *)>ip6_src,
@@ -6908,10 +6884,28 @@ done:
action = PF_DROP;
break;
}
-   if (pd.naf == AF_INET)
-   pf_route(, r, s);
-   if (pd.naf == AF_INET6)
-   pf_route6(, r, s);
+   if (r->rt) {
+   switch (pd.naf) {
+   case AF_INET:
+   pf_route(, r, s);
+   break;
+   case AF_INET6:
+   pf_route6(, r, s);
+   break;
+   }
+   }
+   if (pd.m) {
+   pd.m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
+   switch (pd.naf) {
+   case AF_INET:
+   ip_output(pd.m, NULL, NULL, 0, NULL, NULL, 0);
+   break;
+