Han Zhou [Fri, 12 Apr 2019 23:26:26 +0000 (16:26 -0700)]
ovsdb raft: Test cases for cluster failures when there are pending transactions.
Implement test cases for the failure scenarios when there are pending
transactions from clients. This patch implements test cases for different
combinations of conditions with the help of previously added test
commands and options for cluster mode. The conditions include:
- Connected node from which client transaction is executed: leader, follower
- Crashed node: leader, follower that is connected, or the other follower
- Crash point:
- For leader:
- before/after receiving execute_command_request
- before/after sending append_request
- before/after sending execute_command_reply
- For follower:
- before/after sending execute_command_request
- after receiving append_request
There are 16 test cases in total, and 9 of them are skipped purposely
because of the bugs found by the test cases to avoid CI failure. They will
be enabled in coming patches when the corresponding bugs are fixed.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Fri, 12 Apr 2019 23:26:25 +0000 (16:26 -0700)]
ovn-nbctl: Support --no-shuffle-remotes.
Support --no-shuffle-remotes option for ovn-nbctl, which is mainly for testing
purpose, so that we can specify the order that client will failover when the
connected node is down, to have more predictability in the test cases.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Fri, 12 Apr 2019 23:26:24 +0000 (16:26 -0700)]
ovsdb-idl: Support optionally not shuffling multiple remotes.
This patch allows remotes not being shuffled if desired (mostly for
testing purpose, when we need the order of remotes during retrying
be predictable). By default it still shuffles as how it behaves today.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Fri, 12 Apr 2019 23:26:23 +0000 (16:26 -0700)]
ovsdb raft: Support commands that are required for testing failure scenarios.
Added unix commands cluster/... for ovsdb raft, which will be used in a future
patch to test more fine-grained failure scenarios. The commands either causes
a node to crash at certain point, or manipulate the election timer so that
we can control the election process to elect a new leader we desired for the
test cases.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Fri, 12 Apr 2019 23:26:22 +0000 (16:26 -0700)]
ovsdb raft: Sync commit index to followers without delay.
When update is requested from follower, the leader sends AppendRequest
to all followers and wait until AppendReply received from majority, and
then it will update commit index - the new entry is regarded as committed
in raft log. However, this commit will not be notified to followers
(including the one initiated the request) until next heartbeat (ping
timeout), if no other pending requests. This results in long latency
for updates made through followers, especially when a batch of updates
are requested through the same follower.
$ time for i in `seq 1 100`; do ovn-nbctl ls-add ls$i; done
real 0m34.154s
user 0m0.083s
sys 0m0.250s
This patch solves the problem by sending heartbeat as soon as the commit
index is updated in leader. It also avoids unnessary heartbeat by resetting
the ping timer whenever AppendRequest is broadcasted. With this patch
the performance is improved more than 50 times in same test:
$ time for i in `seq 1 100`; do ovn-nbctl ls-add ls$i; done
real 0m0.564s
user 0m0.080s
sys 0m0.199s
Torture test cases are also updated because otherwise the tests will
all be skipped because of the improved performance.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Fri, 12 Apr 2019 23:26:21 +0000 (16:26 -0700)]
ovsdb raft: Move ovsdb cluster tests to separate testsuite.
Tests in ovsdb-cluster.at are relatively slow, especially torture
tests, and they will be changed in the future costing high CPU, to
make the tests more effective. So we move the tests to a separate
testsuite, so that we can execute them separately, probably with
lower parallelism to avoid exhausting system resources.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 12 Apr 2019 17:00:02 +0000 (10:00 -0700)]
debian: Remove Ben Pfaff from Uploaders field.
I don't want to claim to be in charge of upstream Debian packaging anymore. Reviewed-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Chris Mi [Sat, 13 Apr 2019 08:09:37 +0000 (16:09 +0800)]
netdev-vport: Use the dst_port in tunnel netdev name
If tunnel device dst_port is not the default one, "ovs-dpctl dump-flows"
will fail. The error message for vxlan is:
netdev_linux|INFO|ioctl(SIOCGIFINDEX) on vxlan_sys_4789 device failed: No such device
That's because when calling netdev_vport_construct() for netdev
vxlan_sys_xxxx, the default dst_port is used. Actually, the dst_port
value is in the netdev name. Use it to avoid the error.
Signed-off-by: Chris Mi <chrism@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Unlike manual splitting, 'splitlines' correctly handles different
line endings. Without this change script fails to check files with
'\r\n' endings treating the whole patch as a header.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
LIU Yulong [Tue, 9 Apr 2019 07:48:08 +0000 (15:48 +0800)]
Docs: fix conntrack flow ct_state input
In the following envrionment:
ovs-vsctl (Open vSwitch) 2.11.0
DB Schema 7.16.1
we meet the following error during the tutorials
conntrack test:
"ovs-ofctl: field +est missing value"
"ovs-ofctl: field +trk missing value"
ovs-vsctl 2.9.0 has the same issue.
This patch gives the tutorials with right
conntrack input.
The flow action buffer can be resized if it's not big enough to contain
all the requested flow actions. However, this resize doesn't take into
account the new requested size, the buffer is only increased by a factor
of 2x. This might be not enough to contain the new data, causing a
buffer overflow, for example:
Fix by making sure the new buffer is properly resized to contain all the
requested data.
BugLink: https://bugs.launchpad.net/bugs/1813244 Signed-off-by: Andrea Righi <andrea.righi@canonical.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Cc: Andrea Righi <andrea.righi@canonical.com> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
acinclude: Use AC_SEARCH_LIBS for linking with dl.
DPDK uses dlopen to load plugins and we need to search for
library containing this function. But we should not do this
in a loop because 'AC_SEARCH_LIBS' could do this for us.
Also, 'AC_SEARCH_LIBS' prints user-visible messages that are
useful for debuging.
Also added the new 'checking' message and code normalized to
be more readable.
With this change we'll have following additional messages:
checking for library containing dlopen... -ldl
checking whether linking with dpdk works... yes
acinclude: Transparent checking for DPDK dependencies.
'AC_CHECK_DECL' makes almost same thing as 'AC_COMPILE_IFELSE', but
looks more pretty. Additionally it prints checking results in a
user-visible way making it easy to understand which configs checked
and why we need one or another dependency.
For exmaple, with this patch, configure log may look like this:
checking whether dpdk datapath is enabled... yes
checking for rte_config.h... yes
checking whether RTE_LIBRTE_VHOST_NUMA is declared... no
checking whether RTE_EAL_NUMA_AWARE_HUGEPAGES is declared... yes
checking for library containing get_mempolicy... -lnuma
checking whether RTE_LIBRTE_VHOST_NUMA is declared... (cached) no
checking whether RTE_LIBRTE_PMD_PCAP is declared... yes
checking for library containing pcap_dump... -lpcap
checking whether RTE_LIBRTE_PDUMP is declared... yes
checking whether RTE_LIBRTE_MLX5_PMD is declared... no
checking whether RTE_LIBRTE_MLX4_PMD is declared... yes
checking whether RTE_LIBRTE_MLX4_DLOPEN_DEPS is declared... yes
Instead of just:
checking whether dpdk datapath is enabled... yes
checking for rte_config.h... yes
checking for library containing get_mempolicy... -lnuma
checking for library containing pcap_dump... -lpcap
Anyway, code looks more clean and easier to understand. Also, with
this change we're defining VHOST_NUMA only if RTE_LIBRTE_VHOST_NUMA
defined. This costs nothing as all the checks with 'AC_CHECK_DECL'
are cached.
Ilya Maximets [Tue, 5 Mar 2019 16:28:27 +0000 (19:28 +0300)]
netdev-dpdk: Allocate vhost_id dynamically.
'vhost_id' is an array of 'PATH_MAX' bytes in the middle of
'netdev_dpdk' structure. That is 4K bytes.
'vhost_id' never used on a hot path and there is no need to keep
it inside the structure memory. Dynamic allocation will allow to
decrease 'struct netdev_dpdk' significantly, saving 4KB per ETH
port (ETH ports doesn't use 'vhost_id') and almost same value per
vhost ports (real 'vhost_id's, in common case, are much shorter).
We could save the pointer space by making the union with 'devargs'
which is mutually exclusive with 'vhost_id'.
As we're just removing the single 'PADDED_MEMBER', the total
cacheline layout is not affected.
John Hurley [Tue, 9 Apr 2019 14:36:14 +0000 (15:36 +0100)]
ovs-tc: offload datapath rules matching on internal ports
Rules applied to OvS internal ports are not represented in TC datapaths.
However, it is possible to support rules matching on internal ports in TC.
The start_xmit ndo of OvS internal ports directs packets back into the OvS
kernel datapath where they are rematched with the ingress port now being
that of the internal port. Due to this, rules matching on an internal port
can be added as TC filters to an egress qdisc for these ports.
Allow rules applied to internal ports to be offloaded to TC as egress
filters. Rules redirecting to an internal port are also offloaded. These
are supported by the redirect ingress functionality applied in an earlier
patch.
Signed-off-by: John Hurley <john.hurley@netronome.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
John Hurley [Tue, 9 Apr 2019 14:36:13 +0000 (15:36 +0100)]
ovs-tc: allow offloading TC rules to egress qdiscs
Offloading rules to a TC datapath only allows the creating of ingress hook
qdiscs and the application of filters to these. However, there may be
certain situations where an egress qdisc is more applicable (e.g. when
offloading to TC rules applied to OvS internal ports).
Extend the TC API in OvS to allow the creation of egress qdiscs and to add
or interact with flower filters applied to these.
Signed-off-by: John Hurley <john.hurley@netronome.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
John Hurley [Tue, 9 Apr 2019 14:36:12 +0000 (15:36 +0100)]
ovs-tc: allow offloading of ingress mirred TC actions to datapath
The TC datapath only permits the offload of mirred actions if they are
egress. To offload TC actions that output to OvS internal ports, ingress
mirred actions are required. At the TC layer, an ingress mirred action
passes the packet back into the network stack as if it came in the action
port rather than attempting to egress the port.
Update OvS-TC offloads to support ingress mirred actions. To ensure
packets that match these rules are properly passed into the network stack,
add a TC skbedit action along with ingress mirred that sets the pkt_type
to PACKET_HOST. This mirrors the functionality of the OvS internal port
kernel module.
Signed-off-by: John Hurley <john.hurley@netronome.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
John Hurley [Tue, 9 Apr 2019 14:36:11 +0000 (15:36 +0100)]
compat: add compatibility headers for tc skbedit action
OvS includes compat code for several TC actions including vlan, mirred and
tunnel key. Add support for using skbedit actions when compiling
user-space code against older kernel headers.
Signed-off-by: John Hurley <john.hurley@netronome.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Raed Salem [Mon, 8 Apr 2019 12:42:11 +0000 (15:42 +0300)]
netdev-tc-offloads: Fix probe tc block support
Current implementation will try to create an qdisk of type ingress with
block id 1 to check for kernel ingress block support, this check is
insufficient as old kernels without ingress block support will
successfully create an ingress qdisc, ignoring the ingress block.
Fix by trying to add a test rule on the ingress block.
Fixes 093c9458fb02 ("tc: allow offloading of block ids") Signed-off-by: Raed Salem <raeds@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Acked-by: John Hurley <john.hurley@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
datapath-windows: Add guards around IpHelper adapter binding calls
Protect internal adapter up/down calls with a dispatch lock. It was
observed that the InternalAdapter bind calls could happen out of order
thereby causing encap packets to not be sent properly.
Add assert around the IpHelper bind calls to ensure Up/Down gets called
only for the appropriate vports.
Signed-off-by: Sairam Venugopal <vsairam@vmware.com> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
datapath-windows: Address memory allocation issues for OVS_BUFFER_CONTEXT
With current implementation, when nbl pool is allocated, context size is
specified as 64 bytes, while the OVS_BUFFER_CONTEXT size is only 32 bytes.
Since context size is never changed, additional memory is not required.
This patch makes it simpler to allocate memory for OVS_BUFFER_CONTEXT so
that it is always aligned to MEMORY_ALLOCATION_ALIGNMENT.
This is acheived by updating "value" field in the context
structure, so that number of elements in array is always a multiple of
MEMORY_ALLOCATION_ALIGNMENT.
Also change the DEFAULT_CONTEXT_SIZE to accomodate OVS_BUFFER_CONTEXT size.
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>
Ansis Atteka [Tue, 26 Mar 2019 18:12:01 +0000 (11:12 -0700)]
rhel: Include all header files in the Fedora's devel package
While the header files added by this patch into Fedora's devel
rpm package can be considered private, the other devel packages
for RHEL/CentOS and Debian/Ubuntu distros include them.
So this patch simply makes the Fedora devel package consistent with
the other devel packages.
Toms Atteka [Wed, 20 Mar 2019 20:40:19 +0000 (13:40 -0700)]
lib: added check to prevent int overflow
If enough large input is given ofpact_finish will fail.
Implemented ofpbuf_oversized function to check for oversized
buffer. Checks were added for parse functions and error messages
returned.
Basic manual testing performed.
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Reported-by: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12972 Signed-off-by: Toms Atteka <cpp.code.lv@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Fri, 22 Mar 2019 12:58:39 +0000 (15:58 +0300)]
bridge: Propagate patch port pairing errors to db.
Virtual ports like 'patch' ports that almost fully implemented on
'ofproto' layer could have internal to 'ofproto' statuses that
could not be retrieved from 'netdev' or other layers. For example,
in current implementation there is no way to get the patch port
pairing status (i.e. if it has usable peer?).
New 'ofproto-provider' API function 'vport_get_status' introduced to
cover this gap. It allowes 'bridge' layer to retrive current status
of ofproto virtual ports and propagate it to DB.
For now we're only interested in pairing errors of 'patch' ports.
That are propagated to the 'error' column of the 'Interface' table.
Ex.:
$ ovs-vsctl show
...
Bridge "br1"
...
Port "patch1"
Interface "patch1"
type: patch
options: {peer="patch0"}
error: "No usable peer 'patch0' exists in 'system' datapath."
Bridge "br0"
datapath_type: netdev
...
Port "patch0"
Interface "patch0"
type: patch
options: {peer="patch1"}
error: "No usable peer 'patch1' exists in 'netdev' datapath."
Ben Pfaff [Tue, 26 Mar 2019 16:58:20 +0000 (09:58 -0700)]
hmap: Improve debug log message when reporting unusually large buckets.
I was seeing a lot of these messages, including a lot of them suppressed
by rate-limiting, and I wondered whether any really big messages were
being suppressed. By reporting the largest bucket, instead of just every
large bucket, it becomes more likely that the truly too-large buckets get
reported.
(The problem I saw was a false alarm.)
Acked-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Mark Michelson [Mon, 25 Mar 2019 21:29:56 +0000 (17:29 -0400)]
OVN: Make periodic RAs consistent with RA responder.
This commit makes periodic RAs from OVN consistent with the RAs sent in
response to RSs. Specifically, this ensures that prefix flags are set
correctly for each address mode.
This commit also gets rid of some redundant definitions for RA prefix
option flags from packets.h in favor of the ones in ovn-l7.h.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Mark Michelson [Mon, 25 Mar 2019 21:29:55 +0000 (17:29 -0400)]
OVN: Always send prefix option in RAs
OVN's behavior when sending router advertisements has been to include IP
prefix information only if the address mode is set to "slaac" or
"dhcp_stateless". In these modes, sending the prefix to the client is
necessary so that it may automatically provision its IP address. We do
not send the prefix option when the address mode is set to
"dhcp_stateful" since there is no need for the client to automatically
provision an IP address.
This logic is flawed, however. When using dhcp_stateful, we provide a
managed IPv6 address for a client. However, because we do not provide
prefix information in our RAs, the client does not know the prefix
length for the address it has been allocated. With dhclient, we have
seen it assume either /64 or /128, depending on which version is being
used. This may not accurately reflect the prefix length being used by
the DHCP server though.
The fix here is to always send prefix information in our RAs, regardless
of address mode. The key difference lies in how we set the A
(autonomous addressing) flag. For slaac and dhcp_stateless address
modes, we will set this flag, indicating the client should provision its
own address based on the prefix we have sent. For dhcp_stateful, we will
not set this flag. This way, it is clear the prefix is informational,
and the client should not try to provision its own IPv6 address.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Mark Michelson [Mon, 25 Mar 2019 21:29:54 +0000 (17:29 -0400)]
OVN: Use offset instead of pointer into ofpbuf
In general, maintaining a pointer into an ofpbuf is risky. As the ofpbuf
grows, it can reallocate its data. If this happens, then pointers into
the data will become invalid.
A safer practice is to track an offset into the ofpbuf's data where a
structure you are interested in is kept. This way, if the ofpbuf data is
reallocated, you can find your structure again by using the offset.
In practice, this patch is not fixing any issues with OVN. Even though
the ra pointer is pointing to ofpbuf data that can be reallocated, it
will never actually happen. ovn-northd and all test cases always encode
the address mode first, meaning we will only ever read from the ra
pointer before the ofpbuf has a chance to expand.
However, this base work is essential for an upcoming patch in this series.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Sat, 16 Mar 2019 05:57:41 +0000 (11:27 +0530)]
ovn-controller: Add a new thread in pinctrl module to handle packet-ins.
Prior to this patch, ovn-controller was single threaded and everytime the
poll_block() at the end of the main while() loop wakes up, it processes
the whole SB DB and translates the logical flows to OF flows.
There are few issues with this -
* For every packet-in received, ovn-controller does this translation
resulting in unnecessary CPU cycles.
* If the translation takes a lot of time, then the packet-in handling
would get delayed. The delay in responses to DHCP requests could
result in resending of these requests.
This patch addresses these issues by creating a new pthread in pinctrl module
to handle packet-ins. This thread doesn't access the Southbound DB IDL object.
Since some of the OVN actions - like dns_lookup, arp, put_arp, put_nd
require access to the Southbound DB contents and gARPs, periodic IPv6 RA
generation also requires the DB access, pinctrl_run() called by the main
ovn-controller thread accesses the Southbound DB IDL and builds the local
datastructures. pinctrl_handler thread accesses these data structures
in handling such requests. An ovs_mutex is used between the pinctr_run() and
the pinctrl_handler thread to protect these data structures.
Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Sat, 16 Mar 2019 05:57:26 +0000 (11:27 +0530)]
ovn pinctrl: Pass 'struct rconn *swconn' to all the functions which use it
In pinctrl.c, many functions use 'swconn' variable which is declared as
global static. This patch passes 'swconn' as a variable to functions.
This will help in an upcoming patch which makes processing
packet-ins in a separate pthread.
Suggested-by: Mark Michelson <mmichels@redhat.com> Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Sat, 16 Mar 2019 18:50:24 +0000 (11:50 -0700)]
tests: Remove maximum version kernel check.
The macro 'OVS_CHECK_KERNEL' was checking for maximum supported kernel
version. This means checks like 'OVS_CHECK_KERNEL(3, 10, 4, 18)'
in various tests need to be updated when each new kernel version is
supported. This is unnecessary as these tests are expected to continue
to work in later kernel versions.
Fix this by changing the macro to check for minimum version only and
update the macro name accordingly.
This patch does not make any possible corrections to the indicated minimum
kernel version specified by the callers of the macro.
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
gdb) bt
at ../sysdeps/posix/libc_fatal.c:175
ar_ptr=<optimized out>) at malloc.c:5049
group_id=<optimized out>, cb=cb@entry=0x55cab8fd6cd0 <append_group_stats>) at ofproto/ofproto.c:6790
Signed-off-by: solomon <liwei.solomon@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 19 Mar 2019 11:08:20 +0000 (14:08 +0300)]
dpif-netdev-perf: Fix millisecond stats precision with slower TSC.
Unlike x86 where TSC frequency usually matches with CPU frequency,
another architectures could have much slower TSCs.
For example, it's common for Arm SoCs to have 100 MHz TSC by default.
In this case perf module will check for end of current millisecond
each 10K cycles, i.e 10 times per millisecond. This could be not
enough to collect precise statistics.
Fix that by taking current TSC frequency into account instead of
hardcoding the number of cycles.
CC: Jan Scheurich <jan.scheurich@ericsson.com> Fixes: 79f368756ce8 ("dpif-netdev: Detailed performance stats for PMDs") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Jan Scheurich <jan.scheurich@ericsson.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Han Zhou [Thu, 21 Mar 2019 23:06:31 +0000 (16:06 -0700)]
ovn-ctl: Make sure OVN_RUNDIR is created for central nodes.
When ovn-ctl tries to start ovsdb, it didn't ensure the rundir
(e.g. /var/run/openvswitch) exist, because it is not calling
start_daemon(). Usually, if OVS is started by ovs-ctl before
on the same node, the folder is created already. However, for
OVN central node, OVS is usually not needed. If the folder is
not created (it is common case when system restarted because
/var/run is usually tmpfs), ovn-ctl will fail to start ovsdb.
This patch always ensures the OVN_RUNDIR is created.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Thu, 21 Mar 2019 23:06:30 +0000 (16:06 -0700)]
ovn-ctl: Unify OVN_RUNDIR usage.
In this script $rundir and $OVN_RUNDIR is used in a mixed way, which
can cause different folders used for different runtime files. This
patch unifies the usage to the correct one.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Timothy Redaelli [Fri, 22 Mar 2019 18:45:46 +0000 (19:45 +0100)]
rhel: Fix sphinx BuildRequires on Fedora Rawhide
On Fedora Rawhide only python3-sphinx is available, but currently
python2-sphinx is used.
This commit changes the BuildRequires for sphinx to use
/usr/bin/sphinx-build directly instead of python2-sphinx in order to make
it work on current Fedora Rawhide too.
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Thu, 21 Mar 2019 10:56:47 +0000 (13:56 +0300)]
ovs-vsctl: Add datapath_type column to show command.
Sometimes it's unclear which datapath type is in use by particular
bridge. For example, if all the interfaces supported by both system
and netdev datapaths it needs a DB query or log analysis to find out
which 'datapath_type' is in use.
Another case is that it's hard to figure out if patch ports are really
connected to each other. They are definitely not connected if datapath
types of their bridges differs.
With this change non-default 'datapath_type's will be exposed to
'ovs-vsctl show' command, so it'll be easier to spot misconfiguration.
$ ovs-vsctl show
...
Bridge "br0"
datapath_type: netdev
Port "br0"
Interface "br0"
type: internal
...
Han Zhou [Fri, 22 Mar 2019 20:41:05 +0000 (13:41 -0700)]
reconnect.c: Don't transition back to ACTIVE when forced to RECONNECT.
Currently, whenever there is activity on the session, the FSM is
transitioned to ACTIVE. However, this causes reconnect_force_reconnect()
failed to work once there are traffic received from remote after
transition to RECONNECT, it will skip the reconnection phase and directly
go back to ACTIVE for the old session. This patch fixes it so that
when FSM is in RECONNECT state, it doesn't transition back to ACTIVE
directly.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
The ifupdown.sh script passes the --may-exist option
to ovs-vsctl invocations in order for it to exit without failing
if the device to be added already exists. This holds true for
all cases of adding objects to ovs-vswitchd except for when
configuring a bond interface.
This patch adds the --may-exist option to the missing
statement, which suppresses the logging of such errors in
syslog.
Additionally, running the unpatched version of this script when
the bond interface already exists appears to break
networking with some versions of ifupdown found in debian
testing (0.8.35), where the service won't start up properly
because of the aforementioned errors.
Signed-off-by: George Diamantopoulos <georgediam@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ted Elhourani [Fri, 25 Jan 2019 19:10:01 +0000 (19:10 +0000)]
python: Monitor Database table to manage lifecycle of IDL client.
The Python IDL implementation supports ovsdb cluster connections.
This patch is a follow up to commit 31e434fc98, it adds the option of
connecting to the leader (the default) in the Raft-based cluster. It mimics
the exisiting C IDL support for clusters introduced in commit 1b1d2e6daa.
The _Server database schema is first requested, then a monitor of the
Database table in the _Server Database. Method __check_server_db verifies
the eligibility of the server. If the attempt to obtain a monitor of the
_Server database fails and a cluster id was not provided this implementation
proceeds to request the data monitor. If a cluster id was provided via the
set_cluster_id method then the connection is aborted and a connection to a
different node is instead attempted, until a valid cluster node is found.
Thus, when supplied, cluster id is interpreted as the intention to only
allow connections to a clustered database. If not supplied, connections to
standalone nodes, or nodes that do not have the _Server database are
allowed. change_seqno is not incremented in the case of Database table
updates.
Timothy Redaelli [Fri, 22 Mar 2019 14:02:14 +0000 (15:02 +0100)]
python: Fix package requirements with old setuptools
Commit 00fcc832d598 ("Update Python package requirements") added a
PEP 508 environment marker to install pywin32 on Windows systems.
This requires a new setuptools version (>= 20.5), but (at least)
RHEL/CentOS7 and Debian Jessie are using an older version of
setuptools and so python extension failed to build.
This commit adds "extras_require" instead of the PEP 508 environment
markers in order to have the conditional dependency of pywin32, but by
remaining compatible with the old setuptools versions.
CC: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> CC: Lucian Petrut <lpetrut@cloudbasesolutions.com> Fixes: 00fcc832d598 ("Update Python package requirements") Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
Roni Bar Yanai [Tue, 5 Mar 2019 16:49:31 +0000 (16:49 +0000)]
netdev-dpdk: Move offloading code to a new file
Hardware offloading code is moved to a new file called
netdev-rte-offloads.c. The original offloading code is copied
from the netdev-dpdk.c file to the new file, where future
offloading code should be added as well.
The copied code was refactored based on coding style.
The netdev-dpdk.c file will remain unchanged as new offloading
code is added.
Before offloading code was added to the netdev-dpdk.c file (MARK and
RSS actions) the only DPDK RTE calls in use were rte_flow_create() and
rte_flow_destroy(). In preparation for splitting the offloading code
from the netdev-dpdk.c file to a separate file, it is required
to embed these RTE calls into a global netdev-dpdk-* API so that
they can be called from the new file. An example for this requirement
can be seen in the handling of dev->mutex, which should be encapsulated
inside netdev-dpdk class (netdev-dpdk.c file), and should be unknown
to the outside callers. This commit embeds the rte_flow_create() call
inside the netdev_dpdk_flow_create() API and the rte_flow_destroy()
call inside the netdev_dpdk_rte_flow_destroy() API.
Ilya Maximets [Mon, 18 Mar 2019 13:01:13 +0000 (16:01 +0300)]
dpif-netdev-perf: Fix double update of perf histograms.
Real values of 'packets per batch' and 'cycles per upcall' already
added to histograms in 'dpif-netdev' on receive. Adding the averages
makes statistics wrong. We should not add to histograms values that
never really appeared.
For exmaple, in current code following situation is possible:
Ilya Maximets [Thu, 14 Mar 2019 14:43:48 +0000 (17:43 +0300)]
dpdk: Stop dumping memzones to stdout.
Information about memzones reserved on init is not much useful.
Anyway, we need to log it in more civilized manner, i.e. through
the OVS logging subsystem.
Ilya Maximets [Mon, 18 Mar 2019 11:02:30 +0000 (14:02 +0300)]
dpctl: Drop parser debug information.
This information is not that useful.
Anyway, no need to print it each time to the logs.
CC: Ben Pfaff <blp@ovn.org> Fixes: d1fd1ea91242 ("ovs-dpctl: New --names option to use port names in flow dumps.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Or Gerlitz [Sun, 17 Mar 2019 14:13:25 +0000 (16:13 +0200)]
netdev-tc-offloads: Properly get the block id on flow del/get
Currnetly, when a tc flow is installed on a bond port using shared blocks,
we get these failures from the validator threads:
2019-03-17T10:02:58.919Z|13369|dpif(revalidator93)|WARN|system@ovs-system: failed to flow_del \
(No such file or directory) ufid:ebe2888b-9886-4835-a42e-c2911f6af6e8 skb_priority(0),skb_mark(0),in_port(2), \
packet_type(ns=0,id=0),eth(src=e4:11:22:33:44:71,dst=24:8a:07:88:28:12),eth_type(0x0806), [..]
The block id must be retrieved from the device we got by ufid lookup and
not from the input to the related function, fix that for flow del and get.
While here, add the block id to existing debug print.
Fixes: 88dcf2aa8234 ('netdev-provider: add class op to get block_id') Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com> Signed-off-by: Paul Blakey <paulb@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Moshe Levi [Thu, 28 Feb 2019 19:29:10 +0000 (21:29 +0200)]
netdev-tc-offloads: Improve log message for icmpv6 offload not supported
Signed-off-by: Moshe Levi <moshele@mellanox.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Ashish Varma [Wed, 13 Mar 2019 18:31:05 +0000 (11:31 -0700)]
ofp-protocol: Changed the number of bits in OFPUTIL_P_ANY from 10 to 9.
The removal of support for OpenFlow 1.6 (draft) resulted in the removal of
"OFPUTIL_P_OF16_OXM 1 << 9". OFPUTIL_P_ANY which represets all protocols will
now have only 9 valid bits.
Fixes: 29718ad49d61 ("Remove support for OpenFlow 1.6 (draft).") Signed-off-by: Ashish Varma <ashishvarma.ovs@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Fri, 15 Mar 2019 22:01:20 +0000 (15:01 -0700)]
conntrack: Replace structure copy by memcpy().
There are a few cases where structure copy can be replaced by
memcpy(), for possible portability benefit. This is because
the structures involved have padding and elements of the
structure are used to generate hashes.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Fri, 15 Mar 2019 22:01:19 +0000 (15:01 -0700)]
conntrack: Lookup only 'UNNAT conns' in 'nat_clean()'.
When freeing 'UNNAT conns', lookup only 'UNNAT conns' to
protect against possible address overlap with 'default
conns' during a DOS attempt. This is very unlikely, but
protection is simple.
Darrell Ball [Fri, 15 Mar 2019 22:01:18 +0000 (15:01 -0700)]
conntrack: Fix race for NAT cleanup.
Reference lists are not fully protected during cleanup of
NAT connections where the bucket lock is transiently not held during
list traversal. This can lead to referencing freed memory during
cleaning from multiple contexts. Fix this by protecting with
the existing 'cleanup' mutex in the missed cases where 'conn_clean()'
is called. 'conntrack_flush()' is converted to expiry list traversal
to support the proper bucket level protection with the 'cleanup' mutex.
The NAT exhaustion case cleanup in 'conn_not_found()' is also modified
to avoid the same issue.
Ilya Maximets [Mon, 4 Mar 2019 10:35:30 +0000 (13:35 +0300)]
treewide: Clean up inclusions of netdev-dpdk header.
'netdev-dpdk.h' provides only 'netdev_dpdk_register' and
'free_dpdk_buf' which are not used in these files and should
not be used.
Leftovers from the already removed code.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Justin Pettit [Mon, 4 Mar 2019 22:28:58 +0000 (14:28 -0800)]
ovn-nbctl: Don't segfault when ovn-northd doesn't configure dynamic addresses.
When ovn-nbctl is used to configure a logical switch port's addresses, it
does a sanity-check to make sure that a duplicate address isn't being
used. If a port is configured as "dynamic", ovn-northd is supposed to
populate the "dynamic_addresses" column in the Logical_Switch_Port
table. If it isn't ovn-nbctl, would dereference a null pointer as part
of the duplicate address check. This patch checks that "dynamic_addresses"
is actually set first.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Tue, 26 Feb 2019 10:38:39 +0000 (13:38 +0300)]
dp-packet: Copy flow mark on packet clone.
Dummy interfaces clones dp-packet while 'receive' appctl processing.
In general, we should do this anyway to avoid any possible issues in
the future with real interfaces.
Sairam Venugopal [Wed, 27 Feb 2019 00:45:10 +0000 (16:45 -0800)]
datapath-windows: Fix potential deadlock in event subscription
Move the EventQueue lock acquisition after the dispatchLock to prevent a
potential deadlock in port creation pipeline. There could be a case where
a port event could try to take up the Dispatch Lock before the Event Queue
lock and the subscription queue event could take up the event queue lock
before the dispatch lock.
Found while testing with Driver Verifier enabled.
Signed-off-by: Sairam Venugopal <vsairam@vmware.com> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
datapath-windows: Fix nbl cleanup when memory allocation fails
StartNblIngressError should be called only when an NBL hasn't been
modified. In this case the nbl context was initialized. Rely on existing
packet completion mechanism to cleanup the NBL.
Found while testing with DriverVerifier with limited memory setting
enabled.
Ilya Maximets [Tue, 26 Feb 2019 10:38:37 +0000 (13:38 +0300)]
dp-packet: Refactor offloading API.
1. No reason to have mbuf related APIs in a generic code.
2. Not only RSS/checksums should be invalidated in case of tunnel
decapsulation or sending to 'ring' ports.
In order to fix two above issues, new function
'dp_packet_reset_offload' introduced. In order to clean up/unify
the code and simplify addition of new offloading features to non-DPDK
version of dp_packet, introduced 'ol_flags' bitmask. Additionally
reduced code complexity in 'dp_packet_clone_with_headroom' by using
already existent generic APIs.
Unfortunately, we still need to have a special case for mbuf
initialization inside 'dp_packet_init__()'.
'dp_packet_init_specific()' introduced for this purpose as a generic
API for initialization of the implementation-specific fields.
Roi Dayan [Mon, 11 Mar 2019 12:47:08 +0000 (14:47 +0200)]
netdev-linux: Remove ingress qdisc before trying to add shared block
Adding shared ingress block with ingress qdisc already exists results
in a failure. So remove the ingress qdisc first.
Also while at it log the slave name.
Signed-off-by: Roi Dayan <roid@mellanox.com> Acked-by: John Hurley <john.hurley@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Roi Dayan [Mon, 11 Mar 2019 14:34:05 +0000 (16:34 +0200)]
netdev-tc-offloads: Remove ingress qdisc on tc init flow api
It could be a port added to ovs bridge already has ingress qdisc
which will make the block probe fail.
The probes should start clean and ingress is being added later
so just remove ingress in case it exists.
Signed-off-by: Roi Dayan <roid@mellanox.com> Acked-by: John Hurley <john.hurley@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Han Zhou [Wed, 6 Mar 2019 02:16:50 +0000 (18:16 -0800)]
ovsdb-idl: Fix memory leak of ovsdb_idl_db_clear.
ovsdb_idl_row_destroy() doesn't free the memory of row structure itself.
This is because of the ovsdb change tracking feature: the deleted row
may be accessed in the current iteration of main loop. The function
ovsdb_idl_row_destroy_postprocess() is called at the end of
ovsdb_idl_run() to free the deleted rows that are not tracked; the
function ovsdb_idl_db_track_clear() is called (indirectly) by user
at the end of each main loop iteration to free the deleted rows that
are tracked. However, in ovsdb_idl_db_clear(), which may be called when
a session is reset, or when the idl is destroyed, it didn't call
ovsdb_idl_row_destroy_postprocess(), which would result in all the
untracked rows leaked. This patch fixes that.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Fri, 1 Mar 2019 18:56:37 +0000 (10:56 -0800)]
ovsdb raft: Precheck prereq before proposing commit.
In current OVSDB Raft design, when there are multiple transactions
pending, either from same server node or different nodes in the
cluster, only the first one can be successful at once, and following
ones will fail at the prerequisite check on leader node, because
the first one will update the expected prerequisite eid on leader
node, and the prerequisite used for proposing a commit has to be
committed eid, so it is not possible for a node to use the latest
prerequisite expected by the leader to propose a commit until the
lastest transaction is committed by the leader and updated the
committed_index on the node.
Current implementation proposes the commit as soon as the transaction
is requested by the client, which results in continously retry which
causes high CPU load and waste.
Particularly, even if all clients are using leader_only to connect to
only the leader, the prereq check failure still happens a lot when
a batch of transactions are pending on the leader node - the leader
node proposes a batch of commits using the same committed eid as
prerequisite and it updates the expected prereq as soon as the first
one is in progress, but it needs time to append to followers and wait
until majority replies to update the committed_index, which results in
continously useless retries of the following transactions proposed by
the leader itself.
This patch doesn't change the design but simplely pre-checks if current
eid is same as prereq, before proposing the commit, to avoid waste of
CPU cycles, for both leader and followers. When clients use leader_only
mode, this patch completely eliminates the prereq check failures.
In scale test of OVN with 1k HVs and creating and binding 10k lports,
the patch resulted in 90% CPU cost reduction on leader and >80% CPU cost
reduction on followers. (The test was with leader election base time
set to 10000ms, because otherwise the test couldn't complete because
of the frequent leader re-election.)
This is just one of the related performance problems of the prereq
checking mechanism dicussed at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-February/048243.html Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
OVN: Add support for DHCP option 150 - TFTP server address
OpenStack Ironic relies on a few DHCP options [0] that were not
supported in OVN yet. This patch is adding the last one which is the
option 150 (TFTP server address, RFC5859 [1]).
Note that this option is Cisco proprietary, the IEEE standard that
matches with this requirement is Option 66. The difference is that 150
allows to multiple IPs to be specified and 66 only allows one.
Han Zhou [Wed, 6 Mar 2019 17:01:21 +0000 (09:01 -0800)]
ovsdb-idl: Fix memory leak of idl->remote.
Reported by Address Sanitizer.
Fixes: 5e07b8f93f03 ("ovsdb-idl: New function ovsdb_idl_create_unconnected().") Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
projects / ovs.git / log