sliver-openvswitch.git
14 years agodatapath: Fix OOPS when dp_sysfs_add_if() fails.
Ben Pfaff [Wed, 5 Aug 2009 22:22:25 +0000 (15:22 -0700)]
datapath: Fix OOPS when dp_sysfs_add_if() fails.

Until now, when dp_sysfs_add_if() failed, the caller ignored the failure.
This is a minor problem, because everything else should continue working,
without sysfs entries for the interface, in theory anyhow.  In actual
practice, the error exit path of dp_sysfs_add_if() does a kobject_put(),
and that kobject_put() calls release_nbp(), so that the new port gets
freed.  The next reference to the new port (usually in an ovs-vswitchd call
to the ODP_PORT_LIST ioctl) will then use the freed data and probably OOPS.

The fix is to make the datapath code, as opposed to the sysfs code,
responsible for creating and destroying the net_bridge_port kobject.  The
dp_sysfs_{add,del}_if() functions then just attach and detach the kobject
to sysfs and their cleanup routines no longer need to destroy the kobject
and indeed we don't care whether dp_sysfs_add_if() really succeeds.

This commit also makes the same transformation to the datapath's ifobj,
for consistency.

It is easy to trigger the OOPS fixed by this commit by adding a network
device A to a datapath, then renaming network device A to B, then renaming
network device C to A, then adding A to the datapath.  The last attempt to
add A will fail because a file named /sys/class/net/<datapath>/brif/A
already exists from the time that C was added to the datapath under the
name A.

This commit also adds some compatibility infrastructure, because it moves
code out of #ifdef SUPPORT_SYSFS and it otherwise wouldn't build.

14 years agodatapath: Prepare to extend lifetime of kobjects.
Ben Pfaff [Wed, 5 Aug 2009 20:45:13 +0000 (13:45 -0700)]
datapath: Prepare to extend lifetime of kobjects.

The following commit will move the initialization of the datapath and
net_bridge_port kobjects earlier and the destruction later, without
changing when those kobjects are attached to sysfs.  To do so, the
initialization of kobjects and attaching to sysfs has to be done as
separate steps.  That's already the case for net_bridge_port kobjects, and
this commit makes it so for datapath kobjects too.

This commit also simplifies some code, since the split API exists both
before and after 2.6.25, but the combined functions changed names.

Also, in dp_sysfs_add_if() call kobject_init() after initializing the
kset member, since kobject_init() expects that.  This makes no actual
difference in this case since the kobj is obtained from kzalloc(), but
it still seems better.

14 years agodatapath: Rename brc_sysfs_* to dp_sysfs_*.
Ben Pfaff [Wed, 5 Aug 2009 19:56:23 +0000 (12:56 -0700)]
datapath: Rename brc_sysfs_* to dp_sysfs_*.

These files and names are now part of the datapath, not brcompat, so name
them appropriately so as not to confuse anyone.

14 years agodatapath: Move sysfs support from brcompat_mod into openvswitch_mod.
Ben Pfaff [Wed, 5 Aug 2009 19:51:30 +0000 (12:51 -0700)]
datapath: Move sysfs support from brcompat_mod into openvswitch_mod.

In the past problems have arisen due to the different ways that datapaths
are created and destroyed in the three different cases:

1. sysfs supported, brcompat_mod loaded.
2. sysfs supported, brcompat_mod not loaded.
3. sysfs not supported.

