Xu Binbin [Mon, 13 Aug 2018 02:27:42 +0000 (10:27 +0800)]
netdev-dpdk: Support the link speed of XL710
In the scenario of XL710, the link speed which stored in the table
of Interface is not 40G. Because the implementation of query of link
speed only support to 10G, the parameter 'current' will be a random
value in the scenario of higher link speed. In this case, incorrect
link speed of XL710 nic will be stored in the database.
Signed-off-by: Xu Binbin <xu.binbin1@zte.com.cn> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Han Zhou [Mon, 20 Aug 2018 05:27:31 +0000 (22:27 -0700)]
ovn-northd: Support learning neighbor from ARP request.
Current LR dynamic ARP learning support only ARP responses. If a
IP-MAC binding is learned, it will not get updated even if a host
send a GARP *request* to inform the new binding. This patch supports
learning neighbor changes from ARP requests, including GARP requests.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Mon, 20 Aug 2018 05:27:30 +0000 (22:27 -0700)]
ovn-northd: LR respond ARP from valid subnet only.
Currently ovn LR datapath responds ARP requests even if the ARP
requestor's src IP doesn't belong to the LR port's subnets. This
may generate unnecessary ARP responses and there could also be
security concerns. This patch restricts the ARP response only if
the requestor's IP matches the LR port's subnets.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Fri, 24 Aug 2018 19:26:52 +0000 (00:56 +0530)]
ovn: Fix the issue in IPv6 Neigh Solicitation responder for router IPs
Commit [1] added a new action 'nd_na_router' to set the router bit
in the 'flags' field of the Neighbour Adv packet for router IPs.
This action was used in the router pipeline. But the logical switch
pipeline also adds the Neighbour Adv flows for router IPs but with
'nd_na' action (which the commit [1] didn't handle).
This patch fixes this by changing the action to 'nd_na_router' for
router IPs.
Without this patch, the IPv6 functionality is broken.
[1] - "c9756229ed: ovn: Set proper Neighbour Adv flag when replying
for NS request for router IP"
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Han Zhou <hzhou8@ebay.com>
Yunjian Wang [Mon, 27 Aug 2018 11:52:55 +0000 (19:52 +0800)]
dpctl: Fix memory leak in dp_exists().
Fixes: ffdcd110fa62 ("dpctl: Make opt_dpif_open() more general.") Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Greg Rose [Fri, 24 Aug 2018 20:36:29 +0000 (13:36 -0700)]
ofproto-dpif: Check for EBUSY as well
Guru reported that we can't create more than one geneve tunnel.
Sometimes a driver will return EBUSY as well as EEXIST for some
duplicate configurations. Check for EBUSY too.
Fixes: 7521e0cf9e ("ofproto-dpif: Let the dpif report when a ...")
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047214.html Reported-by: Guru Shetty <guru@ovn.org> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
William Tu [Tue, 21 Aug 2018 23:03:14 +0000 (16:03 -0700)]
erspan: add big endian bit fields.
Big-endian systems arrange bit fields in the opposite order.
The patch follows the linux kernel's approach by defining the
big and little endian bit-field of ERSPAN header using #ifdef.
Tested on zelenka.debian.org
(https://db.debian.org/machines.cgi?host=zelenka).
Tested-by: Ben Pfaff <blp@ovn.org> Reported-by: James Page <james.page@canonical.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/351382.html Signed-off-by: William Tu <u9012063@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 21 Aug 2018 16:22:03 +0000 (09:22 -0700)]
tests: Fix hash function dependencies in "tunnel - ERSPAN v1/v2 metadata".
This test only worked if each OpenFlow port was assigned a particular
datapath port number: p1 to port 3, p2 to port 2, p3 and p4 to port 1.
This happened consistently on little-endian architectures because of the
use of a particular hash function, but on big-endian architectures it
failed because the hash function was different.
This commit fixes the problem by adding the non-dummy ports separately.
(Dummy ports try to take the datapath port number corresponding to their
name, when it is available.) This does result in swapping a couple of
datapaths port numbers, so that p1 has port 1, p2 has port 2, and the
erspan ports have port 3, hence the size of the patch.
Reported-by: James Page <james.page@canonical.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/351382.html Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: William Tu <u9012063@gmail.com>
Ben Pfaff [Sat, 18 Aug 2018 17:17:37 +0000 (10:17 -0700)]
netdev-linux: Avoid division by 0 if kernel reports bad scheduler data.
If the kernel reported a value of 0 for the second value in
/proc/net/psched, it would cause a division-by-zero fault in
read_psched(). I don't know of a kernel that would actually do that, but
it's still better to be safe.
Found by clang static analyzer.
Reported-by: Bhargava Shastry <bshastry@sect.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Ben Pfaff [Wed, 15 Aug 2018 21:57:13 +0000 (14:57 -0700)]
ofp-actions: Avoid assertion failure for clone(ct(...bad actions...)).
decode_NXAST_RAW_CT() temporarily pulls data off the beginning of its
ofpacts output ofpbuf and, on its error path, fails to push it back on.
At a higher layer, decode_NXAST_RAW_CLONE() asserts, via
ofpact_finish_CLONE(), that the ofpact_clone that it put is still in the
place where it put it, which causes an assertion failure.
The root cause here is the failure to re-push the clone header. One could
fix that, but it would be pretty easy for that to go wrong again on some
other obscure error path. Instead, this commit just makes the problem go
away by always saving and restoring 'ofpact->data' if a decode fails.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9862 Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Justin Pettit [Fri, 17 Aug 2018 19:48:54 +0000 (12:48 -0700)]
dpif-netlink: Prevent abort in probe_broken_meters().
Commit 92d0d515d ("dpif-netlink: Probe for broken Linux meter
implementations.") introduced a deadlock on the 'once' structure
declared in probe_broken_meters() with the following callstack:
Yi-Hung Wei [Fri, 17 Aug 2018 09:05:09 +0000 (02:05 -0700)]
dpif-netlink: Implement conntrack zone limit
This patch provides the implementation of conntrack zone limit
in dpif-netlink. It basically utilizes the netlink API to
communicate with OVS kernel module to set, delete, and get conntrack
zone limit.
Yi-Hung Wei [Fri, 17 Aug 2018 09:05:07 +0000 (02:05 -0700)]
dpif: Support conntrack zone limit.
This patch defines the dpif interface to support conntrack
per zone limit. Basically, OVS users can use this interface
to set, delete, and get the conntrack per zone limit for various
dpif interfaces. The following patch will make use of the proposed
interface to implement the feature.
Currently, nf_conntrack_max is used to limit the maximum number of
conntrack entries in the conntrack table for every network namespace.
For the VMs and containers that reside in the same namespace,
they share the same conntrack table, and the total # of conntrack entries
for all the VMs and containers are limited by nf_conntrack_max. In this
case, if one of the VM/container abuses the usage the conntrack entries,
it blocks the others from committing valid conntrack entries into the
conntrack table. Even if we can possibly put the VM in different network
namespace, the current nf_conntrack_max configuration is kind of rigid
that we cannot limit different VM/container to have different # conntrack
entries.
To address the aforementioned issue, this patch proposes to have a
fine-grained mechanism that could further limit the # of conntrack entries
per-zone. For example, we can designate different zone to different VM,
and set conntrack limit to each zone. By providing this isolation, a
mis-behaved VM only consumes the conntrack entries in its own zone, and
it will not influence other well-behaved VMs. Moreover, the users can
set various conntrack limit to different zone based on their preference.
The proposed implementation utilizes Netfilter's nf_conncount backend
to count the number of connections in a particular zone. If the number of
connection is above a configured limitation, ovs will return ENOMEM to the
userspace. If userspace does not configure the zone limit, the limit
defaults to zero that is no limitation, which is backward compatible to
the behavior without this patch.
The following high leve APIs are provided to the userspace:
- OVS_CT_LIMIT_CMD_SET:
* set default connection limit for all zones
* set the connection limit for a particular zone
- OVS_CT_LIMIT_CMD_DEL:
* remove the connection limit for a particular zone
- OVS_CT_LIMIT_CMD_GET:
* get the default connection limit for all zones
* get the connection limit for a particular zone
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
Yi-Hung Wei [Fri, 17 Aug 2018 09:05:04 +0000 (02:05 -0700)]
datapath: compat: Introduce static key support
Static keys allow the inclusion of seldom used features in
performance-sensitive fast-path kernel code, via a GCC feature and a
code patching technique. For more information:
* https://www.kernel.org/doc/Documentation/static-keys.txt
Since upstream ovs kernel module now uses some static key API that was
introduced in v4.3 kernel, we shall backport them to the compat module
for older kernel supprots.
This backport is based on upstream net-next commit 11276d5306b8
("locking/static_keys: Add a new static_key interface").
Yi-Hung Wei [Fri, 17 Aug 2018 09:05:03 +0000 (02:05 -0700)]
datapath: compat: Backports nf_conncount
This patch backports the nf_conncount backend that counts the number
of connections matching an arbitrary key. The following patch will
use the feature to support connection tracking zone limit in ovs
kernel datapath.
This backport is based on an upstream net-next upstream commits. 5c789e131cbb ("netfilter: nf_conncount: Add list lock and gc worker, and RCU for init tree search") 34848d5c896e ("netfilter: nf_conncount: Split insert and traversal") 2ba39118c10a ("netfilter: nf_conncount: Move locking into count_tree()") 976afca1ceba ("netfilter: nf_conncount: Early exit in nf_conncount_lookup() and cleanup") cb2b36f5a97d ("netfilter: nf_conncount: Switch to plain list") 2a406e8ac7c3 ("netfilter: nf_conncount: Early exit for garbage collection") b36e4523d4d5 ("netfilter: nf_conncount: fix garbage collection confirm race") 21ba8847f857 ("netfilter: nf_conncount: Fix garbage collection with zones") 5e5cbc7b23ea ("netfilter: nf_conncount: expose connection list interface") 35d8deb80c30 ("netfilter: conncount: Support count only use case") 6aec208786c2 ("netfilter: Refactor nf_conncount") d384e65f1e75 ("netfilter: return booleans instead of integers") 625c556118f3 ("netfilter: connlimit: split xt_connlimit into front and backend")
The upstream nf_conncount has a couple of export functions while
this patch only export the ones that ovs kernel module needs.
Yi-Hung Wei [Fri, 17 Aug 2018 09:05:02 +0000 (02:05 -0700)]
compat: Backport nf_ct_netns_{get, put}()
This patch backports nf_ct_netns_get/put() in order to support a feature
in the follow up patch.
nf_ct_netns_{get,put} were first introduced in upstream net-next commit ecb2421b5ddf ("netfilter: add and use nf_ct_netns_get/put") in kernel
v4.10, and then updated in commmit 7e35ec0e8044 ("netfilter: conntrack:
move nf_ct_netns_{get,put}() to core") in kernel v4.15. We need to
invoke nf_ct_netns_get/put() when the underlying nf_conntrack_l3proto
supports net_ns_{get,put}().
Therefore, there are 3 cases that we need to consider.
1) Before nf_ct_{get,put}() is introduced.
We just mock nf_ct_nets_{get,put}() and do nothing.
2) After 1) and before v4.15
Backports based on commit 7e35ec0e8044 .
Darrell Ball [Fri, 10 Aug 2018 15:56:17 +0000 (08:56 -0700)]
dpctl: Make opt_dpif_open() more general.
By making opt_dpif_open() more general, it can be used effectively
by all potential callers and avoids trying to open potentially bogus
datapaths provided by the user. Also, the error handling is improved by
reducing bogus errors and having more specific real errors.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Mon, 6 Aug 2018 17:55:26 +0000 (10:55 -0700)]
dpctl: Simplify dpctl_flush_conntrack.
The function dpctl_flush_conntrack() and other such new functions with
multiple optional arguments can be simplified by reodering the checks
for optional parameters, where the datapath argument is checked for
last.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 16 Aug 2018 16:52:55 +0000 (09:52 -0700)]
porting: Add fixes to support kernel 4.15.x
This patch enables OVS kernel module to run on kernel 4.15.x.
Two conntrack-related tests failed:
- conntrack - multiple zones, local
- conntrack - multi-stage pipeline, local
This might be due to conntrack policy changes for packets coming
from local ports on kernel 4.15. More survey will be done later.
Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Co-authored-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Gregory Rose <gvrose8192@gmail.com> Reviewed-by: Gregory Rose <gvrose8192@gmail.com>
Ben Pfaff [Tue, 14 Aug 2018 18:31:46 +0000 (11:31 -0700)]
ovsdb-idl: Adjust indexes during transactions.
When transactions modified tables with indexes, the indexes were not
properly updated to reflect the changes. For deleted rows, in particular,
this could cause use-after-free errors.
This commit fixes the problem and adds some simple test cases provided by
Han Zhou that, without the fix, cause a crash.
Reported-by: Han Zhou <zhouhan@gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047185.html Signed-off-by: Ben Pfaff <blp@ovn.org>
Justin Pettit [Thu, 9 Aug 2018 00:31:17 +0000 (17:31 -0700)]
dpif-netlink: Probe for broken Linux meter implementations.
Meter support was introduced in Linux 4.15. In some versions of Linux
4.15, 4.16, and 4.17, there was a bug that never set the id when the
meter was created, so all meters essentially had an id of zero. This
commit adds a probe to check for that condition and disable meters on
those kernels.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Justin Pettit [Wed, 8 Aug 2018 02:51:26 +0000 (19:51 -0700)]
dpif: Don't pass in '*meter_id' to meter_set commands.
The original intent of the API appears to be that the underlying DPIF
implementaion would choose a local meter id. However, neither of the
existing datapath meter implementations (userspace or Linux) implemented
that; they expected a valid meter id to be passed in, otherwise they
returned an error. This commit follows the existing implementations and
makes the API somewhat cleaner.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Daniel Alvarez [Mon, 13 Aug 2018 12:07:45 +0000 (14:07 +0200)]
netdev: Retry getting interfaces on inconsistent dumps from kernel
This patch in glibc [0] is fixing a bug where we may be getting
inconsistent dumps from the kernel when listing interfaces due to
a race condition.
This could happen if we try to retrieve them while interfaces are
being added/removed from the system at the same time.
For systems running against old glibc versions, this patch is retrying
the operation up to 3 times and then proceeding by logging a
warning.
Note that 3 times should be enough to not delay the operation much
and since it's unlikely that we hit the race condition 3 times in
a row. Still, if this happened, this patch is not changing the
current behavior.
Ben Pfaff [Fri, 15 Jun 2018 22:11:10 +0000 (15:11 -0700)]
test-unixctl.py: Don't suppress exceptions.
A user reported a failure of test 2364 "vlog - RFC5424 facility - Python2"
with an exit code that says that the test-unixctl process died from an
uncaught exception. Unfortunately the exception didn't show up in the log.
This commit should make the exception show up (it deletes some boilerplate
we use in our Python-based daemons to make them restart themselves on
failure, which isn't needed or appropriate for a test script).
Ben Pfaff [Fri, 15 Jun 2018 22:11:09 +0000 (15:11 -0700)]
ovsdb-server: Don't log closing session at program termination.
When ovsdb-server closes a remote connection, it logs a message about it
that includes the reason. Until now this has included sessions that it
closes when it exits. That meant that, when --run was used, there was a
race between noticing that the subprocess exited and noticing that the
session that that subprocess (presumably) had open had been closed. If
it noticed the latter first, nothing was logged (because it didn't log
anything if a session was closed in the ordinary way by the client). If
it noticed the former first, it logged a message about closing the session
itself.
This is a benign race that causes no real problems--except that the tests
didn't expect to see the log message from the former case and fail with
errors like the following:
1826. ovsdb-server.at:92: testing truncating database log with bad transaction ...
./ovsdb-server.at:96: ovsdb-tool create db schema
stderr:
stdout:
./ovsdb-server.at:104: ovsdb-server --remote=punix:socket db --run="sh txnfile"
--- /dev/null 2018-04-24 08:50:58.769000000 +0000
+++ /root/openvswitch-2.9.2/rpm/rpmbuild/BUILD/openvswitch-2.9.2/tests/testsuite.dir/at-groups/1826/stderr 2018-05-29 14:29:56.529257295 +0000
@@ -0,0 +1,2 @@
+2018-05-29T14:29:56Z|00001|ovsdb_jsonrpc_server|INFO|unix#0: disconnecting (removing ordinals database due to server termination)
+2018-05-29T14:29:56Z|00002|ovsdb_jsonrpc_server|INFO|unix#0: disconnecting (removing _Server database due to server termination)
This fixes the race. This particular log message isn't too useful since
it's pretty obvious that ovsdb-server is closing those sessions, since
after all it's exiting!
Numan Siddique [Tue, 7 Aug 2018 11:38:11 +0000 (17:08 +0530)]
python jsonrpc: Allow jsonrpc_session to have more than one remote.
Python IDL implementation doesn't have the support to connect to the
cluster dbs. This patch adds this support. We are still missing the
support in python idl class to connect to the cluster master. That
support will be added in an upcoming patch.
This patch is similar to the commit 8cf6bbb184 which added multiple remote
support in the C jsonrpc implementation.
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 [Tue, 7 Aug 2018 11:37:58 +0000 (17:07 +0530)]
ovs python: ovs.stream.open_block() returns success even if the remote is unreachable
The python function ovs.socket_util.check_connection_completion() uses select()
(provided by python) to monitor the socket file descriptor. The select()
returns 1 when the file descriptor becomes ready. For error cases like -
111 (Connection refused) and 113 (No route to host) (POLLERR), ovs.poller._SelectSelect.poll()
expects the exceptfds list to be set by select(). But that is not the case.
As per the select() man page, writefds list will be set for POLLERR.
Please see "Correspondence between select() and poll() notifications" section of select(2)
man page.
Because of this behavior, ovs.socket_util.check_connection_completion() returns success
even if the remote is unreachable or not listening on the port.
This patch fixes this issue by using poll() to check the connection status similar to
the C implementation of check_connection_completion().
A new function 'get_system_poll() is added in ovs/poller.py which returns the
select.poll() object. If select.poll is monkey patched by eventlet/gevent, it
gets the original select.poll() and returns it.
The test cases added in this patch fails without the fix.
Suggested-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
dpif: Move common meter checks into the dpif layer.
Another dpif provider will soon add support for meters, so move
some of the common sanity checks up into the dpif layer so that each
provider doesn't need to re-implement them.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Greg Rose [Mon, 13 Aug 2018 23:00:16 +0000 (16:00 -0700)]
compat: Substitute more dependable define
The compat layer ip_tunnel_get_stats64 function was checking for the
Linux kernel version to determine if the return was void or a pointer.
This is not very reliable and caused compile warnings on SLES 12 SP3.
In acinclude.m4 create a more reliable method of determining when to
use a void return vs. a pointer return.
Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Sat, 11 Aug 2018 03:29:46 +0000 (20:29 -0700)]
ovsdb-idl.c: Fix IDL index problem when rows are updated.
In current IDL index code it doesn't updated index when handling
"update2" messages, which is the default case. The consequence
is that when a row is updated, the index is not updated accordingly,
and even worse, it causes crash when calling ovsdb_idl_destroy().
It can be easily reproduced by the test cases added in this patch.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Kevin Traynor [Thu, 9 Aug 2018 15:13:58 +0000 (16:13 +0100)]
vswitch.xml: Update dpdk-init documentation.
dpdk-init is now a string. Add description of 'true' and 'try'.
Fixes: 3e52fa5644cd ("dpdk: reflect status and version in the database") Cc: aconole@redhat.com Signed-off-by: Kevin Traynor <ktraynor@redhat.com> Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Instead, it must be configured as two different commands,
"
ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
options:dpdk-devargs=0000:05:00.1
ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true
"
The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' fields before
trying to configuring the dpdk ethdev. Hence OVS can no longer set the
'dont care' fields to just '0' as before. This commit make sure all the
'rte_eth_fc_conf' fields are populated with default values before the dev
init.
Also to avoid read error on unsupported ports, the flow control parameters
are now read only when user is trying to configure/update it.
Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Aaron Conole [Wed, 8 Aug 2018 00:34:52 +0000 (20:34 -0400)]
table: fix html buffer output
Prior to this commit, html output exhibits a doppler effect for
content by continually printing strings passed from
table_print_html_cell.
Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table as a string.") Cc: Ben Pfaff <blp@ovn.org> Cc: Jakub Sitnicki <jsitnicki@gmail.com> Signed-off-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Aaron Conole [Wed, 8 Aug 2018 00:34:51 +0000 (20:34 -0400)]
table: append newline when printing tables
With commit cb139fa8b3a1 ("table: New function table_format() for
formatting a table as a string.") a new mechanism for formatting
tables was introduced, and the table_print method was refactored to
use this.
During that refactor, calls to 'puts' were replaced with
'ds_put_cstr', and table print was changed to use 'fputs(...,
stdout)'. Unfortunately, fputs() does not append a newline to the
string provided, and changes the output strings of, for example,
ovsdb-client dump to print all on one line. This means
post-processing scripts that are chained after ovsdb-client would
either block indefinitely (if they don't detect EOF), or process the
entire bundle at once (rather than seeing each table on a separate
line).
Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table as a string.") Cc: Ben Pfaff <blp@ovn.org> Cc: Jakub Sitnicki <jsitnicki@gmail.com> Reported-by: Terry Wilson <twilson@redhat.com>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508 Signed-off-by: Aaron Conole <aconole@redhat.com> Suggested-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Terry Wilson <twilson@redhat.com> Tested-by: Terry Wilson <twilson@redhat.com>
In the case there was no sorting criteria the flows on Windows were being
rearranged because it was always returning zero.
Also check if there we need sorting to save a few cycles.
CC: Ben Pfaff <blp@ovn.org> Co-authored-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Markos Chandras [Wed, 8 Aug 2018 14:27:25 +0000 (17:27 +0300)]
rhel: Use correct user in the logrotate configuration file
The /var/log/openvswitch directory is owned by the openvswitch user but
logrotate could be running as root or as another user. As a result of
which, rpmlint prints the following warning when building the spec file
on SUSE Linux Enterprise:
openvswitch.x86_64: W: suse-logrotate-user-writable-log-dir /var/log/openvswitch openvswitch:openvswitch 0750
The log directory is writable by unprivileged users. Please fix the
permissions so only root can write there or add the 'su' option
to your logrotate config
In order to fix that, we should run the logrotate script as the same
user which runs the various Open vSwitch daemons. If this is a new
installation, then this user is the 'openvswitch' one, but if we are
upgrading from an older release, then the user is normally 'root'.
As such, we set the initial user to 'root' and we fix this up in the
%post scriptlet.
Justin Pettit [Tue, 7 Aug 2018 23:45:26 +0000 (16:45 -0700)]
datapath: meter: Fix setting meter id for new entries
Upstream commit:
From: Justin Pettit <jpettit@ovn.org>
Date: Sat, 28 Jul 2018 15:26:01 -0700
Subject: [PATCH] openvswitch: meter: Fix setting meter id for new entries
The meter code would create an entry for each new meter. However, it
would not set the meter id in the new entry, so every meter would appear
to have a meter id of zero. This commit properly sets the meter id when
adding the entry.
Fixes: 96fbc13d7e77 ("openvswitch: Add meter infrastructure") Signed-off-by: Justin Pettit <jpettit@ovn.org> Cc: Andy Zhou <azhou@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Cc: Justin Pettit <jpettit@ovn.org> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Tue, 7 Aug 2018 02:44:02 +0000 (19:44 -0700)]
ovn-trace: Fix warnings when port is found but not in current datapath.
When port group is used, ovn-trace may print warnings like this:
$ ovn-trace ls1 'inport == "lp111" && eth.src == f0:00:00:00:01:11 && eth.dst == f0:00:00:00:01:12 && ip4.src == 192.168.11.1 && ip4.dst == 192.168.11.2 && ip.ttl == 10'
2018-08-02T01:43:23Z|00001|ovntrace|WARN|lp211: not in datapath ls1
2018-08-02T01:43:23Z|00002|ovntrace|WARN|lp211: unknown logical port
2018-08-02T01:43:23Z|00003|ovntrace|WARN|lp221: not in datapath ls1
2018-08-02T01:43:23Z|00004|ovntrace|WARN|lp221: unknown logical port
2018-08-02T01:43:23Z|00005|ovntrace|WARN|lp231: not in datapath ls1
2018-08-02T01:43:23Z|00006|ovntrace|WARN|lp231: unknown logical port
There are 2 warnings:
For the first one, it might be reasonable
before port group is supported, but now since ports in a port group
can span across multiple datapaths, this situation is normal, and
warning should not be printed.
For the second one, it is misleading, and it should not be printed
in this situation even before port group is supported. It should be
printed only if the port is not found at all.
This patch fixes both.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Han Zhou [Tue, 7 Aug 2018 02:44:01 +0000 (19:44 -0700)]
ovn-northd: Improve efficiency of stateful checking for ACLs on port groups.
Currently in has_stateful_acl(), to check if a datapath has stateful ACLs,
it needs to iterate all port groups and check if the current datapath is
related to each port group, and then iterate the ACLs on the port group. This
is inefficient if there are a lot of port groups. A typical scenario is in
OpenStack each tenant will have a default security group which will be mapped
as a port group, and the default security group is supposed to contain ports
of the tenant only, so most likely only the logical switches belonging to the
tenant should be related to the port group, but we are checking all the port
groups belonging to all tenants for each datapath.
To improve this, a reverse direction of hmap is built from logical switch to
port group, so that the iteration is avoided. The time complexity of this
function improves from O(P * A) to O(PL * A), P = total number of port groups
in NB, PL = number of port groups related to the logical switch, A = number
of ACLs.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Ben Pfaff [Thu, 12 Jul 2018 21:55:31 +0000 (14:55 -0700)]
tests: Ignore recirc_id in "MPLS xlate action" test.
When I run this test with DPDK enabled, it fails because it ends up using
a different recirculation ID when DPDK is not enabled. I guess that's a
little weird but the recirculation IDs are not supposed to be significant,
so this change makes the test ignore it.
Ben Pfaff [Tue, 24 Jul 2018 19:53:20 +0000 (12:53 -0700)]
ovn-nbctl: Make daemon mode more transparent.
This makes ovn-nbctl transparently use daemon mode if an appropriate
environment variable is set.
It also transforms ovn-nbctl.at so that it runs each ovn-nbctl test in
"direct" mode and in daemon mode. It uses a combination of m4 macros and
shell functions to keep from expanding the generated testsuite more than
necessary.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Ben Pfaff [Mon, 6 Aug 2018 21:35:27 +0000 (14:35 -0700)]
raft: Fix use-after-free error in raft_store_snapshot().
raft_store_snapshot() constructs a new snapshot in a local variable then
destroys the current snapshot and replaces it by the new one. Until now,
it has not cloned the data in the new snapshot until it did the
replacement. This led to the unexpected consequence that, if 'servers' in
the old and new snapshots was the same, then it would first be freed and
later cloned, which could cause a segfault.
Multiple people reported the crash. Gurucharan Shetty provided a
reproduction case.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
utilities: Install ovs-tcp{dump, undump} also when only Python3 is enabled
Since commit 793bdb6c0500 ("ovs-tcpdump: Fix incompatibilities with python3")
and commit 227abb77d3d1 ("ovs-tcpundump: Fix incompatibilities with python3")
ovs-tcpdump and ovs-tcpdump works with Python3 as well.
This commit allows ovs-tcpdump and ovs-tcpundump to be installed also when
only Python3 is enabled.
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
When ovsdb-server is starting, it performs some DB steps such as
creating and upgrading the OvS DB. When we are running as
'non-root' user, the 'runuser' tool is used to manage the privileges.
However, when this happens during systemd boot, we observe the following
errors in journald:
Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to
scope's control group: No such process
Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user openvswitch.
Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state.
According to the analysis performed on openSUSE bugzilla[1], it seems
that ovsdb-server.service creates (via the call to runuser) a user
session and therefore call pam_systemd which in its turn tries to start
a systemd user instance: "user@474.service". However "user@474.service"
is supposed to be started after systemd-user-sessions.service which is
supposed to be started after network.target. Additionally,
ovsdb-server.service uses Before=network.target hence the deadlock.
This commit uses "setpriv" instead of "runuser" to launch "ovsdb-tool" that
doesn't use PAM and so it permits to launch "ovsdb-tool" as a user without
having the deadlock. Since some old versions for "setpriv" (such as the
one used by RHEL7) doesn't support the username / groupname, but only the
user ids / group ids, "id" is used to get the user ID and the group IDs.
To replicate the same behaviour of "runuser", the effective group ID of
the user is used as GID (usually "openvswitch") and the remaining group
IDs are used as supplementary groups (usually "hugetlbfs", if OVS is
built with DPDK support).
stream-ssl: Don't enable new TLS versions by default
Currently protocol_flags is populated by the list of SSL and TLS
protocols by hand. This means that when a new TLS version is added to
openssl (in this case TLS v1.3 is added to openssl 1.1.1 beta)
ovsdb-server automatically enable support to it with the default ciphers.
This can be a security problem (since other ciphers can be enabled) and it
also makes a test (SSL db: implementation) to fail.
This commit changes the 'protocol_flags' to use the list of all protocol
flags as provided by openssl library (SSL_OP_NO_SSL_MASK) so there is no
need to keep the list updated by hand.
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 31 Jul 2018 21:51:54 +0000 (14:51 -0700)]
meta-flow: Make mf_vl_mff_mf_from_nxm_header() require a valid field.
All the users of mf_vl_mff_mf_from_nxm_header() expect it to always obtain
a valid field or to report an error. In practice, it did not report an
error in the case where the field was unknown (although it did report an
error in some other cases). This commit fixes the problem.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9652 Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
Ben Pfaff [Wed, 25 Jul 2018 21:10:26 +0000 (14:10 -0700)]
ovsdb-tool: Only check leader completeness when we can, in "check-cluster".
Generally when we know the leader for a term, in "check-cluster", it's
because we read that leader's log file. In that case, we have the leader's
log_end because it told us. However, taking a snapshot can discard that
data. In that case, log_end is 0 and we should not try to check for leader
completeness on that basis.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Ben Pfaff [Wed, 25 Jul 2018 20:57:38 +0000 (13:57 -0700)]
ovsdb-tool: Read server headers first, before full logs, in "check-cluster".
Having the headers available before reading the complete logs means that
server IDs can be associated with the server file names earlier, which can
improve error messages in some cases.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Ben Pfaff [Wed, 25 Jul 2018 17:55:59 +0000 (10:55 -0700)]
tests: Fix use of variable in cluster torture test.
remove_server() is supposed to deal with its argument $i, not $victim. In
this case they happen to have the same value so the difference is moot,
but it's still best to be clear.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Ben Pfaff [Wed, 25 Jul 2018 17:31:47 +0000 (10:31 -0700)]
tests: Fix cluster torture test.
A previous commit to improve timing also caused the cluster torture test to
be skipped (unless it failed early). This is related to the shell "while"
loop's use of a variable $phase to indicate how far it got in the test
procedure. A very fast machine, or one on which the races went just the
right way, might finish the test before all the torture properly starts, so
the code is designed to just skip the test if that happens. However, a
commit to improve the accuracy ended up skipping it all the time.
Prior to the timing commit, the loop looked something like this:
phase=0
while :; do
...things that eventually increment $phase to 2...
done
AT_SKIP_IF([test $phase != 2])
This works fine.
The timing commit changed the "while :" to "(...something...) | while
read". This looks innocuous but it actually causes everything inside the
"while" loop to run in a subshell. Thus, the increments to $phase are not
visible after the loop ends, and the test always gets skipped.
This commit fixes the problem by storing the phase in a file instead of a
shell variable.
Fixes: 0f03ae3754ec ("ovsdb: Improve timing in cluster torture test.") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Ben Pfaff [Thu, 26 Jul 2018 22:43:27 +0000 (15:43 -0700)]
nx-match: Fix memory leak in oxm_pull_field_array() error case.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9424 Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Martin Xu [Wed, 1 Aug 2018 23:00:59 +0000 (16:00 -0700)]
rhel: bug fix kmod spec file, rhel6
This patch fixes a scenario not working for RHEL7.3 in commit 89dd5819cf18.
When multiple versions passed into the kversion for the spec file, the
variable is used as is for the kernel module paths for command
weak-modules --add-modules. Then the modules cannot be found.
Fixes: 89dd5819cf18 (rhel: support kmod-openvswitch build against
multiple kernels, rhel6)
Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com> CC: Greg Rose <gvrose8192@gmail.com> CC: Ben Pfaff <blp@ovn.org> CC: Flavio Leitner <fbl@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>
Martin Xu [Wed, 1 Aug 2018 23:00:58 +0000 (16:00 -0700)]
rhel: support kmod build against multiple 7.2 kernels, rhel6
This patch extends commit 89dd5819cf18 (rhel: support kmod-openvswitch
build against multiple kernels, rhel6) to support building kmod RPMs
with multiple minor revisions within 3.10.0-327 kernels. It was
discovered for RHEL 7.2 that 41.3 minor revision introduced backward
incompatible changes.
Ben Pfaff [Tue, 24 Jul 2018 16:58:56 +0000 (09:58 -0700)]
unixctl: Use absolute paths on Windows too.
When this case was adapted for Windows, asb_file_name() simply didn't work
at all there. Now, it should work OK, and it seems like the right thing
to do, and it makes the code more straightforward too.
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 24 Jul 2018 16:48:45 +0000 (09:48 -0700)]
util: Fix abs_file_name() bugs on Windows.
abs_file_name() believed that a file name that begins with / or contains :
is absolute and that any other file name is relative. On Windows, this is
wrong in at least the following ways:
* / and \ are interchangeable on Windows.
* A name that begins with \\ or // is also absolute.
* A name that begins with X: but not X:\ is not absolute.
* A name with : in some position other than the second position is
not absolute (although it might not be valid either?).
Furthermore, Windows has more than one current working directory (one per
volume letter), so trying to make a file name absolute by just prefixing
the current working directory for the current volume results in silliness.
This patch attempts to fix the problem.
This makes OVS link against shlwapi, which is needed to use
PathIsRelative().
Found by inspection.
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Mark Michelson [Mon, 30 Jul 2018 13:47:45 +0000 (09:47 -0400)]
ovn: Modify restart_controller in ovn-ctl to use --restart
The --restart flag allows for uninterrupted packet flowage when exiting
ovn-controller. This patch modifies the restart_controller argument to
ovn-ctl to use --restart.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Mark Michelson [Mon, 30 Jul 2018 13:47:44 +0000 (09:47 -0400)]
ovn: Add '--restart' flag to ovn-controller exit.
When "--restart" is passed to ovn-controller's exit command, then
database entries are not removed for this hypervisor. This means that
* Encaps
* Chassis
* OVS ports
are not removed.
The reasoning is that if the intent is to restart ovn-controller, this
will allow for tunnels to remain up and allow for traffic not to be
interrupted during the restart. When ovn-controller is started again, it
picks back up from where it was.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
projects / ovs.git / log