xtables: Add an interval option for xtables lock waitcm-14.1cm-14.0

ip[6]tables currently waits for 1 second for the xtables lock to be
freed if the -w option is used. We have seen that the lock is held
much less than that resulting in unnecessary delay when trying to
acquire the lock. This problem is even severe in case of latency
sensitive applications.

Introduce a new option 'W' to specify the wait interval in microseconds.
If this option is not specified, the command sleeps for 1 second by
default.

v1->v2: Change behavior to take millisecond sleep as an argument to
-w as suggested by Pablo. Also maintain current behavior for -w to
sleep for 1 second as mentioned by Liping.

v2->v3: Move the millisecond behavior to a new option as suggested
by Pablo.

v3->v4: Use select instead of usleep. Sleep every iteration for
the time specified in the "-W" argument. Update man page.

v4->v5: Fix compilation error when enabling nftables

v5->v6: Simplify -W so it only takes the interval wait in microseconds.
Bail out if -W is specific but -w is not.

Joint work with Pablo Neira.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
[subashab@codeaurora.org: Fix trivial merge conflicts]

CRs-Fixed: 982615
Change-Id: I2ff76167928db61a5b86108ce8d7c8a72dc5aec4

7 files changed, 180 insertions, 26 deletions

diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c
index 31bc502..2993667 100644
--- a/iptables/ip6tables-restore.c
+++ b/iptables/ip6tables-restore.c
@@ -14,6 +14,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <time.h>
 #include "ip6tables.h"
 #include "xshared.h"
 #include "xtables.h"
