Greg Rose [Wed, 3 Jul 2019 17:04:54 +0000 (10:04 -0700)]
compat: Clean up gre_calc_hlen
It's proliferated throughout three .c files so let's pull them all
together in gre.h where the inline function belongs. This requires
some adjustments to the compat layer so that the various iterations
of gre_calc_hlen and ip_gre_calc_hlen since the 3.10 kernel are
handled correctly.
Signed-off-by: Greg Rose <gvrose8192@gmail.com> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
ip_gre.c and ip6_gre.c both had duplicate code for handling the tunnel
metadata destinations. Move the duplicate code over into the right
header file, dst_metadata.h.
Signed-off-by: Greg Rose <gvrose8192@gmail.com> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Bhargava Shastry [Fri, 21 Jun 2019 12:50:35 +0000 (14:50 +0200)]
ossfuzz: Remove duplicate tcp flags parsing in flow extract target
During a code audit, the flow extraction fuzzer target was seen to be
parsing tcp flags from the fuzzer supplied input twice. This is
probably a typo since the second call to `parse_tcp_flags()` is
identical to the first.
Since a call to `parse_tcp_flags()` parses the Ethernet and IP headers
contained in the packet, the second (buggy) call to `parse_tcp_flags()`
creates an expectation that there is a second set of Ethernet and IP
headers beyond the first which is incorrect. This patch fixes this
problem by removing the duplicate code in question.
Signed-off-by: Bhargava Shastry <bshas3@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Patch is mostly intended to prevent valgrind to report memory leak issues
while running unit tests. Otherwise it does not benefit anything since
the application exits immediately after freeing the memory.
Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Mon, 24 Jun 2019 14:20:17 +0000 (17:20 +0300)]
vswitchd: Always cleanup userspace datapath.
'netdev' datapath is implemented within ovs-vswitchd process and can
not exist without it, so it should be gracefully terminated with a
full cleanup of resources upon ovs-vswitchd exit.
This change forces dpif cleanup for 'netdev' datapath regardless of
passing '--cleanup' to 'ovs-appctl exit'. Such solution allowes to
not pass this additional option everytime for userspace datapath
installations and also allowes to not terminate system datapath in
setups where both datapaths runs at the same time.
The main part is that dpif_port_del() will lead to netdev_close()
and subsequent netdev_class->destroy(dev) which will stop HW NICs
and free their resources. For vhost-user interfaces it will invoke
vhost driver unregistering with a properly closed vhost-user
connection. For upcoming AF_XDP netdev this will allow to gracefully
destroy xdp sockets and unload xdp programs from linux interfaces.
Another important thing is that port deletion will also trigger
flushing of flows offloaded to HW NICs.
Exception made for 'internal' ports that could have user ip/route
configuration. These ports will not be removed without '--cleanup'.
This change fixes OVS disappearing from the DPDK point of view
(keeping HW NICs improperly configured, sudden closing of vhost-user
connections) and will help with linux devices clearing with upcoming
AF_XDP netdev support.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Tested-by: William Tu <u9012063@gmail.com> Acked-by: Flavio Leitner <fbl@sysclose.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Wed, 19 Jun 2019 08:05:38 +0000 (11:05 +0300)]
netdev-offload-tc: Fix requesting match on wildcarded vlan tpid.
'mask' must be checked first before configuring key in flower.
CC: Eli Britstein <elibr@mellanox.com> Fixes: 0b0a84783cd6 ("netdev-tc-offloads: Support match on priority tags") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Roi Dayan <roid@mellanox.com> Reviewed-by: Eli Britstein <elibr@mellanox.com>
While checking unit tests with valgrind option (make check-valgrind) I have
noticed several memory leaks of the following format:
.....
==20019== 13,883 (296 direct, 13,587 indirect) bytes in 1 blocks are definitely lost in loss record 346 of 346
==20019== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20019== by 0x530F52: xcalloc (util.c:121)
==20019== by 0x5037A1: ovsdb_idl_row_create__ (ovsdb-idl.c:3120)
==20019== by 0x5045A3: ovsdb_idl_row_create (ovsdb-idl.c:3133)
==20019== by 0x507240: ovsdb_idl_process_update2 (ovsdb-idl.c:2478)
==20019== by 0x507240: ovsdb_idl_db_parse_update__ (ovsdb-idl.c:2328)
==20019== by 0x507240: ovsdb_idl_db_parse_update (ovsdb-idl.c:2380)
==20019== by 0x508128: ovsdb_idl_process_response (ovsdb-idl.c:742)
==20019== by 0x508128: ovsdb_idl_process_msg (ovsdb-idl.c:831)
==20019== by 0x508128: ovsdb_idl_run (ovsdb-idl.c:915)
==20019== by 0x4106D9: bridge_run (bridge.c:2977)
==20019== by 0x40719C: main (ovs-vswitchd.c:127)
==20019==
==20019== LEAK SUMMARY:
==20019== definitely lost: 296 bytes in 1 blocks
==20019== indirectly lost: 13,587 bytes in 10 blocks
==20019== possibly lost: 0 bytes in 0 blocks
==20019== still reachable: 43,563 bytes in 440 blocks
==20019== suppressed: 288 bytes in 1 blocks
....
The problem is that table records maintained by database which is going to
be destroyed with ovsdb_idl_db_destroy() function are not destroyed.
Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
==25513==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
==25513==The signal is caused by a READ memory access.
==25513==Hint: address points to the zero page.
#0 0x7ff7582aa0a9 in __GI___strlen_sse2
#1 0x7ff759bdde81 (/lib64/libasan.so.5+0xace81)
#2 0x7ff759479932 (/lib64/libcrypto.so.1.1+0xb3932)
#3 0x7ff759473c5a in BIO_ctrl (/lib64/libcrypto.so.1.1+0xadc5a)
#4 0x7ff7598decc1 in SSL_CTX_use_certificate_file (/lib64/libssl.so.1.1+0x40cc1)
#5 0x4dbaa7 in stream_ssl_set_certificate_file__ lib/stream-ssl.c:1170
#6 0x4dca2e in stream_ssl_set_key_and_cert lib/stream-ssl.c:1216
#7 0x4146b2 in reconfigure_ssl ovsdb/ovsdb-server.c:1254
#8 0x409c83 in main ovsdb/ovsdb-server.c:368
#9 0x7ff758233812 in __libc_start_main
#10 0x40f6bd in _start (ovsdb-server+0x40f6bd)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0x9a0a9) in __GI___strlen_sse2
==25513==ABORTING
Another way to reproduce is to use non-initialized DB entry for
private-key and a file for certificate in ovsdb-server cmdline.
The root cause is that stream_ssl_set_key_and_cert() triggers
configuration for both key and cert if any of them is valid, keeping
it possible for one of them to be NULL.
Fixes: 6f1e91b1d7c0 ("stream-ssl: Make changing keys and certificate at runtime reliable.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ben Pfaff <blp@ovn.org>
David Marchand [Thu, 27 Jun 2019 09:43:36 +0000 (11:43 +0200)]
netdev-dpdk: Reset queue number for vhost devices on vm shutdown.
Rather than poll all disabled queues and waste some memory for vms that
have been shutdown, we can reconfigure when receiving a destroy
connection notification from the vhost library.
$ while true; do
ovs-appctl dpif-netdev/pmd-rxq-show |awk '
/port: / {
tot++;
if ($5 == "(enabled)") {
en++;
}
}
END {
print "total: " tot ", enabled: " en
}'
sleep 1
done
total: 66, enabled: 66
total: 6, enabled: 2
This change requires a fix in the DPDK vhost library, so bump the minimal
required version to 18.11.2.
Co-authored-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
William Tu [Tue, 25 Jun 2019 21:52:38 +0000 (14:52 -0700)]
vswitchd: Separate disable system and route.
Previously, '--disable-system' disables both system dp and the system
routing table. The patch makes '--disable-system' only disable system
dp and adds '--disable-system-route' for disabling the route table.
This fixes failures when 'make check-system-userspace' for tunnel cases.
As a consequence, hitting errors due to OVS userspace parses the IGMP packet
but its datapaths do not, so odp_flow_key_to_flow() return ODP_FIT_TOO_LITTLE.
commit c645550bb249 ("odp-util: Always report ODP_FIT_TOO_LITTLE for IGMP.")
Fix it by filtering out the IGMP-related error message.
Signed-off-by: William Tu <u9012063@gmail.com> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Co-authored-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Commit e981a45a6cae4 (ovs-atomic: Add 64 bit apis.)
added a few 64 bit apis (e.g: atomic_count_inc64). For C++,
this invokes std::atomic_fetch_*_explicit() functions in
lib/ovs-atomic-c++.h.
The function overloading for 64 bit function fails without
specifiying something like: std::atomic_fetch_*_explicit<std::uint64_t>().
But it looks tricky to do this with macros.
This patch tries to fix the compilation failures by calling atomic
functions on the variables itself.
Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
David Marchand [Thu, 25 Apr 2019 15:22:09 +0000 (17:22 +0200)]
netdev-dpdk: Avoid reconfiguration on VIRTIO_NET_F_MQ changes.
At the moment, a malicious guest might negotiate VIRTIO_NET_F_MQ and
!VIRTIO_NET_F_MQ in a loop which would be seen as qp_num going from 1 to
n and n to 1 continuously, triggering datapath reconfigurations at each
transition.
Limit this by only reconfiguring on increased qp_num.
The previous patch reduced the observed cost of polling disabled queues,
so the only cost is memory.
Co-authored-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Kevin Traynor <ktraynor@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
David Marchand [Thu, 25 Apr 2019 15:22:08 +0000 (17:22 +0200)]
dpif-netdev: Only poll enabled vhost queues.
We currently poll all available queues based on the max queue count
exchanged with the vhost peer and rely on the vhost library in DPDK to
check the vring status beneath.
This can lead to some overhead when we have a lot of unused queues.
To enhance the situation, we can skip the disabled queues.
On rxq notifications, we make use of the netdev's change_seq number so
that the pmd thread main loop can cache the queue state periodically.
Greg Rose [Tue, 25 Jun 2019 18:45:52 +0000 (11:45 -0700)]
rhel: Fix upgrade path
There is a bug in the upgrade path from the old kmod-openvswitch SysV
based RPM to the new openvswitch-kmod systemd based RPM. Since the
name of the package is changed it is not possible to use the yum
or rpm upgrade options. This prevents passing in a 1 or 2 to the
%postun scriptlet section of the older RPM and that causes the section
to be treated as an 'erase'. The old kmod-openvswitch %postun section
proceeds to erase the symlinks in ../weak-updates/openvwswitch that
the installation of the new package had just created.
Fix this by adding a %posttrans tag to the systemd spec file. This
scriptlet is called after the symlinks have just been erased and
it calls the ovs-kmod-manage.sh script to recreate the symlinks and
run depmod -a again so that the correct kernel modules will be
found and loaded.
Yi-Hung Wei [Fri, 21 Jun 2019 17:51:23 +0000 (10:51 -0700)]
ofproto-dpif: Fix continuation with patch port
This patch fixes the ofp_port to odp_port translation issue on patch
port with nxt_resume. When OVS resumes processing a packet from
nxt_resume, OVS does not translate the ofp in_port to odp in_port
correctly if the packet is originally received from a patch port.
Currently,OVS sets the odp in_port for this resume pakcet as ODPP_NONE
and push the resume packet back to the datapath. Later on, if the packet
goes through a recirc, OVS will generate the following message since it
can not translate odp in_port (ODPP_NONE) back to ofp in_port during upcall,
and push down a datapath rule to drop the packet.
ofproto_dpif_upcall(handler16)|INFO|received packet on unassociated
datapath port 4294967295
When OVS revalidates the drop datapath flow with ODPP_NONE in_port, we
will see the following warning.
ofproto_dpif_upcall(revalidator18)|WARN|Failed to acquire udpif_key
corresponding to unexpected flow (Invalid argument): ufid:....
This patch resolves this issue by storing the odp in_port in the
continuation messages, and restores the odp in_port before push the
packet back to the datapath.
VMWare-BZ: 2364696 Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
David Marchand [Wed, 19 Jun 2019 07:26:29 +0000 (09:26 +0200)]
travis: Make it possible to build against a dpdk branch.
Rework the build script so that we can pass branches and tags.
With this, DPDK_VER can be passed as:
- a string starting with refs/ which is understood as a git reference.
This triggers a git clone on DPDK_GIT (default value points to
https://dpdk.org/git/dpdk) for a single branch pointing to this
reference (to save some disk),
- else, any other string which is understood as an official release.
This triggers a tarball download on dpdk.org.
Signed-off-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
David Marchand [Wed, 19 Jun 2019 07:26:28 +0000 (09:26 +0200)]
travis: Do not patch dpdk sources.
Rather than patch the dpdk makefile and a template config file, we can
pass the -fPIC flag via EXTRA_CFLAGS.
This is more reliable than expecting the dpdk file names to be kept
unchanged.
Signed-off-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Yanqin Wei [Thu, 13 Jun 2019 10:38:07 +0000 (18:38 +0800)]
util: implement count_1bits with Neon intrinsics or gcc built-in for aarch64.
Userspace datapath needs to traverse through miniflow values many times. In
this process, 'count_1bits' operation for 'Flowmap' significantly impact
performance. On arm, this function was defined by portable implementation
because gcc for arm does not support popcnt feature.
But in the aarch64, VCNT neon instruction can accelerate "count_1bits".
From Gcc-7, the built-in function is implemented with neon intruction.
In this patch, count_1bits function will be impelmented with gcc built-in
from gcc-7 on, and with neon intrinsics in gcc-6.
Performance test was run in two aarch64 machines. In the NIC2NIC test, one
tuple dpcls lookup case achieves around 4% throughput improvement and
10(average) tuples case achieves around 5% improvement.
Yifeng Sun [Wed, 12 Jun 2019 22:35:29 +0000 (15:35 -0700)]
datapath: Support kernel version 5.0.x
This patch updated acinclude.m4 so that OVS can be compiled on
5.0.x kernels.
This patch also updated travis files so that 5.0.x kernel versions
are used during travis test builds.
Besides, NEWS and releases.rst are also updated to reflect this
new support.
Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
net: core: dev: Add extack argument to dev_change_flags()
In order to pass extack together with NETDEV_PRE_UP notifications, it's
necessary to route the extack to __dev_open() from diverse (possibly
indirect) callers. One prominent API through which the notification is
invoked is dev_change_flags().
Therefore extend dev_change_flags() with and extra extack argument and
update all users. Most of the calls end up just encoding NULL, but
several sites (VLAN, ipvlan, VRF, rtnetlink) do have extack available.
Since the function declaration line is changed anyway, name the other
function arguments to placate checkpatch.
Signed-off-by: Petr Machata <petrm@mellanox.com> Acked-by: Jiri Pirko <jiri@mellanox.com> Reviewed-by: Ido Schimmel <idosch@mellanox.com> Reviewed-by: David Ahern <dsahern@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
This patch backports the above upstream patch and also adds fixes
in compat code.
Cc: Petr Machata <petrm@mellanox.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 12 Jun 2019 22:35:27 +0000 (15:35 -0700)]
datapath: Backport the removal of __tcp_checksum_complete()
Upstream commit 6ab6dfa6bb500f5cbb9b7a0f23a1613417ca2d12 ("net: get
rid of __tcp_checksum_complete())" deleted __tcp_checksum_complete()
and caused compilation failure for OVS on newer kernels.
This patch fixes it by using __skb_checksum_complete(), which is
100% the same with __tcp_checksum_complete().
Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
This is a minimal change to allow removing of VLAN_TAG_PRESENT.
It leaves OVS unable to use CFI bit, as fixing this would need
a deeper surgery involving userspace interface.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> Signed-off-by: David S. Miller <davem@davemloft.net>
(2) commit 6083e28aa02d7c9e6b87f8b944e92793094ae047
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Sat Nov 10 19:55:34 2018 +0100
OVS: remove VLAN_TAG_PRESENT - fixup
It turns out I missed one VLAN_TAG_PRESENT in OVS code while rebasing.
This fixes it.
Fixes: 9df46aefafa6 ("OVS: remove use of VLAN_TAG_PRESENT") Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> Signed-off-by: David S. Miller <davem@davemloft.net>
This patch backports the above upstream patch to OVS and adds
extra checking in kernel module's compat code.
Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 12 Jun 2019 22:35:25 +0000 (15:35 -0700)]
datapath: Check extack argument of rtnl_create_link()
Upstream commit d0522f1cd25edb796548f91e04766fa3cbc3b6df ("net:
Add extack argument to rtnl_create_link") added new argument
to rtnl_create_link(). This introduced compiling errors in
the code of kernel datapath.
This patch fixes this issue.
Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Raed Salem [Mon, 10 Jun 2019 11:58:40 +0000 (14:58 +0300)]
netdev-tc-offloads: Use correct hook qdisc at init tc flow
A preliminary netdev qdisc cleanup is done during init tc flow.
The cited commit allows for creating of egress hook qdiscs on internal
ports. This breaks the netdev qdisc cleanup as currently only ingress
hook qdiscs type is deleted. As a consequence the check for tc ingress
shared block support fails when the check is done on internal port.
Issue can be reproduced by the following steps:
- start openvswitch service
- create ovs bridge
- restart openvswitch service
Fix by using the correct hook qdisc type at netdev hook qdisc cleanup.
Fixes 608ff46aaf0d ("ovs-tc: offload datapath rules matching on internal ports") Signed-off-by: Raed Salem <raeds@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Acked-by: John Hurley <john.hurley@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Dumitru Ceara [Wed, 12 Jun 2019 15:59:02 +0000 (17:59 +0200)]
ovn-controller: Fix parsing of OVN tunnel IDs
Encap tunnel-ids are of the form:
<chassis-id><OVN_MVTEP_CHASSISID_DELIM><encap-ip>.
In physical_run we were checking if a tunnel-id corresponds
to the local chassis-id by searching if the chassis-id string
is included in the tunnel-id (strstr). This can break quite
easily, for example, if the local chassis-id is a substring
of a remote chassis-id. In that case we were wrongfully
skipping the tunnel creation.
To fix that new tunnel-id creation and parsing functions are added in
encaps.[ch]. These functions are now used everywhere where applicable.
Acked-by: Venu Iyer <iyervl@ymail.com>
Reported-at: https://bugzilla.redhat.com/1708131 Reported-by: Haidong Li <haili@redhat.com> Fixes: b520ca7 ("Support for multiple VTEP in OVN") Signed-off-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 11 Jun 2019 15:31:21 +0000 (18:31 +0300)]
travis: Don't install kernel for DPDK checks.
We don't need to build DPDK kernel modules to test build with OVS.
And we don't need to build OVS datapath modules for checking
userspace with DPDK.
Removed 'max-inline-insns-single' changes that only was needed for
DPDK kernel modules. Config modifications changed to update
generated build/.config instead of changing sources.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Reviewed-by: David Marchand <david.marchand@redhat.com> Tested-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ilya Maximets [Tue, 7 May 2019 09:24:08 +0000 (12:24 +0300)]
netdev: Split up netdev offloading to separate module.
New module 'netdev-offload' created to manage different flow API
implementations. All the generic and provider independent code moved
there from the 'netdev' module.
Flow API providers further encapsulated.
The only function that was changed is 'netdev_any_oor'.
Now it uses offloading related hmap instead of common 'netdev_shash'.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ben Pfaff <blp@ovn.org> Acked-by: Roi Dayan <roid@mellanox.com>
Ilya Maximets [Wed, 15 May 2019 14:32:32 +0000 (17:32 +0300)]
dpctl: Update docs about dump-flows and HW offloading.
Since introduction of dynamic flow API for netdevs, tricky
accesses to uninitialized flow API are no longer possible.
So, ovs-dpctl doesn't support dumping HW offloaded flows now.
Claim this in docs and man pages. Additionally forbidden
'type' argument for 'ovs-dpctl dump-flows'.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Roi Dayan <roid@mellanox.com>
Ilya Maximets [Tue, 7 May 2019 09:24:07 +0000 (12:24 +0300)]
netdev: Dynamic per-port Flow API.
Current issues with Flow API:
* OVS calls offloading functions regardless of successful
flow API initialization. (ex. on init_flow_api failure)
* Static initilaization of Flow API for a netdev_class forbids
having different offloading types for different instances
of netdev with the same netdev_class. (ex. different vports in
'system' and 'netdev' datapaths at the same time)
Solution:
* Move Flow API from the netdev_class to netdev instance.
* Make Flow API dynamic, i.e. probe the APIs and choose the
suitable one.
Side effects:
* Flow API providers localized as possible in their modules.
* Now we have an ability to make runtime checks. For example,
we could check if particular device supports features we
need, like if dpdk device supports RSS+MARK action.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Roi Dayan <roid@mellanox.com>
Recent versions of systemd restores RuntimeDirectory ownership to the
unit's User in between execution of *Exec directives (see [1]). Using
ExecStartPre to reset RuntimeDirectory ownership to OVS_USER no longer
works as expected.
The ctl scripts already handle creation of the runtime directory with
correct ownership and permissions so we can basically remove
RuntimeDirectory from systemd unit file. There is still need to handle
ownsership to cover some upgrade scenarios, but success of that will be
optional as the directory itself wont exist at first time run.
OVN database directory is createid on first run so make ownership
handling optional.
Fixes: 94e1e8be3187 ("rhel: run ovn with the same user as ovs") Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
systemd assesses the presssence of all EnvironmentFile before execution
of Exec* directives, thus useropts needs to be optional even though it
will always be created at ExecStartPre.
Fixes: 94e1e8be3187 ("rhel: run ovn with the same user as ovs") Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Nitin Katiyar [Sun, 9 Jun 2019 14:18:10 +0000 (14:18 +0000)]
lacp: Don't send or receive PDUs when carrier state of slave is down
Fortville NICs (or their drivers) can get into an inconsistent state,
in which the NIC can actually transmit and receive packets even
though they report "PHY down". In such a state, OVS can exchange and
process LACP messages and enable a LACP slave. However, further packet
exchange over the slave fails because OVS sees that the PHY is down.
This commit fixes the problem by making OVS ignore received LACP PDUs
and suppress transmitting LACP PDUs when carrier is down. In addition,
when a LACP PDU is received with carrier down, this commit triggers
rechecking the carrier status (by incrementing the connectivity sequence
number) to ensure that it is updated as quickly as possible.
Nitin Katiyar [Sun, 9 Jun 2019 14:17:45 +0000 (14:17 +0000)]
lacp: Avoid packet drop on LACP bond after link up
Problem:
========
The OVS state machine that enables and disables bond slaves runs in
the OVS main thread. The OVS code that processes received LACP packets
runs in a different thread. Until now, when the latter processes a LACP
PDU that should enable a slave, the slave was only enabled when the
main thread was able to run the state machine. In some cases this led
to delays of up to 350ms when the main thread was busy or not scheduled,
which led to corresponding delays in which packets were dropped due to
the bond-admissibility check.
Fix:
====
When a LACP PDU is received, evaluate whether LACP slave can be enabled
(slave_may_enable()) and set LACP slave's may_enable from the datapath
thread itself. When may_enable = TRUE, it means L1 state is UP and
LACP-SYNC is done and it is waiting for the main thread to enable the
slave. Relax the check in bond_check_admissibility() to check for both
"enable" and "may_enable" of the LACP slave. This would avoid dropping
of packets until the main thread enables the slave from bundle_run().
Ilya Maximets [Thu, 16 May 2019 16:39:21 +0000 (19:39 +0300)]
travis: Test with latest stable kernel releases.
Instead of managing kernel minor versions manually we could always test
with the most recent stable release of the desired branch.
With this patch applied Travis will always check with the most recent
kernels, so we'll be notified about changes in upstream kernels that
breaks the build of our kernel module. However, this will also break
Travis checks on patches that doesn't touch the kernel parts until
we fix the module.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Acked-by: Ben Pfaff <blp@ovn.org>
Both ovn and ovs share the same log and run directories which are owned
by the user running ovs so it makes sense that ovn runs under that user
too to diminish security concerns and possible problems with log rotation.
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
The openvswitch useropts file is being stored in a directory where the
openvswitch user has write permissions. The openvswitch user can then
manipulate the file to change the user under which switchd daemon runs.
This patch changes the file to /var/openvswitch.useropts preventing any
manipulation.
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Damijan Skvarc [Fri, 10 May 2019 08:57:16 +0000 (10:57 +0200)]
nsh: Fix "shadow" warnings while compiling with clang.
Because of the macro implementation of htonX() and ntohX(), using one in
the argument of the other yields warnings. This commit avoids the issue by
using a temporary variable.
This does not fix a bug, only suppresses a warning.
Submitted-at: https://github.com/openvswitch/ovs/pull/283 Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 30 Apr 2019 22:06:34 +0000 (15:06 -0700)]
ofp-actions: Eliminate redundant error messages from ofpacts_parse__().
These duplicate messages emitted by ofpacts_verify(), so drop them.
These were previously useful because ofpacts_verify()'s error messages were
not as good as those emitted by ofpacts_parse__(), but that's been fixed
now.
Acked-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Lorenzo Bianconi [Wed, 15 May 2019 13:33:08 +0000 (15:33 +0200)]
ovn: grab pinctrl_mutex before running pinctrl_handle_buffered_packets
pinctrl_handle_buffered_packets can insert new elements in
buffered_packets_map hasmap and it runs concurrently with pinctrl_run
starting from commit 3594ffab6b4b. Fix possible races grabbing
pinctrl_mutex before running pinctrl_handle_buffered_packets
Fixes: 3594ffab6b4b ("ovn-controller: Add a new thread in pinctrl module to handle packet-ins.") Acked-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Fri, 24 May 2019 18:24:25 +0000 (11:24 -0700)]
datapath: Fix build errors for 4.9.172+ kernels
4.9.172+ kernel backported upstream patch 70b095c843266
("ipv6: remove dependency of nf_defrag_ipv6 on ipv6 module")
and this caused compilation errors of OVS kernel module.
This patch fixes it by checking and using new functions
introduced by the upstream patch.
Travis tests passed at
https://travis-ci.org/yifsun/ovs-travis/builds/536527230
with latest Linux kernel version.
In addition, this patch doesn't introduce failed tests for latest kernels
of Ubuntu (bionic, trusty, xenial), fedora, centos 73, rhel (74, 75, 76).
Reported-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Anju Thomas [Wed, 29 May 2019 09:54:04 +0000 (09:54 +0000)]
ofproto-dpif-xlate: Fix match criteria for in-band control rule
As part of in-band control, OVS is expected to send DHCP server replies
to the LOCAL port as well. In this case, OVS implicitly adds an
additional action to output to the bridge’s LOCAL port after the ofproto
translation for the packet is completed in the ofproto layer but before
sending the actions to datapath for installation.
However, the match criteria is unchanged and as a result all packets
(not just DHCP server replies) are also sent to the LOCAL port.
The fix is to add the IP protocol type (UDP), the UDP source and
destination ports to the match criteria so that a specific datapath flow
that matches only DHCP server replies is installed. As a result, only
DHCP server reply packets will be sent to the LOCAL port.
Signed-off-by: Anju Thomas <anju.thomas@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
flow: Don't include ports of first fragments in hash
For a series of IP fragments, only the first packet includes the transport
header (TCP/UDP/SCTP) and the src/dst ports. By including these port
numbers in the hash, it may happen that a first fragment hashes to a
different value than subsequent packets, causing different packets from
the same flow to follow different paths. This in turn may result in
out-of-order delivery or failed reassembly. This patch excludes port
numbers from the hash calculation in case of IP fragmentation.
Signed-off-by: Jeroen van Bemmel <jeroen.van_bemmel@nokia.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
For Openstack Internal DNS functionality we need
to provide support for domain_name option.
DHCP option 15 was previously used only in parser
tests and according to RFC it should be renamed to
domain_name [1].
This patch modifies its name in the tests from
'domain' to 'domain_name' and adds its support
to the code.
Ilya Maximets [Tue, 28 May 2019 13:39:46 +0000 (16:39 +0300)]
travis: Use enable-Werror knob.
Unlike manual injecting of "-Werror" to CFLAGS, '--enable-Werror'
enables failure on "sparse" and flake8 warnings. At least we wasn't
notified about flake8 warnings previously.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 28 May 2019 12:34:14 +0000 (15:34 +0300)]
acinclude: Add vector defines to sparse.
By adding compiler default flags for vector instructions to
cgcc we'll be able to check the same sources that we're building.
Also, this will allow to avoid re-defining these flags and
types specifically for "sparse" includes.
"sparse" headers "bmi2intrin.h" and "emmintrin.h" dropped as
not needed anymore.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Mon, 13 May 2019 12:39:11 +0000 (15:39 +0300)]
dpif-netdev: Forbid vport offloading attempts.
'netdev_flow_put()' for vports could eventually succeed for
userspace datapath in case there is a kernel datapath with
similar vport at the same time. The root cause is that vports
like 'vxlan' uses same 'vxlan_sys_<port>' system interfaces
for flow offloading and there is no way to distinguish system
and userspace vports using only 'netdev' structure.
Let's forbid vport offloading from userspace datapath to avoid
installing userspace flows to unrelated system devices.
Future dynamic flow API management will allow to enable vport
offloading back using more flexible checks.
Fixes: 241bad15d99a ("dpif-netdev: associate flow with a mark id") Reported-by: Ophir Munk <ophirmu@mellanox.com> Acked-By: Roni Bar Yanai <roniba@mellanox.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Ilya Maximets [Mon, 13 May 2019 11:37:26 +0000 (14:37 +0300)]
dpif-netdev: Fix flow mark leak on port lookup failure.
Flow mark should be properly freed in all error cases.
Fixes: 241bad15d99a ("dpif-netdev: associate flow with a mark id") Acked-By: Roni Bar Yanai <roniba@mellanox.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Darrell Ball [Tue, 28 May 2019 18:14:43 +0000 (11:14 -0700)]
conntrack: Fix missed 'conn' lookup checks.
Whenever a 'conn' entry is removed or added, we need to reverify it's
existence status under lock protection. There were some cases that
were missed, so fix them.
Yifeng Sun [Fri, 31 May 2019 21:58:09 +0000 (14:58 -0700)]
rhel7.6: Support kmod build and load for mulitple kernel versions
RHEL 7.6's new kernel 3.10.0-957.12.1.el7 introduced some ABI changes.
This leads to an issue that OVS kernel module built against
kernels newer than 12.1 cannot be loaded on systems running kernels
older than 12.1, vice versa.
By inspecting datapath/linux/openvswitch.mod.c, signatures of
the following kernel functions have changed in 3.10.0-957.12.1.el7:
nf_ct_get_tuple
nf_ct_invert_tuplepr
__nf_ct_l4proto_find
This patch enables OVS to build kernel modules against multiple
kernel versions and enables RHEL 7.6 system to load the compatible
module at runtime.
make rpm-fedora-kmod \
RPMBUILD_OPT='-D "kversion 3.10.0-957.10.1.el7.x86_64 \
3.10.0-957.12.1.el7.x86_64"'
Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Eli Britstein [Tue, 21 May 2019 12:11:51 +0000 (12:11 +0000)]
netdev-tc-offloads: Support match on priority tags
The logic by which a TC rule has a VLAN match is by the VLAN TCI field,
either the VID, PCP or CFI are non-zero. For priority-tag packets
there is a VLAN tag header with a zero VLAN TCI. Match on existence of
VLAN header (TPID) regardless of TCI matching.
Signed-off-by: Eli Britstein <elibr@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Bhargava Shastry [Thu, 30 May 2019 10:38:23 +0000 (12:38 +0200)]
ossfuzz: Fix expr target build failure
Google oss-fuzz reported a build failure for the OVN expression parser.
Upon investigation, it turned out that the expr_parse_target fuzzer was
not being run by Google due to the said failure.
The root cause of the build failure turned out to be a change in the
definition of the expr_parse_string() API. Now, this API accepts an
additional parameter of type struct sset * that points to the set of
address set referenced which may be NULL if unused.
This patch adds this additional parameter to expr_parse_string()
setting the pointer to the set of address set referenced to NULL.
Once this patch is applied, ossfuzz's expr_parse_target should build
and subsequently be fuzzed.
CC: Han Zhou <hzhou8@ebay.com> Fixes: 43e6900a7991 ("ovn-controller: Maintain resource references for logical flows.") Signed-off-by: Bhargava Shastry <bshastry@sect.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Mon, 11 Feb 2019 17:34:06 +0000 (20:34 +0300)]
dpif-netdev: Fix unsafe access to pmd polling lists.
All accesses to 'pmd->poll_list' should be guarded by
'pmd->port_mutex'. Additionally fixed inappropriate usage
of 'HMAP_FOR_EACH_SAFE' (hmap doesn't change in a loop)
and dropped not needed local variable 'proc_cycles'.
CC: Nitin Katiyar <nitin.katiyar@ericsson.com> Fixes: 5bf84282482a ("Adding support for PMD auto load balancing") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Kevin Traynor <ktraynor@redhat.com> Acked-by: Aaron Conole <aconole@redhat.com>
Ankur Sharma [Fri, 17 May 2019 00:27:28 +0000 (00:27 +0000)]
OVN: Fix the ovn-controller 100% usage issue with put_mac_bindings
ISSUE:
a. As soon as entries get added to put_mac_bindings in pinctrl.c,
ovn-controller CPU consumption reached 100%.
b. This happens because in wait_put_mac_bindings,
if !hmap_is_empty(&put_mac_bindings) succeeds and
calls poll_immediat_wake().
ovn-controller.log:
"2019-05-10T19:43:28.035Z|00035|poll_loop|INFO|wakeup due to
0-ms timeout at ovn/controller/pinctrl.c:2520 (99% CPU usage)"
ROOT_CAUSE:
a. Earlier it used to work fine, because in run_put_mac_bindings
all the bindings in put_mac_bindings would be flushed.
b. As a part of adding a new thread in pinctrl, this
line got replaced with calling buffer_put_mac_bindings.
"
.
.
.
/* Move the mac bindings from 'put_mac_bindings' hmap to
* 'buffered_mac_bindings' and notify the pinctrl_handler.
* pinctrl_handler will reinject the buffered packets. */
if (!hmap_is_empty(&put_mac_bindings)) {
buffer_put_mac_bindings();
notify_pinctrl_handler();
}
"
And buffer_put_mac_binding would pop the bindings from
put_mac_bindings, thereby emptying it.
c. However, 1c24b2f490ba002bbfeb23006965188a7c5b9747
changed the buffer dequeueing logic and in the process
removed buffer_put_mac_binding, as a result put_mac_bindings
would never get empty.
FIX:
a. Added call to flush_put_mac_bindings back in
run_put_mac_bindings.
When displaying a connection entry, several TCP fields are read
from a connection entry. Hence, expand the 'conn' locking so the display
does not potentially include fields values from different aggregate
states.
Even if mac_binding is not used for workload provisioning, the
mac_binding table update will happen for gateway external ARP
processing. This patch ensures ARP events on gateway nodes causing
mac-binding changes do not trigger full recomputing on all chassises.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Jakub Sitnicki [Fri, 17 May 2019 19:56:40 +0000 (12:56 -0700)]
ovn: Test for full logical flow processing in ovn-controller.
From: Jakub Sitnicki <jkbs@redhat.com>
Add a test that performs typical operations of creating & destroying
logical routers, switches, ports, address sets and ACLs while checking
if they trigger full logical flow processing in the ovn-controller.
This way confirm that incremental processing is taking effect when we
expect it to.
Place the new test in a separate module - tests/ovn-performance.at,
instead of the usual tests/ovn.at as it doesn't test OVN's functionality
but rather a performance aspect of ovn-controller.
Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> Acked-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Jakub Sitnicki [Fri, 17 May 2019 19:56:39 +0000 (12:56 -0700)]
coverage: Add command for reading counter value.
From: Jakub Sitnicki <jkbs@redhat.com>
Facilitate checking coverage counters from scripts and tests with a new
coverage/read-counter command that gets the total count for a counter.
Same could be achieved by scraping the output of coverage/show command
but the difficulties there are that output is in human readable format
and zero-value counters are not listed.
Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> Acked-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Fri, 17 May 2019 19:56:36 +0000 (12:56 -0700)]
ovn-controller: Incremental processing for address-set changes.
When the content of an address set changes, ovn-controller will
not recompute all flows but only the ones related to the changed
address-set. The performance test result is discussed at [1].
projects / ovs.git / log