HEADS UP on HEAD - PF ABI changes - Re: cvs commit: src/usr.sbin/pfctl parse.y pf.conf.5 pfctl_parser.c src/sys/net/pf pf.c pfvar.h

[Max Laier]

On Sunday 06 April 2008 23:23:37 Matthew Dillon wrote:
> :dillon      2008/04/06 14:12:42 PDT
> :
> :DragonFly src repository
> :
> :  Modified files:
> :    usr.sbin/pfctl       parse.y pf.conf.5 pfctl_parser.c
> :    sys/net/pf           pf.c pfvar.h
> :  Log:
> :  Bring the 'probability' keyword into PF from NetBSD.  This feature
> : allows
>
>     Please note that this commit modifies the ABI structure passed
> between kernel and userland.  If you are using PF (packet filter), both
> the kernel and pfctl must be recompiled and pfctl needs the kernel's
> new pfvar.h, e.g.:
>
>     (build and install new kernel)
>     ...
>     (build and install new pfctl)
>
> 	cp /usr/src/sys/net/pf/pfvar.h /usr/include/net/pf/pfvar.h
> 	cd /usr/src/usr.sbin/pfctl
> 	make clean obj depend all install
>
>     A buildworld/kernel sequence also works of course, I'm simply
> describing how to avoid doing a full buildworld.

Wait ... you might want to consider to store the hash in the state instead 
of calculating it over and over again:

i.e.
		if (s) {
			m->m_pkthdr.fw_flags |= ALTQ_MBUF_STATE_HASHED;
-			m->m_pkthdr.altq_state_hash = pf_state_hash(s);
+			m->m_pkthdr.altq_state_hash = s->hash;
		}

and calculate the hash on state creation.

The hash operation is not exactly expensive, but still.  I think this 
would be space well spent.  Unless there is some padding in your pf state 
this will result in another ABI change.  OpenBSD recently separated the 
in-kernel state entry from the information used for pfsync and userland 
copys, this is a good idea.

-- 
/"\  Best regards,                      | mlaier at freebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News