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>
Somehow some such patches snuck through. checkpatch caught them (and the
committer missed that) but this makes it even more explicit.
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
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>
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>
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.
Signed-off-by: Tiago Lam <tiago.lam@intel.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Flavio Leitner <fbl@sysclose.org>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
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/
Co-authored-by: Tiago Lam <tiago.lam@intel.com>
Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
Signed-off-by: Tiago Lam <tiago.lam@intel.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Flavio Leitner <fbl@sysclose.org>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
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>
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().
Suggested-by: Flavio Leitner <fbl@sysclose.org>
Acked-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
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.
Fixes: 8a9562d21a ("dpif-netdev: Add DPDK netdev.")
Acked-by: Ilya Maximets <i.maximets@samsung.com>
Co-authored-by: Flavio Leitner <fbl@sysclose.org>
Signed-off-by: Flavio Leitner <fbl@sysclose.org>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
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: 02bb2824e5 ("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>
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>
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.
Cc: Ciara Loftus <ciara.loftus@intel.com>
Acked-by: Ciara Loftus <ciara.loftus@intel.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
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>
This is useful for understanding which flows offloaded to
which ports.
Code refactored a bit to reduce number of casts.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Data pointed by cmap node must not be freed while iterating.
ovsrcu_postpone should be used instead.
CC: Finn Christensen <fc@napatech.com>
Fixes: e8a2b5bf92 ("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>
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: e8a2b5bf92 ("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>
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: e8a2b5bf92 ("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>
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>
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>
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.
Fixes: 57924fc91c ("revalidator: Rebalance offloaded flows")
Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
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>
We introduced flush-conntrack in force-reload-kmod script by commit
8bea39b186 ("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
1219059847 ("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>
The memory returned by json_parser_finish needs to be freed by the caller.
Signed-off-by: Eric Lapointe <elapointe@corsa.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
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: 7521e0cf9e ("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>
This fixes travis build on OSX:
https://travis-ci.org/openvswitch/ovs/jobs/446920531
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: b47e7e2bac ("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>
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>
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>
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>
Before this patch, most dump-table-stats outputs would contain about
250 lines of the form:
table #: ditto
With this patch, they have one line like this:
tables 2...254: ditto
which is much easier to read.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>
Sometimes the 'errors' list is passed as null, and in that case it should
not be used.
Found by inspection.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>
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>
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>
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>
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>
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>
Remove the IP neighboring entries when adapter is down,
so that when 'OVS_IPHELPER_INSTANCE' is deleted, no stale entries
are present
Also fix accessing iphelper instance without acquiring the lock.
Signed-off-by: Anand Kumar <kumaranand@vmware.com>
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
Tested-by: Alin Gabriel Serdean <aserdean@ovn.org>
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
The daemon-windows file is missing a `set_detach` routine, so add it.
This will be useful in the long run.
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
Acked-by: Anand Kumar <kumaranand@vmware.com>
