In some mail from itojun@iijlab.net, sie said:
> 
> >> 	or i can forget about pfrdr variable for now, and change pf to
> >> 	use pfil(9).  then you will be happy.
> >
> >So what this "pfrdr" does, no thanks to the lack of appropriate comments,
> >is allow redirects back out the same interface a packet was received from
> >without generating an ICMP redirect.  Reviewing this, the comment header
> >for ip_forward() should be updated to indicate the *real* role of srcrt
> >(and that is to prevent redirects from being issued) and the code in
> >ip_input() should....well, if deemed useful it should go around the
> >pfil(9) hooks BUT putting an IP address in an "int" just seems so very
> >very wrong!  Maybe I'm spending too much time with C++ these days.  In
> >general, that seems like a useful change to make, just its nature seems
> >offensive.
> 
> 	pfrdr is in_addr_t, not int.

So it is...the way its used would not suggest that, though, which
highlights the problem with the existing code!

> 	anyways, after some thinking pfrdr does not make sense to me as it
> 	is checking for overwritten ip_dst (rewritten), not the alternate route.

This is correct.

> 	if it were to notify ip_forward() the use of "route-to" (alternate
> 	nexthop) PF directive, it makes sense.  so forget about it, now my
> 	patch uses pfil(9).

That's never how IPFilter did "next-hop" and given pf is a copy, likewise
it doesn't do it that way either.

"pfrdr" is there so you can use redirect NAT rules to redirect traffic
back out to something on the same LAN as the incoming interface.

You should expand the role of pfil4_wrapper() to take care of the
ip_len/ip_off fields being around the wrong way.

> >On the API issues, moving the pf*() functions into uipc_mbuf2.c does
> >not seem like a long term solution, am I right ?
> 
> 	for netbsd, i suppose it is a long term solution, for tagname-to-tag
> 	/reverse mapping.  now you can make changes to ipfilter so that
> 	ipfilter can query tag value (u_int16_t) from tagname, or vice versa,
> 	without requiring sys/net/pf.c.

In that case, might I suggest a change name is in order ?

> >Or will you be merging those changes back into OpenBSD but have not
> >yet ?
> >
> >I look in the pf code within the patch and I see #ifdef's to not use
> >the uipc_mbuf2.c routines for OpenBSD.  There should be no need for
> >those #ifdef's if you've settled on the API and both OS's are going
> >to start using the same code.
> 
> 	i don't think openbsd would take this API back to them, as they
> 	do not need it.

NetBSD doesn't "need" it either, but it's better for it to be there
than it to be there in some other fasion.

Why doesn't pftag_unref() do the deletion when the ref count becomes
0 anyway ?  Looking at pftag_purge(), there should be no need for it.

pftag_tag2tagname() should return a boolean, well, it might if it was
actually used, but nothing calls pf_tag2tagname(), either.

Why on earth do I get the feeling I'm debugging OpenBSD stuff when I
shouldn't have to ? ...  don't they have any sort of review process?

Darren