Ben Pfaff [Thu, 5 Mar 2009 01:32:53 +0000 (17:32 -0800)]
secchan: Flush datapath flow table when initializing the switch.
The switch can't recover the entire OpenFlow state from the datapath, so
it has the choice of either guessing at some missing information (which
seems undesirable) or just flushing the table. The latter is easier, so
do that.
Ben Pfaff [Thu, 5 Mar 2009 01:16:16 +0000 (17:16 -0800)]
classifier: Tolerate old==new in cls_rule_moved().
The primary purpose of cls_rule_moved() is to deal gracefully with memory
blocks that have been realloc()'d. realloc() can return the original
memory block so it's best to tolerate that instead of assert-failing.
Ben Pfaff [Thu, 5 Mar 2009 00:54:42 +0000 (16:54 -0800)]
secchan: Use classifier_for_each() instead of secchan_for_each_with_wildcards().
The classifier_for_each() function now provides what we actually needed
from secchan_for_each_with_wildcards(), and the interface is more sensible
to boot.
Ben Pfaff [Thu, 5 Mar 2009 00:48:39 +0000 (16:48 -0800)]
secchan: Fix random memory corruption due to uninitialized pointer.
The kernel returns flow stats and actions to userspace on flow deletion.
By not initializing the odp_flow's "actions" or "n_actions" members we
caused it to use whatever happened to be in that space on the stack, which
caused random memory corruption.
(There is no need to initialize the "stats" member, since it is not read,
only written, by the kernel, but by doing so we quiet valgrind.)
Ben Pfaff [Wed, 4 Mar 2009 23:08:57 +0000 (15:08 -0800)]
secchan: Fix segfault due to access-after-free in expiration.
classifier_for_each() keeps a pointer to the *next* rule to be visited, so
that the rule currently be visited can be deleted. That means that if
the callback frees the next rule to be visited, then we get an
access-after-free error.
In particular, this was occurring when expire_rule() expired a superflow
whose
Ben Pfaff [Wed, 4 Mar 2009 22:55:20 +0000 (14:55 -0800)]
secchan: Fix segfault when subrules are invalidated.
The subrules were being freed, but not removed from the classifier, so a
segfault would occur later when they were accessed during a lookup or
traversal.
Thanks to Dan and Natasha for the report and testcases.
Ben Pfaff [Wed, 4 Mar 2009 21:08:18 +0000 (13:08 -0800)]
classifier: In testing, don't put cls_rule at beginning of test_rule.
If we put cls_rule at the beginning of struct test_rule, then a cast is
sufficient to convert a pointer between the two, but we want to make sure
that we don't ever take that shortcut, because it is not valid in general.
Ben Pfaff [Wed, 4 Mar 2009 21:26:19 +0000 (13:26 -0800)]
classifier: Allow classifier_for_each_match() callback to free the rule.
classifier_for_each_match() would segfault if the callback passed in
deleted and freed the rule in question, because it accessed the rule after
calling the callback. This commit should fix the problem.
Ben Pfaff [Tue, 3 Mar 2009 22:24:12 +0000 (14:24 -0800)]
rconn: Make queued packet counting harder to screw up.
The semantics of the 'n_queued' parameter to rconn_send() and
rconn_send_with_limit() were too easy to screw up: if the memory area in
which the passed-in data lived was destroyed before the rconn was
destroyed, then rconn_destroy() (or simply flushing out the transmission
queue) would access invalid memory or, worse, decrement a random integer
in reused memory. It was possible to avoid this by destroying the rconn
before destroying the queue count data area, but this is difficult to
remember and not always possible in the general case.
This commit changes to using a reference-counted structure, which is harder
to get wrong.
Ben Pfaff [Fri, 27 Feb 2009 23:48:39 +0000 (15:48 -0800)]
vswitch: Fix connection to a remote controller.
Without this change, vswitchd will kill secchan almost as soon as it
starts it, because it fails to recognize that it is connecting to a remote
controller instead of to vswitchd.
Ben Pfaff [Mon, 2 Mar 2009 22:16:48 +0000 (14:16 -0800)]
secchan: Fix logging of datapath ID.
ofproto was logging the datapath ID passed in as part of its settings, but
that's allowed to be 0. Instead, it needs to log the datapath ID that is
actually in use.
Ben Pfaff [Mon, 2 Mar 2009 22:13:36 +0000 (14:13 -0800)]
vswitchd: Fix bad assumption about byte order of flow_t's "in_port".
In the big restructuring of secchan and the datapath, the "in_port"
member was changed from network byte order to host byte order, but vswitchd
hadn't quite caught up. This fixes the problem.
With this commit, at least the most basic use of vswitchd now works again.
Ben Pfaff [Mon, 2 Mar 2009 22:09:19 +0000 (14:09 -0800)]
vswitchd: Don't pass --monitor to secchan.
The --monitor option was deleted from secchan, because it was intended for
monitoring the OpenFlow connection between secchan and the kernel. Since
secchan no longer uses OpenFlow to talk to the kernel, the option made
no sense.
Ben Pfaff [Mon, 2 Mar 2009 21:42:44 +0000 (13:42 -0800)]
Refactor the OpenFlow implementation.
This new implementation has an architecture that is much more suited to
eventually getting pushed upstream into the Linux kernel, because it does
not do any OpenFlow processing in the kernel. Rather, we define a new
"datapath protocol" that secchan uses, via ioctl calls, to set up the
flow table in the kernel.
This implementation also should have much better performance with flows
that contain wildcards, since it uses a flow classifier that should be
much better than linear search in the cases that we suspect are important.
This release does contain some feature regressions; see the new file
MISSING at the root of the tree for more information. We will be fixing
these regressions over the next weeks and months.
This has not been tested much. It needs plenty of testing and QA before it
will be suitable for any kind of production environment. The vswitchd
changes, in particular, have not been tested at all and thus vswitchd is
likely to be broken.
Ben Pfaff [Mon, 2 Mar 2009 19:07:53 +0000 (11:07 -0800)]
vconn: Distinguish between parse errors and other messages in rate-limiting.
The vconn code wants to rate-limit errors, which there's not too much
point in reporting a lot of, from the log of all OpenFlow messages, which
are very important if we really want to log them at all. So use a
different rate-limiter for each category.
Ben Pfaff [Mon, 2 Mar 2009 18:47:26 +0000 (10:47 -0800)]
netdev: Don't cache network device features.
The new implementation of secchan wants to get updates of network device
features by keeping a network device open for each port and checking its
features when notified of a port status change. This wouldn't work, since
the features were cached once at startup. This commit makes the netdev
code check the actual devices features on each call.
Also, generalizes do_ethtool() to be useful for other kinds of ethtool
operations.
Ben Pfaff [Mon, 2 Mar 2009 20:43:47 +0000 (12:43 -0800)]
netdev: Avoid some system calls in the common case in netdev_open().
The new secchan opens one netdev per OpenFlow port. We should be able to
handle this in the common case without one file descriptor per netdev
(because most netdev operations can be performed using a single AF_INET
socket). This change starts along that path by moving the operations
that are required only to receive netdev packets out of the common path.
Justin Pettit [Fri, 13 Feb 2009 18:36:44 +0000 (10:36 -0800)]
Support multiple NetFlow collectors.
Add support for sending NetFlow messages to up to eight different
collectors. With these changes, secchan now reads configuration files
using the same syntax as vswitchd. This address Redmine feature #901.
Justin Pettit [Wed, 11 Feb 2009 23:23:23 +0000 (15:23 -0800)]
Check wildcards for in_port != out_port output validation. (udatapath)
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, 6 Feb 2009 23:07:38 +0000 (15:07 -0800)]
vswitchd: Avoid 100% CPU when secchan dies too many times.
vswitchd restarts secchan when necessary, but it limits the maximum number
of tries to avoid wasting CPU when secchan repeatedly dies. Unfortunately,
when this happens it also throws vswitchd into a busy-wait by calling
process_wait() on the dead secchan process, because it doesn't clear out
the process from the bridge structure.
This commit clears out the secchan process from the bridge structure, so
that we don't attempt to wait on it any longer, and should fix the busy-wait
problem.
Ben Pfaff [Wed, 4 Feb 2009 18:54:32 +0000 (10:54 -0800)]
poll-loop: Add support for logging the reason for wakeups.
It is useful to log the reason for wakeups, to debug why a program is
waking up more often than it should (for example, consuming 100% CPU load
for no apparent reason). This adds that logging at DBG level in the
poll loop.
Ben Pfaff [Tue, 3 Feb 2009 18:27:22 +0000 (10:27 -0800)]
Don't define skb_copy_{to,from}_linear_data_offset if it is available.
Linux 2.6.22 introduced functions skb_copy_from_linear_data_offset()
and skb_copy_to_linear_data_offset(). In earlier versions we defined them.
But Xen backports these functions, so this became a duplicate definition.
So check for them at configure time instead of depending on the kernel
version number.
Ben Pfaff [Mon, 2 Feb 2009 17:55:32 +0000 (09:55 -0800)]
datapath: Fix up checksum on Xen before forwarding to controller.
On Xen, the datapath can receive a packet that lacks a correct checksum
from a VM, because the VMs expect to use the host's hardware TX
checksumming. Until now, we haven't fixed up the checksum before we sent
the packet to the controller. The controller doesn't normally verify
the checksum (nor can it in general, since it doesn't necessarily get the
entire packet), so that part isn't a problem.
The problem here is in the buffered packet. fwd_save_skb() makes a copy
(not a clone) of the packet, but skb_copy() doesn't make a copy of the
skbuff's proto_csum_blank, which is what dev_queue_xmit() uses (via
skb_checksum_setup()) to decide whether checksumming needs to be forced.
Thus, the buffered packet is transmitted with a bad checksum.
A partial solution would be to copy proto_csum_blank from the original
skb into the buffered copy, or to make the buffers use clones instead of
copies (they really should do this anyhow). But this would still send
a bad checksum to the controller. So instead we do the full checksum
calculation before we send the packet to the controller.
This change affects only Xen. This situation cannot occur without Xen,
because any packets that arrive on physical interfaces must already have
correct checksums.