Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 21 Aug 2009 11:20:10 +0000 (UTC)
From:      Julian Elischer <julian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r196423 - in head/sys/netinet: . ipfw
Message-ID:  <200908211120.n7LBKAjw021131@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: julian
Date: Fri Aug 21 11:20:10 2009
New Revision: 196423
URL: http://svn.freebsd.org/changeset/base/196423

Log:
  Fix ipfw's initialization functions to get the correct order of evaluation
  to allow vnet and non vnet operation. Move some functions from ip_fw_pfil.c
  to ip_fw2.c and mode to mostly using the SYSINIT and VNET_SYSINIT handlers
  instead of the modevent handler. Correct some spelling errors in comments
  in the affected code. Note this bug fixes a crash in NON VIMAGE kernels when
  ipfw is unloaded.
  
  This patch is a minimal patch for 8.0
  I have a much larger patch that actually fixes the underlying problems
  that will be applied after 8.0
  
  Reviewed by:	zec@, rwatson@, bz@(earlier version)
  Approved by:	re (rwatson)
  MFC after:	Immediatly

Modified:
  head/sys/netinet/ip_fw.h
  head/sys/netinet/ipfw/ip_fw2.c
  head/sys/netinet/ipfw/ip_fw_pfil.c

Modified: head/sys/netinet/ip_fw.h
==============================================================================
--- head/sys/netinet/ip_fw.h	Fri Aug 21 11:17:25 2009	(r196422)
+++ head/sys/netinet/ip_fw.h	Fri Aug 21 11:20:10 2009	(r196423)
@@ -645,8 +645,10 @@ int ipfw_check_out(void *, struct mbuf *
 
 int ipfw_chk(struct ip_fw_args *);
 
-int ipfw_init(void);
-void ipfw_destroy(void);
+int ipfw_hook(void);
+int ipfw6_hook(void);
+int ipfw_unhook(void);
+int ipfw6_unhook(void);
 #ifdef NOTYET
 void ipfw_nat_destroy(void);
 #endif

Modified: head/sys/netinet/ipfw/ip_fw2.c
==============================================================================
--- head/sys/netinet/ipfw/ip_fw2.c	Fri Aug 21 11:17:25 2009	(r196422)
+++ head/sys/netinet/ipfw/ip_fw2.c	Fri Aug 21 11:20:10 2009	(r196423)
@@ -102,6 +102,8 @@ __FBSDID("$FreeBSD$");
 #include <security/mac/mac_framework.h>
 #endif
 
+static VNET_DEFINE(int, ipfw_vnet_ready) = 0;
+#define	V_ipfw_vnet_ready	VNET(ipfw_vnet_ready)
 /*
  * set_disable contains one bit per set value (0..31).
  * If the bit is set, all rules with the corresponding set
@@ -2237,7 +2239,7 @@ ipfw_chk(struct ip_fw_args *args)
 	/* end of ipv6 variables */
 	int is_ipv4 = 0;
 
-	if (m->m_flags & M_SKIP_FIREWALL)
+	if (m->m_flags & M_SKIP_FIREWALL || (! V_ipfw_vnet_ready))
 		return (IP_FW_PASS);	/* accept */
 
 	dst_ip.s_addr = 0;		/* make sure it is initialized */
@@ -4579,12 +4581,10 @@ done:
 	CURVNET_RESTORE();
 }
 
-
-
 /****************
  * Stuff that must be initialised only on boot or module load
  */
-int
+static int
 ipfw_init(void)
 {
 	int error = 0;
@@ -4623,9 +4623,11 @@ ipfw_init(void)
 		default_to_accept ? "accept" : "deny");
 
 	/*
-	 * Note: V_xxx variables can be accessed here but the iattach()
-     	 * may not have been called yet for the VIMGE case.
-	 * Tuneables will have been processed.
+	 * Note: V_xxx variables can be accessed here but the vnet specific
+	 * initializer may not have been called yet for the VIMAGE case.
+	 * Tuneables will have been processed. We will print out values for
+	 * the default vnet. 
+	 * XXX This should all be rationalized AFTER 8.0
 	 */
 	if (V_fw_verbose == 0)
 		printf("disabled\n");
@@ -4636,6 +4638,20 @@ ipfw_init(void)
 		    V_verbose_limit);
 
 	/*
+	 * Hook us up to pfil.
+	 * Eventually pfil will be per vnet.
+	 */
+	if ((error = ipfw_hook()) != 0) {
+		printf("ipfw_hook() error\n");
+		return (error);
+	}
+#ifdef INET6
+	if ((error = ipfw6_hook()) != 0) {
+		printf("ipfw6_hook() error\n");
+		return (error);
+	}
+#endif
+	/*
 	 * Other things that are only done the first time.
 	 * (now that we a re cuaranteed of success).
 	 */
