sliver-openvswitch.git
12 years agoMerge 'next' into 'master'.
Ben Pfaff [Wed, 18 May 2011 21:01:13 +0000 (14:01 -0700)]
Merge 'next' into 'master'.

I know already that this breaks the statsfixes that were implemented by the
following commits:

827ab71c97f "ofproto: Datapath statistics accounted twice."
6f1435fc8f7 "ofproto: Resubmit statistics improperly account during..."

These were already broken in a previous merge.  I will work on a fix.

12 years agoofproto: Datapath statistics accounted twice.
Ethan Jackson [Wed, 18 May 2011 18:52:08 +0000 (11:52 -0700)]
ofproto: Datapath statistics accounted twice.

Due to an error introduced in  Commit 6f1435f "ofproto: Resubmit
statistics improperly account during failover." Flow statistics
could be double accounted when removed from the datapath.

Reported-by: KK Yap <yapkke@stanford.edu>
12 years agodatapath: Hash and compare only the part of sw_flow_key actually used.
Andrew Evans [Wed, 18 May 2011 18:30:07 +0000 (11:30 -0700)]
datapath: Hash and compare only the part of sw_flow_key actually used.

Currently the whole flow key struct is hashed on every packet received from the
network or userspace. The whole struct is also compared byte-for-byte when
doing flow table lookups. This consumes a fair percentage of CPU time, and most
of the time part of the structure is unused (e.g. the IPv6 fields when handling
IPv4 traffic; the IPv4 fields when handling Ethernet frames).

This commit reorders the fields in the flow key struct to put the least
commonly used elements at the end and changes the hash and comparison functions
to look only at the part that contains data.

Signed-off-by: Andrew Evans <aevans@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
12 years agoAdd tar.gz output option in ovs-bugtool
Shih-Hao Li [Wed, 18 May 2011 18:22:41 +0000 (11:22 -0700)]
Add tar.gz output option in ovs-bugtool

12 years agobridge: Fill in ofport column of Interface records.
Ben Pfaff [Wed, 18 May 2011 16:16:00 +0000 (09:16 -0700)]
bridge: Fill in ofport column of Interface records.

This was lost in the transition to the "next" branch.

12 years agoofproto-dpif: Revalidate flows in some previously missed cases.
Ben Pfaff [Tue, 17 May 2011 22:49:26 +0000 (15:49 -0700)]
ofproto-dpif: Revalidate flows in some previously missed cases.

Reported-by: Justin Pettit <jpettit@nicira.com>
Bug #5655.

12 years agoofproto-dpif: Avoid unnecessary ODP-to-OFP port conversion.
Ben Pfaff [Tue, 17 May 2011 22:46:32 +0000 (15:46 -0700)]
ofproto-dpif: Avoid unnecessary ODP-to-OFP port conversion.

12 years agobond: Bonds never sleep if carrier changes.
Ethan Jackson [Tue, 17 May 2011 20:47:23 +0000 (13:47 -0700)]
bond: Bonds never sleep if carrier changes.

The bonding code neglected to call netdev_monitor_poll() on its
monitor during bond_run().  Thus carrier changes would be
permanently queued in the monitor, preventing it from ever allowing
poll_loop to sleep.

12 years agoofproto-dpif: Fix null pointer dereference in is_admissible().
Ben Pfaff [Tue, 17 May 2011 20:41:32 +0000 (13:41 -0700)]
ofproto-dpif: Fix null pointer dereference in is_admissible().

If in_port is NULL then we must not dereference it.

12 years agoxenserver: remove unneeded macro definitions
Sajjad Lateef [Tue, 17 May 2011 02:16:51 +0000 (19:16 -0700)]
xenserver: remove unneeded macro definitions

The macro binsuffix is no longer needed. It has been replaced with
kernel_flavor that is passed on the command line.

The macro kernel_version is passed on the command line. Redefining
the macro inside the spec file leads to build failures.

Signed-off-by: Sajjad Lateef <slateef@nicira.com>
12 years agoxenserver: remove unneeded macro definitions
Sajjad Lateef [Tue, 17 May 2011 02:15:59 +0000 (19:15 -0700)]
xenserver: remove unneeded macro definitions

The macro binsuffix is no longer needed. It has been replaced with
kernel_flavor that is passed on the command line.

The macro kernel_version is passed on the command line. Redefining
the macro inside the spec file leads to build failures.

Signed-off-by: Sajjad Lateef <slateef@nicira.com>
12 years agoxenserver: modify module spec file
Sajjad Lateef [Tue, 17 May 2011 00:29:04 +0000 (17:29 -0700)]
xenserver: modify module spec file

Based on feedback from Citrix about building for multiple kernels,
the spec file has been modified to take three arguments on the
command line: kernel_name, kernel_version and kernel_flavor.

The kernel_flavor is either xen or kdump. The kernel_name is the
Name value embedded in the kernel rpm and the kernel_version
is Version-Release values embedded in the kernel rpm. The
xen_version is calculated.

The INSTALL document has been updated to reflect these changes.

Signed-off-by: Sajjad Lateef <slateef@nicira.com>
(cherry picked from commit b11e4aa7e92854612a4d139b8a620d036a5d41a2)

12 years agoxenserver: modify module spec file
Sajjad Lateef [Tue, 17 May 2011 00:24:20 +0000 (17:24 -0700)]
xenserver: modify module spec file

Based on feedback from Citrix about building for multiple kernels,
the spec file has been modified to take three arguments on the
command line: kernel_name, kernel_version and kernel_flavor.

The kernel_flavor is either xen or kdump. The kernel_name is the
Name value embedded in the kernel rpm and the kernel_version
is Version-Release values embedded in the kernel rpm. The
xen_version is calculated.

The INSTALL document has been updated to reflect these changes.

