Ben Pfaff [Mon, 10 Dec 2018 17:43:19 +0000 (09:43 -0800)]
tests: Simplify and improve the daemon tests.
The daemon tests used files a lot when shell variables were easier to use
and easier to understand. This commit changes that.
The tests created empty databases that aren't really needed anymore. This
commit changes them to use the ovsdb-server --no-db option instead.
The tests had a lot of common code for checking the ancestry of processes.
This commit factors out a new shell function check_ancestors.
The tests tended to use random pidfile names. This switches to just using
the defaults, which are fine.
The tests didn't check the names of the child processes. This adds those
checks using the new check_process_name shell function. This should avoid
regression of the bug fixed by commit 266f79e32c60 ("daemon-unix: Use
same name for original or restarted children.")
Other minor improvements too.
I only made small updates to the Windows-specific test, because it is hard
for me to verify.
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Mon, 19 Nov 2018 19:09:26 +0000 (11:09 -0800)]
dpctl: Simplify opt_dpif_open().
The commonly used function, opt_dpif_open(), recently became more complex
to check for a datapath argument. Unnecessary dummy parameters for most users
were hence added. Revert back and call the intended api, dp_arg_exists(), to
query for a datapath argument being supplied.
Fixes: 4eeec031d4c4 ("dpctl: Implement dpctl commands for conntrack per zone limit") Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
David Marchand [Thu, 22 Nov 2018 15:37:57 +0000 (16:37 +0100)]
ovs-ctl: fix system-id.conf owner
As far as RPMs are concerned, system-id.conf file is declared as being
owned by openvswitch.
At the first ovs startup, ovs-ctl creates this file if none exists without
ensuring this.
We end up with an inconsistency:
$ rpm -V openvswitch
.....UG.. c /etc/openvswitch/system-id.conf
Fix this when ovs-ctl is the one who creates the file.
Note: this issue ends up being hidden after a RPM upgrade, since the
openvswitch user is enforced on the whole /etc/openvswitch directory as a
%post operation.
Acked-by: Timothy Redaelli <tredaelli@redhat.com> Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Timothy Redaelli [Tue, 20 Nov 2018 18:40:50 +0000 (19:40 +0100)]
rhel: Don't ship static libraries
Since commit bc4fd439586f ("rhel: Ship ovs shared libraries, fedora")
openvswitch-devel RPM package includes both static and shared library.
This is against the Fedora Packaging Guidelines [1].
This commit prevent the static libraries and libtool archives to be shipped.
Ilya Maximets [Mon, 10 Dec 2018 17:05:22 +0000 (20:05 +0300)]
ovs-thread: Drop xpthread_meutex_{un}lock finctions.
There are no users of these functions.
This change fixes clang build on FreeBSD:
lib/ovs-thread.c:158:1: error: \
mutex 'mutex' is still held at the end of function \
[-Werror,-Wthread-safety-analysis]
XPTHREAD_FUNC1(pthread_mutex_lock, pthread_mutex_t *);
^
lib/ovs-thread.c:138:5: note: expanded from macro 'XPTHREAD_FUNC1'
}
^
Fixes: 4dff0893c376 ("ovs-atomic-pthreads: Use global shared locks for atomic_flag also.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Mon, 10 Dec 2018 17:05:20 +0000 (20:05 +0300)]
configure.ac: More enhanced check for pthread library.
FreeBSD 12 supports 'pthread_rwlock_tryrdlock' without 'pthread'
library. Let's add check for more rare function.
OTOH, Travis-CI environment supports 'pthread_rwlockattr_destroy',
but does not support 'pthread_rwlock_tryrdlock' without 'pthread'.
So, both checks needed.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yousong Zhou [Tue, 4 Dec 2018 01:41:26 +0000 (01:41 +0000)]
ovs-ctl: fallback to "uname -n" for fetching hostname
The command "hostname" is not available in OpenWrt by default. Strace
result of hostname-3.13 on centos7 shows that bare "hostname" command
calls uname() to fetch node name.
Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 28 Nov 2018 00:10:11 +0000 (16:10 -0800)]
ofctl_parse_target: Fix memory leaks if there is no usable protocol
When there is no usable protocol, ofctl_parse_flows__ returns without
properly freeing memory. A previous patch failed to fix this issue.
This patch fixes it.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11406
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11408 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 28 Nov 2018 00:10:12 +0000 (16:10 -0800)]
odp-util: Validate values of vid and pcp in push_vlan action
Oss-fuzz complains that 'vid << VLAN_VID_SHIFT' is causing an error of
"Undefined-shift in parse_odp_action". This is because an invalid
value of vid is passed in push_vlan. This patch adds validation to
the value of vid, in addition to the value of pcp.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11520 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 29 Nov 2018 18:37:02 +0000 (10:37 -0800)]
unixctl: Avoid 100% CPU for slowly processed requests with another queued.
If another request came in on a particular connection while the previous
request was still being processed, unixctl_server_wait() would wake up the
main loop but unixctl_server_run() wouldn't read the request, resulting in
100% CPU use.
I doubt whether this is a real problem because it's unusual for a client
to attempt to make requests in parallel. I found it while pursuing a 100%
CPU issue but it turned out not to be a bug (the 100% CPU was caused by
a client making requests as fast as possible).
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Tiago Lam [Tue, 27 Nov 2018 16:54:23 +0000 (16:54 +0000)]
netdev-dpdk: Add mbuf HEADROOM after alignment.
Commit dfaf00e started using the result of dpdk_buf_size() to calculate
the available size on each mbuf, as opposed to using the previous
MBUF_SIZE macro. However, this was calculating the mbuf size by adding
up the MTU with RTE_PKTMBUF_HEADROOM and only then aligning to
NETDEV_DPDK_MBUF_ALIGN. Instead, the accounting for the
RTE_PKTMBUF_HEADROOM should only happen after alignment, as per below.
Before alignment:
ROUNDUP(MTU(1500) + RTE_PKTMBUF_HEADROOM(128), 1024) = 2048
After aligment:
ROUNDUP(MTU(1500), 1024) + 128 = 2176
This might seem insignificant, however, it might have performance
implications in DPDK, where each mbuf is expected to have 2k +
RTE_PKTMBUF_HEADROOM of available space. This is because not only some
NICs have course grained alignments of 1k, they will also take
RTE_PKTMBUF_HEADROOM bytes from the overall available space in an mbuf
when setting up their Rx requirements. Thus, only the "After alignment"
case above would guarantee a 2k of available room, as the "Before
alignment" would report only 1920B.
Some extra information can be found at:
https://mails.dpdk.org/archives/dev/2018-November/119219.html
Note: This has been found by Ian Stokes while going through some
af_packet checks.
Reported-by: Ian Stokes <ian.stokes@intel.com> Fixes: dfaf00e ("netdev-dpdk: fix mbuf sizing") Signed-off-by: Tiago Lam <tiago.lam@intel.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Numan Siddique [Mon, 19 Nov 2018 16:17:38 +0000 (21:47 +0530)]
ovn: Avoid tunneling for VLAN packets redirected to a gateway chassis
An OVN deployment can have multiple logical switches each with a
localnet port connected to a distributed logical router in which one
logical switch may provide external connectivity and the rest of
the localnet logical switches use VLAN tagging in the physical
network.
As reported in [1], external traffic from these localnet VLAN tagged
logical switches are tunnelled to the gateway chassis (chassis hosting
a distributed gateway port which applies NAT rules). As part of the
discussion in [1], there are few possible solutions proposed by
Russell [2]. This patch implements the first option in [2].
With this patch, a new option 'reside-on-redirect-chassis' in 'options'
column of Logical_Router_Port table is added. If the value of this
option is set to 'true' and if the logical router also have a
distributed gateway port, then routing for this logical router port
is centralized in the chassis hosting the distributed gateway port.
If a logical switch 'sw0' is connected to a router 'lr0' with the
router port - 'lr0-sw0' with the address - "00:00:00:00:af:12 192.168.1.1"
, and it has a distributed logical port - 'lr0-public', then the
below logical flow is added in the logical switch pipeline
of 'sw0' if the 'reside-on-redirect-chassis' option is set on 'lr-sw0' -
"cr-lr0-public" is an internal port binding created by ovn-northd of type
'chassisredirect' for lr0-public in SB DB. Please see "man ovn-sb" for more details.
With the above flow, the packet doesn't enter the router pipeline in
the source chassis. Instead the packet is sent out via the localnet
port of 'sw0'. The gateway chassis upon receiving this packet, runs
the logical router pipeline applying NAT rules and sends the traffic
out via the localnet port of the logical switch providing external connectivity.
The gateway chassis will also reply to the ARP requests for the router port IPs.
With this approach, we avoid redirecting the external traffic to the
gateway chassis via the tunnel port. There are a couple of drawbacks
with this approach:
- East - West routing is no more distributed for the VLAN tagged
localnet logical switches if 'reside-on-redirect-chassis' option is defined
- 'dnat_and_snat' NAT rules with 'logical_mac' and 'logical_port'
columns defined will not work for these logical switches.
This approach is taken for now as it is simple. If there is a requirement
to support distributed routing for these VLAN tenant networks, we
can explore other possible solutions.
Ben Pfaff [Thu, 15 Nov 2018 16:25:52 +0000 (08:25 -0800)]
configure: Check for more specific function to pull in pthread library.
On my laptop, pthread_create() is always available without -lpthread, but
when I use -fsanitize=address, -lpthread is required to pull in other
threading functions such as pthread_rwlock_tryrdlock(). Thus, with
-fsanitize=address I have to manually add -lpthread to link commands one
way or another. This commit avoids that problem by checking for a
function that is sometimes only available in -lpthread.
Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 15 Nov 2018 00:07:30 +0000 (16:07 -0800)]
ofp-actions: Make all actions a multiple of OFPACT_ALIGNTO bytes.
The functions to put ofpacts into ofpbufs have always padded them to
OFPACT_ALIGNTO boundaries, but the underlying structures weren't
necessarily padded out. That led to difficulties in a few places where
structures were allocated on the stack instead in an ofpbuf, because
functions like ofpact_init_*() would access beyond the end of the actual
structure. This is true, for example, in test_multipath_main() in
tests/test-multipath.c, which allocates a struct ofpact_multipath on the
stack, and in lswitch_handshake() in learning-switch.c, which allocates
a struct ofpact_output on the stack.
It's possible to fix these individual cases, but it's possible that there
are others that haven't been identified. This commit addresses the issue
another way, by padding all of the ofpact structures to a full multiple
of OFPACT_ALIGNTO and adding assertions to ensure that it can't be screwed
up in the future.
This commit removes the OFPACT_*_SIZE enums, because they are now
equivalent to sizeof(struct ofpact_*) in every case.
Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Wed, 14 Nov 2018 23:39:05 +0000 (15:39 -0800)]
tests: Always use --no-chdir with --detach.
With --detach but not --no-chdir, core files and Address Sanitizer logs
don't go into the testsuite directory but end up dropped because it tries
to write them in the root directory.
Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 13 Nov 2018 21:25:08 +0000 (13:25 -0800)]
raft: Fix notifications when a server leaves the cluster.
When server A sends the leader a request to remove server B from the
cluster, where A != B, the leader sends both A and B a notification when
the removal is complete. Until now, however, the notification (which is a
raft_remove_server_reply message) did not say which server had been
removed, and the receiver did not check. Instead, the receiver assumed
that it had been removed. The result was that B was removed and A stopped
serving out the database even though it was still part of the cluster,
This commit fixes the problem.
Reported-by: ramteja tadishetti <ramtejatadishetti@gmail.com> Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 13 Nov 2018 21:17:43 +0000 (13:17 -0800)]
raft: Avoid null dereference in raft_update_our_match_index().
When the server is leaving the cluster but remains leader, the
raft_find_server() call can return NULL. Previously this caused a null
dereference. This commit fixes the problem.
Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 13 Nov 2018 17:50:29 +0000 (09:50 -0800)]
raft: Avoid use-after-free error in raft_update_commit_index().
raft_update_commit_index() iterates over a sequence of entries that may
have up to two components: a set of servers and a piece of data. When
a set of servers is present, it calls raft_run_reconfigure(), which can
call through the following chain of functions in some cases:
and raft_add_entry() can reallocate raft->entries, which turns the pointer
'e' that raft_update_commit_index() has to the current entry into a wild
pointer.
This commit fixes the problem.
Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 13 Nov 2018 17:26:40 +0000 (09:26 -0800)]
ovsdb-idl: Treat "unknown database" error as reason to reconnect.
Ordinarily the IDL finds out in advance whether a particular database is
on its server, or it finds out via notifications. But it's also a good
idea to adopt a belt-and-suspenders approach so that, if the IDL does
receive an "unknown database" error, we treat it as a "soft" error that
can be fixed by reconnecting to another server, rather than a "hard" error
that should cause an immediate abort.
Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 13 Nov 2018 17:20:50 +0000 (09:20 -0800)]
ovsdb-idl: Avoid sending transactions when the DB is not synced up.
Until now the code here would happily try to send transactions to the
database server even if the database connection was not in the correct
state. In some cases this could lead to strange behavior, such as sending
a database transaction for a database that the IDL had just learned did not
exist on the server.
Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 16 Nov 2018 17:24:51 +0000 (09:24 -0800)]
pcap-file: Correctly format enum type.
The underlying type for an enum is somewhat unpredictable in that the
compiler and the ABI influence it. The format specifier I used here was
apparently correct for i386 on Linux but wrong for x86-64. It's better to
just use a cast.
Fixes: 597177a283da ("pcap-file: Add support for Linux SLL formatted PCAP files.") Reported-by: Simon Horman <simon.horman@netronome.com> Reviewed-by: Simon Horman <simon.horman@netronome.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 15 Nov 2018 17:38:39 +0000 (09:38 -0800)]
pcap: Fix reading regular old Ethernet pcap files.
This broke the unit tests.
Fixes: 597177a283da ("pcap-file: Add support for Linux SLL formatted PCAP files.") Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> Tested-by: Alin Gabriel Serdean <aserdean@ovn.org> Reported-by: Alin Gabriel Serdean <aserdean@ovn.org> Tested-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Tue, 13 Nov 2018 19:25:24 +0000 (11:25 -0800)]
odp-util: Add checking to prevent buffer overflow when parsing push_nsh
Previously, the buffer size of 'struct ofpbuf b' is less than the
size of 'char buf[512]', this could cause memory overflow of ofpbuf
when calling ofpbuf_put_hex. This patch fixes it.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10865 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 14 Nov 2018 23:14:05 +0000 (15:14 -0800)]
oss-fuzz: Fix memory leak in ofctl_parse_flow
If parse_ofp_flow_mod_str returns no error, ofputil_flow_mod.match
contains allocated memory that should be free. This patch fixes it.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11343 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Chris Mi [Mon, 12 Nov 2018 02:08:38 +0000 (11:08 +0900)]
netdev-tc-offloads: Delete ufid tc mapping in the right place
Currently, the ufid tc mapping is deleted in add_ufid_tc_mapping().
But if tc_replace_flower() failed, the old ufid tc mapping will not
be deleted. If another thread adds the same tc mapping successfully,
then there will be multiple mappings for the same ifindex, handle
and prio.
Fixes: 9116730db ("netdev-tc-offloads: Add ufid to tc/netdev map") Signed-off-by: Chris Mi <chrism@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Timothy Redaelli [Sun, 11 Nov 2018 10:04:17 +0000 (11:04 +0100)]
ipsec: Install ovs-monitor-ipsec in script directory
In commit d5cc46e3d185 ("ipsec: Use @PYTHON@ directly instead of
"/usr/bin/env python"") ovs-monitor-ipsec is installed in bin directory,
but it's supposed to be installed in script directory.
This commit removes also the manual copy of "ovs-monitor-ipsec" in spec file
since it's installed directly in "make install".
Fixes: d5cc46e3d185 ("ipsec: Use @PYTHON@ directly instead of "/usr/bin/env python"") Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Timothy Redaelli [Sun, 11 Nov 2018 10:04:33 +0000 (11:04 +0100)]
gitignore: Ignore ovs-monitor-ipsec
Commit d5cc46e3d185 ("ipsec: Use @PYTHON@ directly instead of "/usr/bin/env
python"") introduced ovs-monitor-ipsec.in that generates
ovs-monitor-ipsec.
This commit adds ovs-monitor-ipsec to ipsec/.gitignore.
Fixes: d5cc46e3d185 ("ipsec: Use @PYTHON@ directly instead of "/usr/bin/env python"") Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Mon, 12 Nov 2018 12:20:39 +0000 (15:20 +0300)]
pinctrl: Fix dp_packet structure leak.
Buffered packets are always packets created by 'dp_packet_clone_data()'
i.e. they are malloced. It's not enough to free the packet data,
dp_packet structure must be freed too. 'dp_packet_delete()' will take
care of that.
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Fixes: d7abfe39cfd2 ("OVN: add buffering support for ip packets") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Mon, 12 Nov 2018 12:19:57 +0000 (15:19 +0300)]
pinctrl: Fix crash on buffered packets hmap double remove.
'destroy_buffered_packets()' removes the hmap node which was
already removed by 'HMAP_FOR_EACH_POP()' producing following
crash log:
Invalid read of size 8
at 0x134EDB: hmap_remove (hmap.h:287)
by 0x134EDB: destroy_buffered_packets (pinctrl.c:237)
by 0x13AB3B: destroy_buffered_packets_map (pinctrl.c:246)
by 0x13AB3B: pinctrl_destroy (pinctrl.c:1804)
by 0x12C0CF: main (ovn-controller.c:916)
Address 0x8 is not stack'd, malloc'd or (recently) free'd
Could be captured by check-valgrind on the following test:
'2720. ovn -- IP packet buffering'
Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Fixes: d7abfe39cfd2 ("OVN: add buffering support for ip packets") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Eelco Chaudron [Mon, 12 Nov 2018 09:26:22 +0000 (04:26 -0500)]
netdev-dpdk: Bring link down when NETDEV_UP is not set
When the netdev link flags are changed, !NETDEV_UP, the DPDK ports are not
actually going down. This is causing problems for people trying to bring
down a bond member. The bond link is no longer being used to receive or
transmit traffic, however, the other end keeps sending data as the link
remains up.
With OVS 2.6 the link was brought down, and this was changed with commit 3b1fb0779. In this commit, it's explicitly mentioned that the link down/up
DPDK APIs are not called as not all PMD devices support it.
However, this patch does call the appropriate DPDK APIs and ignoring
errors due to the PMD not supporting it. PMDs not supporting this should
be fixed in DPDK upstream.
I verified this patch is working correctly using the
ovs-appctl netdev-dpdk/set-admin-state <port> {up|down} and
ovs-ofctl mod-port <bridge> <port> {up|down} commands on a XL710
and 82599ES.
Timothy Redaelli [Sat, 10 Nov 2018 15:52:01 +0000 (16:52 +0100)]
rtnetlink: Remove executable bit from rtnetlink.h
In commit 135ee7ef362f ("rtnetlink: extend parser to include kind of master and
slave") the file mode of rtnetlink.h accidentaly changed from 0644 to 0755.
This commit restores the previous file mode (0644) on rtnetlink.h.
CC: John Hurley <john.hurley@netronome.com> Fixes: 135ee7ef362f ("rtnetlink: extend parser to include kind of master and slave") Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Timothy Redaelli [Sat, 10 Nov 2018 15:52:00 +0000 (16:52 +0100)]
bond: Remove executable bit from bond.c
In commit 90061ea7d1dd ("bond: Fix LACP fallback to active-backup when recirc
is enabled.") the file mode of bond.c accidentaly changed from 0644 to 0755.
This commit restores the previous file mode (0644) on bond.c.
CC: Ben Pfaff <blp@ovn.org> Fixes: 90061ea7d1dd ("bond: Fix LACP fallback to active-backup when recirc is enabled.") Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Timothy Redaelli [Sat, 10 Nov 2018 15:29:07 +0000 (16:29 +0100)]
ipsec: Use @PYTHON@ directly instead of "/usr/bin/env python"
Using "/usr/bin/env" is against Fedora Packaging Guidelines [1].
Moreover, in this specific case, it also prevent "make rpm-fedora" to
successfully complete on "Fedora Rawhide" since "#!/usr/bin/env python"
must not be used anymore [2].
This patch adds IPsec support for OVN tunnel. Basically, OVN offers a
binary option to its user for encryption configuration. If the IPsec
option is turned on, all tunnels will be encrypted. Otherwise, no tunnel
will be encrypted.
The changes are summarized as below:
1) Added a ipsec column on the NB_Global table and SB_Global table. The
value of ipsec column is propagated by ovn-northd from NB_Global to
SB_Global.
2) ovn-controller monitors the ipsec column in SB_Global. If the ipsec
value is true, ovn-controller sets options of the tunnel interface by
specifying "options:remote_name=<remote_chassis_name>". If the ipsec
value is false, ovn-controller removes these options.
3) ovs-monitor-ipsec daemon
(https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348701.html)
monitors the tunnel interface options and configures IKE daemon
accordingly for IPsec encryption.
Signed-off-by: Qiuyu Xiao <qiuyu.xiao.qyx@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
This patch reintroduces ovs-monitor-ipsec daemon that
was previously removed by commit 2b02d770 ("openvswitch:
Allow external IPsec tunnel management.")
After this patch, there are no IPsec flavored tunnels anymore.
IPsec is enabled by setting up the right values in:
1. OVSDB:Interface:options column;
2. OVSDB:Open_vSwitch:other_config column;
3. OpenFlow pipeline.
GRE, VXLAN, GENEVE, and STT IPsec tunnels are supported. LibreSwan and
StrongSwan IKE daemons are supported. User can choose pre-shared key,
self-signed peer certificate, or CA-signed certificate as authentication
methods.
Han Zhou [Thu, 8 Nov 2018 06:29:44 +0000 (22:29 -0800)]
ofproto.c: Handle the situation when ofp_port number exhausted.
When ofp_port number is exhausted, OFPP_NONE (65535) will be
returned by alloc_ofp_port(). In this case we should error out
instead of continue using 65535 as port number.
Using the invalid number causes unpredictable consequences:
2018-11-06T01:29:10.042Z|142103|dpif(ovs-vswitchd)|WARN|system@ovs-system: failed to add ovn-aded97-0 as port: Device or resource busy
2018-11-06T01:29:10.045Z|142104|bridge(ovs-vswitchd)|INFO|bridge br-int: added interface ovn-aded97-0 on port 65535
2018-11-06T01:29:11.479Z|142108|ofproto(ovs-vswitchd)|WARN|br-int: cannot configure bfd on nonexistent port 65535
2018-11-06T01:29:11.479Z|142109|ofproto(ovs-vswitchd)|WARN|br-int: cannot configure LLDP on nonexistent port 65535
2018-11-06T01:29:11.479Z|142110|ofproto(ovs-vswitchd)|WARN|br-int: cannot configure datapath on nonexistent port 65535
...
2018-11-06T01:29:18.783Z|142117|bfd(ovs-vswitchd)|INFO|ovn-aded97-0: BFD state change: admin_down->down "No Diagnostic"->"No Diagnostic".
2018-11-06T01:29:18.785Z|00061|bfd(monitor82)|INFO|Interface ovn-aded97-0 remote mult value 0 changed to 3
2018-11-06T01:29:18.785Z|00062|bfd(monitor82)|INFO|ovn-aded97-0: New remote min_rx.
...
2018-11-06T01:29:18.773Z|142111|bridge(ovs-vswitchd)|INFO|bridge br-int: deleted interface ovn-aded97-0 on port 65535
...
2018-11-06T01:29:18.779Z|142115|dpif(ovs-vswitchd)|WARN|system@ovs-system: failed to add ovn-aded97-0 as port: Device or resource busy
2018-11-06T01:29:18.782Z|142116|bridge(ovs-vswitchd)|INFO|bridge br-int: added interface ovn-aded97-0 on port 65535
...
2018-11-06T01:29:18.785Z|00064|bfd(monitor82)|WARN|ovn-aded97-0: Incorrect your_disc.
...
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Thu, 8 Nov 2018 06:29:43 +0000 (22:29 -0800)]
ofproto.c: Fix port number leaking.
When there is an error in ofport_install(), the ofp port number is
not deallocated, which leads to port number leak. For example,
when there is an redundant tunnel port added in an OVS bridge,
ovs-vswitchd will try to add the port to ofproto whenever OVSDB
changes, which would trigger the port number leak, and over the
time there won't be any port available for valid requests.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 7 Nov 2018 21:44:34 +0000 (13:44 -0800)]
dns-resolve: Improve on handling of system DNS nameserver
This patch enables OVS on windows to read system nameserver configuration.
In addition, a new environment variable OVS_RESOLV_CONF is introduced.
If set, it can be used as DNS server configuration file. This variable
is supposed to be used for sandboxing other things. It is documented
accordingly.
Suggested-by: Ben Pfaff <blp@ovn.org> Suggested-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 7 Nov 2018 21:44:33 +0000 (13:44 -0800)]
dns-resolve: Stop dns resolving if no DNS server configured
DNS resolution should fail if no DNS servers are available. This
patch fixes it.
Suggested-by: Ben Pfaff <blp@ovn.org> Suggested-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 7 Nov 2018 20:42:16 +0000 (12:42 -0800)]
ofctl_parse_target: Avoid passing invalid ofputil_protocol to ofputil_protocol_to_ofp_version
In this test, the involved ovs functions expect valid ofputil_protocol
values. Therefore, if usable_protocols is invalid, we should return.
Otherwise, ovs will abort.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11165 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 7 Nov 2018 20:42:17 +0000 (12:42 -0800)]
odp-util: Set a limit for nested parse_odp_key_mask_attr call
This patch puts a limit on the nested depth in flow key string to avoid
stackoverflow. An example to show this issue is a key string contains
thousands of nested encaps. In addition, a new test is added for this fix.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11149 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 7 Nov 2018 20:42:15 +0000 (12:42 -0800)]
actions: Enforce a maximum limit for nested action depth
If nested depth of actions is too deep, then the stack will be overflown
and ovs-vswitch crashes. This patch prevents this by adding a depth limit
to nested actions.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11237 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Lorenzo Bianconi [Fri, 26 Oct 2018 16:25:59 +0000 (18:25 +0200)]
OVN: configure L2 address according to the used IP address
Configure L2 dynamic address according to used IPv4 address.
This patch allows to define a deterministic relationship between
L2 and L3 addresses when dynamic IPAM is used.
This patch allows to fix a possible L2/L3 address mismatch than can
occur when pods are created and destroyed at high rate [1] since if
there is no relation between MAC and IP addresses ARP cache can be
poisoned with a wrong correspondence
Lorenzo Bianconi [Fri, 26 Oct 2018 16:25:58 +0000 (18:25 +0200)]
OVN: assign new addresses at the end of build_ipam routine
Visit all ovn datapaths before adding new dynamic addresses to the
system in order to avoid possible L2 address duplication when
the same MAC address is configured on different ovn logical switches.
Current implementation can miss the duplicated address since macam
is cleared at each ovn run and there is no guarantee on visit order
of ovn datapath hash table
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Lorenzo Bianconi [Fri, 26 Oct 2018 16:20:44 +0000 (18:20 +0200)]
OVN: introduce mac_prefix support to IPAM
Add the possibility to specify a given mac address prefix for
dynamically generated mac address. Mac address prefix can be
specified in nbdb NB_Global table, options:mac_prefix=<mac_prefix>
This patch fix a possible issue of L2 address duplication if
multiple OVN deployments share a single broadcast domain
Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 2 Nov 2018 18:25:45 +0000 (11:25 -0700)]
ofproto-dpif-upcall: Don't purge ukeys while in a quiescent state.
revalidator_purge() iterates and modifies umap->cmap. This should
not happen in quiescent state, because cmap implementation based
on rcu protected variables. Let's narrow the quiescent period
to avoid possible wrong memory accesses.
CC: Joe Stringer <joe@ovn.org> Fixes: 9fce0584a643 ("revalidator: Use 'cmap' for storing ukeys.") Reported-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
When processing icmp unreachable message for erspan tunnel, tunnel id
should be erspan_net_id instead of ipgre_net_id.
Fixes: 84e54fe0a5ea ("gre: introduce native tunnel support for ERSPAN") Cc: William Tu<u9012063@gmail.com> Signed-off-by: Haishuang Yan<yanhaishuang@cmss.chinamobile.com> Acked-by: William Tu<u9012063@gmail.com> Signed-off-by: David S. Miller<davem@davemloft.net> Fixes: 8e53509c ("gre: introduce native tunnel support for ERSPAN") Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 1 Nov 2018 17:33:03 +0000 (10:33 -0700)]
odp-util: Validate close-brace in scan_geneve and fix return values of san_xxx functions
This patch adds validation of close-braces in scan_geneve. An simple
example is "set(encap(tunnel(geneve({{))))". When scan_geneve returns,
(struct geneve_scan *key)->len equals to 2*sizeof(struct geneve_opt).
That seems not correct.
Found this issue while inspecting oss-fuzz
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11153.
In addition, SCAN_TYPE expects scan_XXX functions to return 0
on errors. This patch inspects all related scan_XXX functions
and fixes their return values.
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Fri, 2 Nov 2018 11:41:01 +0000 (17:11 +0530)]
ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build
When 'make check' is called by the mock rpm build (which disables networking),
the test "ovn-nbctl: LBs - daemon" fails when it runs the command
"ovn-nbctl lb-add lb0 30.0.0.1a 192.168.10.10:80,192.168.10.20:80". ovn-nbctl
extracts the vip by calling the socket util function 'inet_parse_active()',
and this function blocks when libunbound function ub_resolve() is called
further down. ub_resolve() is a blocking function without timeout and all the
ovs/ovn utilities use this function.
As reported by Timothy Redaelli, the issue can also be reproduced by running
the below commands
$ sudo unshare -mn -- sh -c 'ip addr add dev lo 127.0.0.1 && \
mount --bind /dev/null /etc/resolv.conf && runuser $SUDO_USER'
$ make sandbox SANDBOXFLAGS="--ovn"
$ ovn-nbctl -vsocket_util:off lb-add lb0 30.0.0.1a \
192.168.10.10:80,192.168.10.20:80
To address this issue, this patch adds a new bool argument 'resolve_host' to
the function inet_parse_active() to resolve the host only if it is 'true'.
ovn-nbctl/ovn-northd will pass 'false' when it calls this function to parse
the load balancer values.
Zak Whittington [Fri, 2 Nov 2018 22:25:29 +0000 (15:25 -0700)]
documentation: man vswitchd.conf.db(5) updated flow-restore-wait
Commit 7ed73428a changed the behavior of flow-restore-wait to
also prevent the switch from connecting to controllers in the
controller table, but failed to update the man page documentation
generated by vswitchd/vswitch.xml to reflect this.
This commit adds that documentation.
Signed-off-by: Zak Whittington <zwhitt.vmware@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
When there are both pop and push ethernet header actions among the
actions to be applied to a packet, an unexpected EINVAL (Invalid
argument) error is obtained. This is due to mac_proto not being reset
correctly when those actions are validated.
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html Fixes: 91820da6ae85 ("openvswitch: add Ethernet push and pop actions") Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047554.html Fixes: 6fcecb85ab ("datapath: add Ethernet push and pop actions") Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 1 Nov 2018 15:06:32 +0000 (08:06 -0700)]
checkpatch: Speed up checking when spell checking not enabled.
On my machine it takes almost a second for enchant to read its dictionary.
This time is wasted when spell checking is not enabled. This commit makes
checkpatch read the dictionary only when it will be used.
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 1 Nov 2018 22:05:31 +0000 (15:05 -0700)]
ofp-actions: Let parse_UNROLL_XLATE return error message instead of aborting program
Currently, if unroll_xlate is passed to ovs-ofctl as one of actions,
let say 'ovs-ofctl add-flow br0 in_port=1,actions=unroll_xlate',
ovs-ofctl will crash. This patch fixes it by returning an error
message.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11184 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
projects / ovs.git / log