Date: Mon, 6 Feb 2006 14:32:39 +0300 From: Yar Tikhiy <yar@comp.chem.msu.su> To: Max Laier <max@love2party.net> Cc: freebsd-net@freebsd.org, Hajimu UMEMOTO <ume@freebsd.org>, Luigi Rizzo <rizzo@icir.org> Subject: Re: if_bridge.ko requires INET6... Message-ID: <20060206113239.GD59508@comp.chem.msu.su> In-Reply-To: <200602041616.57224.max@love2party.net> References: <20060201005658.A70005@xorpc.icir.org> <20060202124328.GK29980@comp.chem.msu.su> <200602021437.38385.max@love2party.net> <200602041616.57224.max@love2party.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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@freebsd.org
> \ / Max Laier | ICQ #67774661
> X http://pf4freebsd.love2party.net/ | mlaier@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060206113239.GD59508>