Signed-off-by: Sajjad Lateef <slateef@nicira.com>
(cherry picked from commit b11e4aa7e92854612a4d139b8a620d036a5d41a2)

12 years agoconfigure: Run sparse automatically if C=1 specified on "make" command.
Ben Pfaff [Fri, 6 May 2011 20:00:49 +0000 (13:00 -0700)]
configure: Run sparse automatically if C=1 specified on "make" command.

The C=1 convention matches the kernel's convention, so running "make C=1"
will now get sparse results for both userspace and kernel compiles.

12 years agoMake the source tree sparse clean.
Ben Pfaff [Fri, 6 May 2011 19:59:51 +0000 (12:59 -0700)]
Make the source tree sparse clean.

With this commit, the tree compiles clean with sparse commit 87f4a7fda3d
"Teach 'already_tokenized()' to use the stream name hash table" with patch
"evaluate: Allow sizeof(_Bool) to succeed" available at
http://permalink.gmane.org/gmane.comp.parsers.sparse/2461 applied, as long
as the "include/sparse" directory is included for use by sparse (only),
e.g.:
     make CC="CHECK='sparse -I../include/sparse' cgcc"

12 years agotest-sha1: Suppress sparse warning.
Ben Pfaff [Fri, 6 May 2011 19:39:27 +0000 (12:39 -0700)]
test-sha1: Suppress sparse warning.

Otherwise sparse warns: warning: memset with byte count of 1000000

12 years agoFix up usage of flow_wildcards_t.
Ben Pfaff [Fri, 6 May 2011 19:38:20 +0000 (12:38 -0700)]
Fix up usage of flow_wildcards_t.

The flow_wildcards_t type is defined as a distinct type from sparse's
perspective (with __attribute__((bitwise))) so that we don't accidentally
mix it with only-partially-compatible OFPFW_* flags.  But we were weren't
using it quite right in a few plces.  This fixes it up.

12 years agocsum: Annotate byte order to match actual usage.
Ben Pfaff [Fri, 6 May 2011 19:34:46 +0000 (12:34 -0700)]
csum: Annotate byte order to match actual usage.

The IP checksum algorithm yields identical results regardless of whether
arithmetic little-endian or big-endian, but in practice OVS only passes in
big-endian data, so it seems reasonable to annotate these functions that
way.

12 years agoutil: Suppress build assertions when building with sparse.
Ben Pfaff [Fri, 6 May 2011 18:43:04 +0000 (11:43 -0700)]
util: Suppress build assertions when building with sparse.

sparse simply doesn't like our build assertions on packed structures.
It seems that its ideas about struct packing are different from GCC's:

../lib/cfm.h:50:1: error: invalid bitfield width, -1.
../lib/packets.h:206:1: error: invalid bitfield width, -1.
../lib/packets.h:213:1: error: invalid bitfield width, -1.
../lib/packets.h:367:1: error: invalid bitfield width, -1.

sparse isn't generating code so we don't really care how it lays out
structures.  We might as well just skip the assertions, as done here.

12 years agoSuppress sparse warnings for global variables initialized in headers.
Ben Pfaff [Fri, 6 May 2011 18:38:19 +0000 (11:38 -0700)]
Suppress sparse warnings for global variables initialized in headers.

sparse warns if a non-static variable with external linkage has an
initializer at first declaration, because it suspects that it should be
static instead.  Generally it's correct, but not in these cases, so add
a redundant declaration to suppress the warning.

The suppress warnings look like:
../ofproto/connmgr.c:40:1: warning: symbol 'VLM_connmgr' was not declared. Should it be static?
../ofproto/collectors.c:31:1: warning: symbol 'vlog_module_ptr_collectors' was not declared. Should it be static?
../ofproto/connmgr.c:43:1: warning: symbol 'counter_ofconn_stuck' was not declared. Should it be static?

12 years agocompiler: Suppress sparse complaints about function attributes.
Ben Pfaff [Fri, 6 May 2011 18:34:59 +0000 (11:34 -0700)]
compiler: Suppress sparse complaints about function attributes.

GCC allows __attribute__s to be included in function prototypes and
then omitted later on the function definition, but sparse complains about
this.  Furthermore, sparse doesn't like the placement of the __attribute__s
that we tend to use in OVS.

I don't see any value in "fixing" these to suit sparse so it seems better
to just omit them.

12 years agoFix incorrect byte order annotations.
Ben Pfaff [Fri, 6 May 2011 18:27:05 +0000 (11:27 -0700)]
Fix incorrect byte order annotations.

These are not actual bugs, just deceptive use of the wrong function or
type.

Found by sparse.

12 years agoopenflow: Change types from uint<N>_t to ovs_be<N>.
Ben Pfaff [Wed, 4 May 2011 21:03:43 +0000 (14:03 -0700)]
openflow: Change types from uint<N>_t to ovs_be<N>.

I've been reluctant in the past to make wholesale changes to openflow.h
because it would be a divergence from upstream that would make comparisons
and merges more difficult.  But, in practice, no one does such comparisons
and no merges happen (because OpenFlow 1.0 is not changing).  I'd still be
inclined to resist, except that in this series I'm adding actual checking
for byte order conventions (as opposed to just documentation).

12 years agoConvert remaining network-byte-order "uint<N>_t"s into "ovs_be<N>"s.
Ben Pfaff [Tue, 29 Mar 2011 21:42:20 +0000 (14:42 -0700)]
Convert remaining network-byte-order "uint<N>_t"s into "ovs_be<N>"s.

I looked at almost every uint<N>_t in the tree to determine whether it was
really in network byte order, and converted the ones that were.

The only remaining ones, modulo my mistakes, are in openflow.h.  I'm not
sure whether we should convert those, because there might be some value
in remaining close to upstream for this header.

