svn commit: r193516 - in head: sbin/ipfw sys/netinet

[Luigi Rizzo]

Author: luigi
Date: Fri Jun  5 16:16:07 2009
New Revision: 193516
URL: http://svn.freebsd.org/changeset/base/193516

Log:
  Several ipfw options and actions use a 16-bit argument to indicate
  pipes, queues, tags, rule numbers and so on.
  These are all different namespaces, and the only thing they have in
  common is the fact they use a 16-bit slot to represent the argument.
  
  There is some confusion in the code, mostly for historical reasons,
  on how the values 0 and 65535 should be used. At the moment, 0 is
  forbidden almost everywhere, while 65535 is used to represent a
  'tablearg' argument, i.e. the result of the most recent table() lookup.
  
  For now, try to use explicit constants for the min and max allowed
  values, and do not overload the default rule number for that.
  
  Also, make the MTAG_IPFW declaration only visible to the kernel.
  
  NOTE: I think the issue needs to be revisited before 8.0 is out:
  the 2^16 namespace limit for rule numbers and pipe/queue is
  annoying, and we can easily bump the limit to 2^32 which gives
  a lot more flexibility in partitioning the namespace.
  
  MFC after:	5 days

Modified:
  head/sbin/ipfw/ipfw2.c
  head/sys/netinet/ip_fw.h

Modified: head/sbin/ipfw/ipfw2.c
==============================================================================
--- head/sbin/ipfw/ipfw2.c	Fri Jun  5 15:57:07 2009	(r193515)
+++ head/sbin/ipfw/ipfw2.c	Fri Jun  5 16:16:07 2009	(r193516)
@@ -2859,7 +2859,7 @@ chkarg:	
 			if (have_tag)
 				errx(EX_USAGE, "tag and untag cannot be "
 				    "specified more than once");
-			GET_UINT_ARG(tag, 1, IPFW_DEFAULT_RULE - 1, i,
+			GET_UINT_ARG(tag, IPFW_ARG_MIN, IPFW_ARG_MAX, i,
 			   rule_action_params);
 			have_tag = cmd;
 			fill_cmd(cmd, O_TAG, (i == TOK_TAG) ? 0: F_NOT, tag);
@@ -3336,7 +3336,7 @@ read_options:
 			if (c->limit_mask == 0)
 				errx(EX_USAGE, "limit: missing limit mask");
 
-			GET_UINT_ARG(c->conn_limit, 1, IPFW_DEFAULT_RULE - 1,
+			GET_UINT_ARG(c->conn_limit, IPFW_ARG_MIN, IPFW_ARG_MAX,
 			    TOK_LIMIT, rule_options);
 
 			ac--; av++;
@@ -3464,7 +3464,7 @@ read_options:
 			else {
 				uint16_t tag;
 
-				GET_UINT_ARG(tag, 1, IPFW_DEFAULT_RULE - 1,
+				GET_UINT_ARG(tag, IPFW_ARG_MIN, IPFW_ARG_MAX,
 				    TOK_TAGGED, rule_options);
 				fill_cmd(cmd, O_TAGGED, 0, tag);
 			}

Modified: head/sys/netinet/ip_fw.h
==============================================================================
--- head/sys/netinet/ip_fw.h	Fri Jun  5 15:57:07 2009	(r193515)
+++ head/sys/netinet/ip_fw.h	Fri Jun  5 16:16:07 2009	(r193516)
@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2002 Luigi Rizzo, Universita` di Pisa
+ * Copyright (c) 2002-2009 Luigi Rizzo, Universita` di Pisa
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -43,6 +43,20 @@
 #define	IPFW_TABLES_MAX		128
 
 /*
+ * Most commands (queue, pipe, tag, untag, limit...) can have a 16-bit
+ * argument between 1 and 65534. The value 0 is unused, the value
+ * 65535 (IP_FW_TABLEARG) is used to represent 'tablearg', i.e. the
+ * can be 1..65534, or 65535 to indicate the use of a 'tablearg'
+ * result of the most recent table() lookup.
+ * Note that 16bit is only a historical limit, resulting from
+ * the use of a 16-bit fields for that value. In reality, we can have
+ * 2^32 pipes, queues, tag values and so on, and use 0 as a tablearg.
+ */
+#define	IPFW_ARG_MIN		1
+#define	IPFW_ARG_MAX		65534
+#define IP_FW_TABLEARG		65535	/* XXX should use 0 */
+
+/*
  * The kernel representation of ipfw rules is made of a list of
  * 'instructions' (for all practical purposes equivalent to BPF
  * instructions), which specify which fields of the packet
@@ -239,8 +253,6 @@ typedef struct	_ipfw_insn {	/* template 
  */
 #define	F_INSN_SIZE(t)	((sizeof (t))/sizeof(u_int32_t))
 
-#define MTAG_IPFW	1148380143	/* IPFW-tagged cookie */
-
 /*
  * This is used to store an array of 16-bit entries (ports etc.)
  */
@@ -558,13 +570,13 @@ typedef struct	_ipfw_table {
 	ipfw_table_entry ent[0];	/* entries			*/
 } ipfw_table;
 
-#define IP_FW_TABLEARG	65535
-
 /*
  * Main firewall chains definitions and global var's definitions.
  */
 #ifdef _KERNEL
 
+#define MTAG_IPFW	1148380143	/* IPFW-tagged cookie */
+
 /* Return values from ipfw_chk() */
 enum {
 	IP_FW_PASS = 0,