Han Zhou [Sat, 16 Feb 2019 02:49:52 +0000 (18:49 -0800)]
ovn-nbctl: Daemon mode should retry when IDL connection lost.
When creating IDL, "retry" was set to false. However, in daemon
mode, reconnecting upon DB server failure should be transparent
to user. This even impacts HA mode. E.g. in clustered mode, although
IDL tries to connect to next server, but at the first retry the
server fail-over may not be completed yet, and it stops retry after
N (N = number of remotes) times.
This patch makes sure in daemon mode retry is set to true so that
the daemon will automatically retry forever.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
venu iyer [Tue, 15 Jan 2019 01:30:43 +0000 (17:30 -0800)]
Support for multiple VTEP in OVN
OVN uses tunnels to achieve logical network connectivity. The tunnel IP
to be used when communicating with a node is configured using an external_ids
field called "ovn-encap-ip" (and "ovn-encap-type" to indicate the type of
tunnel - geneve, vxlan, stt).
The fact that "ovn-encap-ip" is a single IP is significantly limiting when
used in certain scenarios. Primarly, if we have multiple NICs on a system and
want to assign SR-IOV VFs from different NICs to a guest (as logical ports),
then we'll still end up using the "ovn-encap-ip" to encapsulate traffic from
different VFs. This means we'll end up using only one NIC on the
physical, thereby not maintaining the VF-PF association while also not using
all the physical NICs. It is possible to bond all the NICs and use the
bond IP as the "encap-ip", but bonding multiple NICs has its own limitations,
i.e. NICs supporting OVS flows offload don't work with bonding - this
severly undermines SR-IOV use with OVS (i.e. if all the processing needs
to be done in the host despite giving VFs to guests).
Note: The above uses a NIC that supports OVS with SR-IOV (e.g. Mellanox CX-5) which
uses a "representor" to plug in a VF to the OVS bridge.
This patch enables a list of comma separated IP addresses to be specified in
"ovn-encap-ip", thus allowing the node to be reached via any IP combined with the
"ovn-encap-type" - assuming physical routing allows that. Additionally, it also
introduces an way to specify the encap IP to be used for a logical port (so that
the VF-PF mapping is maintained when traversing the logical path over
a tunnel). A new "encap-ip" external_ids can be configured on an
Interface to indicate this.
On the SB these changes appear as an additional column in port_bindings
as "encap". The encap record for a port points to an encap record
on its chassis. If the port is not explicitly associated with an
encap-ip (using external_ids), the encap record is empty, which
means the preferred tunnel will be used to reach the port's chassis.
The intention is also to have no functional changes in the default case, i.e
when there is only one "ovn-encap-ip".
The changes have been tested with multiple encap-ip addresses, SR-IOV and
for backwards compatibality (in the case where there is only one ovn-encap-ip)
with an OVN SB that doesn't include these changes.
Ilya Maximets [Mon, 18 Feb 2019 15:35:02 +0000 (18:35 +0300)]
checkpatch: Escape range operators inside regex.
' -(' matches a single character in the range between ' ' (index 32)
and '(' (index 40). This leads to the false positive:
WARNING: Line lacks whitespace around operator
#445 FILE: ovsdb/monitor.c:573:
if (--mcs->n_refs == 0) {
Need to escape '-' to have a right behaviour.
This patch additionally escapes all other '-' chars in the similar
regexes and makes them be one per line to ease the review in case of
future changes.
Basic unit tests added.
CC: Joe Stringer <joe@ovn.org> Fixes: 0d7b16daea50 ("checkpatch: Check for infix operator whitespace.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Toms Atteka [Tue, 19 Feb 2019 18:55:02 +0000 (10:55 -0800)]
netlink: added check to prevent netlink attribute overflow
If enough large input is passed to odp_actions_from_string it can
cause netlink attribute to overflow.
Check for buffer size was added to prevent entering this function
and returning appropriate error code.
Basic manual testing was performed.
Reported-by: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12231 Signed-off-by: Toms Atteka <cpp.code.lv@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Mon, 12 Nov 2018 09:28:39 +0000 (12:28 +0300)]
netdev-dpdk: Flow validation refactoring.
* Dropped 'is_all_zero' function, which is equal to 'is_all_zeros'
from util.h .
* util.h added to includes. Includes re-sorted within their blocks.
(it's hard to figure out where to put new one if there is no order.)
Note: linux/if.h depends on sys/socket.h .
* 'ovs_u128_is_zero' used instead of manual checking of fields.
* Code simplified by using direct pointer to 'match->wc.masks'.
* 'sizeof's rewritten to be coding-style complient.
Darrell Ball [Wed, 13 Feb 2019 23:34:21 +0000 (15:34 -0800)]
Userspace datapath: Add fragmentation handling.
Fragmentation handling is added for supporting conntrack.
Both v4 and v6 are supported.
After discussion with several people, I decided to not store
configuration state in the database to be more consistent with
the kernel in future, similarity with other conntrack configuration
which will not be in the database as well and overall simplicity.
Accordingly, fragmentation handling is enabled by default.
This patch enables fragmentation tests for the userspace datapath.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Tue, 12 Feb 2019 02:19:21 +0000 (18:19 -0800)]
monitor: Fix crash when monitor condition adds new columns.
The OVSDB conditional monitor implementation allows many clients
to share same copy of monitored data if the clients are sharing
same tables and columns being monitored, while they can have
different monitor conditions. In monitor conditions they can
have different columns which can be different from the columns
being monitored. So the struct ovsdb_monitor_table maintains the
union of the all the columns being used in any conditions.
The problem of the current implementation is that for each change
set generated, it doesn't maintain any metadata for the number of
columns for the data that has already populated in it. Instead, it
always rely on the n_columns field of the struct ovsdb_monitor_table
to manipulate the data. However, the n_columns in struct
ovsdb_monitor_table can increase (e.g. when a client changes its
condition which involves more columns). So it can result in that
the existing rows in a change set with N columns being later processed
as if it had more than N columns, typically, when the row is freed.
This causes the ovsdb-server crashing (see an example of the
backtrace).
The patch fixes the problem by maintaining n_columns for each
change set, and added a test case which fails without the fix.
(gdb) bt
at lib/ovsdb-data.c:1031
out>, mt=<optimized out>) at ovsdb/monitor.c:320
mt=0x1e7b940) at ovsdb/monitor.c:333
out>, transaction=<optimized out>) at ovsdb/monitor.c:527
initial=<optimized out>, cond_updated=cond_updated@entry=false,
unflushed_=unflushed_@entry=0x20dae70,
condition=<optimized out>, version=<optimized out>) at ovsdb/monitor.c:1156
(m=m@entry=0x20dae40, initial=initial@entry=false) at
ovsdb/jsonrpc-server.c:1655
at ovsdb/jsonrpc-server.c:1729
ovsdb/jsonrpc-server.c:551
ovsdb/jsonrpc-server.c:586
ovsdb/jsonrpc-server.c:401
exiting=0x7ffdb947f76f, run_process=0x0, remotes=0x7ffdb947f7c0,
unixctl=0x1e7a560, all_dbs=0x7ffdb947f800,
jsonrpc=<optimized out>, config=0x7ffdb947f820) at ovsdb/ovsdb-server.c:209
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
DPDK 18.11 builds using the more modern meson build system no more
provide the -ldpdk linker script. Instead it is expected to use
pkgconfig for linker options as well.
This change will set DPDK_LIB from pkg-config (if pkg-config was
available) and since that already carries the whole-archive flags
around the PMDs skips the further wrapping in more whole-archive
if that is already part of DPDK_LIB.
To work reliable in all environments this needs pkg-config 0.29.1.
We want to be able to use PKG_CHECK_MODULES_STATIC which
is not yet available in 0.24. Therefore update pkg.m4
to pkg-config 0.29.1.
This should be backport-safe as these macro files are all versioned.
autoconf is smart enough to check the version if you have it locally,
and if the system's is higher, it will use that one instead.
Acked-by: Luca Boccassi <bluca@debian.org> Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Greg Rose [Tue, 12 Feb 2019 20:37:03 +0000 (12:37 -0800)]
debian: Add libelf-dev dependency for dkms
Newer kernels define CONFIG_UNWINDER_ORC for their kernel configurations
and to build this the kernel compilation requires the libelf-dev
package. Add the dependency to the dkms build requirements.
VMware-BZ: #2287968 Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 12 Feb 2019 13:19:31 +0000 (16:19 +0300)]
acinclude: Check for rte_config.h before checking dependencies.
Current ./configure script shows misleading errors in case of wrong
DPDK path:
# ./configure --with-dpdk=/wrong/path
...
checking whether dpdk datapath is enabled... yes
checking for library containing get_mempolicy... -lnuma
checking for library containing pcap_dump... -lpcap
checking for library containing mnl_attr_put... no
configure: error: unable to find libmnl, install the dependency package
This happens because we're not checking for headers before checking
for dependencies. All the compile attempts fails and script thinks
that we need more dependencies.
With this change script will check for 'rte_config.h' availability
and produce sane error message:
# ./configure --with-dpdk=/wrong/path
...
checking for rte_config.h... no
configure: error: unable to find rte_config.h in /wrong/path
'AC_INCLUDES_DEFAULT' passed explicitly to avoid preprocessor test.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ilya Maximets [Thu, 7 Feb 2019 13:00:51 +0000 (16:00 +0300)]
acinclude: Use NUMA_AWARE_HUGEPAGES too for libnuma check.
This fixes build with NUMA_AWARE_HUGEPAGES enabled and VHOST_NUMA
disabled. This should not be a usual case. But it's possible to
configure DPDK this way.
Darrell Ball [Tue, 5 Feb 2019 00:23:07 +0000 (16:23 -0800)]
conntrack: Exclude l2 padding in 'conn_key_extract()'.
'conn_key_extract()' in userspace conntrack is including L2
(Ethernet) pad bytes for both L3 and L4 sizes. One problem is
any packet with non-zero L2 padding can incorrectly fail L4
checksum validation.
This patch fixes conn_key_extract() by ignoring L2 pad bytes.
Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") CC: Daniele Di Proietto <diproiettod@ovn.org> Co-authored-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Co-authored-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com> Co-authored-by: Nitin Katiyar <nitin.katiyar@ericsson.com> Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Signed-off-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com> Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com> Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Li RongQing [Mon, 11 Feb 2019 02:52:54 +0000 (10:52 +0800)]
conntrack: Remove unnecessary check in process_ftp_ctl_v4
It has been assured that both first and second int from ftp
command are not bigger than 255, so their combination(first
int << 8 +second int) must not bigger than 65535
Co-authored-by: Wang Li <wangli39@baidu.com> Signed-off-by: Wang Li <wangli39@baidu.com> Signed-off-by: Li RongQing <lirongqing@baidu.com> Cc: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Timothy Redaelli [Mon, 11 Feb 2019 18:55:53 +0000 (19:55 +0100)]
rhel: Add an example to specify custom options
Add an example to specify custom options of ovs-vswitchd and
ovsdb-server.
In the example, the log level for file and console destinations is set to dbg.
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Timothy Redaelli [Mon, 11 Feb 2019 18:55:52 +0000 (19:55 +0100)]
ovs-ctl: Permit to specify additional options
Currently using ovs-ctl is not possible to specify additional options
for ovs-vswitchd and ovsdb-server (for example to specify a different
loglevel during daemon startup).
This patch adds --ovs-vswitchd-options and --ovsdb-server-options
options to ovs-ctl in order to specify the additional options.
Due to word splitting it may not be possible to specify an option that
includes whitespaces.
Reported-at: https://bugzilla.redhat.com/1664794 Reported-by: Matt Flusche <mflusche@redhat.com> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Daniel Alvarez [Mon, 11 Feb 2019 16:06:14 +0000 (17:06 +0100)]
ovn: change load balancer references to weak in NB schema
When a load balancer is added to multiple logical switches
and routers it has be to be removed from all of them before
being able to delete due to the current 'strong' reference
in the NB schema.
By changing it to 'weak', users can simply remove the load
balancer without having to remove all the references manually.
In particular, this will make things easier for networking-ovn,
the OpenStack integration project as it'll save some
calculations upon load balancer deletion.
The update path has been successfully from the previous version
of the schema.
Acked-by: Lucas Alvares Gomes <lucasagomes@gmail.com> Signed-off-by: Daniel Alvarez <dalvarez@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Fri, 8 Feb 2019 16:48:56 +0000 (19:48 +0300)]
travis: Run testsuite with desired options.
'make distcheck' executes it's own './configure' without any options
provided to the script. This means that in current configuration
Travis CI always re-builds and runs testsuite on a defualt binaries.
i.e. we're not checking testsuite with DPDK, not checking testsuite
with '--enable-shared' and not checking it with '-ljemalloc'.
We just 8 times running the testsuite without arguments. Only compiler
changes (gcc or clang) because CC is exported by Travis.
This patch reorders the commands in the build script and provides
'DISTCHECK_CONFIGURE_FLAGS' to force 'make distcheck' using our
desired configuration.
Another issue that addressed here is that we will no longe build
twice in case of TESTSUITE.
For linking inside the distcheck we also need to provide absulute path
to DPDK libraries.
'configure' executed before 'distcheck' to have a Makefile target.
It's executed without arguments because 'configure' inside the
'distcheck' will fail if we'll use sparse-wrapped CC.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Fri, 8 Feb 2019 16:48:55 +0000 (19:48 +0300)]
automake: Clean up cxxtest.cc.
'distcheck' complains on some configurations:
ERROR: files left in build directory after distclean:
./include/openvswitch/cxxtest.cc
CC: Ben Pfaff <blp@ovn.org> Fixes: 994bfc298502 ("Automatically verify that OVS header files work OK in C++ also.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Adi Nissim [Thu, 17 Jan 2019 15:41:36 +0000 (17:41 +0200)]
lib/tc: Support optional tunnel id
Currently the TC tunnel_key action is always
initialized with the given tunnel id value. However,
some tunneling protocols define the tunnel id as an optional field.
This patch initializes the id field of tunnel_key:set and tunnel_key:unset
only if a value is provided.
In the case that a tunnel key value is not provided by the user
the key flag will not be set.
Signed-off-by: Adi Nissim <adin@mellanox.com> Acked-by: Paul Blakey <paulb@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Ilya Maximets [Thu, 7 Feb 2019 12:20:20 +0000 (15:20 +0300)]
acinclude: Drop DPDK_EXTRA_LIB variable.
AC_SEARCH_LIBS enables the libraries itself:
checking for library containing get_mempolicy... -lnuma
checking for library containing pcap_dump... -lpcap
So, they are available in LIBS. No need to add them twice.
Also, DPDK_EXTRA_LIB doesn't even work, because each check overwrites
the variable instead of appending the new library. It was first time
misused while making libnuma optional and copy-pasted to several places
after that.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ben Pfaff [Tue, 9 Oct 2018 18:15:00 +0000 (11:15 -0700)]
ofproto: Don't always treat passive controllers as "equal".
If a passive controller chooses to configure itself as a slave controller,
I don't know a reason why it should be considered "equal" for some
purposes.
Acked-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 26 Oct 2018 22:53:55 +0000 (15:53 -0700)]
vswitchd: Allow user to configure controllers as "primary" or "service".
Normally it makes sense for an active connection to be primary and a
passive connection to be a service connection, but I've run into a corner
case where it is better for a passive connection to be a primary
connection. This specific case is for use with OFtest, which expects to be
a primary controller. However, it also wants to reconnect frequently,
which is slow for active connections because of the backoff; by
configuring a passive, primary controller, OFtest can reconnect as
frequently and as quickly as it wants, making the overall test much faster.
Acked-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 18 Jan 2019 00:20:20 +0000 (16:20 -0800)]
Remove support for OpenFlow 1.6 (draft).
ONF abandoned the OpenFlow specification, so that OpenFlow 1.6 will never
be completed. It did not contain much in the way of useful features, so
remove what support Open vSwitch already had.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Ben Pfaff [Wed, 24 Oct 2018 21:23:38 +0000 (14:23 -0700)]
connmgr: Make treatment of active and passive connections more uniform.
Until now, connmgr has handled active and passive OpenFlow connections in
quite different ways. Any active connection, whether it was currently
connected or not, was always maintained as an ofconn. Whenever such a
connection (re)connected, its settings were cleared. On the other hand,
passive connections had a separate listener which created an ofconn when
a new connection came in, and these ofconns would be deleted when such a
connection was closed. This approach is inelegant and has occasionally
led to bugs when reconnection didn't clear all of the state that it
should have.
There's another motivation here. Currently, active connections are
always primary controllers and passive connections are always service
controllers (as documented in ovs-vswitchd.conf.db(5)). Sometimes it would
be useful to have passive primary controllers (maybe active service
controllers too but I haven't personally run into that use case). As is,
this is difficult to implement because there is so much different code in
use between active and passive connections. This commit will make it
easier.
Ilya Maximets [Tue, 5 Feb 2019 07:16:04 +0000 (10:16 +0300)]
travis: Speed up linux kernel downloads.
CDN links are much faster in average. https://www.kernel.org/
links shows usually less than 10 MB/s, while https://cdn.kernel.org/
could give up to 200 MB/s and usually shows speeds much higher than
10 MB/s. Also, 'xz' archives are 30-50 MB smaller than gzip ones.
It takes a bit more time to unpack them, but it's negligible in
compare with download time.
For exmaple,
linux-3.16.54.tar.gz - 122064395 (116M)
linux-3.16.54.tar.xz - 81057528 (77M)
'xz' archive download via CDN link is the default way for kernel
downloading that provided by the kernel.org.
psiyengar [Thu, 17 Jan 2019 00:53:52 +0000 (16:53 -0800)]
Fix OpenFlow v1.3.4 Conf test failures: 430.500, 430.510
This commit adds additional verification to nx_pull_header__()
in lib/nx-match.c to distinguish between bad match and bad action
header conditions and return the appropriate error type/code.
Signed-off-by: Prashanth Iyengar <prashanth_iyengar@alliedtelesis.com> Reviewed-by: Tony van der Peet <tony.vanderpeet@alliedtelesis.co.nz> Reviewed-by: Rahul Gupta <Rahul_Gupta@alliedtelesis.com>
Ilya Maximets [Tue, 29 Jan 2019 13:09:55 +0000 (16:09 +0300)]
skiplist: Drop data comparison in skiplist_delete.
Current version of 'skiplist_delete' uses data comparator to check
if the node that we're removing exists on current level. i.e. our
node 'x' is the next of update[i] on the level i.
But it's enough to just check pointers for that purpose.
Here is the small example of how the data structures looks at
this moment:
i a b c x d e f
0 [ ]>[ ]>[*] ---> [ ] ---> [#]>[ ]>[ ]
1 [ ]>[*] -------> [ ] -------> [#]>[ ]
2 [ ]>[*] -------> [ ] -----------> [#]
3 [ ]>[*] ------------------------> [ ]
4 [*] ----------------------------> [ ]
0 1 2 3 4
update[] = { c, b, b, b, a }
x.forward[] = { d, e, f }
c.forward[0] = x
b.forward[1] = x
b.forward[2] = x
b.forward[3] = f
a.forward[4] = f
Target:
i a b c d e f
0 [ ]>[ ]>[*] ------------> [#]>[ ]>[ ]
1 [ ]>[*] --------------------> [#]>[ ]
2 [ ]>[*] ------------------------> [#]
3 [ ]>[*] ------------------------> [ ]
4 [*] ----------------------------> [ ]
c.forward[0] = x.forward[0] = d
b.forward[1] = x.forward[1] = e
b.forward[2] = x.forward[2] = f
b.forward[3] = f
a.forward[4] = f
i.e. we're updating forward pointers while update[i].forward[i] == x.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 25 Jan 2019 20:22:06 +0000 (12:22 -0800)]
skiplist: Remove 'height' from skiplist_node.
This member was write-only: it was initialized and never used later on.
Thanks to Esteban Rodriguez Betancourt <estebarb@hpe.com> for the
following additional rationale:
In this case you are right, the "height" member is not only not
used, it is in fact not required, and can be safely removed,
without causing security issues.
The code can't read past the end of the 'forward' array because
the skiplist "level" member, that specifies the maximum height of
the whole skiplist.
The "level" field is updated in insertions and deletions, so that
in insertion the root node will point to the newly created item
(if there isn't a list there yet). At the deletions, if the
deleted item is the last one at that height then the root is
modified to point to NULL at that height, and the whole skiplist
height is decremented.
For the forward_to case:
- If a node is found in a list of level/height N, then it has
height N (that's why it was inserted in that list)
- forward_to travels throught nodes in the same level, so it is
safe, as it doesn't go up.
- If a node has height N, then it belongs to all the lists
initiated at root->forward[n, n-1 ,n-2, ..., 0]
- forward_to may go to lower levels, but that is safe, because of
previous point.
So, the protection is given by the "level" field in skiplist root
node, and it is enough to guarantee that the code won't go off
limits at 'forward' array. But yes, the height field is unused,
unneeded, and can be removed safely.
CC: Esteban Rodriguez Betancourt <estebarb@hpe.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Tue, 5 Feb 2019 00:02:15 +0000 (16:02 -0800)]
conntrack: Fix possibly uninitialized memory.
There are a few cases where struct 'conn_key' padding may be unspecified
according to the C standard. Practically, it seems implementations don't
have issue, but it is better to be safe. The code paths modified are not
hot ones. Fix this by doing a memcpy in these cases in lieu of a
structure copy.
Found by inspection.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ashish Varma [Mon, 4 Feb 2019 23:34:34 +0000 (15:34 -0800)]
ofproto-dpif-trace: Fix for the segmentation fault in ofproto_trace().
Added the check for NULL in "next_ct_states" argument passed to the
"ofproto_trace()" function. Under normal scenario, this is non-NULL. A NULL
"next_ct_states" argument is passed from the "upcall_xlate()" function on
encountering XLATE_RECURSION_TOO_DEEP or XLATE_TOO_MANY_RESUBMITS error.
VMware-BZ: #2282287 Signed-off-by: Ashish Varma <ashishvarma.ovs@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
The previous commit fa642f08839b
("openvswitch: Derive IP protocol number for IPv6 later frags")
introduces IP protocol number parsing for IPv6 later frags that can mess
up the network header length calculation logic, i.e. nh_len < 0.
However, the network header length calculation is mainly for deriving
the transport layer header in the key extraction process which the later
fragment does not apply.
Therefore, this commit skips the network header length calculation to
fix the issue.
Reported-by: Chris Mi <chrism@mellanox.com> Reported-by: Greg Rose <gvrose8192@gmail.com> Fixes: fa642f08839b ("openvswitch: Derive IP protocol number for IPv6 later frags") Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Fixes: 9a4ab6da01f7 ("datapath: Derive IP protocol number for IPv6 later frags") Cc: Yi-Hung Wei <yihung.wei@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> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
openvswitch: Avoid OOB read when parsing flow nlattrs
For nested and variable attributes, the expected length of an attribute
is not known and marked by a negative number. This results in an OOB
read when the expected length is later used to check if the attribute is
all zeros. Fix this by using the actual length of the attribute rather
than the expected length.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Cc: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Tue, 29 Jan 2019 22:18:08 +0000 (14:18 -0800)]
datapath: Add support for kernel 4.18.x
No code changing is necessary to support 4.18.x.
Only one kernel test failed and it is in the process of being fixed.
Updated .travis.yml to include 4.18.x and also use latest 4.17 version.
Updated test files to test 4.18 kernel.
Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 31 Jan 2019 23:10:00 +0000 (15:10 -0800)]
dpif-netlink: Fix a bug that causes duplicate key error in datapath
Kmod tests 122 and 123 failed and kernel reports a "Duplicate key of
type 6" error. Further debugging reveals that nl_attr_find__() should
start looking for OVS_KEY_ATTR_ETHERTYPE from offset returned by
a previous called nl_msg_start_nested(). This patch fixes it.
Tests 122 and 123 were skipped by kernel 4.15 and older versions.
Kernel 4.16 and later kernels start showing this failure.
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Fri, 1 Feb 2019 17:56:53 +0000 (09:56 -0800)]
test: Fix failed test "flow resume with geneve tun_metadata"
Test "flow resume with geneve tun_metadata" failed because there is
no controller running to handle the continuation message. A previous
commit deleted the line that starts ovs-ofctl as a controller in
order to avoid a race condition on monitor log. This patch adds
back this line but omits the log file because this test doesn't
depend on the log file.
Fixes: e8833217914f9c071c49 ("system-traffic.at: avoid a race condition on monitor log") Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> CC: David Marchand <david.marchand@redhat.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Support for match & set ICMPv6 reserved and options type fields
Currently OVS supports all ARP protocol fields as OXM match fields to
implement the relevant ARP procedures for IPv4. This includes support
for matching copying and setting ARP fields. In IPv6 ARP has been
replaced by ICMPv6 neighbor discovery (ND) procedures, neighbor
advertisement and neighbor solicitation.
The support for ICMPv6 fields in OVS is not complete for the use cases
equivalent to ARP in IPv4. OVS lacks support for matching, copying and
setting the “ND option type” and “ND reserved” fields. Without these user
cannot implement all ICMPv6 ND procedures for IPv6 support.
This commit adds additional OXM fields to OVS for ICMPv6 “ND option type“
and ICMPv6 “ND reserved” using the OXM extension mechanism. This allows
support for parsing these fields from an ICMPv6 packet header and extending
the OpenFlow protocol with specifications for these new OXM fields for
matching, copying and setting.
Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Co-authored-by: Ashvin Lakshmikantha <ashvin.lakshmikantha@ericsson.com> Signed-off-by: Ashvin Lakshmikantha <ashvin.lakshmikantha@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Fri, 1 Feb 2019 23:56:04 +0000 (15:56 -0800)]
odp-util: Stop parse odp actions if nlattr is overflow
`encap = nl_msg_start_nested(key, OVS_KEY_ATTR_ENCAP)` ensures that
key->size >= (encap + NLA_HDRLEN), so the `if` statement is safe.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11306 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Sat, 2 Feb 2019 00:44:26 +0000 (16:44 -0800)]
ofp-actions: Set an action depth limit to prevent stackoverflow by ofpacts_parse
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12557 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Hyong Youb Kim [Sat, 2 Feb 2019 07:19:40 +0000 (23:19 -0800)]
ovs-tcpdump: Fix an undefined variable
Run ovs-tcpdump without --span, and it throws the following
exception. Define mirror_select_all to avoid the error.
Traceback (most recent call last):
File "/usr/local/bin/ovs-tcpdump", line 488, in <module>
main()
File "/usr/local/bin/ovs-tcpdump", line 454, in main
mirror_select_all)
UnboundLocalError: local variable 'mirror_select_all' referenced before assignment
Fixes: 0475db71c650 ("ovs-tcpdump: Add --span to mirror all ports on bridge.") Acked-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Mon, 4 Feb 2019 16:50:31 +0000 (22:20 +0530)]
ovn-controller: Fix chassisredirect port flapping when ovs-vswitchd crashes
On a chassis when ovs-vswitchd crashes for some reason, the BFD status doesn't
get updated in the ovs db. ovn-controller will be reading the old BFD status
even though ovs-vswitchd is crashed. This results in the chassiredirect port
claim flapping between the master chassis and the chasiss with the next higher
priority if ovs-vswitchd crashes in the master chassis.
All the other chassis notices the BFD status down with the master chassis
and hence the next higher priority claims the port. But according to
the master chassis, the BFD status is fine and it again claims back the
chassisredirect port. And this results in flapping. The issue gets resolved
when ovs-vswitchd comes back but until then it leads to lot of SB DB
transactions and high CPU usage in ovn-controller's.
This patch fixes the issue by checking the OF connection status of the
ovn-controller with ovs-vswitchd and calculates the active bfd tunnels
only if it's connected.
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Fri, 1 Feb 2019 07:35:40 +0000 (23:35 -0800)]
conntrack: fix ftp ipv4 address substitution.
When replacing the ipv4 address in repl_ftp_v4_addr(), the remaining size
was incorrectly calculated which could lead to the wrong replacement
adjustment.
This goes unnoticed most of the time, unless you choose carefully your
initial and replacement addresses.
Example fail address combination with 10.1.1.200 DNAT'd to 10.1.100.1.
Fix this by doing something similar to V6 and also splicing out common
code for better coverage and maintainability.
A test is updated to exercise different initial and replacement addresses
and another test is added.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.") Reported-by: David Marchand <david.marchand@redhat.com> Reviewed-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 29 Jan 2019 08:11:50 +0000 (11:11 +0300)]
dpdk: Limit DPDK memory usage.
Since 18.05 release, DPDK moved to dynamic memory model in which
hugepages could be allocated on demand. At the same time '--socket-mem'
option was re-defined as a size of pre-allocated memory, i.e. memory
that should be allocated at startup and could not be freed.
So, DPDK with a new memory model could allocate more hugepage memory
than specified in '--socket-mem' or '-m' options.
This change adds new configurable 'other_config:dpdk-socket-limit'
which could be used to limit the ammount of memory DPDK could use.
It uses new DPDK option '--socket-limit'.
Ex.:
ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-limit="1024,1024"
Also, in order to preserve old behaviour, if '--socket-limit' is not
specified, it will be defaulted to the amount of memory specified by
'--socket-mem' option, i.e. OVS will not be able to allocate more.
This is needed, for example, to disallow OVS to allocate more memory
than reserved for it by Nova in OpenStack installations.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
lib/tc: add set ipv6 traffic class action offload via pedit
Extend ovs-tc translation by allowing non-byte-aligned fields
for set actions. Use new boundary shifts and add set ipv6 traffic
class action offload via pedit.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> Signed-off-by: Louis Peens <louis.peens@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
lib/tc: add set ipv4 dscp and ecn action offload via pedit
Add setting of ipv4 dscp and ecn fields in tc offload using pedit.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> Signed-off-by: Louis Peens <louis.peens@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
lib/tc: fix 32 bits shift for pedit offset calculation
pedit allows setting entire words with an optional mask and OVS
makes use of such masks to allow setting fields that do not span
entire words. One mask for leading bytes that should not be
updated and another mask for trailing bytes that should not be
updated. The masks are created using bit shifts.
In the case of the mask to omit trailing bytes a right bit shift
is used. Currently the code can produce shifts of 1, 2, 3 or 4
bytes (8, 16, 24 or 32 bits) based on the alignment of the end
of field being set.
However, a shift of 32 bits on a 32bit value is undefined.
As it stands the code relies on the result of UINT32_MAX >> 32
being UINT32_MAX. Or in other words a mask that results in the
pedit action setting all bytes of the word under operation.
This patch adjusts the code to use a shift of 0 for this case,
which gives the same result as the undefined behaviour that was
relied on, and appears logically correct as the desire is for no
trailing bytes (or bits!) to be omitted from the set action.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
lib/tc: make pedit mask calculations byte order agnostic
pedit allows setting entire words with an optional mask and OVS
makes use of such masks to allow setting fields that do not span
entire words.
The struct tc_pedit_key structure, which is part of the kernel
ABI, uses host byte order fields to store the mask and value for
a pedit action, however, these fields contain values in network
byte order.
In order to allow static analysis tools to check for endianness
problems this patch adds a local version of struct tc_pedit_key
which uses big endian types and refactors the relevant code as
appropriate.
In the course of making this change it became apparent that the
calculation of masks was occurring using host byte order although
the values are in network byte order. This patch also fixes that
problem by shifting values in host byte order and then converting
them to network byte order. It is believe this fixes a bug on big
endian systems although we are not in a position to test that.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Fix test 'testing ovn -- IP packet buffering' on Windows
The test fails on Windows because of:
<--cut-->
ovn-nbctl: sw0: invalid network address: 2001;1\64
ovn-nbctl: sw1: invalid network address: 2002;1\64
<--cut-->
This is due to the fact msys converts '::1' into ';1'.
Use IPv6 long form instead of its short variant.
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> Acked-by: Numan Siddique <nusiddiq@redhat.com> Acked-by: Ben Pfaff <blp@ovn.org>
Anand Kumar [Fri, 11 Jan 2019 00:45:24 +0000 (16:45 -0800)]
datapath-windows: Add support for 'OVS_KEY_ATTR_ENCAP' key attribute.
Add a new structure in l2 header to accomodate vlan header,
based of commit "d7efce7beff25052bd9083419200e1a47f0d6066
datapath: 802.1AD Flow handling, actions, vlan parsing, netlink attributes"
Also reset vlan header in flow key, after deleting vlan tag from nbl
With this change a sample vlan flow would look like,
eth(src=0a:ea:8a:24:03:86,dst=0a:cd:fa:4d:15:5c),in_port(3),eth_type(0x8100),
vlan(vid=2239,pcp=0),encap(eth_type(0x0800),ipv4(src=13.12.11.149,dst=13.12.11.107,
proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0))
Signed-off-by: Anand Kumar <kumaranand@vmware.com> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
This patch introduced a regression in OSP environments using internal
ports in other netns. Their networking configuration is lost when
the service is restarted because the ports are recreated now.
Before the patch it checked using netlink if the port with a specific
"name" was already there. The check is a lookup in all ports attached
to the DP regardless of the port's netns.
After the patch it relies on the kernel to identify that situation.
Unfortunately the only protection there is register_netdevice() which
fails only if the port with that name exists in the current netns.
If the port is in another netns, it will get a new dp_port and because
of that userspace will delete the old port. At this point the original
port is gone from the other netns and there a fresh port in the current
netns.
Signed-off-by: Flavio Leitner <fbl@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
The original commit 7521e0cf9e88 ("ofproto-dpif: Let the dpif report
when a port is a duplicate.") relies on the kernel to check if the
port exists or not. However, the current kernel code doesn't handle
when the port is moved to another network namespace.
Signed-off-by: Flavio Leitner <fbl@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
The original commit 7521e0cf9e88 ("ofproto-dpif: Let the dpif report
when a port is a duplicate.") relies on the kernel to check if the
port exists or not. However, the current kernel code doesn't handle
when the port is moved to another network namespace.
Signed-off-by: Flavio Leitner <fbl@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
projects / mirror_ovs.git / log