Module Name:    src
Committed By:   christos
Date:           Thu Dec 20 21:42:28 UTC 2012

Modified Files:
        src/sys/external/bsd/ipf/netinet: fil.c ip_compat.h ip_nat.c ip_state.c
            ipf_rb.h

Log Message:
- Replace the seemingly broken built-in ipf rbtree implementation with ours.
- Fix typos in comments
- Fix 2 mutex errors
>From Geoff Adams


To generate a diff of this commit:
cvs rdiff -u -r1.6 -r1.7 src/sys/external/bsd/ipf/netinet/fil.c \
    src/sys/external/bsd/ipf/netinet/ip_nat.c
cvs rdiff -u -r1.4 -r1.5 src/sys/external/bsd/ipf/netinet/ip_compat.h
cvs rdiff -u -r1.3 -r1.4 src/sys/external/bsd/ipf/netinet/ip_state.c \
    src/sys/external/bsd/ipf/netinet/ipf_rb.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/external/bsd/ipf/netinet/fil.c
diff -u src/sys/external/bsd/ipf/netinet/fil.c:1.6 src/sys/external/bsd/ipf/netinet/fil.c:1.7
--- src/sys/external/bsd/ipf/netinet/fil.c:1.6	Tue Oct  9 17:32:54 2012
+++ src/sys/external/bsd/ipf/netinet/fil.c	Thu Dec 20 16:42:27 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: fil.c,v 1.6 2012/10/09 21:32:54 christos Exp $	*/
+/*	$NetBSD: fil.c,v 1.7 2012/12/20 21:42:27 christos Exp $	*/
 
 /*
  * Copyright (C) 2012 by Darren Reed.
@@ -138,7 +138,7 @@ extern struct timeout ipf_slowtimer_ch;
 #if !defined(lint)
 #if defined(__NetBSD__)
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: fil.c,v 1.6 2012/10/09 21:32:54 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fil.c,v 1.7 2012/12/20 21:42:27 christos Exp $");
 #else
 static const char sccsid[] = "@(#)fil.c	1.36 6/5/96 (C) 1993-2000 Darren Reed";
 static const char rcsid[] = "@(#)Id: fil.c,v 1.1.1.2 2012/07/22 13:45:07 darrenr Exp $";
@@ -9436,11 +9436,10 @@ ipf_rule_expire(ipf_main_softc_t *softc)
 }
 
 
-static int ipf_ht_node_cmp(struct host_node_s *, struct host_node_s *);
+static int ipf_ht_node_cmp(const struct host_node_s *, const struct host_node_s *);
 static void ipf_ht_node_make_key(host_track_t *, host_node_t *, int,
 				 i6addr_t *);
 
-host_node_t RBI_ZERO(ipf_rb);
 RBI_CODE(ipf_rb, host_node_t, hn_entry, ipf_ht_node_cmp)
 
 
@@ -9463,7 +9462,7 @@ RBI_CODE(ipf_rb, host_node_t, hn_entry, 
 /* for /48's, etc.                                                          */
 /* ------------------------------------------------------------------------ */
 static int
