This patch adds a new configuration variable to control the OVS
OpenFlow rule processing operations:
* ``openflow_processed_per_port``: by default "False". If enabled,
all OpenFlow rules associated to a port will be processed at once,
in one single transaction. If disabled, the flows will be processed
in batches of "AGENT_RES_PROCESSING_STEP=100" number of OpenFlow
rules.
With ``openflow_processed_per_port`` enabled, all Firewall
OpenFlow rules related to a port are processed in one transaction
(executed in one single command). That ensures the rules are written
atomically and apply all of them at the same time.
That means all needed rules to handle the ingress and egress traffic
of a port using the Open vSwitch Firewall, are committed in the OVS
DB at the same time. That will prevent from partially applied OpenFlow
sets in the Firewall and inconsistencies when applying new SG rules or
during the OVS agent restart.
That will override, if needed, the hard limit of
"AGENT_RES_PROCESSING_STEP=100" OpenFlow rules that could be
processed in OVS at once.
If the default configuration values are not modified, the behaviour of
the OVS library does not change.
Closes-Bug: #1934917
Change-Id: If4984dece266a789d607725f8497f1aac3d73d23
Setting new controller for bridge every time when neutron-ovs-agent
is restarted or is doing full-sync may cause some short data plane
connectivity loss and is not needed if same controller is already
configured for the bridge.
With this patch neutron-ovs-agent will first check if controller is
configured for the bridge and if it's the same as what should be
configured, it will skip setting it up.
With this patch also protocols added to the bridge will be first checked
if they aren't already there and only missing ones will be added if
necessary.
Setting of the connectivity mode and inactivity probe is
always performed as this don't cause connectivity issues and is cheap
so we can always ensure that those parameters are configured properly.
Closes-Bug: #1948642
Change-Id: Idfa763df8c60d8ae46cd6351d1b6dc7d950b4c67
This change removes the "_check_ofport" function and its use form
the ovs_lib.py file.
By skipping ports without a unique ofport in the "get_vifs_by_ids"
and "get_vifs_by_id" functions, the OVS agent incorrectly treated
newly added port with an ofport of -1 as removed ports in the
"treat_devices_added_or_updated" function.
Co-Authored-By: Rodolfo Alonso Hernandez <ralonsoh@redhat.com>
Change-Id: I79158baafbb99bee99a1d687039313eb454d3a9b
Partial-Bug: #1734320
Partial-Bug: #1815989
When creating the OVS manager, define the command timeout
(CONF.OVS.ovsdb_timeout) and inactivity probe time
(CONF.OVS.of_inactivity_probe)
NOTE: CONF.OVS.of_inactivity_probe is defined in seconds but the
parameter should be passed to ovs-vsctl in milliseconds [1].
[1]http://www.openvswitch.org/support/dist-docs/ovs-vswitchd.conf.db.5.txt
Depends-On: https://review.opendev.org/#/c/720785
Change-Id: I8ed1fc85c2f78710bf6589ba3deca518471339b8
Closes-Bug: #1868686
When a Port is deleted, the QoS extension will reset any rule (QoS
and Queue registers) applied on this port or will reset the
related Interface policing parameters.
If the Port and the related Interface are deleted during the QoS
extension operation, those commands will fail. This patch makes those
operations more resiliant by not checking the errors when writing on
the Port or the Interface register.
Change-Id: I2cc4cdf5be25fab6adbc64acabb3fffebb693fa6
Closes-Bug: #1884512
Now that we are python3 only, we should move to using the built
in version of mock that supports all of our testing needs and
remove the dependency on the "mock" package.
This patch moves all references to "import mock" to
"from unittest import mock". It also cleans up some new line
inconsistency.
Fixed an inconsistency in the OVSBridge.deferred() definition
as it needs to also have an *args argument.
Fixed an issue where an l3-agent test was mocking
functools.partial, causing a python3.8 failure.
Unit tests only, removing from tests/base.py affects
functional tests which need additional work.
Change-Id: I40e8a8410840c3774c72ae1a8054574445d66ece
skb mark is not supported when using ovs hw-offload and using it
breaks the vxlan offload.
This patch clear skb mark only if ovs hw-offload is disabled.
This should be fine as ovs with hw-offload runs on the
compute node (DVR is not supported), so clear the skb mark for
the qrouter is not needed.
Closes-Bug: #1855888
Change-Id: I71f45fcd9b7e7bdacaafc7fa96c775e88333ab48
get_port_external_ids and get_port_mac are no longer used after
cf37563c83, this is to remove them.
Besides, unit testcases test_get_port_external_ids_retry/fails
are added along with the adding of get_port_external_ids in
0a4596aa20, so though they don't
call get_port_external_ids directly, we may remove them as well.
Change-Id: I84633221b916f18ec099080d66f0cd2018731d7c
Sometimes ports can have a lot of security group rules. Adding OpenFlow
rules to the integration bridge can be a bottle neck when bundle option
is not used.
As OVS firewall currently uses CLI interface, the bundle support is
implemented only for that. The native interface has on-going work but
before we switch completely to the native, we can benefit from bundle
option now.
e.g. when adding 100 000 flows the bundle option takes the time down
from ~10 seconds to 1 seconds. So it's about 10 times faster.
Change-Id: I1feaeb659c8badf23230e36145235d49d50b6bfb
Adding ability to set DSCP field in OVS tunnels outer header, or
inherit it from the inner header's DSCP value for OVS and linuxbridge.
Change-Id: Ia59753ded73cd23019605668e60cfbc8841e803d
Closes-Bug: #1692951
When we delete vm port with attached QoS policy,
it is just doing nothing if vif_port does not exist.
This is fine for egress bandwidth limit as it is configured
directly on vif_port in OVS.
For ingress bw limit however it uses additional records in
Openvswitch database: qos and queue. Those records are not
cleaned up in such case.
This patch also records port in self.ports in the case of
bandwidth limit rules, just as in the case of dscp rules.
Never execute port clear if vif_port not exists. Finally, ovs
driver can clean such qos and queue records
Change-Id: Iddeb49e1e6538a178ca468df0fdf9e0617ca4f1c
Closes-Bug: #1726732
neutron-lib contains a number of the plugin related constants from
neutron.plugins.common.constants. This patch consumes those constants
from neutron-lib and removes them from neutron. In addition the notion
of the dummy plugin service type is moved strictly into the test
package of neutron since it's not a real service plugin.
NeutronLibImpact
Change-Id: I767c626f3fe6159ab3abd6a7ae3cb9893b79bf66
If port update event is received by L2 agent and there is
no QoS policy assigned to such port, agent tries to delete
any existing QoS rules from port.
For bandwidth limit rules OVS qos driver tries to update
port in OVS database.
In case if port not exists in OVS database agent raised
exception.
This patch fixes that by checking if port really exists and
removing QoS bandwidth limit rules only for existing ports.
Change-Id: I3775ee7b383ada6e4e4ace53b5405aa3c7c22027
Closes-Bug: 1712913
This change moves into ovs_lib the behavior ensuring that
ovs-ofctl is used with the right OF protocol version: the
highest of all versions configured with use_at_least_protocol().
This use_at_least_protocol() method keeps track of the highest
version requested, and also calls add_protocols to ensure that
this version is actually available on the bridge.
This change allows to avoid a wrapping/decorator in networking-sfc
(see I25fb8f35f8236c59a7dca9685a47de0ae77846bc).
For this to be possible another change is needed: if the OF version
that ends up being used is >1.0 then the strip_vlan actions can't
be used without having matched on a vlan_id ; this is achieved
in I76ee34a614237bbc99989ce9c1b96a30456be282 of which this change
is a follow-up.
Change-Id: I16a35b5d6c54901899d24fc94bd3438c1f1be05e
Needed-By: I25fb8f35f8236c59a7dca9685a47de0ae77846bc
In some cases we would want to refrain from cleaning up specific
openvswitch ports.
In Octavia, the health manager service is using a predefined[1]
openvswitch port which will gets nuked by the ovs_cleanup script in the
boot process.
That port is created by the operating system NIC configuration file
(by using OVS_EXTRA[2]), but due to the order of actions in the boot
process, the ovs_cleanup script gets invoked by systemd only at a later
stage. As a result the port will be deleted each time and the Octavia
health manager service will fail to bind.
This patch takes advantage of the 'external_ids' column that already
exists for ovs ports, in order to filter out ports we would like to
skip. We filter those ports by adding 'skip_cleanup' to the
'external_ids' column.
It is important to note that this will work if we append the following
to the port: -- set Interface o-hm0 external_ids:skip_cleanup=true"
Related-Bug: #1685223
[1] http://git.openstack.org/cgit/openstack/octavia/tree/devstack/plugin.sh?h=stable/ocata#n190
[2] https://github.com/osrg/openvswitch/blob/master/rhel/README.RHEL#L102
Change-Id: If483d0ee027596999370ab0d21b1743d4ef16acb
n8g-sfc currently has its own variant of OVSBridge to allow the use
of priority in a delete_flows call
This change is meant to make this available outside n8g-sfc and
simplify n8g-sfc code.
This change adds a 'strict' boolean parameter to mod_flow and delete_flows
that results in ovs-ofctl to be run with --strict for del-flows and
mod-flows actions. When strict is set, the use of priority is allowed
and hence not rejected anymore.
Note that for batched actions in a deffered bridge, we disallow mixing
calls with strict and without strict, which can't be translated in one CLI
call.
Needed-By: I3bf939590dd43bff685f133bff86eb7e9068de91
Change-Id: I289d546780f10dc1002ab6bc2e1b38c9ef2d728f
When neutron is deployed with hypervisor is XenServer, current
implementation will grab port's iface-id via xapi, but this isn't
the proper way:
Port's iface-id is already set when creating VM or hot plugging
VIFs in nova project, so there is no need to grab it via xapi
Change-Id: Ie07527cc89ac81ff1e3519db66925cee482f77a4
Closes-Bug: #1649747
With this change delete_flows will only remove flows matching the default
cookie of the bridge.
The uninstall_flows implementation in the native bridge is also modified
to touch only the flows with the bridge cookie.
To still allow deletion of all cookies, cookie=COOKIE_ANY is introduced
as a special value, and used in the agent code in the places where the
intent is indeed to clean all flows whatever their cookie is.
Partial-Bug: #1557620
Change-Id: Idd0531cedda87224531cb8fb6a912ccd0f1554d5
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
With this change calling delete_flows with no kwargs will (instead
of resulting in calling "ovs-ofctl <action> <bridge> -", which does
nothing with no flow spec given on stdin) result in calling
"ovs-ofctl <action> <bridge>", which will delete all flows.
This aligns the behavior of delete_flows with the behavior currently seen by
all callers for the same method shadowed by the implementations in
OpenFlowSwitchMixin classes.
Change-Id: Ic0449acb3a0d4915ce025300d6f3c507a3cd8e48
Closes-Bug: 1658019
Provide a method called get_port_mac()
networking-sfc needs the mac address of port "patch-tun".
This is not a neutron port.
Change-ID: I61f607f0ed40da13bb2cc1ea72993f43d4a9c65b
While debugging the related bug, this showed up in the logs:
AddBridgeCommand(datapath_type=None, may_exist=system, name=test-brd8f3648c)
This was caused by the add_bridge command not passing in the datapath
type as named argument so it was taking the positional argument of
may_exist. So the datapath type was being lost whenever this command
was being used.
This fixes it by makeing add_bridge use a named argument for
datapath_type and it also switches it to using the
OVSBridge.create() method to unify the code-paths for bridge
creation.
It's not clear that this was a cause of the bug though, so
this fix is only related.
Change-Id: I5711c29c21741dd847f2c27fb621f723b24cb7fd
Related-Bug: #1627106
- patch DB retry count for tests excercising retry decorator.
These tests were taking 40 seconds per run (one used by 4 classes)
by going through the full retry count with backoff. ~240 seconds of
CPU time lost to them.
- Adjust vsctl_timeout down on ofport retry test.
This one was taking up to 10 seconds with the default timeout.
TrivialFix
Change-Id: Iabe99c06abde81ced7e8dfa48bfe8b066c59ce70
We are replacing all usages of the 'retrying' package with
'tenacity' as the author of retrying is not actively maintaining
the project. Tenacity is a fork of retrying, but has improved the
interface and extensibility (see [1] for more details). Our end
goal here is removing the retrying package from our requirements.
Tenacity provides the same functionality as retrying, but has the
following major differences to account for:
- Tenacity uses seconds rather than ms as retrying did.
- Tenacity has different kwargs for the decorator and
Retrying class itself.
- Tenacity has a different approach for retrying args by
using classes for its stop/wait/retry kwargs.
- By default tenacity raises a RetryError if a retried callable
times out; retrying raises the last exception from the callable.
Tenacity provides backwards compatibility here by offering
the 'reraise' kwarg.
- Tenacity defines 'time.sleep' as a default value for a kwarg.
That said consumers who need to mock patch time.sleep
need to account for this via mocking of time.sleep before
tenacity is imported.
- For retries that check a result, tenacity will raise if the retried
function raises, whereas retrying retried on all exceptions.
This patch updates all usages of retrying with tenacity.
Unit tests will be included where applicable.
Note: This change is not newton critical so projects are welcome
to hold off on committing until post-newton. Ideally this change
will merge by the first part of Ocata so dependant functionality
can land and have time to solidify for Ocata.
[1] https://github.com/jd/tenacity
Closes-Bug: #1635395
Change-Id: I1c0620894d07d58efbba5226b5244fec950354ca
- unit tests were fixed mainly by mocking
Connection class of native implementation.
- some ovs-lib tests rely on direct ovs-vsctl
output. Temporarily decorated with @vsctl_only.
UpgradeImpact
Change-Id: I2632b0e21edd61536867a9fc830a45d9899091e4
When the OVS bridge is still being initialized we get
a "failed to connect to socket" error when running ovs-ofctl.
This shows up quite frequently in our functional tests and
may be the source of their high failure rate.
Ultimately we need to change the behavior of run_ofctl to not
ignore errors by default, but this will require a lot of effort
because there are many places that likely expect this behavior.
As a workaround, this patch checks for the specific socket failure
and attempts the command again up to 10 times, sleeping for 1
second between each attempt to wait for the bridge to be ready.
Closes-Bug: #1550623
Closes-Bug: #1551593
Change-Id: I663a54608ed96133014104fe033ecea0a867ac4c
- Introduces an API to allow l2-agents to access resources within
the Open vSwitch Agent, specifically the integration and tunnel bridges.
- adds consume_api method to the AgentCoreResourceExtension class.
- modifies the AgentExtensionManager class to accept the AgentExtensionAPI
class as an optional argument.
- adds the OVSAgentExtensionAPI class.
- modifies ovs_lib and ofswitch to include a list of uuid stamps
to exempt from flow deletion.
- adds the OVSBridgeCookieMixin class that manages the distribution of
cookies and maintains the list of reserved cookies.
- modifies OVSNeutronAgent to initialize OVSAgentExtensionAPI and pass
into the AgentExtensionManager.
Partial-Bug: #1517903
Co-Authored-By: Nate Johnston <nate_johnston@cable.comcast.com>
Co-Authored-By: Thomas Morin <thomas.morin@orange.com>
Implements: blueprint l2-api-extensions
Change-Id: I7cb61f30689dff2d7895d444060dedc1532a63ec
This change will not force a resync in the case where a virtual machine is
deleted, and therefore its OVS port deleted, in between the time an RPC
call was made to get the devices and where we make the call to correlate
those devices to vif ports.
Change-Id: Ie55eb69ad7ee177f0cf8ee8fc7fc585fbd0d4a22
Closes-Bug: #1499488
Neutron should use the specific assertion:
self.assertIsNone(observed)
instead of the generic assertion:
self.assertEqual(None, observed)
as it raises more specific errors.
Closes-Bug: #1503055
Change-Id: Ib7e5875bd0a95320d89a7504f951998fb210acc1
The new option for the ovs agent will enable to set/unset the
csum option for the vxlan/gre tunnels. The default is maintained as False.
Change-Id: I18dcd8946b585e70f8890a5c222ea37059c4a0c5
Implements: bp ovs-tunnel-csum-option
Closes-bug: #1492111
Without that fix flows applied to br-tun through
DeferredOVSBridge are created without cookie.
That results in l2pop flows being deleted in the process of
cleanup of stale flows.
Solution is to add cookie to all add/mod-flows of OVSBrigde
class in the method do_action_flows.
Also, agent_uuid_stamp moved to a proper place - into the
base OVSBridge class as storing attributes in Mixing was
just a wrong code design.
Change-Id: Ic09a0dbc04fc5da38d30e1392cf2ea27d576040c
Closes-Bug: #1489372
This change introduces a new datapath_type parameter
to allow specification of the ovs datapath to be used.
This change introduces new functional and unit tests.
DocImpact
Change-Id: I929d8d15fc6cfdb799c53ef0f3722f4ed5c1096d
Partial-Bug: #1469871
When agent is restarted it drops all existing flows. This
breaks all networking until the flows are re-created.
This change adds an ability to drop only old flows.
Agent_uuid_stamp is added for agents. This agent_uuid_stamp is set as
cookie for flows and then flows with stale cookies are deleted during
cleanup.
Co-Authored-By: Ann Kamyshnikova<akamyshnikova@mirantis.com>
Closes-bug: #1383674
DocImpact
Change-Id: I95070d8218859d4fff1d572c1792cdf6019dd7ea
The idea here was to remove redundant unit tests.
The approach here has been that if the function being tested does not
implement any custom logic (apart from calling ovsdb), the unit test
does not help.
Refer to the bug description for more details of the specific tests
removed.
Change-Id: I35dc60bb714566c33f5cee5aab3e5b83bd0610e3
Closes-Bug: #1459811
OVSBridge was inheriting db_list from BaseOVS, which was
returning the information of all the ports on the machine,
not only the ones belonging to the bridge.
The OVSNeutronAgent was using that method with the assumption
that ports were filtered by bridge.
To avoid confusion, this patch add a new method to OVSBridge
get_ports_attributes to query the info for all the ports
belonging to the bridge.
db_list is removed from BaseOVS since that method is already
available in ovsdb/api.py
ovs_lib methods that use db_list are refactored accordingly.
Co-Authored-By: Assaf Muller <amuller@redhat.com>
Change-Id: I2ce6d232744f48ba7fc0f824a7db32e3655bc2aa
Closes-Bug: 1473199
This patch updates get_vif_ports so that it skips
ports which aren't in the 'Interfaces' table.
This fixes an issue where neutron-ovs-cleanup would
fail if any sort of OVS bond was on the bridge getting
cleaned up. This is because bonds don't have the same
attributes as ports, and thus fail subsequent ovs-vsctl
queries.
Change-Id: Ic9d30e5916122ce23c5dc8631fbb71115ae8a960
Closes-bug: #1473179
Slawek Kaplonski