Skip to contentSkip to conversation

iptables: Sort table names in ip[6]tables-save

PATCH Superseded Phil Oester 2 participants 3 messages Jul 3, 2013 – Jul 8, 2013 netfilter

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Phil Oester

author
#0 lore raw

PATCH v2 1/1 iptables: Sort table names in ip[6]tables-save

Depending upon the load order of rules, the output from ip[6]tables-save will vary, as ip[6]_tables_names is sorted LIFO. As reported by Linus van Geuns, this makes comparing output from ip[6]tables-save across reboots difficult. Fix this by sorting table names prior to walking the tables, making output consistent.

This closes bugzilla #580.

Phil

Signed-off-by Phil Oester <[email protected]>

---
v2: Add option to disable sorting - feedback from Maciej Żenczykowski

include/xtables.h +2 -0
index c35a6e6..dc6e566 100644
@@ -479,6 +479,8 @@ extern void xtables_ip6parse_any(const char *, struct in6_addr **,
 extern void xtables_ip6parse_multiple(const char *, struct in6_addr **,
 	struct in6_addr **, unsigned int *);
 
+extern int stringcmp(const void *, const void *);
+
 /**
  * Print the specified value to standard output, quoting dangerous
  * characters if required.
iptables/ip6tables-save.8 +4 -0
index 457be82..cc70e43 100644
@@ -39,6 +39,10 @@ include the current values of all packet and byte counters in the output
 \fB\-t\fR, \fB\-\-table\fR \fItablename\fP
 restrict output to only one table. If not specified, output includes all
 available tables.
+.TP
+\fB\-u\fR, \fB\-\-unsorted\fR
+by default, tables are output in alphabetical order.  This option disables
+sorting, and tables are output in LIFO order.
 .SH BUGS
 None known as of iptables-1.2.1 release
 .SH AUTHORS
iptables/ip6tables-save.c +19 -4
index d819b30..3a51fe4 100644
@@ -9,6 +9,7 @@
 #include <sys/errno.h>
 #include <stdio.h>
 #include <fcntl.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <time.h>

@@ -23,12 +24,14 @@
 #endif
 
 static int show_counters = 0;
+static bool sort_tables = true;
 
 static const struct option options[] = {
 	{.name = "counters", .has_arg = false, .val = 'c'},
 	{.name = "dump",     .has_arg = false, .val = 'd'},
 	{.name = "table",    .has_arg = true,  .val = 't'},
 	{.name = "modprobe", .has_arg = true,  .val = 'M'},
+	{.name = "unsorted", .has_arg = false, .val = 'u'},
 	{NULL},
 };
 

@@ -36,8 +39,9 @@ static const struct option options[] = {
 /* Debugging prototype. */
 static int for_each_table(int (*func)(const char *tablename))
 {
-	int ret = 1;
+	int i, count = 0, ret = 1;
 	FILE *procfile = NULL;
+	char **tables = NULL;
 	char tablename[XT_TABLE_MAXNAMELEN+1];
 
 	procfile = fopen("/proc/net/ip6_tables_names", "re");

@@ -50,10 +54,18 @@ static int for_each_table(int (*func)(const char *tablename))
 				   "Badly formed tablename `%s'\n",
 				   tablename);
 		tablename[strlen(tablename) - 1] = '\0';
-		ret &= func(tablename);
+		count++;
+		tables = (char **)realloc(tables, sizeof(char*)*count);
+		tables[count-1] = strdup(tablename);
 	}
-
 	fclose(procfile);
+
+	if (sort_tables == true)
+		qsort(tables, count, sizeof(char *), stringcmp);
+	for (i = 0 ; i < count ; i++) {
+		ret &= func(tables[i]);
+	}
+
 	return ret;
 }
 

@@ -141,7 +153,7 @@ int ip6tables_save_main(int argc, char *argv[])
 	init_extensions6();
 #endif
 
