if_bridge.ko requires INET6...

Mon Feb 6 03:33:03 PST 2006

On Sat, Feb 04, 2006 at 04:16:49PM +0100, Max Laier wrote:
> On Thursday 02 February 2006 14:37, Max Laier wrote:
> > On Thursday 02 February 2006 13:43, Yar Tikhiy wrote:
> > > > This needs to be fixed in pf then.
> > >
> > > Max Laier and I discussed this issue once, and Max had concern
> > > over possible performance degradation that might result from
> > > calling pflog functions through pointers to be set by a separate
> > > pflog module.  We can skip touching the pf module in RELENG_6 for
> > > now and leave the issue to after 6.1-RELEASE is out.
> >
> > I have convinced myself that we should really use a function pointer here. 
> > I will try to commit a sollution to HEAD over the weekend.  If you are
> > MFC'ing the changes *now*, I'd appreciate if you could spare out pf, but I
> > am willing to MFC the changes before 6.1 if testing goes well.
> 
> Here it is.  I'd appreciate feedback.  pflog_packet() uses a lot of complex 
> types which makes it necessary to include pfvar.h.  This is ugly, but I don't 
> know how to work around this.

pflog_packet() takes pointers to the types, which are structs, so
it should be possible to declare the structs opaquely.  AFAIK this
trick is legal C and used here and there in our code.  E.g.:

+#ifdef __FreeBSD__
+struct pfi_kif;
+struct pf_rule;
+struct pf_rulese;
+
+typedef int pflog_packet_t(struct pfi_kif *, struct mbuf *, sa_family_t,
+    u_int8_t, u_int8_t, struct pf_rule *, struct pf_rule *,
+    struct pf_ruleset *);
+extern pflog_packet_t *pflog_packet_ptr;
+#define      PFLOG_PACKET(i,x,a,b,c,d,e,f,g) do {            \
+     if (pflog_packet_ptr != NULL)                   \
+             pflog_packet_ptr(i,a,b,c,d,e,f,g);      \
+} while (0)
+#else

Please see another small remark below.

> -- 
> /"\  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

