[ovs-dev] [PATCH 1/3] ofproto-dpif-upcall: Avoid unnecessarily installing datapath flows.

[Ben Pfaff]

handle_upcalls() always installed a flow for every packet, as long as
the datapath didn't already have too many flows, but there are cases where
we don't want to do this:

    - If we get multiple packets in a single microflow all in one batch
      (perhaps due to GSO breaking up a large TCP packet for sending to
      userspace, or for another reason), then we only need to install the
      datapath flow once.

    - For a slow-pathed flow received via a slow-path action in the kernel,
      we know that the kernel flow is already there (because otherwise it
      would have been received as "no match" instead of an action), so
      there is no benefit to reinstalling it.

Noticed because a CFM slow-pathed flow was getting reinstalled every time
a CFM packet was received.

Reported-by: Guolin Yang <gyang at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 ofproto/ofproto-dpif-upcall.c |   20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index dd24f5c..c3a1a03 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -208,6 +208,8 @@ struct flow_miss {
     struct odputil_keybuf mask_buf;
 
     struct xlate_out xout;
+
+    bool put;
 };
 
 static void upcall_destroy(struct upcall *);
@@ -971,6 +973,7 @@ handle_upcalls(struct handler *handler, struct list *upcalls)
                 miss->stats.used = time_msec();
                 miss->stats.tcp_flags = 0;
                 miss->odp_in_port = odp_in_port;
+                miss->put = false;
 
                 n_misses++;
             } else {
@@ -1107,10 +1110,23 @@ handle_upcalls(struct handler *handler, struct list *upcalls)
             miss->flow.vlan_tci = 0;
         }
 
-        if (may_put) {
+        /* Do not install a flow into the datapath if:
+         *
+         *    - The datapath already has too many flows.
+         *
+         *    - An earlier iteration of this loop already put the same flow.
+         *
+         *    - Slow-pathed flows only: we received this packet via some flow
+         *      installed in the kernel already. */
+        if (may_put
+            && !miss->put
+            && (!miss->xout.slow
+                || upcall->dpif_upcall.type == DPIF_UC_MISS)) {
             struct ofpbuf mask;
             bool megaflow;
 
+            miss->put = true;
+
             atomic_read(&enable_megaflows, &megaflow);
             ofpbuf_use_stack(&mask, &miss->mask_buf, sizeof miss->mask_buf);
             if (megaflow) {
-- 
1.7.10.4