ofproto-dpif: Don't slow-path controller actions.

Controller actions have become more commonly used for purposes other
than just making forwarding decisions (e.g., packet logging).  A packet
that needs to be copied to the controller and forwarded would always be
sent to ovs-vswitchd to be handled, which could negatively affect
performance and cause heavier CPU utilization in ovs-vswitchd.

This commit changes the behavior so that OpenFlow controller actions
become userspace datapath actions while continuing to let packet
forwarding and manipulation continue to be handled by the datapath
directly.

This patch still slow-paths controller actions with the "pause" flag
set.  A future patch will stop slow-pathing these pause actions as
well.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>

Documentation/tutorials/faucet.rst

@@ -620,9 +620,7 @@ trivial example::
 
   Final flow: unchanged
   Megaflow: recirc_id=0,eth,in_port=1,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=00:00:00:00:00:00,dl_type=0x0000
-  Datapath actions: push_vlan(vid=100,pcp=0),pop_vlan,2,3
-  This flow is handled by the userspace slow path because it:
-	  - Sends "packet-in" messages to the OpenFlow controller.
+  Datapath actions: push_vlan(vid=100,pcp=0),userspace(pid=0,controller(reason=1,flags=1,recirc_id=1,rule_cookie=0x5adc15c0,controller_id=0,max_len=96)),pop_vlan,2,3
 
 The first line of output, beginning with ``Flow:``, just repeats our
 request in a more verbose form, including the L2 fields that were
@@ -640,18 +638,7 @@ Summary information follows the numbered tables.  The packet hasn't
 been changed (overall, even though a VLAN was pushed and then popped
 back off) since ingress, hence ``Final flow: unchanged``.  We'll look
 at the ``Megaflow`` information later.  The ``Datapath actions``
-summarize what would actually happen to such a packet.  Finally, the
-note at the end gives a hint that this flow would not perform well for
-large volumes of traffic, because it has to be handled in the switch's
-slow path since it sends OpenFlow messages to the controller.
-
-.. note::
-
-  This performance limitation is probably not problematic in this case
-  because it is only used for MAC learning, so that most packets won't
-  encounter it.  However, the Open vSwitch 2.9 release (which is
-  upcoming as of this writing) will likely remove this performance
-  limitation anyway.
+summarize what would actually happen to such a packet.
 
 Triggering MAC Learning
 ~~~~~~~~~~~~~~~~~~~~~~~
@@ -753,18 +740,15 @@ without any further MAC learning, e.g.::
 Performance
 ~~~~~~~~~~~
 
-We've already seen one factor that can be important for performance:
-Open vSwitch forces any flow that sends a packet to an OpenFlow
-controller into its "slow path", which means that processing packets
-in the flow will be orders of magnitude slower than otherwise.  This
-distinction between "slow path" and "fast path" is the key to making
-sure that Open vSwitch performs as fast as possible.
-
-In addition to sending packets to a controller, some other factors can
-force a flow or a packet to take the slow path.  As one example, all
-CFM, BFD, LACP, STP, and LLDP processing takes place in the slow path,
-in the cases where Open vSwitch processes these protocols itself
-instead of delegating to controller-written flows.  As a second
+Open vSwitch has a concept of a "fast path" and a "slow path"; ideally
+all packets stay in the fast path.  This distinction between slow path
+and fast path is the key to making sure that Open vSwitch performs as
+fast as possible.
+
+Some factors can force a flow or a packet to take the slow path.  As one
+example, all CFM, BFD, LACP, STP, and LLDP processing takes place in the
+slow path, in the cases where Open vSwitch processes these protocols
+itself instead of delegating to controller-written flows.  As a second
 example, any flow that modifies ARP fields is processed in the slow
 path.  These are corner cases that are unlikely to cause performance
 problems in practice because these protocols send packets at a
