Ben Pfaff [Thu, 7 Jan 2010 23:52:58 +0000 (15:52 -0800)]
Cleanly separate IDL annotations from OVSDB schema information.
Until now, the OVSDB IDL annotations have been glommed together with the
schema information in a single file, and then we've used ovsdb-idlc to
extract the schema from that file. This commit reverses the process:
the schema and the annotations are stored separately and then glommed
together as necessary at build time.
This new arrangement has a few advantages:
- We can now easily have multiple different sets of IDL annotations
for a single OVSDB schema. For example, some users may not need
access to columns that other users do.
- Bugs in ovsdb-idlc cannot screw up the underlying schema (as shown
by a recent commit).
Ben Pfaff [Thu, 7 Jan 2010 23:26:36 +0000 (15:26 -0800)]
vswitch: Fix types of NetFlow columns in db schema.
This bug was masked by the translation done by ovsdb-idlc, which doesn't
fully validate its input. In particular, it ignores fields that it doesn't
know about and drops them when it does translation, so nothing later can
find the problem.
An upcoming commit will ensure that it can't happen again, by dropping the
ovsdb-idlc translation step.
Ben Pfaff [Tue, 26 Jan 2010 17:32:38 +0000 (09:32 -0800)]
ovsdb-idlc: With "doc" command, output tables and columns alphabetically.
Until this commit, tables and columns in output were output in random
order (according to the ordering of Python's internal hash table).
Alphabetical order makes more sense.
The compiler warns about next_cfg being uninitialized but in the cases
where it is unset we will never reach the point where it is used. This
just sets next_cfg to 0 to stop the whining.
Jesse Gross [Wed, 20 Jan 2010 18:50:48 +0000 (13:50 -0500)]
hash: Prevent warnings about strict aliasing rules.
Some versions of GCC complain about violations of strict aliasing
rules due to a cast between different pointer types. This avoids
any aliasing by copying the value first.
Jesse Gross [Mon, 18 Jan 2010 22:26:02 +0000 (17:26 -0500)]
netdev: Compare full arguments instead of hash for reconfigure.
We only reconfigure netdevs if the arguments have changed, which
was previously detected based on a hash. This stores and compares
the full argument list to avoid any chance of missing changes due
to collisions.
Jesse Gross [Mon, 18 Jan 2010 20:05:58 +0000 (15:05 -0500)]
netdev-linux: Use the netdev list of devices instead of cachemap.
We previously maintained a list of open devices inside of the
linux netdev. Since the netdev library now maintains this list,
it is better to use that list instead of our own.
Jesse Gross [Sat, 16 Jan 2010 00:24:53 +0000 (19:24 -0500)]
netdev-linux: Avoid potential issues with unset FD.
Never close the file descriptor if it is 0, since it is never a
valid FD in this context. Also initialize the FD to -1 so that
it is never set to a valid but incorrect value.
Keith Amidon [Mon, 18 Jan 2010 00:55:30 +0000 (16:55 -0800)]
Add ability to set controller for specific bridges
Some applications would prefer that the controller only be enabled for
a subset of bridges. This commit introduces an alternative syntax for
the XAPI vSwitchController key to enable the specification of such a
configuration.
The implementation of the configuration is currently a hack. The
problem is that some bridges (especially internal bridges) are only
created when a VIF is added to them. This means that attempting to
set the controller on boot as we used to do won't work. Ideally we
would should hook the creation of the bridge and set the controller at
that time. However, in XenServer 5.5 I can't find an appropriate
place to hook to do so.
To meet immediate requirements, this commit hacks the startup-based
approach by spawning background processes that watch for the creation
of the bridge and set the controller when it becomes available. We'll
have to replace it with a better and more robust solution ASAP.
Jesse Gross [Sat, 16 Jan 2010 14:45:45 +0000 (09:45 -0500)]
netdev-linux: Properly store netdev_dev pointer for RTNL callbacks.
We were storing a struct netdev_dev_linux ** instead of a
netdev_dev_linux * in the cache map. This prevented the cache
from being invalidated on changes such as link status.
The default burst rate was 10Kb. This increases it to 1000kb, since
we were having problems getting traffic through at 10kb. A better value
probably exists between these two points, but that will require
additional experimentation.
Ben Pfaff [Fri, 15 Jan 2010 23:22:54 +0000 (15:22 -0800)]
idl: Gracefully handle destroying a transaction before receiving its reply.
If ovsdb_idl_txn_destroy() is called to destroy a transaction before its
reply has been received from the database server, then until now we would
drop the connection to the database when the reply actually arrived,
because we would have no record of that transaction ID any longer.
Notably, ovs-vswitchd does this: it "fires and forgets" database
transactions. (Really, it should not do that, but that's a bigger commit.)
This commit fixes the problem by not dropping the database connection in
such a case.
This fixes an observed problem such that sometimes ovs-vsctl took a long
time to complete, which was because ovs-vswitchd was dropping its
connection to the database and backing off.
Ben Pfaff [Fri, 15 Jan 2010 22:06:15 +0000 (14:06 -0800)]
netdev-linux: Don't close(0) when closing an ordinary netdev.
Calling close(0) at random points is bad. It means that the next call to
socket() or open() returns fd 0. Then the next time a netdev gets closed,
that socket or file fd gets closed too, and you end up with weird "Bad
file descriptor" errors.
Found by installing the following as lib/unistd.h in the source tree:
#ifndef UNISTD_H
#define UNISTD_H 1
#include <stdlib.h>
#include_next <unistd.h>
#undef close
#define close(fd) rpl_close(fd)
static inline int rpl_close(int fd)
{
if (!fd) {
abort();
}
return (close)(fd);
}
Ben Pfaff [Fri, 15 Jan 2010 21:09:32 +0000 (13:09 -0800)]
Enable daemon monitoring and automatic restart by default.
Tested on a XenServer by running "kill -SEGV" on the daemon processes and
watching them restart automatically and by running plain "kill" and
watching them terminate normally.
Ben Pfaff [Fri, 15 Jan 2010 23:28:14 +0000 (15:28 -0800)]
fatal-signal: After fork, clear hooks instead of disabling them.
Until now, fatal_signal_fork() has simply disabled all the fatal signal
callback hooks. This worked fine, because a daemon process forked only
once and the parent didn't do much before it exited.
But upcoming commits will introduce a --monitor option, which requires
processes to fork multiple times. Sometimes the parent process will fork,
then run for a while, then fork again. It's not good to disable the
hooks in the child process in such a case, because that prevents e.g.
pidfiles from being removed at the child's exit.
So this commit changes the semantics of fatal_signal_fork() to just
clearing out hooks. After hooks are cleared, new hooks can be added and
will be executed on process termination in the usual way.
This commit also introduces a cancellation callback function so that a
canceled hook can free resources.
Ben Pfaff [Fri, 15 Jan 2010 18:31:06 +0000 (10:31 -0800)]
unixctl: Improve error reporting.
The ovs_error() function is our standard way of reporting startup errors,
so use it. (It also outputs the program name at the beginning of the
message.)
Justin Pettit [Fri, 15 Jan 2010 16:33:03 +0000 (08:33 -0800)]
xenserver: Give ovs-vsctl a bit more time to do its work
Creating bonds sometimes fails due to ovs-vsctl timing out. This commit
increases the time interface-reconfigure gives ovs-vsctl from five to
twenty seconds. We should investigate why it's taking ovs-vsctl so
long, but this helps for now.
Jesse Gross [Tue, 12 Jan 2010 20:58:40 +0000 (15:58 -0500)]
netdev-linux: Cleanup tap netdev.
TAP devices need to be treated slightly differently from other other
devices because they cannot be opened multiple times. Instead we
open them once and share the file descriptor. This means that if
the netdev is opened multiple times one reader can drain the buffers
of another. While this is a deviation from the normal convention,
it does not impact current or planned users.
In addition, this cleans up some confusion between the file
descriptor for tap devices versus other FD's.
Jesse Gross [Tue, 12 Jan 2010 21:01:43 +0000 (16:01 -0500)]
netdev: Fully handle netdev lifecycle through refcounting.
This builds on earlier work that implemented netdev object refcounting.
However, rather than requiring explicit create and destroy calls,
these operations are now performed automatically based on the referenece
count. This is important because in certain situations it is not
possible to know whether a netdev has already been created. A
workaround existed (which looked fairly similar to this paradigm) but
introduced it's own issues. This simplifies and unifies the API.
Justin Pettit [Fri, 15 Jan 2010 05:53:43 +0000 (21:53 -0800)]
ovs-vsctl: Add option to create fake iface when adding a bond
Some systems, such as XenServer, expect that bonds have their own interface.
This commit adds the ability to do that with the "--fake-iface" option
in ovs-vsctl's add-bond command. It also has XenServer's
interface-reconfigure use it.
Jesse Gross [Fri, 15 Jan 2010 01:58:20 +0000 (20:58 -0500)]
gre: Workarounds for large packets over GRE
The first change is to not propagate the IP DF bit from the inner
packet to the outer packet. Large TCP packets can get segmented
first which will set the DF bit. However these segmented packets
might still be too large after the GRE header is added, requiring
fragmentation.
The second change is to raise the MTU of the GRE tunnel device.
This prevents packets from being dropped in the datapath before
they can be fragmented. Since the datapath is layer 2 it does not
do any fragmentation and drops any packets that are too large.
Both of these are temporary workarounds that need to be addressed
more carefully in the future.
Justin Pettit [Fri, 15 Jan 2010 02:06:33 +0000 (18:06 -0800)]
ovs-vsctl: Don't mask timeout argument
ovs-vsctl supports the "--timeout" option, which specifies the amount
of time that the operation is allowed to take before a SIGALRM is
raised. The code that parsed options had a local "timeout" that masked
the global one that was supposed to be set.
Jesse Gross [Wed, 13 Jan 2010 22:43:09 +0000 (17:43 -0500)]
bonding: Use device names instead of Xen OpaqueRefs
When configuring the slaves of a bond, interface-reconfigure was
directly writing the Xen OpaqueRef as the name instead of first
converting it into a standard device name.
Justin Pettit [Thu, 14 Jan 2010 20:20:09 +0000 (12:20 -0800)]
ovs-brcompatd: Don't consume CPU if can't connect to config db
When ovs-brcompatd can't connect to the database, the "ovs" variable
is never set. The function "brc_recv_update" takes care of draining
brcompat kernel module's netlink messages. When the netlink message
comes in to modify the bridge, that function never gets called, so a
netlink message always appears to be ready and we consume 100% CPU
looping.
With this commit, we log a warning and drop the request on the floor.
Ben Pfaff [Wed, 13 Jan 2010 17:55:52 +0000 (09:55 -0800)]
ovsdb-client: Fix regression introduced with changes to daemonize().
daemonize() now closes the standard file descriptors, but ovsdb-client's
"monitor" command uses stdout even after daemonizing. This caused
tests that used "ovsdb-client --detach monitor" to fail without printing
their complete output. This commit fixes the problem.
Ben Pfaff [Tue, 12 Jan 2010 22:10:11 +0000 (14:10 -0800)]
daemon: Close standard file descriptors when daemonizing.
Before SSH terminates, it waits for the PTYs that it creates for use as
stdin, stdout, and stderr to be closed. When any of the Open vSwitch
daemons were started in the background over an SSH session, they held
those file descriptors open and thus the SSH session hung. This commit
fixes the problem by closing those file descriptors, allowing SSH to
terminate.
Justin Pettit [Tue, 12 Jan 2010 02:36:56 +0000 (18:36 -0800)]
vswitchd: Fix issue of networking not working when Tools not installed
During the transition to the configuration database, not all code from
the bridge compatibility layer was updated. In particular, the code
which removes port configuration based on RTNL notifications of port
removal was not updated. This commit brings that code back.
Note that the code that removes ports based on actively checking whether
ports exist is still commented out pending a review of its impact on GRE
support.
Justin Pettit [Tue, 12 Jan 2010 00:57:29 +0000 (16:57 -0800)]
ovs-vsctl: Print correct fail-mode
When printing the fail-mode, ovs-vsctl would always attempt to print the
top-level one--even if it didn't exist. So, in addition to sometimes
being wrong, it could cause segfaults.
Ben Pfaff [Tue, 12 Jan 2010 01:00:25 +0000 (17:00 -0800)]
reconnect: Fix repeated RECONNECT_CONNECT that was confusing JSON-RPC.
reconnect_run() returns RECONNECT_CONNECT to tell the client that it should
start a new connection. The client is then supposed to call
reconnect_connecting() to tell the FSM that it has begun a connection
attempt. However, even after reconnect_connecting() was called,
reconnect_run() continued to return RECONNECT_CONNECT on each call until
the connection succeeded or failed. This confused the jsonrpc_session
client, which expected that it would get a 0 return value from
reconnect_run() while the connection attempt was in progress. Connections
that required multiple trips through the main poll loop, e.g. for SSL
negotiation, would often get cut off to start a second connection attempt.
This commit change reconnect_run() to return RECONNECT_CONNECT only until
the client tells it that a connection is in progress, which fixes the
problem. This change entails a change to the internal details of the
reconnect FSM, so this commit also updates the reconnect tests to match.
Ben Pfaff [Mon, 11 Jan 2010 21:14:54 +0000 (13:14 -0800)]
ovsdb: Save some space in the log for newly inserted records.
When a new record is inserted into a database, ovsdb logs the values of all
of the fields in the record. However, often new records have many columns
that contain default values. There is no need to log those values, so this
commit causes them to be omitted.
As a side effect, this also makes "ovsdb-tool show-log --more --more"
output easier to read, because record insertions print less noise. (Adding
--more --more to this command makes it print changes to database records.
The --more option will be introduced in an upcoming commit.)
Ben Pfaff [Fri, 8 Jan 2010 23:36:06 +0000 (15:36 -0800)]
jsonrpc: Properly implement connection timeout.
jsonrpc_session_connect() indirectly called reconnect_disconnected(), which
told the reconnect object that the connection had failed, before it told it
that the connection attempt had started. When the connection didn't
complete immediately, this caused the connection to time out immediately,
without any backoff.
Ben Pfaff [Fri, 8 Jan 2010 21:09:10 +0000 (13:09 -0800)]
datapath: Add a find the kernel source directory for Debian 2.6.32+.
Debian's kernel-headers packages starting from 2.6.32 (or thereabouts) put
links to the kernel build and source directories at the same level, named
"build" and "source" respectively. Add support for this structure.
Ben Pfaff [Fri, 8 Jan 2010 18:41:48 +0000 (10:41 -0800)]
xenserver: Fix fallout from removing remote command execution.
Commit 2280e7223 "ofproto: Drop remote command execution feature." removed
the remote command execution feature from the vswitch. This commit removes
one bit that was overlooked.
Ben Pfaff [Thu, 7 Jan 2010 22:00:58 +0000 (14:00 -0800)]
vconn: Test SSL vconns too.
This test should help avoid simple bugs in the SSL vconn and SSL stream
implementations in the future. It would have found the bugs fixed by
recent commits.
Ben Pfaff [Thu, 7 Jan 2010 21:55:35 +0000 (13:55 -0800)]
stream: Make passive SSL and TCP streams report bound addresses as names.
The names of passive SSL and TCP streams were being poorly reported: TCP
always simply reported "ptcp", and SSL reported whatever was passed in.
This commit makes them report the addresses that were actually bound by
the TCP/IP stack, which is more useful for testing, debugging, and logging.
Ben Pfaff [Thu, 7 Jan 2010 19:12:36 +0000 (11:12 -0800)]
socket-util: Allow binding without a port number in inet_open_passive().
The test-vconn program binds a socket to a nonspecific port number. To
add SSL support to this program, it needs to be able to use SSL, and the
stream library is the easiest way to do that. But the stream library
can't bind to a nonspecific port. This commit adds that feature, by adding
it to the function that the stream SSL library uses as a building block.
Ben Pfaff [Tue, 22 Dec 2009 00:38:58 +0000 (16:38 -0800)]
Remove "fault" module.
This module, which catches segmentation faults and prints a backtrace
before exiting, was useful for a while, but I believe that it has now
outlived its purpose. It is altogether better to have a core dump from
which one can extract much more information than a usually-poor backtrace,
and core dumps are much better integrated into a typical Unix system.
In addition, the "fault" module was of course not all that portable.
Ben Pfaff [Wed, 6 Jan 2010 22:35:20 +0000 (14:35 -0800)]
vconn: Reimplement in terms of the "stream" abstraction.
This reduces the amount of redundancy in the source tree, by making all of
the current implementations of a vconn simply delegate to the "stream"
abstraction.
Ben Pfaff [Wed, 6 Jan 2010 22:27:46 +0000 (14:27 -0800)]
vconn: Convert vconn code to modern OVS structure.
The vconn code is a relative fossil as OVS code goes. It was written
before we had really figured how code should fit together. Part of that
history is that it used poll_fd_callback() to register callbacks without
the assistance of other code. That isn't how the rest of OVS works now;
this code is the only remaining user of that function.
To make it more like the rest of the system, this code gets rid of the use
of poll_fd_callback(). It also adds vconn_run() and vconn_run_wait()
functions and calls to them from the places where they are now required.
SSL, which will be added in an upcoming commit, requires some background
processing, which is best done in a "run" function in our architecture.
This commit adds stream_run() and stream_run_wait() and calls to them from
the places where they will be required.
Ben Pfaff [Wed, 6 Jan 2010 22:20:19 +0000 (14:20 -0800)]
Check invariants earlier in vconn and stream code.
These invariants are checked by vconn_open() and stream_open(), but there
is no reason not to check them earlier also. vconn and stream creation
don't have to go through vconn_open() and stream_open(), so this ensures
that the invariants get checked either way.
Jesse Gross [Tue, 8 Dec 2009 22:11:22 +0000 (14:11 -0800)]
fatal-signal: Run signal hooks outside of actual signal handlers.
Rather than running signal hooks directly from the actual signal
handler, simply record the fact that the signal occured and run
the hook next time around the poll loop. This allows significantly
more freedom as to what can actually be done in the signal hooks.
Ben Pfaff [Tue, 22 Dec 2009 01:02:17 +0000 (17:02 -0800)]
ofproto: Drop remote command execution feature.
At one point Nicira had deployment plans for which adding a remote command
execution feature to the OpenFlow stack made a lot of sense. We no longer
have those plans, as far as I know, and leaving the feature in seems like
a huge potential security hole. So this commit blows away the entire
feature.
Sometimes it is useful to limit the number of connection attempts, either
from policy or because it is not possible to reconnect at all (e.g. because
a connection was accepted from a listening socket instead of made with
connect()). This commit adds that feature.
Justin Pettit [Fri, 18 Dec 2009 22:40:12 +0000 (14:40 -0800)]
xenserver: Use ovs-appctl to reopen log files
During the changes to use the configuration database the "reload"
command was removed from the init script. In addition to reloading the
configuration file, binaries also reopened their log files, which is the
behavior the logrotate configuration expected. This change makes the
logrotate configuration no longer user the "reload" command.
Justin Pettit [Fri, 18 Dec 2009 22:34:05 +0000 (14:34 -0800)]
Remove cfg library
A few places in bridge.c still reference the old cfg library, but they
were commented out a while back. This removes the library, since we've
now switched over to a configuration database.
Ben Pfaff [Thu, 17 Dec 2009 18:56:01 +0000 (10:56 -0800)]
daemon: Allow daemon child process to report success or failure to parent.
There are conflicting pressures in startup of a daemon process:
* The parent process should exit with an error code if the daemon
cannot start up successfully.
* Some startup actions must be performed in the child process, not in
the parent. The most obvious of these are file locking, since
child processes do not inherit locks, and anything that requires
knowing the child process's PID (e.g. unixctl sockets).
Until now, this conflict has usually been handled by giving up part of the
first property, i.e. in some cases the parent process would exit
successfully and the child immediately afterward exit with a failure code.
This commit introduces a better approach, by allowing daemons to perform
startup work in the child and only then signal the parent that they have
successfully started. If the child instead exits without signaling
success, the parent passes this exit code along to its own parent.
This commit also modifies the daemons that can usefully take advantage of
this new feature to do so.