The brcompat_mod loaded versus not loaded distinction is the stickiest
because we have to do all the calls into brcompat_mod through hook
functions, which in turn causes pressure to keep the number of hook
functions small and well-defined, which makes it really difficult to put
the hook call points at exactly the right place.  Witness, for example,
this piece of code in datapath.c:

        int dp_del_port(struct net_bridge_port *p)
        {
                ASSERT_RTNL();

        #ifdef SUPPORT_SYSFS
                if (p->port_no != ODPP_LOCAL && dp_del_if_hook)
                        sysfs_remove_link(&p->dp->ifobj, p->dev->name);
        #endif

The code inside the #ifdef is logically part of the brcompat_mod sysfs
support, but the author of this code (quite reasonably) didn't want to
add a hook function call there.  After all, what would you call the
hook function?  There's no obvious name from the dp_del_port() caller's
perspective.

All this argues that sysfs support should be in openvswitch_mod itself,
since it has to be tightly integrated, not bolted on.  So this commit
moves it there.

Now, this is not to say that openvswitch_mod should actually be
implementing bridge-compatible sysfs.  In the future, it probably should
not be; rather, it should implement something appropriate for Open vSwitch
datapaths instead.  But right now we have bridge-compatible sysfs, and so
that's what this commit moves.

14 years agoxenserver: Really take devices down in interface-reconfigure.
Ben Pfaff [Wed, 5 Aug 2009 18:39:03 +0000 (11:39 -0700)]
xenserver: Really take devices down in interface-reconfigure.

When down_netdev was called with deconfigure=True (which is the default),
it would invoke, e.g. "/sbin/ifconfig eth0 down 0.0.0.0", with the
intention of taking the interface down and removing any IP address from it
at the same time.

In fact, this removed any IP address from it and brought the device *up*,
because ifconfig executes its commands in the order that they are
specified, and setting or unsetting an IP address brings a device up.

We could specify the commands in the opposite order ("0.0.0.0 down") but
it seems to me more obviously correct to just run ifconfig twice, so that
is what this commit does.

This commit could break things, because it could be that there is a bug
elsewhere that depends on down_netdev not actually bringing a device down,
but it is needed for the upcoming device renaming commit (to fix bug
NIC-20), because a network device has to be down to be renamed.

14 years agobrcompat: Add comments to sysfs code.
Ben Pfaff [Wed, 5 Aug 2009 20:09:15 +0000 (13:09 -0700)]
brcompat: Add comments to sysfs code.

I got tired of figuring out over and over what these function calls do.

14 years agovswitchd: Fix logged warnings for new internal ports.
Ben Pfaff [Thu, 6 Aug 2009 17:33:26 +0000 (10:33 -0700)]
vswitchd: Fix logged warnings for new internal ports.

Justin reported that adding an internal port to a bridge caused
ovs-vswitchd to log a pair of warnings.  This commit suppresses those
warnings, which were harmless.

14 years agodatapath: Support jumbo frames in the datapath device
Justin Pettit [Sat, 1 Aug 2009 07:09:56 +0000 (00:09 -0700)]
datapath: Support jumbo frames in the datapath device

The datapath has no problems switching jumbo frames (frames with a payload
greater than 1500 bytes), but it has not supported sending and receiving
them to the device itself.  With this commit, the MTU can be set as large
as the minimum MTU size of the devices that are directly attached, or 1500
bytes if there are none.  This mimics the behavior of the Linux bridge.

Feature #1736

14 years agovswitchd: Initialize cfg properly and check return values
Justin Pettit [Sat, 1 Aug 2009 08:03:23 +0000 (01:03 -0700)]
vswitchd: Initialize cfg properly and check return values

A previous checkin added the cfg_init() function, so we now call it.  We
also check the return value of the initial call to cfg_read(), since if
it fails, there's not much point in continuing.

14 years agocfg: Terminate cfg to prevent crashes
Justin Pettit [Sat, 1 Aug 2009 07:58:31 +0000 (00:58 -0700)]
cfg: Terminate cfg to prevent crashes

If cfg_* accessor calls were made before cfg_read() was called (or it
returned error), they could cause segfault.  This checkin terminates the
cfg structure in such a way that will prevent these run-time problems.

Bug #1693

14 years agovswitchd: Update /proc/net/bonding when bonded port properties change.
Ben Pfaff [Wed, 29 Jul 2009 16:56:46 +0000 (09:56 -0700)]
vswitchd: Update /proc/net/bonding when bonded port properties change.

Until now ovs-vswitchd has created the files in /proc/net/bonding, but not
updated them, because there was little need.  But the Citrix QA tests check
that the list of bond hashes in that file is kept up-to-date, so we need
to update them whenever the bond hashes (or other data in the file) change.
This commit does that.

Bug NIC-16.

14 years agovswitchd: Add bond hashes to /proc/net/bonding compatibility layer.
Ben Pfaff [Wed, 29 Jul 2009 00:10:10 +0000 (17:10 -0700)]
vswitchd: Add bond hashes to /proc/net/bonding compatibility layer.

The Citrix QA scripts require the bond hashes and their assigned devices
to be noted in /proc/net/bonding.  We weren't doing that, so this commit
adds them.

Bug NIC-16.

14 years agoXenServer: Correct license in spec file
Justin Pettit [Thu, 30 Jul 2009 06:37:28 +0000 (23:37 -0700)]
XenServer: Correct license in spec file

The license file indicated that the software is licensed under the GPLv3
license.  This commit corrects that to state that it's licensed under
the Apache 2.0 license with the exception of the "datapath" directory,
which is GPLv2.  The style and abbreviations were from the following
page:

    http://fedoraproject.org/wiki/Packaging/LicensingGuidelines

14 years agovswitchd: Add unixctl command to dump all flows, including hidden ones
Justin Pettit [Tue, 14 Jul 2009 20:03:57 +0000 (13:03 -0700)]
vswitchd: Add unixctl command to dump all flows, including hidden ones

Previously, the only way to query the flow table was to run "ovs-ofctl
dump-flows".  This returned most flows, but not those marked hidden by
secchan.  Hidden flows are setup by mechanisms such as in-band control,
since they must not be modified by users of the controller.  However,
when debugging problems on the switch, it is often useful to see what
the flow table is actually doing.  The new "bridge/dump-flows" command
added to ovs-appctl shows all flows being used by the OpenFlow stack.

14 years agoFix DHCP request source port and add port #define's
Justin Pettit [Thu, 16 Jul 2009 22:02:53 +0000 (15:02 -0700)]
Fix DHCP request source port and add port #define's

DHCP requests were sent with a source port of 66, when it should be 68.
This code has been tested, so apparently many DHCP servers don't pay
attention to the source port.  This commit also adds #define's for the
DHCP ports, so that magic numbers don't need to be used.

14 years agoFix tab/space issue in datapath-protocol.h
Justin Pettit [Thu, 16 Jul 2009 22:21:47 +0000 (15:21 -0700)]
Fix tab/space issue in datapath-protocol.h

14 years agovswitchd: Remove non-used function to prevent compiler warnings
Justin Pettit [Wed, 8 Jul 2009 21:51:47 +0000 (14:51 -0700)]
vswitchd: Remove non-used function to prevent compiler warnings

The function send_ofmp_error_msg() is not currently used, so it was
producing compiler warnings that it was defined.  Remove the function
until it's needed.

14 years agoCleanup warnings about "save_ptr" in strtok_r argument
Justin Pettit [Wed, 8 Jul 2009 21:43:29 +0000 (14:43 -0700)]
Cleanup warnings about "save_ptr" in strtok_r argument

The compiler warns about the "save_ptr" argument to strtok_r being
unitialized.  This cleans that up.

14 years agoReduce default probe interval to 5 seconds (and fail-open timeout to 15).
Ben Pfaff [Wed, 29 Jul 2009 18:31:07 +0000 (11:31 -0700)]
Reduce default probe interval to 5 seconds (and fail-open timeout to 15).

Before now, the default probe interval (the idle time after which an echo
request is sent on an OpenFlow connection) was set to 15 seconds.  The
fail-open timeout is 3 times the probe interval, so this meant that it
took 45 seconds for a switch to fail open.

Users at Nicira have commented that this is too long.  They don't like the
idea that the network will be down for most of a minute before it begins to
recover.  So this commit changes the default probe interval to 5 seconds,
hence the fail-open timeout to 15 seconds.

14 years agoReduce default maximum connection timeout from 15 seconds to 8 seconds.
Ben Pfaff [Tue, 28 Jul 2009 17:21:03 +0000 (10:21 -0700)]
Reduce default maximum connection timeout from 15 seconds to 8 seconds.

Users at Nicira have commented that a maximum reconnection time of 15
seconds, which was the default, is too long.  This commit cuts it to 8
seconds, on the theory that an administrator is willing to wait that long
before deciding that a change that should restore connectivity did not
work.

14 years agoxenserver: Do not set or remove vSwitchVersion xapi parameter anymore.
Ben Pfaff [Tue, 28 Jul 2009 22:35:57 +0000 (15:35 -0700)]
xenserver: Do not set or remove vSwitchVersion xapi parameter anymore.

The other-config:vSwitchVersion parameter was used to announce the Open
vSwitch version installed on a XenServer host, but this had the problem
that it could not be read or updated if the connection to the pool master
was down.  Because of this problem, the only user of this parameter in
Open vSwitch was removed (in commit 3cdc31a4c3a "xenserver: Retrieve
vSwitch version from binary in xsconsole").  So this commit finished the
process and removes the parameter entirely.

This should fix hangs on Open vSwitch installation and removal due to
waiting on the connection to the pool master.

14 years agoxenserver: Honor the MAC address specified in xapi database for bonds.
Ben Pfaff [Wed, 22 Jul 2009 19:21:25 +0000 (12:21 -0700)]
xenserver: Honor the MAC address specified in xapi database for bonds.

The xapi database for PIFs specifies the MAC address that should be used
for bonds, but interface-reconfigure didn't honor it and ovs-vswitchd
didn't have a way to configure it anyhow.  This commit fixes both problems.

Bug #1645.

14 years agoxenserver: Enable ARP filtering to work around xhad bug.
Ben Pfaff [Tue, 28 Jul 2009 22:44:58 +0000 (15:44 -0700)]
xenserver: Enable ARP filtering to work around xhad bug.

This works around a bug in xhad, which binds to a particular Ethernet
device, which in turn causes ICMP port unreachable messages if packets are
received are on the wrong interface, which in turn can happen if we send
out ARP replies on every interface (as Linux does by default) instead of
just on the interface that has the IP address being ARPed for, which this
sysctl setting in turn works around.

Justin Pettit did most of the work tracking down the origin of this bug.

Bug #1378.

14 years agoxenserver: Retrieve vSwitch version from binary in xsconsole
Justin Pettit [Tue, 28 Jul 2009 22:24:32 +0000 (15:24 -0700)]
xenserver: Retrieve vSwitch version from binary in xsconsole

The xsconsole plugin shows status information about Open vSwitch.  The
version information was retrieved from XAPI, but this could cause
problems.  The most easily reproduced is to make a XenServer part of a
pool, then remove it.  The version string is no longer in the
XenServer's local XAPI view, so it reports "<unknown>".  A more direct
way to get the information is to directly query the binary, which is
what this commit does.

Bug #1626

14 years agoLabel the current "citrix" release 0.90.4. v0.90.4
Justin Pettit [Tue, 28 Jul 2009 18:05:28 +0000 (11:05 -0700)]
Label the current "citrix" release 0.90.4.

14 years agoDo not try to resolve DNS for OpenFlow controllers or netflow collectors.
Ben Pfaff [Wed, 15 Jul 2009 22:29:49 +0000 (15:29 -0700)]
Do not try to resolve DNS for OpenFlow controllers or netflow collectors.

Until now, setting a netflow collector to a DNS name would cause
secchan to attempt to resolve that DNS name each time that the set of
netflow collectors is re-set.  For the vswitch, this is every time that
the vswitch reconfigures itself.

Unfortunately, DNS lookup within secchan cannot work as currently
implemented, because it needs both an asynchronous DNS resolver library
and in-band control updates.  Currently we have neither.  Attempting to
look up DNS anyway just hangs.

This commit disables DNS lookup entirely, and updates the documentation to
change user expectations.  DNS still won't work, but at least it won't
hang.

Bug #1609.

14 years agoxenserver: Fix creating, destroying bonds with the management connection.
Ben Pfaff [Fri, 17 Jul 2009 19:37:01 +0000 (12:37 -0700)]
xenserver: Fix creating, destroying bonds with the management connection.

Creating a bond from the network device that holds the Xen management
connection automatically transfers the management connection to that bond.
However, we weren't properly removing the IP address from the network
devices that constituted the bond.  This commit fixes that problem.

Bug #1566.

14 years agobrcompat: Make "brctl showmacs" honor Linux notion of bridge composition.
Ben Pfaff [Thu, 16 Jul 2009 22:16:06 +0000 (15:16 -0700)]
brcompat: Make "brctl showmacs" honor Linux notion of bridge composition.

The kernel only handles a single VLAN per bridge, but vswitchd can deal
with all the VLANs on a single bridge.  This commit makes "brctl showmacs"
pretend that the former is the case even though the latter is the
implementation.

Bug #1567.

14 years agovswitchd: Make "fdb/show" output more meaningful port numbers.
Ben Pfaff [Thu, 16 Jul 2009 22:15:02 +0000 (15:15 -0700)]
vswitchd: Make "fdb/show" output more meaningful port numbers.

The "fdb/show" unixctl command was showing vswitch-internal port indexes,
which cannot be meaningfully interpreted by software outside vswitchd.
Also, they potentially change every time the vswitchd configuration file
changes.  This commit changes it to use a datapath port index instead,
which are both more meaningful and more stable.

14 years agocfg: New function cfg_get_matches().
Ben Pfaff [Thu, 16 Jul 2009 22:07:05 +0000 (15:07 -0700)]
cfg: New function cfg_get_matches().

14 years agoImplement "brctl showmacs" support in brcompat and ovs-brcompatd.
Ben Pfaff [Wed, 15 Jul 2009 19:46:06 +0000 (12:46 -0700)]
Implement "brctl showmacs" support in brcompat and ovs-brcompatd.

This is needed by the Citrix test infrastructure.

14 years agobrcompat: Refactor infrastructure for communication with ovs-brcompatd.
Ben Pfaff [Wed, 15 Jul 2009 19:40:16 +0000 (12:40 -0700)]
brcompat: Refactor infrastructure for communication with ovs-brcompatd.

In an upcoming change, brcompat will need to receive bulk data from
ovs-brcompatd.  The existing brcompat infrastructure for receiving data
only supports returning an error code, so this commit adds the ability to
receive an arbitrary sk_buff and updates the existing users to handle it.

14 years agobrcompatd: Factor code out of prune_ports().
Ben Pfaff [Wed, 15 Jul 2009 19:09:43 +0000 (12:09 -0700)]
brcompatd: Factor code out of prune_ports().

An upcoming commit will also need to gather all of the interfaces on a
specified bridge, so break this code into a helper function.

14 years agoprocess: New function process_run_capture().
Ben Pfaff [Wed, 15 Jul 2009 19:45:44 +0000 (12:45 -0700)]
process: New function process_run_capture().

In an upcoming commit, ovs-brcompatd will need to create a subprocess and
capture both its stdout and stderr separately, which one cannot do with
simple interfaces such as popen().  This function provides that ability.

14 years agoprocess: Factor code out of process_start() into helper functions.
Ben Pfaff [Wed, 15 Jul 2009 19:07:02 +0000 (12:07 -0700)]
process: Factor code out of process_start() into helper functions.

An upcoming commit will add a new function that can also use these helper
functions.

14 years agoprocess: Fix races on fatal signal handling in process_start().
Ben Pfaff [Wed, 15 Jul 2009 19:05:04 +0000 (12:05 -0700)]
process: Fix races on fatal signal handling in process_start().

To prevent fatal signals in a child process from causing the parent
process's pidfile, etc. to be deleted, we need to block fatal signals
around fork and call fatal_signal_fork() in the child process.

This problem was noticed through code inspection; it has not been observed
in practice.

14 years agovswitchd: New unixctl command "fdb/show" to print the MAC learing table.
Ben Pfaff [Wed, 15 Jul 2009 18:05:37 +0000 (11:05 -0700)]
vswitchd: New unixctl command "fdb/show" to print the MAC learing table.

To implement "brctl showmacs" for bridge compatibility, brcompatd needs to
be able to extract the MAC learning table from ovs-vswitchd.  This provides
a way, and it may be directly useful to switch administrators also.

14 years agomac-learning: New function mac_entry_age().
Ben Pfaff [Wed, 15 Jul 2009 18:02:24 +0000 (11:02 -0700)]
mac-learning: New function mac_entry_age().

This function will be used as part of printing the MAC learning table at
user request.

14 years agoAdd macros for parsing MAC addresses from strings.
Ben Pfaff [Wed, 15 Jul 2009 17:58:30 +0000 (10:58 -0700)]
Add macros for parsing MAC addresses from strings.

14 years agoNew function ds_steal_cstr().
Ben Pfaff [Wed, 15 Jul 2009 17:57:17 +0000 (10:57 -0700)]
New function ds_steal_cstr().

14 years agoAdd function get_null_fd(), to reduce code redundancy.
Ben Pfaff [Wed, 15 Jul 2009 17:54:51 +0000 (10:54 -0700)]
Add function get_null_fd(), to reduce code redundancy.

14 years agobrcompat: Improve comments in header file.
Ben Pfaff [Tue, 14 Jul 2009 16:16:55 +0000 (09:16 -0700)]
brcompat: Improve comments in header file.

14 years agovswitchd: Skip updelay on slave when only a single bond slave is up.
Ben Pfaff [Mon, 13 Jul 2009 18:09:52 +0000 (11:09 -0700)]
vswitchd: Skip updelay on slave when only a single bond slave is up.

If a network device takes a few seconds to detect carrier, as some do, then
when bringing up a network device and then immediately adding that device
to a bridge, the bond code would start out with that slave considered down
and apply the full updelay to it before bringing it up.  With the 31-second
updelay set by XenServer, this is excessive: we end up having no
connectivity at all for 31 seconds even though there is no reason for it.

This commit makes the bond code disregard the updelay when an interface
comes up on a bond that has no enabled interfaces, and updates the
documentation to match.

Part of bug #1566.

14 years agovswitchd: Fix log messages when bond slaves are enabled or disabled.
Ben Pfaff [Mon, 13 Jul 2009 17:04:45 +0000 (10:04 -0700)]
vswitchd: Fix log messages when bond slaves are enabled or disabled.

We were printg "enabled" when a slave was disabled and vice versa.

Part of bug #1566.

14 years agoFix unitialized variable in coverage_log()
Justin Pettit [Tue, 14 Jul 2009 07:25:44 +0000 (00:25 -0700)]
Fix unitialized variable in coverage_log()

When providing the ability to force coverage printouts to occur, some
code was moved around that allowed the "hash" variable to be used
unitialized.  This fixes that.

Thanks to Ben for pointing out the problem.

Bug #1577

14 years agodatapath: Don't orphan packets in dp_dev transmit path.
Ben Pfaff [Mon, 13 Jul 2009 22:50:32 +0000 (15:50 -0700)]
datapath: Don't orphan packets in dp_dev transmit path.

Before commit 72ca14c1 "datapath: Fix race against workqueue in dp_dev and
simplify code," the dp_dev network device had a device queue, and we would
orphan packets before sticking them on the queue.  This screwed up socket
accounting a bit, but the effect was limited to the device queue length.

Now, after that commit, the dp_dev device has no device queue, but it still
orphans packets.  This screws up socket accounting a *lot*, because the
effect is now unlimited, since there is no queue to limit it.

The solution is to not orphan packets at all.  There is little need for it
now since packet transmission now happens immediately, not in a workqueue
whose execution may be delayed.

This should fix bug #1519, which tests "netperf -t UDP_STREAM" performance,
finding that an unrealistically high number of UDP packets could be sent
but that none at all were received.  The send rate is due to the orphaning,
the receive rate presumably because at least one out of approx. 65535/1500
= 44 fragments per full packet were dropped in each case.

14 years agovconn: Fix detection of vconn local IP address, to fix in-band control.
Ben Pfaff [Mon, 13 Jul 2009 22:05:51 +0000 (15:05 -0700)]
vconn: Fix detection of vconn local IP address, to fix in-band control.

The in-band control code needs to know the IP and port of both ends of the
control connection.  However, the vconn code was only reporting the local
address after the connection had already succeeded, which created a
chicken-and-egg problem.  In practice we would fail to connect until the
switch went into fail-open, at which point the connection would go through.

Fortunately, we can get the local IP address right after we try to connect,
not just after the connection completes, so this commit changes the code
to do that.

This commit also breaks setting the remote IP and port into functions
separate from vconn_init(), which makes the code more readable.

14 years agoRevert "datapath: Don't orphan packets in dp_dev transmit path."
Ben Pfaff [Mon, 13 Jul 2009 18:56:26 +0000 (11:56 -0700)]
Revert "datapath: Don't orphan packets in dp_dev transmit path."

This reverts commit 0858ad2b9d2f594f99bb11fc9b331ce8b1de953d.
Although that commit partially fixed a performance regression
relative to the Linux bridge, it introduced other problems that
were not apparent in the performance testing (e.g.
WARN_ON_ONCE(skb->destructor) now triggers in dp_process_received_packet()).
The fix is not completely obvious, so reverting now to ensure stability
while investigating the problem.

14 years agodatapath: Don't orphan packets in dp_dev transmit path.
Ben Pfaff [Mon, 13 Jul 2009 16:41:26 +0000 (09:41 -0700)]
datapath: Don't orphan packets in dp_dev transmit path.

Before commit 72ca14c1 "datapath: Fix race against workqueue in dp_dev and
simplify code," the dp_dev network device had a device queue, and we would
orphan packets before sticking them on the queue.  This screwed up socket
accounting a bit, but the effect was limited to the device queue length.

Now, after that commit, the dp_dev device has no device queue, but it still
orphans packets.  This screws up socket accounting a *lot*, because the
effect is now unlimited, since there is no queue to limit it.

The solution is to not orphan packets at all.  There is little need for it
now since packet transmission now happens immediately, not in a workqueue
whose execution may be delayed.

This should fix bug #1519, which tests "netperf -t UDP_STREAM" performance,
finding that an unrealistically high number of UDP packets could be sent
but that none at all were received.  The send rate is due to the orphaning,
the receive rate presumably because at least one out of approx. 65535/1500
= 44 fragments per full packet were dropped in each case.

14 years agoRemove "coverage/clear" command due its limited use
Justin Pettit [Sat, 11 Jul 2009 00:36:09 +0000 (17:36 -0700)]
Remove "coverage/clear" command due its limited use

The poll loop calls coverage_clear on every pass anyway, so provide a
function to call it separately is of limited value.

14 years agoFix small typo in ovs-ofctl man page.
Justin Pettit [Sat, 11 Jul 2009 00:33:41 +0000 (17:33 -0700)]
Fix small typo in ovs-ofctl man page.

14 years agoProvide ability to retrieve coverage information
Justin Pettit [Fri, 10 Jul 2009 22:09:41 +0000 (15:09 -0700)]
Provide ability to retrieve coverage information

Previously, there was no way to induce coverage information to be
displayed; it would only print when the system noticed unusual delays
between polling intervals.  Now, production of coverage logs can be
forced with "coverage/log" command in ovs-appctl.  Coverage counters may
be reset with "coverage/clear".

14 years agodatapath: Fix races in updating dp_dev port statistics.
Ben Pfaff [Wed, 8 Jul 2009 19:32:07 +0000 (12:32 -0700)]
datapath: Fix races in updating dp_dev port statistics.

14 years agodatapath: Fix race in datapath creation.
Ben Pfaff [Wed, 8 Jul 2009 18:31:59 +0000 (11:31 -0700)]
datapath: Fix race in datapath creation.

Before we create the local port, we should allocate and assign the table.
Otherwise packets sent on the local port before we do so will cause an
OOPS.

This is a theoretical race that has not been observed in practice.

14 years agodatapath: Fix race against workqueue in dp_dev and simplify code.
Ben Pfaff [Wed, 8 Jul 2009 19:23:32 +0000 (12:23 -0700)]
datapath: Fix race against workqueue in dp_dev and simplify code.

The dp_dev_destroy() function failed to cancel the xmit_queue work, which
allowed it to run after the device had been destroyed, accessing freed
memory.  However, simply canceling the work with cancel_work_sync() would
be insufficient, since other packets could get queued while the work
function was running.  Stopping the queue with netif_tx_disable() doesn't
help, because the final action in dp_dev_do_xmit() is to re-enable the TX
queue.

This issue led me to re-examine why the dp_dev needs to use a work_struct
at all.  This was implemented in commit 71f13ed0b "Send of0 packets from
workqueue, to avoid recursive locking of ofN device" due to a complaint
from lockdep about recursive locking.

However, there's no actual reason that we need any locking around
dp_dev_xmit().  Until now, it has accepted the standard locking provided
by the network stack.  But looking at the other software devices (veth,
loopback), those use NETIF_F_LLTX, which disables this locking, and
presumably do so for this very reason.  In fact, the lwn article at
http://lwn.net/Articles/121566/ hints that NETIF_F_LLTX, which is otherwise
discouraged in the kernel, is acceptable for "certain types of software
device."

So this commit switches to using NETIF_F_LLTX for dp_dev and gets rid
of the work_struct.

In the process, I noticed that veth and loopback also take advantage of
a network device destruction "hook" using the net_device "destructor"
member.  Using this we can automatically get called on network device
destruction at the point where rtnl_unlock() is called.  This allows us
to stop stringing the dp_devs that are being destroyed onto a list so
that we can free them, and thus simplifies the code along all the paths
that call dp_dev_destroy().

This commit gets rid of a call to synchronize_rcu() (disguised as a call
to synchronize_net(), which is a macro that expands to synchronize_rcu()),
so it probably speeds up deleting ports, too.

14 years agodatapath: Remove declarations of functions that are never defined or used.
Ben Pfaff [Wed, 8 Jul 2009 17:53:18 +0000 (10:53 -0700)]
datapath: Remove declarations of functions that are never defined or used.

14 years agodatapath: Fix use-after-free error in datapath destruction.
Ben Pfaff [Fri, 26 Jun 2009 21:15:04 +0000 (14:15 -0700)]
datapath: Fix use-after-free error in datapath destruction.

When we create a datapath we do this:

1. Create local port.
2. Call add_dp hook.
3. Allow userspace to add more ports.

When we deleted a datapath we were doing this:

1. Call del_dp hook
2. Delete all the ports.

Unfortunately step 1 destroys dp->ifobj, then dp_del_port on any port other
than the local port in step 2 tries to reference dp->ifobj through a call
to sysfs_remove_link().

This commit fixes the problem by changing datapath deletion to mirror
creation:

1. Delete all the ports but the local port.
2. Call dp_del hook.
3. Delete local port.

Commit 010082639 "datapath: Add sysfs support for all (otherwise supported)
Linux versions" makes this problem obvious on a 2.6.25+ kernel configured
with slab debugging, because on such kernels the ifobj is a pointer to a
slab object that is freed by the del_dp hook function (when brcompat_mod
is loaded).  This bug may be just as present on older kernels, but there
the ifobj is part of struct datapath, not a pointer, and thus it is much
harder to trigger.

Bug #1465.

14 years agodatapath: Remove redundant synchronize_rcu() call.
Ben Pfaff [Fri, 26 Jun 2009 19:20:02 +0000 (12:20 -0700)]
datapath: Remove redundant synchronize_rcu() call.

There is no benefit to synchronizing twice, and it might cost us a lot of
time.

14 years agosecchan: Various fixes to run in-band on non-"local" port
Justin Pettit [Wed, 8 Jul 2009 17:40:55 +0000 (10:40 -0700)]
secchan: Various fixes to run in-band on non-"local" port

The in-band control code assumed that we would connect to the controller
over the datapath's "local" port.  If the switch had multiple datapaths,
and the controller was only available through one of them, the other
datapaths would fail to connect.  This is addressed by the following
changes:

    - The controller's MAC address is looked up through the connection's
      interface instead of the datapath's local one.

    - We allow ARP traffic to be sent by the connection's interface.

    - We allow ARP traffic to be sent to the controller's MAC address.
      This is necessary if the controller is running locally (e.g. in a VM).

Bug #1466

14 years agoProvide method to locate device by IP and add more nodev functions to netdev
Justin Pettit [Mon, 6 Jul 2009 22:37:19 +0000 (15:37 -0700)]
Provide method to locate device by IP and add more nodev functions to netdev

In some cases we need to be able to locate a device by its IPv4 address.
There doesn't seem to be an easy way to do this on Linux, so we iterate
through all devices until it's found.  The caller can provide a hint as
to the device, so subsequent checks can be quicker.

This checkin also adds nodev versions of functions to lookup ARP entries
and the IPv4 address of a device.

14 years agoShow more information about connection when querying status.
Justin Pettit [Thu, 2 Jul 2009 20:40:20 +0000 (13:40 -0700)]
Show more information about connection when querying status.

It is often useful in debugging to know the ports and IP addresses being
used in an OpenFlow connection.  This commit adds the remote and local
IP addresses and ports used for a connection to the "status" information
returned by ovs-ofctl.

Feature #1484.

14 years agoHave rconn and vconn export information about IPs and ports
Justin Pettit [Mon, 6 Jul 2009 22:15:11 +0000 (15:15 -0700)]
Have rconn and vconn export information about IPs and ports

Previously, rconn and vconn only allowed users to find out about the
remote IP address.  This set of changes allows users to retrieve the
remote port, local IP, and local port used for the connection.

14 years agoInclude "inttypes.h" to pickup definition of PRIu8.
Justin Pettit [Thu, 2 Jul 2009 19:49:44 +0000 (12:49 -0700)]
Include "inttypes.h" to pickup definition of PRIu8.

14 years agosecchan: Display mgmt and datapath id in status messages
Justin Pettit [Tue, 7 Jul 2009 16:34:37 +0000 (09:34 -0700)]
secchan: Display mgmt and datapath id in status messages

Since it's not always easy to determine the management id, this adds the
ability to see it (and the datapath id) with the "ovs-ofctl status"
command.

Feature #1533

14 years agovswitchd: Add missing argument to check for update-resolv.conf config
Justin Pettit [Tue, 7 Jul 2009 05:12:53 +0000 (22:12 -0700)]
vswitchd: Add missing argument to check for update-resolv.conf config

14 years agovswitchd: Enable updating resolv.conf by default when using discovery
Justin Pettit [Wed, 1 Jul 2009 20:58:19 +0000 (13:58 -0700)]
vswitchd: Enable updating resolv.conf by default when using discovery

When vSwitch does discovery, it is supposed to update resolv.conf by
default.  The way configuration parameters were being read, it would
disable this update by default.

14 years agovswitch: Set minimum probe interval to 5 seconds
Justin Pettit [Tue, 30 Jun 2009 22:24:54 +0000 (15:24 -0700)]
vswitch: Set minimum probe interval to 5 seconds

In vSwitch, the minimum probe interval is supposed to be 5 seconds, but
that was not enforced.  If no interval was specified in the config file,
a value of 0 was being used, which would cause probes to never be sent
and the rconn not to move out of its ACTIVE state.

Possible fix to Bug #1466.

14 years agoLog more rconn status.
Ben Pfaff [Fri, 26 Jun 2009 23:00:07 +0000 (16:00 -0700)]
Log more rconn status.

14 years agoxenserver: Remove cacert when user reconfigures the controller
Justin Pettit [Fri, 26 Jun 2009 19:39:50 +0000 (12:39 -0700)]
xenserver: Remove cacert when user reconfigures the controller

If a user moves from one controller to another, we did not remove the
cacert.  This prevents the switch from connecting to the new controller.
To ease confusion, we now delete the cacert when the user changes or
removes the controller in xsconsole.

Note: This commit has a minor security issue, since we do not remove
trust for the old certificate until the switch is restarted.  In
general, users should only be connected to trusted servers, so the
impact should be low.  Fixes this would require larger changes to the
vconn-ssl code, which we don't want to do so late in the release cycle.

Bug #1457

14 years agoxenserver: Validate controller IP address in xsconsole
Justin Pettit [Thu, 25 Jun 2009 04:52:34 +0000 (21:52 -0700)]
xenserver: Validate controller IP address in xsconsole

When a switch is using in-band control, the controller must be specified
in dotted quad format, since DNS names cannot be resolved until a
connection to the controller has been established.  This commit
validates the user input in the xsconsole plugin.

14 years agovswitchd: Adding and removing mgmt interface breaks connection
Justin Pettit [Thu, 25 Jun 2009 01:03:44 +0000 (18:03 -0700)]
vswitchd: Adding and removing mgmt interface breaks connection

When a managment connection is configured and then removed, putting it
back causes the management connection to never be reestablished.  The
management code checks whether the configuration file has changed before
it attempts to reconfigure itself.  If the only thing that changed was
the lack of a management connection, then it tore down the connection
but didn't update its view of the configuration.  When the same
manager IP is configured, the cached version matches the new version, so
no changes are made.  This commit clears the cached version, so that a
removing and then adding the manager will be detected as a change.

Bug #1448

14 years agoxenserver: Also log ovs-brcompatd messages at INFO level to syslog.
Ben Pfaff [Tue, 23 Jun 2009 18:03:52 +0000 (11:03 -0700)]
xenserver: Also log ovs-brcompatd messages at INFO level to syslog.

INFO level messages are meant to be logged in the ordinary case, and they
are useful for debugging problems, so turn them on by default.

It would be a good idea to do so for ovs-vswitchd also, but we have not
tested how much this would increase the log volume.

14 years agoovs-brcompatd: Turn up log level of port removal messages.
Ben Pfaff [Tue, 23 Jun 2009 18:02:30 +0000 (11:02 -0700)]
ovs-brcompatd: Turn up log level of port removal messages.

These messages proved useful during debugging, and they should not be too
common, so log them at a higher level.

14 years agoovs-brcompatd: Handle XS Tools 5.0.0 destroying and recreating devices
Ben Pfaff [Tue, 23 Jun 2009 18:00:43 +0000 (11:00 -0700)]
ovs-brcompatd: Handle XS Tools 5.0.0 destroying and recreating devices

XenServer Tools version 5.0.0 destroys and recreates network devices with
the same name on boot of (at least) Windows VMs.  We had a race such that
ovs-brcompatd would delete the new device from the vswitchd configuration
file (not the old one).  This commit fixes that problem.

Bug #1429.

14 years agovswitchd: Reduce number of calls to reconfigure() during mgmt updates
Justin Pettit [Sat, 20 Jun 2009 00:41:42 +0000 (17:41 -0700)]
vswitchd: Reduce number of calls to reconfigure() during mgmt updates

When we receive an OpenFlow management protocol Config Update, we
immediately force the switch to reconfigure itself.  This is
functionally correct, but it can cause long delays before return control
back to the switch.  We now keep track of whether there were any changes
and then only force a reconfigure once per management run.

14 years agocfg: Log accurate waiting times in cfg_lock().
Ben Pfaff [Sat, 20 Jun 2009 00:13:57 +0000 (17:13 -0700)]
cfg: Log accurate waiting times in cfg_lock().

When cfg_lock() has to block for some time to obtain the configuration file
lock, it logs the amount of time that it waited.  However, it did not
refresh the current time before it began waiting, so the time that it
logged could be off by a significant amount, which make interpreting the
log file more challenging than it should have been.

This change should mainly affect log output.  It should have little or no
effect on Open vSwitch operation because the factor by which the timeouts
were off is an order of magnitude smaller than the actual timeouts that we
pass into the function.

This is related to bug #1426, but it is not a fix for this bug, which will
be committed separately.

14 years agoxenserver: xsconsole plugin doesn't need execute permissions
Justin Pettit [Thu, 18 Jun 2009 21:17:32 +0000 (14:17 -0700)]
xenserver: xsconsole plugin doesn't need execute permissions

When the vSwitch xsconsole plugin is installed, it doesn't need execute
permissions.  This commit changes the permissions from 755 to 644 to
match the other plugins.

14 years agoxenserver: Handle slave disconnection more gracefully in xsconsole plugin
Justin Pettit [Thu, 18 Jun 2009 21:13:00 +0000 (14:13 -0700)]
xenserver: Handle slave disconnection more gracefully in xsconsole plugin

When a slave cannot connect to the master, the vSwitch xsconsole plugin
complained with some Python style errors on the main display.  This
commit cleans up that behavior.

Bug #1341

14 years agoxenserver: Force reload of config file after VIF deleted
Justin Pettit [Tue, 16 Jun 2009 19:57:25 +0000 (12:57 -0700)]
xenserver: Force reload of config file after VIF deleted

When a VIF is deleted, the "vif" script modifies "/etc/ovs-vswitchd.conf".
After changes are made to the config file, ovs-vswitchd should be told
to reload it, but this wasn't happening.  Now it does.

Thanks to Natasha for catching this.

14 years agoReplace SHA-1 library with one that is clearly licensed.
Ben Pfaff [Mon, 15 Jun 2009 23:03:28 +0000 (16:03 -0700)]
Replace SHA-1 library with one that is clearly licensed.

The SHA-1 library that we used until now was taken from RFC 3174.  That
library has no clearly free license statement, only a license on the text
of the RFC.  This commit replaces this library with a modified version of
the code from the Apache Portable Runtime library from apr.apache.org,
which is licensed under the Apache 2.0 license, the same as the rest of
Open vSwitch.

14 years agoUpdate primary code license to Apache 2.0.
Ben Pfaff [Mon, 15 Jun 2009 22:11:30 +0000 (15:11 -0700)]
Update primary code license to Apache 2.0.

14 years agovswitch: Avoid segfault when revalidating ARP flows.
Ben Pfaff [Mon, 15 Jun 2009 19:14:47 +0000 (12:14 -0700)]
vswitch: Avoid segfault when revalidating ARP flows.

The 'packet' argument to process_flow() is allowed to be null, but some of
the code was assuming that it was always non-null, which caused a segfault
while revalidating ARP flows.

Bug #1394.

14 years agoxenserver: Fix key used to identify network UUID
Justin Pettit [Fri, 12 Jun 2009 01:30:41 +0000 (18:30 -0700)]
xenserver: Fix key used to identify network UUID

The "dump-vif-details" script adds the network UUID to the
ovs-vswitchd.conf file.  Unfortunately, it wrote the key as
"network-uuid", but the code that retrieves it for the management
protocol checked our "net-uuid".  The script now uses the key
"net-uuid".

Thanks to Natasha for catching the problem.

14 years agoxenserver: Pass network UUID to controller for internal networks too.
Ben Pfaff [Thu, 11 Jun 2009 18:33:39 +0000 (11:33 -0700)]
xenserver: Pass network UUID to controller for internal networks too.

vNetManager needs to know the xapi UUIDs for the networks that correspond
to OpenFlow connections.  For some time now we have passed these to it
over the management connection using bridge.<bridgename>.xs-network-uuids
configuration keys, but only now did we notice that this didn't get set for
internal networks.

The reason that it didn't get set is that interface-reconfigure is the
script that sets up these configuration keys, but interface-reconfigure
is never called for internal networks.  Instead, xapi creates them itself
using directly calls to bridge ioctls.  So no amount of tweaks to
interface-reconfigure will help.

This commit fixes the problem by modifying the vif script instead.  This
works acceptably only because xapi is lazy about creating bridges for
internal networks: it creates them only just before it is about to add the
first vif to them.  Thus, by setting up the configuration key in the vif
script, it gets added just after the bridge itself is created.  There is
a race, of course, meaning that there may be a delay between the initial
OpenFlow connection and the time when the configuration key is set up,
but vNetManager can tolerate that.

14 years agovswitch: Add support for large OFMP messages
Justin Pettit [Thu, 11 Jun 2009 08:58:52 +0000 (01:58 -0700)]
vswitch: Add support for large OFMP messages

OpenFlow uses a 16-bit field to describe the message length, which
limits messages to a maximum 65535 bytes.  Some of the messages passed
by the management protocol may be larger than this, so a general
Extended Data message has been added to management protocol.  It
encapsulates a single giant OpenFlow-like message, and breaks it into
however many vaild smaller ones are required.

14 years agoxenserver: Remove debugging redirect from vif script
Justin Pettit [Wed, 10 Jun 2009 22:30:55 +0000 (15:30 -0700)]
xenserver: Remove debugging redirect from vif script

I redirected some output to a temporary file during debugging, but
forgot to remove it.  Now it's removed.

Thanks to Ben for pointing it out.

14 years agovswitch: Fix memory leak when OFMP resource updates are sent
Justin Pettit [Wed, 10 Jun 2009 20:00:10 +0000 (13:00 -0700)]
vswitch: Fix memory leak when OFMP resource updates are sent

When a resource update message is generated by vSwitch, it creates a
couple of svec objects that need to be explicitly destroyed.  This
wasn't happening, so memory would leak after each resource update.  This
commit properly destroys them after use.

14 years agoAdd unit test for SHA-1 library.
Ben Pfaff [Wed, 10 Jun 2009 23:45:28 +0000 (16:45 -0700)]
Add unit test for SHA-1 library.

14 years agoAdd file missing from commit baf099 "Fix glibc 2.7 strtok_r() bug...".
Ben Pfaff [Wed, 10 Jun 2009 23:45:02 +0000 (16:45 -0700)]
Add file missing from commit baf099 "Fix glibc 2.7 strtok_r() bug...".

Oops.

14 years agovswitch: Fix handling of multicast packets received by bonds.
Ben Pfaff [Wed, 10 Jun 2009 22:24:03 +0000 (15:24 -0700)]
vswitch: Fix handling of multicast packets received by bonds.

As long as bonding has been implemented, the vswitch has refused to learn
from multicast packets that arrive on a bond slave if it has already
learned any other port for that source MAC, because it is likely that we
sent the packet out ourselves and are only now receiving a copy of it on
our active slave.

This is entirely correct, but it does not go far enough.  In fact, the
bridge needs to entirely drop such packets.  Otherwise, a host whose MAC
is assigned to a slave other than the active slave will receive a second
copy of multicast packets that it sends out the bond, and other ports
will receive two copies of every multicast packet sent by such a host.

This commit implements this new policy, which simplifies the code at the
same time.

Bug #1387.

14 years agovswitch: Add ovs-appctl commands for debugging and managing bonds.
Ben Pfaff [Wed, 10 Jun 2009 22:13:21 +0000 (15:13 -0700)]
vswitch: Add ovs-appctl commands for debugging and managing bonds.

14 years agoFix glibc 2.7 strtok_r() bug in a more permanent fashion.
Ben Pfaff [Wed, 10 Jun 2009 21:16:40 +0000 (14:16 -0700)]
Fix glibc 2.7 strtok_r() bug in a more permanent fashion.

The glibc 2.7 headers contain a bug that causes strtok_r() to segfault
in some circumstances.  Until now, we have been working around this
problem at each invocation, but this depends on the programmer to remember
to do so each time.

This commit instead adds a shim that adds a work-around to the string.h
header itself, so that it is much more difficult to miss the workaround.

14 years agovswitch: Clarify what "ingress" means in policing
Justin Pettit [Wed, 10 Jun 2009 06:32:30 +0000 (23:32 -0700)]
vswitch: Clarify what "ingress" means in policing

The man page for ovs-vswitchd.conf explains how ingress policing works.
However, what "ingress" means is a bit confusing depending on the
perspective.  For vSwitch, it's from the switch's perspective.  This
means on a PIF, it's the rate traffic comes into the box.  On a VIF,
it's the rate traffic can be *transmitted* from a VM.  This commit
clarifies the man page a bit.

Thanks to Johan for pointing out the problem.

14 years agoxenserver: Add dump-vif-details to automake
Justin Pettit [Wed, 10 Jun 2009 01:40:15 +0000 (18:40 -0700)]
xenserver: Add dump-vif-details to automake

14 years agoxenserver: Send VIF details to controller
Justin Pettit [Wed, 10 Jun 2009 01:19:02 +0000 (18:19 -0700)]
xenserver: Send VIF details to controller

The controller needs to know various things about virtual interfaces as
they move about the network.  This commit sends the VIF, virtual
machine, and network UUIDs associated with the VIF, as well as its MAC
address over the management channel.

Feature #1324

14 years agoXen: Fixes for core handling configuration
Justin Pettit [Tue, 9 Jun 2009 22:46:50 +0000 (15:46 -0700)]
Xen: Fixes for core handling configuration

An improper string comparison operator was used to check whether
FORCE_COREFILES was enabled.  Further, the check to enable core files
was only down when vswitch was started, and not when restarted.

Thanks to Ben for help debugging the issue.

14 years agoFix url in RPM spec.
Justin Pettit [Tue, 9 Jun 2009 00:29:37 +0000 (17:29 -0700)]
Fix url in RPM spec.

14 years agoAdd unit test for TCP/IP checksumming code.
Ben Pfaff [Wed, 10 Jun 2009 00:10:18 +0000 (17:10 -0700)]
Add unit test for TCP/IP checksumming code.

14 years agoFix another "make" versus "make dist" build problem.
Ben Pfaff [Tue, 9 Jun 2009 22:19:24 +0000 (15:19 -0700)]
Fix another "make" versus "make dist" build problem.

14 years agoRemove "dpkg-buildpackage" test from "make distcheck".
Ben Pfaff [Tue, 9 Jun 2009 21:22:52 +0000 (14:22 -0700)]
Remove "dpkg-buildpackage" test from "make distcheck".

Now the Debian packaging is regularly tested via the autobuilder, so there
is less need to do it from "make distcheck", and not doing it saves time
there.