@@ -1142,9 +1126,7 @@ this::
 
   Final flow: udp,in_port=1,dl_vlan=100,dl_vlan_pcp=0,vlan_tci1=0x0000,dl_src=00:01:02:03:04:05,dl_dst=0e:00:00:00:00:01,nw_src=10.100.0.1,nw_dst=10.200.0.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=0
   Megaflow: recirc_id=0,eth,ip,in_port=1,vlan_tci=0x0000/0x1fff,dl_src=00:01:02:03:04:05,dl_dst=0e:00:00:00:00:01,nw_dst=10.200.0.0/25,nw_frag=no
-  Datapath actions: push_vlan(vid=100,pcp=0)
-  This flow is handled by the userspace slow path because it:
-	  - Sends "packet-in" messages to the OpenFlow controller.
+  Datapath actions: push_vlan(vid=100,pcp=0),userspace(pid=0,controller(reason=1,flags=0,recirc_id=6,rule_cookie=0x5adc15c0,controller_id=0,max_len=128))
 
 Observe that the packet gets recognized as destined to the router, in
 table 3, and then as properly destined to the 10.200.0.0/24 network,
@@ -1201,9 +1183,7 @@ reply::
 
   Final flow: arp,in_port=4,dl_vlan=200,dl_vlan_pcp=0,vlan_tci1=0x0000,dl_src=00:10:20:30:40:50,dl_dst=0e:00:00:00:00:01,arp_spa=10.200.0.1,arp_tpa=10.200.0.254,arp_op=2,arp_sha=00:10:20:30:40:50,arp_tha=0e:00:00:00:00:01
   Megaflow: recirc_id=0,eth,arp,in_port=4,vlan_tci=0x0000/0x1fff,dl_dst=0e:00:00:00:00:01,arp_tpa=10.200.0.254
-  Datapath actions: push_vlan(vid=200,pcp=0)
-  This flow is handled by the userspace slow path because it:
-	  - Sends "packet-in" messages to the OpenFlow controller.
+  Datapath actions: push_vlan(vid=200,pcp=0),userspace(pid=0,controller(reason=1,flags=0,recirc_id=7,rule_cookie=0x5adc15c0,controller_id=0,max_len=128))
 
 It shows up in ``inst/faucet.log``::
 

NEWS

@@ -10,6 +10,9 @@ Post-v2.8.0
      * ovsdb-tool: New "db-name" and "schema-name" commands.
    - ovs-vsctl and other commands that display data in tables now support a
      --max-column-width option to limit column width.
+   - No longer slow-path traffic that sends to a controller.  Applications,
+     such as OVN ACL logging, want to send a copy of a packet to a
+     controller while leaving the actual packet forwarding in the datapath.
    - OVN:
      * The "requested-chassis" option for a logical switch port now accepts a
        chassis "hostname" in addition to a chassis "name".

lib/odp-util.c

@@ -478,6 +478,21 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr,
                 odp_portno_name_format(portno_names,
                                        cookie.ipfix.output_odp_port, ds);
                 ds_put_char(ds, ')');
+            } else if (cookie.type == USER_ACTION_COOKIE_CONTROLLER) {
+                ds_put_format(ds, ",controller(reason=%"PRIu16
+                              ",dont_send=%"PRIu8
+                              ",recirc_id=%"PRIu32
+                              ",rule_cookie=%#"PRIx64
+                              ",controller_id=%"PRIu16
+                              ",max_len=%"PRIu16,
+                              cookie.controller.reason,
+                              cookie.controller.dont_send ? 1 : 0,
+                              cookie.controller.recirc_id,
+                              ntohll(get_32aligned_be64(
+                                         &cookie.controller.rule_cookie)),
+                              cookie.controller.controller_id,
+                              cookie.controller.max_len);
+                ds_put_char(ds, ')');
             } else {
                 userdata_unspec = true;
             }
@@ -1128,6 +1143,15 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
         uint32_t collector_set_id;
         uint32_t obs_domain_id;
         uint32_t obs_point_id;