> Index: contrib/pf/net/if_pflog.c
> ===================================================================
> RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/if_pflog.c,v
> retrieving revision 1.18
> diff -u -r1.18 if_pflog.c
> --- contrib/pf/net/if_pflog.c	5 Dec 2005 11:58:31 -0000	1.18
> +++ contrib/pf/net/if_pflog.c	4 Feb 2006 15:09:11 -0000
> @@ -376,9 +376,15 @@
>  	case MOD_LOAD:
>  		LIST_INIT(&pflog_list);
>  		if_clone_attach(&pflog_cloner);
> +		PF_LOCK();
> +		pflog_packet_ptr = pflog_packet;
> +		PF_UNLOCK();
>  		break;
>  
>  	case MOD_UNLOAD:
> +		PF_LOCK();
> +		pflog_packet_ptr = NULL;
> +		PF_UNLOCK();
>  		if_clone_detach(&pflog_cloner);
>  		break;
>  
> @@ -400,4 +406,5 @@
>  
>  DECLARE_MODULE(pflog, pflog_mod, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY);
>  MODULE_VERSION(pflog, PFLOG_MODVER);
> +MODULE_DEPEND(pflog, pf, PF_MODVER, PF_MODVER, PF_MODVER);
>  #endif /* __FreeBSD__ */
> Index: contrib/pf/net/if_pflog.h
> ===================================================================
> RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/if_pflog.h,v
> retrieving revision 1.6
> diff -u -r1.6 if_pflog.h
> --- contrib/pf/net/if_pflog.h	10 Jun 2005 16:49:03 -0000	1.6
> +++ contrib/pf/net/if_pflog.h	4 Feb 2006 15:08:59 -0000
> @@ -70,10 +70,24 @@
>  
>  #ifdef _KERNEL
>  
> +#ifdef __FreeBSD__
> +/* XXX */
> +#include <net/pfvar.h>
> +
> +typedef int pflog_packet_t(struct pfi_kif *, struct mbuf *, sa_family_t,
> +    u_int8_t, u_int8_t, struct pf_rule *, struct pf_rule *,
> +    struct pf_ruleset *);
> +extern pflog_packet_t *pflog_packet_ptr;
> +#define	PFLOG_PACKET(i,x,a,b,c,d,e,f,g) do {		\
> +	if (pflog_packet_ptr != NULL)			\
> +		pflog_packet_ptr(i,a,b,c,d,e,f,g);	\
> +} while (0)
> +#else
>  #if NPFLOG > 0
>  #define	PFLOG_PACKET(i,x,a,b,c,d,e,f,g) pflog_packet(i,a,b,c,d,e,f,g)
>  #else
>  #define	PFLOG_PACKET(i,x,a,b,c,d,e,f,g)	((void)0)
>  #endif /* NPFLOG > 0 */
> +#endif /* __FreeBSD__ */
>  #endif /* _KERNEL */
>  #endif /* _NET_IF_PFLOG_H_ */
> Index: contrib/pf/net/pf_ioctl.c
> ===================================================================
> RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pf_ioctl.c,v
> retrieving revision 1.22
> diff -u -r1.22 pf_ioctl.c
> --- contrib/pf/net/pf_ioctl.c	5 Dec 2005 11:58:31 -0000	1.22
> +++ contrib/pf/net/pf_ioctl.c	4 Feb 2006 15:09:30 -0000
> @@ -108,6 +108,10 @@
>  #include <net/if_pfsync.h>
>  #endif /* NPFSYNC > 0 */
>  
> +#ifdef __FreeBSD__
> +#include <net/if_pflog.h>
> +#endif
> +
>  #ifdef INET6
>  #include <netinet/ip6.h>
>  #include <netinet/in_pcb.h>
> @@ -230,6 +234,7 @@
>  
>  static volatile int pf_pfil_hooked = 0;
>  struct mtx pf_task_mtx;
> +pflog_packet_t *pflog_packet_ptr = NULL;
>  
>  void
>  init_pf_mutex(void)
> Index: modules/Makefile
> ===================================================================
> RCS file: /usr/store/mlaier/fcvs/src/sys/modules/Makefile,v
> retrieving revision 1.472
> diff -u -r1.472 Makefile
> --- modules/Makefile	31 Jan 2006 23:11:35 -0000	1.472
> +++ modules/Makefile	3 Feb 2006 22:57:36 -0000
> @@ -180,6 +180,7 @@
>  	pcn \
>  	${_pecoff} \
>  	${_pf} \
> +	${_pflog} \
>  	plip \
>  	${_pmc} \
>  	portalfs \
> @@ -307,6 +308,7 @@
>  
>  .if !defined(NO_PF) || defined(ALL_MODULES)
>  _pf=		pf
> +_pflog=		pflog
>  .endif
>  
>  .if ${MACHINE_ARCH} == "i386"
> Index: modules/pf/Makefile
> ===================================================================
> RCS file: /usr/store/mlaier/fcvs/src/sys/modules/pf/Makefile,v
> retrieving revision 1.8
> diff -u -r1.8 Makefile
> --- modules/pf/Makefile	14 Oct 2005 23:30:14 -0000	1.8
> +++ modules/pf/Makefile	3 Feb 2006 22:46:23 -0000
> @@ -6,7 +6,6 @@
>  
>  KMOD=	pf
>  SRCS = 	pf.c pf_if.c pf_subr.c pf_osfp.c pf_ioctl.c pf_norm.c pf_table.c \
> -	if_pflog.c \
>  	in4_cksum.c \
>  	opt_pf.h opt_inet.h opt_inet6.h opt_bpf.h
>  
> @@ -15,7 +14,6 @@
>  .if !defined(KERNBUILDDIR)
>  opt_pf.h:
>  	echo "#define DEV_PF 1" > opt_pf.h
> -	echo "#define DEV_PFLOG 1" >> opt_pf.h

If all is right, defining DEV_PF here shouldn't be needed.

>  opt_inet.h:
>  	echo "#define INET 1" > opt_inet.h
> Index: modules/pflog/Makefile
> ===================================================================
> RCS file: modules/pflog/Makefile
> diff -N modules/pflog/Makefile
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ modules/pflog/Makefile	3 Feb 2006 22:48:31 -0000
> @@ -0,0 +1,29 @@
> +# $FreeBSD: src/sys/modules/pf/Makefile,v 1.8 2005/10/14 23:30:14 yar Exp $
> +
> +.PATH: ${.CURDIR}/../../contrib/pf/net
> +.PATH: ${.CURDIR}/../../contrib/pf/netinet
> +.PATH: ${.CURDIR}/../../netinet
> +
> +KMOD=	pflog
> +SRCS = 	if_pflog.c \
> +	opt_pf.h opt_inet.h opt_inet6.h opt_bpf.h
> +
> +CFLAGS+=  -I${.CURDIR}/../../contrib/pf
> +
> +.if !defined(KERNBUILDDIR)
> +opt_pf.h:
> +	echo "#define DEV_PFLOG 1" > opt_pf.h

Ditto for DEV_PFLOG.

> +opt_inet.h:
> +	echo "#define INET 1" > opt_inet.h
> +
> +.if !defined(NO_INET6)
> +opt_inet6.h:
> +	echo "#define INET6 1" > opt_inet6.h
> +.endif
> +
> +opt_bpf.h:
> +	echo "#define DEV_BPF 1" > opt_bpf.h
> +.endif
> +
> +.include <bsd.kmod.mk>

Thanks for your work!

-- 
Yar