@@ -4645,8 +4661,8 @@ ipfw_init(void)
 }
 
 /****************
- * Stuff that must be initialised for every instance
- * (including the forst of course).
+ * Stuff that must be initialized for every instance
+ * (including the first of course).
  */
 static int
 vnet_ipfw_init(const void *unused)
@@ -4726,17 +4742,17 @@ vnet_ipfw_init(const void *unused)
 #endif
 
 	/* First set up some values that are compile time options */
+	V_ipfw_vnet_ready = 1;		/* Open for business */
 	return (0);
 }
 
 /**********************
  * Called for the removal of the last instance only on module unload.
  */
-void
+static void
 ipfw_destroy(void)
 {
-	ip_fw_chk_ptr = NULL;
-	ip_fw_ctl_ptr = NULL;
+
 	uma_zdestroy(ipfw_dyn_rule_zone);
 	IPFW_DYN_LOCK_DESTROY();
 	printf("IP firewall unloaded\n");
@@ -4750,7 +4766,9 @@ vnet_ipfw_uninit(const void *unused)
 {
 	struct ip_fw *reap;
 
+	V_ipfw_vnet_ready = 0; /* tell new callers to go away */
 	callout_drain(&V_ipfw_timeout);
+	/* We wait on the wlock here until the last user leaves */
 	IPFW_WLOCK(&V_layer3_chain);
 	flush_tables(&V_layer3_chain);
 	V_layer3_chain.reap = NULL;
@@ -4766,10 +4784,86 @@ vnet_ipfw_uninit(const void *unused)
 	return 0;
 }
 
-VNET_SYSINIT(vnet_ipfw_init, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY - 255,
-    vnet_ipfw_init, NULL);
+/*
+ * Module event handler.
+ * In general we have the choice of handling most of these events by the
+ * event handler or by the (VNET_)SYS(UN)INIT handlers. I have chosen to
+ * use the SYSINIT handlers as they are more capable of expressing the
+ * flow of control during module and vnet operations, so this is just
+ * a skeleton. Note there is no SYSINIT equivalent of the module
+ * SHUTDOWN handler, but we don't have anything to do in that case anyhow.
+ */
+static int
+ipfw_modevent(module_t mod, int type, void *unused)
+{
+	int err = 0;
+
+	switch (type) {
+	case MOD_LOAD:
+		/* Called once at module load or
+	 	 * system boot if compiled in. */
+		break;
+	case MOD_UNLOAD:
+		break;
+	case MOD_QUIESCE:
+		/* Yes, the unhooks can return errors, we can safely ignore
+		 * them. Eventually these will be done per jail as they
+		 * shut down. We will wait on each vnet's l3 lock as existing
+		 * callers go away.
+		 */
+		ipfw_unhook();
+#ifdef INET6
+		ipfw6_unhook();
+#endif
+		/* layer2 and other entrypoints still come in this way. */
+		ip_fw_chk_ptr = NULL;
+		ip_fw_ctl_ptr = NULL;
+		/* Called during unload. */
+		break;
+	case MOD_SHUTDOWN:
+		/* Called during system shutdown. */
+		break;
+	default:
+		err = EOPNOTSUPP;
+		break;
+	}
+	return err;
+}
+
+static moduledata_t ipfwmod = {
+	"ipfw",
+	ipfw_modevent,
+	0
+};
 
-VNET_SYSUNINIT(vnet_ipfw_uninit, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY - 255,
-    vnet_ipfw_uninit, NULL);
+/* Define startup order. */
+#define	IPFW_SI_SUB_FIREWALL	SI_SUB_PROTO_IFATTACHDOMAIN
+#define	IPFW_MODEVENT_ORDER	(SI_ORDER_ANY - 255) /* On boot slot in here. */
+#define	IPFW_MODULE_ORDER	(IPFW_MODEVENT_ORDER + 1) /* A little later. */
+#define	IPFW_VNET_ORDER		(IPFW_MODEVENT_ORDER + 2) /* Later still. */
+
+DECLARE_MODULE(ipfw, ipfwmod, IPFW_SI_SUB_FIREWALL, IPFW_MODEVENT_ORDER);
+MODULE_VERSION(ipfw, 2);
+/* should declare some dependencies here */
+
+/*
+ * Starting up. Done in order after ipfwmod() has been called.
+ * VNET_SYSINIT is also called for each existing vnet and each new vnet.
+ */
+SYSINIT(ipfw_init, IPFW_SI_SUB_FIREWALL, IPFW_MODULE_ORDER,
+	    ipfw_init, NULL);
+VNET_SYSINIT(vnet_ipfw_init, IPFW_SI_SUB_FIREWALL, IPFW_VNET_ORDER,
+	    vnet_ipfw_init, NULL);
+ 
+/*
+ * Closing up shop. These are done in REVERSE ORDER, but still
+ * after ipfwmod() has been called. Not called on reboot.
+ * VNET_SYSUNINIT is also called for each exiting vnet as it exits.
+ * or when the module is unloaded.
+ */
+SYSUNINIT(ipfw_destroy, IPFW_SI_SUB_FIREWALL, IPFW_MODULE_ORDER,
+	    ipfw_destroy, NULL);
+VNET_SYSUNINIT(vnet_ipfw_uninit, IPFW_SI_SUB_FIREWALL, IPFW_VNET_ORDER,
+	    vnet_ipfw_uninit, NULL);
 
  

Modified: head/sys/netinet/ipfw/ip_fw_pfil.c
==============================================================================
--- head/sys/netinet/ipfw/ip_fw_pfil.c	Fri Aug 21 11:17:25 2009	(r196422)
+++ head/sys/netinet/ipfw/ip_fw_pfil.c	Fri Aug 21 11:20:10 2009	(r196423)
@@ -53,6 +53,7 @@ __FBSDID("$FreeBSD$");
 #include <net/if.h>
 #include <net/route.h>
 #include <net/pfil.h>
+#include <net/vnet.h>
 
 #include <netinet/in.h>
 #include <netinet/in_systm.h>
@@ -441,7 +442,7 @@ nodivert:
 	return 1;
 }
 