12 years agoofproto: Maintain ofp_phy_port for each ofport in network byte order.
Ben Pfaff [Tue, 29 Mar 2011 21:11:39 +0000 (14:11 -0700)]
ofproto: Maintain ofp_phy_port for each ofport in network byte order.

It's rather confusing to have an instance of a whole structure in an
unexpected byte order.  This commit gets rid of that oddity.

12 years agoConsistently write null pointer constants as NULL instead of 0.
Ben Pfaff [Wed, 4 May 2011 20:49:42 +0000 (13:49 -0700)]
Consistently write null pointer constants as NULL instead of 0.

Found with sparse.

12 years agoofproto: Drop duplicate "const" in parameter declaration.
Ben Pfaff [Wed, 4 May 2011 21:45:31 +0000 (14:45 -0700)]
ofproto: Drop duplicate "const" in parameter declaration.

Found by sparse.

12 years agoAdd missing "static" keywords.
Ben Pfaff [Wed, 4 May 2011 20:58:10 +0000 (13:58 -0700)]
Add missing "static" keywords.

Found by sparse.

12 years agoRemove unnecessary #include directives.
Ben Pfaff [Fri, 6 May 2011 18:40:26 +0000 (11:40 -0700)]
Remove unnecessary #include directives.

12 years agoofproto: Fix ofproto_send_packet() treatment of vlan_tci parameter.
Ben Pfaff [Wed, 4 May 2011 22:44:51 +0000 (15:44 -0700)]
ofproto: Fix ofproto_send_packet() treatment of vlan_tci parameter.

ofproto_send_packet() seems very confused about whether vlan_tci is in
host or network byte order.  But the callers always pass 0 anyway, so we
might as well remove it.

12 years agostream-ssl: Fix call to accept().
Ben Pfaff [Wed, 4 May 2011 22:46:27 +0000 (15:46 -0700)]
stream-ssl: Fix call to accept().

GCC and glibc conspire to allow struct sockaddr_in * to be passed in
place of struct sockaddr *, but that's non-standard and we're better
off not taking advantage of it.

Found by sparse.

12 years agonetdev-linux: Initialize rx_compressed, tx_compressed when converting.
Ben Pfaff [Mon, 16 May 2011 20:22:05 +0000 (13:22 -0700)]
netdev-linux: Initialize rx_compressed, tx_compressed when converting.

rtnl_link_stats64 has rx_compressed and tx_compressed members that
struct netdev_stats lacks, so we need to initialize them to zero when
converting.

Found by valgrind.

12 years agobridge: Avoid double-free of bond fake ifaces.
Ben Pfaff [Mon, 16 May 2011 20:03:49 +0000 (13:03 -0700)]
bridge: Avoid double-free of bond fake ifaces.

Found by valgrind.

13 years agoofproto-dpif: Fix null pointer dereference in get_ofp_port().
Ben Pfaff [Fri, 13 May 2011 23:50:20 +0000 (16:50 -0700)]
ofproto-dpif: Fix null pointer dereference in get_ofp_port().

13 years agobridge: Fix uninitialized bond_stable_ids in port_configure_bond().
Ben Pfaff [Fri, 13 May 2011 23:47:01 +0000 (16:47 -0700)]
bridge: Fix uninitialized bond_stable_ids in port_configure_bond().

The recent merge of "master" added a new bond_stable_ids member to
struct ofproto_bundle_settings, but neglected to initialize it.  This fixes
the problem.

Found and verified using valgrind.

13 years agotests: Check ovs-openflowd log output instead of ignoring it.
Ben Pfaff [Mon, 2 May 2011 19:52:56 +0000 (12:52 -0700)]
tests: Check ovs-openflowd log output instead of ignoring it.

ovs-openflowd outputs a number of log messages that we don't want to
suppress.  We do want to know if it logs anything that we don't expect.
So this commit starts checking the log output, discarding any normal,
expected messages.

Reviewed-by: Simon Horman <horms@verge.net.au>
13 years agonetdev-linux: Open AF_PACKET socket only when it is needed.
Ben Pfaff [Mon, 2 May 2011 16:53:31 +0000 (09:53 -0700)]
netdev-linux: Open AF_PACKET socket only when it is needed.

Only a privileged process can open a raw AF_PACKET socket, so netdev-linux
will fail to initialize if run as non-root and you get a cascade of error
messages, like this:

netdev_linux|ERR|failed to create packet socket: Operation not permitted
netdev|ERR|failed to initialize system network device class: Operation not permitted
netdev|ERR|failed to initialize internal network device class: Operation not permitted
netdev|ERR|failed to initialize tap network device class: Operation not permitted

But in fact the AF_PACKET socket is not needed for most operations (only
for sending packets) and it is never needed for testing with the "dummy"
datapath and network device, so we can avoid logging all of these errors
by opening the packet socket only on demand, as this commit does.

Reviewed-by: Simon Horman <horms@verge.net.au>
13 years agonetdev-linux: Only call set_nonblocking() if socket creation succeeds.
Ben Pfaff [Fri, 29 Apr 2011 22:53:36 +0000 (15:53 -0700)]
netdev-linux: Only call set_nonblocking() if socket creation succeeds.

Reviewed-by: Simon Horman <horms@verge.net.au>
13 years agoofp-util: Revise OpenFlow 1.0 ofp_match normalization.
Ben Pfaff [Mon, 2 May 2011 18:04:33 +0000 (11:04 -0700)]
ofp-util: Revise OpenFlow 1.0 ofp_match normalization.

For a long time, Open vSwitch has "normalized" OpenFlow 1.0 flows in a
funny way: it tries to change fields that are wildcarded into fields
that are exact-match.  For example, the normalize_match() function
knows that if dl_type is wildcarded, then all of the L3 and L4 fields
will always be extracted as 0, so it sets those fields to exact-match
and their values to 0.

