When a row is modified, python IDL doesn't perform any operations on
existing client-side indexes. This means that if the column on which
index is created changes, the old value will remain in the index and
the new one will not be added to the index. Beside lookup failures
this is also causing inability to remove modified rows, because the
new column value doesn't exist in the index causing an exception on
attempt to remove it:
Traceback (most recent call last):
File "ovsdbapp/backend/ovs_idl/connection.py", line 110, in run
self.idl.run()
File "ovs/db/idl.py", line 465, in run
self.__parse_update(msg.params[2], OVSDB_UPDATE3)
File "ovs/db/idl.py", line 924, in __parse_update
self.__do_parse_update(update, version, self.tables)
File "ovs/db/idl.py", line 964, in __do_parse_update
changes = self.__process_update2(table, uuid, row_update)
File "ovs/db/idl.py", line 991, in __process_update2
del table.rows[uuid]
File "ovs/db/custom_index.py", line 102, in __delitem__
index.remove(val)
File "ovs/db/custom_index.py", line 66, in remove
self.values.remove(self.index_entry_from_row(row))
File "sortedcontainers/sortedlist.py", line 2015, in remove
raise ValueError('{0!r} not in list'.format(value))
ValueError: Datapath_Binding(
uuid=UUID('498e66a2-70bc-4587-a66f-0433baf82f60'),
tunnel_key=16711683, load_balancers=[], external_ids={}) not in list
Fix that by always removing an existing row from indexes before
modification and adding back afterwards. This ensures that old
values are removed from the index and new ones are added.
This behavior is consistent with the C implementation.
The new test that reproduces the removal issue is added. Some extra
testing infrastructure added to be able to handle and print out the
'indexed' table from the idltest schema.
Fixes: 13973bc41524 ("Add multi-column index support for the Python IDL")
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2024-May/053159.html
Reported-by: Roberto Bartzen Acosta <roberto.acosta@luizalabs.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Terry Wilson <twilson@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
'pkg_resources' module is deprecated and no longer available in newer
versions of python, so pytest tests are skipped:
DeprecationWarning: pkg_resources is deprecated as an API.
See https://setuptools.pypa.io/en/latest/pkg_resources.html
Unfortunately, there is no direct replacement for it and functionality
is scattered between different packages.
Using a new standard library importlib.metadata to find installed
packages and their versions. Using packaging.requirements to parse
lines from the requirements file and compare versions. This covers
all we need.
The 'packaging' is a project used by pip and a dependency for many
other libraries, so should be available for any supported verison of
python. 'importlib' was introduced in python 3.8. Since we support
older versions of python and 'packaging' is not part of the standard
library, checking that import is possible and falling back to
'pkg_resources' if needed. We may remove the fallback when we stop
supporting python below 3.8.
Even though 'packaging' is a common dependency, added to the test
requirements so it will not be missed in CI.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
In general, most actions must be lists since the keys can be repeated.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Simon Horman <horms@ovn.org>
Avoid code duplication by moving the section testing code to its own
function.
Also, verify the length of the kv_list meets the expectations.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Simon Horman <horms@ovn.org>
Add the sample action to those that can be called in nested actions
(such as clone).
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
[simon: resolved conflict]
Signed-off-by: Simon Horman <horms@ovn.org>
Parsing of info and matches was being tested as generic k-v parsing.
Also verify we don't find any unexpected field.
Also, verify the length of the kv_list meets the expectations.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Simon Horman <horms@ovn.org>
OpenFlow actions names can be capitalized so in order to support this,
support case-insensitive KVDecoders and use it in Openflow actions.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Simon Horman <horms@ovn.org>
Sometimes we don't want to return the result of a nested key-value
decoding as a dictionary but as a list of dictionaries. This happens
when we parse actions where keys can be repeated.
Refactor code that already takes that into account from ofp_act.py to
kv.py and use it for datapath action "clone".
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Simon Horman <horms@ovn.org>
Currently, if a key is not found in the decoder information, we use the
default decoder which typically returns a string.
This not only means we can go out of sync with the C code without
noticing but it's also error prone as malformed flows could be parsed
without warning.
Make KeyValue parsing strict, raising an error if a decoder is not found
for a key.
This behaviour can be turned off globally by running 'KVDecoders.strict
= False' but it's generally not recommended. Also, if a KVDecoder does
need this default behavior, it can be explicitly configured specifying
it's default decoder.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Simon Horman <horms@ovn.org>
We were silently relying on some ofp actions to be decoded by the
default decoder which would yield decent string values.
In order to be more safe and robust, add an explicit decoder for all
missing actions.
This patch also reworks the learn action decoding to make it more
explicit and verify all the fields specified in the learn action are
actually valid fields.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Simon Horman <horms@ovn.org>
Currently python-ovs claims to be "db change aware" but does not
parse the "monitor_canceled" notification. Transactions can continue
being made, but the monitor updates will not be sent. This handles
monitor_cancel similarly to how ovsdb-cs currently does.
Fixes: c39751e44539 ("python: Monitor Database table to manage lifecycle of IDL client.")
Signed-off-by: Terry Wilson <twilson@redhat.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Simon Horman <horms@ovn.org>
E721: "do not compare types, for exact checks use `is` / `is not`, for
instance checks use `isinstance()`"
This fixes `make flake8-check` target when running with
pycodestyle>=1.2.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
While sending a reply to the monitor_cond_since request, server
includes the last transaction ID. And it sends new IDs with each
subsequent update. Current implementation doesn't use the one
supplied with a monitor reply, and only takes into account IDs
provided with monitor updates. That may cause various issues:
1. Performance: During initialization, the last-id is set to zero.
If re-connection will happen after receiving a monitor reply,
but before any monitor update, the client will send a new
monitor request with an all-zero last-id and will re-download
the whole database again.
2. Data inconsistency: Assuming one of the clients sends a
transaction, but our python client disconnects before receiving
a monitor update for this transaction. The last-id will point
to a database state before this transaction. On re-connection,
this last-id will be sent and the monitor reply will contain
a diff with a new data from that transaction. But if another
disconnection happens right after that, on second re-connection
our python client will send another monitor_cond_since with
exactly the same last-id. That will cause receiving the same
set of updates again. And since it's an update2 message with
a diff of the data, the client will remove previously applied
result of the transaction. At this point it will have a
different database view with the server potentially leading
to all sorts of data inconsistency problems.
Fix that by always updating the last-id from the latest monitor
reply.
Fixes: 46d44cf3be0d ("python: idl: Add monitor_cond_since support.")
Acked-by: Simon Horman <horms@ovn.org>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Quoting Paul Ganssle, setuptools maintainer:
* The setuptools project has stopped maintaining all direct
invocations of setup.py years ago, and distutils is deprecated.
There are undoubtedly many ways that your setup.py-based system is
broken today, even if it's not failing loudly or obviously.
* Direct invocations of setup.py cannot bootstrap their own
dependencies, and so some CLI is necessary for dependency
management.
* The setuptools project no longer wants to provide any public CLI,
and will be actively removing the existing interface (though the
time scale for this is long).
* PEP 517, 518 and other standards-based packaging are the future of
the Python ecosystem and a lot of progress has been made on making
this upgrade seamless.
As described in the recommendations in the end of the article: `python3
setup.py sdist` should be replaced by `python3 -m build --sdist`.
Link: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html#summary
Signed-off-by: Robin Jarry <rjarry@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
setup.py upload is now deprecated. When used, pypi.org returns an error:
Upload failed (400): Invalid value for blake2_256_digest. Error: Use
a valid, hex-encoded, BLAKE2 message digest.
Use twine which is the recommended replacement tool to upload on
pypi.org.
Link: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html#summary
Reported-by: Terry Wilson <twilson@redhat.com>
Signed-off-by: Robin Jarry <rjarry@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
The python/build folder contents are completely unrelated to the ovs
python bindings. These files are only used during the build for various
subsystems (docs, man pages, code generation, etc.).
Having that folder in that location prevents from running:
cd python && python3 -m build
Which is a way to generate PEP517 compatible source archives and binary
wheel packages.
Rename that folder to ovs_build_helpers which is more explicit. Update
all imports accordingly.
Link: https://peps.python.org/pep-0517/
Link: https://pypi.org/project/build/
Signed-off-by: Robin Jarry <rjarry@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
In some cases ovsdb server or relay gets restarted, ovsdb python clients
may keep the local socket open. Instead of reconnecting a lot of failures
will be logged.
This can be reproduced with ssl connections to the server/relay and
restarting it, so it has the same IP after restart.
This patch catches the Exceptions at do_handshake to recreate the
connection on the client side.
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Stefan Hoffmann <stefan.hoffmann@cloudandheat.com>
Signed-off-by: Luca Czesla <luca.czesla@mail.schwarz>
Signed-off-by: Max Lamprecht <max.lamprecht@mail.schwarz>
Co-authored-by: Luca Czesla <luca.czesla@mail.schwarz>
Co-authored-by: Max Lamprecht <max.lamprecht@mail.schwarz>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Fix the following erros in odp decoding:
- Missing push_mpls action
- Typos in collector_set_id, tp_src/tp_dst and csum
- Missing two fields in vxlan match
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When initializing a monitor table the default monitor condition is
[True] which matches the behavior of the server (to send all rows of
that table). There's no need to include this default condition in the
initial monitor request so we can consider it implicitly acked by the
server.
This fixes the incorrect (one too large) expected condition sequence
number reported by ovsdb_idl_set_condition() when application is
trying to set a [True] condition for a new table.
Reported-by: Numan Siddique <numans@ovn.org>
Suggested-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Fixes idl.Row's __str__ method to only print if the column exists on
the object. The Row object passed to the 'updates' argument of
Idl.notify only contains a subset of columns. Printing that argument
causes an AttributeError.
Fixes: 6a1c98461b46 ("Add a __str__ method to idl.Row")
Submitted-at: https://github.com/openvswitch/ovs/pull/392
Acked-by: Terry Wilson <twilson@redhat.com>
Signed-off-by: Christopher Aubut <christopher@aubut.me>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
pyOpenSSL was recently switched for the Python standard library ssl
module in the cited commit. Python SSLsocket.send() does not allow
non-zero optional flag and it will explicitly raise an exception for
that. pyOpenSSL did nothing with this flag but kept it to be
compatible with socket API:
https://github.com/pyca/pyopenssl/blob/main/src/OpenSSL/SSL.py#L1844
Fixes: 68543dd523bd ("python: Replace pyOpenSSL with ssl.")
Reported-at: https://bugzilla.redhat.com/2115035
Acked-By: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Miro Tomaska <mtomaska@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
With just released flake8 5.0 we're getting a bunch of E275 errors:
utilities/bugtool/ovs-bugtool.in:959:23: E275 missing whitespace after keyword
tests/test-ovsdb.py:623:11: E275 missing whitespace after keyword
python/setup.py:105:8: E275 missing whitespace after keyword
python/setup.py:106:8: E275 missing whitespace after keyword
python/ovs/db/idl.py:145:15: E275 missing whitespace after keyword
python/ovs/db/idl.py:167:15: E275 missing whitespace after keyword
make[2]: *** [flake8-check] Error 1
This breaks CI on branches below 2.16. We don't see a problem right
now on newer branches because we're installing extra dependencies
that backtrack flake8 down to 4.1 or even 3.9.
Acked-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
On Python 3.12, distutils will be removed and it's currently (3.10+)
deprecated (see PEP 632).
Since the suggested and simplest replacement is setuptools, this commit
replaces distutils to use setuptools instead.
setuptools < 59.0 doesn't have setuptools.errors and so, in this case,
distutils.errors is still used.
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
In order to correctly link with static libopenvswitch.a library,
users should also provide required cflags and all the libraries
libopenvswitch.a was actually built with and depends on. Otherwise,
it's not possible to link correctly.
Fixes: 671f93fe42d3 ("python: Allow building json C extension with static OVS library.")
Acked-by: Frode Nordahl <frode.nordahl@canonical.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
PyModule_AddObject() may fail and it doesn't steal references
in this case. The error condition should be handled to avoid
possible memory leaks.
And while it's not strictly specified if PyModule_Create may
fail, most of the examples in python documentation include
handling of a NULL case.
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Since Python 2 is not supported anymore, remove Python 2 support from C
extension too
Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Add unit test for OFFilter class.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Add unit tests for OFPFlow class and ip-port range decoder
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Use pytest to run unit tests as part of the standard testsuite.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
The json encoder can be used to convert Flows to json.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Based on pyparsing, create a very simple filtering syntax.
It supports basic logic statements (and, &, or, ||, not, !), numerical
operations (<, >), equality (=, !=), and masking (~=). The latter is only
supported in certain fields (IntMask, EthMask, IPMask).
Masking operation is semantically equivalent to "includes",
therefore:
ip_src ~= 192.168.1.1
means that ip_src field is either a host IP address equal to 192.168.1.1
or an IPMask that includes it (e.g: 192.168.1.1/24).
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
A ODPFlow is a Flow with the following sections:
ufid
info (e.g: bytes, packets, dp, etc)
match
actions
Only three datapath actions require special handling:
gre: because it has double parenthesis
geneve: because it supports many concatenated lists of options
nat: we reuse the decoder used for openflow actions
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Introduce OFPFlow class and all its decoders.
Most of the decoders are generic (from decoders.py). Some have special
syntax and need a specific implementation.
Decoders for nat are moved to the common decoders.py because it's syntax
is shared with other types of flows (e.g: dpif flows).
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
It simplifies the implementation of different types of flows by creating
the concept of Section (e.g: match, action) and automatic accessors for
all the provided Sections
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Based on meta-field information extracted by extract_ofp_fields,
autogenerate the right decoder to be used.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
In order to be able to reuse the core extraction logic, split the command
in two parts. The core extraction logic is moved to python/build while
the command that writes the different files out of the extracted field
info is kept in build-aux.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Some openflow or dpif flows encode their arguments in lists, eg:
"some_action(arg1,arg2,arg3)". In order to decode this in a way that can
be then stored and queried, add ListParser and ListDecoders classes
that parse lists into KeyValue instances.
The ListParser / ListDecoders mechanism is quite similar to KVParser and
KVDecoders. Since the "key" of the different KeyValue objects is now
ommited, it has to be provided by ListDecoders.
For example, take the openflow action "resubmit" that can be written as:
resubmit([port],[table][,ct])
Can be decoded by creating a ListDecoders instance such as:
ListDecoders([
("port", decode_default),
("table", decode_int),
("ct", decode_flag),
])
Naturally, the order of the decoders must be kept.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Add more decoders that can be used by KVParser.
For IPv4 and IPv6 addresses, create a new class that wraps
netaddr.IPAddress.
For Ethernet addresses, create a new class that wraps netaddr.EUI.
For Integers, create a new class that performs basic bitwise mask
comparisons
netaddr is added as a new shoft dependency:
- extras_require in setup.py
- Suggests in deb and rpm packages
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Most of ofproto and dpif flows are based on key-value pairs. These
key-value pairs can be represented in several ways, eg: key:value,
key=value, key(value).
Add the following classes that allow parsing of key-value strings:
* KeyValue: holds a key-value pair
* KeyMetadata: holds some metadata associated with a KeyValue such as
the original key and value strings and their position in the global
string
* KVParser: is able to parse a string and extract it's key-value pairs
as KeyValue instances. Before creating the KeyValue instance it tries
to decode the value via the KVDecoders
* KVDecoders holds a number of decoders that KVParser can use to decode
key-value pairs. It accepts a dictionary of keys and callables to
allow users to specify what decoder (i.e: callable) to use for each
key
Also, flake8 seems to be incorrectly reporting an error (E203) in:
"slice[index + offset : index + offset]" which is PEP8 compliant. So,
ignore this error.
Acked-by: Terry Wilson <twilson@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Allow caller of setup.py to pass in libopenvswitch.a as an object
for linking through the use of LDFLAGS environment variable when
not building a shared openvswitch library.
To accomplish this set the `enable_shared` environment variable to
'no'.
Example:
LDFLAGS=lib/libopenvswitch.a enable_shared=no setup.py install
Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Prior to 4e3966e64, when calling _uuid_to_row, it would raise an
AttributeError when trying to access base.ref_table.rows if the
referenced table was not registered. When called from
Row.__getattr__(), this would appropriately raise an AttributeError.
After 4e3966e64, a KeyError would be raised, which is not expected
from a getattr() or hasattr() call, which could break existing
code.
Fixes: 4e3966e64bed ("python: Politely handle misuse of table.condition.")
Signed-off-by: Terry Wilson <twilson@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
