Implement serializing the state of packet traversal in "continuations".

One purpose of OpenFlow packet-in messages is to allow a controller to
interpose on the path of a packet through the flow tables.  If, for
example, the controller needs to modify a packet in some way that the
switch doesn't directly support, the controller should be able to
program the switch to send it the packet, then modify the packet and
send it back to the switch to continue through the flow table.

That's the theory.  In practice, this doesn't work with any but the
simplest flow tables.  Packet-in messages simply don't include enough
context to allow the flow table traversal to continue.  For example:

    * Via "resubmit" actions, an Open vSwitch packet can have an
      effective "call stack", but a packet-in can't describe it, and
      so it would be lost.

    * A packet-in can't preserve the stack used by NXAST_PUSH and
      NXAST_POP actions.

    * A packet-in can't preserve the OpenFlow 1.1+ action set.

    * A packet-in can't preserve the state of Open vSwitch mirroring
      or connection tracking.

This commit introduces a solution called "continuations".  A continuation
is the state of a packet's traversal through OpenFlow flow tables.  A
"controller" action with the "pause" flag, which is newly implemented in
this commit, generates a continuation and sends it to the OpenFlow
controller in a packet-in asynchronous message (only NXT_PACKET_IN2
supports continuations, so the controller must configure them with
NXT_SET_PACKET_IN_FORMAT).  The controller processes the packet-in,
possibly modifying some of its data, and sends it back to the switch with
an NXT_RESUME request, which causes flow table traversal to continue.  In
principle, a single packet can be paused and resumed multiple times.

Another way to look at it is:

    - "pause" is an extension of the existing OFPAT_CONTROLLER
      action.  It sends the packet to the controller, with full
      pipeline context (some of which is switch implementation
      dependent, and may thus vary from switch to switch).

    - A continuation is an extension of OFPT_PACKET_IN, allowing for
      implementation dependent metadata.

    - NXT_RESUME is an extension of OFPT_PACKET_OUT, with the
      semantics that the pipeline processing is continued with the
      original translation context from where it was left at the time
      it was paused.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>

2 files changed, 1 insertions, 60 deletions

diff --git a/ovn/TODO b/ovn/TODO
index a827421a6..b08a4f4d6 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -50,63 +50,6 @@ ovn-sb.xml includes a tentative specification for this action.
 IPv6 will probably need an action or actions for ND that is similar to
 the "arp" action, and an action for generating
 
-*** ovn-controller translation to OpenFlow
-
-The following two translation strategies come to mind.  Some of the
-new actions we might want to implement one way, some of them the
-other, depending on the details.
-
-*** Implementation strategies
-
-One way to do this is to define new actions as Open vSwitch extensions
-to OpenFlow, emit those actions in ovn-controller, and implement them
-in ovs-vswitchd (possibly pushing the implementations into the Linux
-and DPDK datapaths as well).  This is the only acceptable way for
-actions that need high performance.  None of these actions obviously
-need high performance, but it might be necessary to have fairness in
-handling e.g. a flood of incoming packets that require these actions.
-The main disadvantage of this approach is that it ties ovs-vswitchd
-(and the Linux kernel module) to supporting these actions essentially
-forever, which means that we'd want to make sure that they are
-general-purpose, well designed, maintainable, and supportable.
-
-The other way to do this is to send the packets across an OpenFlow
-channel to ovn-controller and have ovn-controller process them.  This
-is acceptable for actions that don't need high performance, and it
-means that we don't add anything permanently to ovs-vswitchd or the
-kernel (so we can be more casual about the design).  The big
-disadvantage is that it becomes necessary to add a way to resume the
-OpenFlow pipeline when it is interrupted in the middle by sending a
-packet to the controller.  This is not as simple as doing a new flow
-table lookup and resuming from that point.  Instead, it is equivalent
-to the (very complicated) recirculation logic in ofproto-dpif-xlate.c.
-Much of this logic can be translated into OpenFlow actions (e.g. the
-call stack and data stack), but some of it is entirely outside
-OpenFlow (e.g. the state of mirrors).  To implement it properly, it
-seems that we'll have to introduce a new Open vSwitch extension to
-OpenFlow, a "send-to-controller" action that causes extra data to be
-sent to the controller, where the extra data packages up the state
-necessary to resume the pipeline.  Maybe the bits of the state that
-can be represented in OpenFlow can be embedded in this extra data in a
-controller-readable form, but other bits we might want to be opaque.
-It's also likely that we'll want to change and extend the form of this
-opaque data over time, so this should be allowed for, e.g. by
-including a nonce in the extra data that is newly generated every time
-ovs-vswitchd starts.
-
-*** OpenFlow action definitions
-
-Define OpenFlow wire structures for each new OpenFlow action and
-implement them in lib/ofp-actions.[ch].
-
-*** OVS implementation
-
-Add code for action translation.  Possibly add datapath code for
-action implementation.  However, none of these new actions should
-require high-bandwidth processing so we could at least start with them
-implemented in userspace only.  (ARP field modification is already
-userspace-only and no one has complained yet.)
-
 ** IPv6
 
 *** ND versus ARP
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 97b7f6c03..9e944254e 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -75,10 +75,8 @@ process_packet_in(struct controller_ctx *ctx OVS_UNUSED,
                   const struct ofp_header *msg)
 {
     struct ofputil_packet_in pin;
-    uint32_t buffer_id;
-    size_t total_len;
 
-    if (ofputil_decode_packet_in(msg, &pin, &total_len, &buffer_id) != 0) {
+    if (ofputil_decode_packet_in(msg, true, &pin, NULL, NULL, NULL) != 0) {
         return;
     }
     if (pin.reason != OFPR_ACTION) {