OpenFlow 1.3 says that the band_types member of struct ofp_meter_features
is a bitmap of OFPMBT_* values. The OFPMBT_* values are 1-based, so
until now, to avoid wasting bit 0, OVS mapped an OFPMBT_* with value 1 to
bit 0, value 2 to bit 1, and so on. However, according to
http://openvswitch.org/pipermail/dev/2013-July/029666.html,
other OpenFlow implementations directly translate value 1 to bit 1,
value 2 to bit 2, and so on. This commit changes Open vSwitch to use this
more common interpretation.
A request for clarification of this issue in the OpenFlow standard has been
filed with the ONF Extensibility Working Group as issue EXT-345.
Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
When user switches between using CFM and BFD, there will be a short
down time before the new protocol goes up. This can unintentionally
change the traffic pattern of the bundled ports. To prevent this,
it is proposed that user can enable both CFM and BFD before transition,
wait for the new protocol to go up, and then disable the old protocol.
To make this scheme work, this commit modifies the port_run() in
ofproto-dpif.c, so that when both CFM and BFD are used, if either shows
correct status, the port is considered usable in the bundle.
Signed-off-by: Alex Wang <alexw@nicira.com>
When there is no incoming data traffic at the interface for a period,
BFD decay allows the bfd session to increase the min_rx. This is
helpful in that some interfaces may usually be idle for a long time.
And cpu consumption can be reduced by processing fewer bfd control
packets.
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Avoid relying on a non-portable implementation detail for atomic_flag
tests. Per the standard, the only way to obtain the value of the flag
is via the return value from atomic_flag_test_and_set.
Signed-off-by: Ed Maste <emaste@freebsd.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This commit fixes the bug introduced by commit 26131299fa (bfd: Make the
BFD module thread safe.). The bug caused the opposite of the intended
behaviour.
Unit test is added for the 'check_tnl_key' feature.
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The Linux kernel datapath enables matching and setting the skb mark
but this functionality is currently used only internally by
ovs-vswitchd. This exposes it through NXM to enable external
controllers to interact with other kernel subsystems. Although this
is simply exporting the skb mark, the intention is that this is a
platform independent mechanism to access some system metadata and
therefore may have different implementations on various systems.
Bug #17855
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
Currently we remove the IPSEC_MARK flag from all packets that are
egressing on non-tunnel ports. However, this behavior is confusing
if we allow OpenFlow controllers to match and set the pkt_mark field
because the tunnel behavior applies even on non-tunnel ports.
This instead clears the mark on tunnel input which should have the
same effect for tunnel ports. However, on non-tunnel traffic (or
even for traffic entering on a tunnel port but leaving on a non-
tunnel port) it allows the mark to pass through without change.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
The skb_mark field is currently only available with the Linux datapath
and is only used internally. However, it is desirable to expose this
through OpenFlow and when it is exposed ideally it would not be system-
specific. In preparation for this, skb_mark is rename to pkt_mark in
internal data structures for consistency.
This does not rename the Linux interfaces because doing so would break
the API. It would not necessarily be desirable to do anyways since in
Linux-specific code it is clearer to use the actual name rather than a
generic one. This can lead to confusion in some places, however, because
we do not always strictly separate generic and platform dependent code
(one example is actions). This seems inevitable though at this point if
the lower and upper layers have different names (as they must given the
above requirements).
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
Threaded ofproto-dpif uses a queue to pass packets from the forwarding
threads to the main thread for (mega)flow setup and for learning. When
learning occurs, causing revalidations, this races against flow setup, so
that sometimes a datapath (mega)flow does get set up for a packet that
causes learning and sometimes it doesn't. This caused this test to
sometimes fail because one megaflow or the other that was expected to be
set up was not.
This commit fixes the problem by sending a second packet in each flow.
These additional packets don't cause any additional changes to the flow
table but they do cause flows to be set up, fixing the problem.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This test had two problems. First, it had a bizarre dependency on stats
that were not up-to-date: the "ovs-ofctl dump-flows" assumed that only
the first one of ten packets sent through the switch had been accounted
to OpenFlow flow statistics. Adding a 1-second time warp fixed this
problem by ensuring that all ten packets were accounted. (That's why this
patch updates the expected output of "ovs-ofctl dump-flows".)
Second, multithreading has made packet processing less predictable in
general. This commit adds 10-ms time warps after sending each packet,
which seems to make the test reliable for me.
Signed-off-by: Ben Pfaff <blp@nicira.com>
The current Netlink protocol allows a default value of zero if either mark
or priority is not specified (this is part of the ABI). Until now, when
userspace serializes either the value or mask, it looked at the value and
omitted the netlink attribute if it is zero. This is a bug because an
exact match on zero turns into a wildcard of the field.
These two fields (plus input port and EtherType) are special because they
can be omitted whereas most other values are required to be fully
specified. These protocol variations tend to cause bugs (as above) when we
evolve the protocol because an exception that makes sense in one context
might not be logical in another. Since the default value for mark and
priority are merely shorthands, we can push the protocol in a more
consistent direction by ignoring the shortcut and always serializing the
values. This is what this commits does.
Signed-off-by: Andy Zhou <azhou@nicira.com>
[blp@nicira.com added Jesse's text to the commit message]
Signed-off-by: Ben Pfaff <blp@nicira.com>
Run the ovs-appctl time/stop command after OVS_VSWITCHD_START.
Also increase the wait time before checking if BFD session is up in
test 4.
Signed-off-by: Pavithra Ramesh <paramesh@vmware.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
When verbose mode tuned on, all dp flow fields described by the netlink
attributes are displayed, including fully wildcarded attributes.
Otherwise, the fully wildcarded attributes are omitted for brevity.
Added -m option to "ovs-dpctl dump-flows" to enable verbose mode. It is
off by default.
Signed-off-by: Andy Zhou <azhou@nicira.com>
[blp@nicira.com added documentation]
Signed-off-by: Ben Pfaff <blp@nicira.com>
The ODP library has an optimization to not set a header if the field was
not changed, regardless of whether an action to set the field was
present. That library is also responsible for un-wildcarding fields
that are bieng modified. This leads to a problem where a packet matches
a flow that updates a field, but that particular packet's field already
has that value. As such, an overly loose megaflow will be generated
that doesn't match on that field and the actions won't update it. A
second packet that should have the field set will match that flow and
will not be modified.
This commit changes the behavior to always un-wildcard fields that are
being modified. Since the ODP library updates the entire header if a
field in it is modified, and all those fields will be un-wildcarded, the
generated flows may be different. However, they should be correct.
Bug #18946.
Reported-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
This patch reverts commit 5559942 (ofproto-dpif: GOTO_TABLE recursion
removal.) by reintroducing the recursion through xlate_table_action().
The main reason to do this is the introduction of new rule locking in
future patches. The code before this patch was relatively difficult
to lock in a clean straight-forward manner.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Commit 97be153858 (clang: Add
annotations for thread safety check.) defined 'struct ovs_mutex'
variable in 'atomic_flag' in 'ovs-atomic-pthreads.h'. This
casued "mutex: has incomplete type" error in compilation when
'ovs-atomic-pthreads.h' is included.
This commit goes back to use 'pthread_mutex_t' for that variable
and adds test for the 'atomic_flag' related functions.
Reported-by: Gurucharan Shetty <gshetty@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This bug causes the flag mask to always mask only 1 bit, not the 2 bits
possible. While at it, make the top 6 bits exact match.
Bug #18834.
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
This reverts commit 05d299e0cc (test-atomic:
Drop atomic read-modify-write tests for the moment.) because the
test for detecting whether GCC support atomic operation built-ins has
been fixed.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
This can happen on ESX.
Also adds a test to make sure this works.
Bug #17634.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Tested-by: Guolin Yang <gyang@vmware.com>
OpenFlow 1.0, 1.1, and 1.2 all have the same struct size for port and
queue stats. OpenFlow 1.3 has larger structs, but the ofp-util code didn't
realize that. This fixes the problem.
It appears that the only consequence of this problem would have been
printing the wrong count in ofp-print output.
Signed-off-by: Ben Pfaff <blp@nicira.com>
The "ovs-appctl dpif/dump-flows" command wasn't updated to print
megaflows, so the output would not include wildcards even though the
datapath may, so the output was inconsistent and confusing.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Fix a corner case bug where ARP packet with ARP opcode value of 1 would cause
tp_src and tp_dst to appear in the output string.
This bug caused some output from flow_format() to not be accepted by
'ovs-appctl ofproto/trace'
Added test coverage by using ARP opcode 1 in one unit test case, that
would have exposed the bug.
Bug #18334
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
I don't see anything that guaranteed that ovs-ofctl would receive and print
the packets before it exited. This commit inserts explicit waits, to avoid
the problem.
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com>
hindex_next() make the completely wrong assumption that head nodes within
a bucket were sorted in ascending order by hash. This commit removes
that assumption.
Also add a test that would have found the problem.
Signed-off-by: ZhengLingyun <konghuarukhr@163.com>
[blp@nicira.com changed how hindex_head_node() is implemented and
other code details]
Signed-off-by: Ben Pfaff <blp@nicira.com>
Commit 4e022ec09e (Create specific types for ofp and odp port) broke
OpenFlow OFPP_PACKET_OUT requests that use in_port=OFPP_CONTROLLER. This
commit fixes the problem and adds a regression test.
CC: Alex Wang <alexw@nicira.com>
Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Commit 89f3c258fe (ovs-vsctl: Improve error message for "ovs-vsctl del-port
<bridge>".) changed the behavior of
ovs-vsctl --if-exists del-port <bridge>
from a silent no-op to a hard failure. This commit fixes this regression.
This caused problems on XenServer, for which the Open vSwitch integration
runs commands like:
/usr/bin/ovs-vsctl --timeout=20 \
-- --with-iface --if-exists del-port xapi103 \
-- --if-exists del-br xapi103
Bug #18276.
Reported-by: Michael Hu <mhu@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The latest version of sparse triggers the following warning in the
checksum tests. This patch suppresses it.
warning: too long initializer-string for array of char
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Until now, failure to parse a flow in the ofp-parse module has caused the
program to abort immediately with a fatal error. This makes it hard to
use these functions from any long-lived program. This commit fixes the
problem.
Signed-off-by: Ben Pfaff <blp@nicira.com>
XenServer builds are failing because of link errors reporting that
__sync_fetch_and_<op>_<size> were not found, for <op> in add/sub/and/xor/or
and <size> in 1/2/4/8. We're not actually using these RMW operations yet,
so as a stopgap measure just drop the tests.
The correct long-term fix is probably to do Autoconf linkage tests for
these operations instead of just testing the GCC version (if we really need
the operations at all).
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
None of these test programs are threaded, but has little cost and means
that "grep" doesn't turn up any instances of these thread-unsafe functions
in our tree.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This library should prove useful for the threading changes coming up.
The following commit introduces one (very simple) user.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
The commit allows a user to add a database file to a
ovsdb-server during run time. One can also remove a
database file from ovsdb-server's control.
Feature #14595.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Co-authored-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
OpenFlow 1.0 can match on flows that have the IPv6 Ethertype, but
ofputil_usable_protocols() incorrectly reported that such a match required
NXM or OXM. This commit fixes the problem.
Also, add some related tests.
Reported-by: Nagi Reddy Jonnala <njonnala@Brocade.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
ovs-dpctl sometimes displays wildcarded fields as exact match. This
patch fixes those cases.
This patch implements the following logic. When OVS_FLOW_ATTR_MASK is
missing, the entire key attributes will be displayed as exact match fields.
When OVS_FLOW_ATTR_MASK is present, but some individual key attributes do
not have matching attributes in the mask, those key attributes will be
displayed as wildcarded fields.
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
