o Make ipfw set more robust -- now it is possible:

- to show a specific set: ipfw set 3 show
    - to delete rules from the set: ipfw set 9 delete 100 200 300
    - to flush the set: ipfw set 4 flush
    - to reset rules counters in the set: ipfw set 1 zero

PR:		kern/113388
Submitted by:	Andrey V. Elsukov
Approved by:	re (kensmith)
MFC after:	6 weeks

sbin/ipfw/ipfw.8

@@ -1,7 +1,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd May 4, 2007
+.Dd June 16, 2007
 .Dt IPFW 8
 .Os
 .Sh NAME
@@ -14,15 +14,17 @@
 .Ar rule
 .Nm
 .Op Fl acdefnNStT
+.Op Cm set Ar N
 .Brq Cm list | show
 .Op Ar rule | first-last ...
 .Nm
 .Op Fl f | q
+.Op Cm set Ar N
 .Cm flush
 .Nm
 .Op Fl q
+.Op Cm set Ar N
 .Brq Cm delete | zero | resetlog
-.Op Cm set
 .Op Ar number ...
 .Nm
 .Cm enable
@@ -2493,6 +2495,22 @@ terminates, and your ruleset will be left active.
 Otherwise, e.g.\& if
 you cannot access your box, the ruleset will be disabled after
 the sleep terminates thus restoring the previous situation.
+.Pp
+To show rules of the specific set:
+.Pp
+.Dl "ipfw set 18 show"
+.Pp
+To show rules of the disabled set:
+.Pp
+.Dl "ipfw -S set 18 show"
+.Pp
+To clear a specific rule counters of the specific set:
+.Pp
+.Dl "ipfw set 18 zero NN"
+.Pp
+To delete a specific rule of the specific set:
+.Pp
+.Dl "ipfw set 18 delete NN"
 .Ss NAT, REDIRECT AND LSNAT
 First redirect all the traffic to nat instance 123:
 .Pp

sbin/ipfw/ipfw2.c

@@ -74,6 +74,7 @@ int
 		do_expired,		/* display expired dynamic rules */
 		do_compact,		/* show rules in compact mode */
 		do_force,		/* do not ask for confirmation */
+		use_set,		/* work with specified set number */
 		show_sets,		/* display rule sets */
 		test_only,		/* only check syntax */
 		comment_only,		/* only print action and comment */
@@ -2498,6 +2499,7 @@ list(int ac, char *av[], int show_counters)
 	u_long rnum, last;
 	char *endptr;
 	int seen = 0;
+	uint8_t set;
 
 	const int ocmd = do_pipe ? IP_DUMMYNET_GET : IP_FW_GET;
 	int nalloc = 1024;	/* start somewhere... */
