Ilya Maximets [Thu, 20 Dec 2018 17:23:12 +0000 (20:23 +0300)]
ofproto-dpif.at: Make sFlow sampling tests invariant to IP version.
sflow.log reports the first ip address of the 'loopback' interface.
It could be different on different systems. For example, on FreeBSD
IPv6 [::1] address goes first despite of IPv4 127.0.0.1 on Linux.
Let's just replace it to IPv4 always to make tests work.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Thu, 20 Dec 2018 17:31:39 +0000 (20:31 +0300)]
ofproto-macros.at: Ignore attempts to open '127.0.0.1' as a device.
While configuring sFlow agent OVS tries to treat the value as a name
of the interface at first, after that it tries to treat it as an ip
address. While trying to create netdev from the 'agent', netdev-bsd
calls 'netdev_get_flags()' which produces following warning:
failed to get flags for network device 127.0.0.1
This does not happen with netdev-linux because it uses its own
implementation of 'get_flags' while creating the netdev.
Let's just ignore the warning for sFlow tests.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Thu, 20 Dec 2018 17:35:57 +0000 (20:35 +0300)]
python: Catch setsockopt exceptions for TCP stream.
'sock.setsockopt' could throw exceptions. For example, if non-blocking
connection failed before the call:
Traceback (most recent call last):
File "../.././test-ovsdb.py", line 896, in <module>
main(sys.argv)
File "../.././test-ovsdb.py", line 891, in main
func(*args)
File "../.././test-ovsdb.py", line 604, in do_idl
ovs.stream.Stream.open(r))
File "/root/git_/ovs/python/ovs/stream.py", line 190, in open
error, sock = cls._open(suffix, dscp)
File "/root/git_/ovs/python/ovs/stream.py", line 744, in _open
sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
File "/usr/local/lib/python2.7/socket.py", line 228, in meth
return getattr(self._sock,name)(*args)
socket.error: [Errno 54] Connection reset by peer
This fixes tests on FreeBSD.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Wed, 19 Dec 2018 15:00:20 +0000 (18:00 +0300)]
odp.at: Make 'sed -i' BSD compatible.
'sed -i' on FreeBSD always expects backup filename extention
passed while GNU version expects it only if specified without
extra space after the '-i'. Let's specify the backup extention
to make BSD sed work.
This fixes test on FreeBSD.
CC: Joe Stringer <joe@ovn.org> Fixes: 07659514c3c1 ("Add support for connection tracking.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 18 Dec 2018 15:56:16 +0000 (18:56 +0300)]
odp-util: Don't try to parse geneve data if not provided.
Despite of linux, 'strtoull' on FreeBSD sets errno to EINVAL in case
of no digits found. This causes odp_flow parsing failure if
there is no geneve data inside it. For example, ovs fails to parse
following flow on FreeBSD:
Ilya Maximets [Tue, 18 Dec 2018 12:54:53 +0000 (15:54 +0300)]
ovn.at: Drop bash specific 'function' keyword.
This keyword is not portable and also optional in bash.
Fixes test on FreeBSD.
CC: Miguel Angel Ajo <majopela@redhat.com> Fixes: 508b7f961bd6 ("ovn: l3ha, make is_chassis_active aware of gateway_chassis") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 18 Dec 2018 16:38:27 +0000 (19:38 +0300)]
stopwatch: Fix qsort comparison function.
Current version is broken because it converts first argument to
integer and after that substracts the duoble value. At the end
the result converted to integer again.
This does not cause unit test failures because qsort on linux
accidentially makes right order. On FreeBSD this leads to the
test failure:
Ilya Maximets [Tue, 18 Dec 2018 11:57:24 +0000 (14:57 +0300)]
tests: Don't pass TESTSUITEFLAGS while re-checking.
This fixes 'RECHECK=yes' functionality in case of configured test
range. For example, before the patch following cmdline will result in
running all of the 1000 tests re-checking in case of any failure:
make -j8 check TESTSUITEFLAGS='1000-1999' RECHECK=yes
This happens because ranges and pattern matching options has higher
priority than the '--recheck'.
With patch, only failed tests from the range will be re-checked.
With this patch applied we're dropping support of '--verbose' and
'--trace' options while re-check, but, IMHO, these options makes
sense mostly while debugging individual tests and not much usable
while running recheck of a whole testsuite.
'--jobs' we're resetting each time anyway.
Not sure if someone overrides default '--directory'. For me it looks
not very useful. Changing the color mode also looks not much
profitable.
OTOH, re-checking the ranges or keyword matched tests is very useful,
for example, if you're trying to split up single testsuite check in
a few independent CI jobs.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Mon, 17 Dec 2018 22:43:13 +0000 (14:43 -0800)]
conntrack: Enforce conn_type for flush tuple.
The user should only reference a conntrack entry by the forward
direction context, as per 'conntrack_flush()', enforce this by
checking for 'default' conn_type. The likelihood of a user
not using the original tuple is low, but it should be guarded
against, logged and documented.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Mon, 17 Dec 2018 22:43:12 +0000 (14:43 -0800)]
conntrack: Check all addresses for ephemeral ports.
When fallback to ephemeral ports triggers to find a NAT translation,
it may happen that the full address range is not explored; i.e. if
all ephemeral ports are being used for the address range >= the
first address checked and there are other addresses in the
available range, then they would not be explored for availability.
The likelihood of hitting this condition is rare. The fix is to
reset the first address to the minimum address when starting to
search ephemeral ports. Found by inspection.
Darrell Ball [Mon, 17 Dec 2018 22:43:11 +0000 (14:43 -0800)]
conntrack: Skip ephemeral ports fallback for DNAT.
Ephemeral port fallback is being done for DNAT and the code could be hit in
some special cases and testing configurations. Also good packets are
expected to be persistently dropped in this case, which is not a common
user goal. Regardless, this is incorrect, so filter this out. Also, rename
the variable used for checking whether ephemeral ports need to be checked.
Greg Rose [Tue, 18 Dec 2018 17:43:19 +0000 (09:43 -0800)]
Revert "datapath: Derive IP protocol number for IPv6 later frags"
This reverts commit 2f748bf8016c ("datapath: Derive IP protocol...")
This commit is causing some ipv6 fragmentation errors in some older
kernels. Revert for now and then we can determine how to implement
this patch with appropriate compatability layer changes to prevent
errors on older kernels.
CC: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yi-Hung Wei [Fri, 14 Dec 2018 23:28:55 +0000 (15:28 -0800)]
datapath: compat: Fix static key backport
The original static key backport is based on the upstream
net-next commit 11276d5306b8
("locking/static_keys: Add a new static_key interface").
However, in Canonical's Trusty kernel, it introduced partial static
support which have different definition of some of the macros that
breaks the compatibility code.
For example, in net-next git tree commit 11276d5306b8
("locking/static_keys: Add a new static_key interface").
+ #define DEFINE_STATIC_KEY_TRUE(name) \
+ struct static_key_true name = STATIC_KEY_TRUE_INIT
On the other hand, in Canonical's Trusty git tree commit 13f5d5d1cccb6
("x86/KVM/VMX: Add module argument for L1TF mitigation")
+ #define DEFINE_STATIC_KEY_TRUE(name) \
+ struct static_key name = STATIC_KEY_INIT_TRUE
This commit resolves the ovs kernel module compatibility issue on
Trusty kernel.
VMware-BZ: #2251101 Fixes: 6660a9597a49 ("datapath: compat: Introduce static key support") Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Mon, 17 Dec 2018 16:19:44 +0000 (21:49 +0530)]
ovn: Fix the invalid eth.dst and ip6.dst set by nd_ns action for certain cases.
When an IPv6 packet enters a router pipeline and it needs to be routed via
the nexthop IP address set in the static route, OVN generates an IPv6
Neigh Solicitation request if the nexthop IP is not resolved yet. But
right now, the generated IPv6 Neigh Solicitation packet doesn't set
the eth.dst to the mutlicast address derived from the nexthop and
ip6.dst to the solicited-node multicast address corresponding to the
nexthop address. Instead it generates these values from the actual
ip6.dst of the original packet.
This patch fixes this issue.
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Add 'clone' action to kernel datapath by using existing functions.
When actions within clone don't modify the current flow, the flow
key is not cloned before executing clone actions.
This is a follow up patch for this incomplete work:
https://patchwork.ozlabs.org/patch/722096/
v1 -> v2:
Refactor as advised by reviewer.
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Co-authored-by: Andy Zhou <azhou@ovn.org> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Andy Zhou <azhou@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Sun, 16 Dec 2018 16:25:59 +0000 (08:25 -0800)]
tests: Strip "lt-" prefix when checking daemon process names.
When libtool is in the picture to deal with shared libraries, processes
end up with an "lt-" prefix in their process names. This caused the
process name check in daemon.at to fail. This commit fixes the problem by
stripping off that prefix.
Fixes: d8c6955a03ea ("tests: Simplify and improve the daemon tests.") Reported-by: Timothy Redaelli <tredaelli@redhat.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-December/354574.html Acked-by: Timothy Redaelli <tredaelli@redhat.com> Tested-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
There is a spelling mistake in a net_warn_ratelimited message, fix this.
Signed-off-by: Colin Ian King <colin.king@canonical.com> Reviewed-by: Simon Horman <simon.horman@netronome.com> Signed-off-by: David S. Miller <davem@davemloft.net> CC: Colin Ian King <colin.king@canonical.com> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Load the respective NAT helper module if the flow uses it.
Signed-off-by: Flavio Leitner <fbl@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> CC: Flavio Leitner <fbl@redhat.com> Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
net: ovs: fix return type of ndo_start_xmit function
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com> Signed-off-by: David S. Miller <davem@davemloft.net> CC: YueHaibing <yuehaibing@huawei.com> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
openvswitch: Derive IP protocol number for IPv6 later frags
Currently, OVS only parses the IP protocol number for the first
IPv6 fragment, but sets the IP protocol number for the later fragments
to be NEXTHDF_FRAGMENT. This patch tries to derive the IP protocol
number for the IPV6 later frags so that we can match that.
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> CC: Yi-Hung Wei <yihung.wei@gmail.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
datapath: check tunnel option type in tunnel flags
Upstream commit:
commit 256c87c17c53e60882a43dcf3e98f3bf859eaf6f
Author: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Date: Tue Jun 26 21:39:36 2018 -0700
net: check tunnel option type in tunnel flags
Check the tunnel option type stored in tunnel flags when creating options
for tunnels. Thereby ensuring we do not set geneve, vxlan or erspan tunnel
options on interfaces that are not associated with them.
Make sure all users of the infrastructure set correct flags, for the BPF
helper we have to set all bits to keep backward compatibility.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Signed-off-by: David S. Miller <davem@davemloft.net> CC: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
// 2-factor product with sizeof(type/expression) and identifier or constant.
@@
type TYPE;
expression THING;
identifier COUNT_ID;
constant COUNT_CONST;
@@
// Any remaining multi-factor products, first at least 3-factor products,
// when they're not all constants...
@@
expression E1, E2, E3;
constant C1, C2, C3;
@@
// And then all remaining 2 factors products when they're not all constants,
// keeping sizeof() as the second factor argument.
@@
expression THING, E1, E2;
type TYPE;
constant C1, C2, C3;
@@
Signed-off-by: Kees Cook <keescook@chromium.org> CC: Kees Cook <keescook@chromium.org> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
// 2-factor product with sizeof(type/expression) and identifier or constant.
@@
type TYPE;
expression THING;
identifier COUNT_ID;
constant COUNT_CONST;
@@
// Any remaining multi-factor products, first at least 3-factor products,
// when they're not all constants...
@@
expression E1, E2, E3;
constant C1, C2, C3;
@@
// And then all remaining 2 factors products when they're not all constants,
// keeping sizeof() as the second factor argument.
@@
expression THING, E1, E2;
type TYPE;
constant C1, C2, C3;
@@
Signed-off-by: Kees Cook <keescook@chromium.org> CC: Kees Cook <keescook@chromium.org> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ophir Munk [Mon, 10 Dec 2018 22:15:38 +0000 (22:15 +0000)]
dpdk: Update to use DPDK 18.11.
This commit adds support for DPDK v18.11, it includes the following
changes.
1. Enable compilation and linkage with dpdk 18.11.0
The following dpdk commits which were introduced after dpdk 17.11.x
require OVS updates to accommodate to the dpdk changes.
- ce17edde ("ethdev: introduce Rx queue offloads API")
- ab3ce1e0 ("ethdev: remove old offload API")
- c06ddf96 ("meter: add configuration profile")
- e58638c3 ("ethdev: fix TPID handling in flow API")
- cd8c7c7c ("ethdev: replace bus specific struct with generic dev")
- ac8d22de ("ethdev: flatten RSS configuration in flow API")
2. Limit configured rss hash functions to only those supported
by the eth device.
3. Set default RSS key in struct action_rss_data, required by OVS
commit- e8a2b5bf ("netdev-dpdk: implement flow offload with rte flow")
when configured with "other_config:hw-offload=true".
4. DEV_RX_OFFLOAD_CRC_STRIP has been removed from DPDK 18.11.
DEV_RX_OFFLOAD_KEEP_CRC can now be used to keep the CRC.
Use the correct flag and check it is supported.
5. rte_eth_dev_attach/detach have been removed from DPDK 18.11.
Replace them with rte_dev_probe/remove.
6. Update docs and travis to use DPDK18.11.
This commit squashes the following commits present on the dpdk-latest
branch:
7f021f902bb3 ("netdev-dpdk: Upgrade to dpdk v18.08") 270d9216f1ed ("netdev-dpdk: Set scatter based on capabilities") bef2cdc8f412 ("netdev-dpdk: Fix returning the field of malloced struct.") 73c1a65167fc ("redhat: change variable used for non-root user support") eb485f60ce44 ("dpdk: Update to use DPDK 18.11.")
For credit all authors of the original commits above have been added as
co-authors for this commmit.
Lorenzo Bianconi [Wed, 21 Nov 2018 16:03:39 +0000 (17:03 +0100)]
OVN: add selected mac address to MACAM in update_dynamic_addresses
Add selected dynamic mac address to MACAM in update_dynamic_addresses
and not just in in ipam_add_port_addresses/ipam_insert_lsp_addresses
since the second approach can produce a duplicated L2 address in a
IPv6-only network if ipv6_prefix is provided after logical port creation.
The issue can be triggered with the following reproducer:
Gregory Smith [Wed, 12 Dec 2018 18:46:11 +0000 (10:46 -0800)]
pinctrl: Check requested IP in DHCPREQUEST messages
See RFC 2131, section 4.3.2. When handling a DHCPREQUEST message, the
server should validate that the client's requested IP matches the
offered IP. If not, the server should reply with a DHCPNAK. The client's
requested IP is either specified as the Requested IP Address (option
50), or as the ciaddr, depending on the client's state.
Signed-off-by: Gregory Smith <gasmith@nutanix.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Daniel Alvarez [Tue, 4 Dec 2018 18:14:35 +0000 (19:14 +0100)]
ovn-controller: Inject GARPs to logical switch pipeline to update neighbors
Prior to this patch, GARPs announcing NAT addresses or new VIFs
were sent out to localnet ofport through an output action.
This can lead to problems since local datapaths won't get those
GARPs and ovn-controller won't update MAC_Binding entries (as
upstream switch will not send back the GARP to this port hence
other logical routers won't update their neighbours).
This patch is changing the behavior so that GARPs get injected
to OVN pipeline of the external switch. This way, they'll get
broadcasted to local pipelines and also sent out to the external
network through the localnet port.
Acked-by: Han Zhou <hzhou8@ebay.com> Acked-by: Numan Siddique <nusiddiq@redhat.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047604.html Signed-off-by: Daniel Alvarez <dalvarez@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Daniel Alvarez [Fri, 16 Nov 2018 10:42:29 +0000 (11:42 +0100)]
netdev: Add comment to allow removing a workaround in the future
This patch [0] in glibc fixes an issue which is right now workarounded
in OVS by [1]. I'm adding a comment to indicate that from glibc 2.28
and beyond, the workaround is not needed so that we can eventually
remove it.
ofproto: Return correct error codes from meter_set.
This routine should return enum ofperr, but in a couple of places
doesn't. When adding one more meter when the meter table is full,
this results in an incorrect error message.
Signed-off-by: Tony van der Peet <tony.vanderpeet@alliedtelesis.co.nz> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 11 Dec 2018 14:34:17 +0000 (17:34 +0300)]
cirrus: Add Cirrus CI integration for FreeBSD build
CirrusCI [1] is free for open-sorce projects and provides similar
to TravisCI interfaces. One significant difference is ability
to run tasks on FreeBSD instances.
This patch adds simple configuration file to test OVS build
on two FreeBSD releases with gcc and clang.
Unit tests are commented out because they are broken for now.
To enable the automated checks Cirrus CI application from GitHub
Marketplace should be installed. See details in Quick Start guide [2].
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>