Ben Pfaff [Tue, 6 Dec 2016 21:48:50 +0000 (13:48 -0800)]
ofproto: Update access rules for 'flow_cookie'.
The 'flow_cookie' member of struct rule is read during flow translation
without holding a mutex, breaking the documented OVS_GUARDED access rule.
However, the flow_cookie member is actually never modified after a rule is
initialized, so this commit changes the access rules to reflect that.
When the add-port command fails, vsctl reports the failure and just
suggests to check the logs for more details.
ovs-vswitchd fills the error column in the Interface table with a
description of the error, so it might be helpful to print that.
This is useful especially for dpdk devices, because the port naming
change could use a better error reporting.
I'm planning another patch to make sure that ovs-vswitch writes
appropriates information in the error column, after the dpdk port naming
changes are merged.
CC: Ben Pfaff <blp@ovn.org> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> Acked-by: Ben Pfaff <blp@ovn.org>
Mickey Spiegel [Tue, 20 Dec 2016 21:23:46 +0000 (13:23 -0800)]
ovn-controller: Log chassis claiming lport only when changes occur.
With recent OVN commits, the logic for a chassis to claim
or release a logical port was consolidated. This is a
good thing. However, there was a logic change that
resulted in VLOG_INFO being generated every time on the
ovn-controller. This patch changes the logic so that
VLOG_INFO is only generated when there is a change, for
example when the chassis claims an lport the first time.
Signed-off-by: Mickey Spiegel <mickeys.dev@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Wed, 21 Dec 2016 00:17:31 +0000 (16:17 -0800)]
ovsdb-idlc: Initialize nonnull string columns for inserted rows.
When a schema column has type "exactly one string", the corresponding
struct member has type "char *" and the documented and expected behavior
is that the string should always be nonnull. (The code generator even
adds a comment /* Always nonnull. */ in the struct definition.) In the
case where a value is not available, the string is supposed to be
initialized to "" instead of to NULL.
However, the IDL code for inserting a new row did not properly initialize
the column to "", instead leaving it NULL. This could cause null pointer
dereferences in corner cases.
Ben Pfaff [Tue, 20 Dec 2016 21:15:51 +0000 (13:15 -0800)]
tests: Regenerate atlocal when running tests.
A previous patch fixed double rebuilds when running tests in some cases.
That patch removed dependencies from targets in tests/automake.mk that
were redundant because the "all" target already depended on them. A
dependency on tests/atlocal was also removed in the belief that "all"
depended on tests/atlocal. This belief was incorrect, which meant that
tests/atlocal would not get regenerated if it was removed or out of date.
This commit fixes the problem.
Reported-by: Joe Stringer <joe@ovn.org> Fixes: a8cb456227b0 ("tests: Fix double-rebuild of testsuite for "check-valgrind" and similar.") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Joe Stringer <joe@ovn.org>
Alin Serdean [Tue, 20 Dec 2016 19:41:22 +0000 (19:41 +0000)]
Windows: Add internal switch port per OVS bridge
This patch updates the following commands in the vswitch:
ovs-vsctl add-br br-test
ovs-vsctl del-br br-test
ovs-vsctl add-br br-test:
This command will now create an internal port on the MSFT virtual switch
using the WMI interface from Msvm_VirtualEthernetSwitchManagementService
leveraging the method AddResourceSettings.
Before creating the actual port, the switch will be queried to see if there
is not a port already created (good for restarts when restarting the
vswitch daemon). If there is a port defined it will return success and log
a message.
After checking if the port already exists the command will also verify
if the forwarding extension (windows datapath) is enabled and on a single
switch. If it is not activated or if it is activated on multiple switches
it will return an error and a message will be logged.
After the port was created on the switch, we will disable the adapter on
the host and rename to the corresponding OVS bridge name for consistency.
The user will enable and set the values he wants after creation.
ovs-vsctl del-br br-test
This command will remove an internal port on the MSFT virtual switch
using the Msvm_VirtualEthernetSwitchManagementService class and executing
the method RemoveResourceSettings.
Both commands will be blocking until the WMI job is finished, this allows us
to guarantee that the ports are created and their name are set before issuing
a netlink message to the windows datapath.
This patch also includes helpers for normal WMI retrievals and initializations.
Appveyor and documentation has been modified to include the libraries needed
for COM objects.
This patch was tested individually using IMallocSpy and CRT heap checks
to ensure no new memory leaks are introduced.
Tested on the following OS's:
Windows 2012, Windows 2012r2, Windows 2016
Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> Acked-by: Paul Boca <pboca@cloudbasesolutions.com> Acked-by: Sairam Venugopal <vsairam@vmware.com> Signed-off-by: Gurucharan Shetty <guru@ovn.org>
Sairam Venugopal [Fri, 16 Dec 2016 22:28:12 +0000 (14:28 -0800)]
datapath-windows: Conntrack - Enable FTP support
Enable the support for tracking FTP connections in the Connection tracker.
This checks an incoming ftp control connection to extract the related data
connection. When a matching data connection arrives, it would then update
the connection entry to point to the original control connection.
Sairam Venugopal [Fri, 16 Dec 2016 22:28:11 +0000 (14:28 -0800)]
datapath-windows: Conntrack - Introduce support for tracking related connections
Introduce a new table to track related connections. This table will be
used to track FTP data connections based on the control connection. There
is a new Conntrack-ftp.c to parse incoming FTP messages to determine the
related data ports. It creates a new entry in the related connections
tracker table. If there is a matching FTP data connection, then the state
for that connection is marked as RELATED.
Alin Serdean [Tue, 13 Dec 2016 18:52:35 +0000 (18:52 +0000)]
windows: Incorrect check while fetching adapter addresses
Checking for ERROR_INSUFFICIENT_BUFFER is incorrect per
MSFT documentation:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365915(v=vs.85).aspx
Also, the initial call to GetAdaptersAddresses was wrong. In the case
of a successful return 'all_addr' was not allocated leading to a crash.
Guru Shetty [Mon, 19 Dec 2016 12:12:17 +0000 (04:12 -0800)]
ovn-controller: Fix conntrack zone in gateway routers.
The gateway router was using the ct_next action to
reassemble packets. But ct_next action by default would
use the zone allocated for a logical port and in case of
gateway routers that value was zero. This would make
the flow use the default zone of zero. This had some
unintended consequences as the zone used to track packets
and the zone used to eventually commit it (DNAT zone)
was different. As a result, a packet would never have ct.est set.
With this commit, when ct_next action is used in a gateway
router, we use the DNAT zone. This is similar to the
strategy used in commit c2e954a117a8 (ovn-controller: Datapath
based conntrack zone for load-balancing.)
Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 20 Dec 2016 03:25:59 +0000 (19:25 -0800)]
ovn-controller: Monitor only necessary southbound rows.
Until now, ovn-controller has replicated all of the southbound database
(through the IDL). This is inefficient, especially in a large OVN setup
where many logical networks are not present on an individual hypervisor.
This commit improves on the situation somewhat, by making ovn-controller
replicate (almost) only the port bindings, logical flows, and multicast
groups that are actually relevant to the particular hypervisor on which
ovn-controller is running. This is easily possible by replicating the
patch ports from the Port_Binding table and using these relationships to
determine connections between datapaths.
This patch is strongly influenced by earlier work from the CCed developers.
I am grateful for their assistance.
Ben Pfaff [Tue, 20 Dec 2016 04:55:35 +0000 (20:55 -0800)]
ovsdb-idl: Change interface to conditional monitoring.
Most users of OVSDB react to whatever is currently in their view of the
database, as opposed to keeping track of changes and reacting to those
changes individually. The interface to conditional monitoring was
different, in that it expected the client to say what to add or remove from
monitoring instead of what to monitor. This seemed reasonable at the time,
but in practice it turns out that the usual approach actually works better,
because the condition is generally a function of the data visible in the
database. This commit changes the approach.
This commit also changes the meaning of an empty condition for a table.
Previously, an empty condition meant to replicate every row. Now, an empty
condition means to replicate no rows. This is more convenient for code
that gradually constructs conditions, because it does not need special
cases for replicating nothing.
This commit also changes the internal implementation of conditions from
linked lists to arrays. I just couldn't see an advantage to using linked
lists.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Liran Schour <lirans@il.ibm.com>
Ben Pfaff [Sat, 17 Dec 2016 18:51:49 +0000 (10:51 -0800)]
ovn-controller: Rename all_lports to local_lports.
This sset has always just contained the names of logical ports that are
local to the current hypervisor, but the name 'all_lports' implied that it
contained the name of every logical port.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mickey Spiegel <mickeys.dev@gmail.com>
Ben Pfaff [Mon, 19 Dec 2016 22:53:18 +0000 (14:53 -0800)]
ovn-controller: Drop most uses of OVS patch ports.
Until now, ovn-controller has implemented OVN logical patch ports and
l3gateway ports in terms of OVS patch ports. It is a hassle to create and
destroy ports, and it is also wasteful compared to what the patch ports
actually buy us: the ability to "save and restore" a packet around a
recursive trip through the flow table. The "clone" action can do that too,
without the need to create a port. This commit takes advantage of the
clone action for that purpose, getting rid of most of the patch ports
previously created by ovn-controller.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mickey Spiegel <mickeys.dev@gmail.com>
Ben Pfaff [Mon, 19 Dec 2016 21:20:07 +0000 (13:20 -0800)]
ovn-controller: Handle only relevant ports and flows.
On a particular hypervisor, ovn-controller only needs to handle ports
and datapaths that have some relationship with it, that is, the
ports that actually reside on the hypervisor, plus all the other ports on
those ports' datapaths, plus all of the ports and datapaths that are
reachable from those via logical patch ports. Until now, ovn-controller
has done a poor job of limiting what it deals with to this set. This
commit improves the situation.
This commit gets rid of the concept of a "patched_datapath" which until now
was used to represent any datapath that contained a logical patch port.
Previously, the concept of a "local_datapath" meant a datapath with a VIF
that resides on the local hypervisor. This commit extends that concept to
include any other datapath that can be reached from a VIF on the local
hypervisor, which is a simplification that makes the code easier to
understand in a few places.
Ben Pfaff [Fri, 2 Dec 2016 07:24:41 +0000 (23:24 -0800)]
lport: Be a little more careful building lport index.
It seems like a good idea to check for and warn about all kinds of
duplicates, and to avoid segfaulting if a datapath column is empty.
(However, the database schema should prevent both issues.)
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mickey Spiegel <mickeys.dev@gmail.com>
William Tu [Sun, 18 Dec 2016 08:13:02 +0000 (00:13 -0800)]
ofp-actions: Add clone action.
This patch adds OpenFlow clone action with syntax as below:
"clone([action][,action...])". The clone() action makes a copy of the
current packet and executes the list of actions against the packet,
without affecting the packet after the "clone(...)" action. In other
word, the packet before the clone() and after the clone() is the same,
no matter what actions executed inside the clone().
Use case 1:
Set different fields and output to different ports without unset
actions=
clone(mod_dl_src:<mac1>, output:1), clone(mod_dl_dst:<mac2>, output:2), output:3
Since each clone() has independent packet, output:1 has only dl_src modified,
output:2 has only dl_dst modified, output:3 has original packet.
Similar to case1
actions=
push_vlan(...), output:2, pop_vlan, push_vlan(...), output:3
can be changed to
actions=
clone(push_vlan(...), output:2),clone(push_vlan(...), output:3)
without having to add pop_vlan.
case 2: resubmit to another table without worrying packet being modified
actions=clone(resubmit(1,2)), ...
Signed-off-by: William Tu <u9012063@gmail.com>
[blp@ovn.org revised this to omit the "sample" action] Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 15 Dec 2016 17:24:24 +0000 (09:24 -0800)]
ofp-actions: Use struct ext_action_header for appropriate actions.
A few Open vSwitch extension actions have no fixed arguments but do have
variable-length options that follow the header, and an upcoming commit will
add another such action. There is little value in having individual
structures for these actions, since they all have the same form, so this
commit makes all of them use the existing struct ext_action_header.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mickey Spiegel <mickeys.dev@gmail.com>
Shashank Ram [Thu, 8 Dec 2016 21:31:49 +0000 (13:31 -0800)]
datapath-windows: Fix issues related to packet completion
In OvsTunnelPortTx() function, for packets coming from the
VIF port, the srcVportNo, srcPortId and srcNicIndex were
getting modified for the original NBL prior to creation
of newNbl. This is not correct since modifying the original
packet's forwarding detail can cause completion issues.
Instead, we should keep the forwarding detail of the original
packet as is, and only update the forwarding detail for the
newNbl.
Yi-Hung Wei [Fri, 9 Dec 2016 02:34:07 +0000 (18:34 -0800)]
system-traffic: Skip test cases if firewalld is on.
On RHEL 7.3, test cases that use vxlan, gre, and geneve tunnels fail because
traffic is blocked by default firewall configuration. This commit detects the
status of firewalld, and skips the tests if firewalld is on.
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Yi-Hung Wei [Fri, 9 Dec 2016 02:34:06 +0000 (18:34 -0800)]
system-traffic: Skip test cases if netcat is not installed.
Test cases that use netcat will fail if netcat is not installed. This patch
detects if netcat is present, and skips those test cases if netcat is not there.
Singed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
Yi-Hung Wei [Fri, 9 Dec 2016 02:34:05 +0000 (18:34 -0800)]
datapath: compat: Fix build on RHEL 7.3
RHEL 7.3 provides upstream tunnel but it does not support name_assign_type
attribute in net-device. This patch fixes the build problem by backporting
functions with name_assign_type, and using proper flags in acinclude.m4 to
invoke backport functions.
Tested on RHEL 7.3 with kernel 3.10.0-514.el7.x86_64
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
The order of the options in the packet generated by ovs-controller
depends on the hash function. I believe that murmur hash (our default)
produces different outputs depending on the endianness of the system.
Also, if SSE4.2 is enabled at build time, we use CRC32 for hashing which
gives different results even on x86.
This causes one unit test to fail on big endian or with SSE4.2:
ovn -- dhcpv4 : 1 HV, 2 LS, 2 LSPs/LS
This commit fixes the problem in ovn-northd by always sorting dhcp
options inside the logical flow put_dhcp(v6)_opts action.
Reported-at: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=840770 Suggested-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> Acked-by: Ben Pfaff <blp@ovn.org>
According to RFC 791, every internet module must be
able to forward a datagram of 68 octets without further
fragmentation. This is because an internet header may
be up to 60 octets, and the minimum fragment is 8 octets.
The maximum size of IP packets is 65535 bytes. The range
of MTU values allowes for the MTU configuration parameter
is 68 to 65535.
Signed-off-by: nickcooper-zhangtonghao <nic@opencloud.tech>
[blp@ovn.org changed this to just a documentation patch] Signed-off-by: Ben Pfaff <blp@ovn.org>
The ovs-sandbox runs in the "dummy mode" by default.
In this mode of testing, no packets travel across
physical or virtual networks. But sometimes, we may
create veth network devices and add them to ovs bridge
for developing and testing. It's necessary to add an option.
Signed-off-by: nickcooper-zhangtonghao <nic@opencloud.tech> Signed-off-by: Ben Pfaff <blp@ovn.org>
Guoshuai Li [Wed, 7 Dec 2016 06:38:22 +0000 (14:38 +0800)]
python: Reconnect SSL connections when ovsdb-server restarts.
The do_handshake() function throws the exception OpenSSL.SSL.SysCallError
when the peer's SSL connection is closed, And the recv() function also
throws the exception OpenSSL.SSL.SysCallError when the peer's SSL
connection is abnormally closed, This commit catches the exception and
return error's errno.
Similarly, the recv() function also throws the exception
OpenSSL.SSL.ZeroReturnError when the peer's SSL connection is closed. This
exception refers to TCP connection normal closed, return (0, "")
Signed-off-by: Guoshuai Li <ligs@dtdream.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <nusiddiq@redhat.com>
Ben Pfaff [Mon, 5 Dec 2016 22:29:35 +0000 (14:29 -0800)]
acinclude: Fix -Wstrict-prototypes and -Wold-style-definition detection.
AC_LANG_PROGRAM(,) uses a program like this:
int main() { return 0; }
but that triggers warnings for -Wstrict-prototypes and for
-Wold-style-definition, since this definition of main() lacks a prototype
and is therefore old-style. This meant that -Wstrict-prototypes and
-Wold-style-definition weren't being turned on for new-enough GCC. This
commit fixes the problem by changing the program that is test-compiled to:
int x;
which doesn't make any compilers mad, as far as I know.
I recently upgraded to GCC 6.1 and just now noticed the issue, so I think
that GCC somewhere between version 4.9 and version 6.1 must have started
warning about main() when it's declared this way.
Also, fix a few functions that lacked prototypes.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org>
ovn: Encode dhcpv6 PACKET_IN userdata as big endian.
The packet in userdata generated by ovn-controller when translating the
put_dhcpv6_opt action includes 16-bit integers.
Currently these 16-bit integers are encoded with native endianness.
This is ok becase the only consumer of that userdata is ovn-controller
itself, but it means that the OpenFlow action we're generating might
not really be the same on different hosts.
I think it's better to encode the userdata as big-endian, like the rest
of OpenFlow messages.
This fixes a test failure on big-endian platforms, because the generated
OpenFlow bytes were different than expected (the expectation was
generated on a little endian machine).
Now 'struct dhcp_opt6_header' is identical to 'struct
dhcpv6_opt_header', but I chose to keep them separate, because they
have different purposes. I also renamed the members to avoid confusion.
Aaron Conole [Fri, 9 Dec 2016 16:22:27 +0000 (11:22 -0500)]
lib/dpdk: fix double free on exit
The DPDK EAL library intents that all argc/argv arguments passed on the
command line will be in the form:
progname dpdk arguments program arguments
This means the argv array will look something like:
argv[0] = progname
argv[1..x] = dpdk arguments
argv[x..y] = program arguments
When the eal initialization routine completes, it will modify the argv array
to set argv[ret] = progname, such that the arguments can then be passed to
something like getopts for further processing.
When the dpdk arguments rework was initially added, the assignment mentioned
above was not considered. This means two errors were introduced:
1. Leak of the element at argv[ret]
2. Double-free of the element at argv[0]
Reported-by: Ilya Maximets <i.maximets@samsung.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325442.html Fixes: bab694097133 ("netdev-dpdk: Convert initialization from cmdline to db") Signed-off-by: Aaron Conole <aconole@redhat.com>
Shashank Ram [Wed, 23 Nov 2016 01:32:54 +0000 (17:32 -0800)]
datapath-windows: Avoid busy wait in OvsStartIpHelper
Previously, the IP Helper thread would wait for an event
but with a timeout of 0, which resulted in the thread
busy waiting causing high CPU usage by the kernel.
Since the IP Helper thread is only required based on
certain events, it makes sense to wait indefinitely
till we receieve such an event notification to wake up
the thread. This change aims to address this issue.
When OvsEnqueueIpHelperRequest() or OvsInternalAdapterUp()
is called, the ovsNumIpHelperRequests counter is incremented,
but upon consumption of the request, is not decremented.
Since the wakeup logic for the thread is determined by this
counter value, we need to reset the counter back correctly
once the request has been consumed by the IP Helper thread.
There are a couple of references to these for various build systems.
The website is going to be our "one true resource" for all docs, so
simply remove these references.
Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Ben Pfaff <blp@ovn.org>
This is a dumb move of all 'INSTALL*' docs, with very little
refactoring (mostly updating links and making the titles a little more
consistent. Additional refactoring will be done in subsequent changes.
Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Ben Pfaff <blp@ovn.org>
Create a series of sections, all of which are currently empty, using
the general design established by Jacob Kaplan-Moss and the Django
project [1]. Five sections are provided:
This is essentially the output of 'sphinx-quickstart' but with the
following changes:
- Parts of the generated Makefile are merged into the existing
Documentation/automake.mk Makefile
- A license is added to the index.rst file
- The OVS logo is added
- A 'contents' page is added, so we don't need to include a TOC on the
home page
- The theme is switched to 'bizstyle', which makes better use of
horizontal real estate than the default 'alabaster' theme
Copyright is assigned to "The Open vSwitch Development Community".
Signed-off-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 9 Dec 2016 18:28:04 +0000 (10:28 -0800)]
ovn-sbctl: Fix a few minor typesetting errors.
In manpages, \- is used primarily for literal "-" in literal text, that is,
it should usually be used for bold (literal) text. (It's also used for
minus signs in mathematics.) It should not generally be used for hyphens
in English text, as it was used here in italic (metasyntactic variable)
text. This fixes the problem.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Russell Bryant <russell@ovn.org>
Ilya Maximets [Thu, 8 Dec 2016 08:55:31 +0000 (11:55 +0300)]
netdev: Set the default number of queues at removal from the database
Expected behavior for attribute removal from the database is
resetting it to default value. Currently this doesn't work for
n_rxq/n_txq options of pmd netdevs (last requested value used):
Fix that by using NR_QUEUE or 1 as a default value for 'smap_get_int'.
Fixes: a14b8947fd13 ("dpif-netdev: Allow different numbers of
rx queues for different ports.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Tested-by: Ian Stokes <ian.stokes@intel.com> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
Jarno Rajahalme [Sat, 10 Dec 2016 01:22:42 +0000 (17:22 -0800)]
datapath: Allow compile against current net-next.
This patch allows openvswitch kernel module in the OVS tree to be
compiled against the current net-next Linux kernel. The changes are
due to these upstream commits:
56989f6d856 ("genetlink: mark families as __ro_after_init") 489111e5c25 ("genetlink: statically initialize families") a07ea4d9941 ("genetlink: no longer support using static family IDs")
struct genl_family initialization is changed be completely static and
to include the new (in Linux 4.6) __ro_after_init attribute. Compat
code defines it as an empty macro if not defined already.
GENL_ID_GENERATE is no longer defined, but since it was defined as 0,
it is safe to drop it from all initializers also on older Linux
versions. A compiletime_assert is added to make sure this is true
whenever GENL_ID_GENERATE is defined.
Tested with current Linux net-next (4.9) and 3.16.
It should be noted that there are still a number of fixes and new
features in upstream net-next that are yet to be backported.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org>
Ben Pfaff [Fri, 9 Dec 2016 23:09:42 +0000 (15:09 -0800)]
ofproto-dpif-ipfix: Fix assertion failure for bad configuration.
The assertions in dpif_ipfix_set_options() made some bad assumptions about
flow exporters. The code that added and removed exporters would add a flow
exporter even if it had an invalid configuration ("broken"), but the
assertions checked that broken flow exporters were not added. Thus, the
when a flow exporter was broken, ovs-vswitchd would crash due to an
assertion failure.
Here is an example vsctl command that, run in the sandbox, would crash
ovs-vswitchd:
The minimal fix would be to remove the assertions, but this would leave
broken flow exporters in place. This commit goes a little farther and
actually removes broken flow exporters.
This fix pulls code out of an "if" statement to a higher level, so it is a
smaller fix when viewed igoring space changes.
This bug dates back to the introduction of IPFIX in 2013.
Andy Zhou [Fri, 9 Dec 2016 01:26:10 +0000 (17:26 -0800)]
rpms: Remove more OVN files form openvswitch rpm builds
OVN is packaged with openvswitch-fedora.spec.in, but not with
openvswitch.spec.in. Remove OVN files from openvswitch.spec.in
builds to make rpm build happy.
Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Russell Bryant <russell@ovn.org>
ovs-vswitchd: Avoid segfault for "netdev" datapath.
When the datapath, whose type is "netdev", processes packets
in userspce action, it may cause a segmentation fault. In the
dp_execute_userspace_action(), we pass the "wc" argument to
dp_netdev_upcall() using NULL. In the dp_netdev_upcall() call tree,
the "wc" will be used. For example, dp_netdev_upcall() uses the
&wc->masks for debugging, and flow_wildcards_init_for_packet()
uses the "wc" if we disable megaflow, which is described in
more detail below.
Segmentation fault in flow_wildcards_init_for_packet:
Joe Stringer [Thu, 8 Dec 2016 18:22:16 +0000 (10:22 -0800)]
treewide: Fix recent flake8-check.
Without this patch, I see errors like this on master:
../ofproto/ipfix-gen-entities:115:1: E305 expected 2 blank lines after
class or function definition, found 1
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Daniele Di Proietto <diproiettod@vmware.com>
Guoshuai Li [Sat, 3 Dec 2016 07:38:12 +0000 (15:38 +0800)]
Fix ovndb_servers master and VirtualIP are not on the same node.
PCS man page says role=Stopped/Started/Master/Slave.
A role can be master or slave (if no role is specified,
it defaults to 'started').
Command line
"$pcs constraint colocation add ovndb_servers-master with
master VirtualIP"
means that the Started role node of ovsdb follows the master node of
VirtualIP. But we actually want the ovsdb master node to follow the
VirtualIP's Started node.
Guoshuai Li [Tue, 6 Dec 2016 12:35:08 +0000 (20:35 +0800)]
ovn: Fix pacemaker Master node does not monitor the OVSDB status
OVSDB resource is multi-state resource, pacemaker monitor actions are
insufficient to monitor a multi-state resource. The usual one will
cover the slave role, so it is need to configure monitoring for the
master.
Ben Pfaff [Sun, 23 Oct 2016 17:50:35 +0000 (10:50 -0700)]
tests: Fix order confusion in "ovn -- 2 HVs, 4 lports/HV, localnet ports".
The order of src and dst was swapped both in assignment and reference,
which meant that the result worked OK but was really confusing to try to
extend or modify.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Russell Bryant <russell@ovn.org>
Ilya Maximets [Tue, 6 Dec 2016 11:48:43 +0000 (14:48 +0300)]
tests/bundle: Use active_backup algorithm for up/down test.
HRW algorithm uses hash function which is dependent from the build
environment. This leads to constant fails of the testsuite
with CFLAGS='-march=native' if CPU supports hashing instructions:
Using of 'active_backup' algorithm will help to avoid such issues.
CC: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> Fixes: 63460a30c53e ("tests/bundle: test bundle action with ports up and down") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Russell Bryant [Mon, 5 Dec 2016 17:04:32 +0000 (12:04 -0500)]
ovn-controller(8): ovn-remote must be an ovsdb remote.
Document that the value of the ovn-remote configuration
option must be in the form of an ovsdb remote as previously documented
in this man page. This came up on IRC where someone trying OVN
put a hostname here and observed that it did not work.
Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Jarno Rajahalme [Thu, 1 Dec 2016 21:10:53 +0000 (13:10 -0800)]
ofproto: Honor OFPFF_RESET_COUNTS flag in flow modify message.
While a flow modify must keep the original flow's flags, it must reset
counts if (and only if) the reset_counts flag is present in the flow
mod message.
Behavior prior to this patch is broken in a few ways:
- OpenFlow 1.0 and 1.1 mod-flows did reset the counts, if the flow had
reset_counts flag set. Only add-flow should reset counts.
- With OpenFlow 1.2 and later, if the old flow had the reset_counts
flag set, the counts would be reset by mod-flows, even if the
flow-mod message does not have the reset_counts flag set.
- With OpenFlow 1.2 and later, mod-flows with a reset_count did not
reset the counts, if the old flow did not have the reset_counts flag
set.
Even though the prevailing interpretation seems to be that the
reset_counts flag in the flow-mod message should be stored as part of
the flow state (and reported back in flow dumps with OpenFlow >= 1.3),
we should always just look at the reset_counts flag in the current
flow-mod and ignore the reset_counts flag stored in the flow when
processing a flow mod.
For OpenFlow 1.0 and 1.1 we already implicitly add the reset_counts
flag for add-flow messages (only) to maintain the expected behavior.
This patch adds a comprehensive test case to prevent future regressions.
Suggested-by: Tony van der Peet <tony.vanderpeet@alliedtelesis.co.nz> Fixes: 748eb2f5b1 ("ofproto-dpif: Always forward 'used' from the old_rule.") Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Jarno Rajahalme [Thu, 1 Dec 2016 22:05:24 +0000 (14:05 -0800)]
mpls: Fix MPLS restoration after patch port and group bucket.
This patch fixes problems with MPLS handling related to patch ports
and group buckets.
If a group bucket or a peer bridge across a patch port pushes MPLS
headers to a non-MPLS packet and outputs, the flow translation after
returning from the group bucket or patch port would undo the packet
transformations so that the processing could continue with the packet
as it was before entering the patch port. There were two problems
with this:
1. As part of the first MPLS push on a non-MPLS packet, the flow
translation would first clear the L3/4 headers of the 'flow' to mark
those fields invalid. Later, when committing 'flow' changes to
datapath actions before output, the necessary datapath MPLS actions
are created and the corresponding changes updated to the 'base flow'.
This was done using the same flow_push_mpls() function that clears
the L2/3 headers, so also the 'base flow' L2/3 headers were cleared.
Then, when translation returns from a patch port or group bucket, the
original 'flow' is restored, now showing no sign of the MPLS labels.
Since the 'base flow' now has the MPLS labels, following translations
know to issue MPLS POP actions before any output actions. However, as
part of checking for changes to IP headers we test that the IP
protocol type was not changed. But now the 'base flow's 'nw_proto'
field is zero and an assert fail crashes OVS.
This is solved by not clearing the L3/4 fields of the 'base
flow'. This allows the processing after the patch port to continue
with L3/4 fields as if no MPLS was done, after first issuing the
necessary MPLS POP actions.
2. IP header updates were done before the MPLS POP actions were
issued. This caused incorrect packet output after, e.g., group action
or patch port. For example, with actions:
group 1234: all bucket=push_mpls,output:LOCAL
ip actions=group:1234,dec_ttl,output:LOCAL,output:LOCAL
the dec_ttl would only be executed before the last output to LOCAL,
since at the time of committing IP changes after the group action the
packet was still an MPLS packet.
This is solved by checking the dl_type of both 'flow' and 'base flow'
and issuing MPLS actions if they can transform the packet from an MPLS
packet to a non-MPLS packet. For an IP packet the change in ttl can
then be correctly committed before the last two output actions.
Two test cases are added to prevent future regressions.
Reported-by: Thomas Morin <thomas.morin@orange.com> Suggested-by: Takashi YAMAMOTO <yamamoto@ovn.org> Fixes: 8bfd0fdac ("Enhance userspace support for MPLS, for up to 3 labels.") Fixes: 1b035ef20 ("mpls: Allow l3 and l4 actions to prior to a push_mpls action") Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: YAMAMOTO Takashi <yamamoto@ovn.org>
billyom [Tue, 29 Nov 2016 10:24:53 +0000 (10:24 +0000)]
ovs-numa: Allow leading 0x on pmd-cpu-mask.
pmd-cpu-mask is interpreted as a hex bit mask. So it should be written
with a leading 0x to indicate this. But if this is done, while the value
is interpreted correctly and the PMDs pinned as expected, a confusing
warning message is also issued.
This patch allows but does not require a leading 0x or 0X to be used
without a warning. Existing functionality is not affected. Relevant DPDK
docs also updated.
Signed-off-by: Billy O'Mahony <billy.o.mahony@intel.com> Tested-by: Kevin Traynor <ktraynor@redhat.com> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
Ilya Maximets [Tue, 29 Nov 2016 12:20:56 +0000 (15:20 +0300)]
netdev-dpdk: Don't try to unregister empty vhost_id.
If 'vhost-server-path' not provided for vhostuserclient port,
'netdev_dpdk_vhost_destruct()' will try to unregister an empty string.
This leads to error message in log:
netdev_dpdk|ERR|vhost2: Unable to unregister vhost driver for socket ''.
CC: Ciara Loftus <ciara.loftus@intel.com> Fixes: 2d24d165d6a5 ("netdev-dpdk: Add new 'dpdkvhostuserclient' port type") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ciara Loftus <ciara.loftus@intel.com> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
Ben Pfaff [Fri, 2 Dec 2016 00:34:03 +0000 (16:34 -0800)]
ovn-controller: Remove unused members from local_datapath.
Nothing used these, except to initialize and free them. 'logical_port'
wasn't meaningful in any case, it was just the name of the first logical
port encountered from a particular logical datapath when traversing the
database's Port_Binding table, which isn't in a meaningful order.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mickey Spiegel <mickeys.dev@gmail.com>
projects / mirror_ovs.git / log