-static int
+int
 ipfw_hook(void)
 {
 	struct pfil_head *pfh_inet;
@@ -458,7 +459,7 @@ ipfw_hook(void)
 	return 0;
 }
 
-static int
+int
 ipfw_unhook(void)
 {
 	struct pfil_head *pfh_inet;
@@ -476,7 +477,7 @@ ipfw_unhook(void)
 }
 
 #ifdef INET6
-static int
+int
 ipfw6_hook(void)
 {
 	struct pfil_head *pfh_inet6;
@@ -493,7 +494,7 @@ ipfw6_hook(void)
 	return 0;
 }
 
-static int
+int
 ipfw6_unhook(void)
 {
 	struct pfil_head *pfh_inet6;
@@ -517,6 +518,10 @@ ipfw_chg_hook(SYSCTL_HANDLER_ARGS)
 	int enable = *(int *)arg1;
 	int error;
 
+#ifdef VIMAGE /* Since enabling is global, only let base do it. */
+	if (! IS_DEFAULT_VNET(curvnet))
+		return (EPERM);
+#endif
 	error = sysctl_handle_int(oidp, &enable, 0, req);
 	if (error)
 		return (error);
@@ -549,50 +554,3 @@ ipfw_chg_hook(SYSCTL_HANDLER_ARGS)
 	return (0);
 }
 
-static int
-ipfw_modevent(module_t mod, int type, void *unused)
-{
-	int err = 0;
-
-	switch (type) {
-	case MOD_LOAD:
-		if ((err = ipfw_init()) != 0) {
-			printf("ipfw_init() error\n");
-			break;
-		}
-		if ((err = ipfw_hook()) != 0) {
-			printf("ipfw_hook() error\n");
-			break;
-		}
-#ifdef INET6
-		if ((err = ipfw6_hook()) != 0) {
-			printf("ipfw_hook() error\n");
-			break;
-		}
-#endif
-		break;
-
-	case MOD_UNLOAD:
-		if ((err = ipfw_unhook()) > 0)
-			break;
-#ifdef INET6
-		if ((err = ipfw6_unhook()) > 0)
-			break;
-#endif
-		ipfw_destroy();
-		break;
-
-	default:
-		return EOPNOTSUPP;
-		break;
-	}
-	return err;
-}
-
-static moduledata_t ipfwmod = {
-	"ipfw",
-	ipfw_modevent,
-	0
-};
-DECLARE_MODULE(ipfw, ipfwmod, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY - 256);
-MODULE_VERSION(ipfw, 2);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908211120.n7LBKAjw021131>