ovn-controller: eliminate stall in ofctrl state machine
The "ovn -- 2 HVs, 3 LRs connected via LS, static routes"
test case currently exhibits frequent failures. These failures
occur because, at the time that the test packets are sent to
verify forwarding, no flows have been installed in the vswitch
for one of the hypervisors.
The state machine implemented by ofctrl_run() is intended to
iterate as long as progress is being made, either as long as
the state continues to change or as long as packets are being
received. Unfortunately, the code had a bug: if receiving a
packet caused the state to change, it didn't call the state's
run function again to try to see if it would change the state.
This caused a real problem in the following case:
1) The state is S_TLV_TABLE_MOD_SENT.
2) An OFPTYPE_NXT_TLV_TABLE_REPLY message is received.
3) No event (other than SB probe timer expiration) is expected
that would unblock poll_block() in the main ovn-controller
loop.
In such a case, ofctrl_run() would receive the packet and
advance the state, but not call the run function for the new
state, and then leave the state machine paused until the next
event (e.g. a timer event) occurred.
This commit fixes the problem by continuing to iterate the state
machine until the state remains the same and no packet is
received in the same iteration. Without this fix, around 40
failures are seen out of 100 attempts, with this fix no failures
have been observed in several hundred attempts (using an earlier
version of this patch).
Signed-off-by: Lance Richardson <lrichard@redhat.com>
[blp@ovn.org refactored for clarity] Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Lance Richardson <lrichard@redhat.com>
ovs-lib: Keep internal interface ip during upgrade.
Commit 9b5422a98f81("ovs-lib: Try to call exit before killing.")
introduced a problem where internal interfaces are destroyed and
recreated, losing their IP address.
Commit 9aad5a5a96ba("ovs-vswitchd: Preserve datapath ports across
graceful shutdown.") fixed the problem by changing ovs-vswitchd
to preserve the ports on `ovs-appctl exit`. Unfortunately, this fix is
not enough during upgrade from <= 2.5.0, where an old ovs-vswitchd is
running (without the fix) and a new ovs-lib script is performing the
restart.
The problem seem to affect both RHEL and ubuntu.
This commit fixes the upgrade by looking at the running daemon
version and avoid using `ovs-appctl exit` if it's < 2.5.90.
Suggested-by: Gurucharan Shetty <guru@ovn.org> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> Acked-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Terry Wilson [Tue, 12 Jul 2016 21:37:34 +0000 (16:37 -0500)]
json: Move from lib to include/openvswitch.
To easily allow both in- and out-of-tree building of the Python
wrapper for the OVS JSON parser (e.g. w/ pip), move json.h to
include/openvswitch. This also requires moving lib/{hmap,shash}.h.
Both hmap.h and shash.h were #include-ing "util.h" even though the
headers themselves did not use anything from there, but rather from
include/openvswitch/util.h. Fixing that required including util.h
in several C files mostly due to OVS_NOT_REACHED and things like
xmalloc.
Signed-off-by: Terry Wilson <twilson@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
These failures were caused by a combination of problems in
handling physical changes:
1. When a vif was removed, the localvif_to_ofport entry was not
removed.
2. When a physical change was detected, ovn-controller would wait
a poll cycle before processing the logical flow table.
This patch set addresses both of these issues while simultaneously
cleaning up the code in physical.c. A side effect is a modification
of where OF flows are dumped in the gateway router case that allowed
the root causes of this issue to be found.
With these changes, all of the above tests had a 100/100 success rate.
Andy Zhou [Fri, 22 Jul 2016 20:49:09 +0000 (13:49 -0700)]
ovsdb: Add ovsdb-client options for testing lock
RFC 7047 lock operation has been fully implemented in ovsdb-server
for a while, but it is not well covered in unit testing. This
patch adds options for the ovsdb-client tool to issue lock operations.
The next patch will make use those options.
Please see ovsdb-client(1) changes for more details.
Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Benli Ye [Thu, 7 Jul 2016 15:17:48 +0000 (23:17 +0800)]
tests: Fix IPFIX test cases issue.
IPFIX statistics 'tx pkts' means the number of successfully
sending IPFIX packets, while 'tx errs' means sending error
IPFIX packets. These two parameters can be affected by whether
listening on port 4739 on local host. This case should be
solved entirely by introducing PARSE_LISTENING_PORT as sFlow,
but it depends on implementing IPFIX packet analysis and it
will take some time. Disable these field first, as IPFIX statistics
check are failed on Windows due to 'tx pkts' and 'tx errs' fields.
Windows marks all packets sending successfully, even if port 4739
on local host is not listened.
Remove XFAIL check for 'Flow IPFIX sanity check - tunnel set',
as this test had “UNEXPECTED PASS” on Windows.
More detail, please refer the following link.
https://www.mail-archive.com/dev@openvswitch.org/msg65229.html
Reported-by: Paul Boca <pboca@cloudbasesolutions.com> Acked-by: Paul Boca <pboca@cloudbasesolutions.com> Signed-off-by: Benli Ye <daniely@vmware.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
netdev-dummy: fix crash with more than one passive connection
Investigation found that Some of the occasional failures in the
"ovn -- vtep: 3 HVs, 1 VIFs/HV, 1 GW, 1 LS" test case are caused
by ovs-vswitchd crashing with SIGSEGV. It turns out that the
crash occurrs when the number of netdev-dummy passive connections
transitions from 1 to 2. When xrealloc() copies the array of
dummy_packet_stream structures from the original buffer to a
newly allocated one, the struct ovs_list txq member of the structure
becomes corrupt (e.g. if ovs_list_is_empty() would have returned
false before the copy, it will return true after the copy, which
will lead to a crash when the bogus packet buffer on the list is
dereferenced).
Fix by taking a hint from David Wheeler and adding a level of
indirection.
Signed-off-by: Lance Richardson <lrichard@redhat.com>
[blp@ovn.org folded in an additional bug fix] Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Sat, 2 Jul 2016 01:05:40 +0000 (18:05 -0700)]
ovs-pki: Use SHA-512 instead of SHA-1 as message digest.
The upcoming OpenSSL 1.1.0 release disables use of SHA-1, which breaks the
OVS unit tests, which use SHA-1. We last tried to switch to SHA-512 in
2014 with commit 9ff33ca75e9fcc ("ovs-pki: Use SHA-512 instead of MD5 as
message digest."), but we had to downgrade to SHA-1 in commit 4a1f9610682d
("ovs-pki: Use SHA-1 instead of SHA-512 as message digest.") because
XenServer did not support SHA-512. It has been a few years, so let's try
again.
CC: 828478@bugs.debian.org
Reported-at: https://bugs.debian.org/828478 Reported-by: Kurt Roeckx <kurt@roeckx.be> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Ryan Moats <rmoats@us.ibm.com>
Using sleep's is prone to runtime system dependent races, and indeed
this test started consistently failing on my dev VM after an unrelated
change to ovs-vswitchd. Get git of the sleeps and explicitly wait for
the transaction on ovsdb1 to become visible on ovsdb2.
Also fix the name of the test.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org> Tested-by: Joe Stringer <joe@ovn.org>
Ben Pfaff [Tue, 19 Jul 2016 15:36:35 +0000 (08:36 -0700)]
ovn-northd: Only peer router ports to other router ports.
A router port's "peer", if set, must point to another router port, but the
code as written also accepted switch ports. This caused problems when
switch ports were actually specified.
William Tu [Tue, 19 Jul 2016 00:05:35 +0000 (17:05 -0700)]
netdev-provider: Apply batch object to netdev provider.
Commit 1895cc8dbb64 ("dpif-netdev: create batch object") introduces
batch process functions and 'struct dp_packet_batch' to associate with
batch-level metadata. This patch applies the packet batch object to
the netdev provider interface (dummy, Linux, BSD, and DPDK) so that
batch APIs can be used in providers. With batch metadata visible in
providers, optimizations can be introduced at per-batch level instead
of per-packet.
Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/145694197 Signed-off-by: William Tu <u9012063@gmail.com> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
Joe Stringer [Fri, 17 Jun 2016 19:42:30 +0000 (12:42 -0700)]
debian: Fix OVS upgrade dependencies.
Commit 0dcc739e7a28 ("debian: Move ovs-lib to openvswitch-common.")
shifted a file between debian packages, but didn't update the
destination package annotations to indicate that it replaces a file
from earlier versions of the source package.
As a result, if one installs openvswitch-switch-2.5* (or earlier) and
then tries to upgrade to openvswitch-{switch,common}-2.5.90+, the
install of openvswitch-common will fail like the following:
dpkg: error processing archive
/tmp/openvswitch-common_2.5.90-1_amd64.deb (--install):
trying to overwrite '/usr/share/openvswitch/scripts/ovs-lib', which is
also in package openvswitch-switch 2.5.0-1
Fix the issue by adding "Replaces" and "Breaks" tags to the new
openvswitch-common section of debian/control.
Fixes: 0dcc739e7a28 ("debian: Move ovs-lib to openvswitch-common.") Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Daniele Di Proietto <diproiettod@vmware.com>
Joe Stringer [Tue, 19 Jul 2016 19:54:08 +0000 (12:54 -0700)]
system-traffic: Fix up FTP tests.
Prior to commit b87a5aacefe2 ("datapath: Fix cached ct with helper."),
we were relying on automatic helpers to ensure that FTP connections were
tracked correctly, regardless of the flows that existed in the datapath.
Now, we can drop the automatic helpers in the root namespace and still
have related connections work correctly. Also, the ALG should only be
specified when committing the connection. Update the rules.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
Joe Stringer [Tue, 19 Jul 2016 19:54:06 +0000 (12:54 -0700)]
system-traffic: Update tests in flat tables.
A few of the earlier tests were written with all flows in a single flat
table. While this is a possible way to write your flows to use
connection tracking, it's easier to understand if the processing
proceeds forward from one table to the next. Update these tests.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
tunneling: get skb marking to work properly with tunnels
There are two issues that this patch fixes:
1. it was impossible to set skb mark at all through
NXM_NX_PKT_MARK register for tunnel packets; AND
2. ipsec_xxx tunnels would not be marked with the default
IPsec mark (broken by d23df9a87 "lib/odp: Use masked set
actions.").
This patch also adds anti-regression tests to prevent such
breakages in the future.
IPsec: refactor out some code in OVS_MONITOR_IPSEC_START macro
This OVS_MONITOR_IPSEC_START macro will be helpful in the next
patch where it will be used also from tests/tunnel.at file to test
that skb marking happens correctly. Otherwise, without ovs-monitor-ipsec
running the ovs-vswitchd would refuse to configure ipsec_XXX tunnels.
Russell Bryant [Thu, 30 Jun 2016 20:14:05 +0000 (16:14 -0400)]
ovn: Apply ACL changes to existing connections.
Prior to this commit, once a connection had been committed to the
connection tracker, the connection would continue to be allowed, even
if the policy defined in the ACL table changed. This patch changes
the implementation so that existing connections are affected by policy
changes.
The implementation is based on the suggested approach in this mailing
list thread:
Instead of always allowing packets associated with an established
connection, we now put all packets in the request direction through
the flows generated based on OVN ACLs. If a packet associated with an
established connection hits a "drop" ACL, that means we have
encountered a policy change and should drop packets associated with
this connection from now on. We handle this by setting "ct_label" on
the associated connection tracking entry.
These changes also account for re-allowing a known connection after
ct_label had been set on it. This can happen if you delete an ACL and
then re-create it while connection state is still known.
The proposal on the mailing list also discussed the idea that
ovn-controller could periodically sweep the connection tracker and
delete entries with ct_label set. That is not implemented in this
patch. Instead, we rely on connections dying since we're dropping
its packets and then allowing the connection tracking entry to
eventually time out. More proactively clearing them out could be a
future enhancement.
As a realistic example of how this works, consider this security policy
from an OpenStack+OVN development environment.
The OpenStack Neutron plugin creates ACLs that drop traffic by default
and higher priority ACLs for each type of traffic that is allowed. In
this case, the ACLs for a port using the "default" security group are:
One way I tested this by leaving ping running, ensuring that it was
blocked when the rule for ICMP was deleted, and then re-allowed when
the rule allowing ICMP was restored. In this case, the ICMP
connection is still known by the connection tracker, but the flows
ensure that ct_label gets reset back to 0.
Reported-by: Xiao Li Xu <xiaolixu@cn.ibm.com>
Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1536080 Suggested-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Han Zhou <zhouhan@gmail.com> Acked-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org> Tested-by: Babu Shanmugam <bschanmu@redhat.com>
Justin Pettit [Thu, 23 Jun 2016 01:20:08 +0000 (18:20 -0700)]
ovn-util: Add solicited node addresses to ipv6_netaddr.
Every IPv6 host has a link-local solicited node multicast address for
neighbor discovery. This commit defines the solicited node address for
each IPv6 address added to a logical switch or router port.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
I presume the flags are supposed to map to neighbor discovery
advertisement "Router", "Solicited", and "Override" flags, which would
be "rso" instead of "rco".
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 19 Jul 2016 16:07:13 +0000 (09:07 -0700)]
ovn-northd: Ensure that flows are added to correct types of datapaths.
A DP_TYPE_SWITCH_* flow should only be added to a logical switch datapath,
and a DP_TYPE_ROUTER_* flow should only be added to a logical router
datapath, but the code previously did not verify this and it caused a
problem in practice.
Suggested-by: Guru Shetty <guru@ovn.org>
Suggested-at: http://openvswitch.org/pipermail/dev/2016-July/075557.html Signed-off-by: Ben Pfaff <blp@ovn.org>
FreeBSD returns a socklen of sockaddr_storage when doing an accept on an unix
STREAM socket. The current code will assume it means a sun_path larger than 0.
That breaks some tests like the one below which don't expect to find "unix::" on
the logs.
As a Linux abstract address would not have a more useful name either, it's
better to check that sun_path starts with a non-zero byte and return 0 length in
case it doesn't.
402: ovs-ofctl replace-flows with --bundle FAILED (ovs-ofctl.at:2928)
2016-07-08T12:44:30.068Z|00020|vconn|DBG|unix:: sent (Success): OFPT_HELLO (OF1.6) (xid=0x1):
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
ovn-sbctl: eliminate a spurious test case error cause
The "ovn-sbctl" test fails occasionally due to log messages
similar to these:
jsonrpc|WARN|unix: receive error: Connection reset by peer
reconnect|WARN|unix: connection dropped (Connection reset by peer)
Since we're already ignoring "Broken pipe" messages in this test
case, and the difference between EPIPE and ECONNRESET on send
is simply a matter of whether the peer had unconsumed data
in its receive buffer when the peer socket was closed, it should
be OK to ignore "reset by peer" logs as well.
This same type of failure has been observed in ovs-nbctl and
ovn-vtep-controller tests, so fix it there as well.
Signed-off-by: Lance Richardson <lrichard@redhat.com> Acked-by: Ryan Moats <rmoats@us.ibm.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Add to IDL API that allows the user to add and remove clauses on a table's condition
iteratively. IDL maintain tables condition and send monitor_cond_change to the server
upon condition change.
Add tests for conditional monitoring to IDL.
Signed-off-by: Liran Schour <lirans@il.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
IDL uses now a uuid to specify a monitoring session that is being
sent to the server on "monitor_cond" request.
This uuid will be used to issue ongoing "monitor_cond_change" requests
for this monitoring session.
Signed-off-by: Liran Schour <lirans@il.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Add monitor_cond method to ovsdb-client. Enable testing of monitor_cond_change
via unixctl command.Add unit tests for monitor_cond and monitor_cond_change.
See ovsdb-client(1) man page for details.
Replace monitor2 with monitor_cond.
Signed-off-by: Liran Schour <lirans@il.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Optimize ovsdb_condition_match_any_clause() to be in O(#columns in condition)
and not O(#clauses) in case condition's caluses function is boolean or "==".
Signed-off-by: Liran Schour <lirans@il.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
ovsdb: enable jsonrpc-server to service "monitor_cond_change" request
ovsdb-server now accepts "monitor_cond_change" request. After conditions change
we compose update notification according to the current state of the
database without using a change list before sending reply to the monitor_cond_change
request.
Sees ovsdb-server (1) man page for details of monitor_cond_change.
Signed-off-by: Liran Schour <lirans@il.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
ovsdb: generate update notifications for monitor_cond session
Hold session's conditions in ovsdb_monitor_session_condition. Pass it
to ovsdb_monitor for generating "update2" notifications.
Add functions that can generate "update2" notification for a
"monitor_cond" session.
JSON cache is enabled only for session's with true condition only.
"monitor_cond" and "monitor_cond_change" are RFC 7047 extensions
described by ovsdb-server(1) manpage.
Performance evaluation:
OVN is the main candidate for conditional monitoring usage. It is clear that
conditional monitoring reduces computation on the ovn-controller (client) side
due to the reduced size of flow tables and update messages. Performance
evaluation shows up to 75% computation reduction.
However, performance evaluation shows also a reduction in computation on the SB
ovsdb-server side proportional to the degree that each logical network is
spread over physical hosts in the DC. Evaluation shows that in a realistic
scenarios there is a computation reduction also in the server side.
Evaluation on simulated environment of 50 hosts and 1000 logical ports shows
the following results (cycles #):
ovsdb: allow unmonitored columns in condition evaluation
This commit allows to add unmonitored columns to a monitored table
due to condition update.
It will be used to evaluate conditions on unmonitored columns.
Update notification includes only monitored columns.
Due to the limited number of columns, we do not remove unused unmonitored
columns on condition update for code simplicity.
Signed-off-by: Liran Schour <lirans@il.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
ovsdb: add conditions utilities to support monitor_cond
Change ovsdb_condition to be a 3-element json array or a boolean value (see ovsdb-server
man page).
Conditions utilities will be used later for conditional monitoring.
Signed-off-by: Liran Schour <lirans@il.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ryan Moats [Mon, 18 Jul 2016 21:21:16 +0000 (16:21 -0500)]
ovn-controller: Persist ovn flow tables
Ensure that ovn flow tables are persisted so that changes to
them chan be applied incrementally - this is a prereq for
making lflow_run and physical_run incremental.
As part of this change, add a one-to-many hindex for finding
desired flows by their parent's UUID. Also extend the mapping
by match from one-to-one to one-to-many.
Signed-off-by: Ryan Moats <rmoats@us.ibm.com>
[blp@ovn.org adjusted style and comments and added
HINDEX_FOR_EACH_WITH_HASH_SAFE] Signed-off-by: Ben Pfaff <blp@ovn.org>
Russell Bryant [Mon, 18 Jul 2016 20:25:20 +0000 (16:25 -0400)]
ovn-controller: Drop remove_local_datapath_by_binding().
ovn-controller has an hmap called 'local_datapaths' which tracks
all OVN datapaths that have at least one port binding on the local
chassis. This patch corrects the case where a port binding row is
deleted from the southbound DB while it's still bound to the chassis,
meaning it was deleted before the ovs interface was deleted.
The previous code tried to handle this case by calling
remove_local_datapath_by_binding(). The function appears to try
to look up local_datapath by the binding UUID. If it finds it,
it will delete the local datapath entry. On the surface, this
looks like a bug where it deletes a local datapath entry even
when there could be other ports still bound to the chassis.
The reality is that this function was always a no-op. It was
doing a lookup using a different hash value than how local_datapath
entries are actually hashed. In practice, this wasn't a big problem
because local_datapaths are correctly cleaned in in the
process_full_binding case after an ovs interface is added or removed.
The new change ensures that we run the process_full_binding code
in this case right away, even if the interface is not deleted.
Fixes: 263064aeaa31 ("Convert binding_run to incremental processing.") Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Ryan Moats <rmoats@us.ibm.com>
dataoath: compat: Do not use upstream fill-meta-data function for compat tunnel
upstream dev_fill_metadata_dst() uses upstream tunnel-dst which could
be different from OVS defined tun-dst. Therefore use fill-meta-data
function from compat layer.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
openvswitch: allow output of MPLS packets on tunnel vports
Currently output of MPLS packets on tunnel vports is not allowed by Open
vSwitch. This is because historically encapsulation was done in such a way
that the inner_protocol field of the skb needed to hold the inner protocol
for both MPLS and tunnel encapsulation in order for GSO segmentation to be
performed correctly.
Since b2acd1dc3949 ("openvswitch: Use regular GRE net_device instead of
vport") Open vSwitch makes use of lwt to output to tunnel netdevs which
perform encapsulation. As no drivers expose support for MPLS offloads this
means that GSO packets are segmented in software by validate_xmit_skb(),
which is called from __dev_queue_xmit(), before tunnel encapsulation occurs.
This means that the inner protocol of MPLS is no longer needed by the time
encapsulation occurs and the contention on the inner_protocol field of the
skb no longer occurs.
Thus it is now safe to output MPLS to tunnel vports.
Signed-off-by: Simon Horman <simon.horman@netronome.com> Reviewed-by: Jesse Gross <jesse@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
API changes are related commit:
openvswitch: Revert: "Enable memory mapped Netlink i/o"
revert commit 795449d8b846 ("openvswitch: Enable memory mapped Netlink i/o").
Following the mmaped netlink removal this code can be removed.
Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Russell Bryant [Fri, 15 Jul 2016 23:29:55 +0000 (19:29 -0400)]
ovn-controller: Remove local_datapaths_by_uuid.
binding.c included a static local_datapaths_by_uuid but it was not used
for anything. In fact, the hash node used when inserting into this hmap
is overwritten in another code path for a different hmap.
Fixes: 263064aeaa31 ("Convert binding_run to incremental processing.") Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Ryan Moats <rmoats@us.ibm.com>
Russell Bryant [Tue, 12 Jul 2016 17:33:08 +0000 (13:33 -0400)]
ovn-controller: Clean up bindings handling.
Remove the global set of logical port IDs called 'all_lports'. This is
no longer used for anything after conntrack ID assignment was moved out
of binding.c.
Remove the global smap of logical port IDs to ovsrec_interface records.
We can't persist references to these records, as we may be holding
references to freed memory. Instead, replace it with a new global sset
of logical port IDs called 'local_ids'. This is used to track when
interfaces have been added or removed. We also build a temporary
shash of logical port IDs to ovs interfaces used for fast lookup
of the right interface as needed.
Found by inspection.
Fixes: a478c4efef4d ("ovn-controller: Refactor conntrack zone allocation.") Fixes: 263064aeaa31 ("Convert binding_run to incremental processing.") Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Ryan Moats <rmoats@us.ibm.com>
ovn.at: A "peer" is only for interconnected routers.
We should not use "peer" while connecting a router to a switch.
(Doing so, will cause ovn-northd to constantly create and destroy
logical_flow records which causes CPU utilization of ovn-controller to
spike up.)
Fixes: 31114af758c7e6 ("ovn-nbctl: Update logical router port commands.") Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org> Acked-by: Flavio Fernandes <flavio@flaviof.com>
system-ovn.at: Add a OVN NAT test using OVN gateway.
This unit test adds a basic OVN NAT test that tests north-south
DNAT, south-north SNAT and east-west DNAT and SNAT. It uses network
namespaces connected to br-int using veth pairs to act as logical
ports. This test does not cover multi-host scenarios, so there is
a gap. But userspace OVN tests do multi-host scenarios (without NAT
testing), so it should still be a decent coverage.
Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Joe Stringer <joe@ovn.org>
In the case of CHECKSUM_COMPLETE the skb checksum should be updated in
{push,pop}_mpls() as they the type in the ethernet header.
As suggested by Pravin Shelar.
Cc: Pravin Shelar <pshelar@ovn.org> Fixes: 25cd9ba0abc0 ("openvswitch: Add basic MPLS support to kernel") Signed-off-by: Simon Horman <simon.horman@netronome.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Set name_assign_type of internal port to NET_NAME_USER.
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
ovs: do not allocate memory from offline numa node
When openvswitch tries allocate memory from offline numa node 0:
stats = kmem_cache_alloc_node(flow_stats_cache, GFP_KERNEL | __GFP_ZERO, 0)
It catches VM_BUG_ON(nid < 0 || nid >= MAX_NUMNODES || !node_online(nid))
[ replaced with VM_WARN_ON(!node_online(nid)) recently ] in linux/gfp.h
This patch disables numa affinity in this case.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Signed-off-by: Fabian Frederick <fabf@skynet.be> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
A temporary version (nla_put_be64_32bit()) is added for nla_put_net64().
This function is removed in the next patch.
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
This patch implements bookkeeping support to compute the maximum
headroom for all the devices in each datapath. When said value
changes, the underlying devs are notified via the
ndo_set_rx_headroom method.
This also increases the internal vports xmit performance.
Signed-off-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
I also fix commit 8b32ab9e6ef1: use nla_total_size_64bit() for
OVS_FLOW_ATTR_USED in ovs_flow_cmd_msg_size().
Fixes: 8b32ab9e6ef1 ("ovs: use nla_put_u64_64bit()") Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
ovs: allow nl 'flow set' to use ufid without flow key
When we want to change a flow using netlink, we have to identify it to
be able to perform a lookup. Both the flow key and unique flow ID
(ufid) are valid identifiers, but we always have to specify the flow
key in the netlink message. When both attributes are there, the ufid
is used. The flow key is used to validate the actions provided by
the userland.
This commit allows to use the ufid without having to provide the flow
key, as it is already done in the netlink 'flow get' and 'flow del'
path. The flow key remains mandatory when an action is provided.
Signed-off-by: Samuel Gauthier <samuel.gauthier@6wind.com> Reviewed-by: Simon Horman <simon.horman@netronome.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
openvswitch: retain parsed IPv6 header fields in flow on error skipping extension headers
When an error occurs skipping IPv6 extension headers retain the already
parsed IP protocol and IPv6 addresses in the flow. Also assume that the
packet is not a fragment in the absence of information to the contrary;
that is always use the frag_off value set by ipv6_skip_exthdr().
This allows matching on the IP protocol and IPv6 addresses of packets
with malformed extension headers.
Signed-off-by: Simon Horman <simon.horman@netronome.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
openvswitch: allow management from inside user namespaces
Operations with the GENL_ADMIN_PERM flag fail permissions checks because
this flag means we call netlink_capable, which uses the init user ns.
Instead, let's introduce a new flag, GENL_UNS_ADMIN_PERM for operations
which should be allowed inside a user namespace.
The motivation for this is to be able to run openvswitch in unprivileged
containers. I've tested this and it seems to work, but I really have no
idea about the security consequences of this patch, so thoughts would be
much appreciated.
v2: use the GENL_UNS_ADMIN_PERM flag instead of a check in each function
v3: use separate ifs for UNS_ADMIN_PERM and ADMIN_PERM, instead of one
massive one
Reported-by: James Page <james.page@canonical.com> Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com> CC: Eric Biederman <ebiederm@xmission.com> CC: Pravin Shelar <pshelar@ovn.org> CC: Justin Pettit <jpettit@ovn.org> CC: "David S. Miller" <davem@davemloft.net> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Paul Boca [Mon, 4 Jul 2016 09:53:23 +0000 (09:53 +0000)]
datapath-windows: Fixed packet limits on STT protocol
"If the length of the payload to be encapsulated exceeds 64KB, or if
the offset to the L4 header exceeds 255 bytes, then it will not be
possible to offload the packet to the NIC for segmentation." (STT RFC)
In this case the packet needs to be segmented by us, before sending it.
ovs-vtep: Handle tunnel key configuration in any order.
Presently, ovs-vtep expects the datapath tunnel key to be available
in the VTEP DB at startup. This may not be the case which is also
observed as interrmittent unit test failures. This patch allows
for the tunnel key to later appear in the VTEP database.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Acked-by: Russell Bryant <russell@ovn.org> Acked-by: Daniele Di Proietto <diproiettod@vmware.com>
ovn-controller-vtep: occasional failure in "binding 1" test case
The ovn-controller-vtep "binding 1" test case fails occasionally
due to a race with the ovs-vtep daemon. If ovs-vtep happens to
execute handle_physical() after "ovs-vsctl del-port p0", but before
the test script has executed "vtep-ctl del-port br-vtep p0", the
latter command will fail because ovs-vtep will have already deleted
p0 from the vtep db.
Eliminate this race while adding an additional check to verify that
ovs-vtep is working as expected by waiting for ovs-vtep to remove
entries for the deleted physical ports from the vtep db.
William Tu [Fri, 1 Jul 2016 16:45:52 +0000 (09:45 -0700)]
system-traffic: Remove datapath specific tests and macro.
We generally try to keep the testsuite independent of the underlying
datapath. This patch removes the datapath-specific tests and macros.
Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/141642065 Signed-off-by: William Tu <u9012063@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
IPv4 multicast is already dropped by a previous logical flow
written for the logical router, so there is no reason to add
a lower priority rule for it.
Signed-off-by: Ryan Moats <rmoats@us.ibm.com> Acked-by: Flavio Fernandes <flavio@flaviof.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Testing out the named pipe implementation revealed a problem in
"daemon --detach startup errors". If the daemon actually started nobody
is stopping it.
In the case of test failure kill the daemon.
Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> Acked-by: Paul Boca <pboca@cloudbasesolutions.com> Signed-off-by: Gurucharan Shetty <guru@ovn.org>
Ben Pfaff [Wed, 13 Jul 2016 22:53:20 +0000 (15:53 -0700)]
ofp-actions: Translate OF1.0 "enqueue" actions for OF1.1+.
Previously, the OF1.0 "enqueue" action was simply omitted when actions
were translated into OpenFlow 1.1 or later, which do not have a similar
action. This commit translates this action into an equivalent sequence
of actions.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
Russell Bryant [Thu, 7 Jul 2016 19:03:41 +0000 (15:03 -0400)]
OVN-Tutorial: Update sample output.
A previous patch made some additions to section 1 of this tutorial.
This patch includes updates to the sample output that I got while
testing the additions to the tutorial.
Signed-off-by: Russell Bryant <russell@ovn.org> Acked-by: Ryan Moats <rmoats@us.ibm.com>