@@ -2552,6 +2554,10 @@ list(int ac, char *av[], int show_counters)
 	bcwidth = pcwidth = 0;
 	if (show_counters) {
 		for (n = 0, r = data; n < nstat; n++, r = NEXT(r)) {
+			/* skip rules from another set */
+			if (use_set && r->set != use_set - 1)
+				continue;
+
 			/* packet counter */
 			width = snprintf(NULL, 0, "%llu",
 			    align_uint64(&r->pcnt));
@@ -2567,6 +2573,13 @@ list(int ac, char *av[], int show_counters)
 	}
 	if (do_dynamic && ndyn) {
 		for (n = 0, d = dynrules; n < ndyn; n++, d++) {
+			if (use_set) {
+				/* skip rules from another set */
+				bcopy(&d->rule + sizeof(uint16_t),
+				      &set, sizeof(uint8_t));
+				if (set != use_set - 1)
+					continue;
+			}
 			width = snprintf(NULL, 0, "%llu",
 			    align_uint64(&d->pcnt));
 			if (width > pcwidth)
@@ -2580,14 +2593,24 @@ list(int ac, char *av[], int show_counters)
 	}
 	/* if no rule numbers were specified, list all rules */
 	if (ac == 0) {
-		for (n = 0, r = data; n < nstat; n++, r = NEXT(r) )
+		for (n = 0, r = data; n < nstat; n++, r = NEXT(r)) {
+			if (use_set && r->set != use_set - 1)
+				continue;
 			show_ipfw(r, pcwidth, bcwidth);
+		}
 
 		if (do_dynamic && ndyn) {
 			printf("## Dynamic rules (%d):\n", ndyn);
-			for (n = 0, d = dynrules; n < ndyn; n++, d++)
+			for (n = 0, d = dynrules; n < ndyn; n++, d++) {
+				if (use_set) {
+					bcopy(&d->rule + sizeof(uint16_t),
+					      &set, sizeof(uint8_t));
+					if (set != use_set - 1)
+						continue;
+				}
 				show_dyn_ipfw(d, pcwidth, bcwidth);
 		}
+		}
 		goto done;
 	}
 
@@ -2606,6 +2629,8 @@ list(int ac, char *av[], int show_counters)
 		for (n = seen = 0, r = data; n < nstat; n++, r = NEXT(r) ) {
 			if (r->rulenum > last)
 				break;
+			if (use_set && r->set != use_set - 1)
+				continue;
 			if (r->rulenum >= rnum && r->rulenum <= last) {
 				show_ipfw(r, pcwidth, bcwidth);
 				seen = 1;
@@ -2634,6 +2659,12 @@ list(int ac, char *av[], int show_counters)
 				bcopy(&d->rule, &rulenum, sizeof(rulenum));
 				if (rulenum > rnum)
 					break;
+				if (use_set) {
+					bcopy(&d->rule + sizeof(uint16_t),
+					      &set, sizeof(uint8_t));
+					if (set != use_set - 1)
+						continue;
+				}
 				if (r->rulenum >= rnum && r->rulenum <= last)
 					show_dyn_ipfw(d, pcwidth, bcwidth);
 			}
@@ -2672,6 +2703,7 @@ help(void)
 "		reverse|proxy_only|redirect_addr linkspec|\n"
 "		redirect_port linkspec|redirect_proto linkspec}\n"
 "set [disable N... enable N...] | move [rule] X to Y | swap X Y | show\n"
+"set N {show|list|zero|resetlog|delete} [N{,N}] | flush\n"
 "table N {add ip[/bits] [value] | delete ip[/bits] | flush | list}\n"
 "\n"
 "RULE-BODY:	check-state [PARAMS] | ACTION [PARAMS] ADDR [OPTION_LIST]\n"
@@ -3189,6 +3221,11 @@ delete(int ac, char *av[])
 	av++; ac--;
 	NEED1("missing rule specification");
 	if (ac > 0 && _substrcmp(*av, "set") == 0) {
+		/* Do not allow using the following syntax:
+		 *	ipfw set N delete set M
+		 */
+		if (use_set)
+			errx(EX_DATAERR, "invalid syntax");
 		do_set = 1;	/* delete set */
 		ac--; av++;
 	}
@@ -3214,6 +3251,10 @@ delete(int ac, char *av[])
 				    do_pipe == 1 ? p.pipe_nr : p.fs.fs_nr);
 			}
 		} else {
+			if (use_set)
+				rulenum = (i & 0xffff) | (5 << 24) |
+				    ((use_set - 1) << 16);
+			else
 			rulenum =  (i & 0xffff) | (do_set << 24);
 			i = do_cmd(IP_FW_DEL, &rulenum, sizeof rulenum);
 			if (i) {
@@ -5674,9 +5715,10 @@ add(int ac, char *av[])
 static void
 zero(int ac, char *av[], int optname /* IP_FW_ZERO or IP_FW_RESETLOG */)
 {
-	int rulenum;
+	uint32_t arg, saved_arg;
 	int failed = EX_OK;
 	char const *name = optname == IP_FW_ZERO ?  "ZERO" : "RESETLOG";
+	char const *errstr;
 
 	av++; ac--;
 
@@ -5694,15 +5736,21 @@ zero(int ac, char *av[], int optname /* IP_FW_ZERO or IP_FW_RESETLOG */)
 	while (ac) {
 		/* Rule number */
 		if (isdigit(**av)) {
-			rulenum = atoi(*av);
+			arg = strtonum(*av, 0, 0xffff, &errstr);
+			if (errstr)
+				errx(EX_DATAERR,
+				    "invalid rule number %s\n", *av);
+			saved_arg = arg;
+			if (use_set)
+				arg |= (1 << 24) | ((use_set - 1) << 16);
 			av++;
 			ac--;
-			if (do_cmd(optname, &rulenum, sizeof rulenum)) {
+			if (do_cmd(optname, &arg, sizeof(arg))) {
 				warn("rule %u: setsockopt(IP_FW_%s)",
-				    rulenum, name);
+				    saved_arg, name);
 				failed = EX_UNAVAILABLE;
 			} else if (!do_quiet)
-				printf("Entry %d %s.\n", rulenum,
+				printf("Entry %d %s.\n", saved_arg,
 				    optname == IP_FW_ZERO ?
 					"cleared" : "logging count reset");
 		} else {
@@ -5733,7 +5781,12 @@ flush(int force)
 		if (c == 'N')	/* user said no */
 			return;
 	}
-	if (do_cmd(cmd, NULL, 0) < 0)
+	/* `ipfw set N flush` - is the same that `ipfw delete set N` */
+	if (use_set) {
+		uint32_t arg = ((use_set - 1) & 0xffff) | (1 << 24);
+		if (do_cmd(IP_FW_DEL, &arg, sizeof(arg)) < 0)
+			err(EX_UNAVAILABLE, "setsockopt(IP_FW_DEL)");
+	} else if (do_cmd(cmd, NULL, 0) < 0)
 		err(EX_UNAVAILABLE, "setsockopt(IP_%s_FLUSH)",
 		    do_pipe ? "DUMMYNET" : "FW");
 	if (!do_quiet)
@@ -5944,6 +5997,7 @@ static int
 ipfw_main(int oldac, char **oldav)
 {
 	int ch, ac, save_ac;
+	const char *errstr;
 	char **av, **save_av;
 	int do_acct = 0;		/* Show packet/byte count */
 
@@ -6134,6 +6188,16 @@ ipfw_main(int oldac, char **oldav)
 		do_pipe = 1;
 	else if (_substrcmp(*av, "queue") == 0)
 		do_pipe = 2;
+	else if (!strncmp(*av, "set", strlen(*av))) {
+		if (ac > 1 && isdigit(av[1][0])) {
+			use_set = strtonum(av[1], 0, RESVD_SET, &errstr);
+			if (errstr)
+				errx(EX_DATAERR,
+				    "invalid set number %s\n", av[1]);
+			ac -= 2; av += 2; use_set++;
+		}
+	}
+
 	if (do_pipe || do_nat) {
 		ac--;
 		av++;
@@ -6152,37 +6216,45 @@ ipfw_main(int oldac, char **oldav)
 		av[1] = p;
 	}
 
-	if (_substrcmp(*av, "add") == 0)
+	int try_next = 0;
-		add(ac, av);
+	if (use_set == 0) {
-	else if (do_nat && _substrcmp(*av, "show") == 0)
+		if (_substrcmp(*av, "add") == 0)
- 		show_nat(ac, av);
+			add(ac, av);
-	else if (do_pipe && _substrcmp(*av, "config") == 0)
+		else if (do_nat && _substrcmp(*av, "show") == 0)
-		config_pipe(ac, av);
+ 			show_nat(ac, av);
-	else if (do_nat && _substrcmp(*av, "config") == 0) 
+		else if (do_pipe && _substrcmp(*av, "config") == 0)
- 		config_nat(ac, av);
+			config_pipe(ac, av);
-	else if (_substrcmp(*av, "delete") == 0)
+		else if (do_nat && _substrcmp(*av, "config") == 0)
-		delete(ac, av);
+ 			config_nat(ac, av);
-	else if (_substrcmp(*av, "flush") == 0)
+			else if (_substrcmp(*av, "set") == 0)
-		flush(do_force);
+				sets_handler(ac, av);
-	else if (_substrcmp(*av, "zero") == 0)
+			else if (_substrcmp(*av, "table") == 0)
-		zero(ac, av, IP_FW_ZERO);
+				table_handler(ac, av);
-	else if (_substrcmp(*av, "resetlog") == 0)
+			else if (_substrcmp(*av, "enable") == 0)
-		zero(ac, av, IP_FW_RESETLOG);
+				sysctl_handler(ac, av, 1);
-	else if (_substrcmp(*av, "print") == 0 ||
+			else if (_substrcmp(*av, "disable") == 0)
-	         _substrcmp(*av, "list") == 0)
+				sysctl_handler(ac, av, 0);
-		list(ac, av, do_acct);
+			else
-	else if (_substrcmp(*av, "set") == 0)
+				try_next = 1;
-		sets_handler(ac, av);
+	}
-	else if (_substrcmp(*av, "table") == 0)
+
-		table_handler(ac, av);
+	if (use_set || try_next) {
-	else if (_substrcmp(*av, "enable") == 0)
+		if (_substrcmp(*av, "delete") == 0)
-		sysctl_handler(ac, av, 1);
+			delete(ac, av);
-	else if (_substrcmp(*av, "disable") == 0)
+		else if (_substrcmp(*av, "flush") == 0)
-		sysctl_handler(ac, av, 0);
+			flush(do_force);
-	else if (_substrcmp(*av, "show") == 0)
+		else if (_substrcmp(*av, "zero") == 0)
-		list(ac, av, 1 /* show counters */);
+			zero(ac, av, IP_FW_ZERO);
-	else
+		else if (_substrcmp(*av, "resetlog") == 0)
-		errx(EX_USAGE, "bad command `%s'", *av);
+			zero(ac, av, IP_FW_RESETLOG);
+		else if (_substrcmp(*av, "print") == 0 ||
+		         _substrcmp(*av, "list") == 0)
+			list(ac, av, do_acct);
+		else if (_substrcmp(*av, "show") == 0)
+			list(ac, av, 1 /* show counters */);
+		else
+			errx(EX_USAGE, "bad command `%s'", *av);
+	}
 
 	/* Free memory allocated in the argument parsing. */
 	free_args(save_ac, save_av);

sys/netinet/ip_fw2.c

@@ -3878,6 +3878,7 @@ free_chain(struct ip_fw_chain *chain, int kill_default)
  *	2	move rules with given number to new set
  *	3	move rules with given set number to new set
  *	4	swap sets with given numbers
+ *	5	delete rules with given number and with given set number
  */
 static int
 del_entry(struct ip_fw_chain *chain, u_int32_t arg)
@@ -3890,11 +3891,9 @@ del_entry(struct ip_fw_chain *chain, u_int32_t arg)
 	cmd = (arg >> 24) & 0xff;
 	new_set = (arg >> 16) & 0xff;
 
-	if (cmd > 4)
+	if (cmd > 5 || new_set > RESVD_SET)
 		return EINVAL;
-	if (new_set > RESVD_SET)
+	if (cmd == 0 || cmd == 2 || cmd == 5) {
-		return EINVAL;
-	if (cmd == 0 || cmd == 2) {
 		if (rulenum >= IPFW_DEFAULT_RULE)
 			return EINVAL;
 	} else {
@@ -3958,6 +3957,25 @@ del_entry(struct ip_fw_chain *chain, u_int32_t arg)
 			else if (rule->set == new_set)
 				rule->set = rulenum;
 		break;
+	case 5: /* delete rules with given number and with given set number.
+		 * rulenum - given rule number;
+		 * new_set - given set number.
+		 */
+		for (; rule->rulenum < rulenum; prev = rule, rule = rule->next)
+			;
+		if (rule->rulenum != rulenum) {
+			IPFW_WUNLOCK(chain);
+			return (EINVAL);
+		}
+		flush_rule_ptrs(chain);
+		while (rule->rulenum == rulenum) {
+			if (rule->set == new_set)
+				rule = remove_rule(chain, rule, prev);
+			else {
+				prev = rule;
+				rule = rule->next;
+			}
+		}
 	}
 	/*
 	 * Look for rules to reclaim.  We grab the list before
@@ -3991,23 +4009,39 @@ clear_counters(struct ip_fw *rule, int log_only)
 
 /**
  * Reset some or all counters on firewall rules.
- * @arg frwl is null to clear all entries, or contains a specific
+ * The argument `arg' is an u_int32_t. The low 16 bit are the rule number,
- * rule number.
+ * the next 8 bits are the set number, the top 8 bits are the command:
- * @arg log_only is 1 if we only want to reset logs, zero otherwise.
+ *	0	work with rules from all set's;
+ *	1	work with rules only from specified set.
+ * Specified rule number is zero if we want to clear all entries.
+ * log_only is 1 if we only want to reset logs, zero otherwise.
  */
 static int
-zero_entry(struct ip_fw_chain *chain, int rulenum, int log_only)
+zero_entry(struct ip_fw_chain *chain, u_int32_t arg, int log_only)
 {
 	struct ip_fw *rule;
 	char *msg;
 
+	uint16_t rulenum = arg & 0xffff;
+	uint8_t set = (arg >> 16) & 0xff;
+	uint8_t cmd = (arg >> 24) & 0xff;
+
+	if (cmd > 1)
+		return (EINVAL);
+	if (cmd == 1 && set > RESVD_SET)
+		return (EINVAL);
+
 	IPFW_WLOCK(chain);
 	if (rulenum == 0) {
 		norule_counter = 0;
-		for (rule = chain->rules; rule; rule = rule->next)
+		for (rule = chain->rules; rule; rule = rule->next) {
+			/* Skip rules from another set. */
+			if (cmd == 1 && rule->set != set)
+				continue;
 			clear_counters(rule, log_only);
+		}
 		msg = log_only ? "ipfw: All logging counts reset.\n" :
-				"ipfw: Accounting cleared.\n";
+		    "ipfw: Accounting cleared.\n";
 	} else {
 		int cleared = 0;
 		/*
@@ -4017,7 +4051,8 @@ zero_entry(struct ip_fw_chain *chain, int rulenum, int log_only)
 		for (rule = chain->rules; rule; rule = rule->next)
 			if (rule->rulenum == rulenum) {
 				while (rule && rule->rulenum == rulenum) {
-					clear_counters(rule, log_only);
+					if (cmd == 0 || rule->set == set)
+						clear_counters(rule, log_only);
 					rule = rule->next;
 				}
 				cleared = 1;
@@ -4028,7 +4063,7 @@ zero_entry(struct ip_fw_chain *chain, int rulenum, int log_only)
 			return (EINVAL);
 		}
 		msg = log_only ? "ipfw: Entry %d logging count reset.\n" :
-				"ipfw: Entry %d cleared.\n";
+		    "ipfw: Entry %d cleared.\n";
 	}
 	IPFW_WUNLOCK(chain);
 
@@ -4377,6 +4412,13 @@ ipfw_getrules(struct ip_fw_chain *chain, void *buf, size_t space)
 					bcopy(p, dst, sizeof *p);
 					bcopy(&(p->rule->rulenum), &(dst->rule),
 					    sizeof(p->rule->rulenum));
+					/*
+					 * store set number into high word of
+					 * dst->rule pointer.
+					 */
+					bcopy(&(p->rule->set), &dst->rule +
+					    sizeof(p->rule->rulenum),
+					    sizeof(p->rule->set));
 					/*
 					 * store a non-null value in "next".
 					 * The userland code will interpret a
@@ -4406,7 +4448,7 @@ static int
 ipfw_ctl(struct sockopt *sopt)
 {
 #define	RULE_MAXSIZE	(256*sizeof(u_int32_t))
-	int error, rule_num;
+	int error;
 	size_t size;
 	struct ip_fw *buf, *rule;
 	u_int32_t rulenum[2];
@@ -4524,15 +4566,15 @@ ipfw_ctl(struct sockopt *sopt)
 		break;
 
 	case IP_FW_ZERO:
-	case IP_FW_RESETLOG: /* argument is an int, the rule number */
+	case IP_FW_RESETLOG: /* argument is an u_int_32, the rule number */
-		rule_num = 0;
+		rulenum[0] = 0;
 		if (sopt->sopt_val != 0) {
-		    error = sooptcopyin(sopt, &rule_num,
+		    error = sooptcopyin(sopt, rulenum,
-			    sizeof(int), sizeof(int));
+			    sizeof(u_int32_t), sizeof(u_int32_t));
 		    if (error)
 			break;
 		}
-		error = zero_entry(&layer3_chain, rule_num,
+		error = zero_entry(&layer3_chain, rulenum[0],
 			sopt->sopt_name == IP_FW_RESETLOG);
 		break;