-	while ((c = getopt_long(argc, argv, "bcdt:", options, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, "bcdt:u", options, NULL)) != -1) {
 		switch (c) {
 		case 'c':
 			show_counters = 1;

@@ -154,6 +166,9 @@ int ip6tables_save_main(int argc, char *argv[])
 		case 'M':
 			xtables_modprobe_program = optarg;
 			break;
+		case 'u':
+			sort_tables = false;
+			break;
 		case 'd':
 			do_output(tablename);
 			exit(0);
iptables/iptables-save.8 +4 -0
index c2e0a94..862fb9d 100644
@@ -39,6 +39,10 @@ include the current values of all packet and byte counters in the output
 \fB\-t\fR, \fB\-\-table\fR \fItablename\fP
 restrict output to only one table. If not specified, output includes all
 available tables.
+.TP
+\fB\-u\fR, \fB\-\-unsorted\fR
+by default, tables are output in alphabetical order.  This option disables
+sorting, and tables are output in LIFO order.
 .SH BUGS
 None known as of iptables-1.2.1 release
 .SH AUTHOR
iptables/iptables-save.c +19 -4
index e599fce..c194d9f 100644
@@ -9,6 +9,7 @@
 #include <sys/errno.h>
 #include <stdio.h>
 #include <fcntl.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <time.h>

@@ -22,20 +23,23 @@
 #endif
 
 static int show_counters = 0;
+static bool sort_tables = true;
 
 static const struct option options[] = {
 	{.name = "counters", .has_arg = false, .val = 'c'},
 	{.name = "dump",     .has_arg = false, .val = 'd'},
 	{.name = "table",    .has_arg = true,  .val = 't'},
 	{.name = "modprobe", .has_arg = true,  .val = 'M'},
+	{.name = "unsorted", .has_arg = false, .val = 'u'},
 	{NULL},
 };
 
 /* Debugging prototype. */
 static int for_each_table(int (*func)(const char *tablename))
 {
-	int ret = 1;
+	int i, count = 0, ret = 1;
 	FILE *procfile = NULL;
+	char **tables = NULL;
 	char tablename[XT_TABLE_MAXNAMELEN+1];
 
 	procfile = fopen("/proc/net/ip_tables_names", "re");

@@ -48,10 +52,18 @@ static int for_each_table(int (*func)(const char *tablename))
 				   "Badly formed tablename `%s'\n",
 				   tablename);
 		tablename[strlen(tablename) - 1] = '\0';
-		ret &= func(tablename);
+		count++;
+		tables = (char **)realloc(tables, sizeof(char*)*count);
+		tables[count-1] = strdup(tablename);
 	}
-
 	fclose(procfile);
+
+	if (sort_tables == true)
+		qsort(tables, count, sizeof(char *), stringcmp);
+	for (i = 0 ; i < count ; i++) {
+		ret &= func(tables[i]);
+	}
+
 	return ret;
 }
 

@@ -140,7 +152,7 @@ iptables_save_main(int argc, char *argv[])
 	init_extensions4();
 #endif
 
-	while ((c = getopt_long(argc, argv, "bcdt:", options, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, "bcdt:u", options, NULL)) != -1) {
 		switch (c) {
 		case 'c':
 			show_counters = 1;

@@ -153,6 +165,9 @@ iptables_save_main(int argc, char *argv[])
 		case 'M':
 			xtables_modprobe_program = optarg;
 			break;
+		case 'u':
+			sort_tables = false;
+			break;
 		case 'd':
 			do_output(tablename);
 			exit(0);
libxtables/xtables.c +7 -0
index ebc77b6..ca94f4e 100644
@@ -1957,3 +1957,10 @@ void get_kernel_version(void)
 	sscanf(uts.release, "%d.%d.%d", &x, &y, &z);
 	kernel_version = LINUX_VERSION(x, y, z);
 }
+
+int stringcmp(const void *a, const void *b) 
+{ 
+	const char **ia = (const char **)a;
+	const char **ib = (const char **)b;
+	return strcmp(*ia, *ib);
+}
maintainer
#1 lore raw
Quoted 5 lines · Phil Oester

Depending upon the load order of rules, the output from ip[6]tables-save will vary, as ip[6]_tables_names is sorted LIFO. As reported by Linus van Geuns, this makes comparing output from ip[6]tables-save across reboots difficult. Fix this by sorting table names prior to walking the tables, making output consistent.

Better add an option to explicitly request the sorting, so we stick to the old behaviour by default.

But, how can the unsorted table output be useful?

author
#2 lore raw
Quoted 9 lines · Pablo Neira Ayuso
Quoted 5 lines · Phil Oester

Depending upon the load order of rules, the output from ip[6]tables-save will vary, as ip[6]_tables_names is sorted LIFO. As reported by Linus van Geuns, this makes comparing output from ip[6]tables-save across reboots difficult. Fix this by sorting table names prior to walking the tables, making output consistent.

Better add an option to explicitly request the sorting, so we stick to the old behaviour by default.

The old behavior is random depending upon module load order. We should keep random behavior?

Quoted 1 line

But, how can the unsorted table output be useful?

Ask Maciej - he is the one that requested this be provided as an option.

Phil