@@ -36,7 +37,8 @@ static const struct option options[] = {
 	{.name = "test",     .has_arg = false, .val = 't'},
 	{.name = "help",     .has_arg = false, .val = 'h'},
 	{.name = "noflush",  .has_arg = false, .val = 'n'},
-	{.name = "wait",     .has_arg = false, .val = 'w'},
+	{.name = "wait",     .has_arg = true,  .val = 'w'},
+	{.name = "wait-interval", .has_arg = true, .val = 'W'},
 	{.name = "modprobe", .has_arg = true,  .val = 'M'},
 	{.name = "table",    .has_arg = true,  .val = 'T'},
 	{NULL},
@@ -46,7 +48,7 @@ static void print_usage(const char *name, const char *version) __attribute__((no
 
 static void print_usage(const char *name, const char *version)
 {
-	fprintf(stderr, "Usage: %s [-b] [-c] [-v] [-t] [-h] [-w]\n"
+	fprintf(stderr, "Usage: %s [-b] [-c] [-v] [-t] [-h] [-w] [-W]\n"
 			"	   [ --binary ]\n"
 			"	   [ --counters ]\n"
 			"	   [ --verbose ]\n"
@@ -54,6 +56,7 @@ static void print_usage(const char *name, const char *version)
 			"	   [ --help ]\n"
 			"	   [ --noflush ]\n"
 			"	   [ --wait ]\n"
+			"	   [ --wait-interval ]\n"
 			"          [ --modprobe=<command>]\n", name);
 
 	exit(1);
@@ -191,7 +194,10 @@ int ip6tables_restore_main(int argc, char *argv[])
 	int in_table = 0, testing = 0;
 	const char *tablename = NULL;
 	const struct xtc_ops *ops = &ip6tc_ops;
-
+	struct timeval wait_interval = {
+		.tv_sec		= 1,
+	};
+	bool wait_interval_set = false;
 	line = 0;
 
 	ip6tables_globals.program_name = "ip6tables-restore";
@@ -207,7 +213,7 @@ int ip6tables_restore_main(int argc, char *argv[])
 	init_extensions6();
 #endif
 
-	while ((c = getopt_long(argc, argv, "bcvthnwM:T:", options, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, "bcvthnwM:W:T:", options, NULL)) != -1) {
 		switch (c) {
 			case 'b':
 				binary = 1;
@@ -229,7 +235,26 @@ int ip6tables_restore_main(int argc, char *argv[])
 				noflush = 1;
 				break;
 			case 'w':
-				wait = 1;
+				wait = -1;
+				if (optarg) {
+					if (sscanf(optarg, "%i", &wait) != 1)
+						xtables_error(PARAMETER_PROBLEM,
+							"wait seconds not numeric");
+				} else if (optind < argc && argv[optind][0] != '-'
+							&& argv[optind][0] != '!')
+					if (sscanf(argv[optind++], "%i", &wait) != 1)
+						xtables_error(PARAMETER_PROBLEM,
+							"wait seconds not numeric");
+				break;
+			case 'W':
+				if (optarg)
+					parse_wait_interval(optarg, &wait_interval);
+				else if (optind < argc &&
+					argv[optind][0] != '-' &&
+					argv[optind][0] != '!')
+					parse_wait_interval(argv[optind++],
+					&wait_interval);
+				wait_interval_set = true;
 				break;
 			case 'M':
 				xtables_modprobe_program = optarg;
@@ -254,7 +279,7 @@ int ip6tables_restore_main(int argc, char *argv[])
 	}
 	else in = stdin;
 
-	if (!xtables_lock(wait)) {
+	if (!xtables_lock(wait, &wait_interval)) {
 		fprintf(stderr, "Another app is currently holding the xtables lock. "
 			"Perhaps you want to use the -w option?\n");
 		exit(RESOURCE_PROBLEM);
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 24abc31..682c742 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -103,6 +103,7 @@ static struct option original_opts[] = {
 	{.name = "out-interface", .has_arg = 1, .val = 'o'},
 	{.name = "verbose",       .has_arg = 0, .val = 'v'},
 	{.name = "wait",          .has_arg = 2, .val = 'w'},
+	{.name = "wait-interval", .has_arg = 2, .val = 'W'},
 	{.name = "exact",         .has_arg = 0, .val = 'x'},
 	{.name = "version",       .has_arg = 0, .val = 'V'},
 	{.name = "help",          .has_arg = 2, .val = 'h'},
@@ -258,7 +259,10 @@ exit_printhelp(const struct xtables_rule_match *matches)
 "				network interface name ([+] for wildcard)\n"
 "  --table	-t table	table to manipulate (default: `filter')\n"
 "  --verbose	-v		verbose mode\n"
-"  --wait	-w [seconds]	wait for the xtables lock\n"
+"  --wait	-w [seconds]	maximum wait to acquire xtables lock before give up\n"
+"  --wait-interval -W [usecs]	wait time to try to acquire xtables lock\n"
+"				interval to wait for xtables lock\n"
+"				default is 1 second\n"
 "  --line-numbers		print line numbers when listing\n"
 "  --exact	-x		expand numbers (display exact values)\n"
 /*"[!] --fragment	-f		match second or further fragments only\n"*/
@@ -1297,6 +1301,10 @@ int do_command6(int argc, char *argv[], char **table,
 
 	int verbose = 0;
 	int wait = 0;
+	struct timeval wait_interval = {
+		.tv_sec	= 1,
+	};
+	bool wait_interval_set = false;
 	const char *chain = NULL;
 	const char *shostnetworkmask = NULL, *dhostnetworkmask = NULL;
 	const char *policy = NULL, *newname = NULL;
@@ -1332,7 +1340,7 @@ int do_command6(int argc, char *argv[], char **table,
 
 	opts = xt_params->orig_opts;
 	while ((cs.c = getopt_long(argc, argv,
-	   "-:A:C:D:R:I:L::S::M:F::Z::N:X::E:P:Vh::o:p:s:d:j:i:bvw::nt:m:xc:g:46",
+	   "-:A:C:D:R:I:L::S::M:F::Z::N:X::E:P:Vh::o:p:s:d:j:i:bvw::W::nt:m:xc:g:46",
 					   opts, NULL)) != -1) {
 		switch (cs.c) {
 			/*
@@ -1595,6 +1603,23 @@ int do_command6(int argc, char *argv[], char **table,
 						"wait seconds not numeric");
 			break;
 
+		case 'W':
+			if (restore) {
+				xtables_error(PARAMETER_PROBLEM,
+					      "You cannot use `-W' from "
+					      "ip6tables-restore");
+			}
+			if (optarg)
+				parse_wait_interval(optarg, &wait_interval);
+			else if (optind < argc &&
+				argv[optind][0] != '-' &&
+				argv[optind][0] != '!')
+				parse_wait_interval(argv[optind++],
+						    &wait_interval);
+
+			wait_interval_set = true;
+			break;
+
 		case 'm':
 			command_match(&cs);
 			break;
@@ -1699,6 +1724,10 @@ int do_command6(int argc, char *argv[], char **table,
 		cs.invert = FALSE;
 	}
 
+	if (!wait && wait_interval_set)
+		xtables_error(PARAMETER_PROBLEM,
+			      "--wait-interval only makes sense with --wait\n");
+
 	for (matchp = cs.matches; matchp; matchp = matchp->next)
 		xtables_option_mfcall(matchp->match);
 	if (cs.target != NULL)
@@ -1747,7 +1776,7 @@ int do_command6(int argc, char *argv[], char **table,
 			   chain, XT_EXTENSION_MAXNAMELEN);
 
 	/* Attempt to acquire the xtables lock */
-	if (!restore && !xtables_lock(wait)) {
+	if (!restore && !xtables_lock(wait, &wait_interval)) {
 		fprintf(stderr, "Another app is currently holding the xtables lock. ");
 		if (wait == 0)
 			fprintf(stderr, "Perhaps you want to use the -w option?\n");
diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
index 2009f73..09a0f1c 100644
--- a/iptables/iptables-restore.c
+++ b/iptables/iptables-restore.c
@@ -11,6 +11,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <time.h>
 #include "iptables.h"
 #include "xshared.h"
 #include "xtables.h"
@@ -33,7 +34,8 @@ static const struct option options[] = {
 	{.name = "test",     .has_arg = false, .val = 't'},
 	{.name = "help",     .has_arg = false, .val = 'h'},
 	{.name = "noflush",  .has_arg = false, .val = 'n'},
-	{.name = "wait",     .has_arg = false, .val = 'w'},
+	{.name = "wait",     .has_arg = true,  .val = 'w'},
+	{.name = "wait-interval", .has_arg = true, .val = 'W'},
 	{.name = "modprobe", .has_arg = true,  .val = 'M'},
 	{.name = "table",    .has_arg = true,  .val = 'T'},
 	{NULL},
@@ -53,6 +55,7 @@ static void print_usage(const char *name, const char *version)
 			"	   [ --help ]\n"
 			"	   [ --noflush ]\n"
 			"	   [ --wait ]\n"
+			"	   [ --wait-interval ]\n"
 			"	   [ --table=<TABLE> ]\n"
 			"          [ --modprobe=<command>]\n", name);
 
@@ -191,7 +194,10 @@ iptables_restore_main(int argc, char *argv[])
 	int in_table = 0, testing = 0;
 	const char *tablename = NULL;
 	const struct xtc_ops *ops = &iptc_ops;
-
+	struct timeval wait_interval = {
+		.tv_sec		= 1,
+	};
+	bool wait_interval_set = false;
 	line = 0;
 
 	iptables_globals.program_name = "iptables-restore";
@@ -229,7 +235,26 @@ iptables_restore_main(int argc, char *argv[])
 				noflush = 1;
 				break;
 			case 'w':
-				wait = 1;
+				wait = -1;
+				if (optarg) {
+					if (sscanf(optarg, "%i", &wait) != 1)
+						xtables_error(PARAMETER_PROBLEM,
+							"wait seconds not numeric");
+				} else if (optind < argc && argv[optind][0] != '-'
+							&& argv[optind][0] != '!')
+					if (sscanf(argv[optind++], "%i", &wait) != 1)
+						xtables_error(PARAMETER_PROBLEM,
+							"wait seconds not numeric");
+				break;
+			case 'W':
+				if (optarg)
+					parse_wait_interval(optarg, &wait_interval);
+				else if (optind < argc &&
+					argv[optind][0] != '-' &&
+					argv[optind][0] != '!')
+					parse_wait_interval(argv[optind++],
+					&wait_interval);
+				wait_interval_set = true;
 				break;
 			case 'M':
 				xtables_modprobe_program = optarg;
@@ -254,7 +279,7 @@ iptables_restore_main(int argc, char *argv[])
 	}
 	else in = stdin;
 
-	if (!xtables_lock(wait)) {
+	if (!xtables_lock(wait, &wait_interval)) {
 		fprintf(stderr, "Another app is currently holding the xtables lock. "
 			"Perhaps you want to use the -w option?\n");
 		exit(RESOURCE_PROBLEM);
diff --git a/iptables/iptables.8.in b/iptables/iptables.8.in
index 1c08095..7c424cf 100644
--- a/iptables/iptables.8.in
+++ b/iptables/iptables.8.in
@@ -359,6 +359,13 @@ the program will exit if the lock cannot be obtained.  This option will
 make the program wait (indefinitely or for optional \fIseconds\fP) until
 the exclusive lock can be obtained.
 .TP
+\fB\-W\fP, \fB\-\-wait-interval\fP \fImicroseconds\fP
+Interval to wait per each iteration.
+When running latency sensitive applications, waiting for the xtables lock
+for extended durations may not be acceptable. This option will make each
+iteration take the amount of time specified. The default interval is
+1 second. This option only works with \fB\-w\fP.
+.TP
 \fB\-n\fP, \fB\-\-numeric\fP
 Numeric output.
 IP addresses and port numbers will be printed in numeric format.
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 8048e36..1dc1cfb 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -100,6 +100,7 @@ static struct option original_opts[] = {
 	{.name = "out-interface", .has_arg = 1, .val = 'o'},
 	{.name = "verbose",       .has_arg = 0, .val = 'v'},
 	{.name = "wait",          .has_arg = 2, .val = 'w'},
+	{.name = "wait-interval", .has_arg = 2, .val = 'W'},
 	{.name = "exact",         .has_arg = 0, .val = 'x'},
 	{.name = "fragments",     .has_arg = 0, .val = 'f'},
 	{.name = "version",       .has_arg = 0, .val = 'V'},
@@ -252,7 +253,9 @@ exit_printhelp(const struct xtables_rule_match *matches)
 "				network interface name ([+] for wildcard)\n"
 "  --table	-t table	table to manipulate (default: `filter')\n"
 "  --verbose	-v		verbose mode\n"
-"  --wait	-w [seconds]	wait for the xtables lock\n"
+"  --wait	-w [seconds]	maximum wait to acquire xtables lock before give up\n"
+"  --wait-interval -W [usecs]	wait time to try to acquire xtables lock\n"
+"				default is 1 second\n"
 "  --line-numbers		print line numbers when listing\n"
 "  --exact	-x		expand numbers (display exact values)\n"
 "[!] --fragment	-f		match second or further fragments only\n"
@@ -1290,7 +1293,10 @@ int do_command4(int argc, char *argv[], char **table,
 	unsigned int nsaddrs = 0, ndaddrs = 0;
 	struct in_addr *saddrs = NULL, *smasks = NULL;
 	struct in_addr *daddrs = NULL, *dmasks = NULL;
-
+	struct timeval wait_interval = {
+		.tv_sec = 1,
+	};
+	bool wait_interval_set = false;
 	int verbose = 0;
 	int wait = 0;
 	const char *chain = NULL;
@@ -1327,7 +1333,7 @@ int do_command4(int argc, char *argv[], char **table,
 	opterr = 0;
 	opts = xt_params->orig_opts;
 	while ((cs.c = getopt_long(argc, argv,
-	   "-:A:C:D:R:I:L::S::M:F::Z::N:X::E:P:Vh::o:p:s:d:j:i:fbvw::nt:m:xc:g:46",
+	   "-:A:C:D:R:I:L::S::M:F::Z::N:X::E:P:Vh::o:p:s:d:j:i:fbvw::W::nt:m:xc:g:46",
 					   opts, NULL)) != -1) {
 		switch (cs.c) {
 			/*
@@ -1588,6 +1594,23 @@ int do_command4(int argc, char *argv[], char **table,
 						"wait seconds not numeric");
 			break;
 
+		case 'W':
+			if (restore) {
+				xtables_error(PARAMETER_PROBLEM,
+					      "You cannot use `-W' from "
+					      "iptables-restore");
+			}
+			if (optarg)
+				parse_wait_interval(optarg, &wait_interval);
+			else if (optind < argc &&
+				 argv[optind][0] != '-' &&
+				 argv[optind][0] != '!')
+				parse_wait_interval(argv[optind++],
+						    &wait_interval);
+
+			wait_interval_set = true;
+			break;
+
 		case 'm':
 			command_match(&cs);
 			break;
@@ -1688,6 +1711,10 @@ int do_command4(int argc, char *argv[], char **table,
 		cs.invert = FALSE;
 	}
 
+	if (!wait && wait_interval_set)
+		xtables_error(PARAMETER_PROBLEM,
+			      "--wait-interval only makes sense with --wait\n");
+
 	if (strcmp(*table, "nat") == 0 &&
 	    ((policy != NULL && strcmp(policy, "DROP") == 0) ||
 	    (cs.jumpto != NULL && strcmp(cs.jumpto, "DROP") == 0)))
@@ -1743,7 +1770,7 @@ int do_command4(int argc, char *argv[], char **table,
 			   chain, XT_EXTENSION_MAXNAMELEN);
 
 	/* Attempt to acquire the xtables lock */
-	if (!restore && !xtables_lock(wait)) {
+	if (!restore && !xtables_lock(wait, &wait_interval)) {
 		fprintf(stderr, "Another app is currently holding the xtables lock. ");
 		if (wait == 0)
 			fprintf(stderr, "Perhaps you want to use the -w option?\n");
diff --git a/iptables/xshared.c b/iptables/xshared.c
index b18022e..fcf0aa2 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -8,12 +8,15 @@
 #include <string.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <sys/time.h>
 #include <unistd.h>
 #include <xtables.h>
+#include <math.h>
 #include "xshared.h"
 
 #define XT_SOCKET_NAME "xtables"
 #define XT_SOCKET_LEN 8
+#define BASE_MICROSECONDS	100000
 
 /*
  * Print out any special helps. A user might like to be able to add a --help
@@ -243,11 +246,16 @@ void xs_init_match(struct xtables_match *match)
 		match->init(match->m);
 }
 
-bool xtables_lock(int wait)
+bool xtables_lock(int wait, struct timeval *wait_interval)
 {
 	int i = 0, ret, xt_socket;
 	struct sockaddr_un xt_addr;
-	int waited = 0;
+	struct timeval time_left, wait_time, waited_time;
+
+	time_left.tv_sec = wait;
+	time_left.tv_usec = 0;
+	waited_time.tv_sec = 0;
+	waited_time.tv_usec = 0;
 
 	memset(&xt_addr, 0, sizeof(xt_addr));
 	xt_addr.sun_family = AF_UNIX;
@@ -262,12 +270,43 @@ bool xtables_lock(int wait)
 			   offsetof(struct sockaddr_un, sun_path)+XT_SOCKET_LEN);
 		if (ret == 0)
 			return true;
-		else if (wait >= 0 && waited >= wait)
+		if (++i % 10 == 0) {
+			if (wait != -1)
+				fprintf(stderr, "Another app is currently holding the xtables lock; "
+					"still %lds %ldus time ahead to have a chance to grab the lock...\n",
+					time_left.tv_sec, time_left.tv_usec);
+			else
+				fprintf(stderr, "Another app is currently holding the xtables lock; "
+						"waiting for it to exit...\n");
+		}
+
+		wait_time = *wait_interval;
+		select(0, NULL, NULL, NULL, &wait_time);
+		if (wait == -1)
+			continue;
+
+		timeradd(&waited_time, wait_interval, &waited_time);
+		timersub(&time_left, wait_interval, &time_left);
+		if (!timerisset(&time_left))
 			return false;
-		if (++i % 2 == 0)
-			fprintf(stderr, "Another app is currently holding the xtables lock; "
-				"waiting (%ds) for it to exit...\n", waited);
-		waited++;
-		sleep(1);
 	}
 }
+
+void parse_wait_interval(const char *str, struct timeval *wait_interval)
+{
+	unsigned int usec;
+	int ret;
+
+	ret = sscanf(str, "%u", &usec);
+	if (ret == 1) {
+		if (usec > 999999)
+			xtables_error(PARAMETER_PROBLEM,
+				      "too long usec wait %u > 999999 usec",
+				      usec);
+
+		wait_interval->tv_sec = 0;
+		wait_interval->tv_usec = usec;
+		return;
+	}
+	xtables_error(PARAMETER_PROBLEM, "wait interval not numeric");
+}
diff --git a/iptables/xshared.h b/iptables/xshared.h
index 66dc46e..816e138 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -84,7 +84,9 @@ extern struct xtables_match *load_proto(struct iptables_command_state *);
 extern int subcmd_main(int, char **, const struct subcommand *);
 extern void xs_init_target(struct xtables_target *);
 extern void xs_init_match(struct xtables_match *);
-extern bool xtables_lock(int wait);
+bool xtables_lock(int wait, struct timeval *wait_interval);
+
+void parse_wait_interval(const char *str, struct timeval *wait_interval);
 
 extern const struct xtables_afinfo *afinfo;