The reason for this was originally that exact-match flows were much
cheaper for Open vSwitch to implement, because they could be implemented
with a hash table, whereas other kinds of flows had to be implemented
with an expensive linear search.  But these days Open vSwitch has a
smarter classifier in which wildcarded flows have minimal cost.  Also,
it is no longer possible for OpenFlow 1.0 to specify truly exact-match
flows, because Open vSwitch supports fields for which OpenFlow 1.0
cannot specify values and therefore will always be wildcarded.

Now, it no longer makes sense to do this transformation, so this commit
removes it.  Presumably, this will be less surprising for users.

Reviewed-by: Simon Horman <horms@verge.net.au>
13 years agoofp-util: Simplify OpenFlow 1.0 ofp_match normalization.
Ben Pfaff [Mon, 2 May 2011 18:46:17 +0000 (11:46 -0700)]
ofp-util: Simplify OpenFlow 1.0 ofp_match normalization.

The normalize_match() function does more work than really needed.  It goes
to some trouble to zero out fields that are wildcarded.  This is not
necessary, because cls_rule_from_match() will take care of it later.

Also make normalize_match() private to ofp-util.c, since it has no other
users now and I don't expect more later.

Reviewed-by: Simon Horman <horms@verge.net.au>
13 years agoofp-util: Don't warn for different forms of nw_{src,dst} wildcards.
Ben Pfaff [Fri, 29 Apr 2011 22:47:26 +0000 (15:47 -0700)]
ofp-util: Don't warn for different forms of nw_{src,dst} wildcards.

OpenFlow 1.0 uses a 6-bit field to express the number of wildcarded bits
in the nw_src and nw_dst field.  Any value 32 or greater in these fields
(binary 1xxxxx) means that all of the bits are wildcarded.  That means
that there are 32 different ways to express a wildcarded nw_src or nw_dst.
At least two of those seem sensible (100000 and 111111) so we shouldn't
warn about one of them.

This fixes the problem by ORing with 100000 instead of 111111, so that any
already-correct wildcarded mask won't be affected.

This fix allows us to update some tests.

Reviewed-by: Simon Horman <horms@verge.net.au>
13 years agolockfile: Don't warn if successful lock takes a little time.
Ben Pfaff [Fri, 13 May 2011 21:43:44 +0000 (14:43 -0700)]
lockfile: Don't warn if successful lock takes a little time.

This code issues a warning if obtaining a lock takes even 1 millisecond.
That's far too aggressive.  There's no need to warn if we have to wait
a few milliseconds.  This function already warns elsewhere if locking takes
more than 1 second, which is much more reasonable.

This change allows us to test ovsdb-server stderr output more carefully.
Before now, the tests had to ignore what ovsdb-server writes to stderr
because sometimes it would log a warning that locking took 1 ms (or so).

Reviewed-by: Simon Horman <horms@verge.net.au>
13 years agotests: Check test output more carefully.
Ben Pfaff [Fri, 29 Apr 2011 17:44:58 +0000 (10:44 -0700)]
tests: Check test output more carefully.

It's better to check output than to ignore it, because ignoring
output can fail to detect real bugs later if the output changes.

Reviewed-by: Simon Horman <horms@verge.net.au>
13 years agopoll-loop: Make wakeup logging more portable and easier to understand.
Ben Pfaff [Fri, 13 May 2011 20:06:49 +0000 (13:06 -0700)]
poll-loop: Make wakeup logging more portable and easier to understand.

Until now, when the poll_loop module's log level was turned up to "debug",
it would log a backtrace of the call stack for the event that caused poll()
to wake up in poll_block().  This was pretty useful from time to time to
find out why ovs-vswitchd was using more CPU than expected, because we
could find out what was causing it to wake up.

But there were some issues.  One is simply that the backtrace was printed
as a series of hexadecimal numbers, so GDB or another debugger was needed
to translate it into human-readable format.  Compiler optimizations meant
that even the human-readable backtrace wasn't, in my experience, as helpful
as it could have been.  And, of course, one needed to have the binary to
interpret the backtrace.  When the backtrace couldn't be interpreted or
wasn't meaningful, there was essentially nothing to fall back on.

This commit changes the way that "debug" logging for poll_block() wakeups
works.  Instead of logging a backtrace, it logs the source code file name
and line number of the call to a poll_loop function, using __FILE__ and
__LINE__.  This is by itself much more meaningful than a sequence of
hexadecimal numbers, since no additional interpretation is necessary.  It
can be useful even if the Open vSwitch version is only approximately known.

In addition to the file and line, this commit adds, for wakeups caused by
file descriptors, information about the file descriptor itself: what kind
of file it is (regular file, directory, socket, etc.), the name of the file
(on Linux only), and the local and remote endpoints for socket file
descriptors.

Here are a few examples of the new output format:

932-ms timeout at ../ofproto/in-band.c:507
[POLLIN] on fd 20 (192.168.0.20:35388<->192.168.0.3:6633) at ../lib/stream-fd.c:149
[POLLIN] on fd 7 (FIFO pipe:[48049]) at ../lib/fatal-signal.c:168

13 years agobacktrace: Make backtrace_capture() work on more systems.
Ben Pfaff [Fri, 13 May 2011 18:55:22 +0000 (11:55 -0700)]
backtrace: Make backtrace_capture() work on more systems.

The backtrace_capture() implementation only worked properly with GNU C on
systems that have a simple stack frame with a frame pointer.  Notably,
the x86-64 ABI by default has no frame pointer, so this failed on x86-64.

However, glibc has a function named backtrace() that does what we want.
This commit tests for this function and uses it when it is present, fixing
x86-64 backtraces.

13 years agocfm: Clarify cfm_create() documentation.
Ethan Jackson [Fri, 13 May 2011 19:52:00 +0000 (12:52 -0700)]
cfm: Clarify cfm_create() documentation.

