datapath: Detect and suppress flows that are implicated in loops.
authorBen Pfaff <blp@nicira.com>
Tue, 3 Aug 2010 21:40:29 +0000 (14:40 -0700)
committerBen Pfaff <blp@nicira.com>
Tue, 3 Aug 2010 21:40:29 +0000 (14:40 -0700)
commit55574bb0d21541c13fe67545a74448b36063e461
tree48afec5bc6184a1f18572e274a95dddf6c68b30e
parent9d2094938de6e6d6503a3d0777f64cff945321c3
datapath: Detect and suppress flows that are implicated in loops.

In-kernel loops need to be suppressed; otherwise, they cause high CPU
consumption, even to the point that the machine becomes unusable.  Ideally
these flows should never be added to the Open vSwitch flow table, but it
is fairly easy for a buggy controller to create them given the menagerie
of tunnels, patches, etc. that OVS makes available.

Commit ecbb6953b "datapath: Add loop checking" did the initial work
toward suppressing loops, by dropping packets that recursed more than 5
times.  This at least prevented the kernel stack from overflowing and
thereby OOPSing the machine.  But even with this commit, it is still
possible to waste a lot of CPU time due to loops.  The problem is not
limited to 5 recursive calls per packet: any packet can be sent to
multiple destinations, which in turn can themselves be sent to multiple
destinations, and so on.  We have actually seen in practice a case where
each packet was, apparently, sent to at least 2 destinations per hop, so
that each packet actually consumed CPU time for 2**5 == 32 packets,
possibly more.

This commit takes loop suppression a step further, by clearing the actions
of flows that are implicated in loops.  Thus, after the first packet in
such a flow, later packets for either the "root" flow or for flows that
it ends up looping through are simply discarded, saving a huge amount of
CPU time.

This version of the commit just clears the actions from the flows that a
part of the loop.  Probably, there should be some additional action to tell
ovs-vswitchd that a loop has been detected, so that it can in turn inform
the controller one way or another.

My test case was this:

ovs-controller -H --max-idle=permanent punix:/tmp/controller
ovs-vsctl -- \
    set-controller br0 unix:/tmp/controller -- \
    add-port br0 patch00 -- \
    add-port br0 patch01 -- \
    add-port br0 patch10 -- \
    add-port br0 patch11 -- \
    add-port br0 patch20 -- \
    add-port br0 patch21 -- \
    add-port br0 patch30 -- \
    add-port br0 patch31 -- \
    set Interface patch00 type=patch options:peer=patch01 -- \
    set Interface patch01 type=patch options:peer=patch00 -- \
    set Interface patch10 type=patch options:peer=patch11 -- \
    set Interface patch11 type=patch options:peer=patch10 -- \
    set Interface patch20 type=patch options:peer=patch21 -- \
    set Interface patch21 type=patch options:peer=patch20 -- \
    set Interface patch30 type=patch options:peer=patch31 -- \
    set Interface patch31 type=patch options:peer=patch30

followed by sending a single "ping" packet from an attached Ethernet
port into the bridge.  After this, without this commit the vswitch
userspace and kernel consume 50-75% of the machine's CPU (in my KVM
test setup on a single physical host); with this commit, some CPU is
consumed initially but it converges on 0% quickly.

A more challenging test sends a series of packets in multiple flows;
I used "hping3" with its default options.  Without this commit, the
vswitch consumes 100% of the machine's CPU, most of which is in the
kernel.  With this commit, the vswitch consumes "only" 33-50% CPU,
most of which is in userspace, so the machine is more responsive.

A refinement on this commit would be to pass the loop counter down to
userspace as part of the odp_msg struct and then back up as part of
the ODP_EXECUTE command arguments.  This would, presumably, reduce
the CPU requirements, since it would allow loop detection to happen
earlier, during initial setup of flows, instead of just on the second
and subsequent packets of flows.
datapath/datapath.c
datapath/vport.c