[ovs-dev] [PATCH 07/10] connmgr: Make connmgr_wants_packet_in_on_miss() lock-free.

[Jarno Rajahalme]

Make connmgr_wants_packet_in_on_miss() use an atomic int instead of a
list traversal taking the 'ofproto_mutex'.  This allows
connmgr_wants_packet_in_on_miss() to be called also when
'ofproto_mutex' is already held, and makes it faster, too.

Remove unused ofproto_dpif_wants_packet_in_on_miss().

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
---
 ofproto/connmgr.c      | 59 ++++++++++++++++++++++++++++++++++----------------
 ofproto/ofproto-dpif.c | 12 ----------
 ofproto/ofproto-dpif.h |  1 -
 3 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index d70d990..255e369 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -32,6 +32,7 @@
 #include "openvswitch/ofpbuf.h"
 #include "openvswitch/vconn.h"
 #include "openvswitch/vlog.h"
+#include "ovs-atomic.h"
 #include "pinsched.h"
 #include "poll-loop.h"
 #include "pktbuf.h"
@@ -212,6 +213,8 @@ struct connmgr {
     struct sockaddr_in *extra_in_band_remotes;
     size_t n_extra_remotes;
     int in_band_queue;
+
+    ATOMIC(int) want_packet_in_on_miss;
 };
 
 static void update_in_band_remotes(struct connmgr *);
@@ -251,6 +254,8 @@ connmgr_create(struct ofproto *ofproto,
     mgr->n_extra_remotes = 0;
     mgr->in_band_queue = -1;
 
+    atomic_init(&mgr->want_packet_in_on_miss, 0);
+
     return mgr;
 }
 
@@ -1212,6 +1217,32 @@ ofconn_get_target(const struct ofconn *ofconn)
     return rconn_get_target(ofconn->rconn);
 }
 
+/* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the
+ * packet rather than to send the packet to the controller.
+ *
+ * This function maintains the count of pre-OpenFlow1.3 with controller_id 0,
+ * as we assume these are the controllers that should receive "table-miss"
+ * notifications.
+ *
+ * This function must be called with 'add' = 'true' whenever a controller is
+ * added, and with 'add' = 'false' whenever a controller is removed.
+ */
+static void
+update_want_packet_in_on_miss(const struct ofconn *ofconn, bool add)
+{
+    if (ofconn->controller_id == 0) {
+        enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
+
+        if (protocol == OFPUTIL_P_NONE ||
+            ofputil_protocol_to_ofp_version(protocol) < OFP13_VERSION) {
+            int count;
+            atomic_read_relaxed(&ofconn->connmgr->want_packet_in_on_miss,
+                                &count);
+            atomic_store_relaxed(&ofconn->connmgr->want_packet_in_on_miss,
+                                 count + (add ? 1 : -1));
+        }
+    }
+}
 static struct ofconn *
 ofconn_create(struct connmgr *mgr, struct rconn *rconn, enum ofconn_type type,
               bool enable_async_msgs)
@@ -1232,6 +1263,8 @@ ofconn_create(struct connmgr *mgr, struct rconn *rconn, enum ofconn_type type,
 
     ofconn_flush(ofconn);
 
+    update_want_packet_in_on_miss(ofconn, true);
+
     return ofconn;
 }
 
@@ -1296,6 +1329,8 @@ ofconn_destroy(struct ofconn *ofconn)
 {
     ofconn_flush(ofconn);
 
+    update_want_packet_in_on_miss(ofconn, false);
+
     if (ofconn->type == OFCONN_PRIMARY) {
         hmap_remove(&ofconn->connmgr->controllers, &ofconn->hmap_node);
     }
@@ -1487,28 +1522,14 @@ ofconn_receives_async_msg(const struct ofconn *ofconn,
  * which connected using an OpenFlow version earlier than OpenFlow1.3.
  *
  * False otherwise.
- *
- * This logic assumes that "table-miss" packet_in messages
- * are always sent to controller_id 0. */
+ */
 bool
-connmgr_wants_packet_in_on_miss(struct connmgr *mgr) OVS_EXCLUDED(ofproto_mutex)
+connmgr_wants_packet_in_on_miss(struct connmgr *mgr)
 {
-    struct ofconn *ofconn;
-
-    ovs_mutex_lock(&ofproto_mutex);
-    LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
-        enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
+    int count;
 
-        if (ofconn->controller_id == 0 &&
-            (protocol == OFPUTIL_P_NONE ||
-             ofputil_protocol_to_ofp_version(protocol) < OFP13_VERSION)) {
-            ovs_mutex_unlock(&ofproto_mutex);
-            return true;
-        }
-    }
-    ovs_mutex_unlock(&ofproto_mutex);
-
-    return false;
+    atomic_read_relaxed(&mgr->want_packet_in_on_miss, &count);
+    return count > 0;
 }
 
 /* Returns a human-readable name for an OpenFlow connection between 'mgr' and
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index fd2c2bd..435db4e 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -379,18 +379,6 @@ ofproto_dpif_send_async_msg(struct ofproto_dpif *ofproto,
     /* Wakes up main thread for packet-in I/O. */
     seq_change(ofproto->ams_seq);
 }
-
-/* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the
- * packet rather than to send the packet to the controller.
- *
- * This function returns false to indicate that a packet_in message
- * for a "table-miss" should be sent to at least one controller.
- * False otherwise. */
-bool
-ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *ofproto)
-{
-    return connmgr_wants_packet_in_on_miss(ofproto->up.connmgr);
-}
 
 /* Factory functions. */
 
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index a3b1d6b..f912f6e 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -156,7 +156,6 @@ int ofproto_dpif_execute_actions__(struct ofproto_dpif *, const struct flow *,
                                    struct dp_packet *);
 void ofproto_dpif_send_async_msg(struct ofproto_dpif *,
                                  struct ofproto_async_msg *);
-bool ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *);
 int ofproto_dpif_send_packet(const struct ofport_dpif *, bool oam,
                              struct dp_packet *);
 void ofproto_dpif_flow_mod(struct ofproto_dpif *,
-- 
2.1.4