Ben Pfaff [Tue, 10 Mar 2009 23:54:00 +0000 (16:54 -0700)]
secchan: In ofproto_set_controller(), only reconnect if controller really changed.
The vswitchd bridge code tries not to call this function if it doesn't
have to, but it's not doing a good job. We should fix it in vswitchd, but
it's also a good idea to do it here.
Ben Pfaff [Tue, 10 Mar 2009 21:04:16 +0000 (14:04 -0700)]
vswitch: Work in terms of ODP port numbers.
The vswitch used to be an OpenFlow client, so all of its internals were in
terms of OpenFlow port numbering. When it was converted to be a datapath
client instead, a shim layer that converted between OpenFlow and ODP port
numbering was inserted, so that it could still work internally in terms of
OpenFlow port numbers.
This commit makes the vswitch use ODP port numbering internally, removing
this shim layer.
Ben Pfaff [Mon, 9 Mar 2009 18:28:01 +0000 (11:28 -0700)]
secchan: Keep track of ofproto even when translating nested actions.
Allowing vswitch to hook OFPP_NORMAL will require nested actions (via
NXAST_RESUBMIT) to access the ofproto, so we need to be able to get to it
in that case.
Ben Pfaff [Mon, 9 Mar 2009 22:37:16 +0000 (15:37 -0700)]
vswitch: Eliminate OpenFlow connection to ofproto.
The vswitch interface to the ofproto is now entirely a functional
interface, instead of going partially over an OpenFlow connection that
"loops back" to the ofproto via a socketpair. So this commit drops
the code that connects and maintains that connection.
Ben Pfaff [Mon, 9 Mar 2009 21:12:25 +0000 (14:12 -0700)]
secchan: Make netflow expiration arguments more sensible.
This cleanup is useful preparation for adding a flow expiration hook for
vswitchd to use, since that hook wants to receive essentially the same
information.
Ben Pfaff [Mon, 9 Mar 2009 18:07:06 +0000 (11:07 -0700)]
secchan: Retain original input port for NXAST_RESUBMIT action.
The NXAST_RESUBMIT action causes a secondary lookup in the flow table with
a different input port value. However, we want to retain the original
input port for processing the result of that secondary lookup. Otherwise,
output to OFPP_IN_PORT will send the packet to the wrong port, and output
to the input port without specifying OFPP_IN_PORT will be mistakenly
allowed.
Ben Pfaff [Fri, 6 Mar 2009 22:34:46 +0000 (14:34 -0800)]
Keep secchan and vswitchd from consuming 100% CPU when a datapath is deleted.
Before this commit, "dpctl deldp x" would cause secchan or vswitchd to
consume 100% CPU if they were responsible for the given datapath. This
fixes the problem.
Ben Pfaff [Fri, 6 Mar 2009 22:12:36 +0000 (14:12 -0800)]
vswitch: Keep existing datapaths when starting up.
Until now, vswitchd has deleted all existing datapaths when it started up,
and then re-created the ones that are actually configured. This is a very
"clean", conservative design, but it has undesirable effects in practice.
In particular, if any datapath has a local port with an IP address
configured on it, then deleting the datapath and recreating it will drop
that IP address, which can mean that your machine just lost connectivity
to the outside world.
So, with this commit, now vswitchd only deletes datapaths at startup that
don't have configured bridges, preserving local port IP addresses.
Ben Pfaff [Fri, 6 Mar 2009 22:03:24 +0000 (14:03 -0800)]
dpif: New function dpif_get_name().
This function is equivalent to querying the ODPP_LOCAL port, but its name
better reflects the caller's intent, and its interface is slightly more
convenient.
Ben Pfaff [Fri, 6 Mar 2009 21:30:09 +0000 (13:30 -0800)]
flow: Properly translate port numbers in flow_from_match().
The OpenFlow and datapath port numbers for the local port are different.
We were translating them in one direction (in flow_to_match()) but not
in the other (in flow_from_match()).
This fixes a performance problem (we were setting up the wrong flow).
Ben Pfaff [Fri, 6 Mar 2009 17:52:08 +0000 (09:52 -0800)]
secchan: Fix use-after-free by allocating rule actions as separate blocks.
The ofproto code tried to cleverly save memory and time by allocating
rule actions as a flexible array member at the end of "struct rule". When
the actions changed, this required a realloc() call. Unfortunately, there
are sometimes pointers to rules (e.g. the "super" pointer from subrules)
that were not getting adjusted to point to the new location.
It's better to just allocate actions separately, so fix it by doing that.
Ben Pfaff [Fri, 6 Mar 2009 01:34:42 +0000 (17:34 -0800)]
secchan: Correctly maintain rule's number of actions.
We were reallocating memory properly but not updating the "n_actions"
member, which sometimes led to reading past the end of a block.
This problem possibly caused segfaults, since the OpenFlow->ODP action
translation code assumes that OpenFlow actions are prevalidated, and
random memory is not necessarily valid OpenFlow actions :-)
Ben Pfaff [Thu, 5 Mar 2009 23:09:00 +0000 (15:09 -0800)]
netdev: Fall back to /proc/net/dev on kernels that don't support RTM_GETLINK.
RTM_GETLINK is the best way to get network device statistics, but it was
only enabled in all kernels in 2.6.19. So now test for support at startup
and fall back to reading /proc/net/dev if RTM_GETLINK does not work.
Ben Pfaff [Thu, 5 Mar 2009 20:30:36 +0000 (12:30 -0800)]
vswitchd: Integrate secchan into vswitchd.
This change means that vswitchd is now a single process that manages all
of the datapaths, instead of a parent process and any number of secchan
subprocesses.
It may be better to fork off processes that each manage a few bridges, to
take advantage of multiple cores. Perhaps this is upcoming.
There are numerous new configuration file directives that are, sadly, not
yet documented.
Ben Pfaff [Thu, 5 Mar 2009 19:13:49 +0000 (11:13 -0800)]
cfg: Fix behavior of cfg_get(0, "a") when a key "a.b" exists.
The intent of cfg_get_*(0, "a") is to get the first value of a key with
the exact name "a", but in the presence of a key with a longer name, e.g.
"a.b", it would return the value of that key instead.
This file really needs a unit test! I'm really not certain that
the fix didn't break other things (e.g. cfg_has_section()).
Ben Pfaff [Thu, 5 Mar 2009 19:01:45 +0000 (11:01 -0800)]
dpif: Don't rely on caller to keep dpif arg valid, in dpifmon_create().
dpifmon_create() stored away the dpif pointer that it was passed and
continued to use it, but ofproto_create() didn't keep that dpif in a
constant place in memory, so dpifmon_poll() would randomly fail with
errors like "Bad file descriptor". Having the dpifmon keep its own
fd to a dpif is more reliable.
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.