Dumitru Ceara [Tue, 10 Nov 2020 14:34:28 +0000 (15:34 +0100)]
ovsdb-idl: Return correct seqno from ovsdb_idl_db_set_condition().
If an IDL client sets the same monitor condition twice, the expected
seqno when the IDL contents are updated should be the same for both
calls.
In the following scenario:
1. Client calls ovsdb_idl_db_set_condition(db, table, cond1)
2. ovsdb_idl sends monitor_cond_change(cond1) but the server doesn't yet
reply.
3. Client calls ovsdb_idl_db_set_condition(db, table, cond1)
At step 3 the returned expected seqno should be the same as at step 1.
Similarly, if step 2 is skipped, i.e., the client calls sets
the condition twice in the same iteration, then both
ovsdb_idl_db_set_condition() calls should return the same value.
Mark Gray [Tue, 20 Oct 2020 15:07:07 +0000 (11:07 -0400)]
ovsdb-idl: Fix *_is_new() IDL functions.
Currently all functions of the type *_is_new() always return
'false'. This patch resolves this issue by using the
'OVSDB_IDL_CHANGE_INSERT' 'change_seqno' instead of the
'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno' to determine if a row
is new and by resetting the 'OVSDB_IDL_CHANGE_INSERT'
'change_seqno' on clear.
Further to this, the code is also updated to match the following
behaviour:
When a row is inserted, the 'OVSDB_IDL_CHANGE_INSERT'
'change_seqno' is updated to match the new database
change_seqno. The 'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno'
is not set for inserted rows (only for updated rows).
At the end of a run, ovsdb_idl_db_track_clear() should be
called to clear all tracking information, this includes
resetting all row 'change_seqno' to zero. This will ensure
that subsequent runs will not see a previously 'new' row.
add_tracked_change_for_references() is updated to only
track rows that reference the current row.
Also, update unit tests in order to test the *_is_new(),
*_is_delete() functions.
Suggested-by: Dumitru Ceara <dceara@redhat.com>
Reported-at: https://bugzilla.redhat.com/1883562 Fixes: ca545a787ac0 ("ovsdb-idl.c: Increase seqno for change-tracking of table references.") Signed-off-by: Mark Gray <mark.d.gray@redhat.com> Acked-by: Han Zhou <hzhou@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Invocations of CHECK_STREAM_OPEN_BLOCK_PY was accidentally removed
during python2 to python3 conversion. So, these tests was not
checked since that time.
This change returns tests back. CHECK_STREAM_OPEN_BLOCK_PY needed
updates, so instead I refactored function for C tests to be able to
perform python tests too. Also, added test for python with IPv6.
Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Gaetan Rivet <grive@u256.net>
Greg Rose [Thu, 12 Nov 2020 23:10:39 +0000 (15:10 -0800)]
compat: Fix compile warning.
In ../compat/nf_conntrack_reasm.c nf_frags_cache_name is declared
if OVS_NF_DEFRAG6_BACKPORT is defined. However, later in the patch
it is only used if HAVE_INET_FRAGS_WITH_FRAGS_WORK is defined and
HAVE_INET_FRAGS_RND is not defined. This will cause a compile warning
about unused variables.
Fix it up by using the same defines that enable its use to decide
if it should be declared and avoid the compiler warning.
Greg Rose [Thu, 12 Nov 2020 23:10:38 +0000 (15:10 -0800)]
compat: Fix build issue on RHEL 7.7.
RHEL 7.2 introduced a KABI fixup in struct sk_buff for the name
change of l4_rxhash to l4_hash. Then patch 9ba57fc7cccc ("datapath: Add hash info to upcall") introduced a
compile error by using l4_hash and not fixing up the HAVE_L4_RXHASH
configuration flag.
Remove all references to HAVE_L4_RXHASH and always use l4_hash to
resolve the issue. This will break compilation on RHEL 7.0 and
RHEL 7.1 but dropping support for these older kernels shouldn't be
a problem.
Fixes: 9ba57fc7cccc ("datapath: Add hash info to upcall") Signed-off-by: Greg Rose <gvrose8192@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Greg Rose [Thu, 12 Nov 2020 23:10:37 +0000 (15:10 -0800)]
compat: Remove stale code.
Remove stale and unused code left over after support for kernels
older than 3.10 was removed.
Fixes: 8063e0958780 ("datapath: Drop support for kernel older than 3.10") Signed-off-by: Greg Rose <gvrose8192@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Tue, 10 Nov 2020 11:51:48 +0000 (12:51 +0100)]
netdev-dpdk: Add ability to set MAC address.
It is possible to set the MAC address of DPDK ports by calling
rte_eth_dev_default_mac_addr_set(). OvS does not actually call
this function for non-internal ports, but the implementation is
exposed to be used in a later commit.
Terry Wilson [Tue, 15 Sep 2020 21:29:06 +0000 (16:29 -0500)]
python: Don't raise an Exception on failure to connect via SSL.
With other socket types, trying to connect and failing will return
an error code, but if an SSL Stream is used, then when
check_connection_completion(sock) is called, SSL will raise an
exception that doesn't derive from socket.error which is handled.
This adds handling for SSL.SysCallError which has the same
arguments as socket.error (errno, string). A future enhancement
could be to go through SSLStream class and implement error
checking for all of the possible exceptions similar to how
lib/stream-ssl.c's interpret_ssl_error() works across the various
methods that are implemented.
Fixes: d90ed7d65ba8 ("python: Add SSL support to the python ovs client library") Signed-off-by: Terry Wilson <twilson@redhat.com> Acked-by: Thomas Neuman <thomas.neuman@nutanix.com> Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
netdev-offload-dpdk: Pass L4 proto-id to match in the L3 rte_flow_item.
The offload layer clears the L4 protocol mask in the L3 item, when the
L4 item is passed for matching, as an optimization. This can be confusing
while parsing the headers in the PMD. Also, the datapath flow specifies
this field to be matched. This optimization is best left to the PMD.
This patch restores the code to pass the L4 protocol type in L3 match.
Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Acked-by: Eli Britstein <elibr@mellanox.com> Tested-by: Emma Finn <emma.finn@intel.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
lldp: fix a buffer overflow when handling management address TLV
When a remote device was advertising a too large management address
while still respecting TLV boundaries, lldpd would crash due to a buffer
overflow. However, the buffer being a static one, this buffer overflow
is not exploitable if hardening was not disabled. This bug exists since
version 0.5.6.
Fixes: be53a5c447c3 ("auto-attach: Initial support for Auto-Attach standard") Reported-by: Jonas Rudloff <jonas.t.rudloff@gmail.com>
Reported-at: https://github.com/openvswitch/ovs/pull/335 Co-authored-by: Fabrizio D'Angelo <fdangelo@redhat.com> Signed-off-by: Fabrizio D'Angelo <fdangelo@redhat.com> Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Notably, we ensure the order and unicity of Chassis ID, Port ID and
TTL TLV. For Chassis ID and Port ID, we also ensure the maximum size
does not exceed 256.
Renat Nurgaliyev [Sun, 15 Nov 2020 14:52:38 +0000 (15:52 +0100)]
sha1: Fix algorithm for data bigger than 512 megabytes.
In modern systems, size_t is 64 bits. There is a 32 bit overflow check
in sha1_update(), which will not work correctly, because compiler will
do an automatic cast to 64 bits, since size_t type variable is in the
expression. We do want however to lose data, since this is the whole
idea of this overflow check.
Because of this, computation of SHA-1 checksum will always be incorrect
for any data, that is bigger than 512 megabytes, which in bits is the
boundary of 32 bits integer.
In practice it means that any OVSDB transaction, bigger or equal to 512
megabytes, is considered corrupt and ovsdb-server will refuse to work
with the database file. This is especially critical for OVN southbound
database, since it tends to grow rapidly.
Fixes: 5eccf359391f ("Replace SHA-1 library with one that is clearly licensed.") Signed-off-by: Renat Nurgaliyev <impleman@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Tue, 10 Nov 2020 12:03:53 +0000 (13:03 +0100)]
ovsdb-idlc: Return expected sequence number while setting conditions.
ovsdb_idl_set_condition() returns a sequence number that can be used to
check if the requested conditions are acknowledged by the server.
However, database bindings do not return this value to the user, making
it impossible to check if the conditions are accepted.
Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Mon, 19 Oct 2020 15:14:37 +0000 (17:14 +0200)]
odp-util: Fix overflow of nested netlink attributes.
Length of nested attributes must be checked before storing to the
header. If current length exceeds the maximum value parsing should
fail, otherwise the length value will be truncated leading to
corrupted netlink message and out-of-bound memory accesses:
ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6310002cc838
at pc 0x000000575470 bp 0x7ffc6c322d60 sp 0x7ffc6c322d58
READ of size 1 at 0x6310002cc838 thread T0
SCARINESS: 12 (1-byte-read-heap-buffer-overflow)
#0 0x57546f in format_generic_odp_key lib/odp-util.c:2738:39
#1 0x559e70 in check_attr_len lib/odp-util.c:3572:13
#2 0x56581a in format_odp_key_attr lib/odp-util.c:4392:9
#3 0x5563b9 in format_odp_action lib/odp-util.c:1192:9
#4 0x555d75 in format_odp_actions lib/odp-util.c:1279:13
...
Fix that by checking the length of nested netlink attributes before
updating 'nla_len' inside the header. Additionally introduced
assertion inside nl_msg_end_nested() to catch this kind of issues
before actual overflow happened.
Credit to OSS-Fuzz.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20003 Fixes: 65da723b40a5 ("odp-util: Format tunnel attributes directly from netlink.") Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Mark Gray [Wed, 11 Nov 2020 09:25:30 +0000 (04:25 -0500)]
python: set ovs.dirs variables with build system values
ovs/dirs.py should be auto-generated using the template
ovs/dirs.py.template at build time. This will set the
ovs.dirs python variables with a value specified by the
environment or, if the environment variable is not set, from
the build system.
Signed-off-by: Mark Gray <mark.d.gray@redhat.com> Acked-By: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Mark Gray [Wed, 11 Nov 2020 09:25:29 +0000 (04:25 -0500)]
Documentation: update IPsec tutorial for F32
F32 requires the "python3-openvswitch" package now. Also, the
iptables chain "IN_FedoraServer_allow" does not exist on Fedora 32.
Signed-off-by: Mark Gray <mark.d.gray@redhat.com> Acked-by: Eric Garver <eric@garver.life> Acked-by: Ian Stokes <ian.stokes@intel.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
release-process: Policy for unmaintained branches.
While only 2 branches are formally maintained (LTS and latest release),
OVS team usually provides stable releases for other branches too, at
least for branches between LTS and latest.
When transition period ends for an old LTS, we, according to
backporting-patches.rst, could stop backporting bug fixes to branches
older than new LTS. While this might be OK for an upstream project
it doesn't sound like a user-friendly policy just because it means
that we're dropping support for branches released less than a year
ago.
Below addition to the release process might make the process a bit
smoother in terms that we will not drop support for not so old branches
even after the transition period, if committers will follow the
"as far as it goes" backporting policy. And we will provide stable
releases for these branches for at least 2 years (these releases could
be less frequent than releases on LTS branches).
After 2 year period (4 releases) committers are still free to backport
fixes they think are needed on older branches, however we will likely
not provide actual releases on these branches, unless it's specially
requested and discussed.
Additionally, "4 releases" policy aligns with the DPDK LTS support
policy, i.e. we will be able to validate and release last OVS releases
with the last available DPDK LTS, e.g. OVS 2.11 last stable release
will likely be released with the 18.11 EOL release validated.
release-process: Standardize designation of new LTS releases.
Standardize that we will mark a new release as LTS every two years
to avoid situation where we have a really old LTS branch that no-one
actually uses, but we have to support and provide releases for it.
This will also make release process more predictable, so users will
be able to rely on it and plan their upgrades accordingly.
As a bonus, 2 years support cycle kind of aligns with 2 years support
cycle of DPDK LTS releases.
Still keeping a window for us to discuss and avoid marking some
particular release as LTS in case of significant issues with it.
release-process: Add transition period for LTS releases.
While LTS change happens, according to release-process.rst, we're
immediately dropping support for the old LTS and, according to
backporting-patches.rst could stop backporting bug fixes to branches
older than new LTS. While this might be OK for an upstream project
(some upstream projects like QEMU doesn't support anything at all
except the last release) it doesn't sound like a user-friendly policy.
Below addition to the release process might make the process a bit
smoother in terms that we will continue support of branches a little
bit longer even after changing current LTS, i.e. providing at least a
minimal transition period (1 release frame) for users of old LTS.
Effectively, this change means that we will support branch-2.5 until
2.15 release, i.e. we will provide the last release, if any, on
branch-2.5 somewhere around Feb 2021. (I don't actually expect many
fixes there)
2.5 release is 4.5 years old and I'm not aware of anyone who actually
uses it today. Release process documentation says that there is no
strict time period for nominating a new LTS release and that usually
it happens once in a two years. So, proposing to nominate 2.13 as
our new LTS release since it's a first release that doesn't include
OVN inside, so we will formally not have to support it in this
repository in case there are major issues that might be hard to fix.
Suggested-by: Ben Pfaff <blp@ovn.org> Acked-by: Flavio Leitner <fbl@sysclose.org> Acked-by: Ian Stokes <ian.stokes@intel.com> Acked-by: Kevin Traynor <ktraynor@redhat.com> Reviewed-by: Simon Horman <simon.horman@netronome.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
ovsdb: Remove read permission of *.db from others.
Currently, when ovsdb *.db is created by ovsdb-tool it grants read
permission to others. This may incur security concerns, for example,
IPsec Pre-shared keys are stored in ovs-vsitchd.conf.db.
This patch addresses the concerns by removing permission for others.
Reported-by: Antonin Bas <abas@vmware.com> Acked-by: Mark Gray <mark.d.gray@redhat.com> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Sun, 25 Oct 2020 00:45:05 +0000 (02:45 +0200)]
raft: Make backlog thresholds configurable.
New appctl 'cluster/set-backlog-threshold' to configure thresholds
on backlog of raft jsonrpc connections. Could be used, for example,
in some extreme conditions where size of a database expected to be
very large, i.e. comparable with default 4GB threshold.
Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Wed, 21 Oct 2020 01:32:49 +0000 (03:32 +0200)]
raft: Set threshold on backlog for raft connections.
RAFT messages could be fairly big. If something abnormal happens to
one of the servers in a cluster it may not be able to process all the
incoming messages in a timely manner. This results in jsonrpc backlog
growth on the sender's side. For example if follower gets many new
clients at once that it needs to serve, or it decides to take a
snapshot in a period of high number of database changes.
If backlog grows large enough it becomes harder and harder for follower
to process incoming raft messages, it sends outdated replies and
starts receiving snapshots and the whole raft log from the leader.
Sometimes backlog grows too high (60GB in this example):
jsonrpc|INFO|excessive sending backlog, jsonrpc: ssl:<ip>,
num of msgs: 15370, backlog: 61731060773.
In this case OS might actually decide to kill the sender to free some
memory. Anyway, It could take a lot of time for such a server to catch
up with the rest of the cluster if it has so much data to receive and
process.
Introducing backlog thresholds for jsonrpc connections.
If sending backlog will exceed particular values (500 messages or
4GB in size), connection will be dropped and re-created. This will
allow to drop all the current backlog and start over increasing
chances of cluster recovery.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1888829 Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
William Tu [Wed, 4 Nov 2020 23:16:15 +0000 (15:16 -0800)]
ovs-bugtool: Fix crash when enable --ovs.
When enabling '--ovs' or when not using '-y', ovs-bugtool crashes due to
Traceback (most recent call last):
File "/usr/local/sbin/ovs-bugtool", line 1410, in <module>
sys.exit(main())
File "/usr/local/sbin/ovs-bugtool", line 690, in main
for (k, v) in data.items():
RuntimeError: dictionary changed size during iteration
The patch fixes it by making a copy of the key and value.
VMware-BZ: #2663359 Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.") Acked-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: William Tu <u9012063@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Documentation: Fix rendering of extra repo info for RHEL 8.
In commit a82083ee3091 ("Documentation: Add extra repo info for RHEL 8")
a newline was missing to correctly generate the code block to add
codeready-builder repository.
This commit adds the missing newline to correctly generate the code block
with the RHEL 8 codeready-builder instructions.
Fixes: a82083ee3091 ("Documentation: Add extra repo info for RHEL 8") Acked-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Tue, 20 Oct 2020 16:22:25 +0000 (18:22 +0200)]
raft: Avoid having more than one snapshot in-flight.
Previous commit 8c2c503bdb0d ("raft: Avoid sending equal snapshots.")
took a "safe" approach to not send only exactly same snapshot
installation requests. However, it doesn't make much sense to send
more than one snapshot at a time. If obsolete snapshot installed,
leader will re-send the most recent one.
With this change leader will have only 1 snapshot in-flight per
connection. This will reduce backlogs on raft connections in case
new snapshot created while 'install_snapshot_request' is in progress
or if election timer changed in that period.
Also, not tracking the exact 'install_snapshot_request' we've sent
allows to simplify the code.
Ilya Maximets [Sat, 24 Oct 2020 00:25:48 +0000 (02:25 +0200)]
ovsdb-server: Reclaim heap memory after compaction.
Compaction happens at most once in 10 minutes. That is a big time
interval for a heavy loaded ovsdb-server in cluster mode.
In 10 minutes raft logs could grow up to tens of thousands of entries
with tens of gigabytes in total size.
While compaction cleans up raft log entries, the memory in many cases
is not returned to the system, but kept in the heap of running
ovsdb-server process, and it could stay in this condition for a really
long time. In the end one performance spike could lead to a fast
growth of the raft log and this memory will never (for a really long
time) be released to the system even if the database if empty.
Simple example how to reproduce with OVN sandbox:
1. make sandbox SANDBOXFLAGS='--nbdb-model=clustered --sbdb-model=clustered'
2. Run following script that creates 1 port group, adds 4000 acls and
removes all of that in the end:
# cat ../memory-test.sh
pg_name=my_port_group
export OVN_NB_DAEMON=$(ovn-nbctl --pidfile --detach --log-file -vsocket_util:off)
ovn-nbctl pg-add $pg_name
for i in $(seq 1 4000); do
echo "Iteration: $i"
ovn-nbctl --log acl-add $pg_name from-lport $i udp drop
done
ovn-nbctl acl-del $pg_name
ovn-nbctl pg-del $pg_name
ovs-appctl -t $(pwd)/sandbox/nb1 memory/show
ovn-appctl -t ovn-nbctl exit
---
3. Stopping one of Northbound DB servers:
ovs-appctl -t $(pwd)/sandbox/nb1 exit
Make sure that ovsdb-server didn't compact the database before
it was stopped. Now we have a db file on disk that contains
4000 fairly big transactions inside.
4. Trying to start same ovsdb-server with this file.
# cd sandbox && ovsdb-server <...> nb1.db
At this point ovsdb-server reads all the transactions from db
file and performs all of them as fast as it can one by one.
When it finishes this, raft log contains 4000 entries and
ovsdb-server consumes (on my system) ~13GB of memory while
database is empty. And libc will likely never return this memory
back to system, or, at least, will hold it for a really long time.
This patch adds a new command 'ovsdb-server/memory-trim-on-compaction'.
It's disabled by default, but once enabled, ovsdb-server will call
'malloc_trim(0)' after every successful compaction to try to return
unused heap memory back to system. This is glibc-specific, so we
need to detect function availability in a build time.
Disabled by default since it adds from 1% to 30% (depending on the
current state) to the snapshot creation time and, also, next memory
allocations will likely require requests to kernel and that might be
slower. Could be enabled by default later if considered broadly
beneficial.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1888829 Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Sat, 24 Oct 2020 23:08:03 +0000 (01:08 +0200)]
raft: Avoid annoying debug logs if raft is connected.
If debug logs enabled, "raft_is_connected: true" printed on every
call to raft_is_connected() which is way too frequently.
These messages are not very informative and only litters the log.
Let's log only disconnected state in a rate-limited way and only
log positive case once at the moment cluster becomes connected.
Fixes: 923f01cad678 ("raft.c: Set candidate_retrying if no leader elected since last election.") Acked-by: Han Zhou <hzhou@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Leonid Ryzhyk [Thu, 29 Nov 2018 02:41:34 +0000 (18:41 -0800)]
packets: Un-inline functions needed by DDlog.
DDlog uses these functions from Rust, but Rust can't use inline
functions (since it doesn't compile C headers but only links
against a C-compatible ABI). Thus, move the implementations
of these functions to a .c file.
I don't think any of these functions is likely to be an
important part of a "fast path" in OVS, but if that's wrong,
then we could take another approach.
Signed-off-by: Leonid Ryzhyk <lryzhyk@vmware.com> Co-authored-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <numans@ovn.org>
Jianbo Liu [Wed, 14 Oct 2020 10:31:04 +0000 (13:31 +0300)]
netdev-tc-offloads: Don't delete ufid mapping if fail to delete filter
tc_replace_flower may fail, so the return value must be checked.
If not zero, ufid can't be deleted. Otherwise the operations on this
filter may fail because its ufid is not found.
Signed-off-by: Jianbo Liu <jianbol@nvidia.com> Reviewed-by: Roi Dayan <roid@nvidia.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
David Marchand [Thu, 19 Mar 2020 07:32:40 +0000 (08:32 +0100)]
travis: Fix kernel download retry.
wget stops retrying to download a file when hitting fatal http errors
like 503.
But if a previous try had resulted in a partially downloaded ${file}, the
next wget call tries to download to ${file}.1.
Ben Pfaff [Wed, 17 Jun 2020 21:16:08 +0000 (14:16 -0700)]
Eliminate use of term "slave" in bond, LACP, and bundle contexts.
The new term is "member".
Most of these changes should not change user-visible behavior. One
place where they do is in "ovs-ofctl dump-flows", which will now output
"members:..." inside "bundle" actions instead of "slaves:...". I don't
expect this to cause real problems in most systems. The old syntax
is still supported on input for backward compatibility.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Ilya Maximets [Tue, 20 Oct 2020 09:30:59 +0000 (11:30 +0200)]
NEWS: Move terminology update to correct place.
It's Post-v2.14.0, not v2.14.0.
Fixes: 807152a4ddfb ("Use primary/secondary, not master/slave, as names for OpenFlow roles.") Acked-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Sat, 17 Oct 2020 15:21:29 +0000 (17:21 +0200)]
odp-util: Fix using uninitialized gtpu metadata.
If datapath flow doesn't have one of the fields of gtpu metadata, e.g.
'tunnel(gtpu())', uninitialized stack memory will be used instead.
==3485429==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x853a1b in format_u8x lib/odp-util.c:3474:13
#1 0x86ee9c in format_odp_tun_gtpu_opt lib/odp-util.c:3713:5
#2 0x86a099 in format_odp_tun_attr lib/odp-util.c:3973:13
#3 0x83afe6 in format_odp_key_attr__ lib/odp-util.c:4179:9
#4 0x838afb in odp_flow_format lib/odp-util.c:4563:17
#5 0x738422 in log_flow_message lib/dpif.c:1750:5
#6 0x738e2f in log_flow_put_message lib/dpif.c:1784:9
#7 0x7371a4 in dpif_operate lib/dpif.c:1377:21
#8 0x7363ef in dpif_flow_put lib/dpif.c:1035:5
#9 0xc7aab7 in dpctl_put_flow lib/dpctl.c:1171:13
#10 0xc65a4f in dpctl_unixctl_handler lib/dpctl.c:2701:17
#11 0xaaad04 in process_command lib/unixctl.c:308:13
#12 0xaa87f7 in run_connection lib/unixctl.c:342:17
#13 0xaa842e in unixctl_server_run lib/unixctl.c:393:21
#14 0x51c09c in main vswitchd/ovs-vswitchd.c:128:9
#15 0x7f88344391a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
#16 0x46b92d in _start (vswitchd/ovs-vswitchd+0x46b92d)
Uninitialized value was stored to memory at
#0 0x87da17 in scan_gtpu_metadata lib/odp-util.c:5221:27
#1 0x874588 in parse_odp_key_mask_attr__ lib/odp-util.c:5862:9
#2 0x83ee14 in parse_odp_key_mask_attr lib/odp-util.c:5808:18
#3 0x83e8b5 in odp_flow_from_string lib/odp-util.c:6065:18
#4 0xc7a4f3 in dpctl_put_flow lib/dpctl.c:1145:13
#5 0xc65a4f in dpctl_unixctl_handler lib/dpctl.c:2701:17
#6 0xaaad04 in process_command lib/unixctl.c:308:13
#7 0xaa87f7 in run_connection lib/unixctl.c:342:17
#8 0xaa842e in unixctl_server_run lib/unixctl.c:393:21
#9 0x51c09c in main vswitchd/ovs-vswitchd.c:128:9
#10 0x7f88344391a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
Uninitialized value was created by an allocation of 'msgtype_ma' in the
stack frame of function 'scan_gtpu_metadata'
#0 0x87d440 in scan_gtpu_metadata lib/odp-util.c:5187
Fix that by initializing fields to all zeroes by default.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=21426 Fixes: 3c6d05a02e0f ("userspace: Add GTP-U support.") Acked-by: Yi Yang <yangyi01@inspur.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Lei Wang [Thu, 30 Jul 2020 10:58:38 +0000 (10:58 +0000)]
netdev-offload-dpdk: Support vxlan encap offload with load actions.
Struct match has the tunnel values/masks in
match->flow.tunnel/match->wc.masks.tunnel.
Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields,
but those should not be used for matching.
Offloading fails if masks is not clear. Clear it if no tunnel used.
Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow") Reviewed-by: Eli Britstein <elibr@mellanox.com> Reviewed-by: Gaetan Rivet <gaetanr@mellanox.com> Acked-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Tested-by: Emma Finn <emma.finn@intel.com> Signed-off-by: Lei Wang <leiw@mellanox.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Wed, 14 Oct 2020 16:13:46 +0000 (18:13 +0200)]
ofp-ed-props: Fix using uninitialized padding for NSH encap actions.
OVS uses memcmp to compare actions of existing and new flows, but
'struct ofp_ed_prop_nsh_md_type' and corresponding ofpact structure has
3 bytes of padding that never initialized and passed around within OF
data structures and messages.
Uninitialized bytes in MemcmpInterceptorCommon
at offset 21 inside [0x7090000003f8, 136)
WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x4a184e in bcmp (vswitchd/ovs-vswitchd+0x4a184e)
#1 0x896c8a in ofpacts_equal lib/ofp-actions.c:9121:31
#2 0x564403 in replace_rule_finish ofproto/ofproto.c:5650:37
#3 0x563462 in add_flow_finish ofproto/ofproto.c:5218:13
#4 0x54a1ff in ofproto_flow_mod_finish ofproto/ofproto.c:8091:17
#5 0x5433b2 in handle_flow_mod__ ofproto/ofproto.c:6216:17
#6 0x56a2fc in handle_flow_mod ofproto/ofproto.c:6190:17
#7 0x565bda in handle_single_part_openflow ofproto/ofproto.c:8504:16
#8 0x540b25 in handle_openflow ofproto/ofproto.c:8685:21
#9 0x6697fd in ofconn_run ofproto/connmgr.c:1329:13
#10 0x668e6e in connmgr_run ofproto/connmgr.c:356:9
#11 0x53f1bc in ofproto_run ofproto/ofproto.c:1890:5
#12 0x4ead0c in bridge_run__ vswitchd/bridge.c:3250:9
#13 0x4e9bc8 in bridge_run vswitchd/bridge.c:3309:5
#14 0x51c072 in main vswitchd/ovs-vswitchd.c:127:9
#15 0x7f23a99011a2 in __libc_start_main (/lib64/libc.so.6)
#16 0x46b92d in _start (vswitchd/ovs-vswitchd+0x46b92d)
Uninitialized value was stored to memory at
#0 0x4745aa in __msan_memcpy.part.0 (vswitchd/ovs-vswitchd)
#1 0x54529f in rule_actions_create ofproto/ofproto.c:3134:5
#2 0x54915e in ofproto_rule_create ofproto/ofproto.c:5284:11
#3 0x55d419 in add_flow_init ofproto/ofproto.c:5123:17
#4 0x54841f in ofproto_flow_mod_init ofproto/ofproto.c:7987:17
#5 0x543250 in handle_flow_mod__ ofproto/ofproto.c:6206:13
#6 0x56a2fc in handle_flow_mod ofproto/ofproto.c:6190:17
#7 0x565bda in handle_single_part_openflow ofproto/ofproto.c:8504:16
#8 0x540b25 in handle_openflow ofproto/ofproto.c:8685:21
#9 0x6697fd in ofconn_run ofproto/connmgr.c:1329:13
#10 0x668e6e in connmgr_run ofproto/connmgr.c:356:9
#11 0x53f1bc in ofproto_run ofproto/ofproto.c:1890:5
#12 0x4ead0c in bridge_run__ vswitchd/bridge.c:3250:9
#13 0x4e9bc8 in bridge_run vswitchd/bridge.c:3309:5
#14 0x51c072 in main vswitchd/ovs-vswitchd.c:127:9
#15 0x7f23a99011a2 in __libc_start_main (/lib64/libc.so.6)
Uninitialized value was created by an allocation of 'ofpacts_stub'
in the stack frame of function 'handle_flow_mod'
#0 0x569e80 in handle_flow_mod ofproto/ofproto.c:6170
This could cause issues with flow modifications or other operations.
To reproduce, some NSH tests could be run under valgrind or clang
MemorySantizer. Ex. "nsh - md1 encap over a veth link" test.
Fix that by clearing padding bytes while encoding and decoding.
OVS will still accept OF messages with non-zero padding from
controllers.
New tests added to tests/ofp-actions.at.
Fixes: 1fc11c5948cf ("Generic encap and decap support for NSH") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Jan Scheurich <jan.scheurich@ericsson.com>
Ilya Maximets [Tue, 13 Oct 2020 10:02:10 +0000 (12:02 +0200)]
bond: Fix using uninitialized 'lacp_fallback_ab_cfg' for 'bond-primary'.
's->lacp_fallback_ab_cfg' initialized down below in the code, so
we're using it uninitialized to detect if we need to get 'bond-primary'
configuration.
Found by valgrind:
Conditional jump or move depends on uninitialised value(s)
at 0x409114: port_configure_bond (bridge.c:4569)
by 0x409114: port_configure (bridge.c:1284)
by 0x40F6E6: bridge_reconfigure (bridge.c:917)
by 0x411425: bridge_run (bridge.c:3330)
by 0x406D84: main (ovs-vswitchd.c:127)
Uninitialised value was created by a stack allocation
at 0x408C53: port_configure (bridge.c:1190)
Fix that by moving this code to the point where 'lacp_fallback_ab_cfg'
already initialized. Additionally clarified behavior of 'bond-primary'
in manpages for the fallback to AB case.
Fixes: b4e50218a0f8 ("bond: Add 'primary' interface concept for active-backup mode.") Acked-by: Jeff Squyres <jsquyres@cisco.com> Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
net: openvswitch: use hlist_for_each_entry_rcu instead of hlist_for_each_entry
The struct sw_flow is protected by RCU, when traversing them,
use hlist_for_each_entry_rcu.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Compat fixup - OVS doesn't support lockdep_ovsl_is_held() yet
openvswitch: Distribute switch variables for initialization
Variables declared in a switch statement before any case statements
cannot be automatically initialized with compiler instrumentation (as
they are not part of any execution flow). With GCC's proposed automatic
stack variable initialization feature, this triggers a warning (and they
don't get initialized). Clang's automatic stack variable initialization
(via CONFIG_INIT_STACK_ALL=y) doesn't throw a warning, but it also
doesn't initialize such variables[1]. Note that these warnings (or silent
skipping) happen before the dead-store elimination optimization phase,
so even when the automatic initializations are later elided in favor of
direct initializations, the warnings remain.
To avoid these problems, move such variables into the "case" where
they're used or lift them up into the main function body.
net/openvswitch/flow_netlink.c: In function ‘validate_set’:
net/openvswitch/flow_netlink.c:2711:29: warning: statement will never be executed [-Wswitch-unreachable]
2711 | const struct ovs_key_ipv4 *ipv4_key;
| ^~~~~~~~
[1] https://bugs.llvm.org/show_bug.cgi?id=44916
Signed-off-by: Kees Cook <keescook@chromium.org> Signed-off-by: David S. Miller <davem@davemloft.net> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
net: openvswitch: use skb_list_walk_safe helper for gso segments
This is a straight-forward conversion case for the new function, keeping
the flow of the existing code as intact as possible.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Signed-off-by: David S. Miller <davem@davemloft.net> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
the openvswitch module shares a common conntrack and nat infrastructure
exposed via netfilter. it's possible that a packet needs both snat and
dnat manipulation, due to e.g. tuple collision. netfilter can support
this because it runs through the nat table twice - once on ingress and
again after egress. the openvswitch module doesn't have such capability.
like netfilter hook infrastructure, we should run through nat twice to
keep the symmetry.
fixes: 05752523e565 ("openvswitch: interface with nat.")
signed-off-by: aaron conole <aconole@redhat.com>
signed-off-by: david s. miller <davem@davemloft.net>
If we can't build the flow del notification, we can simply delete
the flow, no need to crash the kernel. Still keep a WARN_ON to
preserve debuggability.
Note: the BUG_ON() predates the Fixes tag, but this change
can be applied only after the mentioned commit.
v1 -> v2:
- do not leak an skb on error
Fixes: aed067783e50 ("openvswitch: Minimize ovs_flow_cmd_del critical section.") Signed-off-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info()
All the callers of ovs_flow_cmd_build_info() already deal with
error return code correctly, so we can handle the error condition
in a more gracefull way. Still dump a warning to preserve
debuggability.
v1 -> v2:
- clarify the commit message
- clean the skb and report the error (DaveM)
Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.") Signed-off-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When user-space sets the OVS_UFID_F_OMIT_* flags, and the relevant
flow has no UFID, we can exceed the computed size, as
ovs_nla_put_identifier() will always dump an OVS_FLOW_ATTR_KEY
attribute.
Take the above in account when computing the flow command message
size.
Fixes: 74ed7ab9264c ("openvswitch: Add support for unique flow IDs.") Reported-by: Qi Jun Ding <qding@redhat.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
nla_total_size returns the total length of attribute
including padding.
Cc: Joe Stringer <joe@ovn.org> Cc: William Tu <u9012063@gmail.com> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
The commit 69c51582ff786 ("dpif-netlink: don't allocate per
thread netlink sockets"), in Open vSwitch ovs-vswitchd, has
changed the number of allocated sockets to just one per port
by moving the socket array from a per handler structure to
a per datapath one. In the kernel datapath, a vport will have
only one socket in most case, if so select it directly in
fast-path.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
net: openvswitch: fix possible memleak on destroy flow-table
When we destroy the flow tables which may contain the flow_mask,
so release the flow mask struct.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net>
Added additional compat layer fixup for WRITE_ONCE()
The most case *index < ma->max, and flow-mask is not NULL.
We add un/likely for performance.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Acked-by: William Tu <u9012063@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Simplify the code and remove the unnecessary BUILD_BUG_ON.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Acked-by: William Tu <u9012063@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
The full looking up on flow table traverses all mask array.
If mask-array is too large, the number of invalid flow-mask
increase, performance will be drop.
One bad case, for example: M means flow-mask is valid and NULL
of flow-mask means deleted.
In that case, without this patch, openvswitch will traverses all
mask array, because there will be one flow-mask in the tail. This
patch changes the way of flow-mask inserting and deleting, and the
mask array will be keep as below: there is not a NULL hole. In the
fast path, we can "break" "for" (not "continue") in flow_lookup
when we get a NULL flow-mask.
"break"
v
+-------------------------------------------+
| M | M | NULL |... | NULL | NULL|
+-------------------------------------------+
This patch don't optimize slow or control path, still using ma->max
to traverse. Slow path:
* tbl_mask_array_realloc
* ovs_flow_tbl_lookup_exact
* flow_mask_find
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
net: openvswitch: don't unlock mutex when changing the user_features fails
Unlocking of a not locked mutex is not allowed.
Other kernel thread may be in critical section while
we unlock it because of setting user_feature fail.
Fixes: 95a7233c4 ("net: openvswitch: Set OvS recirc_id from tc chain index") Cc: Paul Blakey <paulb@mellanox.com> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Acked-by: William Tu <u9012063@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Reviewed-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
In rtnl_net_notifyid(), we certainly can't pass a null GFP flag to
rtnl_notify(). A GFP_KERNEL flag would be fine in most circumstances,
but there are a few paths calling rtnl_net_notifyid() from atomic
context or from RCU critical sections. The later also precludes the use
of gfp_any() as it wouldn't detect the RCU case. Also, the nlmsg_new()
call is wrong too, as it uses GFP_KERNEL unconditionally.
Therefore, we need to pass the GFP flags as parameter and propagate it
through function calls until the proper flags can be determined.
In most cases, GFP_KERNEL is fine. The exceptions are:
* openvswitch: ovs_vport_cmd_get() and ovs_vport_cmd_dump()
indirectly call rtnl_net_notifyid() from RCU critical section,
* rtnetlink: rtmsg_ifinfo_build_skb() already receives GFP flags as
parameter.
Also, in ovs_vport_cmd_build_info(), let's change the GFP flags used
by nlmsg_new(). The function is allowed to sleep, so better make the
flags consistent with the ones used in the following
ovs_vport_cmd_fill_info() call.
Found by code inspection.
Fixes: 9a9634545c70 ("netns: notify netns id events") Signed-off-by: Guillaume Nault <gnault@redhat.com> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net>
Backport the datapath.c portion of this fix.
$ tc filter add dev dev1 ingress \
prio 1 chain 0 proto ip \
flower tcp ct_state -trk \
action ct pipe \
action goto chain 2
Received packets will first travel though tc, and if they aren't stolen
by it, like in the above rule, they will continue to OvS datapath.
Since we already did some actions (action ct in this case) which might
modify the packets, and updated action stats, we would like to continue
the proccessing with the correct recirc_id in OvS (here recirc_id(2))
where we left off.
To support this, introduce a new skb extension for tc, which
will be used for translating tc chain to ovs recirc_id to
handle these miss cases. Last tc chain index will be set
by tc goto chain action and read by OvS datapath.
Signed-off-by: Paul Blakey <paulb@mellanox.com> Signed-off-by: Vlad Buslov <vladbu@mellanox.com> Acked-by: Jiri Pirko <jiri@mellanox.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net>
Backport the local datapath changes from this patch and add compat
layer fixup for the DECLARE_STATIC_KEY_FALSE macro.
openvswitch: Print error when ovs_execute_actions() fails
Currently in function ovs_dp_process_packet(), return values of
ovs_execute_actions() are silently discarded. This patch prints out
an debug message when error happens so as to provide helpful hints
for debugging. Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
net: openvswitch: do not update max_headroom if new headroom is equal to old headroom
When a vport is deleted, the maximum headroom size would be changed.
If the vport which has the largest headroom is deleted,
the new max_headroom would be set.
But, if the new headroom size is equal to the old headroom size,
updating routine is unnecessary.
Signed-off-by: Taehee Yoo <ap420073@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Mon, 5 Oct 2020 10:09:55 +0000 (12:09 +0200)]
dpctl: Fix broken flow deletion via ovs-dpctl due to missing ufid.
Current code generates UFID for flows installed by ovs-dpctl. This
leads to inability to remove such flows by the same command. Ex:
ovs-dpctl add-dp test
ovs-dpctl add-if test vport0
ovs-dpctl add-flow test "in_port(0),eth(),eth_type(0x800),ipv4(src=100.1.0.1)" 0
ovs-dpctl del-flow test "in_port(0),eth(),eth_type(0x800),ipv4(src=100.1.0.1)"
dpif|WARN|system@test: failed to flow_del (No such file or directory)
ufid:e4457189-3990-4a01-bdcf-1e5f8b208711 in_port(0),
eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),eth_type(0x0800),
ipv4(src=100.1.0.1,dst=0.0.0.0,proto=0,tos=0,ttl=0,frag=no)
ovs-dpctl: deleting flow (No such file or directory)
Perhaps you need to specify a UFID?
During del-flow operation UFID is generated too, however resulted
value is different from one generated during add-flow. This happens
because odp_flow_key_hash() function uses random base value for flow
hashes which is different on every invocation. That is not an issue
while running 'ovs-appctl dpctl/{add,del}-flow' because execution
of these requests happens in context of the OVS main process, i.e.
there will be same random seed.
Commit e61984e781e6 was intended to allow offloading for flows
added by dpctl/add-flow unixctl command, so it's better to generate
UFIDs conditionally inside dpctl command handler only for appctl
invocations. Offloading is not possible from ovs-dpctl utility anyway.
There are still couple of corner case: It will not be possible to
remove flow by 'ovs-appctl dpctl/del-flow' without specifying UFID if
main OVS process was restarted since flow addition and it will not
be possible to remove flow by ovs-dpctl without specifying UUID if
it was added by 'ovs-appctl dpctl/add-flow'. But these scenarios
seems minor since these commands intended for testing only.
Ilya Maximets [Wed, 7 Oct 2020 13:23:49 +0000 (15:23 +0200)]
travis: Disable check for array of flexible structures in sparse.
Sparse introduced new checks for flexible arrays and there is a
false-positive in netdev-linux implementation right now that can not
be easily fixed. Patch sent to sparse to fix it, but we need to
disable the check for now to unblock our CI.
lib/netdev-linux.c:1238:19: error: array of flexible structures
The issue is with the following code:
union {
struct cmsghdr cmsg;
char buffer[CMSG_SPACE(sizeof(struct tpacket_auxdata))];
} cmsg_buffers[NETDEV_MAX_BURST];
'struct cmsghdr' contains a flexible array. But this union is a way
to ensure correct alignment of 'buffer', suggested by CMSG manpage.
Ilya Maximets [Thu, 8 Oct 2020 13:53:43 +0000 (15:53 +0200)]
datapath: Fix exposing OVS_TUNNEL_KEY_ATTR_GTPU_OPTS to kernel module.
Kernel module doesn't know about GTPU and it should return correct
out-of-range error in case this tunnel attribute passed there for
any reason. Current out-of-tree module will pass the range check
and will try to access ovs_tunnel_key_lens[] array by index
OVS_TUNNEL_KEY_ATTR_GTPU_OPTS. Even though it might not produce
issues in current code, this is not a good thing to do since
ovs_tunnel_key_lens[] array is not explicitly initialized for
OVS_TUNNEL_KEY_ATTR_GTPU_OPTS and we will likely have misleading
error about incorrect attribute length in the end.
Ted Elhourani [Tue, 6 Oct 2020 20:25:56 +0000 (20:25 +0000)]
dns-resolve: Allow unbound's config file to be set through an env var.
When an unbound context is created, check whether OVS_UNBOUND_CONF has been
set. If a valid config file is supplied then use it to configure the
context. The procedure returns if the config file is invalid. If no config
file is found then the default unbound config is used.
ofproto-dpif-upcall: Log the emergency flow flush.
When the number of flows in the datapath reaches twice the
maximum, revalidators will delete all flows as an emergency
action to recover. In that case, log a message with values
and increase a coverage counter.
ovsdb-idl.at: Queue for termination all OVSDB IDL pids.
When running OVSDB cluster tests on Windows not all the ovsdb processes
are terminated. Queue up the pids of the started processes for
termination when the test stops.
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
system-userspace-packet-type-aware.at: Wait for ip address updates.
ovs-router module checks for the source ip address of the interface
while adding a new route. netdev module doesn't request ip addresses
from the system every time, but instead it caches currently assigned
ip addresses and updates the cache on netlink notifications if needed.
So, there is a slight delay between setting ip address on interface
in a system and a moment OVS updates list of ip addresses of this
interface. If route addition happens within this time frame, it
fails with the following error:
# ovs-appctl ovs/route/add 10.0.0.0/24 br-p1
Error while inserting route.
ovs-appctl: ovs-vswitchd: server returned an error
This makes system tests to fail frequently.
Let's wait until local route successfully added. This will mean
that OVS finished processing of a netlink event and will use up to
date list of ip addresses on desired interface.
Fixes: 526cf4e1d6a8 ("tests: Added unit tests in packet-type-aware.at") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Aaron Conole <aconole@redhat.com> Acked-by: Flavio Leitner <fbl@sysclose.org>
Tomasz Konieczny [Mon, 17 Feb 2020 11:37:36 +0000 (12:37 +0100)]
docs: Add flow control on i40e issue
There is an issue with flow control configuration on i40e devices
and it has a work around. We add this to documentation as known issue
until a permanent solution is developed.
Signed-off-by: Tomasz Konieczny <tomaszx.konieczny@intel.com> Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
projects / mirror_ovs.git / log