Reported-by: Ben Pfaff <blp@nicira.com>
13 years agocfm: Always log on CCM reception.
Ethan Jackson [Thu, 12 May 2011 20:48:23 +0000 (13:48 -0700)]
cfm: Always log on CCM reception.

This commit causes the CFM library to log at debug level when valid
CCMs are received.

13 years agocfm: Replace recv_time with a flag.
Ethan Jackson [Thu, 12 May 2011 01:13:35 +0000 (18:13 -0700)]
cfm: Replace recv_time with a flag.

This makes the code more obviously correct in my opinion.

This patch also removes timer_enabled_at() along with its only
user.

13 years agocfm: No longer keep track of bad CCMs.
Ethan Jackson [Thu, 12 May 2011 00:50:16 +0000 (17:50 -0700)]
cfm: No longer keep track of bad CCMs.

According to the 802.1ag specification, reception of a CCM from an
unexpected source should trigger a fault. This patch causes the CFM
module to simply warn instead.  There are several reasons for this
change outlined below.

  - Faults can cause controllers to make potentially expensive
    changes to the network topology.
  - Faults can be maliciously triggered by crafting invalid CCMs.
  - With this patch, cfm->fault and rmp->fault are only updated in
    cfm_run() making the code easier to debug and reason about.

13 years agocfm: No longer trigger fault upon unexpected ccm_interval.
Ethan Jackson [Thu, 12 May 2011 00:55:41 +0000 (17:55 -0700)]
cfm: No longer trigger fault upon unexpected ccm_interval.

According to the 802.1ag specification, when a CCM is received
which advertises a misconfigured transmission interval, a fault
should be triggered.  This patch goes against the spec by simply
warning when this happens.  This is done for several reasons.

  - Faults can cause controllers to make potentially expensive
    changes in the network topology.
  - Faults can be maliciously triggered by crafting invalid CCMs.
  - Reducing the number of places in the code where rmp->fault and
    cfm->fault are changed makes the code easier to debug and
    reason about.

13 years agoofproto: Call port_modified before closing old netdev in update_port().
Ben Pfaff [Fri, 13 May 2011 16:14:18 +0000 (09:14 -0700)]
ofproto: Call port_modified before closing old netdev in update_port().

Fixes a segmentation fault due to update_port() -> port_modified() ->
bond_slave_set_netdev() -> netdev_monitor_remove() -> netdev_get_name().

Reported-by: Michael MAO <mmao@nicira.com>
13 years agoFix bugs lingering from merge mistakes.
Ben Pfaff [Fri, 13 May 2011 00:24:34 +0000 (17:24 -0700)]
Fix bugs lingering from merge mistakes.

I should have caught these when I did the merge from "master" earlier
today, but I forgot to run the testsuite.

13 years agobridge: Keep default Ethernet address stable between runs.
Ben Pfaff [Tue, 10 May 2011 18:38:24 +0000 (11:38 -0700)]
bridge: Keep default Ethernet address stable between runs.

In some circumstances the bridge can't find a stable physical Ethernet
address to use, so until now it has just picked a random Ethernet address.
In these circumstances, therefore, the bridge Ethernet address would change
from one ovs-vswitchd run to another.  But OVS does have a stable
identifier for a bridge: its UUID.  This commit changes to use that as the
default bridge Ethernet address.

The datapath ID is sometimes derived from the bridge Ethernet address, so
this change also makes the bridge Ethernet address more stable.

CC: Natasha Gude <natasha@nicira.com>
Bug #5594.

13 years agoofproto: Fix duplicate hmap_remove() in ofproto_destroy().
Ben Pfaff [Thu, 12 May 2011 19:21:23 +0000 (12:21 -0700)]
ofproto: Fix duplicate hmap_remove() in ofproto_destroy().

Both ofport_destroy() and its caller ofproto_destroy() were attempting to
remove the ofport's hmap_node from the ofproto's 'ports' hmap, resulting
in a use-after-free error.

Reported-by: Michael MAO <mmao@nicira.com>
13 years agoofproto: Fix typo in comment.
Ben Pfaff [Thu, 12 May 2011 19:08:48 +0000 (12:08 -0700)]
ofproto: Fix typo in comment.

Reported-by: Ethan Jackson <ethan@nicira.com>
13 years agoMerge 'master' into 'next'.
Ben Pfaff [Thu, 12 May 2011 19:05:42 +0000 (12:05 -0700)]
Merge 'master' into 'next'.

13 years agoofproto-dpif: Get rid of effectively unused 'check_special' flag.
Ben Pfaff [Wed, 11 May 2011 18:03:25 +0000 (11:03 -0700)]
ofproto-dpif: Get rid of effectively unused 'check_special' flag.

Nothing ever sets this flag to false any longer, so there's no need to
store it or test its value.

Reported-by: Ethan Jackson <ethan@nicira.com>
13 years agoImplement basic multiple table support.
Ben Pfaff [Thu, 12 May 2011 16:58:01 +0000 (09:58 -0700)]
Implement basic multiple table support.

This implements basic multiple table support in ofproto and supporting
libraries and utilities. The design is the same as the one that has been
on the Open vSwitch "wdp" branch for a long time.  There is no support for
multiple tables in the software switch implementation (ofproto-dpif), only
a set of hooks for other switch implementations to use.

To allow controllers to add flows in a particular table, Open vSwitch adds
an OpenFlow 1.0 extension called NXT_FLOW_MOD_TABLE_ID.

13 years agoofproto: Drop ofproto_rule_lookup().
Ben Pfaff [Tue, 26 Apr 2011 21:25:00 +0000 (14:25 -0700)]
ofproto: Drop ofproto_rule_lookup().

There's no reason not to implement this trivial function in ofproto-dpif,
especially since it makes less sense once multiple table support is
implemented (which table should be searched?).

13 years agoPORTING: Describe usage of ovs_be<N>.
Ben Pfaff [Wed, 27 Apr 2011 19:20:01 +0000 (12:20 -0700)]
PORTING: Describe usage of ovs_be<N>.

