Ben Pfaff [Fri, 10 Oct 2008 17:13:12 +0000 (10:13 -0700)]
rconn: Be pickier about what constitutes a successful connection.
When secchan is configured to "fail open" after failing to connect to
a controller for a period of time, it needs a heuristic for what
constitutes a successful connection. Until now, that heuristic was
simply that when it received an OpenFlow message from the controller
(any OpenFlow message), it considered the connection successful.
However, this is no longer good enough, because NOX performs
admission control on connections after sending a number of OpenFlow
messages, in particular after doing OpenFlow version negotiation and
requesting the switch features (and receiving the reply). Thus, this
commit adjusts the heuristic by only considering certain OpenFlow
messages to demonstrate that admission control checks have passed and
thus that the connection should be considered successful.
As a fallback, any connection that persists for 30 seconds or longer is
also considered successful.
An alternate and complementary approach (that this commit does not
implement) would be to use an OpenFlow error message to indicate why
the connection is closing.
Ben Pfaff [Mon, 6 Oct 2008 23:25:56 +0000 (16:25 -0700)]
Fix use-after-free error.
rconn_destroy() decrements the n_queued counters that are set up by
rconn_send(), so we need to destroy the rconn before we destroy anything
that used it.
(This system is more error-prone than I imagined.)
Justin Pettit [Mon, 6 Oct 2008 22:51:06 +0000 (15:51 -0700)]
Unconditionally set skb->dev in dp_set_origin.
If an invalid port index was used in dp_set_origin, the value of skb->dev
was left unchanged. This change causes skb->dev to be set to NULL in those
circumstances. This makes Packet Out and Add Flow messages that use a
buffer id behave like Packet Out without a buffer id.
Justin Pettit [Mon, 6 Oct 2008 22:14:52 +0000 (15:14 -0700)]
Properly set in_port in skb for Flow Mod messages.
When a buffer id is placed in a Flow Mod message, the actions are expected to
be executed against the referenced packet. The kernel implementation was
not setting the input device to what the controller was telling it to use.
Thanks to Natasha for catching this.
Justin Pettit [Fri, 3 Oct 2008 23:44:13 +0000 (16:44 -0700)]
Add support for vendor-defined and variable-length actions.
Allow vendors to define their own actions. Actions were originally fixed-
length, which was a bit constraining. Actions now contain a length field,
which gives them more flexibility.
The tests haven't been seeing much love and are suffering from bit-rot at
this point. We're removing them, since we have other ways to test at
this point.
Modify VLAN actions to support setting both VID and priority.
Stripping VLANs is now done through the OFPAT_STRIP_VLAN action (i.e., you
don't specify a magic value in the generirc VLAN action). Also, it is now
possible to modify the priority bits associated with the VLAN tag through
the OFPAT_SET_VLAN_PCP action. The OFPAT_SET_DL_VLAN has been renamed to
OFPAT_SET_VLAN_VID.
Initial checkin of OpenFlow specification LaTeX source.
This version of the specification does not reflect the current checked in
version of OpenFlow. Checkins that follow should bring them in-line. The
TeX version of the spec was produced by Brandon Heller.
Ben Pfaff [Tue, 23 Sep 2008 18:24:30 +0000 (11:24 -0700)]
stp: Turn off STP on ports that are disabled or have no link.
Fixes a problem that a port that is disabled via "ifconfig down" does
not come back up properly when it is brought back up again with "ifconfig
up", if STP is enabled.
Modify OpenFlow commands related to ports to be more expressive.
This new OpenFlow message format provides a cleaner interface and greater
detail and control over ports. It is now possible to see what features
the switch's port is currently configured as having, what it's advertising,
and what it's capable of handling. It is also possible to return the
features advertised by the port's peer.
Ben Pfaff [Sat, 20 Sep 2008 00:02:30 +0000 (17:02 -0700)]
Get rid of per-port STP implemented bits, by reducing OFPP_MAX to 255.
802.1D Spanning Tree Protocol supports a maximum of 255 ports per bridge,
but OpenFlow supported 256, so 1 port had to not implement STP. This
gets rid of the problem by reducing the maximum number of ports to 255.
Use device notifier in Linux kernel switch for detecting port status changes.
To report on link status and whether a port is administratively enabled or
disabled, the reference switches poll. This change uses the Linux kernel's
device notification system, so that polling isn't necessary.
Ben Pfaff [Tue, 16 Sep 2008 23:56:37 +0000 (16:56 -0700)]
Make "controller nl:0" work again, by fixing a layering violation.
The kernel datapath was claiming that it implements STP, which is untrue.
In fact, the secchan implements STP, so if the secchan was not running
the STP support did not work. Thus, "controller nl:0" would do the
wrong thing.
This makes the secchan edit the responses from the datapath to claim
STP support.
This cleans up the handling of modifying existing flows. There now exists
OFPFC_MODIFY and OFPFC_MODIFY_STRICT commands, which have the same matching
characteristics to their OFPFC_DELETE* counterparts. When adding a flow that
already exists, the counters and timers are now reset (ie, its treated as a
new flow). This commit also adds the "--strict" option to dpctl to allow
strict matching for "mod-flows" and "del-flows".
Ben Pfaff [Mon, 15 Sep 2008 21:24:57 +0000 (14:24 -0700)]
Fix handling of port flags.
The "port watcher" in secchan was not keeping track of updates to port
flags by the controller, which caused some combinations of secchan
and controller to not turn on ports when the STP state otherwise should
have done so.
A recent change (696db2) modified the way stats were being sent over the
netlink socket. Unfortunately, the responder set the transaction id to the
type, which caused the requestor to ignore the repsone.
Actions are allocated as a pointer to an array of actions in flows. When
flows are freed, the code was always freeing the action pointer.
Under some circumstances the pointer may not be set.
Ben Pfaff [Thu, 11 Sep 2008 18:37:13 +0000 (11:37 -0700)]
Use vconn_init() instead of explicit initialization in one last place.
Commit b5e16b0723, "Respin "Make vconns keep track of their names and
include them in log messages,"" was supposed to do this everywhere, but
this instance was evidently missed.
Use new method to describe table entries in OpenFlow wire protocol.
Older versions of OpenFlow pre-defined a few different types of tables and the
Features Reply message would indicate how many of those entries existed. This
was very inflexible, so now the Features Reply just indicates how many tables
there are. The controller now sends a Table Stats request to find out how
many entries are supported and the fields that can be wildcarded.
This change also removes the "buffer_mb" field from the Features Reply, since
it was not being used and seemed to be of limited value.
Ben Pfaff [Tue, 9 Sep 2008 19:53:47 +0000 (12:53 -0700)]
Send of0 packets from workqueue, to avoid recursive locking of ofN device.
lockdep reported the following warning, showing that dev_queue_xmit()
for of0 and for another ethernet device (eth0?) were taking locks of
the same class. It was not a problem in this particular case, because
of0 != eth0, but if a flow were to send a packet back to of0 then we
would have a deadlock.
Solution: use a workqueue to send packets to avoid recursive locking.
We will still waste a lot of CPU time if we get a packet that loops back
to of0. Solution for that still needed.
=============================================
[ INFO: possible recursive locking detected ]
2.6.26.5 #1
---------------------------------------------
memcheck/1258 is trying to acquire lock:
(_xmit_ETHER){-+..}, at: [<c025b126>] __qdisc_run+0xa0/0x18a
but task is already holding lock:
(_xmit_ETHER){-+..}, at: [<c024f3cd>] dev_queue_xmit+0x23c/0x334
other info that might help us debug this:
9 locks held by memcheck/1258:
#0: (genl_mutex){--..}, at: [<c0262037>] genl_rcv+0x12/0x2b
#1: (dp_mutex){--..}, at: [<c88e9a13>] dp_genl_openflow+0x58/0x91 [openflow_mo
d]
#2: (rcu_read_lock){..--}, at: [<c024e749>] netif_receive_skb+0x9e/0x328
#3: (rcu_read_lock){..--}, at: [<c0267f77>] ip_local_deliver_finish+0x2a/0x1d7
#4: (slock-AF_INET/1){-+..}, at: [<c027f1b8>] tcp_v4_rcv+0x29f/0x5ef
#5: (rcu_read_lock){..--}, at: [<c024f308>] dev_queue_xmit+0x177/0x334
#6: (_xmit_ETHER){-+..}, at: [<c024f3cd>] dev_queue_xmit+0x23c/0x334
#7: (rcu_read_lock){..--}, at: [<c88ea214>] dp_dev_xmit+0x0/0x6c [openflow_mod
]
#8: (rcu_read_lock){..--}, at: [<c024f308>] dev_queue_xmit+0x177/0x334
Ben Pfaff [Sat, 6 Sep 2008 04:23:34 +0000 (21:23 -0700)]
Compatibility fixes for 2.6.18 kernel
Functions added in the 2.6.22 kernel in tcp.h and skbuff.h
have been added to the compatibility headers. Additionally
the data argument was dropped in 2.6.22 for the work queue
macros, requiring them to be redfined for previous kernels.
Ben Pfaff [Sat, 6 Sep 2008 00:09:53 +0000 (17:09 -0700)]
Do not send in-band control traffic to the controller.
This commit prevents the secure channel from sending any traffic to
or from the controller up to the controller over its OpenFlow connection.
Instead, such traffic is always handled completely locally. This is
what the changes to in-band control up until now have been converging on:
each time, we have reduced the category of traffic that can be sent up
to the controller because we have found that doing so can cause a control
loop. This change goes all the way.
projects / openvswitch / log