+
+        /* USER_ACTION_COOKIE_CONTROLLER. */
+        uint8_t dont_send;
+        uint16_t reason;
+        uint32_t recirc_id;
+        ovs_be64 rule_cookie;
+        uint16_t controller_id;
+        uint16_t max_len;
+
         int vid, pcp;
         int n1 = -1;
         if (ovs_scan(&s[n], ",sFlow(vid=%i,"
@@ -1201,8 +1225,26 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
             cookie.ofp_in_port = OFPP_NONE;
             cookie.ofproto_uuid = UUID_ZERO;
             cookie.ipfix.output_odp_port = u32_to_odp(output);
-        } else if (ovs_scan(&s[n], ",userdata(%n",
-                            &n1)) {
+        } else if (ovs_scan(&s[n], ",controller(reason=%"SCNu16
+                              ",dont_send=%"SCNu8
+                              ",recirc_id=%"SCNu32
+                              ",rule_cookie=%"SCNx64
+                              ",controller_id=%"SCNu16
+                              ",max_len=%"SCNu16")%n",
+                              &reason, &dont_send, &recirc_id, &rule_cookie,
+                              &controller_id, &max_len, &n1)) {
+            n += n1;
+            cookie.type = USER_ACTION_COOKIE_CONTROLLER;
+            cookie.ofp_in_port = OFPP_NONE;
+            cookie.ofproto_uuid = UUID_ZERO;
+            cookie.controller.dont_send = dont_send ? true : false;
+            cookie.controller.reason = reason;
+            cookie.controller.recirc_id = recirc_id;
+            put_32aligned_be64(&cookie.controller.rule_cookie,
+                               htonll(rule_cookie));
+            cookie.controller.controller_id = controller_id;
+            cookie.controller.max_len = max_len;
+       } else if (ovs_scan(&s[n], ",userdata(%n", &n1)) {
             char *end;
 
             n += n1;
@@ -3266,7 +3308,7 @@ format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr,
         case OVS_TUNNEL_KEY_ATTR_ID:
             format_be64(ds, "tun_id", nl_attr_get_be64(a),
                         ma ? nl_attr_get(ma) : NULL, verbose);
-	    flags |= FLOW_TNL_F_KEY;
+            flags |= FLOW_TNL_F_KEY;
             if (ma) {
                 mask_flags |= FLOW_TNL_F_KEY;
             }
@@ -3302,10 +3344,10 @@ format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr,
                        ma ? nl_attr_get(ma) : NULL, verbose);
             break;
         case OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT:
-	    flags |= FLOW_TNL_F_DONT_FRAGMENT;
+            flags |= FLOW_TNL_F_DONT_FRAGMENT;
             break;
         case OVS_TUNNEL_KEY_ATTR_CSUM:
-	    flags |= FLOW_TNL_F_CSUM;
+            flags |= FLOW_TNL_F_CSUM;
             break;
         case OVS_TUNNEL_KEY_ATTR_TP_SRC:
             format_be16(ds, "tp_src", nl_attr_get_be16(a),
@@ -3316,7 +3358,7 @@ format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr,
                         ma ? nl_attr_get(ma) : NULL, verbose);
             break;
         case OVS_TUNNEL_KEY_ATTR_OAM:
-	    flags |= FLOW_TNL_F_OAM;
+            flags |= FLOW_TNL_F_OAM;
             break;
         case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS:
             ds_put_cstr(ds, "vxlan(");

lib/odp-util.h

@@ -42,9 +42,8 @@ struct pkt_metadata;
     SPR(SLOW_BFD,        "bfd",        "Consists of BFD packets")       \
     SPR(SLOW_LACP,       "lacp",       "Consists of LACP packets")      \
     SPR(SLOW_STP,        "stp",        "Consists of STP packets")       \
-    SPR(SLOW_LLDP,       "lldp",       "Consists of LLDP packets")    \
-    SPR(SLOW_CONTROLLER, "controller",                                  \
-        "Sends \"packet-in\" messages to the OpenFlow controller")      \
+    SPR(SLOW_LLDP,       "lldp",       "Consists of LLDP packets")      \
+    SPR(SLOW_PAUSE,      "pause",      "Controller action with pause")  \
     SPR(SLOW_ACTION,     "action",                                      \
         "Uses action(s) not supported by datapath")
 
@@ -298,6 +297,7 @@ enum user_action_cookie_type {
     USER_ACTION_COOKIE_SLOW_PATH,    /* Userspace must process this flow. */
     USER_ACTION_COOKIE_FLOW_SAMPLE,  /* Packet for per-flow sampling. */
     USER_ACTION_COOKIE_IPFIX,        /* Packet for per-bridge IPFIX sampling. */
+    USER_ACTION_COOKIE_CONTROLLER,   /* Forward packet to controller. */
 };
 
 /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */
@@ -333,6 +333,16 @@ struct user_action_cookie {
             /* USER_ACTION_COOKIE_IPFIX. */
             odp_port_t output_odp_port; /* The output odp port. */
         } ipfix;
+
+        struct {
+            /* USER_ACTION_COOKIE_CONTROLLER. */
+            bool dont_send;         /* Don't send the packet to controller. */
+            uint16_t reason;
+            uint32_t recirc_id;
+            ovs_32aligned_be64 rule_cookie;
+            uint16_t controller_id;
+            uint16_t max_len;
+        } controller;
     };
 };
 BUILD_ASSERT_DECL(sizeof(struct user_action_cookie) == 48);