13 years agoofproto: Fix number of reported tables in OFPT_FEATURES_REPLY message.
Ben Pfaff [Wed, 27 Apr 2011 18:29:50 +0000 (11:29 -0700)]
ofproto: Fix number of reported tables in OFPT_FEATURES_REPLY message.

This has been wrong for a long time.

13 years agoofproto: Make rule construction and destruction more symmetric.
Ben Pfaff [Wed, 11 May 2011 21:06:48 +0000 (14:06 -0700)]
ofproto: Make rule construction and destruction more symmetric.

Before, ->rule_construct() both created the rule and inserted into the
flow table, but ->rule_destruct() only destroyed the rule.  This makes
->rule_destruct() also remove the rule from the flow table.

13 years agoclassifier: Remove OF1.0 special case from classifier_find_rule_exactly().
Ben Pfaff [Tue, 26 Apr 2011 20:09:24 +0000 (13:09 -0700)]
classifier: Remove OF1.0 special case from classifier_find_rule_exactly().

This special case should never have actually triggered in practice, because
OpenFlow 1.0 cannot set up an exact-match rule as defined by
flow_wildcards_is_exact().  (OpenFlow 1.0 will always, for example,
wildcard all NXM registers.)

OVS implements this OF1.0 special case differently, by changing flow
priority to 65535 in cls_rule_from_match() if the flow is an exact match as
defined by OpenFlow 1.0.

13 years agoofproto: Remove unused coverage counters.
Ben Pfaff [Tue, 26 Apr 2011 19:47:39 +0000 (12:47 -0700)]
ofproto: Remove unused coverage counters.

These were mostly moved into ofproto-dpif.c, but the definitions weren't
deleted along with them.

13 years agoofproto: Update some comments.
Ben Pfaff [Wed, 11 May 2011 19:15:46 +0000 (12:15 -0700)]
ofproto: Update some comments.

13 years agoofproto: Eliminate reference to dpif_upcall from ofproto.
Ben Pfaff [Tue, 26 Apr 2011 19:31:12 +0000 (12:31 -0700)]
ofproto: Eliminate reference to dpif_upcall from ofproto.

The dpif_upcall structure is specific to the ofproto-dpif implementation.
The generic ofproto and connmgr interface have no business using it, so
this commit switches to using ofputil_packet_in instead.

13 years agoRemove unneeded #include directives.
Ben Pfaff [Tue, 26 Apr 2011 18:39:10 +0000 (11:39 -0700)]
Remove unneeded #include directives.

13 years agoofproto: Better document the ofproto_class interface.
Ben Pfaff [Tue, 26 Apr 2011 18:30:46 +0000 (11:30 -0700)]
ofproto: Better document the ofproto_class interface.

Also, make a few minor adjustments to the interface so that it makes a
little more sense.

13 years agoofproto: Break apart into generic and hardware-specific parts.
Ben Pfaff [Wed, 11 May 2011 19:13:10 +0000 (12:13 -0700)]
ofproto: Break apart into generic and hardware-specific parts.

In addition to the changes to ofproto, this commit changes all of the
instances of "struct flow" in the tree so that the "in_port" member is an
OpenFlow port number.  Previously, this member was an OpenFlow port number
in some cases and an ODP port number in other cases.

13 years agoofproto: Complete abstraction by adding enumeration and deletion functions.
Ben Pfaff [Mon, 9 May 2011 16:33:02 +0000 (09:33 -0700)]
ofproto: Complete abstraction by adding enumeration and deletion functions.

This eliminates the final reference from bridge.c directly into the dpif
layer, which will make it easier to change the implementation of ofproto
to support other lower layers.

13 years agoofproto: Improve abstraction by using OpenFlow port numbers in interface.
Ben Pfaff [Mon, 9 May 2011 16:24:39 +0000 (09:24 -0700)]
ofproto: Improve abstraction by using OpenFlow port numbers in interface.

Until now, ofproto has used a mix of datapath and OpenFlow port numbers in
its client interface.  This commit changes it to use OpenFlow port numbers
exclusively, to raise the level of abstraction.

Most of this commit boils down to simple search-and-replace with a few
call to ofp_port_to_odp_port() sprinkled in.  The addition of ofproto_port
is one exception.  An ofproto_port is almost the same as a dpif_port; the
difference is just that its port number is an OpenFlow port number instead
of a datapath port number.

13 years agoofproto: Improve abstraction by adding function ofproto_parse_name().
Ben Pfaff [Mon, 11 Apr 2011 22:08:19 +0000 (15:08 -0700)]
ofproto: Improve abstraction by adding function ofproto_parse_name().

This means that ovs-ofctl and ovs-openflowd don't have to use the dpif
layer at all, making it easier to change the ofproto implementation.

13 years agodpif: Make dp_parse_name() normalize its returned type.
Ben Pfaff [Mon, 11 Apr 2011 22:07:07 +0000 (15:07 -0700)]
dpif: Make dp_parse_name() normalize its returned type.

This means that callers don't have to be concerned with a NULL return value
or unnormalized type.

13 years agodpif: Improve abstraction by making 'run' and 'wait' functions per-dpif.
Ben Pfaff [Fri, 6 May 2011 22:04:29 +0000 (15:04 -0700)]
dpif: Improve abstraction by making 'run' and 'wait' functions per-dpif.

Until now, the dp_run() and dp_wait() functions had to be called at the top
level of the program because they applied to every open dpif.  By replacing
them by functions that take a specific dpif as an argument, we can call
them only from ofproto, which is currently the correct layer to deal with
dpifs.

13 years agobridge: Move packet processing functionality into ofproto.
Ben Pfaff [Wed, 11 May 2011 19:26:06 +0000 (12:26 -0700)]
bridge: Move packet processing functionality into ofproto.

