This is very basic, but it should give enough pointers to get started.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Previously, we reused the jsonrpc byteq with the state from a previous
iteration with potentially less headroom. However, in case we have an
empty byteq, we can forward the pointers to maximize the available
headroom and thus, the number of bytes that we can process in a single
iteration.
Co-authored-by: Felix Huettner <felix.huettner@stackit.cloud>
Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
Signed-off-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This change makes it visible how often we cannot receive a full JSON
message with a single jsonrpc_recv() call.
Signed-off-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Some newer versions of GCC 15 complain:
lib/uuid.c:308:40: error:
initializer-string for array of 'char' truncates NUL terminator but
destination lacks 'nonstring' attribute (37 chars into 36 available)
[-Werror=unterminated-string-initialization]
static const char tmpl[UUID_LEN] = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx";
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
We could add 1 to the size or use the attribute, but it seems simpler
to just convert the template into a const string pointer. There
shouldn't be any practical difference as the string will be static
either way.
It's not really a bug, the code is fine as it is, but adding a 'Fixes'
tag just as a reference for backports.
Fixes: c80eac1f85 ("ovn-controller: Tie OpenFlow and logical flows using OpenFlow cookie.")
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When bind port to multiple logical switches with
vlan and without vlan tag rules are generated without
priority. Rule without tag is more generic and matches
all traffic by in_port selector.
Since both rules has same priority first rule wins.
This patch adds priority to vlan based rule as 200
and 100 for rule without vlan to make sure rules with
vlan checked first.
Signed-off-by: Vasyl Saienko <vsaienko@mirantis.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
Currently ipf will inject completed fragments into the first available
batch. In almost all cases, this is the batch which contained the last
fragment of the packet. However, in cases where the batch is full the
packets are added to whatever random subsequent batch arrives to
conntrack. This could result in packets being processed incorrectly, for
example some completed frags may be inserted into a batch from the
interface that they should have been destined for.
This patch verifies the zone matches, and that the batch contains a
packet of the same in_port as the completed fragments.
Reported-at: https://issues.redhat.com/browse/FDP-1052
Fixes: 4ea96698f6 ("Userspace datapath: Add fragmentation handling.")
Signed-off-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
Currently ipf will inject expired fragments into the odp execution
pipeline in an indeterminate location. Previously this was changed to
segregate by IP version. However, there was nothing to stop fragments
from being inserted into different tables, zones, or being sent in
different directions.
The kernel module just terminates these fragments, and userspace should
as well. There are also new coverage counters to indicate how fragments
expired.
Reported-at: https://issues.redhat.com/browse/FDP-1052
Fixes: 4ea96698f6 ("Userspace datapath: Add fragmentation handling.")
Signed-off-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
The openflow-discuss mailing list archives at mailman.stanford.edu appear
to be offline. So use mail-archive.com URLs instead.
Flagged by make check-docs.
Reviewed-by: David Marchand <david.marchand@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Simon Horman <horms@ovn.org>
DPDK commit 41dd9a6bc2d9 ("doc: reorganize prog guide"), which
was included in DPDK 24.11-rc1:
* Renamed rte_flow.rst as ethdev/flow_offload.rst.
* Moved switch_representation.rst to ethdev/
Update the links to it in OVS documentation accordingly.
Flagged by make check-docs.
Fixes: 260925314b ("dpdk: Update to use v24.11.1.")
Reviewed-by: David Marchand <david.marchand@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Simon Horman <horms@ovn.org>
rte_eth_dev_info_get() could fail due to device reset, etc.
The return value should be checked before the device info
pointer is dereferenced.
Fixes: 2f196c80e7 ("netdev-dpdk: Use LSC interrupt mode.")
Signed-off-by: Jay Ding <jay.ding@broadcom.com>
Co-Authored-by: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
If the dp cache request misses, retry but only a couple of times so we
don't overload the system and only if we're not running on an externally
provided cache.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Use pcapng instead of pcap format and store the result, the key (if
available) and the input port name so they are visible in
wireshark/tshark.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Drop events are generated from the kernel, not ovs-vswitchd so instead
of relying on nla decoding, copy the skb's device name directly into the
event.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Printing just the datapath on each upcall gives little information (most
often, there will only be one well-known datapath). Instead, print both
the input port name (plus the datapath).
In order to do this, refactor decode_nla to always generate the dump
that only gets printed if needed. That way it can be called earlier on.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
The upcall_cost.py script has a useful port mapping cache that can be
very useful for other scripts.
Move it into a library file (usdt_lib.py). While we're at it, refactor
it into a class since setting attributes to functions and having it
return different types depending on input is not very clean.
This patch should not introduce any functional change.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
This patch makes it possible to display only succeeded or errored
upcalls.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Generate events for dropped upcalls and add the print the result.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Without this patch, the chances of events being lost on highly loaded
systems increase.
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Consider a following scenario in a 3-node cluster:
1. node-1 goes down for some reason without leaving the cluster.
2. Some changes are committed to the log.
3. node-2 requests to leave the cluster with the cluster/leave.
4. node-3 also requests to leave the cluster.
5. node-1 comes back online.
In this scenario today the cluster breaks and doesn't recover.
The reason is that both node-2 and node-3 are in the 'leaving' state,
so they do not initiate elections. node-1 is behind on log updates
and can't become a leader. But there is no leader to catch it up.
In order for the cluster to recover in this situation, one of the
leaving servers must become a leader again, then catch up node-1 with
the log updates and then node-1 can become a new leader, process both
server removal requests and become an operational cluster of 1.
The 'leaving' state is not a real state of the server in RAFT and
the server should still participate in the cluster until the removal
is committed by the majority of the NEW configuration. This can be
achieved by allowing the server in a 'leaving' state to initiate
elections and become a leader again this way. Becoming a leader also
means that this server will need to be able to execute commands and
do all the normal tasks of a leader.
Since the leader can't leave the cluster though, this server will need
to attempt to transfer leadership again in order to actually leave.
This should be done after a leave timeout. The time between becoming
a leader and transferring the leadership will allow node-1 to get up
to speed with all the cluster updates and be ready to become a new
leader.
Sending a server removal request right after transferring leadership
can't succeed, because the other server has to go through election
in order to become a leader before it can process a removal request.
So, adding a delay between the transfer and the removal request.
It can be lower than the election timer, just to avoid waiting for
too long if the election timer has a large value. But it should be
randomized, so multiple leaving servers do not just bounce the
leadership without actually getting to the server removal request.
A test is added to stress different scenarios with servers leaving
while some of the cluster members are down.
Fixes: 1b1d2e6daa ("ovsdb: Introduce experimental support for clustered databases.")
Reported-at: https://issues.redhat.com/browse/FDP-662
Tested-by: Terry Wilson <twilson@redhat.com>
Acked-by: Terry Wilson <twilson@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
These logs have been proven useful while debugging complex leadership
change scenarios.
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When the cluster falls apart, there is no leader and so there are no
heartbeats that need to be sent or received on time. In this case
the RAFT_TIMER_THRESHOLD is too small, as the process will only wake
up for a full election timer. When this happens, cooperative
multitasking code will emit warnings and traces thinking there is a
timer overrun.
Fix that by setting the multitasking interval according to the full
election timer, if there is no known leader.
Fixes: d4a15647b9 ("ovsdb: raft: Enable cooperative multitasking.")
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
All the calls to put_drop_action checks first if it is supported.
Instead, embed the check inside.
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Eli Britstein <elibr@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
flake8 2.7.0 introduced the F824 check [0].
While the check sounds reasonable on the surface, we have code in
the repository where the global variable is used only for access.
My conclusion from evaluating the alternative of adjusting the code
to appease the check is that it would invite future mistakes where
the global variable is unintentionally shadowed in addition to
making the code less clear and explicit.
0: https://github.com/PyCQA/flake8/pull/1974
1: https://github.com/PyCQA/pyflakes/pull/825
Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Checkpatch has the ability to scan source files rather than
exclusively scanning patch files. In this mode, some checks
get disabled (ex: all of the header and commit message
parsing) and the display modes are adjusted to simply print
the file and line, rather than including the patch line
details.
When checkpatch was updated for subject parsing, it was
inadvertently ignoring the source file mode. This means
any checks run against a source file will always print a
bogus subject suggestion, and warning. Fix this by not
printing these warnings when in source file mode. This
should have no effect on checkpatch against actual patch
files.
Additionally, introduce two simple tests to verify that
the source file mode still functions.
Fixes: 1b8fa4a66a ("checkpatch: Add checks for the subject line.")
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
Since we use a floating user (user with dynamic userid) and floating
groups (groups with dynamic groupid), when you use bootc the uid/gid of
the directory may change and so it's necessary to be sure that the
uid/gid is updated each time you try to start the daemon.
ovsdb-server.service is the correct place to do that, since
ovs-vswitchd.service uses After=ovsdb-server.service and so it's always started
after it.
See https://github.com/bootc-dev/bootc/issues/673#issuecomment-2493912762
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
This addition uses the existing syscall probes to record statistics
related to the OVS 'Unreasonably long ... ms poll interval' message.
Basically, it records the min/max/average time between system poll
calls. This can be used to determine if a long poll event has
occurred during the capture.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
The weak reference test sends two transactions and then waits for an
update and sends a third transaction. First transaction adds a row
and the second one deletes it. The test relies on an IDL run between
the second and the third transactions to receive and process updates
from both previous transactions, and that is not guaranteed. This
is causing frequent test failures in CI.
If the server is slow to send an update for the second transaction,
the IDL run will return without receiving it and the test application
has no way to know that it needs to wait for one more update. The
change sequence number doesn't have a meaning other than it changes
on updates, so we can't rely on it to wait for two updates.
The strong reference test has somewhat similar issue, it sends two
transactions and then expects both updates to be received together.
And that is, again, not guaranteed. However, the difference here is
that the issue (crash) that it tries to reproduce requires both
updates to be received within the same IDL run, which was not a hard
requirement for the weak reference test. The weak reference test
tolerates receiving updates in two separate IDL runs, so we could have
fixed it by waiting for an update after the first transaction without
clearing the tracking and then waiting again after the second. But
such approach would not work for the strong reference test. And there
is actually no way for us to ensure that both updates will be received
within the same IDL run. Best we can do is to sleep for some time
hoping that it's enough for both updates to be queued up. So, let's
do that. Tests are very simple and fast, so sleeping for 1 second
should be enough.
'refs to link[12]' tests are similar to a strong reference test in
terms that they require changes to be processed within the same
IDL run.
Fixes: 02f31a1262 ("ovsdb-idl: Preserve references for rows deleted in same IDL run as their insertion.")
Fixes: 91e1ff5dde ("ovsdb-idl: Don't reparse orphaned rows.")
Fixes: 4102674b3e ("ovsdb-idl: Preserve change_seqno when deleting rows.")
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
While creating passive TCP connections the library stores part of the
connection method as a bind path regardless of it being a path. And
then it may attempt to unlink it on close:
fatal-signal | WARN | could not unlink "27.0.0.1:58866"
(No such file or directory)
Unlinking only makes sense for unix sockets, not TCP, so the variable
should only be initialized for "punix" case. It's not a big problem
since those files are unlikely to exist, but it generates strange
warnings in the logs.
Fixes: af35823711 ("python: Add TCP passive-mode to IDL.")
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When a new bucket is inserted to an existing group, OVS creates a new
group with just that one bucket and then copies old buckets into it.
The problem is that dp_hash mappings remain initialized for that one
bucket and no traffic can be sent to any of the old buckets.
If a bucket is removed, then OVS creates an empty new group and then
copies old buckets into it, except for the removed one. Mappings are
also not updated in this case and the group behaves as if it had no
buckets at all.
We need to re-hash all the buckets after the copy of the old buckets.
ofproto-provider API already has a callback for this case, but it just
wasn't implemented for ofproto-dpif. It wasn't necessary in the past,
but it became necessary when the hash map construction was moved to
the ofproto-dpif layer.
No locking in this function is required, because the new group must
not be visible during modification. It becomes visible only after the
GROUP_MOD processing is finished.
Fixes: 2e3fd24c7c ("ofproto-dpif: Improve dp_hash selection method for select groups")
Reported-at: https://github.com/openvswitch/ovs-issues/issues/357
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Aaron Conole <aconole@redhat.com>
Load dictionary_code.txt in addition to the default dictionary.
Add a new command line argument --dictionaries to be able
to specify codespell dictionaries.
Acked-by: Salem Sol <salems@nvidia.com>
Signed-off-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
When the 'mirror-to' option was conceived, it was intended to allow users
the ability to arbitrarily build their own ports to use the mirror port
rather than always using an algorithmically defined mirror port. However,
the mirror port code never accommodated a port that the user defined as
part of an OVS bridge. This could be useful for running against all kinds
of OVS specific ports.
Adjust the 'mirror-to' option so that when the user specifies a port, the
utility searches through system interfaces, as well as the OVS DB. This
means that we need to drop the port_exists() check for the mirror port,
but that should be okay.
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2025-March/053508.html
Reported-at: https://issues.redhat.com/browse/FDP-1194
Reported-by: Jun Wang <junwang01@cestc.cn>
Acked-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
A few tests are parsing ovs-vswitchd.log for flow dump debug logs.
Those logs are rate limited and there is a chance required information
will not end up in the file, especially on large systems, since amount
of revalidator logs is proportional to the core count.
Fix that by disabling rate limiting for dpif module in such tests.
Fixes: c2a77f33ad ("tests/ofproto-dpif: Use vlog to test dpif behaviour.")
Reported-at: https://github.com/openvswitch/ovs-issues/issues/361
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
It can happen that ovs key attributes are not in keyAttrs of port.
In this case the call of NlAttrGetU32 will cause a BSOD in Release builds.
Signed-off-by: Frank Wagner <frank.wagner@dbosoft.eu>
Reviewed-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
This commit fixes the 'ICMP related with SNAT' test case when running
'make check-offloads'. The test case was flushing the datapath rules
and then checking the conntrack entries. In the TC offload case, this
was clearing the conntrack entries. This fix moves the conntrack entry
test before flushing the datapath rules.
Fixes: 7b74454c72 ("system-tests: Test openflow matching for ct related packets with SNAT.")
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
Pip is unable to install new pywin32 v309 with the following warning:
WARNING: The scripts pywin32_postinstall.exe and pywin32_testall.exe
are installed in 'C:\Python312-x64\Scripts' which is not on PATH.
This now fails all the windows builds.
It looks like this directory is generally required for pip and some
other packages to work properly, we just didn't hit the issue until
it became required for pywin32.
Let's add it to the PATH.
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
There is no need to start an extra bash session to run these commands.
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
