]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commit
Use diffs for iptables restore instead of all rules
authorKevin Benton <blak111@gmail.com>
Sat, 3 Oct 2015 14:25:19 +0000 (07:25 -0700)
committerKevin Benton <kevinbenton@buttewifi.com>
Mon, 23 Nov 2015 06:09:28 +0000 (06:09 +0000)
commitf066e46bb77362ccca838b541eb42c7ae8ddc125
tree7a386a3118ee75244e0b2fa934acaaf20f6934a5
parent292bdff78b91320ea40739e1ceb01c3cb1a31cc8
Use diffs for iptables restore instead of all rules

This patch changes our iptables logic to generate a delta of
iptables commands (inserts + deletes) to get from the current
iptables state to the new state. This will significantly reduce
the amount of data that we have to shell out to iptables-restore
on every call (and reduce the amount of data iptables-restore has
to parse).

We no longer have to worry about preserving counters since
we are adding and deleting specific rules, so the rule modification
code got a nice cleanup to get rid of the old rule matching.

This also gives us a new method of functionally testing that we are
generating rules in the correct manner. After applying new rules
once, a subsequent call should always have no work to do. The new
functional tests added leverage that property heavily and should
protect us from regressions in how rules are formed.

Performance metrics relative to HEAD~1:
+====================================+============+=======+
|               Scenario             | This patch | HEAD~1|
|------------------------------------|------------|-------|
| 200 VMs*22 rules existing - startup|            |       |
|                       _modify_rules|   0.67s    | 1.05s |
|                 _apply_synchronized|   1.87s    | 2.89s |
|------------------------------------|------------|-------|
| 200 VMs*22 rules existing - add VM |            |       |
|                       _modify_rules|   0.68s    | 1.05s |
|                 _apply_synchronized|   2.07s    | 2.92s |
|------------------------------------+------------+-------+
|200 VMs*422 rules existing - startup|            |       |
|                       _modify_rules|   5.43s    | 8.17s |
|                 _apply_synchronized|  12.77s    |28.00s |
|------------------------------------|------------|-------|
|200 VMs*422 rules existing - add VM |            |       |
|                       _modify_rules|   6.41s    | 8.33s |
|                 _apply_synchronized|  33.09s    |33.80s |
+------------------------------------+------------+-------+

The _apply_synchronized times seem to converge when dealing
with ~85k rules. In the profile I can see that both approaches
seem to wait on iptables-restore for approximately the same
amount of time so it could be hitting the performance limits
of iptables-restore.

DocImpact
Partial-Bug: #1502297
Change-Id: Ia6470c85b6b71979006ffe5da9095fdcce3122c1
neutron/agent/linux/iptables_firewall.py
neutron/agent/linux/iptables_manager.py
neutron/tests/functional/agent/linux/test_iptables_firewall.py
neutron/tests/unit/agent/linux/test_iptables_manager.py
neutron/tests/unit/agent/test_securitygroups_rpc.py