xshared: using the blocking file lock request when we wait indefinitely

When using "-w" to avoid concurrent instances, we try to do flock() every
one second until it success. But one second maybe too long in some
situations, and it's hard to select a suitable interval time. So when
using "iptables -w" to wait indefinitely, it's better to block until
it become success.

Now do some performance tests. First, flush all the iptables rules in
filter table, and run "iptables -w -S" endlessly:
  # iptables -F
  # iptables -X
  # while : ; do
  iptables -w -S >&- &
  done

Second, after adding and deleting the iptables rules 100 times, measure
the time cost:
  # time for i in $(seq 100); do
  iptables -w -A INPUT
  iptables -w -D INPUT
  done

Before this patch:
  real  1m15.962s
  user  0m0.224s
  sys   0m1.475s

Apply this patch:
  real  0m1.830s
  user  0m0.168s
  sys   0m1.130s

Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/iptables/xshared.c b/iptables/xshared.c
index 055acf202f1d5ecf09afbc2155b6e4807d213f9c..f0a5ddd0dea52b9fa1b733dfa9d043ea75b2a347 100644 (file)
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -1,4 +1,5 @@
 #include <getopt.h>
+#include <errno.h>
 #include <libgen.h>
 #include <netdb.h>
 #include <stdbool.h>
@@ -258,27 +259,29 @@ bool xtables_lock(int wait, struct timeval *wait_interval)
        if (fd < 0)
                return true;
 
+       if (wait == -1) {
+               if (flock(fd, LOCK_EX) == 0)
+                       return true;
+
+               fprintf(stderr, "Can't lock %s: %s\n", XT_LOCK_NAME,
+                       strerror(errno));
+               return false;
+       }
+
        while (1) {
                if (flock(fd, LOCK_EX | LOCK_NB) == 0)
                        return true;
-               else if (wait >= 0 && timercmp(&time_left, wait_interval, <))
+               else if (timercmp(&time_left, wait_interval, <))
                        return false;
 
                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");
+                       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);
-               if (wait == -1)
-                       continue;
-
                timersub(&time_left, wait_interval, &time_left);
        }
 }