Ethan Jackson [Thu, 7 Jun 2012 22:27:22 +0000 (15:27 -0700)]
packets: Use RARPs for learning packets.
Traditionally Open vSwitch had used 802.2 SNAP packets to update
upstream switch learning tables when necessary. This approach had
advantages in that debugging information could be embedded in the
packet helping hapless admins figure out what's going on. However,
since both qemu and VMware use RARP for this purpose, it seems
appropriate to fall in line with the defacto standard.
Requested-by: Ben Basler <bbasler@nicira.com> Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Thu, 7 Jun 2012 20:05:41 +0000 (13:05 -0700)]
ofproto: Fix use after free in ofoperation_complete().
In one edge case, ofoperation_complete() destroys its rule, without
updating its ofoperation that the rule is gone. Later in the same
function, ofoperation_destroy() attempts to modify the rule which
already destroyed.
Bug #11797. Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Fri, 1 Jun 2012 21:33:41 +0000 (14:33 -0700)]
packets: Generalize reserved RSPAN protocols.
Open vSwitch refuses to mirror certain destination addresses in
addition to those classified by eth_addr_is_reserved(). Looking
through the uses of eth_addr_is_reserved(), one finds that no
callers should be using the additional addresses which mirroring
drops. This patch folds the additional addresses dropped in the
mirroring code, into the more general eth_addr_is_reserverd()
function.
This patch also changes the implementation in a way that is
slightly less efficient, but much easier to read and extend int he
future.
Bug #11755. Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Thu, 7 Jun 2012 00:37:46 +0000 (17:37 -0700)]
packets: Fix eth_addr_equal_except().
It turns out that eth_addr_equal_except() computed the exact
opposite of what it purported to. It returned true if the two
arguments where *not* equal. This is extremely confusing, so this
patch changes it.
Bruce Davie [Wed, 6 Jun 2012 01:49:51 +0000 (18:49 -0700)]
ovsdb-client: Fix bugs in man page
In commit 53ffefe9 (ovsdb-client: Make "server" and "database"
arguments optional.), two errors were introduced. "list-columns"
appeared twice in the list of commands, the first instance should be
"list-tables". The "monitor" command now lists optional "column"
arguments.
Signed-off-by: Bruce Davie <bsd@nicira.com> Signed-off-by: Bruce Davie <bdavie@nicira.com> Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Fri, 1 Jun 2012 21:40:31 +0000 (17:40 -0400)]
dpif-linux: Log details when a packet is lost.
Until now, when a packet was dropped in the kernel-to-user buffers, we
logged the occurrence but nothing that would allow a person reading the
log after the fact to learn why it was dropped. This commit adds details
that identify the major sources of packets in the buffer, which should
help.
Ben Pfaff [Wed, 23 May 2012 23:55:09 +0000 (16:55 -0700)]
dpif-linux: Slightly refactor internal data structures.
An initial attempt also replaced the 'uint32_t ready_mask' in struct
dpif_linux by a 'bool ready' in each struct dpif_channel, but I wasn't
happy with the result (the ready_mask bitmap works out really well) and so
I dropped that part.
Ben Pfaff [Wed, 23 May 2012 21:56:20 +0000 (14:56 -0700)]
dpif-linux: Avoid pessimal behavior when kernel-to-user buffers overflow.
When a kernel-to-user Netlink buffer overflows, the kernel reports
ENOBUFS without passing along an actual message. When it does this,
we should immediately try again, because we know that there is a
message waiting, instead of reporting the error to the caller.
This improves the OVS response rate to "hping3 --flood" traffic by
a few percentage points in my testing.
Joe Stringer [Mon, 28 May 2012 12:38:21 +0000 (00:38 +1200)]
flow: Adds support for arbitrary ethernet masking
Arbitrary ethernet mask support is one step on the way to support for OpenFlow
1.1+. This patch set seeks to add this capability without breaking current
protocol support.
Signed-off-by: Joe Stringer <joe@wand.net.nz>
[blp@nicira.com made some updates, see
http://openvswitch.org/pipermail/dev/2012-May/017585.html] Signed-off-by: Ben Pfaff <blp@nicira.com>
Justin Pettit [Sat, 24 Mar 2012 08:02:26 +0000 (01:02 -0700)]
ofp-util: Clean up cookie handling.
Commit e72e793 (Add ability to restrict flow mods and flow stats
requests to cookies.) modified cookie handling. Some of its behavior
was unintuitive and there was at least one bug (described below).
Commit f66b87d (DESIGN: Document uses for flow cookies.) attempted to
document a clean design for cookie handling. This commit updates the
DESIGN document and brings the implementation in line with it.
In commit e72e793, the code that handled processing OpenFlow flow
modification requests set the cookie mask to exact-match. This seems
reasonable for adding flows, but is not correct for matching, since
OpenFlow 1.0 doesn't support matching based on the cookie. This commit
changes to cookie mask to fully wildcarded, which is the correct
behavior for modifications and deletions. It doesn't cause any problems
for flow additions, since the mask is ignored for that operation.
Joe Stringer [Tue, 29 May 2012 18:07:16 +0000 (11:07 -0700)]
packets: Adds ethernet-matching helper functions
With OpenFlow 1.1 requiring arbitrary ethernet match support, it simplifies
other code if we have some extra helper functions. This patch adds
eth_mask_is_exact(mask), eth_addr_bitand(src, mask, dst),
eth_addr_equal_except(a, b, mask) and eth_format_masked(eth, mask, output).
Signed-off-by: Joe Stringer <joe@wand.net.nz> Signed-off-by: Ben Pfaff <blp@nicira.com>
Eric Dumazet [Fri, 25 May 2012 18:07:35 +0000 (11:07 -0700)]
datapath: cleanup unsigned to unsigned int
Use of "unsigned int" is preferred to bare "unsigned" in net tree.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Tue, 15 May 2012 19:50:57 +0000 (12:50 -0700)]
odp-util: Update ODPUTIL_FLOW_KEY_BYTES for current kernel flow format.
Before we submitted the kernel module upstream, we updated the flow format
by adding two fields to the description of packets with VLAN headers, but
we forgot to update ODPUTIL_FLOW_KEY_BYTES to reflect these changes. The
result was that a maximum-length flow did not fit in the given space.
This fixes a crash processing IPv6 neighbor discovery packets with VLAN
headers received in a tunnel configured with key=flow or in_key=flow.
This updates some comments to better describe the implications of
ODPUTIL_FLOW_KEY_BYTES (suggested by Justin).
This also updates test-odp.c so that it would have caught this problem, and
updates odp.at to demonstrate that a full 156 bytes are necessary. (To see
that, revert the change to ODPUTIL_FLOW_KEY_BYTES and run the test.)
Reported-by: Dan Wendlandt <dan@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Pravin B Shelar [Thu, 24 May 2012 00:25:41 +0000 (17:25 -0700)]
datapath: Avoid system freeze due to ovs-flow-rehash softlockup.
OVS datapath does periodic flow table rehash which takes genl_lock
in workq context.
In some cases, like ports add or delete, genl_lock can cause softlockup
as vswitchd would take and succeed with genl_lock and rehash workq
would block on the lock. Eventually rehash will proceed, flow rehash
is low priority task so this is not problem for rehashing.
But it is blocking workq thread; some other workq item from other
kernel subsystem would be blocked and can cause system freeze.
To avoid workq blocking and system freeze, we can use OVS compat workq.
It runs in separate kernel thread thus does not block any non-ovs
deferred workq work item.
We will fix it by making genetlink lockless and having fine granular
locking in OVS.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com> Acked-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Tue, 22 May 2012 17:32:02 +0000 (10:32 -0700)]
simap: New data structure for string-to-integer maps.
This commit adapts a couple of existing pieces of code to use the
new data structure. The following commit will add another user
(which is also the first use of the simap_increas() function).
Ben Pfaff [Tue, 1 May 2012 21:03:52 +0000 (14:03 -0700)]
vlog: Take advantage of relaxed "-v" syntax through the tree.
The vlog manpage implies that writing ANY explicitly is obsolete, but
examples elsewhere in the documentation and code still tend to add it.
This removes them.
Ben Pfaff [Mon, 14 May 2012 18:26:36 +0000 (11:26 -0700)]
python: Break unixctl implementation into registry, client, and server.
I wish to add some unixctl commands to the Python vlog module. However,
importing ovs.unixctl in ovs.vlog creates a circular dependency, because
ovs.unixctl imports ovs.vlog already. The solution, in this commit, is to
break the unixctl module into three parts: a register (ovs.unixctl) that
does not depend on ovs.vlog, and client (ovs.unixctl.client) and server
(ovs.unixctl.server) modules that do. This breaks the circular dependency.
Ben Pfaff [Mon, 30 Apr 2012 23:49:59 +0000 (16:49 -0700)]
tests: Fix Emacs syntax highlighting in vlog.at.
[^"] confuses the Emacs syntax highlighter for Autotest mode. It thinks
that it terminates a quoted string and therefore everything following the
next " in the file is highlighted as if it was part of a quoted string.
Ben Pfaff [Tue, 22 May 2012 17:17:00 +0000 (10:17 -0700)]
Use PYTHONDONTWRITEBYTECODE=yes for invoking Python for build or test.
An upcoming commit will break the ovs.vlog module into an ovs.vlog package
with submodules. This commit makes switching between trees with the old
structure and those with the new structure much easier.
This commit works by setting PYTHONDONTWRITEBYTECODE=yes in Python
invocations from the build system and testing. This keeps Python 2.6+ from
creating .pyc and .pyo files. Creating .py[co] works OK for any given
version of Open vSwitch, but it causes trouble if you switch from a version
with foo/__init__.py into an (older) version with plain foo.py, since
foo/__init__.pyc will cause Python to ignore foo.py.
Ethan Jackson [Mon, 21 May 2012 20:20:18 +0000 (13:20 -0700)]
bridge: Ignore "null" interfaces as required.
Commit bae7208e91a0 (bridge: Refactor bridge_reconfigure().)
introduced a regression in which the switch would attempt to
instantiate "null" interfaces in the datapath. This would, of
course, fail and trigger a warning. Though harmless, these
warnings confused users.
Jesse Gross [Thu, 17 May 2012 18:43:15 +0000 (11:43 -0700)]
datapath: Reset upper layer protocol info on internal devices.
It's possible that packets that are sent on internal devices (from
the OVS perspective) have already traversed the local IP stack.
After they go through the internal device, they will again travel
through the IP stack which may get confused by the presence of
existing information in the skb. The problem can be observed
when switching between namespaces. This clears out that information
to avoid problems but deliberately leaves other metadata alone.
This is to provide maximum flexibility in chaining together OVS
and other Linux components.
Bug #10995
Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Arun Sharma [Wed, 16 May 2012 18:39:55 +0000 (11:39 -0700)]
ovs-vsctl: Add "--all" option for "destroy" command in ovs-vsctl.
Adds the ability to delete all records from table. This will help
users to destroy all records from Qos or Queue table using single
command rather then current method.
Feature #11306 Suggested-by: Kevin Mancuso <kevin.mancuso@rackspace.com> Signed-off-by: Arun Sharma <arun.sharma@calsoftinc.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
dpif-netdev: allow for proper destruction of netdev datapaths
Until now, bridges with datapath_type=netdev did not destroy the datapath
when deleted. In particular, the tap device implementing the internal
interface was not close()d, and therefore the tap persists until
ovs-vswitchd exit()s.
This behaviour was caused by the missing callback for 'enumerate' in the
dpif-netdev class. Without this callback 'bridge_reconfigure' failed to
realize that there are datapaths with no bridge, and thus cannot destroy
them. Providing an 'enumerate' callback fixes this.
Signed-off-by: Giuseppe Lettieri <g.lettieri@iet.unipi.it> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Sun, 13 May 2012 23:41:05 +0000 (16:41 -0700)]
ovs-ofctl: Avoid impossible check for !osm in fetch_port_by_stats().
At the time of the call to ofpbuf_at(), we know that the ofp_stats_msg is
present because ofputil_decode_msg_type() reported that it was. Therefore,
we can use ofpbuf_at_assert() and don't have to check for a null pointer.
In ovs-bugtool, we do a bunch of Popen calls to
get the results of some shell commands with stdout
set to PIPE. Once we are done, we need to
close the file descriptors.
Arun Sharma [Wed, 9 May 2012 23:34:21 +0000 (16:34 -0700)]
bugtool: rename label names in plugins related to ovs-appctl
It improves to have proper out file name in bugtool archive with respect
to ovs-appctl commands. E.g. if command is 'ovs-appctl lacp/show' then
the related out file will be 'ovs-appctl-lacp-show.out'
Feature #11283. Signed-off-by: Arun Sharma <arun.sharma@calsoftinc.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Arun Sharma [Wed, 9 May 2012 06:47:30 +0000 (23:47 -0700)]
bugtool: Collect bond state information from ovs.
This is an enhancement in bugtool archive output to determine the bond
state information. It is implemented as a plugin which internally calls
"ovs-appctl bond/show" command to get bond state.
Feature #11283. Signed-off-by: Arun Sharma <arun.sharma@calsoftinc.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Sat, 5 May 2012 18:07:42 +0000 (11:07 -0700)]
ofproto-dpif: Segregate CFM, LACP, and STP traffic into separate queues.
Until now, packets for these special protocols have been mixed with general
traffic in the kernel-to-userspace queues. This means that a big-enough
storm of new flows in these queues can cause packets for these special
protocols to be dropped at this interface, fooling userspace into believing
that, say, no CFM packets have been received even though they are arriving
at the expected rate.
This commit moves special protocols to a dedicated kernel-to-userspace
queue to avoid the problem.
Bug #7550. Signed-off-by: Ben Pfaff <blp@nicira.com>
Most exact-match flows can be handled directly in the datapath, but
for various reasons, some cannot: every packet in these flows must
be sent separately to userspace. Until now, flows that cannot be
handled entirely in the kernel have been allowed to miss each time
in the datapath. This is generally OK, but it has a few
disadvantages:
* It can make troubleshooting at the level where one must look
at datapath flows a bit confusing in some cases, because
datapath misses due to genuinely new flows are mixed in with
datapath misses for known flows that cannot be set up.
* It means that the kernel-to-userspace packets for a given
input port always go to a single kernel-to-userspace queue,
even if we'd like to segregate out some of the packets for
known flows. (An upcoming commit has examples.)
This commit therefore introduces the concept of a "slow path" flow,
one that is installed in the datapath with a single action that
sends the flow's packets to userspace. To make troubleshooting
easier, the action includes a reason code (displayed by "ovs-dpctl
dump-flows") that explains why the flow has been slow-pathed.
Bug #7550. Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Sat, 5 May 2012 17:55:30 +0000 (10:55 -0700)]
ofproto-dpif: Introduce "internal flows" for handling flow table misses.
The ofproto-dpif implementation of "facet"s requires a facet to be
associated with an OpenFlow rule. Until now, this meant that packets
that miss in the OpenFlow table (and thus didn't have OpenFlow rules)
couldn't be set up as facets and thus couldn't be installed in the
kernel. This commit changes that, by introducing "internal" OpenFlow
rules to associate with such packets.
Ben Pfaff [Fri, 4 May 2012 21:56:40 +0000 (14:56 -0700)]
odp-util: Change user_action_cookie from struct to union.
An upcoming commit will introduce a new type and a new use for the
additional members. It seems cleanest to use a union, rather that using
the existing members multiple ways.
Ben Pfaff [Fri, 4 May 2012 21:52:17 +0000 (14:52 -0700)]
ofproto: Don't limit flows in OpenFlow tables by default.
This bug is ordinarily not exposed because bridge_configure_tables() in
bridge.c configures the max number of flows soon after an ofproto is
created. But an upcoming commit will make construct() in ofproto-dpif.c
try to create some built-in flows before bridge gets control, so we need
to allow creating flows immediately upon initialization.
Brian Kruger [Wed, 9 May 2012 16:13:42 +0000 (09:13 -0700)]
rhel: Add timeouts to network scripts.
If the daemon(s) aren't running for whatever reason, the RHEL ovs
ifup/ifdown scripts don't take that into account and an attempt to reboot a
system could take forever. (literally. endless loop!) Here are a couple of
patches (one of ifup, one for ifdown) to add timeouts (10 seconds), because
it runs per interface you have configured and that could take awhile to
reboot a system if needed.
Signed-off-by: Brian Kruger <bkruger+ovsdev@gmail.com>
[blp@nicira.com fixed up a conflict against master] Signed-off-by: Ben Pfaff <blp@nicira.com>
debian: Remove --no-wait option from the ifupdown script.
Using the --no-wait option in the ifupdown script creates a
race condition where-in the network devices may not yet be created
after ovs-vsctl returns successfully.
Justin Pettit [Sat, 5 May 2012 00:27:16 +0000 (17:27 -0700)]
ovs-ofctl: Support large number of ports with "show" command.
OpenFlow Features Reply messages prior to 1.3 can give users the wrong
impression about how many ports are on the system. With this commit,
the command will check if the number of ports may be truncated. If so,
it will send a Port Description stats request to get the complete list
and ignore the Features Reply port list.
Add function to determine whether the max number of ports are contains
in a Features Reply. If so, it removes the port list, since it may be
incomplete. This function will be used in a later commit.
Justin Pettit [Fri, 4 May 2012 21:42:04 +0000 (14:42 -0700)]
ofproto: Add support for OF1.3 port description multipart message.
OpenFlow 1.0 is limited to displaying 1364 ports in the Features Reply
message, and there is no other way to get consolidated port information.
OpenFlow 1.3 adds a new port description multipart message
(OFPMP_PORT_DESC) that is not limited by size. This commit adds support
through the OpenFlow 1.0 stats mechanism, since they have complimentary
enum values.
Justin Pettit [Wed, 2 May 2012 23:35:49 +0000 (16:35 -0700)]
OpenFlow: Move stats message enums into "common".
We will be adding some OpenFlow 1.3 stats (aka multipart request)
messages to our OpenFlow 1.0 implementation. As such, move the
definition of those message numbers to the common location.
Pravin B Shelar [Tue, 8 May 2012 00:04:57 +0000 (17:04 -0700)]
datapath: Validation of IPv6 set port action uses IPv4 header
When the kernel validates set TCP/UDP port actions, it looks at
the ports in the existing flow to make sure that the L4 header exists.
However, these actions always use the IPv4 version of the struct.
Following patch fixes this by checking for flow ip protocol first.
Ben Pfaff [Mon, 7 May 2012 19:30:54 +0000 (12:30 -0700)]
ofp-util: Treat a packet-out in_port of OFPP_CONTROLLER as OFPP_NONE.
Some OpenFlow 1.0 controllers incorrectly use OPFP_CONTROLLER as the
in_port in packet-out messages, when OFPP_NONE is their intent. Until now,
Open vSwitch has rejected such requests with an error message. This commit
makes Open vSwitch instead treat OFPP_CONTROLLER the same as OFPP_NONE for
compatibility with those controllers.
(Also, as of this writing, OpenFlow 1.0.1 appears to be changing the port
to use from OFPP_NONE to OFPP_CONTROLLER.)
Suggested-by: Rob Sherwood <rob.sherwood@bigswitch.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 4 May 2012 17:05:44 +0000 (10:05 -0700)]
ofproto-dpif: Accept OpenFlow-like flows in "ofproto/trace".
Until now it has not been possible to directly trace flows that include
register values and other concepts that are not in datapath flows, because
"ofproto/trace" requires a flow in the format output by "ovs-dpctl
dump-flows", which doesn't know anything about registers. This commit
makes it possible to instead specify an OpenFlow-like flow.
Feature #10084. Requested-by: Igor Ganichev <iganichev@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 4 May 2012 16:52:37 +0000 (09:52 -0700)]
ofp-parse: New function parse_ofp_exact_flow().
This function parses a flow rather than a cls_rule. It will be useful
for "ofproto/trace", which currently requires an odp_flow and thus can't
accept values for registers and other concepts that don't exist in the
kernel.