Justin Pettit [Wed, 14 Jan 2009 22:52:59 +0000 (14:52 -0800)]
Check wildcards for in_port != out_port output validation.
OpenFlow requires that traffic that is to be sent out the interface it
came in on use the OFPP_IN_PORT virtual port. The action validation
code that enforces this ignored the wildcards field, which meant it was
using the garbage 'in_port' value for this check.
Ben Pfaff [Fri, 9 Jan 2009 21:28:12 +0000 (13:28 -0800)]
Make dpctl accept an arbitrary number of actions.
This cleanup has been wanted for a while.
As a side effect, this change deletes some dead code pointed out by Chris
Eagle via Fortify: many of the deleted comparisons against act_len were
never true because of the sizes of the objects invovled.
Ben Pfaff [Fri, 9 Jan 2009 01:10:34 +0000 (17:10 -0800)]
netdev: Fix file descriptor leak.
This could be important since it leaks a file descriptor on every
netdev_open(), but only if an IPv6 address is configured on the network
device (which is rare and indicates an error condition for OpenFlow).
Justin Pettit [Fri, 9 Jan 2009 23:41:42 +0000 (15:41 -0800)]
Add datapath device name to printk's.
To aid debugging, this prints the datapath device name to printk
messages. Not doing this wasn't a big deal when only a single datapath
was running, but it's very confusing when there are multiple.
Ben Pfaff [Fri, 9 Jan 2009 23:18:36 +0000 (15:18 -0800)]
vswitchd: Delete 'ifaces' pointer to interface when deleting interface.
Otherwise we dereference a dangling pointer to the interface when we
look up the interface by datapath port index, causing a segfault.
Introduced in commit 150ac45, "vswitchd: Eliminate "can't forward to bad
port" when interfaces disappear," which deletes an interface that is known
to be in the datapath port index table.
Ben Pfaff [Fri, 9 Jan 2009 20:51:19 +0000 (12:51 -0800)]
vswitchd: Eliminate "can't forward to bad port" when interfaces disappear.
When an interface was deleted from a datapath by a process other than
vswitchd (which is not supposed to happen), vswitchd would not realize it
and would continue to set up flows for that interface (and leave in place
existing flows). This caused the kernel to complain "can't forward to bad
port" for each packet on these flows.
Xen triggered this by destroying vifs that were on vswitchd-controlled
datapaths (which removes them from any datapath that they are on).
This fixes the problem, by making vswitchd notice when interfaces
disappear and fixing up the flow table.
Justin Pettit [Fri, 9 Jan 2009 01:16:33 +0000 (17:16 -0800)]
Add support for sysfs and ethtool.
Add support for sysfs when the bridge compatibility module is running.
Currently, this only works for 2.6.18 kernels. Working on all kernels
should be fixed soon. Also, add ethtool support to the datapath device.
Ben Pfaff [Thu, 8 Jan 2009 22:58:35 +0000 (14:58 -0800)]
Remove 'wait' parameter from dpif_send_openflow().
The 'wait' parameter had no effect (it was not implemented) and it was
only ever set to false. It's easier to simply remove it than to
implement an unneeded feature.
Ben Pfaff [Thu, 8 Jan 2009 20:25:18 +0000 (12:25 -0800)]
datapath: Fix deadlock in switch port removal.
dp_del_switch_port() cancels the port_task and waits for
it to finish, but the port_task requires the RTNL lock
to complete, which dp_del_switch_port() holds, thus a
deadlock.
This commit fixes the problem, by deleting the port_task
entirely and moving its work into secchan.
Ben Pfaff [Thu, 8 Jan 2009 00:57:05 +0000 (16:57 -0800)]
datapath: Avoid deadlock on dp_mutex versus kthread_stop().
When a datapath is deleted, del_dp() acquires dp_mutex and uses
kthread_stop() to wait for the dp_task to die. Meanwhile, dp_task may
be waiting to acquire dp_mutex and won't die until it does so.
This commit fixes the problem by sending SIGKILL to dp_task before
call kthread_stop() and making dp_task give up if it receives a signal.
Ben Pfaff [Wed, 7 Jan 2009 23:19:09 +0000 (15:19 -0800)]
datapath: Fix deadlock in network device notifier.
When a network device that is part of a datapath is destroyed, the
notifier handler in dp_device_event() calls dp_del_switch_port() to
delete the switch port. dp_del_switch_port() then calls rtnl_lock()
to drop the device's promiscuous count. But the RTNL lock has already
been taken at this point, so taking it recursively deadlocks.
The obvious fix is for dp_del_switch_port()'s caller to take the RTNL
lock for it, if it is not already taken. This, however, causes a
different deadlock, as dp_del_switch_port() needs dp_mutex and that
means that we are nesting dp_mutex and the RTNL both ways.
The fix used here is to always nest dp_mutex inside the RTNL lock and
never vice versa.
=============================================
[ INFO: possible recursive locking detected ]
---------------------------------------------
tunctl/2934 is trying to acquire lock:
(rtnl_mutex){--..}, at: [<c029667d>] mutex_lock+0x1c/0x1f
but task is already holding lock:
(rtnl_mutex){--..}, at: [<c029667d>] mutex_lock+0x1c/0x1f
other info that might help us debug this:
1 lock held by tunctl/2934:
#0: (rtnl_mutex){--..}, at: [<c029667d>] mutex_lock+0x1c/0x1f
Problem identified with this script (in conjunction with the brcompat
module):
#! /bin/sh
brctl delbr a
brctl delbr b
brctl delbr c
set -x
stress () {
bridge=$1
shift
while true; do
brctl addbr $bridge; brctl show
for iface
do
case $iface in
tap*) tunctl -t $iface ;;
esac
brctl addif $bridge $iface
done
for iface
do
brctl delif $bridge $iface
case $iface in
tap*) tunctl -d $iface ;;
esac
done
brctl delbr $bridge; brctl show
done
}
stress a tap0 tap1 tap2 &
stress b tap3 tap4 tap5 &
trap 'killall stress-brctl' 0 SIGINT
wait
Ben Pfaff [Mon, 5 Jan 2009 18:52:57 +0000 (10:52 -0800)]
Prevent Debian package upgrades from prompting.
Before, we were using dpkg "conffiles" for our configuration. This means
that if the admin has customized the configuration (as is necessary to
make the switch work) and the default configuration included in the package
changed, then dpkg would prompt the user interactively about what to do.
Now, we take responsibility for creating, upgrading, and removing the
configuration file ourselves, and do not prompt.
Ben Pfaff [Sat, 3 Jan 2009 01:26:26 +0000 (17:26 -0800)]
vswitchd: Improve log options passed down to secchan.
Previously, secchan as started by vswitchd would always log to the console.
Now, it logs to the console only if any logging to the console is enabled
for vswitchd.
Previously, secchan as started by vswitchd would never log to a file. Now,
it logs to a file if vswitchd is configured to do so.
Also, use the bridge name in the vlog log pattern, instead of the datapath
number, so that users can more easily relate log messages to bridges.
Ben Pfaff [Sat, 3 Jan 2009 00:10:19 +0000 (16:10 -0800)]
Improve compatibility fallbacks for allocating multicast groups.
The linux-2.6 compatibility code for allocating multicast groups only
allocated a single multicast group per Generic Netlink family. However,
OpenFlow performance is going to be better if we allocate one per OpenFlow
datapath (which are all in the same Generic Netlink family). This commit
implements that.
Ben Pfaff [Sat, 3 Jan 2009 00:13:07 +0000 (16:13 -0800)]
Use separate Netlink multicast groups for different datapaths.
Otherwise every socket that listens for OpenFlow multicast messages will
get them for every single datapath. Not only is that a waste of memory
and time, it also allows one congested bridge to effectively DoS other
bridges that have little traffic.
Ben Pfaff [Fri, 2 Jan 2009 21:58:08 +0000 (13:58 -0800)]
Discard OpenFlow messages not for us in dpif_recv_openflow().
Currently there is only a single Netlink multicast group that is shared
by all OpenFlow datapaths. This is undesirable and should be fixed, but
a solution in every case is impossible due to Linux kernel limitations.
This commit works around the problem by dropping OpenFlow messages that
are related to a datapath that we are not interested in.
This should fix vswitchd behavior when more than one bridge is configured.
Thanks to Keith and Justin for diagnosing the problem.
Justin Pettit [Wed, 31 Dec 2008 18:55:13 +0000 (10:55 -0800)]
Return meaningful errors for brctl modification commands.
When datapaths and interfaces are modified, we now do more thorough
checks to see whether they will succeed or not. When datapaths are
added, we now block until they are created, so that follow-on ioctl
calls to attach interfaces will immediately work.
Justin Pettit [Wed, 31 Dec 2008 06:31:38 +0000 (22:31 -0800)]
Add support for "brctl show".
This makes datapaths and their interfaces show up as you'd expect when
"brctl show" is run. To get this functionality, you must insmod the
brcompat kernel module.