Until now, packet processing in ovs-vswitchd has been split between two
components: ofproto, for basic OpenFlow functionality, and bridge, for
OFPP_NORMAL processing.  This architecture will not work as Open vSwitch
starts to support a wider variety of underlying hardware, because it
imposes a model in which the bridge needs to be able to look at every
exact-match flow within a OpenFlow flow, which most hardware doesn't
support.

Therefore, this commit moves all of the packet processing code in
bridge into ofproto, as preparation for generalizing further.

13 years agodatapath: Pull data into linear area only on demand.
Jesse Gross [Tue, 10 May 2011 18:48:36 +0000 (11:48 -0700)]
datapath: Pull data into linear area only on demand.

We currently always pull 64 bytes of data (if it exists) into the
skb linear data area when parsing flows.  The theory behind this
is that the data should always be there and it's enough to parse
common flows.  However, this causes a number of problems in
different situations.  The first is that it is not enough to handle
IPv6 so we must pull additional data anyways.  However, the main
problem is that GRO typically allocates a new skb and puts just the
headers in there.  For a typical TCP/IPv4 packet there are 54 bytes
of headers, which means that we must possibly reallocate and copy
on every packet.  In addition, GRO creates frag_lists with this
specific geometry in order to allow later segmentation if the packet
is forwarded to a device that does not support frag_lists.  When
we pull additional data it changes the geometry and causes later
problems for the device.  This patch instead incrementally pulls
data, which avoids these problems.

Signed-off-by: Jesse Gross <jesse@nicira.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
13 years agoxenserver: Fix bugs related to using xe-switch-network-backend in spec file.
Justin Pettit [Tue, 10 May 2011 06:30:07 +0000 (23:30 -0700)]
xenserver: Fix bugs related to using xe-switch-network-backend in spec file.

Commit daf2ebb (xenserver: Use xe-switch-network-stack in RPM spec
file.) changed the spec file to use xe-switch-network-backend instead of
directly modifying "/etc/xensource/network.conf".  It incorrectly
assumed that the command was in the search path.  It also didn't take
into account that the command will remove the "openvswitch" service with
chkconfig.  This commit fixes those errors.

Signed-off-by: Justin Pettit <jpettit@nicira.com>
13 years agostream-ssl: Improve messages when configuring SSL if it is unsupported.
Ben Pfaff [Tue, 10 May 2011 16:17:37 +0000 (09:17 -0700)]
stream-ssl: Improve messages when configuring SSL if it is unsupported.

Previously, if --private-key or another option that requires SSL support
was used, but OVS was built without OpenSSL support, then OVS would fail
with an error message that the specified option was not supported.  This
confused users because it made them think that the option had been removed:
    http://openvswitch.org/pipermail/discuss/2011-April/005034.html

This commit improves the error message: OVS will now report that it was
built without SSL support.  This should be make the problem clear to users.

Reported-by: Aaron Rosen <arosen@clemson.edu>
Feature #5325.

13 years agoINSTALL.XenServer: Document Open vSwitch boot process on XenServer.
Ben Pfaff [Tue, 10 May 2011 16:15:44 +0000 (09:15 -0700)]
INSTALL.XenServer: Document Open vSwitch boot process on XenServer.

Inspired by a conversation with David Erickson <derickso@stanford.edu>.

13 years agoovs-vsctl: Issue warning for likely erroneous "get" commands.
Ben Pfaff [Mon, 9 May 2011 17:29:51 +0000 (10:29 -0700)]
ovs-vsctl: Issue warning for likely erroneous "get" commands.

Suggested-by: Reid Price <reid@nicira.com>
Feature #5527.

13 years agobridge: Don't configure QoS without Queues.
Ethan Jackson [Sat, 7 May 2011 00:02:02 +0000 (17:02 -0700)]
bridge: Don't configure QoS without Queues.

It doesn't make sense to create a QoS object without any queues.
Before this patch, OVS would configure the QoS object and as a
result drop all traffic going through the affected interface.  With
this patch, OVS will simply clear QoS configuration on the
interface.

Bug #5583.

13 years agoofproto: Resubmit statistics improperly account during failover.
Ethan Jackson [Mon, 2 May 2011 20:15:59 +0000 (13:15 -0700)]
ofproto: Resubmit statistics improperly account during failover.

In some cases, when a facet's actions change because it resubmits
into a different rule, it will account all packets it as accrued
in the datapath to the new rule.  Due to the algorithm we are
using, it is acceptable for a facet to miscount at most 1 second
worth of packets in this manner.  This patch implements the proper
behavior.

Generally speaking, when a facet is facet_put__() into the
datapath, the kernel returns the old flow's statistics so they may
be accounted for in user space.  These statistics are generally
pushed down to the relevant facet's resubmit children.  Before this
patch, facet_put__() did not compensate for the fact that many of
the statistics in the datapath may have been already pushed.
Thus the entire packet count stored in the datapath would be pushed
to its children instead of simply the packets which have accrued
since the last accounting.  This patch fixes the behavior by
subtracting already accounted for packets from the statistics
returned by the datapath.

13 years agolacp: New "lacp-heartbeat" mode.
Ethan Jackson [Thu, 5 May 2011 23:52:56 +0000 (16:52 -0700)]
lacp: New "lacp-heartbeat" mode.

This commit creates a new heartbeat mode for LACP.  This mode
treats LACP as a protocol simply for monitoring link status.  It
strips out most of the sanity checks built into the protocol.
Addition of this mode makes "lacp-force-aggregatable" and
"lacp-strict" options obsolete so they are removed.

13 years agobond: Create new "bond-stable-id".
Ethan Jackson [Thu, 5 May 2011 23:01:11 +0000 (16:01 -0700)]
bond: Create new "bond-stable-id".

