[ovs-dev] [thread-safety 09/11] tunnel: Make the ofproto-dpif tunnel module thread safe.

Ethan Jackson ethan at nicira.com
Sat Jul 27 01:07:10 UTC 2013


Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
 ofproto/tunnel.c |  102 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 73 insertions(+), 29 deletions(-)

diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 4b7f304..97f7ef1 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -53,25 +53,36 @@ struct tnl_port {
     struct tnl_match match;
 };
 
-static struct hmap tnl_match_map = HMAP_INITIALIZER(&tnl_match_map);
-static struct hmap ofport_map = HMAP_INITIALIZER(&ofport_map);
+static struct ovs_rwlock rwlock = OVS_RWLOCK_INITIALIZER;
+
+static struct hmap tnl_match_map__ = HMAP_INITIALIZER(&tnl_match_map__);
+static struct hmap *tnl_match_map OVS_GUARDED_BY(rwlock) = &tnl_match_map__;
+
+static struct hmap ofport_map__ = HMAP_INITIALIZER(&ofport_map__);
+static struct hmap *ofport_map OVS_GUARDED_BY(rwlock) = &ofport_map__;
 
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 static struct vlog_rate_limit dbg_rl = VLOG_RATE_LIMIT_INIT(60, 60);
 
-static struct tnl_port *tnl_find(struct tnl_match *);
-static struct tnl_port *tnl_find_exact(struct tnl_match *);
-static struct tnl_port *tnl_find_ofport(const struct ofport_dpif *);
+static struct tnl_port *tnl_find(struct tnl_match *) OVS_REQ_RDLOCK(&rwlock);
+static struct tnl_port *tnl_find_exact(struct tnl_match *)
+    OVS_REQ_RDLOCK(&rwlock);
+static struct tnl_port *tnl_find_ofport(const struct ofport_dpif *)
+    OVS_REQ_RDLOCK(&rwlock);
 
 static uint32_t tnl_hash(struct tnl_match *);
 static void tnl_match_fmt(const struct tnl_match *, struct ds *);
-static char *tnl_port_fmt(const struct tnl_port *);
-static void tnl_port_mod_log(const struct tnl_port *, const char *action);
-static const char *tnl_port_get_name(const struct tnl_port *);
+static char *tnl_port_fmt(const struct tnl_port *) OVS_REQ_RDLOCK(&rwlock);
+static void tnl_port_mod_log(const struct tnl_port *, const char *action)
+    OVS_REQ_RDLOCK(&rwlock);
+static const char *tnl_port_get_name(const struct tnl_port *)
+    OVS_REQ_RDLOCK(&rwlock);
+static void tnl_port_del__(const struct ofport_dpif *) OVS_REQ_WRLOCK(&rwlock);
 
 static bool
 tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev,
                odp_port_t odp_port, bool warn)
+    OVS_REQ_WRLOCK(&rwlock)
 {
     const struct netdev_tunnel_config *cfg;
     struct tnl_port *existing_port;
@@ -108,8 +119,8 @@ tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev,
         return false;
     }
 
