Han Zhou [Thu, 4 Oct 2018 20:01:09 +0000 (13:01 -0700)]
ovs-ctl: Add new option to use short hostname.
Current ovs-ctl forces to set full hostname in external-ids. In
some situation users may want to set short hostname. For example,
in OpenStack - OVN integration, Neutron uses the host-id provided
by Nova, which is usually short hostname, to set "requested-chassis"
in OVN. The mismatch in hypervisor's external-ids:hostname setting
causes OVN port binding failure. It can be overridden to short name
but a openvswitch restart using ovs-ctl would again set it to full
hostname. This patch ensures in such use cases --no-full-hostname
can be specified to ovs-ctl to set short hostname instead.
Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Fri, 5 Oct 2018 22:16:50 +0000 (15:16 -0700)]
extend-table: Fix a bug that iterates wrong table
This seems to be a copy and paste bug that iterates and frees
the wrong table. This commit fixes that.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10730 Co-authored-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yi-Hung Wei [Fri, 5 Oct 2018 16:19:54 +0000 (09:19 -0700)]
ofp-packet: Fix NXT_RESUME with geneve tunnel metadata
The patch address vswitchd crash when it receives NXT_RESUME with geneve
tunnel metadata. The crash is due to segmentation fault with the
following stack trace, and it is observed only in kernel datapath.
A test is added to prevent regression.
Thread 1 "ovs-vswitchd" received signal SIGSEGV, Segmentation fault.
0 0x00007fcffd0c5412 in tun_metadata_to_geneve__ (flow=flow@entry=0x7ffcb7106680, b=b@entry=0x7ffcb70eb5a8, crit_opt=crit_opt@entry=0x7ffcb70eb287)
at lib/tun-metadata.c:676
1 0x00007fcffd0c6858 in tun_metadata_to_geneve_nlattr_flow (b=0x7ffcb70eb5a8, flow=0x7ffcb7106638) at lib/tun-metadata.c:706
2 tun_metadata_to_geneve_nlattr (tun=tun@entry=0x7ffcb7106638, flow=flow@entry=0x7ffcb7106638, key=key@entry=0x0, b=b@entry=0x7ffcb70eb5a8)
at lib/tun-metadata.c:810
3 0x00007fcffd048464 in tun_key_to_attr (a=a@entry=0x7ffcb70eb5a8, tun_key=tun_key@entry=0x7ffcb7106638, tun_flow_key=tun_flow_key@entry=0x7ffcb7106638,
key_buf=key_buf@entry=0x0, tnl_type=<optimized out>, tnl_type@entry=0x0) at lib/odp-util.c:2886
4 0x00007fcffd0551cf in odp_key_from_dp_packet (buf=buf@entry=0x7ffcb70eb5a8, packet=0x7ffcb7106590) at lib/odp-util.c:5909
5 0x00007fcffd0d7870 in dpif_netlink_encode_execute (buf=0x7ffcb70eb5a8, d_exec=0x7ffcb7106428, dp_ifindex=<optimized out>) at lib/dpif-netlink.c:1873
6 dpif_netlink_operate__ (dpif=dpif@entry=0xe65e00, ops=ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at lib/dpif-netlink.c:1959
7 0x00007fcffd0d842e in dpif_netlink_operate_chunks (n_ops=1, ops=0x7ffcb7106418, dpif=<optimized out>) at lib/dpif-netlink.c:2258
8 dpif_netlink_operate (dpif_=0xe65e00, ops=<optimized out>, n_ops=<optimized out>) at lib/dpif-netlink.c:2294
9 0x00007fcffd014680 in dpif_operate (dpif=<optimized out>, ops=<optimized out>, ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at lib/dpif.c:1359
10 0x00007fcffd014c58 in dpif_execute (dpif=<optimized out>, execute=execute@entry=0x7ffcb71064e0) at lib/dpif.c:1324
11 0x00007fcffd40d3e6 in nxt_resume (ofproto_=0xe6af50, pin=0x7ffcb7107150) at ofproto/ofproto-dpif.c:4885
12 0x00007fcffd3f88c3 in handle_nxt_resume (ofconn=ofconn@entry=0xf8c8f0, oh=oh@entry=0xf7ebd0) at ofproto/ofproto.c:3612
13 0x00007fcffd404a3b in handle_openflow__ (msg=0xeac460, ofconn=0xf8c8f0) at ofproto/ofproto.c:8137
14 handle_openflow (ofconn=0xf8c8f0, ofp_msg=0xeac460) at ofproto/ofproto.c:8258
15 0x00007fcffd3f4653 in ofconn_run (handle_openflow=0x7fcffd4046f0 <handle_openflow>, ofconn=0xf8c8f0) at ofproto/connmgr.c:1432
16 connmgr_run (mgr=0xe422f0, handle_openflow=handle_openflow@entry=0x7fcffd4046f0 <handle_openflow>) at ofproto/connmgr.c:363
17 0x00007fcffd3fdc76 in ofproto_run (p=0xe6af50) at ofproto/ofproto.c:1821
18 0x000000000040ca94 in bridge_run__ () at vswitchd/bridge.c:2939
19 0x0000000000411d44 in bridge_run () at vswitchd/bridge.c:2997
20 0x00000000004094fd in main (argc=12, argv=0x7ffcb71085b8) at vswitchd/ovs-vswitchd.c:119
VMWare-BZ: #2210216 Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Add buffering support for IPv4/IPv6 packets that will be processed
by arp{}/nd_ns{} action when L2 address is not discovered yet since
otherwise the packet will be substituted with an ARP/Neighbor
Solicitation frame and this will result in the lost of the first
packet of the connection.
Moreover fix following automatic tests broken by ip-buffering support
since now original ip packets are transmitted by OVN logical
router:
- ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR
- ovn -- /32 router IP address
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Matteo Croce [Sat, 6 Oct 2018 16:19:55 +0000 (18:19 +0200)]
dpif-netlink: Fix null pointer.
In dpif_netlink_port_add__(), socksp could be NULL, because
vport_socksp_to_pids() would allocate a new array and return a single
zero element.
Following vport_socksp_to_pids() removal, a NULL pointer can happen when
dpif_netlink_port_add__() is called and dpif->handlers is 0.
Restore the old behaviour of using a zero pid when dpif->handlers is 0.
Fixes: 69c51582f ("dpif-netlink: don't allocate per thread netlink sockets") Reported-by: Flavio Leitner <fbl@redhat.com> Reported-by: Guru Shetty <guru@ovn.org> Signed-off-by: Matteo Croce <mcroce@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
PCAP header magic numbers are different for microsecond and nanosecond
resolution timestamps. This patch adds support for understanding the
difference and reporting the time correctly with ovs_pcap_read().
When writing pcap files, OVS will always use microsecond resolution, so
no new calculations were added to those functions.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Calling getattr() on a Row object after invoking delkey() with a value
that does not exist in the object will cause getattr() to fail with a
KeyError error. For example:
Oct 05 14:59:28 neutron-server[28435]: File
"/usr/local/lib/python2.7/dist-packages/ovsdbapp/backend/ovs_idl/connection.py",
line 122, in run
Oct 05 14:59:28 neutron-server[28435]:
txn.results.put(txn.do_commit())
Oct 05 14:59:28 neutron-server[28435]: File
"/usr/local/lib/python2.7/dist-packages/ovsdbapp/backend/ovs_idl/transaction.py",
line 86, in do_commit
Oct 05 14:59:28 neutron-server[28435]: command.run_idl(txn)
Oct 05 14:59:28 neutron-server[28435]: File
"/usr/local/lib/python2.7/dist-packages/ovsdbapp/backend/ovs_idl/command.py",
line 299, in run_idl
Oct 05 14:59:28 neutron-server[28435]: if
isinstance(getattr(record, self.column), dict):
Oct 05 14:59:28 neutron-server[28435]: File
"/usr/local/lib/python2.7/dist-packages/ovs/db/idl.py", line 843, in
__getattr__
Oct 05 14:59:28 neutron-server[28435]: del dmap[key]
Oct 05 14:59:28 neutron-server[28435]: KeyError: 'bogusvalue'
This patch is replacing the "del dmap[key]" instruction with a
"dmap.pop(key, None)" instruction instead because a pop() (with a
default value) will not raise an exception in case the key does not
exist in the object in the first place, it will just ignore it.
Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 4 Oct 2018 19:42:21 +0000 (12:42 -0700)]
ovsdb-types: Refactor structs so as to comply with C++ standard
C++ standard only accepts anonymous struct inside
anonymous union. This patch re-organized the structs so
that this header file can be used in C++ source files.
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Mon, 18 Jun 2018 18:36:50 +0000 (11:36 -0700)]
ovn-controller: Honor updates to SSL configuration while waiting for SB DB.
At startup time, ovn-controller connects to the OVS database and retrieves
a pointer to the southbound database, then connects to the southbound
database and retrieves a snapshot. Until now, however, it didn't pay
attention to changes in the OVS database while trying to retrieve the
southbound database, which meant that if the SSL settings changed,
ovn-controller would continue to use the old ones, which probably wouldn't
work.
Also honor changes to the remote for the southbound database while waiting
to connect to it.
Most of the changes in this commit are whitespace only indentation changes,
so passing -w to "git show" (etc.) make it easier to understand.
Reported-by: Dan Williams <dcbw@redhat.com>
Reported-at: https://github.com/openvswitch/ovs-issues/issues/144 Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Mon, 18 Jun 2018 18:36:49 +0000 (11:36 -0700)]
ovsdb-idl: New function ovsdb_idl_create_unconnected().
This new function makes it possible to create an instance of the IDL
without connecting it to a remote OVSDB server. The caller can then
connect and disconnect using ovsdb_idl_set_remote(); the ability to
disconnect is a new feature.
With this patch, the ovsdb_idl 'session' member can be null whereas
previously it was always nonnull. The scattered changes throughout
ovsdb-idl are to cope with this new possibility.
An upcoming patch will introduce the first user of this new feature.
Ben Pfaff [Fri, 7 Sep 2018 02:30:11 +0000 (19:30 -0700)]
condition: Reject <, <=, >=, > with optional scalar against empty set.
When relational comparisons against optional scalars were introduced, it
was meant to work only when the right-hand side of the comparison was a
scalar, not the empty set. The implementation wasn't that picky. This
commit fixes the problem.
CC: Terry Wilson <twilson@redhat.com> Fixes: 09e256031a62 ("ovsdb: Allow comparison on optional scalar types") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Ben Pfaff [Fri, 7 Sep 2018 02:30:10 +0000 (19:30 -0700)]
condition: Fix ==, !=, includes, excludes on optional scalars.
Open vSwitch 2.4 introduced an OVSDB extension in which a column with
type optional integer or real could be compared with the operators <,
<=, >, and >=. At the same time, it broke the implementation of the
operators ==, !=, includes, and excludes on columns with the same types.
This fixes the problem.
Reported-by: Hans Ole Rafaelsen <hrafaelsen@gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047356.html CC: Terry Wilson <twilson@redhat.com> Fixes: 09e256031a62 ("ovsdb: Allow comparison on optional scalar types") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Ben Pfaff [Tue, 2 Oct 2018 22:53:16 +0000 (15:53 -0700)]
AUTHORS: Update or remove all Nicira email addresses.
nicira.com email addresses stopped working long ago. For folks still at
VMware, this updates them. (Also for Martin, because I feel like it.)
For everyone else, this deletes the addresses.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
Han Zhou [Wed, 3 Oct 2018 22:11:20 +0000 (15:11 -0700)]
bfd: Make the tp_dst masking megaflow-friendly.
When there are tunnel ports with BFD enabled, all UDP flows will have
dst port as match condition in datapath, which causes unnecessarily
high flow miss for all UDP traffic, and results in latency increase.
This patch solves the problem by masking tp_dst only for a single
bit that is enough to tell the mismatch when it is not BFD traffic.
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047360.html Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
This patch adds a new oss-fuzz target for the ODP
parser. The target harness has been adapted from test-odp.c. Prominently, it
leaves out "parse_filter" due to an unresolvable bug in that code at the
time of writing.
It also includes the following:
- a fuzzing dictionary
- fuzzing config
- some automake additions for the new target
Signed-off-by: Bhargava Shastry <bshastry@sect.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org>
Andre McCurdy [Wed, 3 Oct 2018 20:23:07 +0000 (13:23 -0700)]
m4: handle configuring with PYTHON="/usr/bin/env python" etc
It may sometimes be desirable to have the python path determined at
run time, ie by setting PYTHON="/usr/bin/env python" etc.
Unfortunately that currently fails as $ovs_cv_python2 and
$ovs_cv_python3 are not always quoted and therefore fail to parse
correctly if PYTHON / PYTHON2 / PYTHON3 are set to values containing
spaces.
Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
This reverts commit 6b4d0211e84a ("bridge: Fix ovs-appctl qos/show
repeated queue information"), which is no longer necessary now that
commit 65f3c34c7417 ("netdev: Properly clear 'details' when iterating
in NETDEV_QOS_FOR_EACH.") has been applied. The former commit fixed
a symptom of the root cause fixed by the latter.
Ben Pfaff [Tue, 2 Oct 2018 23:02:59 +0000 (16:02 -0700)]
netdev: Properly clear 'details' when iterating in NETDEV_QOS_FOR_EACH.
The function comment for netdev_queue_dump_next() said that it cleared its
'detail' argument, but it didn't actually do that, which meant that details
could be incorrectly carried along from one queue to the next.
The max_len field, which stipulates the maximum length of
fuzzed input to feed a fuzzer target, was set to 100 for
the ovn expr_parse_target.
In discussions with Ben, this was deemed too restrictive.
Google folks also suggested the removal of this option
since it cannot be uniformly passed on to all fuzzer
engines behind oss-fuzz.
For the future, the suggested way to limit the length of
fuzzed input is in the C code of the test harness itself.
Signed-off-by: Bhargava Shastry <bshastry@sect.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org>
In previous code, if hexit == 0, then the boundary for 'out' is
not checked. This patch fixes it.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10710 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
datapath-windows: Move OVS_IPHELPER_INSTANCE to IpHelper.h
Move the IPHelper Instance to the main header file and update the usage to
explicitly point to POVS_IPHELPER_INSTANCE instead of PVOID. Also rename
the ipn->context to ipn->instance to make it more readable.
Found by inspection.
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>
The fuzzer target, expr_parse_target.c, comprises test cases adapted
from test-ovn.c.
In addition, this patch contains configuration files for oss-fuzz,
including a dictionary, expr.dict, to aid quick path discovery and a
fuzzer configuration file that customises fuzzing for this target.
Prominently, the patch sets the maximum length of fuzzed input
(the string accepted by lexer/expression parser) to be up to 100
characters long not containing a newline character.
Signed-off-by: Bhargava Shastry <bshastry at sect.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org>
With gcc 7.3.0 a warning is given about two variables possibly being
uninitialized in compose_sample_action(). The code path only allows the
variables to be used if they've been initialized, so this warning is
incorrect. However, this change allows a clean build.
sflow: Set agent address properly based on collector address.
If an agent address is not provided, OVS tries to choose a source
address based on the source IP that would be used to connect to the
sFlow collector. The code previously set the agent address to the
collector's address instead of using the calculated source address.
This patch properly uses the source address.
Reported-by: Neil McKee <neil.mckee@inmon.com> Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Wed, 26 Sep 2018 23:11:40 +0000 (16:11 -0700)]
ofproto: Fix build with some GCC versions.
GCC 4.8.x and possibly other versions don't like a designated initializer
for an anonymous struct, see e.g.
https://travis-ci.org/openvswitch/ovs/jobs/433747674
Fixes: f836888d28ec ("ofproto: Handle OpenFlow version mismatch for requestforward with groups.") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
This patch addresses the issue that the conntrack fields associated
with a packet are missing after a packet is resumed by NXT_RESUME.
For example, the last rule in the following OpenFlow pipeline is not
working without this patch.
Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in "continuations".")
VMware-BZ: #2202764 Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 25 Sep 2018 22:14:13 +0000 (15:14 -0700)]
dpif: Remove support for multiple queues per port.
Commit 69c51582ff78 ("dpif-netlink: don't allocate per thread netlink
sockets") removed dpif-netlink support for multiple queues per port.
No remaining dpif provider supports multiple queues per port, so
remove infrastructure for the feature.
CC: Matteo Croce <mcroce@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Anju Thomas [Mon, 24 Sep 2018 17:29:34 +0000 (22:59 +0530)]
ofproto-dpif-xlate: Fix load balancing for select groups with MPLS.
Before this commit, OVS did not do load balancing for select group buckets
in case of mpls tagged packets. After an MPLS pop action, the expectation
is to trigger recirculation. This recirculation will ensure an RSS
re-computation which will ensure load balancing in case of select group
bucket. Due to a missing return statement before bucket selection, the
bucket selection in case of select group happens before the recirculation
and hence no load balancing is achieved.
Signed-off-by: Anju Thomas <anju.thomas@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 25 Sep 2018 21:06:37 +0000 (14:06 -0700)]
ofproto: Handle OpenFlow version mismatch for requestforward with groups.
OpenFlow 1.4+ supports a feature called requestforward. When a controller
enables this feature, the switch sends that controller a copy of other
controllers' group and meter modification requests. OpenFlow 1.5 supports
some group features not in OpenFlow 1.4. When OVS attempted to forward
such requests to an OpenFlow 1.4 controller, it reported an error and
exited. This commit fixes the problem by making OVS properly translate the
messages to OpenFlow 1.4 format.
Reported-by: Pierre Cregut <pierre.cregut@orange.com> Tested-by: Pierre Cregut <pierre.cregut@orange.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047453.html Signed-off-by: Ben Pfaff <blp@ovn.org>
Fixes memory leaked by call to ovn_extend_table_init that is missing a
corresponding ovn_extend_table_destroy in test-ovn.c. This fixes leaks for
the group_table and meter_table objects.
Signed-off-by: Bhargava Shastry <bshastry@sect.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org>
Martin Xu [Thu, 20 Sep 2018 19:19:30 +0000 (12:19 -0700)]
rhel: fix wrong condition check for ovs-kmod-manage.sh, fedora
In post-install in kmod fedora spec file, the variables storing
different parts of kernel version numbers are renamed. The condition
check to run ovs-kmod-manage.sh for RHEL 7.2 and 7.4 uses the older
variables.
Fixes: c3570519ecaf (rhel: add 4.4 kernel in kmod build with mulitple versions, fedora) Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com> CC: Greg Rose <gvrose8192@gmail.com> CC: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 18 Sep 2018 09:11:20 +0000 (02:11 -0700)]
acinclude.m4: Really check whether GCC support -Wno-null-pointer-arithmetic.
I've noticed recently an annoying quantity of error messages like the
following in builds in various places:
gcc: error: unrecognized command line option ‘-Wunknown-warning-option’
This didn't really make sense because OVS checks whether the compiler
supports warning options before it uses them. Looking closer, the GCC
manual has a note that explains the issue:
When an unrecognized warning option is requested (e.g.,
'-Wunknown-warning'), GCC emits a diagnostic stating that the
option is not recognized. However, if the '-Wno-' form is used,
the behavior is slightly different: no diagnostic is produced for
'-Wno-unknown-warning' unless other diagnostics are being
produced. This allows the use of new '-Wno-' options with old
compilers, but if something goes wrong, the compiler warns that
an unrecognized option is present.
Thus, we can properly check only for the *positive* version of a warning
option, so this commit makes the OVS tests do that.
Martin Xu [Thu, 20 Sep 2018 19:19:29 +0000 (12:19 -0700)]
rhel: use _datadir as path prefix for ovs-kmod-manage.sh, fedora
This patch fixes the path for ovs-kmod-manage.sh script in the
openvswitch-kmod RPM in fedora spec file. Currently the path prefix is
hard coded to /usr/share. Use %{_datadir} instead.
Fixes: 22c33c303932 (rhel: support kmod build against mulitple kernel versions, fedora) Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com> CC: Greg Rose <gvrose8192@gmail.com> CC: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Flavio Leitner <fbl@sysclose.org>
dpif-netlink: don't allocate per thread netlink sockets
When using the kernel datapath, OVS allocates a pool of sockets to handle
netlink events. The number of sockets is: ports * n-handler-threads, where
n-handler-threads is user configurable and defaults to 3/4*number of cores.
This because vswitchd starts n-handler-threads threads, each one with a
netlink socket for every port of the switch. Every thread then, starts
listening on events on its set of sockets with epoll().
On setup with lot of CPUs and ports, the number of sockets easily hits
the process file descriptor limit, and ovs-vswitchd will exit with -EMFILE.
Change the number of allocated sockets to just one per port by moving
the socket array from a per handler structure to a per datapath one,
and let all the handlers share the same sockets by using EPOLLEXCLUSIVE
epoll flag which avoids duplicate events, on systems that support it.
The patch was tested on a 56 core machine running Linux 4.18 and latest
Open vSwitch. A bridge was created with 2000+ ports, some of them being
veth interfaces with the peer outside the bridge. The latency of the upcall
is measured by setting a single 'action=controller,local' OpenFlow rule to
force all the packets going to the slow path and then to the local port.
A tool[1] injects some packets to the veth outside the bridge, and measures
the delay until the packet is captured on the local port. The rx timestamp
is get from the socket ancillary data in the attribute SO_TIMESTAMPNS, to
avoid having the scheduler delay in the measured time.
The first test measures the average latency for an upcall generated from
a single port. To measure it 100k packets, one every msec, are sent to a
single port and the latencies are measured.
The second test is meant to check latency fairness among ports, namely if
latency is equal between ports or if some ports have lower priority.
The previous test is repeated for every port, the average of the average
latencies and the standard deviation between averages is measured.
The third test serves to measure responsiveness under load. Heavy traffic
is sent through all ports, latency and packet loss is measured
on a single idle port.
The fourth test is all about fairness. Heavy traffic is injected in all
ports but one, latency and packet loss is measured on the single idle port.
This is the test setup:
# nproc
56
# ovs-vsctl show |grep -c Port
2223
# ovs-ofctl dump-flows ovs_upc_br
cookie=0x0, duration=4.827s, table=0, n_packets=0, n_bytes=0, actions=CONTROLLER:65535,LOCAL
# uname -a
Linux fc28 4.18.7-200.fc28.x86_64 #1 SMP Mon Sep 10 15:44:45 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
And these are the results of the tests:
Stock OVS Patched
netlink sockets
in use by vswitchd
lsof -p $(pidof ovs-vswitchd) \
|grep -c GENERIC 91187 2227
Test 1
one port latency
min/avg/max/mdev (us) 2.7/6.6/238.7/1.8 1.6/6.8/160.6/1.7
Test 2
all port
avg latency/mdev (us) 6.51/0.97 6.86/0.17
Test 3
single port latency
under load
avg/mdev (us) 7.5/5.9 3.8/4.8
packet loss 95 % 62 %
Test 4
idle port latency
under load
min/avg/max/mdev (us) 0.8/1.5/210.5/0.9 1.0/2.1/344.5/1.2
packet loss 94 % 4 %
CPU and RAM usage seems not to be affected, the resource usage of vswitchd
idle with 2000+ ports is unchanged:
# ps u $(pidof ovs-vswitchd)
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
openvsw+ 5430 54.3 0.3 4263964 510968 pts/1 RLl+ 16:20 0:50 ovs-vswitchd
Additionally, to check if vswitchd is thread safe with this patch, the
following test was run for circa 48 hours: on a 56 core machine, a
bridge with kernel datapath is filled with 2200 dummy interfaces and 22
veth, then 22 traffic generators are run in parallel piping traffic into
the veths peers outside the bridge.
To generate as many upcalls as possible, all packets were forced to the
slowpath with an openflow rule like 'action=controller,local' and packet
size was set to 64 byte. Also, to avoid overflowing the FDB early and
slowing down the upcall processing, generated mac addresses were restricted
to a small interval. vswitchd ran without problems for 48+ hours,
obviously with all the handler threads with almost 99% CPU usage.
This skip including floatn-common.h if it's not available since it
was introduced in glibc 2.27 and OVS doesn't not actually require
that to work with previous glibc version.
Fixes: 07aec2ac1 sparse: Support newer GCC/glibc versions. Signed-off-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 21 Sep 2018 18:25:55 +0000 (11:25 -0700)]
flow: Fix uninitialized flow fields in IPv6 error case.
When parse_ipv6_ext_hdrs__() returned false, half a 64-bit word had been
pushed into the miniflow and the second half was left uninitialized. This
commit fixes the problem.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10518 Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Yifeng Sun [Tue, 18 Sep 2018 21:39:57 +0000 (14:39 -0700)]
tests: Fix broken test of 'truncate and output to gre tunnel'
The test 'truncate and output to gre tunnel' is broken on certain kernels
where OVS kernel module and upstream GRE module can't co-exist. This
patch creates a test that doesn't depend on upstream GRE module but
provides the same testing.
The replaced test is skipped on problematic kernel versions.
On centos, this test may fail due to the default rules of iptables.
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Currently, OVS does not update the flow stats after a packet is
restarted by NXT_RESUME message. This patch fixes the aforementioned
issue and adds an unit test to prevent regression.
Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in "continuations".")
VMware-BZ: #2198435 Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 20 Sep 2018 18:01:35 +0000 (11:01 -0700)]
meta-flow: Make "nw_frag" a synonym for "ip_frag".
Since the time that OVS introduced support for IP fragments, the OVS
functions that format flows have used "nw_frag", but the ones that parse
flows have expected "ip_frag". Obviously this is a bug and it's a surprise
that it's gone so long without anyone reporting the problem. This fixes
it and adds a test.
Yifeng Sun [Tue, 18 Sep 2018 17:35:59 +0000 (10:35 -0700)]
gre: Rename fallback devices to avoid udev's interference
On certain kernel versions, when openvswitch kernel module creates
a gre0 interface, the kernel’s gre module will jump out and compete
to control the gre0 interface. This will cause the failure of
openvswitch kernel module loading.
This fix renames fallback devices by adding a prefix "ovs-".
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
VMware Issue: #2162866
ovsdb-server: Alleviate the possible data loss in an active/standby setup
The present code resets the database when it is in the state -
'RPL_S_SCHEMA_REQUESTED' and repopulates the database when it
receives the monitor reply when it is in the state -
'RPL_S_MONITOR_REQUESTED'. If however, it goes to active mode
before it processes the monitor reply, the whole data is lost.
This patch alleviates the problem by resetting the database when it
receives the monitor reply (before processing it). So that
reset and repopulation of the db happens in the same state.
This approach still has a window for data loss if the function
process_notification() when processing the monitor reply fails for
some reason or ovsdb-server crashes for some reason during
process_notification().
Reported-by: Han Zhou <zhouhan@gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047161.html Tested-by: aginwala <aginwala@ebay.com> Acked-by: Han Zhou <zhouhan@gmail.com> Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Mon, 10 Sep 2018 20:00:59 +0000 (13:00 -0700)]
ovsdb-idlc: Use ALIGNED_CAST to avoid spurious warnings for index rows.
The *_index_init_row() function casts a generic ovsdb_idl_row pointer to
a specific type of row pointer. This can cause an increase in required
alignment with some kinds of data on some architectures. GCC complains,
e.g.:
lib/vswitch-idl.c: In function 'ovsrec_flow_sample_collector_set_index_init_row'
lib/vswitch-idl.c:9277:12: warning: cast increases required alignment of target
However, rows are always allocated with malloc(), which returns member
suitable for any type, so this is a false positive warning and this commit
suppresses it.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Han Zhou <zhouhan@gmail.com>
Martin Xu [Wed, 12 Sep 2018 19:43:51 +0000 (12:43 -0700)]
rhel: Ship ovs shared libraries, fedora
This patch extends 4886d4d2495b (debian, rhel: Ship ovs shared libraries
and header files) to fedora, by packaging the shared libraries in
openvswitch and openvswitch-dvel RPM. These files are always packaged in
the RPMs built with rhel6 spec file.
When we originally added commit 2bdd1f3d96 it was part of an
effort to work around gre module conflicts found while enabling
the ERSPAN feature. Testing at the time did not show any benefit
so in commit a94f9524db we reverted it. However, further
developments showed that in some corner cases it did have a
benefit and it did not do any harm so we reverted the original
revert to restore the code.
Signed-off-by: Greg Rose <roseg@vmware.com> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
Kevin Traynor [Fri, 31 Aug 2018 08:47:55 +0000 (09:47 +0100)]
dpif-netdev: Add round-robin based rxq to pmd assignment.
Prior to OVS 2.9 automatic assignment of Rxqs to PMDs
(i.e. CPUs) was done by round-robin.
That was changed in OVS 2.9 to ordering the Rxqs based on
their measured processing cycles. This was to assign the
busiest Rxqs to different PMDs, improving aggregate
throughput.
For the most part the new scheme should be better, but
there could be situations where a user prefers a simple
round-robin scheme because Rxqs from a single port are
more likely to be spread across multiple PMDs, and/or
traffic is very bursty/unpredictable.
Add 'pmd-rxq-assign' config to allow a user to select
round-robin based assignment.
Signed-off-by: Kevin Traynor <ktraynor@redhat.com> Acked-by: Eelco Chaudron <echaudro@redhat.com> Acked-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
ovs-save: Don't always include the default flow during restore
Currently the default flow (actions=NORMAL) is present in the flow table after
the flow table is restored also when the default flow is removed.
This commit changes the behaviour of the "ovs-save save-flows" command to use
"replace-flows" instead of "add-flows" to restore the flows. This is needed in
order to always have the new flow table as it was before restoring it.
Gavi Teitz [Fri, 10 Aug 2018 08:30:08 +0000 (11:30 +0300)]
dpctl: Expand the flow dump type filter
Added new types to the flow dump filter, and allowed multiple filter
types to be passed at once, as a comma separated list. The new types
added are:
* tc - specifies flows handled by the tc dp
* non-offloaded - specifies flows not offloaded to the HW
* all - specifies flows of all types
The type list is now fully parsed by the dpctl, and a new struct was
added to dpif which enables dpctl to define which types of dumps to
provide, rather than passing the type string and having dpif parse it.
Signed-off-by: Gavi Teitz <gavi@mellanox.com> Acked-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Gavi Teitz [Fri, 10 Aug 2018 08:30:07 +0000 (11:30 +0300)]
dpif-netdev: Initialize dpif_flow attrs
In a previous commit, the dpif_flow struct was expanded, with the
'offloaded' field being moved into a new struct which also includes a
field for the dp layer the flow is handled on. The initialization of
these fields was only done in dpif-netlink.
This completes that commit, by initializing the fields in dpif-netdev
as well. As the 'offloaded' field was previously ignored by
dpif-netdev, the attrs are initialized to the default values of
'false' for the offloaded state, and 'ovs' for the dp layer.
Fixes: d63ca5329ff9 ("dpctl: Properly reflect a rule's offloaded to HW state") Signed-off-by: Gavi Teitz <gavi@mellanox.com> Acked-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Ben Pfaff [Thu, 6 Sep 2018 22:42:06 +0000 (15:42 -0700)]
odp-util: Don't attempt to write IPv6 flow label bits that don't exist.
The ipv6_label field member of struct ovs_key_ipv6 is 32 bits in size,
but an IPv6 label is only 20 bits, so the upper 12 bits are not writable
and must be 0 in the mask. The code wasn't careful about this so it could
try to write them anyway. This commit fixes the problem.
Reported-by: nm_r@directbox.com
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047357.html Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
lib/tc: treat vlan id and prio as independent fields
Previously the key was used to check the presence of vlan id and
prio fields instead of using the mask. Additionally the vlan id
field was considered to be present if only the prio field was set,
and vice versa. f.e. setting the following:
Resulted in (instead of wildcarding vlan_id, filter matches 0):
filter protocol 802.1Q pref 1 flower chain 0
filter protocol 802.1Q pref 1 flower chain 0 handle 0x1
vlan_id 0
vlan_prio 2
vlan_ethtype ip
eth_type ipv4
ip_flags nofrag
in_hw
action order 1: mirred (Egress Redirect to device eth1) stolen
index 2 ref 1 bind 1 installed 5 sec used 5 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
cookie 47040ae7a94fff6afd7ed8aa04b11ba4
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> Reviewed-by: Simon Horman <simon.horman@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Ben Pfaff [Thu, 30 Aug 2018 20:58:50 +0000 (13:58 -0700)]
tests: Add regression tests for all the bugs found by oss-fuzz so far.
This will make it harder for bugs found by oss-fuzz to reappear.
Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 30 Aug 2018 20:58:49 +0000 (13:58 -0700)]
ofp-port: Further cleanups and fixes for ofputil_decode_port_stats().
This fixes leaks on the error path in parse_intel_port_custom_property().
ofp_print_ofpst_port_reply() failed to free the custom_stats in decoded
port stats. This fixes the problem.
parse_intel_port_custom_property() had a memory leak if there was more than
one custom stats property (which there shouldn't be, but still). This
fixes the problem.
There was a function netdev_free_custom_stats_counters() meant for freeing
custom_stats, but hardly anything used it. This adopts it consistently.
It wasn't safe to free the custom stats if ofputil_decode_port_stats()
returned an error. Using netdev_free_custom_stats_counters() avoids this
pitfall.
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9972 Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Louis Peens [Wed, 5 Sep 2018 13:26:24 +0000 (15:26 +0200)]
lib/tc: reject offloading of non-Ethernet packets
When a packet is marked with the special ethtype of OFP_DL_TYPE_NOT_ETH_TYPE
it got wrongly installed into tc datapath as a match on a packet with that
ethtype. This prevents that from happening.
Signed-off-by: Louis Peens <louis.peens@netronome.com> Reviewed-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> Reviewed-by: Simon Horman <simon.horman@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Ben Pfaff [Fri, 7 Sep 2018 17:39:18 +0000 (10:39 -0700)]
vswitch.xml: Better explain vlan-limit.
CC: Eric Garver <e@erig.me> Requested-by: Jerry Lilijun <jerry.lilijun@huawei.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Eric Garver <e@erig.me>
This patch will make sure VXLAN tunnels with and without the group
based policy (GBP) option enabled can not coexist on the same
destination UDP port.
In theory, VXLAN tunnel with and without GBP enables can be
multiplexed on the same UDP port as long as different VNI's are
used. However currently OVS does not support this, hence this patch to
check for this condition.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
dhparams: Fix .c file generation with OpenSSL >= 1.1.1-pre9
Since OpenSSL upstream commit 201b305a2409
("apps/dsaparam.c generates code that is intended to be pasted or included into
an existing source file: the function is static, and the code doesn't include
dsa.h. Match the generated C source style of dsaparam.") "openssl dhparam -C"
generates the get_dh functions as static, but the functions are used inside
stream-ssl.c and so the static keyword cannot be used.
This commit removes the static keyword from the get_dh functions during
dhparams.c file generation by restoring the current behaviour.
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Add TC offload support for classifying single MPLS tagged traffic.
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> Reviewed-by: Simon Horman <simon.horman@netronome.com> Reviewed-by: John Hurley <john.hurley@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Or Gerlitz [Thu, 6 Sep 2018 10:52:26 +0000 (13:52 +0300)]
lib/tc: Avoid matching on tunnel ttl or tos if not needed
The tunnel ttl key is not masked when provided to the tc lib, hence we
wrongly attempted to match on it, when we got non zero ttl key with a zero
mask. Fix it by applying the mask. Use the same practice for the tunnel tos.
Fixes: dd83253e117c ('lib/tc: Support matching on ip tunnel tos and ttl') Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com> Reported-by: Eli Britstein <elibr@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
ovs-ctl: Allow add-remote without vswitchd started.
'add_managers ()' is filtering add-remote if vswitchd is not started.
However, if we actually filter here we end up with a bricked system,
blackholing all traffic. Allowing add_manager() to proceed may mean
extra churn in controllers in some cases, but this is far better than
the alternative of a bricked system.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
system-traffic: Fix conntrack per zone limit test.
Commit 3f1087c added a per zone limit test which relied on the
CHECK_CT_DPIF_FLUSH_BY_CT_TUPLE m4 macro to skip the test when executing
in a userspace datapath (since the per zone limit feature is not yet
implemented in userspace). That macro, however, has been removed in
commit 271e48a ("conntrack: Support conntrack flush by ct 5-tuple")
which was causing the test to fail when executing in userspace.
Instead, a new m4 macro, CHECK_CT_DPIF_PER_ZONE_LIMIT, is introduced to
make the same differentiation, until userspace doesn't support the per
zone limit.
CC: Yi-Hung Wei <yihung.wei@gmail.com> Fixes: 3f1087c ("system-traffic: Add conntrack per zone limit test case") Signed-off-by: Tiago Lam <tiago.lam@intel.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>
Mark Michelson [Thu, 6 Sep 2018 20:01:38 +0000 (16:01 -0400)]
ovn: Detect and prevent duplicate address assignments.
This patch alters the 'ovn-nbctl lsp-set-addresses' command to check if
the IP addresses being added are duplicates of already-set IP addresses.
Test cases have been added for this detection.
This patch also adds a warning message to ovn-northd if duplicate IPv4
addresses are detected on a switch.
Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
erspan: set erspan_ver to 1 by default when adding an erspan dev
After erspan_ver is introudced, if erspan_ver is not set in iproute, its
value will be left 0 by default. Since Commit 02f99df1875c ("erspan: fix
invalid erspan version."), it has broken the traffic due to the version
check in erspan_xmit if users are not aware of 'erspan_ver' param, like
using an old version of iproute.
To fix this compatibility problem, it sets erspan_ver to 1 by default
when adding an erspan dev in erspan_setup. Note that we can't do it in
ipgre_netlink_parms, as this function is also used by ipgre_changelink.
Fixes: 02f99df1875c ("erspan: fix invalid erspan version.") Reported-by: Jianlin Shi <jishi@redhat.com> Signed-off-by: Xin Long <lucien.xin@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Fixes: 5e720da59d ("erspan: fix invalid erspan version.") Cc: Xin Long <lucien.xin@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
ovn.at: Skip ACL rate-limiting test on slow/overloaded systems.
In ACL rate-limiting test, we send three sets of 100 packets. One of
the sets drops packets at a rate of 10 per second, one at a rate of 5
per second, and one not at all. On my setup, it takes roughly 0.67
seconds to send those 300 packets, but we have reports of it taking over
15 seconds on others. The test was intended to allow some flexibility
in run-time, but it's very difficult to design a mechanism that can all
possibilities.
To prevent false test failures, this patch changes the test to check
the duration count of the meter, and if it's greater than nine seconds,
just skip the test.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Reported-by: Thomas Goirand <zigo@debian.org>
projects / mirror_ovs.git / log