Stable bonding mode needs an ID to guarantee consistent slave
selection decisions across ovs-vswitchd instances.  Before this
patch, we used the lacp-port-id for this purpose.  However, LACP
places restrictions on how lacp-port-ids can be allocated which may
be inconvenient.  This patch creates a special purpose
bond-stable-id other_config setting which allows users to tweak
this value directly.

13 years agobond: Convert stb_id to 32bit parameter.
Ethan Jackson [Thu, 5 May 2011 21:27:38 +0000 (14:27 -0700)]
bond: Convert stb_id to 32bit parameter.

The 16 bits currently in use is artificially restrictive.

13 years agoxenserver: Better document scriplet action in RPM spec file.
Justin Pettit [Wed, 4 May 2011 06:16:46 +0000 (23:16 -0700)]
xenserver: Better document scriplet action in RPM spec file.

13 years agoxenserver: Use xe-switch-network-stack in RPM spec file.
Justin Pettit [Wed, 27 Apr 2011 02:58:19 +0000 (19:58 -0700)]
xenserver: Use xe-switch-network-stack in RPM spec file.

The proper way to switch the networking back-end is to use the
"xe-switch-network-stack" command rather than directly modifying
"/etc/xensource/network.conf".  Use that method in the spec file.

13 years agoofp-util: Fix validation of OFPAT_SET_VLAN_PCP actions.
Ben Pfaff [Wed, 4 May 2011 22:47:27 +0000 (15:47 -0700)]
ofp-util: Fix validation of OFPAT_SET_VLAN_PCP actions.

Found by sparse.

13 years agoDESIGN: Move in-band control design discussion here.
Ben Pfaff [Wed, 4 May 2011 20:46:21 +0000 (13:46 -0700)]
DESIGN: Move in-band control design discussion here.

It seems more likely that interested users and administrators will be able
to find it here.

13 years agoofproto: Update ports immediately upon ofproto_port_add() too.
Ben Pfaff [Wed, 20 Apr 2011 22:22:26 +0000 (15:22 -0700)]
ofproto: Update ports immediately upon ofproto_port_add() too.

I don't see a reason to defer this.

13 years agoofproto: Add a pointer to the owning ofproto to struct ofport.
Ben Pfaff [Wed, 4 May 2011 17:38:27 +0000 (10:38 -0700)]
ofproto: Add a pointer to the owning ofproto to struct ofport.

This streamlines a few function calling interfaces.

13 years agoofproto: Initialize ports immediately upon ofproto creation.
Ben Pfaff [Wed, 20 Apr 2011 22:13:46 +0000 (15:13 -0700)]
ofproto: Initialize ports immediately upon ofproto creation.

I don't see why we should delay initializing the ports to the first call
of ofproto_run1().  We originally did initialize the ports in
ofproto_create(), but back in January 2010 Jesse moved the call into
ofproto_run1() in commit 149f577a "netdev: Fully handle netdev lifecycle
through refcounting."  The commit message doesn't explain why this
particular change was made, so I can only assume that it was important at
the time.  Now, however, everything seems to work fine with initialization
done in the most logical place.

13 years agodpif: Better log unusual errors in dpif_port_query_by_name().
Ben Pfaff [Thu, 7 Apr 2011 21:43:14 +0000 (14:43 -0700)]
dpif: Better log unusual errors in dpif_port_query_by_name().

Logging these unusual errors at a low level means that we can remove a
bit of higher-level code from ofproto.

The ofproto change also changes behavior for these error cases, from doing
nothing to removing the port, but I think that's OK.  I've never noticed
this log message.

13 years agohmapx: New data structure.
Ben Pfaff [Fri, 8 Apr 2011 00:10:48 +0000 (17:10 -0700)]
hmapx: New data structure.

13 years agobond: New function bond_slave_set_netdev().
Ben Pfaff [Wed, 4 May 2011 17:26:58 +0000 (10:26 -0700)]
bond: New function bond_slave_set_netdev().

To be used by an upcoming change.

13 years agoofproto: Add 'name' field to struct ofproto and use hmap instead of shash.
Ben Pfaff [Fri, 8 Apr 2011 19:35:38 +0000 (12:35 -0700)]
ofproto: Add 'name' field to struct ofproto and use hmap instead of shash.

It's slightly inconvenient to call into dpif_name() just to get the name
of an ofproto.  Furthermore, we're already keeping a copy of the ofproto's
name around, in the 'name' field of its shash_node.  It seems easier all
around if we just keep the name right in the struct ofproto and use an
hmap instead of a shash.

13 years agoofproto: Rename ofproto_iface_*() functions to ofproto_port_*().
Ben Pfaff [Tue, 5 Apr 2011 23:34:09 +0000 (16:34 -0700)]
ofproto: Rename ofproto_iface_*() functions to ofproto_port_*().

This makes ofproto use the term "port" consistently for a single
purpose (which is unfortunately different from the term "interface"
used in the OVS database, but at least it is now internally
consistent).

13 years agoofproto: Move private definitions to separate header.
Ben Pfaff [Thu, 14 Apr 2011 20:16:47 +0000 (13:16 -0700)]
ofproto: Move private definitions to separate header.

13 years agobridge: Reorder configuration.
Ben Pfaff [Fri, 8 Apr 2011 20:50:21 +0000 (13:50 -0700)]
bridge: Reorder configuration.

This loses the bridge_run_one() before iface_configure_cfm(), which means
that CFM configuration can now take two reconfigurations in a row.  That's
a regression that we had earlier, which had been fixed previously by commit
392730c42bb "bridge: Run once before configuring CFM".  It will, however,
be fixed again in a later commit.

13 years agodpif: New function dpif_normalize_type().
Ben Pfaff [Tue, 5 Apr 2011 19:52:58 +0000 (12:52 -0700)]
dpif: New function dpif_normalize_type().

This allows dpif types to be compared.