Subject: Re: Try again, itojun, patches need more work.
To: Darren Reed <avalon@caligula.anu.edu.au>
From: None <itojun@iijlab.net>
List: tech-net
Date: 06/30/2003 12:25:16
	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.

>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.

>> 	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?

>> 	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.

>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.

>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.

	now, i think we had enough discussions, so i'm going to commit it
	after lunch.  it won't affect you if you don't have 'pseudo-device pf'
	in your kernel config, afterall.

itojun