Ben Pfaff [Wed, 10 Nov 2010 22:51:49 +0000 (14:51 -0800)]
ofp-util: Make make_flow_mod() take cls_rule instead of struct flow.
This reduces code duplication, by eliminating a function that translates
from "struct flow" to "struct ofp_match" in favor of the existing function
ofputil_cls_rule_to_match(). It also allows the caller to specify the
desired priority (as part of the cls_rule).
Ben Pfaff [Wed, 10 Nov 2010 22:39:54 +0000 (14:39 -0800)]
flow: Fully separate flow_wildcards from OpenFlow wildcard bits.
Originally, wildcards were just the OpenFlow OFPFW_* bits. Then, when
OpenFlow added CIDR masks for IP addresses, struct flow_wildcards was born
with additional members for those masks, derived from the wildcard bits.
Then, when OVS added support for tunnels, we added another bit
NXFW_TUN_ID that coexisted with the OFPFW_*. Later we added even more bits
that do not appear in the OpenFlow 1.0 match structure at all. This had
become really confusing, and the difficulties were especially visible in
the long list of invariants in comments on struct flow_wildcards.
This commit cleanly separates the OpenFlow 1.0 wildcard bits from the
bits used inside Open vSwitch, by defining a new set of bits that are
used only internally to Open vSwitch and converting to and from those
wildcard bits at the point where data comes off or goes onto the wire.
It also moves those functions into ofp-util.[ch] since they are only for
dealing with OpenFlow wire protocol now.
Ben Pfaff [Mon, 22 Nov 2010 18:10:14 +0000 (10:10 -0800)]
flow: Remove flow_to/from_match() in favor of cls_rule_to/from_match().
The flow_from_match() and flow_to_match() functions have to deal with most
of the state in a cls_rule anyhow, and this will increase in upcoming
commits, to the point that we might as well just use a cls_rule anyhow.
This commit therefore deletes flow_from_match() and flow_to_match(),
integrating their code into cls_rule_from_match() and the new function
cls_rule_to_match(), respectively. It also changes each of the functions'
callers to use the new cls_rule_*() function.
Ben Pfaff [Sat, 20 Nov 2010 00:41:02 +0000 (16:41 -0800)]
classifier: Fix segfault iterating with rules that differ only in priority.
When CLS_CURSOR_FOR_EACH(_SAFE) iterated through a classifier, the
cls_cursor_next() function did not properly handle the case where there was
more than a single rule on a list. This commit fixes the problem.
The addition to the testsuite would have found the problem earlier.
Reported-by: Teemu Koponen <koponen@nicira.com> CC: Teemu Koponen <koponen@nicira.com>
Ben Pfaff [Fri, 19 Nov 2010 21:07:56 +0000 (13:07 -0800)]
netdev-vport: Fix segfault on bad vport configuration.
When a vport parse error occurs, the vport_class's parse_config function
doesn't necessarily store a valid pointer into the vport_info's 'config'
member, so netdev_vport_create() needs to supply a null pointer here to
avoiding passing a wild pointer to free().
Ben Pfaff [Thu, 18 Nov 2010 22:36:53 +0000 (14:36 -0800)]
ovs-vsctl: Fix br-set-external-id.
The wrong prerequisite function was being called for set-br-external-id, so
it didn't set up to monitor the right columns, so an assertion failed.
This fixes the problem.
Also, change the test for external IDs so that it will find the regression.
(Running all of the ovs-vsctl commands together caused the column to be
monitored due to another command's intervention.)
Reported-by: Mikael Doverhag <mdoverhag@nicira.com>
Bug #4095.
Ben Pfaff [Thu, 18 Nov 2010 18:06:41 +0000 (10:06 -0800)]
dpif: Make dpif_class 'open' function take class instead of type name.
This makes it easier for dpif_provider implementations to share code but
distinguish the class actually in use, because comparing a pointer is
easier than comparing a string.
Ben Pfaff [Thu, 18 Nov 2010 18:07:48 +0000 (10:07 -0800)]
dpif: Use caller-provided dpif_class in dp_register_provider().
I don't see a reason here to copy the dpif_class instead of using the
pointer provided by the caller. Using the caller's pointer allows the
caller to compare a dpif's 'class' member against the address of its
class structure, which seems like a reasonable thing to do.
Ben Pfaff [Tue, 16 Nov 2010 19:00:25 +0000 (11:00 -0800)]
ofp-util: Consistently treat OpenFlow xids as network byte order.
The 'xid' in an ofp_header is not interpreted by the receiver but only by
the sender, so it need not be in any particular byte order. OVS used to
try to take advantage of this to avoid host/network byte order conversions
for this field. Older code in OVS, therefore, treats xid as being in host
byte order. However, as time went on, I forgot that I had introduced this
trick, and so newer code treats xid as being in network byte order.
This commit fixes up the situation by consistently treating xid as being
in network byte order. I think that this will be less surprising and
easier to remember in the future.
This doesn't fix any actual bugs except that some log messages would have
printed xids in the wrong byte order.
Ben Pfaff [Mon, 15 Nov 2010 19:04:40 +0000 (11:04 -0800)]
ofp-util: Use a counter for transaction IDs instead of a random number.
I don't know of any reason why the transaction id should be random. Using
consecutive ids means that there is no chance that two messages sent around
the same time will have the same transaction ID, which is probabilitically
possible with random IDs.
Ben Pfaff [Mon, 15 Nov 2010 20:48:31 +0000 (12:48 -0800)]
netdev-vport: Fix poll_add() implementation.
The existing implementation never worked because it used different strings
for notifier shash addition and lookup: for adding to the shash, it used
the vport name; for lookup, it used "<type>:<name>". This fixes the
problem, by using "<type>:<name>" in both cases.
Ben Pfaff [Tue, 16 Nov 2010 18:25:22 +0000 (10:25 -0800)]
ovs-ofctl: Fix return value of str_to_port_no().
The index into the array of ports returned in the OFPT_FEATURES_REPLY
message is not necessarily the same as the port number, so this fixes a
real bug.
Ben Pfaff [Tue, 16 Nov 2010 23:14:58 +0000 (15:14 -0800)]
Make the "exit" unixctl command reliable in ovsdb-server, ovs-vswitchd.
If "ovs-appctl exit" happens to hit ovsdb-server or ovs-vswitchd at a
moment when nothing else is happening to wake the daemon up, it can take a
long time for them to exit.
This seems to account for occasional "make check" failures on Nicira's
internal builds. It probably fixes some Debian automatic build failures
as well.
Ben Pfaff [Tue, 16 Nov 2010 22:09:41 +0000 (14:09 -0800)]
debian: Fix parallel builds.
Some of the Debian automatic builds are failing apparently because the
build-arch and build-indep targets are being called in parallel and they
are both attempting to rebuild tests/testsuite at the same time, which
doesn't work. We could make the OVS makefiles handle this particular case,
but in general it's not a good idea to independently invoke a single
makefile multiples times in parallel, so this commit avoids that problem.
This also dumps the DEB_BUILD_OPTIONS variable to the log, because there
doesn't appear to be any way to infer it from the automatic builders'
log files.
Ben Pfaff [Tue, 16 Nov 2010 22:06:31 +0000 (14:06 -0800)]
debian: On "make check" failure dump the testsuite log.
The Debian automatic build machines don't save any files from the build,
but they do preserve the build log, so dumping it to stdout is the only
reasonable way to get test failure information.
Ben Pfaff [Tue, 16 Nov 2010 17:15:40 +0000 (09:15 -0800)]
ovs-vsctl: Replicate only tables and columns that are actually needed.
Until now, ovs-vsctl has always fetched a complete replica of the Open
vSwitch database at startup time. This is a little expensive, but that is
not normally a problem because the database is small. However, it can
cause problems when Open vSwitch is partially upgraded: if ovs-vsctl is
upgraded before the database, and new columns or tables were added to the
database schema in the newer version of Open vSwitch, then ovs-vsctl will
report an error and fail to work, because it cannot monitor the new
columns. The same problem occurs if the database is upgraded before
ovs-vsctl and columns or tables were removed in the new version of Open
vSwitch.
This commit fixes the problem in the most common case, by making ovs-vsctl
only replicate the columns and tables in the database that it will actually
use during a given execution. This will still fail if the database has
changed in major ways, but only if the changes would actually cause
problems for what ovs-vsctl is trying to do anyhow.
Ben Pfaff [Tue, 16 Nov 2010 17:14:52 +0000 (09:14 -0800)]
ovsdb-idl: Make selecting tables and columns to replicate more flexible.
Until now, by default the IDL replicated all tables and all columns in the
database, and a few functions made it possible to avoid replicating
selected columns. This commit adds a mode in which nothing is replicated
by default and the client code is responsible for specifying each column
and table that it is interested in. The following commit adds a user for
this mode.
Ben Pfaff [Mon, 30 Aug 2010 21:12:27 +0000 (14:12 -0700)]
ovsdb: Support replicating a table without including any columns.
The OVSDB wire protocol has a "monitor" feature that supports table
replication on a column-by-column basis. This wire protocol in theory
supports replicating a table without replicating any columns in it. In
such a case, the client only tracks the UUIDs of the rows in the table.
However, the ovsdb-server implementation did not support this possibility
properly. This commit fixes the bug.
Ben Pfaff [Mon, 15 Nov 2010 18:21:33 +0000 (10:21 -0800)]
debian: Add procps to Build-Depends.
"make check" runs "ps" so the procps package is necessary. With this
change, I was able on Friday to make the Open vSwitch Debian packages build
successfully under the "sbuild" suite used by the Debian autobuilders, so
I hope that this fixes at least some of the Debian buildd problems.
CC: Simon Horman <horms@verge.net.au>
Debian bug #602891.
Ben Pfaff [Sat, 13 Nov 2010 00:23:26 +0000 (16:23 -0800)]
nicira-ext: New Nicira vendor action NXAST_NOTE.
Our controller group at Nicira has requested a way to annotate flows with
extra information beyond the flow cookie. The new NXAST_NOTE action
provides such a way.
This new action is somewhat controversial. Some have suggested that it
should be added another way (either as part of the Nicira Extended Match
or as a new component of the flow_mod and related messages). Others think
that it has no place in the OpenFlow protocol at all and that an equivalent
should be implemented using the already available features of OVSDB. So
it is possible that this extension will be deleted and the feature will
be reimplemented some other way (or not at all).
CC: Teemu Koponen <koponen@nicira.com> CC: Jeremy Stribling <strib@nicira.com>
Ben Pfaff [Tue, 2 Nov 2010 17:45:33 +0000 (10:45 -0700)]
vswitch: Add other-config:in-band-queue to set queue for in-band control.
Until now, in-band control traffic has always gone to the default queue,
typically queue 0. It makes sense to be able to assing control traffic to
its own queue, so this commit enables that.
Ben Pfaff [Thu, 28 Oct 2010 20:26:31 +0000 (13:26 -0700)]
classifier: Drop CLS_INC_* enumerations and related 'include' parameters.
This type and these parameters were useful when ofproto had the need to
separately traverse exact-match rules looking for subrules, but it no
longer does that because subrules (now called "facets") are not kept in
the classifier any longer. All the callers are now passing CLS_INC_ALL
anyhow, so we might as well delete this feature and simplify the code.
Ben Pfaff [Fri, 29 Oct 2010 23:33:08 +0000 (16:33 -0700)]
ofproto: Fix accounting in facet_revalidate().
When a facet moves from one rule to another, facet_revalidate() would
credit the packet and byte counters for the facet to the new rule (which
hasn't actually had any packets sent with the new actions at this point),
instead of to the old rule (which did potentially get some packets sent
with its old actions). This commit fixes the problem.
Ethan Jackson [Tue, 9 Nov 2010 18:41:47 +0000 (10:41 -0800)]
netdev-linux: HFSC in linux
This commit implements the Hierarchical Fair Service Curve queuing
discipline in linux. HFSC performs better at high bandwidth and
implements min-rate proportional sharing of excess bandwidth. Only
a simplified configuration interface is exposed to the user. This
can be expand to allow more tweaking in the future.
Ben Pfaff [Fri, 29 Oct 2010 18:38:39 +0000 (11:38 -0700)]
ofproto: Disable timeouts for flows added by ofproto_add_flow().
None of the existing callers of ofproto_add_flow() want an idle timeout,
but ofproto_add_flow() was giving them a 5-second timeout anyway. I don't
know how this worked properly--in-band will patiently add the flow back
every few seconds anyway, but the bridge doesn't do that.
Also add an explanatory comment to ofproto_add_flow().
Ben Pfaff [Thu, 11 Nov 2010 19:04:10 +0000 (11:04 -0800)]
ofproto: Fix used time of revalidated subrules.
Despite the comment, the 'used' member of a rule is always set to at least
the value of 'created', except in one case in revalidation. That case
appears to be just a mistake, so this commit changes it and corrects the
comment.
Ben Pfaff [Tue, 9 Nov 2010 23:58:36 +0000 (15:58 -0800)]
socket-util: Work around Unix domain socket path name limits on Linux.
Many Open vSwitch tests fail on Debian's automatic build machines because
the builds occur in deeply nested directories with long names. OVS tries
to bind and connect to Unix domain sockets using absolute path names, which
in combination with long directory names means that the socket's name
exceeds the limit for Unix domain socket names (108 bytes on Linux).
This commit works around the problem on Linux by indirecting through
/proc/self/fd/<dirfd>/<basename> when names exceed the maximum that can be
used directly.
Reported-by: Hector Oron <hector.oron@gmail.com> Reported-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc> Reported-by: Roger Leigh <rleigh@codelibre.net>
Debian bug #602891.
Debian bug #602911.
Ben Pfaff [Tue, 26 Oct 2010 16:41:24 +0000 (09:41 -0700)]
ofp-util: Make validate_actions() take a struct flow *.
The upcoming support for actions on registers will require the flow to
validate actions, so this commit adds the parameter in advance. It is
not yet used.
Ben Pfaff [Thu, 28 Oct 2010 04:20:50 +0000 (21:20 -0700)]
ofproto: Refactor handle_packet_out().
An upcoming commit will require the flow to be passed in as part of
OpenFlow action validation, but handle_packet_out() has until now been
structured to make this difficult. This commit refactors it to better
suit this purpose.
Ben Pfaff [Tue, 9 Nov 2010 21:48:57 +0000 (13:48 -0800)]
datapath: Use "struct rtnl_link_stats64" instead of "struct odp_vport_stats".
Linux 2.6.35 added struct rtnl_link_stats64, which as a set of 64-bit
network device counters is what the OVS datapath needs. We might as well
use it instead of our own.
This commit moves the if_link.h compat header from datapath/ into the
top-level include/ directory so that it is visible both to kernel and
userspace code.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Tue, 19 Oct 2010 23:01:49 +0000 (16:01 -0700)]
nicira-ext: Shuffle comments around.
It seemed to me that the descriptions of what actions do should be just
above the action structures, where the reader can see the arguments,
instead of just above the enumeration name, so I rearranged the code
this way.
A few actions didn't have their own structures, so to do this I had to give
them some.
Ben Pfaff [Mon, 8 Nov 2010 18:43:19 +0000 (10:43 -0800)]
ofproto: Refactor handle_flow_mod().
This breaks this OpenFlow handler into two parts, one responsible
for parsing and constructing OpenFlow messages and one that works
with the flow table. The latter will be reused in a later commit
that implements the Nicira Extended Match flexible flow match
extension.
This breaks this OpenFlow handler into two parts, one responsible
for parsing and constructing OpenFlow messages and one that works
with the flow table. The latter will be reused in a later commit
that implements the Nicira Extended Match flexible flow match
extension.
Ben Pfaff [Fri, 5 Nov 2010 18:59:05 +0000 (11:59 -0700)]
ofproto: Make flow format specific to an OpenFlow connection.
Until now the flow format has been a global property of an ofproto.
This is no longer appropriate, because the Nicira Extended Match
flow format that upcoming commits will add is not compatible with
OpenFlow 1.0-only controllers. This commit changes the flow format
to a property of an individual OpenFlow connection.
Ben Pfaff [Fri, 5 Nov 2010 17:22:18 +0000 (10:22 -0700)]
Add "Manager" and "manager_options" to allow options for OVSDB connections.
I'm retaining the "managers" column in the Open_vSwitch table for now, but
I hope that applications transition to using "manager_options" eventually
so that we could drop it.
CC: Andrew Lambeth <wal@nicira.com> CC: Jeremy Stribling <strib@nicira.com>
Ben Pfaff [Wed, 3 Nov 2010 17:01:38 +0000 (10:01 -0700)]
docs: Implement our own dot->pic translator.
Recent versions of Graphviz no longer support output to PIC format, so this
commit adds our own internal translator from dot's "plain" output format
to PIC format. The "plain" format works best with slightly different
"dot" input (advised by the Graphviz manual description of the "plain"
format) so this commit also adjusts ovsdb-dot's output.
Ben Pfaff [Wed, 3 Nov 2010 16:57:16 +0000 (09:57 -0700)]
configure: Fix Graphviz test and input generation.
This check for Graphviz never actually worked properly because Autoconf
swallows up the [] around [gG], so Graphviz was always detected as missing.
This commit fixes the problem by doubling up to [[gG]].
Because Graphviz was never used, I never noticed that ovsdb-dot had not
been revised to use the latest Python interface to OVSDB, so this commit
fixes up those problems too.
Ben Pfaff [Mon, 18 Oct 2010 23:15:50 +0000 (16:15 -0700)]
ofproto: Simplify send_flow_removed().
I have no evidence that the optimization in this function is valuable.
An upcoming commit will introduce a new form of flow expiration message
that is sent to controllers that ask for it, while the standard OpenFlow
1.0 message is sent to other controllers. Since retaining this
optimization with that logic would complicate the function, this commit
drops it.