The PMD reference taken is not actually used, it is only needed to get
the dp_netdev linked. Additionally, the taking of the PMD reference
does not protect against the disappearance of the dp_netdev,
so it is misleading.
The dp reference is protected by the way the ports are being deleted
during datapath deletion. No further offload request should be found
past a flush, so it is safe to keep this reference in the offload item.
Signed-off-by: Gaetan Rivet <grive@u256.net>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
In dp_netdev_pmd_remove_flow() we schedule the deletion of an
offloaded flow, if a mark has been assigned to the flow. But if
this occurs in the window in which the offload thread completes
offloading the flow and assigns a mark to the flow, then we miss
deleting the flow. This problem has been observed while adding
and deleting flows in a loop. To fix this, always enqueue flow
deletion regardless of the flow->mark being set.
Fixes: 241bad15d99a("dpif-netdev: associate flow with a mark id")
Co-authored-by: Gaetan Rivet <grive@u256.net>
Signed-off-by: Gaetan Rivet <grive@u256.net>
Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Port flush and offload uninit should be moved after the datapath
has been reconfigured. That way, no other thread such as PMDs will
find this port to poll and enqueue further offload requests.
After a flush, almost no further offload request for this port should
be found in the queue.
There will still be some issued by revalidators, but they
will be caught when the offload thread fails to take a netdev ref.
This change fixes the issue of datapath reference being improperly
accessed by offload threads while it is being destroyed.
Fixes: 5b0aa55776 ("dpif-netdev: Execute flush from offload thread.")
Fixes: 62d1c28e9c ("dpif-netdev: Flush offload rules upon port deletion.")
Suggested-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Signed-off-by: Gaetan Rivet <grive@u256.net>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
The subtable search function can be used at any time by a PMD thread.
Setting the subtable search function should be done atomically to
prevent garbage data from being read.
A dpcls_subtable_lookup_reprobe() call can happen at the same time that
DPCLS subtables are being sorted. This could lead to modifications done
by the reprobe() to be lost. Prevent this from happening by locking on
pmd->flow_mutex. After this change both the reprobe function and a
subtable sort will share the flow_mutex preventing modifications by
either one from being lost.
Also remove the pvector_publish() call. The pvector is not being changed
in dpcls_subtable_lookup_reprobe(), only the data pointed to by pointers
in the vector are being changed.
Fixes: 3d018c3ea7 ("dpif-netdev: add subtable lookup prio set command.")
Reported-by: Ilya Maximets <i.maximets@ovn.org>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2022-January/390757.html
Signed-off-by: Cian Ferriter <cian.ferriter@intel.com>
Acked-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Reported by UndefinedBehaviorSanitizer:
tests/idltest.c:3602:12:
runtime error: member access within null pointer of type
'const struct idltest_simple'
#0 0x4295af in idltest_simple_cursor_first_ge tests/idltest.c:3602
#1 0x41c81b in test_idl_compound_index_single_column tests/test-ovsdb.c:3128
#2 0x41e035 in do_idl_compound_index tests/test-ovsdb.c:3277
#3 0x4cf640 in ovs_cmdl_run_command__ lib/command-line.c:247
#4 0x4cf79f in ovs_cmdl_run_command lib/command-line.c:278
#5 0x4072f7 in main tests/test-ovsdb.c:79
#6 0x7fa858675b74 in __libc_start_main (/lib64/libc.so.6+0x27b74)
#7 0x4060ed in _start (/root/ovs/tests/test-ovsdb+0x4060ed)
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
UB Sanitizer report:
lib/stopwatch.c:119:22: runtime error: index 18446744073709551615 out of
bounds for type 'long long unsigned int [50]'
#0 0x698358 in calc_percentile lib/stopwatch.c:119
#1 0x69ada1 in add_sample lib/stopwatch.c:231
#2 0x69c086 in stopwatch_end_sample_protected lib/stopwatch.c:386
#3 0x69c522 in stopwatch_thread lib/stopwatch.c:441
#4 0x684bae in ovsthread_wrapper lib/ovs-thread.c:383
#5 0x7f042838b298 in start_thread (/lib64/libpthread.so.0+0x9298)
#6 0x7f04277f2352 in clone (/lib64/libc.so.6+0x100352)
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
It's actually undefined behavior to pass NULL to standard library
functions that manipulate arrays (e.g., qsort, memcpy, memcmp), even if
the passed number of items is 0.
UB Sanitizer reports:
ovsdb/monitor.c:408:9: runtime error: null pointer passed as argument 1,
which is declared to never be null
#0 0x406ae1 in ovsdb_monitor_columns_sort ovsdb/monitor.c:408
#1 0x406ae1 in ovsdb_monitor_add ovsdb/monitor.c:1683
[...]
lib/ovsdb-data.c:1970:5: runtime error: null pointer passed as argument 2,
which is declared to never be null
#0 0x4071c8 in ovsdb_datum_push_unsafe lib/ovsdb-data.c:1970
#1 0x471cd0 in ovsdb_datum_apply_diff_in_place lib/ovsdb-data.c:2345
[...]
ofproto/ofproto-dpif-rid.c:159:17:
runtime error: null pointer passed as argument 1,
which is declared to never be null
#0 0x4df5d8 in frozen_state_equal ofproto/ofproto-dpif-rid.c:159
#1 0x4dfd27 in recirc_find_equal ofproto/ofproto-dpif-rid.c:179
[...]
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Some sporadic false positive may be visible for the following tests:
- conntrack - IPv6 HTTP
- conntrack - FTP over IPv6
The failures show up randomly.
The reason appears to be source address used when performing the
request using wget:
-tcp,orig=(src=fc00::1,dst=fc00::2,sport=<cleared>,dport=<cleared>),\
reply=(src=fc00::2,dst=fc00::1,sport=<cleared>,dport=<cleared>),\
protoinfo=(state=<cleared>)
+tcp,orig=(src=fe80::f0eb:f8ff:fef0:138f,dst=fc00::2,sport=<cleared>,dport=<cleared>),\
reply=(src=fc00::2,dst=fe80::f0eb:f8ff:fef0:138f,sport=<cleared>,dport=<cleared>),\
protoinfo=(state=<cleared>)
It seems that the problem can be addressed in multiple ways, but using
"nodad" seems to be safe enough to fix the issue that now, after
hundreds of attempts, is no longer present.
Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
turn a bunch of test ranges from 10.1.1.240-10.1.1.255 to
10.1.1.240-10.1.1.254. 10.1.1.255 is the broadcast address for
10.1.1.0/24 and can be picked to SNAT packets causing the subsequent
failure of the test.
Fixes: 9ac0aadab9 ("conntrack: Add support for NAT.")
Fixes: e32cd4c629 ("conntrack: ignore port for ICMP/ICMPv6 NAT.")
Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Until now, the monitor process held its log file open while it waited for
the child to exit, and then it reopened it after the child exited. Holding
the log file open this way prevented log rotation from freeing disk space.
This commit avoids the problem by closing the log file before waiting, then
reopening it afterward.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Reported-by: Antonin Bas <abas@vmware.com>
VMware-BZ: #2743409
Signed-off-by: William Tu <u9012063@gmail.com>
Some specific warning are seen on various systems
which may not be visible on others but good to add
such logs to test to avoid test-case failure.
Thw warning only effects the fuzzy tests due to
more than 1000+ flows being offloading simultanously.
Wilcarding flow count number as for different systems
under test the number could vary in the warning log.
Suggested-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Kumar Amber <kumar.amber@intel.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
Acked-by: Cian Ferriter <cian.ferriter@intel.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
We are currently requiring in_port to be a valid port number for ipfix
sampling even if the sampling is done on the output port (egress).
This restriction is not really needed and can affect pipelines that
intentionally set the in_port to OFPP_NONE during flow processing. For
instance, OVN does this, see:
cfa547821 Fix ovn-controller generated packets from getting dropped for
reject ACL action.
This patch skips ipfix sampling only if both (ingress and egress) ports
are invalid.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2016346
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This patch updates OVS to use DPDK RTE_ETH namespaces.
DPDK commit 295968d17407 ("ethdev: add namespace") [0] added RTE_ETH
namespaces for ethdev enums and macros in DPDK 21.11.
As compatibility for the older names was kept and they were not officially
deprecated in DPDK 21.11, there was no impact to OVS and OVS did not have
to be updated.
In future DPDK releases the older names will be deprecated and that will
cause build warnings for OVS. They may also be removed from DPDK at some
point.
There is no immediate need to update OVS to use the new namespaces while
DPDK 21.11 is being used but at the same time there is no need to wait
until it becomes an issue either. So might as well align with the
updated names in DPDK 21.11.
[0] http://git.dpdk.org/dpdk/commit/?id=295968d1740760337e16b0d7914875c5cac52850
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
Acked-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
This commit fixes the minimum packet size for the vlan/ipv4/tcp
traffic profile, which was previously incorrectly set.
This commit also disallows any fragmented IPv4 packets from being
matched in the optimized miniflow-extract, avoiding complexity of
handling fragmented packets and using scalar fallback instead.
The DF (don't fragment) bit is now ignored, and stripped from the
resulting miniflow.
Fixes: aa85a25095 ("dpif-netdev/mfex: Add more AVX512 traffic profiles.")
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Tested-by: Kumar Amber <kumar.amber@intel.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
GHA is broken due to update to pip>=22.0. This happens because now it
stops backtracking packages on build failure making it impossible to
find working combination of versions.
We're not able to build 'hacking', because 'wheel' is not installed
at that point in time. Installing it separately to fix the issue,
so pip can find compatible versions of packages by backtracking.
Unfortunately, new version of backtracking leads to installation of
incompatible versions of flake8 and hacking. Presumably because
current versions of hacking are not compatible with flake8>=4.0
and very old hacking-0.5.4 for some reason is considered suitable
while resolving dependencies. So, we end up with flake8-4.0.1 and
hacking-0.5.4 installed. And that doesn't work. Limiting the version
of hacking to >=3.0 to have flake8-3.9.2 and hacking-3.0.0 installed
during backtracking.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Dumitru Ceara <dceara@redhat.com>
[i.maximets: 2 tags below carried from v1, that had no >=3.0 change]
Acked-by: Gaetan Rivet <grive@u256.net>
Acked-by: Aaron Conole <aconole@redhat.com>
Otherwise we hide the transaction result from the user. This may cause
problems as the user will not detect error cases. For example, if the
server refuses a transaction with "constraint violation", the user
should be notified because the transaction might need to be retried.
For clients that process database changes incrementally (using change
tracking) this lack of failure notification creates a problem if it
occurs while no other database changes happen. In that case:
- ovsdb_idl_loop_run() silently consumes the failure, initializes a
new transaction.
- no other table update was received from the server so the user will
not add anything to the new transaction.
- ovsdb_idl_loop_commit_and_wait() will "succeed" as nothing changed
from the client's perspective.
In reality, the first transaction failed and the client wasn't given
the chance to handle the failure.
Commit 0401cf5f9e ("ovsdb idl: Try committing the pending txn in
ovsdb_idl_loop_run.") tried to optimize for the common, successful
case. Maintain the same approach and optimize for transactions that
succeeded but fall back to the old mechanism of processing failures
within ovsdb_idl_loop_commit_and_wait() instead.
Fixes: 0401cf5f9e ("ovsdb idl: Try committing the pending txn in ovsdb_idl_loop_run.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When upcall/show is used to collect upcall statistics from a grafana
collector or some agent, upcall/show can be called even during ovs
restart. Occasionally ovs will crash when the revalidator thread
is not really inited. Backtrace:
(gdb) bt
- 0 upcall_unixctl_show at ofproto/ofproto-dpif-upcall.c:2885
- 1 process_command at lib/unixctl.c:308
- 2 run_connection at lib/unixctl.c:342
- 3 unixctl_server_run at lib/unixctl.c:393
- 4 main at vswitchd/ovs-v$witchd.c:140
Fixes: e79a6c833e ("ofproto: Handle flow installation and eviction in upcall.")
Signed-off-by: Wan Junjie <wanjunjie@bytedance.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ubuntu Xenial 16.04 is using GCC 5.4 and it does not support
target "-mavx512vpopcntdq" and cuases error
lib/dpif-netdev-lookup-avx512-gather.c:356:47:
error: attribute(target("avx512vpopcntdq")) is unknown
GCC 7+ supports vpopcntdq:
https://gcc.gnu.org/gcc-7/changes.html
The patch detects vpopcntdq and disables AVX512 when not found.
Fixes: 1e31489134 ("dpcls-avx512: Enable avx512 vector popcount instruction.")
Reported-by: Greg Rose <gvrose8192@gmail.com>
Signed-off-by: William Tu <u9012063@gmail.com>
Tested-by: Greg Rose <gvrose8192@gmail.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Acked-by: Cian Ferriter <cian.ferriter@intel.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
This patch fixes the thread mode part, as the static
thread-to-txq mapping selection depends on whether the
number of queues is strictly greater than the number of
PMD threads, and not greater or equal.
The section is also reworded as per Ilya's suggestion.
Fixes: c18e707b2f ("dpif-netdev: Introduce hash-based Tx packet steering mode.")
Reported-by: Kevin Traynor <ktraynor@redhat.com>
Reported-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Found by AddressSanitizer when running OVN tests:
Direct leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x498fb2 in calloc (/ic/ovn-ic+0x498fb2)
#1 0x5f681e in xcalloc__ ovs/lib/util.c:121:31
#2 0x5f681e in xzalloc__ ovs/lib/util.c:131:12
#3 0x5f681e in xzalloc ovs/lib/util.c:165:12
#4 0x5e3697 in ovsdb_idl_txn_add_map_op ovs/lib/ovsdb-idl.c:4057:29
#5 0x4d3f25 in update_isb_pb_external_ids ic/ovn-ic.c:576:5
#6 0x4cc4cc in create_isb_pb ic/ovn-ic.c:716:5
#7 0x4cc4cc in port_binding_run ic/ovn-ic.c:803:21
#8 0x4cc4cc in ovn_db_run ic/ovn-ic.c:1700:5
#9 0x4c9c1c in main ic/ovn-ic.c:1984:17
#10 0x7f9ad9f4a0b2 in __libc_start_main
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Added PT_MPLS_MC support in function xlate_generic_decap_action to fix
packet drops when decap action is applied on packets with packet_type
(ns=1,type=0x8848).
Fixes: 1917ace893 ("Encap & Decap actions for MPLS packet type.")
Signed-off-by: Martin Varghese <martin.varghese@nokia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When reconnecting, if there are condition changes already sent to the
server but not yet acked, reset the db's 'last-id', esentially clearing
the local cache after reconnect.
This is needed because the client cannot easily differentiate between
the following cases:
a. either the server already processed the requested monitor
condition change but the FSM was restarted before the
client was notified. In this case the client should
clear its local cache because it's out of sync with the
monitor view on the server side.
b. OR the server hasn't processed the requested monitor
condition change yet.
Fixes: 46d44cf3be ("python: idl: Add monitor_cond_since support.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When reconnecting, if there are condition changes already sent to the
server but not yet acked, reset the db's 'last-id', esentially clearing
the local cache after reconnect.
This is needed because the client cannot easily differentiate between
the following cases:
a. either the server already processed the requested monitor
condition change but the FSM was restarted before the
client was notified. In this case the client should
clear its local cache because it's out of sync with the
monitor view on the server side.
b. OR the server hasn't processed the requested monitor
condition change yet.
Conditions changing at the same time with a reconnection happening are
rare so the performance impact of this patch should be minimal.
Also, the tests are updated to cover the fact that we cannot control
which of the two scenarios ("a" and "b" above) are hit during the test.
Reported-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When reconnecting forget about in-flight monitor condition changes
if the user requested a newer condition already.
This matches the C implementation, in ovsdb_cs_db_sync_condition().
Fixes: 46d44cf3be ("python: idl: Add monitor_cond_since support.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-By: Terry Wilson <twilson@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Since we're testing serialization, it also makes sense to test
the opposite operation. Should be useful in the future for
exploring possible optimizations.
CMD: $ ./tests/ovstest json-string-benchmark
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
Currently, if a flow reply results in a message which exceeds
the maximum reply size, it will assert OVS. This would happen
when OVN uses OpenFlow15 to add large flows, and they get read
using OpenFlow10 with ovs-ofctl.
This patch prevents this and adds a test case to make sure the
code behaves as expected.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When testing if an established connection is picked up, it could be
useful to verify that the protocol state matches the expectation, that
is, it moves to ESTABLISHED, as there's a chance that code modifications
may break the TCP conn_update() in a way that it returns CT_UPDATE_VALID
without moving to the correct state leading to a false positive.
Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Gaetan Rivet <grive@u256.net>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
If a single transaction exceeds the size of the whole database (e.g.,
a lot of rows got removed and new ones added), transaction history will
be drained. This leads to sending UUID_ZERO to the clients as the last
transaction id in the next monitor update, because monitor doesn't
know what was the actual last transaction id. In case of a re-connect
that will cause re-downloading of the whole database, since the
client's last_id will be out of sync.
One solution would be to store the last transaction ID separately
from the actual transactions, but that will require a careful
management in cases where database gets reset and the history needs
to be cleared. Keeping the one last transaction instead to avoid
the problem. That should not be a big concern in terms of memory
consumption, because this last transaction will be removed from the
history once the next transaction appeared. This is also not a concern
for a fast re-sync, because this last transaction will not be used
for the monitor reply; it's either client already has it, so no need
to send, or it's a history miss.
The test updated to not check the number of atoms if there is only
one transaction in the history.
Fixes: 317b1bfd7d ("ovsdb: Don't let transaction history grow larger than the database.")
Reported-at: https://bugzilla.redhat.com/2044621
Acked-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Current code doesn't use the last id received in the monitor reply.
That may result in re-downloading the database content if the
re-connection happened after receiving the initial monitor reply,
but before receiving any other database updates.
Fixes: 1c337c43ac ("ovsdb-idl: Break into two layers.")
Reported-at: https://bugzilla.redhat.com/2044624
Acked-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
For patch ports, the is_last_action value is not propagated and is
always set to true. This causes non-reversible actions to modify the
packet, and the original content is not preserved when processing
the remaining actions.
This patch propagates the is_last_action flag for patch port related
actions. In addition, it also fixes a general last action propagation
to the individual actions.
Fixed check_pkt_larger as last action, as it is a valid case for the
drop action, so it should not be skipped.
Fixes: feee58b95 ("ofproto-dpif-xlate: Keep track of the last action")
Fixes: 5b34f8fc3 ("Add a new OVS action check_pkt_larger")
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
The experimantal typo got copy/paste a few times.
Fixes: be56e063d0 ("netdev-offload-dpdk: Support tunnel pop action.")
Fixes: e098c2f966 ("netdev-dpdk-offload: Add vxlan pattern matching function.")
Fixes: 7617d0583c ("netdev-offload-dpdk: Add support for matching on gre fields.")
Signed-off-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Some warning logs must be waived when using the net/pcap DPDK driver.
Those logs can affect different DPDK drivers (like mlx5) and the tests in
system-dpdk are not testing MTU and Rx checksum, we might as well ignore
those warnings from OVS.
Fixes: d446dcb7e0 ("system-dpdk: Refactor common logs matching.")
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Cian Ferriter <cian.ferriter@intel.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
CT marks which are loaded in non-first commit will be lost in ovs-windows.In linux OVS,
the CT mark setting with same flow could be set successfully.
Currenlty Ovs-windows will create one new CT with the flowKey(Extracted from the packet itself)
If the packet is already done DNAT action after the 1st round flow processing. So the ct-mark
Set on previous Conntrack will be lost.In the fix, it will make use of CT tuple src/dst address
stored in the flowKey if the value is not zero and zone in the flowKey is same as the input zone.
In the fix, it is also to adjust function OvsProcessDeferredActions to make it clear.
//DNAT flow
cookie=0x1040000000000, duration=950.326s, table=EndpointDNAT, n_packets=0, n_bytes=0, priority=200,tcp,reg3=0xc0a8fa2b,reg4=0x20050/0x7ffff
actions=ct(commit,table=AntreaPolicyEgressRule,zone=65520,nat(dst=192.168.250.43:80),exec(load:0x1->NXM_NX_CT_MARK[2])
// Append ct_mark flow
cookie=0x1000000000000, duration=11980.701s, table=SNATConntrackCommit, n_packets=6, n_bytes=396, priority=200,ct_state=+new+trk,ip,reg0=0x2
00/0x200,reg4=0/0xc00000 actions=load:0x3->NXM_NX_REG4[22..23],ct(commit,table=SNATConntrackCommit,zone=65520,exec(load:0x1->NXM_NX_CT_MARK[4
],load:0x1->NXM_NX_CT_MARK[5]))
// SNAT flow
cookie=0x1000000000000, duration=11980.701s, table=SNATConntrackCommit, n_packets=6, n_bytes=396, priority=200,ct_state=+new+trk,ip,reg0=0x6
00/0x600,reg4=0xc00000/0xc00000 actions=ct(commit,table=L2Forwarding,zone=65521,nat(src=192.168.250.1),exec(load:0x1->NXM_NX_CT_MARK[2]))
Reported-at:https://github.com/openvswitch/ovs-issues/issues/237
Signed-off-by: Wilson Peng <pweisong@vmware.com>
Signed-off-by: Alin-Gabriel Serdean <aserdean@ovn.org>
Read the user configuration in the netdev-offload module to modify the
number of threads used to manage hardware offload requests.
This allows processing insertion, deletion and modification
concurrently.
The offload thread structure was modified to contain all needed
elements. This structure is multiplied by the number of requested
threads and used separately.
Signed-off-by: Gaetan Rivet <grive@u256.net>
Reviewed-by: Eli Britstein <elibr@nvidia.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
The port mutex protects the netdev mapping, that can be changed by port
addition or port deletion. HW offloads operations can be considered read
operations on the port mapping itself. Use a rwlock to differentiate
between read and write operations, allowing concurrent queries and
offload insertions.
Because offload queries, deletion, and reconfigure_datapath() calls are
all rdlock, the deadlock fixed by [1] is still avoided, as the rdlock
side is recursive as prescribed by the POSIX standard. Executing
'reconfigure_datapath()' only requires a rdlock taken, but it is sometimes
executed in contexts where wrlock is taken ('do_add_port()' and
'do_del_port()').
This means that the deadlock described in [2] is still valid and should
be mitigated. The rdlock is taken using 'tryrdlock()' during offload query,
keeping the current behavior.
[1]: 81e89d5c26 ("dpif-netdev: Make datapath port mutex recursive.")
[2]: 12d0edd75e ("dpif-netdev: Avoid deadlock with offloading during PMD
thread deletion.").
Signed-off-by: Gaetan Rivet <grive@u256.net>
Reviewed-by: Eli Britstein <elibr@nvidia.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
In later commits hardware offloads are managed in several threads.
Each offload is managed by a thread determined by its flow's 'mega_ufid'.
As megaflow to mark and mark to flow mappings are 1:1 and 1:N
respectively, then a single mark exists for a single 'mega_ufid', and
multiple flows uses the same 'mega_ufid'. Because the managing thread will
be chosen using the 'mega_ufid', then each mapping does not need to be
shared with other offload threads.
The mappings are kept as cmap as upcalls will sometimes query them before
enqueuing orders to the offload threads.
To prepare this change, move the mappings within the offload thread
structure.
Signed-off-by: Gaetan Rivet <grive@u256.net>
Reviewed-by: Eli Britstein <elibr@nvidia.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
The dataplane threads (PMDs) send offloading commands to a dedicated
offload management thread. The current implementation uses a lock
and benchmarks show a high contention on the queue in some cases.
With high-contention, the mutex will more often lead to the locking
thread yielding in wait, using a syscall. This should be avoided in
a userland dataplane.
The mpsc-queue can be used instead. It uses less cycles and has
lower latency. Benchmarks show better behavior as multiple
revalidators and one or multiple PMDs writes to a single queue
while another thread polls it.
One trade-off with the new scheme however is to be forced to poll
the queue from the offload thread. Without mutex, a cond_wait
cannot be used for signaling. The offload thread is implementing
an exponential backoff and will sleep in short increments when no
data is available. This makes the thread yield, at the price of
some latency to manage offloads after an inactivity period.
Signed-off-by: Gaetan Rivet <grive@u256.net>
Reviewed-by: Eli Britstein <elibr@nvidia.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
