cvs commit: src/sys/contrib/ipfilter/netinetip_auth.cip_compat.h ip_frag.c ip_log.c ip_nat.h ip_proxy.c ip_state.c ip_state.h mlfk_ipl.c

[Scott Long]

Giorgos Keramidas wrote:
> On 2004-12-24 17:13, Scott Long <scottl at freebsd.org> wrote:
> 
>>Giorgos Keramidas wrote:
>>
>>>On 2004-12-24 16:14, Scott Long <scottl at freebsd.org> wrote:
>>>
>>>
>>>>Giorgos Keramidas wrote:
>>>>
>>>>
>>>>>On 2004-12-24 14:46, Scott Long <scottl at freebsd.org> wrote:
>>>>>
>>>>>
>>>>>>Darren Reed wrote:
>>>>>>
>>>>>>
>>>>>>>darrenr     2004-12-24 09:14:26 UTC
>>>>>>>
>>>>>>>FreeBSD src repository
>>>>>>>
>>>>>>>Modified files:
>>>>>>>sys/contrib/ipfilter/netinet ip_auth.c ip_compat.h ip_frag.c
>>>>>>>                             ip_log.c ip_nat.h ip_proxy.c
>>>>>>>                             ip_state.c ip_state.h mlfk_ipl.c
>>>>>>>Log:
>>>>>>>Enable fine grained locking within IPFilter, using mtx(9) and sx(9)
>>>>>>>allowing
>>>>>>>the the "needs giant" flag to be removed from the driver.
>>>>>>
>>>>>>I'm not sure how this could possibly compile on FreeBSD.  Did you forget
>>>>>>to check in a file perhaps?  Please fix this ASAP, otherwise it's going
>>>>>>to get reverted.
>>>>>
>>>>>Hi Scott,
>>>>>
>>>>>can you test this as a fix for the build problems?  It seems to fix the
>>>>>buildkernel issues here.  The arbitrary choise of __FreeBSD_version's
>>>>>value was based on this snippet from
>>>>>src/sys/ipfilter/netinet/ip_compat.h:
>>>>
>>>>Thanks, I'll test it in a few minutes.  Would you also be willing to fix
>>>>the warning from PFIL_HOOKS being redefined?
>>>
>>>
>>>Sure.  The ip_fil.h header defines PFIL_HOOKS unconditionally, possibly
>>>overriding the definition passed on the command line by the Makefile of
>>>src/sys/modules/ipfilter.
>>>
>>>There are two options:
>>>
>>>a) Either we remove the -DPFIL_HOOKS from the Makefile (possibly resulting
>>>in other breakages at places where ip_fil.h is not included, even though
>>>it should be), or
>>>
>>>b) we commit something like this minor diff to ip_fil.h:
>>>
>>>%%%
>>>gothmog:/usr/src# cvs di -u sys/contrib/ipfilter/netinet/ip_fil.h
>>>Index: sys/contrib/ipfilter/netinet/ip_fil.h
>>>===================================================================
>>>RCS file: /home/ncvs/src/sys/contrib/ipfilter/netinet/ip_fil.h,v
>>>retrieving revision 1.27
>>>diff -u -r1.27 ip_fil.h
>>>--- sys/contrib/ipfilter/netinet/ip_fil.h	27 Aug 2004 20:01:08 -0000
>>>1.27
>>>+++ sys/contrib/ipfilter/netinet/ip_fil.h	25 Dec 2004 00:03:19 -0000
>>>@@ -498,7 +498,9 @@
>>>    (defined(NetBSD1_2) && NetBSD1_2 > 1) || (defined(__FreeBSD_version)
>>>    && \
>>>     (__FreeBSD_version >= 500011))
>>># if (NetBSD >= 199905) || (__FreeBSD_version >= 600001)
>>>-#  define PFIL_HOOKS
>>>+#  ifndef PFIL_HOOKS
>>>+#   define PFIL_HOOKS
>>>+#  endif
>>># endif
>>># ifdef PFIL_HOOKS
>>>#  define NETBSD_PF
>>>%%%
>>
>>I don't see anywhere in the ipfilter source where PFIL_HOOKS is actually
>>checked.  PFIL_HOOKS was removed as an option from FreeBSD a few months
>>ago and is now assumed to always be present.
> 
> 
> True.  I remember this.  The PFIL_HOOKS ifdef doesn't work as a kernel
> option anymore in ipfilter though.
> 
> 
>>Maybe we should just remove the __FreeBSD_version from the clause.
> 
> 
> PFIL_HOOKS is used in ip_fil.h to check if NETBSD_PF should be defined.
> We need NETBSD_PF defined, because it is used in fil.c and ip_fil.c as a
> conditional.
> 
> These will break if NETBSD_PF is not defined, as they depend on
> NETBSD_PF and ip_compat.h to include the pfil interface from
> <net/pfil.h>
> 

Bugger, you're right.  I'll fix it now.  What a tangled web =-(

Scott