After updating pylint, it started emitting additional "R"
warnings in some cases, fix some of them.
use-a-generator,
unnecessary-lambda-assignment,
consider-using-max-builtin,
consider-using-generator,
consider-using-in,
use-list-literal,
consider-using-from-import
Trivialfix
Change-Id: Ife6565cefcc30b4e8a0df9121c9454cf744225df
Fixed bug when config option use_random_fully is
set to False all routers accept one configured
by l3 agent with iptables "--random-fully" option.
Also added storing of use iptables --random-fully
config option to "_random_fully" class variable
of IptablesManager to reduce checks of iptables
version by instances of this class.
Closes-Bug: #2018599
Change-Id: Ia12fc0a3d4812a0aba816b49dec60a7dcfaf0623
When deployed with k8s, k8s service types like NodePort
or ExternalIP will affect vm traffic on nat table's
PREROUTING chain. This PS try to mitigate the effect
by allowing vm traffic to bypass those rules.
Change-Id: Iae12d9c2f37bc0fca9c3d5e85e46c642263e4a77
Closes-Bug: #1908957
In some specific use case, the cloud operator expects the source port
of a packet to stay the same across all masquerading layer up to the
destination host. With the implementation of the random-fully code,
this behavior was changed as source_port is always rewritten no matter
which type of architecture / network CIDRs is being used in the backend.
This setting allows a user to fallback to the original behavior of the
masquerading process which is to keep the source_port consistent across
all layers. The initial random-fully fix prevents packet drops when
duplicate tuples are generated from two different namespace when the
source_ip:source_port goes toward the same destination so enabling this
setting would allow this issue to show again. Perhaps a right approach
here would be to fix this "racey" situation in the kernel by perhaps
using the mac address as a seed to the tuple ...
Change-Id: Idfe5e51007b9a3eaa48779cd01edbca2f586eee5
Closes-bug: #1987396
MAC addresses in the iptables rules are always added by iptables_manager
with uppercase. It was done like that in [1].
For some reason, iptables on Centos 9 Stream (1.8.7) returns MAC
addresses all in lowercase and difflib library treats such new and old
rules as different ones so iptables manager always tries to remove and
add antispoofing rules which have MAC addresses set.
[1] https://review.opendev.org/c/openstack/neutron/+/184355
Related-Bug: #1976323
Change-Id: I63e78fdd030f063a1b943d481a9cbd98850552d7
Replace rootwrap execution with privsep context execution.
This series of patches will progressively replace any
rootwrap call.
This patch migrates any "iptables" and "ipset" command related
to privsep.
Change-Id: I4a1e137b2b414067504ad7c799d68f482bf3d36c
Story: #2007686
Task: #41558
Ensure the TCP traffic leaving the OVN metadata namespace has
the checksum correctly populated. This is necessary when the
OVS datapath is "netdev".
Because the overhead added is minimal and only applies to the
metadata traffic inside the metadata namespace, this rule is
always set.
Change-Id: I7e39f40b325a6974a46ed34641cec5226c9e5a3f
Closes-Bug: #1904871
When routers are migrated from one Neutron agent to the other, the metering IPtables
rules are removed, which can cause some exceptions that can be ignored. The metering
agent already handled this situation. However, it logs the message as an ERROR, which
can triggers alarms. Therefore, we propose here to change the LOG message from error
to warning.
Closes-Bug: #1904874
Change-Id: I1805a07cef7fc7d7b041e582a4d79fb1a805df71
We push a v6 host route to make the guest send its metadata requests
in the direction of our router. We redirect it to haproxy which
mangles the headers and sends the request along to metadata-agent.
Apparently the supported list of dhcp options for dhcpv6 is quite
short in dnsmasq (cf. dnsmasq --help dhcp6) - not including anything
like classless-static-route for dhcpv4. So we must rely solely on
radvd to push host routes to the guest.
Metadata access over IPv6 is supposed to work both on dual-stack and
v6-only networks.
The following v6 subnet modes are supposed to work:
--ipv6-ra-mode slaac --ipv6-address-mode slaac
--ipv6-ra-mode dhcpv6-stateless --ipv6-address-mode dhcpv6-stateless
--ipv6-ra-mode dhcpv6-stateful --ipv6-address-mode dhcpv6-stateful
Change-Id: I28f2914b1b67659af2db7240eae730ac43daccd2
Partial-Bug: #1460177
As spotted in Focal testing patch [0], pep8 test fails with many
C0321 false-positives, reported in pylint as current version does not
support python 3.8 [1]
Use a newer version of pylint and astroid, fixing or disabling some of
the new checks: no-else-*, unnecessary-comprehension, import-outside-toplevel
[0] https://review.opendev.org/#/c/738163/
[1] https://github.com/PyCQA/pylint/issues/2737
Change-Id: Ie646b7093aa8634fd950c136a0eba9adcf56591c
If the configuration flag "debug_iptables_rules" is enabled, the
IPTables rules applied will be logged.
Similar to [1], when the IPTables firewall is enabled, it checks the
status of the following sysctl knobs:
* net.bridge.bridge-nf-call-arptables
* net.bridge.bridge-nf-call-ip6tables
* net.bridge.bridge-nf-call-iptables
In this case, the firewall is not enabling them but just checking the
status and logging it, to make easier the debugging process.
[1] https://review.opendev.org/#/c/371523/
Change-Id: I2ec953228d1d45e1d4c493c0b261901e6dbec0f7
Related-Bug: #1843259
The dhcp-agent is initializing the iptables 'nat' table even
though it is never inserting any rules there besides the
ones being done at init time. Since this table is really
intended for the l3-agent, add an argument so we can control
the initialization.
Change-Id: Iebda49e7da99bd3bc8c985132516ae5edafdfe20
We have a problem with SNAT with too many connections using the
same source and destination on the network nodes.
In addition we can see in the conntrack table that the who
"instert_failed" increases.
This might be a generic problem with conntrack and linux.
We suspect that we encounter the following "limitation / bug"
in the kernel.
There seems to be a workaround to alleviate this behavior by
setting the -random-fully flag in iptables for port consumption.
This patch fixes the problem by adding the --random-fully to
the SNAT rules.
Change-Id: I246c1f56df889bad9c7e140b56c3614124d80a19
Closes-Bug: #1814002
All of the externally consumed variables from neutron.common.constants
now live in neutron-lib. This patch removes neutron.common.constants
and switches all uses over to lib.
NeutronLibImpact
Depends-On: https://review.openstack.org/#/c/647836/
Change-Id: I3c2f28ecd18996a1cee1ae3af399166defe9da87
Reduces E128 warnings by ~260 to just ~900,
no way we're getting rid of all of them at once (or ever).
Files under neutron/tests still have a ton of E128 warnings.
Change-Id: I9137150ccf129bf443e33428267cd4bc9c323b54
Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>
Today the neutron common exceptions already live in neutron-lib and are
shimmed from neutron. This patch removes the neutron.common.exceptions
module and changes neutron's imports over to use their respective
neutron-lib exception module instead.
NeutronLibImpact
Change-Id: I9704f20eb21da85d2cf024d83338b3d94593671e
Two constants: MAX_CHAIN_LEN_{WRAP,NOWRAP} are now moved
to common.constants file and renamed to
MAX_IPTABLES_CHAIN_LEN_{WRAP,NOWRAP}.
It is done because at least one of them might be now used
outside iptables_manager module (in port_forwarding
extensions)
Change-Id: I9b7a24b0264631e74b3c05b73a22a6a9c2752e82
Currently, the neutron-openvswitch-agent does not start on Windows
due to Linux specific imports. This patch addresses this issue.
Also, we're wrapping the object returned by subprocess.Popen using
tpool.Proxy in order to prevent IO operations on the stream
handles from blocking other threads. Currently, the ovs db monitor
blocks the whole process.
Closes-Bug: #1775382
Co-Authored-By: Lucian Petrut <lpetrut@cloudbasesolutions.com>
Change-Id: I8bbc9d1f8332e5644a6071f599a7c6a66bef7928
This incorporates flake8 2.6.x and pycodestyle will be used
instead of older pep8. This ensures future python3 compatibility
and a bit better code styling.
Change-Id: Ia7c7c5a44727f615a151e1e68dd94c7ed42f974f
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
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
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
Change network namespace add/delete/list code to use
pyroute2 library instead of calling /sbin/ip.
Also changed all in-tree callers to use the new calls.
Closes-bug: #1717582
Related-bug: #1492714
Change-Id: Id802e77543177fbb95ff15c2c7361172e8824633
neutron-lib contains the synchronized lockutils decorator as well as
the SYNCHRONIZED_PREFIX global. This patch consumes them from
neutron-lib and removes them from neutron.
NeutronLibImpact
Change-Id: I729da348e340509f2d09f8a6436716e2398f1583
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
Since Pike log messages should not be translated.
This patch removes calls to i18n _LC, _LI, _LE, _LW from
logging logic throughout the code. Translators definition
from neutron._i18n is removed as well.
This patch also removes log translation verification from
ignore directive in tox.ini.
Change-Id: If9aa76fcf121c0e61a7c08088006c5873faee56e
according to https://wiki.openstack.org/wiki/Python3, now we should avoid
using six.iteritems and replace it with dict.items.
Change-Id: I58a399baa2275f280acc0e6d649f81838648ce5c
Closes-Bug: #1680761
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
Refactoring Neutron configuration options for agent common config to be
in neutron/conf/agent/common. This will allow centralization of all
configuration options and provide an easy way to import.
Partial-Bug: #1563069
Change-Id: Iebac0cdd3bcfd0135349128921b7ad7a1a939ab8
Needed-By: Ib676003bbe909b5a9013a3178b12dbe291d936af
The _modify_rules method currently uses nested loop to removes rules
that belong to us but don't have the wrap name. Speed up this operation
by storing our rules as set.
Reduce operation complexity from O(n*m) to O(n).
Change-Id: I82e6184a30ddb25f2258e21fe749573af44a52ca
Related-Bug: #1502297
remove_chain() has three list iterations that can be
reduced to two, brings a small performance increase.
Change-Id: I6f6e05b2336a983062f4787cb933d56fdf71a90a
Partial-bug: #1642770
When we removed the trailing space from iptables rules, we
broke the code that adds/restores them since it was assuming
it could split() based on that space to get the rule.
Just detect if the two values are equal, denoting there was
no rule instead. This is fine since a self-referencing rule
is invalid anyways.
Change-Id: I9157ced10de3099790dea7f94aa4e614ebf7f25c
Closes-bug: #1623958
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
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
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
Zuul