From 19dbbe6f003f3659e561068b6edbb252d435dba7 Mon Sep 17 00:00:00 2001
From: Kevin Benton <kevin@benton.pub>
Date: Fri, 1 Apr 2016 02:46:01 -0700
Subject: [PATCH] Add debug option to verify iptables rules

The iptables apply logic performs diffs between the rules in the
system and the desired rule state. If the format generated in
Neutron does not match the format of iptables-save, Neutron will
generate unnecessary deletions and adds for rules that it thinks
are different when they are actually the same.

A simple way to detect mismatches is to run the _apply function
twice each time and generate an error if the second one results
in changes since that implies that the rules the first one applied
did not match the format of iptables-save. However, this comes at
a performance cost and doesn't offer benefits to operators.

This patch adds a config option to debug_iptables_rules that
is set to False by default. When set to True it will perform
the secondary check described above.

This enables it for all of the jobs that call the gate_hook.sh
script.

This is only meant to assist catching development errors of
iptables, it SHOULD NOT be turned on by operators.

Change-Id: I6bee1d51155488e91857ee8bc45470d6a224fa37
---
 neutron/agent/common/config.py              |  6 ++++++
 neutron/agent/linux/iptables_manager.py     | 14 +++++++++++++-
 neutron/tests/contrib/gate_hook.sh          |  4 ++++
 neutron/tests/contrib/hooks/iptables_verify |  4 ++++
 4 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 neutron/tests/contrib/hooks/iptables_verify

diff --git a/neutron/agent/common/config.py b/neutron/agent/common/config.py
index b6913d58d61..5a769f19541 100644
--- a/neutron/agent/common/config.py
+++ b/neutron/agent/common/config.py
@@ -64,6 +64,12 @@ IPTABLES_OPTS = [
                        "generated iptables rules that describe each rule's "
                        "purpose. System must support the iptables comments "
                        "module for addition of comments.")),
+    cfg.BoolOpt('debug_iptables_rules', default=False,
+                help=_("Duplicate every iptables difference calculation to "
+                       "ensure the format being generated matches the format "
+                       "of iptables-save. This option should not be turned "
+                       "on for production systems because it imposes a "
+                       "performance penalty.")),
 ]
 
 PROCESS_MONITOR_OPTS = [
diff --git a/neutron/agent/linux/iptables_manager.py b/neutron/agent/linux/iptables_manager.py
index 853160da9f1..1debe43d4c4 100644
--- a/neutron/agent/linux/iptables_manager.py
+++ b/neutron/agent/linux/iptables_manager.py
@@ -412,6 +412,9 @@ class IptablesManager(object):
         finally:
             try:
                 self.defer_apply_off()
+            except n_exc.IpTablesApplyException:
+                # already in the format we want, just reraise
+                raise
             except Exception:
                 msg = _('Failure applying iptables rules')
                 LOG.exception(msg)
@@ -436,7 +439,16 @@ class IptablesManager(object):
             lock_name += '-' + self.namespace
 
         with lockutils.lock(lock_name, utils.SYNCHRONIZED_PREFIX, True):
-            return self._apply_synchronized()
+            first = self._apply_synchronized()
+            if not cfg.CONF.AGENT.debug_iptables_rules:
+                return first
+            second = self._apply_synchronized()
+            if second:
+                msg = (_("IPTables Rules did not converge. Diff: %s") %
+                       '\n'.join(second))
+                LOG.error(msg)
+                raise n_exc.IpTablesApplyException(msg)
+            return first
 
     def get_rules_for_table(self, table):
         """Runs iptables-save on a table and returns the results."""
diff --git a/neutron/tests/contrib/gate_hook.sh b/neutron/tests/contrib/gate_hook.sh
index abcfcf75bc2..ccb04dd0fd9 100644
--- a/neutron/tests/contrib/gate_hook.sh
+++ b/neutron/tests/contrib/gate_hook.sh
@@ -54,6 +54,7 @@ case $VENV in
         start_new_ovs
     fi
 
+    load_conf_hook iptables_verify
     # Make the workspace owned by the stack user
     sudo chown -R $STACK_USER:$STACK_USER $BASE
     ;;
@@ -68,6 +69,9 @@ case $VENV in
     load_rc_hook qos
     load_rc_hook trunk
     load_conf_hook osprofiler
+    if [[ "$VENV" =~ "dsvm-scenario" ]]; then
+        load_conf_hook iptables_verify
+    fi
     if [[ "$VENV" =~ "pecan" ]]; then
         load_conf_hook pecan
     fi
diff --git a/neutron/tests/contrib/hooks/iptables_verify b/neutron/tests/contrib/hooks/iptables_verify
new file mode 100644
index 00000000000..72cbd1aeb57
--- /dev/null
+++ b/neutron/tests/contrib/hooks/iptables_verify
@@ -0,0 +1,4 @@
+[[post-config|/etc/neutron/neutron.conf]]
+
+[AGENT]
+debug_iptables_rules=True