This commit adds common_agent_extension class which is agent API
for L2 extension drivers used e.g. by Linuxbridge agent.
This is necessary to be able to use instance of iptables_manager
used in firewall driver also in L2 extension drivers (like qos).
This patch refactors little bit iptables_manager code to make possible
to initialize e.g. mangle or nat table on demand, even if iptables
is created as "state_less"
Change-Id: I3b66e49b7f176124e8aea3eb96d0d465f1ab1ea0
Closes-Bug: #1736674
(cherry picked from commit cbee0f9f88)
The default wait-interval for iptables-restore when
using -w is 1 second between tries. On a busy system
that could mean we timeout before we get the lock. Try
5 times per second instead by using -W 200000.
Change-Id: I8307db20187516be781e37c191d8f09a9a8e3dc3
Related-bug: #1712185
(cherry picked from commit 46081445d6)
In the case where we called iptables-restore with a
-w argument and it succeeded, we should short-circuit
future calls to always use -w, instead of trying
without it, just to fall-back to using it on failure.
While analyzing some l3-agent log files I have seen
lots of "Perhaps you want to use the -w option?",
followed by a call with -w, followed by not using it
the next time. Changing this can save one failing
call to iptables-restore.
Change-Id: Icac99eb1d43648c64b6beaee0d6201f990eacb51
Related-bug: #1712185
(cherry picked from commit 6c50ad5858)
In this case, it's an expected error, and we retry again with -w.
Related-Bug: #1712185
Change-Id: I97bf3032b5cebcbce51a3b3de6cb128ca342bd87
(cherry picked from commit 2f0ffa998a)
Upstream iptables added support for -w ('wait') argument to
iptables-restore. It makes the command grab a 'xlock' that guarantees
that no two iptables calls will mess a table if called in parallel.
[This somewhat resembles what we try to achieve with a file lock we
grab in iptables manager's _apply_synchronized.]
If two processes call to iptables-restore or iptables in parallel, the
second call risks failing, returning error code = 4, and also printing
the following error:
Another app is currently holding the xtables lock. Perhaps you want
to use the -w option?
If we call to iptables / iptables-restore with -w though, it will wait
for the xlock release before proceeding, and won't fail.
Though the feature was added in iptables/master only and is not part of
an official iptables release, it was already backported to RHEL 7.x
iptables package, and so we need to adopt to it. At the same time, we
can't expect any underlying platform to support the argument.
A solution here is to call iptables-restore with -w when a regular call
failed. Also, the patch adds -w to all iptables calls, in the iptables
manager as well as in ipset-cleanup.
Since we don't want to lock agent in case current xlock owner doesn't
release it in reasonable time, we limit the time we wait to ~1/3 of
report_interval, to give the agent some time to recover without
triggering expensive fullsync.
In the future, we may be able to get rid of our custom synchronization
lock that we use in iptables manager. But this will require all
supported platforms to get the feature in and will take some time.
Closes-Bug: #1712185
Change-Id: I94e54935df7c6caa2480eca19e851cb4882c0f8b
(cherry picked from commit a521bf0393)
If the namespace does not exist the current behavior
is to try to apply the iptables rules forever in a
endless loop. This fills up the logs on the network
node and leads to outage.
Change-Id: I628b18a66f9478d7349fa1817431aae2f62ee105
Related-bug: #1623664
Related-bug: #1573073
When update meter label or rule, iptables_manager will update iptables
rule in router's namespace. In order to, it will clean traffic counter
number collected in interval time, the other iptables always trashing
that will clean old iptalbes rule and generate new same significance
iptables rule.
Change-Id: Ide2b26c98587258175234acded38ce481b7e7f76
Closes-Bug: #1618879
iptables_manager will be used by many features including security
groups, FWaaS, metering. The address scope specific code should be
moved out of iptables_manager, so that other feature will not get
the iptables rules that they will not use. For example, dhcp namespace
will not have the address scope iptables rules.
The change to the test code to adapt the change at [1], has also been
reverted in this patch. Instead, a couple of new test cases are added.
[1] https://review.openstack.org/#/c/270001/
Change-Id: Ifc8e7a381f8ab005a9e0216532cc7d0e7378c025
Closes-Bug: #1549513
Python 3 deprecated the logger.warn method, see:
https://docs.python.org/3/library/logging.html#logging.warning
so we prefer to use warning to avoid DeprecationWarning.
Closes-Bugs: #1529913
Change-Id: Icc01ce5fbd10880440cf75a2e0833394783464a0
Co-Authored-By: Gary Kotton <gkotton@vmware.com>
For networks in the same address scope, network traffic routes
directly. This happens not only between internal networks, but also
between internal network and external network. No SNAT is applied
when routing traffic to the external network because addresses on the
internal network are assumed to be viable on the external network.
For networks in different scopes, network traffic can't route
directly. Between internal networks in different scopes, traffic is
blocked. DNAT for floating IPs will still work. Also, shared SNAT to
the external network will still work as it does today.
Change-Id: I439633ebef432b1a2eecee09b647207d5a271bf6
Co-Authored-By: Hong Hui Xiao <xiaohhui@cn.ibm.com>
Implements: blueprint address-scopes
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
This fixes the order of arguments in iptables rules that
are bare jumps (e.g. '-j other-chain').
The previous code was only catching jump rules that appeared
after a chain definition.
Closes-Bug: #1502932
Change-Id: I490792eb08c67a32f9b286d933a776fb76840b6b
Since a packet can only have one mark, and we will need to mark a
packet for multiple purposes, we need to use a coordinated bitmask for
the two cases of simple marking that we currently do in Neutron
leaving the other bits for address scopes.
DocImpact
Change-Id: Id0517758d06e036a36dc8b8772e41af55d986b4e
Partially-Implements: blueprint address-scopes
The code to find the matching entry was scanning through a
list of all rules for every rule. This became extremely slow
as the number of rules became large, leading to long delays
waiting for firewall rules to be applied.
This patch switches to the use of a dictionary so the cost
becomes a hash lookup instead of a list scan.
Closes-Bug: #1453264
Closes-Bug: #1455675
Change-Id: I1e6fe5e50b9c13066c966c252cadc8ed1d08f686
The way we were forming our iptables rules was not matching
the output of iptables-save. This caused the logic that preserves
counters to miss many of the rules.
This patch corrects the order for the comments and the allowed address
pairs to match the output order of iptables-save.
Closes-Bug: #1456823
Change-Id: I34c2249d0865485578767865c82414e1d813d563
ip_conntrack causes security group rule failures when packets share
the same 5-tuple. Use iptables zone option to separate different
conntrack zone. Currently this patch only works for OVS agent.
Co-authored-by: shihanzhang <shihanzhang@huawei.com>
Change-Id: I90b4d2485e3e491f496dfb7bdee03d57f393be35
Partial-Bug: #1359523
This change ensures that the structure of the unit test tree matches
that of the code tree to make it obvious where to find tests for a
given module. A check is added to the pep8 job to protect against
regressions.
The plugin test paths are relocated to neutron/tests/unit/plugins
but are otherwise ignored for now.
Change-Id: If307593259139171be21a71c58e3a34bf148cc7f
Partial-Bug: #1440834
Sławek Kapłoński