-    hmap_insert(&ofport_map, &tnl_port->ofport_node, hash_pointer(ofport, 0));
-    hmap_insert(&tnl_match_map, &tnl_port->match_node,
+    hmap_insert(ofport_map, &tnl_port->ofport_node, hash_pointer(ofport, 0));
+    hmap_insert(tnl_match_map, &tnl_port->match_node,
                 tnl_hash(&tnl_port->match));
     tnl_port_mod_log(tnl_port, "adding");
     return true;
@@ -122,7 +133,9 @@ void
 tnl_port_add(const struct ofport_dpif *ofport, const struct netdev *netdev,
              odp_port_t odp_port)
 {
+    ovs_rwlock_wrlock(&rwlock);
     tnl_port_add__(ofport, netdev, odp_port, true);
+    ovs_rwlock_unlock(&rwlock);
 }
 
 /* Checks if the tunnel represented by 'ofport' reconfiguration due to changes
@@ -133,36 +146,55 @@ bool
 tnl_port_reconfigure(const struct ofport_dpif *ofport,
                      const struct netdev *netdev, odp_port_t odp_port)
 {
-    struct tnl_port *tnl_port = tnl_find_ofport(ofport);
+    struct tnl_port *tnl_port;
+    bool changed = false;
 
+    ovs_rwlock_wrlock(&rwlock);
+    tnl_port = tnl_find_ofport(ofport);
     if (!tnl_port) {
-        return tnl_port_add__(ofport, netdev, odp_port, false);
+        changed = tnl_port_add__(ofport, netdev, odp_port, false);
     } else if (tnl_port->netdev != netdev
                || tnl_port->match.odp_port != odp_port
                || tnl_port->netdev_seq != netdev_change_seq(netdev)) {
         VLOG_DBG("reconfiguring %s", tnl_port_get_name(tnl_port));
-        tnl_port_del(ofport);
-        tnl_port_add(ofport, netdev, odp_port);
-        return true;
+        tnl_port_del__(ofport);
+        tnl_port_add__(ofport, netdev, odp_port, true);
+        changed = true;
     }
-    return false;
+    ovs_rwlock_unlock(&rwlock);
+    return changed;
 }
 
-/* Removes 'ofport' from the module. */
-void
-tnl_port_del(const struct ofport_dpif *ofport)
+static void
+tnl_port_del__(const struct ofport_dpif *ofport)
 {
-    struct tnl_port *tnl_port = ofport ? tnl_find_ofport(ofport) : NULL;
+    struct tnl_port *tnl_port;
 
+    if (!ofport) {
+        return;
+    }
+
+    tnl_port = tnl_find_ofport(ofport);
     if (tnl_port) {
         tnl_port_mod_log(tnl_port, "removing");
-        hmap_remove(&tnl_match_map, &tnl_port->match_node);
-        hmap_remove(&ofport_map, &tnl_port->ofport_node);
+        hmap_remove(tnl_match_map, &tnl_port->match_node);
+        hmap_remove(ofport_map, &tnl_port->ofport_node);
         netdev_close(tnl_port->netdev);
         free(tnl_port);
     }
 }
 
+/* Removes 'ofport' from the module. */
+void
+tnl_port_del(const struct ofport_dpif *ofport)
+{
+    struct tnl_port *tnl_port;
+
+    ovs_rwlock_wrlock(&rwlock);
+    tnl_port_del__(ofport);
+    ovs_rwlock_unlock(&rwlock);
+}
+
 /* Looks in the table of tunnels for a tunnel matching the metadata in 'flow'.
  * Returns the 'ofport' corresponding to the new in_port, or a null pointer if
  * none is found.
@@ -173,6 +205,7 @@ const struct ofport_dpif *
 tnl_port_receive(const struct flow *flow)
 {
     char *pre_flow_str = NULL;
+    const struct ofport_dpif *ofport;
     struct tnl_port *tnl_port;
     struct tnl_match match;
 
@@ -183,14 +216,16 @@ tnl_port_receive(const struct flow *flow)
     match.in_key = flow->tunnel.tun_id;
     match.skb_mark = flow->skb_mark;
 
+    ovs_rwlock_rdlock(&rwlock);
     tnl_port = tnl_find(&match);
+    ofport = tnl_port ? tnl_port->ofport : NULL;
     if (!tnl_port) {
         struct ds ds = DS_EMPTY_INITIALIZER;
 
         tnl_match_fmt(&match, &ds);
         VLOG_WARN_RL(&rl, "receive tunnel port not found (%s)", ds_cstr(&ds));
         ds_destroy(&ds);
-        return NULL;
+        goto out;
     }
 
     if (!VLOG_DROP_DBG(&dbg_rl)) {
@@ -209,7 +244,10 @@ tnl_port_receive(const struct flow *flow)
         free(pre_flow_str);
         free(post_flow_str);
     }
-    return tnl_port->ofport;
+
+out:
+    ovs_rwlock_unlock(&rwlock);
+    return ofport;
 }
 
 /* Given that 'flow' should be output to the ofport corresponding to
@@ -220,12 +258,16 @@ odp_port_t
 tnl_port_send(const struct ofport_dpif *ofport, struct flow *flow,
               struct flow_wildcards *wc)
 {
-    struct tnl_port *tnl_port = tnl_find_ofport(ofport);
     const struct netdev_tunnel_config *cfg;
+    struct tnl_port *tnl_port;
     char *pre_flow_str = NULL;
+    odp_port_t out_port;
 
+    ovs_rwlock_rdlock(&rwlock);
+    tnl_port = tnl_find_ofport(ofport);
+    out_port = tnl_port ?  tnl_port->match.odp_port : ODPP_NONE;
     if (!tnl_port) {
-        return ODPP_NONE;
+        goto out;
     }
 
     cfg = netdev_get_tunnel_config(tnl_port->netdev);
@@ -289,7 +331,9 @@ tnl_port_send(const struct ofport_dpif *ofport, struct flow *flow,
         free(post_flow_str);
     }
 
-    return tnl_port->match.odp_port;
+out:
+    ovs_rwlock_unlock(&rwlock);
+    return out_port;
 }
 
 static uint32_t
@@ -305,7 +349,7 @@ tnl_find_ofport(const struct ofport_dpif *ofport)
     struct tnl_port *tnl_port;
 
     HMAP_FOR_EACH_IN_BUCKET (tnl_port, ofport_node, hash_pointer(ofport, 0),
-                             &ofport_map) {
+                             ofport_map) {
         if (tnl_port->ofport == ofport) {
             return tnl_port;
         }
@@ -319,7 +363,7 @@ tnl_find_exact(struct tnl_match *match)
     struct tnl_port *tnl_port;
 
     HMAP_FOR_EACH_WITH_HASH (tnl_port, match_node, tnl_hash(match),
-                             &tnl_match_map) {
+                             tnl_match_map) {
         if (!memcmp(match, &tnl_port->match, sizeof *match)) {
             return tnl_port;
         }
-- 
1.7.9.5




More information about the dev mailing list