Skip to contentSkip to conversation

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

PATCH Superseded Phil Oester 2 participants 4 messages Jun 27, 2013 – Jul 2, 2013 netfilter

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Phil Oester

author
#0 lore raw

PATCH 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]>
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.c +11 -3
index d819b30..7ec0f75 100644
@@ -36,8 +36,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 +51,17 @@ 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);
+
+	qsort(tables, count, sizeof(char *), stringcmp);
+	for (i = 0 ; i < count ; i++) {
+		ret &= func(tables[i]);
+	}
+
 	return ret;
 }
iptables/iptables-save.c +11 -3
index e599fce..802f94d 100644
@@ -34,8 +34,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/ip_tables_names", "re");

@@ -48,10 +49,17 @@ 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);
+
+	qsort(tables, count, sizeof(char *), stringcmp);
+	for (i = 0 ; i < count ; i++) {
+		ret &= func(tables[i]);
+	}
+
 	return ret;
 }
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);
+}
author
#2 lore raw
Quoted 2 lines · Maciej Żenczykowski

I haven't looked at the patch, but I'm guessing there should be a way to disable this.

Why? Today the behavior is random. If you added a NAT table rule before a FORWARD table rule, the NAT rules would be at the bottom of the iptables-save output. You're suggesting that completely random behavior should be the _default_? Isn't deterministic behavior a better default?

Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 5 lines

Why? Today the behavior is random. If you added a NAT table rule before a FORWARD table rule, the NAT rules would be at the bottom of the iptables-save output. You're suggesting that completely random behavior should be the _default_? Isn't deterministic behavior a better default?

I didn't say it should be the default.
I said there should be a way to disable the sorting. Getting a dump as close as possible to the kernels view of the situation is potentially desirable when debugging stuff.