yahoo-eng-team team mailing list archive
-
yahoo-eng-team team
-
Mailing list archive
-
Message #57702
[Bug 1618430] Re: IptablesFwaasDriver could hang neutron l3-agent
Reviewed: https://review.openstack.org/384943
Committed: https://git.openstack.org/cgit/openstack/neutron-fwaas/commit/?id=b80de376d484e5cd1eb09df4c0720e52f3f29742
Submitter: Jenkins
Branch: master
commit b80de376d484e5cd1eb09df4c0720e52f3f29742
Author: Yann Morice <yann.morice@xxxxxxxxx>
Date: Tue Oct 11 13:34:00 2016 +0200
Deal with the '-m protocol' flag in iptables FwAAS v1 and v2
Iptables automatically add a '-m protocol' flag for rules containing a source
or a destination port. FwAAS do not add this flag so that, on apply, rules are
always different from iptables-save output. This induce a very long loop in
neutron-l3-agent hosting the router as the comparison of each line is just
slightly different.
This patch only add one '-m protocol' flag before port.
Closes-Bug: #1618430
Change-Id: Ia3fa3889dbf3ee10425e7e7fce8a3b8351f14e60
** Changed in: neutron
Status: In Progress => Fix Released
--
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to neutron.
https://bugs.launchpad.net/bugs/1618430
Title:
IptablesFwaasDriver could hang neutron l3-agent
Status in neutron:
Fix Released
Bug description:
Hello!
OVSHybridIptablesFirewallDriver completely hang neutron l3-agent
during a very long time when using a large number of firewall rules
(400 in our case). After debugging, it appears that we are locked in
neutron/agent/linux/iptables_manager.py in function
_generate_chain_diff_iptables_commands and exactly at line "for line
in difflib.ndiff(old_chain_rules, new_chain_rules):".
Ex.
iptables_manager.py(784): for line in difflib.ndiff(old_chain_rules, new_chain_rules):
--- modulename: difflib, funcname: compare
difflib.py(922): for line in g:
--- modulename: difflib, funcname: _dump
difflib.py(927): for i in xrange(lo, hi):
difflib.py(910): for tag, alo, ahi, blo, bhi in cruncher.get_opcodes():
difflib.py(911): if tag == 'replace':
difflib.py(912): g = self._fancy_replace(a, alo, ahi, b, blo, bhi)
difflib.py(922): for line in g:
--- modulename: difflib, funcname: _fancy_replace
difflib.py(966): best_ratio, cutoff = 0.74, 0.75
difflib.py(967): cruncher = SequenceMatcher(self.charjunk)
--- modulename: difflib, funcname: __init__
difflib.py(218): self.isjunk = isjunk
difflib.py(219): self.a = self.b = None
difflib.py(220): self.autojunk = autojunk
difflib.py(221): self.set_seqs(a, b)
--- modulename: difflib, funcname: set_seqs
difflib.py(232): self.set_seq1(a)
--- modulename: difflib, funcname: set_seq1
difflib.py(256): if a is self.a:
difflib.py(258): self.a = a
difflib.py(259): self.matching_blocks = self.opcodes = None
difflib.py(233): self.set_seq2(b)
--- modulename: difflib, funcname: set_seq2
difflib.py(282): if b is self.b:
difflib.py(284): self.b = b
difflib.py(285): self.matching_blocks = self.opcodes = None
difflib.py(286): self.fullbcount = None
difflib.py(287): self.__chain_b()
--- modulename: difflib, funcname: __chain_b
difflib.py(317): b = self.b
difflib.py(318): self.b2j = b2j = {}
difflib.py(320): for i, elt in enumerate(b):
difflib.py(325): junk = set()
difflib.py(326): isjunk = self.isjunk
difflib.py(327): if isjunk:
difflib.py(328): for elt in list(b2j.keys()): # using list() since b2j is modified
difflib.py(334): popular = set()
difflib.py(335): n = len(b)
difflib.py(336): if self.autojunk and n >= 200:
difflib.py(347): self.isbjunk = junk.__contains__
difflib.py(348): self.isbpopular = popular.__contains__
difflib.py(968): eqi, eqj = None, None # 1st indices of equal lines (if any)
difflib.py(973): for j in xrange(blo, bhi):
difflib.py(974): bj = b[j]
difflib.py(975): cruncher.set_seq2(bj)
--- modulename: difflib, funcname: set_seq2
difflib.py(282): if b is self.b:
difflib.py(284): self.b = b
[...]
difflib.py(418): for j in b2j.get(a[i], nothing):
difflib.py(420): if j < blo:
difflib.py(422): if j >= bhi:
difflib.py(424): k = newj2len[j] = j2lenget(j-1, 0) + 1
difflib.py(425): if k > bestsize:
difflib.py(418): for j in b2j.get(a[i], nothing):
difflib.py(420): if j < blo:
difflib.py(422): if j >= bhi:
difflib.py(424): k = newj2len[j] = j2lenget(j-1, 0) + 1
difflib.py(425): if k > bestsize:
difflib.py(418): for j in b2j.get(a[i], nothing):
difflib.py(420): if j < blo:
difflib.py(422): if j >= bhi:
difflib.py(424): k = newj2len[j] = j2lenget(j-1, 0) + 1
difflib.py(425): if k > bestsize:
difflib.py(418): for j in b2j.get(a[i], nothing):
difflib.py(420): if j < blo:
...
In facts, it seems that performance of difflib.ndiff decrease rapidly
with length of arrays. But, if I am right, finally we just add a -D
rule_number ... when a rule has to be deleted and -I rule_number ...
when a rule has to be added. So, we have to do both (-D / -I) when a
new rule is different, nothing if rule is the same, only -D when we
have less rules than before and -I when we have more rules than
before.
Couldn't we change the algorithm with something like that (which is
more than 100x faster) or perhaps something else better ?
def _generate_chain_diff_iptables_commands(chain, old_chain_rules,
new_chain_rules):
statements = []
offset = -1
for i in range(0, max(len(old_chain_rules), len(new_chain_rules))):
if i >= len(old_chain_rules):
rule = new_chain_rules[i][5:].split(' ', 1)[-1]
# rule inserted at this position
statements.append('-I %s %d %s' % (chain, i - offset, rule))
elif i >= len(new_chain_rules):
statements.append('-D %s %d' % (chain, i - offset))
offset += 1
elif old_chain_rules[i] == new_chain_rules[i]:
continue
else:
statements.append('-D %s %d' % (chain, i - offset))
rule = new_chain_rules[i][5:].split(' ', 1)[-1]
# rule inserted at this position
statements.append('-I %s %d %s' % (chain, i - offset, rule))
return statements
To manage notifications about this bug go to:
https://bugs.launchpad.net/neutron/+bug/1618430/+subscriptions
References