-ipf_ht_node_cmp(struct host_node_s *k1, struct host_node_s *k2)
+ipf_ht_node_cmp(const struct host_node_s *k1, const struct host_node_s *k2)
 {
 	int i;
 
@@ -9627,7 +9626,7 @@ ipf_ht_node_add(ipf_main_softc_t *softc,
 /* NOTE: THIS FUNCTION MUST BE CALLED WITH AN EXCLUSIVE LOCK THAT PREVENTS  */
 /*       ipf_ht_node_add FROM RUNNING CONCURRENTLY ON THE SAME htp.         */
 /*                                                                          */
-/* Try and find the address passed in amongst the leavese on this tree to   */
+/* Try and find the address passed in amongst the leaves on this tree to    */
 /* be friend. If found then drop the active account for that node drops by  */
 /* one. If that count reaches 0, it is time to free it all up.              */
 /* ------------------------------------------------------------------------ */
@@ -9695,6 +9694,7 @@ ipf_rb_ht_freenode(host_node_t *node, vo
 void
 ipf_rb_ht_flush(host_track_t *head)
 {
+	/* XXX - May use node members after freeing the node. */
 	RBI_WALK(ipf_rb, &head->ht_root, ipf_rb_ht_freenode, NULL);
 }
 
Index: src/sys/external/bsd/ipf/netinet/ip_nat.c
diff -u src/sys/external/bsd/ipf/netinet/ip_nat.c:1.6 src/sys/external/bsd/ipf/netinet/ip_nat.c:1.7
--- src/sys/external/bsd/ipf/netinet/ip_nat.c:1.6	Mon Jul 30 15:27:46 2012
+++ src/sys/external/bsd/ipf/netinet/ip_nat.c	Thu Dec 20 16:42:28 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: ip_nat.c,v 1.6 2012/07/30 19:27:46 pgoyette Exp $	*/
+/*	$NetBSD: ip_nat.c,v 1.7 2012/12/20 21:42:28 christos Exp $	*/
 
 /*
  * Copyright (C) 2012 by Darren Reed.
@@ -113,7 +113,7 @@ extern struct ifnet vpnif;
 #if !defined(lint)
 #if defined(__NetBSD__)
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip_nat.c,v 1.6 2012/07/30 19:27:46 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip_nat.c,v 1.7 2012/12/20 21:42:28 christos Exp $");
 #else
 static const char sccsid[] = "@(#)ip_nat.c	1.11 6/5/96 (C) 1995 Darren Reed";
 static const char rcsid[] = "@(#)Id: ip_nat.c,v 1.1.1.2 2012/07/22 13:45:27 darrenr Exp";
@@ -3032,12 +3032,12 @@ ipf_nat_newrdr(fr_info_t *fin, nat_t *na
 /* Attempts to create a new NAT entry.  Does not actually change the packet */
 /* in any way.                                                              */
 /*                                                                          */
-/* This fucntion is in three main parts: (1) deal with creating a new NAT   */
+/* This function is in three main parts: (1) deal with creating a new NAT   */
 /* structure for a "MAP" rule (outgoing NAT translation); (2) deal with     */
 /* creating a new NAT structure for a "RDR" rule (incoming NAT translation) */
 /* and (3) building that structure and putting it into the NAT table(s).    */
 /*                                                                          */
-/* NOTE: natsave should NOT be used top point back to an ipstate_t struct   */
+/* NOTE: natsave should NOT be used to point back to an ipstate_t struct    */
 /*       as it can result in memory being corrupted.                        */
 /* ------------------------------------------------------------------------ */
 nat_t *
@@ -3353,6 +3353,7 @@ ipf_nat_insert(ipf_main_softc_t *softc, 
 	u_int hv0, hv1;
 	u_int sp, dp;
 	ipnat_t *in;
+	int ret;
 
 	/*
 	 * Try and return an error as early as possible, so calculate the hash
@@ -3435,7 +3436,10 @@ ipf_nat_insert(ipf_main_softc_t *softc, 
 		nat->nat_mtu[1] = GETIFMTU_4(nat->nat_ifps[1]);
 	}
 
-	return ipf_nat_hashtab_add(softc, softn, nat);
+	ret = ipf_nat_hashtab_add(softc, softn, nat);
+	if (ret == -1)
+		MUTEX_DESTROY(&nat->nat_lock);
+	return ret;
 }
 
 

Index: src/sys/external/bsd/ipf/netinet/ip_compat.h
diff -u src/sys/external/bsd/ipf/netinet/ip_compat.h:1.4 src/sys/external/bsd/ipf/netinet/ip_compat.h:1.5
--- src/sys/external/bsd/ipf/netinet/ip_compat.h:1.4	Sat Sep 15 12:56:45 2012
+++ src/sys/external/bsd/ipf/netinet/ip_compat.h	Thu Dec 20 16:42:28 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: ip_compat.h,v 1.4 2012/09/15 16:56:45 plunky Exp $	*/
+/*	$NetBSD: ip_compat.h,v 1.5 2012/12/20 21:42:28 christos Exp $	*/
 
 /*
  * Copyright (C) 2012 by Darren Reed.
@@ -898,6 +898,10 @@ typedef unsigned int    u_32_t;
 #  endif
 # endif
 
+#if (__NetBSD_Version__ >= 699000000)
+#  define HAVE_RBTREE	1
+#endif
+
 # ifdef _KERNEL
 #  include <sys/cprng.h>
 #  if (__NetBSD_Version__ >= 399001400)

Index: src/sys/external/bsd/ipf/netinet/ip_state.c
diff -u src/sys/external/bsd/ipf/netinet/ip_state.c:1.3 src/sys/external/bsd/ipf/netinet/ip_state.c:1.4
--- src/sys/external/bsd/ipf/netinet/ip_state.c:1.3	Sun Jul 22 10:27:51 2012
+++ src/sys/external/bsd/ipf/netinet/ip_state.c	Thu Dec 20 16:42:28 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: ip_state.c,v 1.3 2012/07/22 14:27:51 darrenr Exp $	*/
+/*	$NetBSD: ip_state.c,v 1.4 2012/12/20 21:42:28 christos Exp $	*/
 
 /*
  * Copyright (C) 2012 by Darren Reed.
@@ -100,7 +100,7 @@ struct file;
 #if !defined(lint)
 #if defined(__NetBSD__)
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip_state.c,v 1.3 2012/07/22 14:27:51 darrenr Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip_state.c,v 1.4 2012/12/20 21:42:28 christos Exp $");
 #else
 static const char sccsid[] = "@(#)ip_state.c	1.8 6/5/96 (C) 1993-2000 Darren Reed";
 static const char rcsid[] = "@(#)Id: ip_state.c,v 1.1.1.2 2012/07/22 13:45:37 darrenr Exp";
@@ -1032,7 +1032,7 @@ ipf_state_putent(ipf_main_softc_t *softc
 /* to pointers and adjusts running stats for the hash table as appropriate. */
 /*                                                                          */
 /* This function can fail if the filter rule has had a population policy of */
-/* IP addresses used with stateful filteirng assigned to it.                */
+/* IP addresses used with stateful filtering assigned to it.                */
 /*                                                                          */
 /* Locking: it is assumed that some kind of lock on ipf_state is held.      */
 /*          Exits with is_lock initialised and held - *EVEN IF ERROR*.      */
@@ -1056,7 +1056,7 @@ ipf_state_insert(ipf_main_softc_t *softc
 	}
 
 	/*
-	 * If we could trust is_hv, then the modulous would not be needed,
+	 * If we could trust is_hv, then the modulus would not be needed,
 	 * but when running with IPFILTER_SYNC, this stops bad values.
 	 */
 	hv = is->is_hv % softs->ipf_state_size;
@@ -1638,6 +1638,10 @@ ipf_state_add(ipf_main_softc_t *softc, f
 		SBUMPD(ipf_state_stats, iss_bucket_full);
 		return 4;
 	}
+
+	/*
+	 * No existing state; create new
+	 */
 	KMALLOC(is, ipstate_t *);
 	if (is == NULL) {
 		SBUMPD(ipf_state_stats, iss_nomem);
@@ -1649,7 +1653,7 @@ ipf_state_add(ipf_main_softc_t *softc, f
 	is->is_rule = fr;
 
 	/*
-	 * Do not do the modulous here, it is done in ipf_state_insert().
+	 * Do not do the modulus here, it is done in ipf_state_insert().
 	 */
 	if (fr != NULL) {
 		ipftq_t *tq;
@@ -1677,7 +1681,7 @@ ipf_state_add(ipf_main_softc_t *softc, f
 	/*
 	 * It may seem strange to set is_ref to 2, but if stsave is not NULL
 	 * then a copy of the pointer is being stored somewhere else and in
-	 * the end, it will expect to be able to do osmething with it.
+	 * the end, it will expect to be able to do something with it.
 	 */
 	is->is_me = stsave;
 	if (stsave != NULL) {
@@ -3578,7 +3582,6 @@ ipf_state_del(ipf_main_softc_t *softc, i
 			softs->ipf_state_stats.iss_orphan++;
 		return refs;
 	}
-	MUTEX_EXIT(&is->is_lock);
 
 	fr = is->is_rule;
 	is->is_rule = NULL;
@@ -3590,6 +3593,7 @@ ipf_state_del(ipf_main_softc_t *softc, i
 	}
 
 	is->is_ref = 0;
+	MUTEX_EXIT(&is->is_lock);
 
 	if (is->is_tqehead[0] != NULL) {
 		if (ipf_deletetimeoutqueue(is->is_tqehead[0]) == 0)
Index: src/sys/external/bsd/ipf/netinet/ipf_rb.h
diff -u src/sys/external/bsd/ipf/netinet/ipf_rb.h:1.3 src/sys/external/bsd/ipf/netinet/ipf_rb.h:1.4
--- src/sys/external/bsd/ipf/netinet/ipf_rb.h:1.3	Sun Jul 22 10:27:51 2012
+++ src/sys/external/bsd/ipf/netinet/ipf_rb.h	Thu Dec 20 16:42:28 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: ipf_rb.h,v 1.3 2012/07/22 14:27:51 darrenr Exp $	*/
+/*	$NetBSD: ipf_rb.h,v 1.4 2012/12/20 21:42:28 christos Exp $	*/
 
 /*
  * Copyright (C) 2012 by Darren Reed.
@@ -6,12 +6,69 @@
  * See the IPFILTER.LICENCE file for details on licencing.
  *
  */
+
+/*
+ * If the OS has a red-black tree implementation, use it.
+ */
+#ifdef HAVE_RBTREE
+
+#include <sys/rbtree.h>
+
+# define	RBI_LINK(_n, _t)
+# define	RBI_FIELD(_n)			rb_node_t
+# define	RBI_HEAD(_n, _t)		rb_tree_t
+
+/* Define adapter code between the ipf-specific and the system rb impls. */
+# define	RBI_CODE(_n, _t, _f, _cmp)				\
+signed int _n##_compare_nodes(void *ctx, const void *n1, const void *n2);\
+signed int _n##_compare_key(void *ctx, const void *n1, const void *key);\
+typedef	void	(*_n##_rb_walker_t)(_t *, void *);			\
+void	_n##_rb_walktree(rb_tree_t *, _n##_rb_walker_t, void *);	\
+									\
+static const rb_tree_ops_t _n##_tree_ops = {				\
+        .rbto_compare_nodes = _n##_compare_nodes,			\
+        .rbto_compare_key = _n##_compare_key,				\
+        .rbto_node_offset = offsetof(_t, _f),				\
+        .rbto_context = NULL						\
+};									\
+									\
+int									\
+_n##_compare_nodes(void *ctx, const void *n1, const void *n2) {		\
+	return _cmp(n1, n2);						\
+}									\
+									\
+int									\
+_n##_compare_key(void *ctx, const void *n1, const void *key) {		\
+	return _cmp(n1, key);						\
+}									\
+									\
+void									\
+_n##_rb_walktree(rb_tree_t *head, _n##_rb_walker_t func, void *arg)	\
+{									\
+	_t *rb;								\
+	/* Take advantage of the fact that the ipf code only uses this  \
+	   method to clear the tree, in order to do it more safely. */	\
+	while ((rb = rb_tree_iterate(head, NULL, RB_DIR_RIGHT)) != NULL) {\
+		rb_tree_remove_node(head, rb);				\
+		func(rb, arg);						\
+	}								\
+}
+  
+# define	RBI_DELETE(_n, _h, _v)		rb_tree_remove_node(_h, _v)
+# define	RBI_INIT(_n, _h)		rb_tree_init(_h, &_n##_tree_ops)
+# define	RBI_INSERT(_n, _h, _v)		rb_tree_insert_node(_h, _v)
+# define	RBI_ISEMPTY(_h)			(rb_tree_iterate(_h, NULL, RB_DIR_RIGHT) == NULL)
+# define	RBI_SEARCH(_n, _h, _k)		rb_tree_find_node(_h, _k)
+# define	RBI_WALK(_n, _h, _w, _a)	_n##_rb_walktree(_h, _w, _a)
+
+#else
+
 typedef enum rbcolour_e {
 	C_BLACK = 0,
 	C_RED = 1
 } rbcolour_t;
 
-#define	RBI_LINK(_n, _t)							\
+#define	RBI_LINK(_n, _t)						\
 	struct _n##_rb_link {						\
 		struct _t	*left;					\
 		struct _t	*right;					\
@@ -26,8 +83,12 @@ struct _n##_rb_head {							\
 	int		(* compare)(struct _t *, struct _t *);		\
 }
 
+#define	RBI_FIELD(_n)			struct _n##_rb_link
+
 #define	RBI_CODE(_n, _t, _f, _cmp)					\
 									\
+_t RBI_ZERO(_n);							\
+									\
 typedef	void	(*_n##_rb_walker_t)(_t *, void *);			\
 									\
 _t *	_n##_rb_delete(struct _n##_rb_head *, _t *);			\
@@ -357,10 +418,11 @@ _n##_rb_search(struct _n##_rb_head *head
 }
 
 #define	RBI_DELETE(_n, _h, _v)		_n##_rb_delete(_h, _v)
-#define	RBI_FIELD(_n)			struct _n##_rb_link
 #define	RBI_INIT(_n, _h)		_n##_rb_init(_h)
 #define	RBI_INSERT(_n, _h, _v)		_n##_rb_insert(_h, _v)
 #define	RBI_ISEMPTY(_h)			((_h)->count == 0)
 #define	RBI_SEARCH(_n, _h, _k)		_n##_rb_search(_h, _k)
 #define	RBI_WALK(_n, _h, _w, _a)	_n##_rb_walktree(_h, _w, _a)
 #define	RBI_ZERO(_n)			_n##_rb_zero
+
+#endif

Reply via email to