In some mail from itojun@iijlab.net, sie said:
> 
> 	btw, i thank you for your comments, but please do not write as if you
> 	are my boss.  a lot of "you should" in your letter make me feel like so.

I don't mean to sound like that..

> >You should expand the role of pfil4_wrapper() to take care of the
> >ip_len/ip_off fields being around the wrong way.
> 
> 	no need for that.  the code handle these fields network byteorder
> 	everywhere, just like other sys/netinet portion.

Ok, was just trying to help elimiate some #ifdef's..

> >> 	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 ?
> 
> 	what kind of name do you suggest?

Now you've put me on the spot!  I'll have to think about something...

But if the functions aren't specific to pf, they shouldn't be named
in a manner that suggests they are.

> >> 	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.
> 
> 	then why did you made me such change?  we can make the API when you
> 	need it.  i'll back out sys/kern/uipc_mbuf2.c change and keep tagname
> 	conversion portion to sys/net/pf.c, until you need the API.

By "need" I mean it would have worked without that change but whilst it
would work, it's better for us if it is changed.  Having such a change
means there's no need for other, ugly, hacks to be put in place to make
other code be able to interface with these new changes to IPSec/ALTQ.

So while they may not be "needed" for NetBSD to work, I think they should
be required to make it easier to work with IPSec/ALTQ and other comments
I've read tend to agree with this.

> >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.
> 
> 	tagnames needs to be reference-counted.  i.e. whenever there are
> 	filtering rules that use certain tagname, you want a consistent value.

Oh sure, I think you've misunderstood my criticism here.

1. There should be no need for pftag_purge(), pftag_unref() should do
   all the work.

2. pftag_tag2tagname() isn't used (dead function?)

> >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?
> 
> 	they do peer review much strictier than netbsd.

Hmmm.  I see the "ok foo@" in commit messages but somehow that doesn't
help convince me that they're always doing "The Right Thing"(tm).

Darren