From 07e2107ef0cbc1b81864c3c0f0ef297a9dfff44d Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Mon, 14 Feb 2022 10:35:56 +0100 Subject: xshared: Implement xtables lock timeout using signals Previously, if a lock timeout is specified using `-wN `, flock() is called using LOCK_NB in a loop with a sleep. This results in two issues. The first issue is that the process may wait longer than necessary when the lock becomes available. For this the `-W` option was added, but this requires fine-tuning. The second issue is that if lock contention is high, invocations using `-w` (without a timeout) will always win lock acquisition from invocations that use `-w N`. This is because invocations using `-w` are actively waiting on the lock whereas those using `-w N` only check from time to time whether the lock is free, which will never be the case. This patch removes the sleep loop and deprecates the `-W` option (making it non-functional). Instead, flock() is always called in a blocking fashion, but the alarm() function is used with a non-SA_RESTART signal handler to cancel the system call. Signed-off-by: Jethro Beekman Signed-off-by: Florian Westphal --- iptables/xshared.c | 63 ++++++++++++++++++++++-------------------------------- 1 file changed, 26 insertions(+), 37 deletions(-) (limited to 'iptables/xshared.c') diff --git a/iptables/xshared.c b/iptables/xshared.c index 1fd7acc9..50a1d48a 100644 --- a/iptables/xshared.c +++ b/iptables/xshared.c @@ -13,11 +13,11 @@ #include #include #include -#include #include #include #include #include +#include #include "xshared.h" /* @@ -243,14 +243,14 @@ void xs_init_match(struct xtables_match *match) match->init(match->m); } -static int xtables_lock(int wait, struct timeval *wait_interval) +static void alarm_ignore(int i) { +} + +static int xtables_lock(int wait) { - struct timeval time_left, wait_time; + struct sigaction sigact_alarm; const char *lock_file; - int fd, i = 0; - - time_left.tv_sec = wait; - time_left.tv_usec = 0; + int fd; lock_file = getenv("XTABLES_LOCKFILE"); if (lock_file == NULL || lock_file[0] == '\0') @@ -263,31 +263,24 @@ static int xtables_lock(int wait, struct timeval *wait_interval) return XT_LOCK_FAILED; } - if (wait == -1) { - if (flock(fd, LOCK_EX) == 0) - return fd; - - fprintf(stderr, "Can't lock %s: %s\n", lock_file, - strerror(errno)); - return XT_LOCK_BUSY; + if (wait != -1) { + sigact_alarm.sa_handler = alarm_ignore; + sigact_alarm.sa_flags = SA_RESETHAND; + sigemptyset(&sigact_alarm.sa_mask); + sigaction(SIGALRM, &sigact_alarm, NULL); + alarm(wait); } - while (1) { - if (flock(fd, LOCK_EX | LOCK_NB) == 0) - return fd; - else if (timercmp(&time_left, wait_interval, <)) - return XT_LOCK_BUSY; + if (flock(fd, LOCK_EX) == 0) + return fd; - if (++i % 10 == 0) { - 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); - } - - wait_time = *wait_interval; - select(0, NULL, NULL, NULL, &wait_time); - timersub(&time_left, wait_interval, &time_left); + if (errno == EINTR) { + errno = EWOULDBLOCK; } + + fprintf(stderr, "Can't lock %s: %s\n", lock_file, + strerror(errno)); + return XT_LOCK_BUSY; } void xtables_unlock(int lock) @@ -296,9 +289,9 @@ void xtables_unlock(int lock) close(lock); } -int xtables_lock_or_exit(int wait, struct timeval *wait_interval) +int xtables_lock_or_exit(int wait) { - int lock = xtables_lock(wait, wait_interval); + int lock = xtables_lock(wait); if (lock == XT_LOCK_FAILED) { xtables_free_opts(1); @@ -334,7 +327,7 @@ int parse_wait_time(int argc, char *argv[]) return wait; } -void parse_wait_interval(int argc, char *argv[], struct timeval *wait_interval) +void parse_wait_interval(int argc, char *argv[]) { const char *arg; unsigned int usec; @@ -354,8 +347,7 @@ void parse_wait_interval(int argc, char *argv[], struct timeval *wait_interval) "too long usec wait %u > 999999 usec", usec); - wait_interval->tv_sec = 0; - wait_interval->tv_usec = usec; + fprintf(stderr, "Ignoring deprecated --wait-interval option.\n"); return; } xtables_error(PARAMETER_PROBLEM, "wait interval not numeric"); @@ -1235,9 +1227,6 @@ xtables_printhelp(const struct xtables_rule_match *matches) " --table -t table table to manipulate (default: `filter')\n" " --verbose -v verbose mode\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"); @@ -1665,7 +1654,7 @@ void do_parse(int argc, char *argv[], "iptables-restore"); } - parse_wait_interval(argc, argv, &args->wait_interval); + parse_wait_interval(argc, argv); wait_interval_set = true; break; -- cgit v1.2.3