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>
Yifeng Sun [Thu, 1 Nov 2018 18:39:59 +0000 (11:39 -0700)]
oss-fuzz: Free error string in ofctl_parse_flow
This patch frees the leaked error string to stop oss-fuzz from
complaining.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11161 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 1 Nov 2018 18:51:21 +0000 (11:51 -0700)]
oss-fuzz: Use unsigned for left shift in ofctl_parse_flows__
Left shift int (1 here) can result in a negative value. This is an undefined
behavior according to ISO C99 (6.5.7).
The error message reported by oss-fuzz is:
runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
This patch fixes it by changing signed int to unsigned int.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11166 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Tiago Lam [Fri, 2 Nov 2018 09:06:34 +0000 (09:06 +0000)]
dp-packet: Fix allocated size on DPDK init.
When enabled with DPDK OvS deals with two types of packets, the ones
coming from the mempool and the ones locally created by OvS - which are
copied to mempool mbufs before output. In the latter, the space is
allocated from the system, while in the former the mbufs are allocated
from a mempool, which takes care of initialising them appropriately.
In the current implementation, during mempool's initialisation of mbufs,
dp_packet_set_allocated() is called from dp_packet_init_dpdk() without
considering that the allocated space, in the case of multi-segment
mbufs, might be greater than a single mbuf. Furthermore, given that
dp_packet_init_dpdk() is on the code path that's called upon mempool's
initialisation, a call to dp_packet_set_allocated() is redundant, since
mempool takes care of initialising it.
To fix this, dp_packet_set_allocated() is no longer called after
initialisation of a mempool, only in dp_packet_init__(), which is still
called by OvS when initialising locally created packets.
Mark Kavanagh [Fri, 2 Nov 2018 09:06:33 +0000 (09:06 +0000)]
dp-packet: Init specific mbuf fields.
dp_packets are created using xmalloc(); in the case of OvS-DPDK, it's
possible the the resultant mbuf portion of the dp_packet contains
random data. For some mbuf fields, specifically those related to
multi-segment mbufs and/or offload features, random values may cause
unexpected behaviour, should the dp_packet's contents be later copied
to a DPDK mbuf. It is critical therefore, that these fields should be
initialized to 0.
This patch ensures that the following mbuf fields are initialized to
appropriate values on creation of a new dp_packet:
- ol_flags=0
- nb_segs=1
- tx_offload=0
- packet_type=0
- next=NULL
Adapted from an idea by Michael Qiu <qiudayu@chinac.com>:
https://patchwork.ozlabs.org/patch/777570/
Mark Kavanagh [Fri, 2 Nov 2018 09:06:32 +0000 (09:06 +0000)]
netdev-dpdk: fix mbuf sizing
There are numerous factors that must be considered when calculating
the size of an mbuf:
- the data portion of the mbuf must be sized in accordance With Rx
buffer alignment (typically 1024B). So, for example, in order to
successfully receive and capture a 1500B packet, mbufs with a
data portion of size 2048B must be used.
- in OvS, the elements that comprise an mbuf are:
* the dp packet, which includes a struct rte mbuf (704B)
* RTE_PKTMBUF_HEADROOM (128B)
* packet data (aligned to 1k, as previously described)
* RTE_PKTMBUF_TAILROOM (typically 0)
Some PMDs require that the total mbuf size (i.e. the total sum of all
of the above-listed components' lengths) is cache-aligned. To satisfy
this requirement, it may be necessary to round up the total mbuf size
with respect to cacheline size. In doing so, it's possible that the
dp_packet's data portion is inadvertently increased in size, such that
it no longer adheres to Rx buffer alignment. Consequently, the
following property of the mbuf no longer holds true:
mbuf.data_len == mbuf.buf_len - mbuf.data_off
This creates a problem in the case of multi-segment mbufs, where that
assumption is assumed to be true for all but the final segment in an
mbuf chain. Resolve this issue by adjusting the size of the mbuf's
private data portion, as opposed to the packet data portion when
aligning mbuf size to cachelines.
Co-authored-by: Tiago Lam <tiago.lam@intel.com> Fixes: 4be4d22 ("netdev-dpdk: clean up mbuf initialization") Fixes: 31b88c9 ("netdev-dpdk: round up mbuf_size to cache_line_size") CC: Santosh Shukla <santosh.shukla@caviumnetworks.com> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com> Signed-off-by: Tiago Lam <tiago.lam@intel.com> Acked-by: Santosh Shukla <santosh.shukla@caviumnetworks.com> Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ian Stokes [Thu, 25 Oct 2018 11:03:57 +0000 (12:03 +0100)]
netdev-dpdk: Add link speed to get_status().
Report the link speed of the device in netdev_dpdk_get_status()
function.
Link speed is already reported as part of the netdev_get_features()
function. However only link speeds defined in the OpenFlow specs are
supported so speeds such as 25 Gbps etc. are not shown. The link
speed for the device is available in Mbps in rte_eth_link.
This commit converts the link speed for a given dpdk device to an
easy to read string and reports it in get_status().
Ian Stokes [Wed, 24 Oct 2018 10:35:17 +0000 (11:35 +0100)]
netdev-dpdk: Fix netdev_dpdk_get_features().
This commit fixes netdev_dpdk_get_features() by initializing a bitmap
that represents current features to zero and accounting for non defined
link speed values in the OpenFlow spec.
The current approach for retrieving netdev dpdk features uses a
pointer allocated in the stack without being initialized. As such there
is no guarantee that the bitmap will be accurate. Fix this by declaring
and initializing local variable 'feature' to be used when building the
bitmap, with its value then assigned to the pointer. Also account for
link speeds not defined in the OpenFlow spec by defaulting to
NETDEV_F_OTHER for undefined link speeds.
Ilya Maximets [Wed, 31 Oct 2018 15:44:09 +0000 (18:44 +0300)]
dpif-netdev: End the quiescent state for flow offloading thread.
Flow offloading thread uses concurrent hash maps which are
based on rcu protected variables. It must use them while in
active state. Working in a quiescent state could cause
segmentation faults because of possible cmap internal
structure changes.
Fixes: 02bb2824e51d ("dpif-netdev: do hw flow offload in a thread") Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ian Stokes [Thu, 25 Oct 2018 16:50:44 +0000 (17:50 +0100)]
Docs: Remove HWOL DPDK limitation.
Partial offload support was added to OVS DPDK in OVS 2.10. As such
remove the limitation that OVS DPDK does not support HWOL from the
DPDK install documentation.
Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ian Stokes [Fri, 19 Oct 2018 13:30:15 +0000 (14:30 +0100)]
Docs: Remove zero-copy QEMU limitation.
Remove note regarding zero-copy compatibility with QEMU >= 2.7.
When zero-copy was introduced to OVS it was incompatible with QEMU >=
2.7. This issue has since been fixed in DPDK with commit 803aeecef123 ("vhost: fix dequeue zero copy with virtio1") and
backported to DPDK LTS branches. Remove the reference to this
issue in the zero-copy documentation.
Ilya Maximets [Fri, 19 Oct 2018 13:51:15 +0000 (16:51 +0300)]
netdev-dpdk: Dump flow patterns only if debug enabled.
No need to waste time for fields checking in case DBG disabled.
Additionally sequence of prints replaced with single print
to avoid output interrupting by other log messages.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ilya Maximets [Thu, 18 Oct 2018 13:29:21 +0000 (16:29 +0300)]
netdev-dpdk: Secure flow offload API.
rte API is not thread safe. We have to get netdev mutex
before uing it and also before using fields of netdev structure.
This is important because offload API used from the separate
thread and could be used at the same time with other netdev
functions called from the main thread.
CC: Finn Christensen <fc@napatech.com> Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ilya Maximets [Thu, 18 Oct 2018 13:29:20 +0000 (16:29 +0300)]
netdev-dpdk: Drop offload API for vhost ports.
vhost ports are not DPDK eth ports and has no rte_flow API.
Stop calling this API with DPDK_ETH_PORT_ID_INVALID to
avoid time wasting and errors in log.
Additionally, DPDK_FLOW_OFFLOAD_API definition moved to .c
file, because there is no need to expose it in header.
CC: Finn Christensen <fc@napatech.com> Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ben Pfaff [Wed, 24 Oct 2018 21:23:38 +0000 (14:23 -0700)]
connmgr: Improve interface for setting controllers.
Using an shash instead of an array simplifies the code for both the caller
and the callee. Putting the set of allowed OpenFlow versions into the
ofproto_controller data structure also simplifies the overall function
interface slightly.
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, 25 Oct 2018 17:34:41 +0000 (10:34 -0700)]
connmgr: Modernize coding style.
This moves declarations closer to first use and merges them with
initialization when possible, moves "for" loop variable declarations into
the "for" statements where possible, and otherwise makes this code look
like it was written a little more recently than it was.
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
dpif: Restore a few lines with form feed characters
A few lines with form feed characters (ASCII: ^L) were accidentally
deleted by a recent commit to support rebalancing of offloaded flows.
This patch reverts those lines.
Ben Pfaff [Tue, 30 Oct 2018 22:03:17 +0000 (15:03 -0700)]
ovn-northd: Improve hashing for chassis queues.
The key for a "struct ovn_chassis_qdisc_queues" is a Chassis UUID and a
queue_id, but only the UUID was being hashed, so if there was more than one
per chassis then they'd all end up in the same hash bucket, which is
needlessly inefficient. (And if there's only one per chassis then why do
we bother allocating them at all?)
Found by inspection.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <nusiddiq@redhat.com>
Yi-Hung Wei [Tue, 30 Oct 2018 20:47:25 +0000 (13:47 -0700)]
ovs-lib.in: Remove unnecessary conntrack flush
We introduced flush-conntrack in force-reload-kmod script by commit 8bea39b186ca ("datapath: Prevent panic") to prevent kernel panic.
It turns out that the kernel panic is actually triggered by the
IPv4 secret timer, and it is fixed by commit 121905984724 ("compat: Initialize IPv4 reassembly secret timer").
This commit removes the unnecessary conntrack flush in the script.
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> CC: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Jianbo Liu [Mon, 29 Oct 2018 08:29:41 +0000 (08:29 +0000)]
dpif-netlink: Don't destroy and recreate port if it exists
In commit 7521e0cf9e ('ofproto-dpif: Let the dpif report when a port is
a duplicate'), the checking of port existence before adding was removed,
and it's up to the dpif to check if port exists and add only if needed.
But the port can't be added to datapath if already exists. Then it will
be destroyed and created again. This causes problem because configuration
may miss. For example, if creating two vxlan on the same port, its ingress
qdisc will be lost after recreated.
Fixes: 7521e0cf9e88 ("ofproto-dpif: Let the dpif report when a port is a duplicate.") Signed-off-by: Jianbo Liu <jianbol@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
lib/ofp-table.c:1454:42: error: \
format specifies type 'unsigned char' but the argument has type 'int'
ds_put_format(s, "\n table %"PRIu8, table);
~~ ^~~~~
CC: Ben Pfaff <blp@ovn.org> Fixes: b47e7e2bac7f ("ofp-table: Always format the table number in table features.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Mon, 29 Oct 2018 14:46:33 +0000 (17:46 +0300)]
manpages: Include ovs.tmac in most man roots.
This allows to not redefine common macroses in every single
file and allowes using things like .EX without warying about
compatibility.
manpages.mk updated automatically.
Files that are already complete pages (i.e. has no *.in sources)
wasn't touched, because this will require additional file
manipulations and changes in makefiles/specs without serious
profit.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 30 Aug 2018 18:03:12 +0000 (11:03 -0700)]
ofp-table: Always format the table number in table features.
Table features should indicate the table number as well as the table
name. Before this, the first line for each table looked like this:
table myname ("myname"):
but it's more useful if it's:
table 123 ("myname"):
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Ben Pfaff [Mon, 27 Aug 2018 22:40:35 +0000 (15:40 -0700)]
ofp-table: Ignore bits that have to change according to OpenFlow.
OpenFlow table feature replies contain a per-table bitmap that indicates
which tables a flow can point to in goto_table actions. OpenFlow requires
that a table only be able to go to higher-numbered tables. This means that
a switch that is general as possible will always have different features
for every table, since each one will have a different bitmap. This makes
the output of "ovs-ofctl dump-table-features" pretty long and ugly because
it has about 250 entries like this:
table %d:
metadata: match=0xffffffffffffffff write=0xffffffffffffffff
max_entries=%d
instructions (table miss and others):
next tables: %d-253
(same instructions)
(same actions)
(same matching)
This commit changes the logic that prints table features messages so that
it considers two sequentially numbered tables to be the same if only the
bit that necessarily must be tunred off changes. This reduces the hundreds
of entries above to just:
tables 1...253: ditto
which is so much more readable.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Zak Whittington [Fri, 26 Oct 2018 22:06:28 +0000 (15:06 -0700)]
ofp-msgs: Added ONF_ and NXT_REQUESTFORWARD for OF1.0-1.3
Backported OFPT14_REQUESTFORWARD to OF1.0-1.3.
OF 1.0-1.2 use an NXT Nicira extension while OF 1.3
uses an ONF extension (ONF version is specified in a
previously published ONF spec sheet).
Includes ofp-print tests for multiple inner message
types, and multiple OF versions including the NXT and ONF.
Also includes more end-to-end ofproto tests for both
NXT OF1.0 and also ONF OF1.3.
VMware-BZ: 2136594 Signed-off-by: Zak Whittington <zwhitt.vmware@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 25 Oct 2018 21:41:50 +0000 (14:41 -0700)]
NSH: Fix NSH-related length macros that cause stack overflow
In the filed of ver_flags_ttl_len of struct nshhdr, there are only 6
bits that are used to indicate header's total length in 4-byte words.
Therefore, the max value for total is 252 (63x4), instead of 256 used
in present code base. This patch fixes it.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10855 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 25 Oct 2018 21:49:14 +0000 (14:49 -0700)]
odp-util: Properly handle the return values of scan_XXX functions
Functions like scan_u8, return 0 when they failed to scan the expected
values. Function scan_geneve failed to check this situation. This leads
to using of uninitialized value of opt_len_mask. This patch fixes it
and further inspects and fixes all the problematic places where
the return values of scan_XXX functions are not properly handled.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10800 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 25 Oct 2018 23:17:23 +0000 (16:17 -0700)]
ofctl_parse_target: Only parse complete ofputil_flow_mod data.
When parse_ofp_flow_mod_str returns error, `fm` is incomplete and pointers
in it may be null, e.g. fm.match.flow. In this case, passing it to
ofctl_parse_flows__ may cause pointer errors because ofctl_parse_flows__
expects a valid input of type struct ofputil_flow_mod.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11110 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Zak Whittington [Thu, 25 Oct 2018 18:09:09 +0000 (11:09 -0700)]
bridge.c: prevent controller connects while flow-restore-wait
When force-reload-kmod is used, it shows an error when reinstalling
tlvs during "Restoring saved flows" step:
OFPT_ERROR (xid=0x4): NXTTMFC_ALREADY_MAPPED
This is caused by a race condition between the restore script,
which calls ofctl, and the connected controllers both adding back
the same TLVs.
The restore script already sets flow-restore-wait to true while
doing flow restoration, and sets it back to false after it is
done, and this patch utilizes that fact to prevent the TLV race.
It does this by preventing vswitchd from connecting to
controllers in the controller table while it is in a
flow-restore-wait state.
With this patch, when bridge_configure_remotes() calls
bridge_get_controllers(), it first checks if flow-restore-wait
has been set, and if so, it ignores any controllers in the
controller database and sets n_controllers to 0.
This solution does preserve the management service controller
which is added via bridge_ofproto_controller_for_mgmt() after
checking whether we should call bridge_get_controllers()
(and thus n_controllers is properly set to 1, etc)
VMware-BZ: 2195377 Signed-off-by: Zak Whittington <zwhitt.vmware@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Bhargava Shastry [Mon, 15 Oct 2018 09:23:33 +0000 (11:23 +0200)]
ossfuzz: Add ofctl parse target
This patch adds a new target called ofctl_parse_target to
ossfuzz. The main idea is to begin to fuzz APIs from the ofctl utility
program. At a later point, these may be added. For the moment, this patch
only fuzzes APIs that parse flow mod commands.
This target is demonstrably capable of finding memory corruption defects
in the parsing path. To aid the fuzzing process, a dictionary file
containing tokens specific to this parsing path have been added.
Signed-off-by: Bhargava Shastry <bshastry@sect.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Thu, 18 Oct 2018 11:17:05 +0000 (16:47 +0530)]
connmgr: Fix vswitchd abort when a port is added and the controller is down
We see the below trace when a port is added to a bridge and the configured
controller is down
0x00007fb002f8b207 in raise () from /lib64/libc.so.6
0x00007fb002f8c8f8 in abort () from /lib64/libc.so.6
0x00007fb004953026 in ofputil_protocol_to_ofp_version () from /lib64/libopenvswitch-2.10.so.0
0x00007fb00494e38e in ofputil_encode_port_status () from /lib64/libopenvswitch-2.10.so.0
0x00007fb004ef1c5b in connmgr_send_port_status () from /lib64/libofproto-2.10.so.0
0x00007fb004efa9f4 in ofport_install () from /lib64/libofproto-2.10.so.0
0x00007fb004efbfb2 in update_port () from /lib64/libofproto-2.10.so.0
0x00007fb004efc7f9 in ofproto_port_add () from /lib64/libofproto-2.10.so.0
0x0000556d540a3f95 in bridge_add_ports__ ()
0x0000556d540a5a47 in bridge_reconfigure ()
0x0000556d540a9199 in bridge_run ()
0x0000556d540a02a5 in main ()
The abort is because of ofputil_protocol_to_ofp_version() is called with invalid
protocol - OFPUTIL_P_NONE. Please see [1] for more details. Similar aborts are
seen as reported in [2].
The commit [3] changed the behavior of the function rconn_get_version().
Before the commit [3], the function ofconn_receives_async_msg() would always
return false if the connection to the controller was down, since
rconn_get_version() used to return -1. This patch now checks the rconn
connection status in ofconn_receives_async_msg() and returns false if not
connected. This would avoid the aborts seen in the above stack trace.
The issue can be reproduced by running the test added in this patch
without the fix.
Yifeng Sun [Thu, 18 Oct 2018 21:10:48 +0000 (14:10 -0700)]
odp-util: Move ufid handling to odp_flow_from_string
When parse_odp_key_mask_attr runs into ufid, it returns length of ufid
without further parsing. This causes problem to set action in function
parse_odp_action where nested nlattrs are expected from ofpbuf after.
Since in all callers of parse_odp_key_mask_attr, only odp_flow_from_string
needs to understand ufid. This patch moves ufid parsing from
parse_odp_key_mask_attr out to odp_flow_from_string.
Reported-by: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10850 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 18 Oct 2018 21:10:49 +0000 (14:10 -0700)]
odp-util: Initialize nsh_hdr in odp_nsh_hdr_from_attr
For function odp_nsh_hdr_from_attr, paramemter `attr` may not contain
all necessary OVS_NSH_KEY_ATTR_ to fully initialize data of nsh_hdr.
On return, nsh_hdr can contain uninitialized values in its fields.
This patch zeroes out nsh_hdr in the beginning in order to prevent it.
Reported-by: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10863 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 18 Oct 2018 21:10:47 +0000 (14:10 -0700)]
ofp-port: Free memory on error in ofp_print_ofpst_port_reply
Counters in ops->custom_stats may already be valid at this error point.
This patch frees the leaked memory.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10322 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
This test is intended to cover flow_put operation for datapath
flow modifications.
Original bug was reported here:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-September/352579.html
And fixed by commit: 35fe9efb2f02 ("dpif-netdev: Add vlan to mask for flow_put operation.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Co-authored-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
revalidator: Rebalance offloaded flows based on the pps rate
This is the third patch in the patch-set to support dynamic rebalancing
of offloaded flows.
The dynamic rebalancing functionality is implemented in this patch. The
ukeys that are not scheduled for deletion are obtained and passed as input
to the rebalancing routine. The rebalancing is done in the context of
revalidation leader thread, after all other revalidator threads are
done with gathering rebalancing data for flows.
For each netdev that is in OOR state, a list of flows - both offloaded
and non-offloaded (pending) - is obtained using the ukeys. For each netdev
that is in OOR state, the flows are grouped and sorted into offloaded and
pending flows. The offloaded flows are sorted in descending order of
pps-rate, while pending flows are sorted in ascending order of pps-rate.
The rebalancing is done in two phases. In the first phase, we try to
offload all pending flows and if that succeeds, the OOR state on the device
is cleared. If some (or none) of the pending flows could not be offloaded,
then we start replacing an offloaded flow that has a lower pps-rate than
a pending flow, until there are no more pending flows with a higher rate
than an offloaded flow. The flows that are replaced from the device are
added into kernel datapath.
A new OVS configuration parameter "offload-rebalance", is added to ovsdb.
The default value of this is "false". To enable this feature, set the
value of this parameter to "true", which provides packets-per-second
rate based policy to dynamically offload and un-offload flows.
Note: This option can be enabled only when 'hw-offload' policy is enabled.
It also requires 'tc-policy' to be set to 'skip_sw'; otherwise, flow
offload errors (specifically ENOSPC error this feature depends on) reported
by an offloaded device are supressed by TC-Flower kernel module.
revalidator: Gather packets-per-second rate of flows
This is the second patch in the patch-set to support dynamic rebalancing
of offloaded flows.
The packets-per-second (pps) rate for each flow is computed in the context
of revalidator threads when the flow stats are retrieved. The pps-rate is
computed only after a flow is revalidated and is not scheduled for
deletion. The parameters used to compute pps and the pps itself are saved
in udpif_key since they need to be persisted across iterations of
rebalancing.
dpif-netlink: Detect Out-Of-Resource condition on a netdev
This is the first patch in the patch-set to support dynamic rebalancing
of offloaded flows.
The patch detects OOR condition on a netdev port when ENOSPC error is
returned by TC-Flower while adding a flow rule. A new structure is added
to the netdev called "netdev_hw_info", to store OOR related information
required to perform dynamic offload-rebalancing.
Lorenzo Bianconi [Thu, 18 Oct 2018 14:05:56 +0000 (16:05 +0200)]
OVN: add buffering support for ip packets
Add buffering support for IPv4/IPv6 packets that will be processed
by arp{}/nd_ns{} action when L2 address is not discovered yet since
otherwise the packet will be substituted with an ARP/Neighbor
Solicitation frame and this will result in the lost of the first
packet of the connection.
Moreover fix following automatic tests broken by ip-buffering support
since now original ip packets are transmitted by OVN logical
router:
- ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR
- ovn -- /32 router IP address
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Wed, 22 Aug 2018 22:12:39 +0000 (15:12 -0700)]
netdev-dummy: Initialize new dummy ports as "up".
Dummy ports started out down and hardly any of the tests ever brought them
up. This led to some odd test results and caused problems for testing with
controllers that didn't bother with ports that were down, like recent
versions of Faucet. There doesn't seem to be a big reason for them to be
down by default, so this commit changes them to be up by default. It also
updates the tests to match the new behavior.
Ben Pfaff [Thu, 23 Aug 2018 20:41:03 +0000 (13:41 -0700)]
ofproto: Move may_enable from ofport_dpif to ofport.
This concept of whether a port is suitable to be "live" in the sense of the
OpenFlow OFPPS_LIVE bit is a generic one that can be handled at the ofproto
layer instead of needing to be part of ofproto-dpif.
An upcoming commit will make more use of this at the ofproto layer.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <nusididq@redhat.com>
Ben Pfaff [Thu, 23 Aug 2018 20:21:59 +0000 (13:21 -0700)]
ofproto: Refactor update_port().
update_port() worked a little too hard to avoid copying and comparing some
bits in the ofputil_phy_port. This seems like a simpler approach all
around. It should behave the same way.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <nusididq@redhat.com>
Ben Pfaff [Wed, 22 Aug 2018 21:59:24 +0000 (14:59 -0700)]
connmgr: Suppress duplicate port status notifications.
When the status of a port changes, ofproto calls into connmgr to notify
controllers. Sometimes, particular changes are only visible to controllers
running specific versions of OpenFlow. Until now, OVS would send those
controllers duplicate port status notifications. This is unnecessary and
somewhat confusing. This commit eliminates it.
This commit updates one of the tests not to expect duplicate notifications.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <nusididq@redhat.com>
Ben Pfaff [Wed, 10 Oct 2018 20:35:00 +0000 (13:35 -0700)]
bond: Honor updelay and downdelay when LACP is in use.
Since OVS added LACP support back in 2011, bonds have ignored the updelay
and downdelay values for bonds with configured LACP. The reason is not
clear, but at least one user needs support in this case, so this commit
enables it.
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047490.html Reported-by: Daniel Leaberry <dleaberry@purestorage.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Ilya Maximets [Tue, 16 Oct 2018 16:47:16 +0000 (19:47 +0300)]
netdev-bsd: Fix build failure because of undefined NO_OFFLOAD_API.
NO_OFFLOAD_API was removed while netdev classes initialization
refactoring, but netdev-bsd still uses it. Instead of
redefining it, I just refactored the BSD classes to be same
as other netdevs.
CC: Ben Pfaff <blp@ovn.org> Fixes: 89c09c1cd1f0 ("netdev: Clean up class initialization.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Eli Britstein [Thu, 11 Oct 2018 07:06:43 +0000 (10:06 +0300)]
netdev-tc-offloads: TC csum option is not matched with tunnel configuration
Tunnels (gre, geneve, vxlan) support 'csum' option (true/false), default is false.
Generated encap TC rule will now be configured as the tunnel configuration.
Signed-off-by: Eli Britstein <elibr@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Ben Pfaff [Mon, 15 Oct 2018 18:33:28 +0000 (11:33 -0700)]
Revert "OVN: add buffering support for ip packets"
This reverts commit 2e5cdb4b13924e275ca0776ca0f4147bf5ff7885. With the
commit applied, testing with only a single CPU core, e.g. by running "make
check" under "taskset -c 2", test '2649: ovn -- 3 HVs, 3 LS, 3 lports/LS, 1
LR' fails, apparently reliably.
The commit should be re-applied once the issue with the test is worked out.
Reported-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yi-Hung Wei [Wed, 10 Oct 2018 23:21:57 +0000 (16:21 -0700)]
datapath: compat: Fix compilation issue with grsecurity patch
Grsecurity patch enables GCC's constify plugin so that it will
automatically constify a class of type that contains only function
pointers. However, if the type is also specified by __read_mostly, it
will put the constify object into the read_mostly section that results
in compilation error. This patch works around the compilation issue by
disabling __ready_mostly when grsecurity patch is applied.
Tested with 4.14.33 kernel with grsecurity patch.
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
2. Fully broken Travis-CI testsuite build:
building 'ovs._json' extension
creating build/temp.linux-x86_64-2.7
error: could not create 'build/temp.linux-x86_64-2.7': \
Permission denied
https://travis-ci.org/openvswitch/ovs/jobs/440693765
3. Broken local testsuite build on Ubuntu 18.04:
running build_ext
building 'ovs._json' extension
creating build/temp.linux-x86_64-3.6
creating build/temp.linux-x86_64-3.6/ovs
<...>
/usr/bin/ld: .libs/libopenvswitch.a(util.o): \
relocation R_X86_64_TPOFF32 against `var.7749' can not be \
used when making a shared object; recompile with -fPIC
<...>
collect2: error: ld returned 1 exit status
4. Fedora build failure because of 'setuptools' ('distutils')
hard dependency on 'redhat-rpm-config' package:
building 'ovs._json' extension
<...>
gcc: error: <...>/redhat-hardened-cc1: No such file or directory
5. Looks like 'setuptools' also could download and install
unwanted python modules during package build.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 21 Aug 2018 03:25:51 +0000 (20:25 -0700)]
ofproto-dpif-xlate: Avoid deadlock on multicast snooping recursion.
Until now, OVS did multicast snooping outputs holding the read-lock on
the mcast_snooping object. This could recurse via a patch port to try to
take the write-lock on the same object, which deadlocked. This patch fixes
the problem, by releasing the read-lock before doing any outputs.
It would probably be better to use RCU for mcast_snooping. That would be
a bigger patch and less suitable for backporting.
Reported-by: Sameh Elsharkawy
Reported-at: https://github.com/openvswitch/ovs-issues/issues/153 Signed-off-by: Ben Pfaff <blp@ovn.org>
system-dpdk: Connect network namespaces via dpdkvhostuser ports
This adds a few unit tests to the 'check-dpdk' subsystem that will
exercise allocations of two network namespaces, PMDs, and the
vhost-user and vhost-user-client code path(separate tests).
As part of the tests, userspace bridge is added and attached to OVS.
Also, the tap devices created are added to the network namespaces.
The ultimatum is to connect the two network namespaces by pinging them.
system-dpdk: Use a different character marker for sed commands
The default marker for sed commands according to the manual is /, but this
is inconvenient when working with paths. The solution is either to escape
all instances of / or use sed's \cREGEXc feature.
Signed-off-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
system-dpdk: Allow running the dpdk tests from a VM
Some VM configurations result in CPU flags that cause warnings to be issued by
the DPDK libraries. When these warnings are issued, the tests will fail.
This commit adds the unreliable tsc warning to the list of ignored warnings.
Signed-off-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
system-dpdk: Skip all tests if there are no hugepages
A failure is quite harsh in this scenario. It's better to
simply skip all the tests and let the user look at the logs
to understand the missing hugepages.
system-dpdk: Update test suite for non-phy testing
This allows a system that doesn't have a dedicated DPDK nic to
execute some DPDK tests. In this fashion, tests that operate on
virtual ports (such as dpdkvhostuserclient) can be executed in
a wider set of environments.
Ilya Maximets [Thu, 11 Oct 2018 12:06:44 +0000 (15:06 +0300)]
dpif-netdev-perf: Clarify frequency number.
'dpif-netdev/pmd-perf-show' command prints the frequency number
calculated from the total number of cycles spent for iterations
for the measured period. This number could be confusing, because
users may think that it should be equal to CPU frequency, especially
on non-x86 systems where TSC frequency likely does not match with
CPU one.
Moreover, counted TSC cycles could differ from the HW TSC cycles
in case of a large number of PMD reloads, because cycles spent
outside of the main polling loop are not taken into account anywhere.
In this case the frequency will not match even TSC frequency.
Let's clarify the meaning in order to avoid this misunderstanding.
'Cycles' replaced with 'Used TSC cycles', which describes how many TSC
cycles consumed by the main polling loop. % of the total TSC cycles
now printed instead of GHz frequency, because GHz is unclear for
understanding, especially without knowing the exact TSC frequency.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Terry Wilson [Tue, 9 Oct 2018 16:31:33 +0000 (11:31 -0500)]
Work around Python/C JSON unicode differences
The OVS C-based JSON parser operates on bytes, so the parser_feed
function returns the number of bytes that are processed. The pure
Python JSON parser currently operates on unicode, so it expects
that Parser.feed() returns a number of characters. This difference
leads to parsing errors when unicode characters are passed to the
C JSON parser from Python.
Signed-off-by: Terry Wilson <twilson@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
Terry Wilson [Tue, 9 Oct 2018 16:31:32 +0000 (11:31 -0500)]
Test the Python C JSON extension
The C JSON parser was added quite a while ago, but unless you
configure with --enable-shared and have the Python 2/3 development
libraries installed, and the resulting python-ovs module installed,
'make check' won't actually test it.
This patch changes Python-based tests to run from the
$builddir/python directory and makes the tests configurable to use
both JSON backends. There are some unicode failures in the C JSON
extension that I left unfixed in this patch to make it easy to
show run the new tests on broken code. The next patch in this set
works around the issue.
Signed-off-by: Terry Wilson <twilson@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
Mark Michelson [Tue, 9 Oct 2018 12:27:05 +0000 (08:27 -0400)]
ovn-nbctl: Add basic port group commands.
This adds the following commands:
pg-add: Add a new port group, optionally adding switch ports at
creation.
pg-set-ports: Sets the logical switch ports on a port group
pg-del: Remove a port group.
The main motivation for these commands is that it allows for adding
logical switch ports by name rather than UUID.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Tue, 9 Oct 2018 07:17:11 +0000 (12:47 +0530)]
ovn-ctl: Fix the wrong pidfile argument passed to ovsdb-servers
When OVN db servers are started usinb ovn-ctl, if the pid files
(/var/run/openvswitch/ovnnb_db.pid for example) are already
present, then ovn-ctl passes "--pidfile=123" if the pid file has
'123' stored in it. Later on when OVN pacemaker RA script calls
status_ovnnb/status_ovnsb() functions, these returns "not running".
The shell function 'pidfile_is_running()' stores the contents of
the pid file as "pid=`cat "$pidfile"`". If the caller also
uses the same variable "pid" to store the file name, it gets
overriden.
This patch fixes this issue by renaming the local variable "pid"
in the "start_ovsdb__()" shell function to "db_file_name".
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Tue, 9 Oct 2018 13:11:57 +0000 (18:41 +0530)]
ovn-trace: Fix tracing when ip.dst has to go via a gateway router
ovn-trace does not trace past an l3gateway port type. This patch
fixes it.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1626080 Suggested-by: Dan Williams <dcbw@redhat.com> Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Numan Siddique [Wed, 10 Oct 2018 06:08:55 +0000 (11:38 +0530)]
ovn: Support configuring the BFD params for the tunnel interfaces
With this commit the users can override the default values of
the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
This can be useful to debug any issues related to BFD (like
frequent BFD state changes).
A new column 'options' is added in NB_Global and SB_Global tables
of OVN_Northbound and OVN_Southbound schemas respectively. CMS
can define the options 'bfd-min-rx', 'bfd-min-tx',
'bfd-decay-min-rx' and 'bfd-mult' in the options column of
NB_Global table row. ovn-northd copies these options from
NB_Global to SB_Global. ovn-controller configures these
options to the tunnel interfaces when enabling BFD.
When BFD is disabled, this patch now clears the 'bfd' column
of the interface row, instead of setting 'enable=false'.
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
aginwala [Tue, 9 Oct 2018 11:00:45 +0000 (04:00 -0700)]
ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode.
For OVN DBs to work with SSL in HA, we need to have capability to pass ssl
certs when starting OVN DBs. Say when starting OVN DBs in active passive mode,
in order for the standby DBs to sync from master node, it cannot sync
because the required ssl certs are not passed when standby DBs are initialized.
Hence, we need to have this option.
e.g. start nb db with ssl certs as below:
/usr/share/openvswitch/scripts/ovn-ctl --ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \
--ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \
--ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \
--db-nb-create-insecure-remote=no start_nb_ovsdb
When certs are passed in the command line, it will read certs from the path
mentioned instead of default db configs.
Certs can be generated based on ovs ssl docs:
http://docs.openvswitch.org/en/latest/howto/ssl/
Signed-off-by: aginwala <aginwala@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Han Zhou <hzhou8@ebay.com>
Ben Pfaff [Thu, 11 Oct 2018 19:44:33 +0000 (12:44 -0700)]
expr: Disallow < <= >= > comparisons against empty value set.
OVN expression syntax does not allow a literal empty value set, like {}.
Rather, any literal value set has to have at least one value. However,
value sets that originate from address sets or from port groups can be
empty. In such a case, == and != comparisons are allowed but < <= >= >
should be errors. The actual implementation failed to properly disallow
the latter and instead tried to access the first element of the value set,
a bad read. This fixes the problem.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767 Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Numan Siddique [Wed, 10 Oct 2018 17:18:59 +0000 (22:48 +0530)]
ovn-controller: Support processing DHCPv6 information request message type
When 'dhcpv6_stateless' is configured on the logical router ports,
the client will send DHCPv6 information request message type (using
dhclient -6 -S) to get additional options like dns-server. This
patch supports this option. Ideally we should have supported this
option when the DHCPv6 support was added.
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 10 Oct 2018 22:15:52 +0000 (15:15 -0700)]
expr: Set a limit on the depth of nested parentheses
This patch checks the depth of nested parentheses to prevent
stack overflow. Since is_chassis_resident doesn't allow
nested parentheses, its following parentheses are not taken
into acount in the parentheses-depth context.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10714 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Suggested-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
projects / mirror_ovs.git / log