Make counters on table addresses optional and disabled by default.

Use the 'counters' table option in pf.conf if you actually need them.
If enabled, memory is not allocated until packets match an address.

This saves about 40% memory if counters are not being used, and paves the way
for some more significant cleanups coming soon.

ok henning mpf deraadt

7 files changed, 74 insertions, 28 deletions

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index d3bb2526888..11e60eeddc8 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.547 2008/06/10 04:28:54 henning Exp $	*/
+/*	$OpenBSD: parse.y,v 1.548 2008/06/10 20:55:02 mcbride Exp $	*/
 
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
@@ -1374,6 +1374,8 @@ table_opt	: STRING		{
 				table_opts.flags |= PFR_TFLAG_CONST;
 			else if (!strcmp($1, "persist"))
 				table_opts.flags |= PFR_TFLAG_PERSIST;
+			else if (!strcmp($1, "counters"))
+				table_opts.flags |= PFR_TFLAG_COUNTERS;
 			else {
 				yyerror("invalid table option '%s'", $1);
 				free($1);
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index bd66c3c081a..8bc81ea44ae 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -1,4 +1,4 @@
-.\" $OpenBSD: pfctl.8,v 1.137 2008/05/16 06:41:43 jmc Exp $
+.\" $OpenBSD: pfctl.8,v 1.138 2008/06/10 20:55:02 mcbride Exp $
 .\"
 .\" Copyright (c) 2001 Kjell Wooding.  All rights reserved.
 .\"
@@ -24,7 +24,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: May 16 2008 $
+.Dd $Mdocdate: June 10 2008 $
 .Dt PFCTL 8
 .Os
 .Sh NAME
@@ -521,7 +521,7 @@ attributes.
 The address/network has been cleared (statistics).
 .El
 .Pp
-Each table maintains a set of counters that can be retrieved using the
+Each table can maintain a set of counters that can be retrieved using the
 .Fl v
 flag of
 .Nm .
@@ -532,7 +532,7 @@ FTP server.
 The following commands configure the firewall and send 10 pings to the FTP
 server:
 .Bd -literal -offset indent
-# printf "table <test> { ftp.openbsd.org }\en \e
+# printf "table <test> counters { ftp.openbsd.org }\en \e
     pass out to <test>\en" | pfctl -f-
 # ping -qc10 ftp.openbsd.org
 .Ed
@@ -566,7 +566,7 @@ the number of rules which reference the table, and the global
 packet statistics for the whole table:
 .Bd -literal -offset indent
 # pfctl -vvsTables
---a-r-  test
+--a-r-C test
     Addresses:   1
     Cleared:     Thu Feb 13 18:55:18 2003
     References:  [ Anchors: 0        Rules: 1        ]
@@ -626,6 +626,8 @@ For tables which are referenced (used) by rules.
 .It h
 This flag is set when a table in the main ruleset is hidden by one or more
 tables of the same name from anchors attached below it.
+.It C
+This flag is set when the per-address counters are enabled on the table.
 .El
 .It Fl t Ar table
 Specify the name of the table.
diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index 4f09ad79c77..7368dbe7d3c 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl_parser.c,v 1.239 2008/06/10 04:28:54 henning Exp $ */
+/*	$OpenBSD: pfctl_parser.c,v 1.240 2008/06/10 20:55:02 mcbride Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1027,6 +1027,8 @@ print_tabledef(const char *name, int flags, int addrs,
 		printf(" const");
 	if (flags & PFR_TFLAG_PERSIST)
 		printf(" persist");
+	if (flags & PFR_TFLAG_COUNTERS)
+		printf(" counters");
 	SIMPLEQ_FOREACH(ti, nodes, entries) {
 		if (ti->file) {
 			printf(" file \"%s\"", ti->file);
diff --git a/sbin/pfctl/pfctl_table.c b/sbin/pfctl/pfctl_table.c
index bee57862650..2eef811419f 100644
--- a/sbin/pfctl/pfctl_table.c
+++ b/sbin/pfctl/pfctl_table.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl_table.c,v 1.66 2007/03/01 17:20:54 deraadt Exp $ */
+/*	$OpenBSD: pfctl_table.c,v 1.67 2008/06/10 20:55:02 mcbride Exp $ */
 
 /*
  * Copyright (c) 2002 Cedric Berger
@@ -364,13 +364,14 @@ print_table(struct pfr_table *ta, int verbose, int debug)
 	if (!debug && !(ta->pfrt_flags & PFR_TFLAG_ACTIVE))
 		return;
 	if (verbose) {
-		printf("%c%c%c%c%c%c\t%s",
+		printf("%c%c%c%c%c%c%c\t%s",
 		    (ta->pfrt_flags & PFR_TFLAG_CONST) ? 'c' : '-',
 		    (ta->pfrt_flags & PFR_TFLAG_PERSIST) ? 'p' : '-',
 		    (ta->pfrt_flags & PFR_TFLAG_ACTIVE) ? 'a' : '-',
 		    (ta->pfrt_flags & PFR_TFLAG_INACTIVE) ? 'i' : '-',
 		    (ta->pfrt_flags & PFR_TFLAG_REFERENCED) ? 'r' : '-',
 		    (ta->pfrt_flags & PFR_TFLAG_REFDANCHOR) ? 'h' : '-',
+		    (ta->pfrt_flags & PFR_TFLAG_COUNTERS) ? 'C' : '-',
 		    ta->pfrt_name);
 		if (ta->pfrt_anchor[0])
 			printf("\t%s", ta->pfrt_anchor);
@@ -425,7 +426,7 @@ void
 print_addrx(struct pfr_addr *ad, struct pfr_addr *rad, int dns)
 {
 	char		ch, buf[256] = "{error}";
-	char		fb[] = { ' ', 'M', 'A', 'D', 'C', 'Z', 'X', ' ', 'Y' };
+	char		fb[] = { ' ', 'M', 'A', 'D', 'C', 'Z', 'X', ' ', 'Y', ' ' };
 	unsigned int	fback, hostnet;
 
 	fback = (rad != NULL) ? rad->pfra_fback : ad->pfra_fback;
@@ -474,6 +475,8 @@ print_astats(struct pfr_astats *as, int dns)
 
 	print_addrx(&as->pfras_a, NULL, dns);
 	printf("\tCleared:     %s", ctime(&time));
+ 	if (as->pfras_a.pfra_fback == PFR_FB_NOCOUNT)
+		return;
 	for (dir = 0; dir < PFR_DIR_MAX; dir++)
 		for (op = 0; op < PFR_OP_ADDR_MAX; op++)
 			printf("\t%-12s [ Packets: %-18llu Bytes: %-18llu ]\n",
diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
index 2212333e107..4b3f03dd8ff 100644
--- a/share/man/man5/pf.conf.5
+++ b/share/man/man5/pf.conf.5
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: pf.conf.5,v 1.400 2008/06/10 16:52:10 jmc Exp $
+.\"	$OpenBSD: pf.conf.5,v 1.401 2008/06/10 20:55:01 mcbride Exp $
 .\"
 .\" Copyright (c) 2002, Daniel Hartmeier
 .\" All rights reserved.
@@ -183,6 +183,11 @@ can be used to add or remove addresses from the table at any time, even
 when running with
 .Xr securelevel 7
 = 2.
+.It Ar counters
+The
+.Ar counters
+flag enables per-address packet and byte counters which can be displayed with 
+.Xr pfctl 8 .
 .El
 .Pp
 For example,
@@ -2864,7 +2869,7 @@ antispoof-rule = "antispoof" [ "log" ] [ "quick" ]
 
 table-rule     = "table" "\*(Lt" string "\*(Gt" [ tableopts-list ]
 tableopts-list = tableopts-list tableopts | tableopts
-tableopts      = "persist" | "const" | "file" string |
+tableopts      = "persist" | "const" | "counters" | "file" string |
                  "{" [ tableaddr-list ] "}"
 tableaddr-list = tableaddr-list [ "," ] tableaddr-spec | tableaddr-spec
 tableaddr-spec = [ "!" ] tableaddr [ "/" mask-bits ]
diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
index 207a4f5a6c9..41b6584f984 100644
--- a/sys/net/pf_table.c
+++ b/sys/net/pf_table.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_table.c,v 1.73 2008/05/07 05:14:21 claudio Exp $	*/
+/*	$OpenBSD: pf_table.c,v 1.74 2008/06/10 20:55:02 mcbride Exp $	*/
 
 /*
  * Copyright (c) 2002 Cedric Berger
@@ -127,6 +127,7 @@ struct pfr_walktree {
 struct pool		 pfr_ktable_pl;
 struct pool		 pfr_kentry_pl;
 struct pool		 pfr_kentry_pl2;
+struct pool		 pfr_kcounters_pl;
 struct sockaddr_in	 pfr_sin;
 struct sockaddr_in6	 pfr_sin6;
 union sockaddr_union	 pfr_mask;
@@ -195,6 +196,8 @@ pfr_initialize(void)
 	    "pfrkentry", &pool_allocator_oldnointr);
 	pool_init(&pfr_kentry_pl2, sizeof(struct pfr_kentry), 0, 0, 0,
 	    "pfrkentry2", NULL);
+	pool_init(&pfr_kcounters_pl, sizeof(struct pfr_kcounters), 0, 0, 0,
+	    "pfrkcounters", NULL);
 
 	pfr_sin.sin_len = sizeof(pfr_sin);
 	pfr_sin.sin_family = AF_INET;
@@ -924,8 +927,10 @@ pfr_clstats_kentries(struct pfr_kentryworkq *workq, long tzero, int negchange)
 		s = splsoftnet();
 		if (negchange)
 			p->pfrke_not = !p->pfrke_not;
-		bzero(p->pfrke_packets, sizeof(p->pfrke_packets));
-		bzero(p->pfrke_bytes, sizeof(p->pfrke_bytes));
+		if (p->pfrke_counters) {
+			pool_put(&pfr_kcounters_pl, p->pfrke_counters);
+			p->pfrke_counters = NULL;
+		}
 		splx(s);
 		p->pfrke_tzero = tzero;
 	}
@@ -1075,10 +1080,16 @@ pfr_walktree(struct radix_node *rn, void *arg)
 			pfr_copyout_addr(&as.pfras_a, ke);
 
 			s = splsoftnet();
-			bcopy(ke->pfrke_packets, as.pfras_packets,
-			    sizeof(as.pfras_packets));
-			bcopy(ke->pfrke_bytes, as.pfras_bytes,
-			    sizeof(as.pfras_bytes));
+			if (ke->pfrke_counters) {
+				bcopy(ke->pfrke_counters->pfrkc_packets,
+				    as.pfras_packets, sizeof(as.pfras_packets));
+				bcopy(ke->pfrke_counters->pfrkc_bytes,
+				    as.pfras_bytes, sizeof(as.pfras_bytes));
+			} else {
+				bzero(as.pfras_packets, sizeof(as.pfras_packets));
+				bzero(as.pfras_bytes, sizeof(as.pfras_bytes));
+				as.pfras_a.pfra_fback = PFR_FB_NOCOUNT;
+			}
 			splx(s);
 			as.pfras_tzero = ke->pfrke_tzero;
 
@@ -2043,9 +2054,15 @@ pfr_update_stats(struct pfr_ktable *kt, struct pf_addr *a, sa_family_t af,
 	}
 	kt->pfrkt_packets[dir_out][op_pass]++;
 	kt->pfrkt_bytes[dir_out][op_pass] += len;
-	if (ke != NULL && op_pass != PFR_OP_XPASS) {
-		ke->pfrke_packets[dir_out][op_pass]++;
-		ke->pfrke_bytes[dir_out][op_pass] += len;
+	if (ke != NULL && op_pass != PFR_OP_XPASS &&
+	    (kt->pfrkt_flags & PFR_TFLAG_COUNTERS)) {
+		if (ke->pfrke_counters == NULL)
+			ke->pfrke_counters = pool_get(&pfr_kcounters_pl,
+			    PR_NOWAIT|PR_ZERO);
+		if (ke->pfrke_counters != NULL) {
+			ke->pfrke_counters->pfrkc_packets[dir_out][op_pass]++;
+			ke->pfrke_counters->pfrkc_bytes[dir_out][op_pass] += len;
+		}
 	}
 }
 
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 2088bfadbca..c9a7015bef6 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.272 2008/06/10 19:32:14 henning Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.273 2008/06/10 20:55:02 mcbride Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -918,9 +918,11 @@ RB_PROTOTYPE(pf_anchor_node, pf_anchor, entry_node, pf_anchor_compare);
 #define PFR_TFLAG_INACTIVE	0x00000008
 #define PFR_TFLAG_REFERENCED	0x00000010
 #define PFR_TFLAG_REFDANCHOR	0x00000020
-#define PFR_TFLAG_USRMASK	0x00000003
+#define PFR_TFLAG_COUNTERS	0x00000040
+/* Adjust masks below when adding flags. */
+#define PFR_TFLAG_USRMASK	0x00000043
 #define PFR_TFLAG_SETMASK	0x0000003C
-#define PFR_TFLAG_ALLMASK	0x0000003F
+#define PFR_TFLAG_ALLMASK	0x0000007F
 
 struct pfr_table {
 	char			 pfrt_anchor[MAXPATHLEN];
@@ -931,7 +933,7 @@ struct pfr_table {
 
 enum { PFR_FB_NONE, PFR_FB_MATCH, PFR_FB_ADDED, PFR_FB_DELETED,
 	PFR_FB_CHANGED, PFR_FB_CLEARED, PFR_FB_DUPLICATE,
-	PFR_FB_NOTMATCH, PFR_FB_CONFLICT, PFR_FB_MAX };
+	PFR_FB_NOTMATCH, PFR_FB_CONFLICT, PFR_FB_NOCOUNT, PFR_FB_MAX };
 
 struct pfr_addr {
 	union {
@@ -972,13 +974,23 @@ struct pfr_tstats {
 #define	pfrts_name	pfrts_t.pfrt_name
 #define pfrts_flags	pfrts_t.pfrt_flags
 
+struct pfr_kcounters {
+	u_int64_t		 pfrkc_packets[PFR_DIR_MAX][PFR_OP_ADDR_MAX];
+	u_int64_t		 pfrkc_bytes[PFR_DIR_MAX][PFR_OP_ADDR_MAX];
+};
+
 SLIST_HEAD(pfr_kentryworkq, pfr_kentry);
 struct pfr_kentry {
 	struct radix_node	 pfrke_node[2];
 	union sockaddr_union	 pfrke_sa;
-	u_int64_t		 pfrke_packets[PFR_DIR_MAX][PFR_OP_ADDR_MAX];
-	u_int64_t		 pfrke_bytes[PFR_DIR_MAX][PFR_OP_ADDR_MAX];
 	SLIST_ENTRY(pfr_kentry)	 pfrke_workq;
+	union {
+		
+		struct pfr_kcounters		*pfrke_counters;
+#if 0
+		struct pfr_kroute		*pfrke_route;
+#endif
+	} u;
 	long			 pfrke_tzero;
 	u_int8_t		 pfrke_af;
 	u_int8_t		 pfrke_net;
@@ -986,6 +998,9 @@ struct pfr_kentry {
 	u_int8_t		 pfrke_mark;
 	u_int8_t		 pfrke_intrpool;
 };
+#define pfrke_counters	u.pfrke_counters
+#define pfrke_route	u.pfrke_route
+
 
 SLIST_HEAD(pfr_ktableworkq, pfr_ktable);
 RB_HEAD(pfr_ktablehead, pfr_ktable);