ofproto/ofproto-dpif-rid.c

@@ -142,6 +142,9 @@ frozen_state_hash(const struct frozen_state *state)
         hash = hash_bytes64(ALIGNED_CAST(const uint64_t *, state->ofpacts),
                             state->ofpacts_len, hash);
     }
+    if (state->userdata && state->userdata_len) {
+        hash = hash_bytes(state->userdata, state->userdata_len, hash);
+    }
     return hash;
 }
 
@@ -158,7 +161,8 @@ frozen_state_equal(const struct frozen_state *a, const struct frozen_state *b)
             && ofpacts_equal(a->ofpacts, a->ofpacts_len,
                              b->ofpacts, b->ofpacts_len)
             && ofpacts_equal(a->action_set, a->action_set_len,
-                             b->action_set, b->action_set_len));
+                             b->action_set, b->action_set_len)
+            && !memcmp(a->userdata, b->userdata, a->userdata_len));
 }
 
 /* Lockless RCU protected lookup.  If node is needed accross RCU quiescent
@@ -203,6 +207,9 @@ frozen_state_clone(struct frozen_state *new, const struct frozen_state *old)
     new->action_set = (new->action_set_len
                        ? xmemdup(new->action_set, new->action_set_len)
                        : NULL);
+    new->userdata = (new->userdata_len
+                     ? xmemdup(new->userdata, new->userdata_len)
+                     : NULL);
 }
 
 static void
@@ -211,6 +218,7 @@ frozen_state_free(struct frozen_state *state)
     free(state->stack);
     free(state->ofpacts);
     free(state->action_set);
+    free(state->userdata);
 }
 
 /* Allocate a unique recirculation id for the given set of flow metadata.

ofproto/ofproto-dpif-rid.h

@@ -149,6 +149,10 @@ struct frozen_state {
     size_t ofpacts_len;           /* Size of 'ofpacts', in bytes. */
     struct ofpact *action_set;
     size_t action_set_len;        /* Size of 'action_set', in bytes. */
+
+    /* User data for controller userspace cookie. */
+    uint8_t *userdata;
+    size_t userdata_len;
 };
 
 /* This maps a recirculation ID to saved state that flow translation can