[ovs-dev] [PATCH 3/3] ofproto-dpif-xlate: Implement RCU locking in ofproto-dpif-xlate.

Ryan Wilson wryan at nicira.com
Fri May 9 09:34:12 UTC 2014


Before, a global read-write lock protected the ofproto-dpif / ofproto-dpif-xlate
interface. Handler and revalidator threads had to wait while configuration was
being changed. This patch implements RCU locking which allows handlers and
revalidators to operate while configuration is being updated.

Signed-off-by: Ryan Wilson <wryan at nicira.com>
---
v2: Split 1 commit into 3:
  - netdev: Free netdev with ovsrcu_postpone.
  - ovs-rcu: Add OVSRCU_TYPE_INITIALIZER which initializes OVSRCU_TYPE variables
    to NULL.
  - ofproto-dpif-xlate: Implement RCU locking in ofproto-dpif-xlate.
---
 ofproto/ofproto-dpif-xlate.c |  546 ++++++++++++++++++++++++++++++++----------
 ofproto/ofproto-dpif-xlate.h |   24 +-
 ofproto/ofproto-dpif.c       |   16 +-
 ofproto/ofproto-dpif.h       |    5 +-
 4 files changed, 439 insertions(+), 152 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index da538b7..8f33a9b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -65,8 +65,6 @@ VLOG_DEFINE_THIS_MODULE(ofproto_dpif_xlate);
  * recursive or not. */
 #define MAX_RESUBMITS (MAX_RESUBMIT_RECURSION * MAX_RESUBMIT_RECURSION)
 
-struct ovs_rwlock xlate_rwlock = OVS_RWLOCK_INITIALIZER;
-
 struct xbridge {
     struct hmap_node hmap_node;   /* Node in global 'xbridges' map. */
     struct ofproto_dpif *ofproto; /* Key in global 'xbridges' map. */
@@ -292,15 +290,22 @@ struct xlate_cache {
     struct ofpbuf entries;
 };
 
-static struct hmap xbridges = HMAP_INITIALIZER(&xbridges);
-static struct hmap xbundles = HMAP_INITIALIZER(&xbundles);
-static struct hmap xports = HMAP_INITIALIZER(&xports);
+/* Xlate config contains hash maps of all bridges, bundles and ports.
+ * Xcfgp contains the pointer to the current xlate configuration.
+ * When the main thread needs to change the configuration, it copies xcfgp to
+ * new_xcfg and edits new_xcfg. This enables the use of RCU locking which
+ * does not block handler and revalidator threads. */
+struct xlate_cfg {
+    struct hmap xbridges;
+    struct hmap xbundles;
+    struct hmap xports;
+};
+OVSRCU_TYPE(struct xlate_cfg *) xcfgp = OVSRCU_TYPE_INITIALIZER;
+struct xlate_cfg *new_xcfg = NULL;
 
 static bool may_receive(const struct xport *, struct xlate_ctx *);
 static void do_xlate_actions(const struct ofpact *, size_t ofpacts_len,
                              struct xlate_ctx *);
-static void xlate_actions__(struct xlate_in *, struct xlate_out *)
-    OVS_REQ_RDLOCK(xlate_rwlock);
 static void xlate_normal(struct xlate_ctx *);
 static void xlate_report(struct xlate_ctx *, const char *);
 static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
@@ -312,9 +317,12 @@ static void output_normal(struct xlate_ctx *, const struct xbundle *,
                           uint16_t vlan);
 static void compose_output_action(struct xlate_ctx *, ofp_port_t ofp_port);
 
-static struct xbridge *xbridge_lookup(const struct ofproto_dpif *);
-static struct xbundle *xbundle_lookup(const struct ofbundle *);
-static struct xport *xport_lookup(const struct ofport_dpif *);
+static struct xbridge *xbridge_lookup(const struct ofproto_dpif *,
+                                      struct xlate_cfg *);
+static struct xbundle *xbundle_lookup(const struct ofbundle *,
+                                      struct xlate_cfg *);
+static struct xport *xport_lookup(const struct ofport_dpif *,
+                                  struct xlate_cfg *);
 static struct xport *get_ofp_port(const struct xbridge *, ofp_port_t ofp_port);
 static struct skb_priority_to_dscp *get_skb_priority(const struct xport *,
                                                      uint32_t skb_priority);
@@ -324,10 +332,75 @@ static bool dscp_from_skb_priority(const struct xport *, uint32_t skb_priority,
 
 static struct xc_entry *xlate_cache_add_entry(struct xlate_cache *xc,
                                               enum xc_type type);
+static void xlate_xbridge_init(struct xbridge *);
+static void xlate_xbundle_init(struct xbundle *);
+static void xlate_xport_init(struct xport *);
+static void xlate_xbridge_set(struct xbridge *xbridge,
+                              const char *name, struct dpif *dpif,
+                              struct rule_dpif *miss_rule,
+                              struct rule_dpif *no_packet_in_rule,
+                              const struct mac_learning *ml, struct stp *stp,
+                              const struct mbridge *mbridge,
+                              const struct dpif_sflow *sflow,
+                              const struct dpif_ipfix *ipfix,
+                              const struct netflow *netflow,
+                              enum ofp_config_flags frag,
+                              bool forward_bpdu, bool has_in_band,
+                              bool enable_recirc,
+                              bool variable_length_userdata,
+                              size_t max_mpls_depth);
+static void xlate_xbundle_set(struct xbundle *xbundle, const char *name,
+                              enum port_vlan_mode vlan_mode, int vlan,
+                              unsigned long *trunks, bool use_priority_tags,
+                              const struct bond *bond, const struct lacp *lacp,
+                              bool floodable);
+static void xlate_xport_set(struct xport *xport, odp_port_t odp_port,
+                            const struct cfm *cfm, const struct bfd *bfd,
+                            int stp_port_no, enum ofputil_port_config config,
+                            enum ofputil_port_state state, bool is_tunnel,
+                            bool may_enable);
+static void xlate_xbridge_remove(struct xbridge *, struct xlate_cfg *xcfg);
+static void xlate_xbundle_remove(struct xbundle *, struct xlate_cfg *xcfg);
+static void xlate_xport_remove(struct xport *, struct xlate_cfg *xcfg);
+static void xlate_xbridge_copy(struct xbridge *);
+static void xlate_xbundle_copy(struct xbridge *, struct xbundle *);
+static void xlate_xport_copy(struct xbridge *, struct xbundle *,
+                             struct xport *);
+static void xlate_xcfg_free(struct xlate_cfg *);
 
-void
-xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
-                  struct dpif *dpif, struct rule_dpif *miss_rule,
+
+static void
+xlate_xbridge_init(struct xbridge *xbridge)
+{
+    list_init(&xbridge->xbundles);
+    hmap_init(&xbridge->xports);
+    hmap_insert(&new_xcfg->xbridges, &xbridge->hmap_node,
+                hash_pointer(xbridge->ofproto, 0));
+}
+
+static void
+xlate_xbundle_init(struct xbundle *xbundle)
+{
+    list_init(&xbundle->xports);
+    list_insert(&xbundle->xbridge->xbundles, &xbundle->list_node);
+    hmap_insert(&new_xcfg->xbundles, &xbundle->hmap_node,
+                hash_pointer(xbundle->ofbundle, 0));
+}
+
+static void
+xlate_xport_init(struct xport *xport)
+{
+    hmap_init(&xport->skb_priorities);
+    hmap_insert(&new_xcfg->xports, &xport->hmap_node,
+                hash_pointer(xport->ofport, 0));
+    hmap_insert(&xport->xbridge->xports, &xport->ofp_node,
+                hash_ofp_port(xport->ofp_port));
+}
+
+static void
+xlate_xbridge_set(struct xbridge *xbridge,
+                  const char *name, struct dpif *dpif,
+                  struct rule_dpif *miss_rule,
                   struct rule_dpif *no_packet_in_rule,
                   const struct mac_learning *ml, struct stp *stp,
                   const struct mbridge *mbridge,
@@ -339,17 +412,6 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
                   bool variable_length_userdata,
                   size_t max_mpls_depth)
 {
-    struct xbridge *xbridge = xbridge_lookup(ofproto);
-
-    if (!xbridge) {
-        xbridge = xzalloc(sizeof *xbridge);
-        xbridge->ofproto = ofproto;
-
-        hmap_insert(&xbridges, &xbridge->hmap_node, hash_pointer(ofproto, 0));
-        hmap_init(&xbridge->xports);
-        list_init(&xbridge->xbundles);
-    }
-
     if (xbridge->ml != ml) {
         mac_learning_unref(xbridge->ml);
         xbridge->ml = mac_learning_ref(ml);
@@ -380,7 +442,6 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
         xbridge->netflow = netflow_ref(netflow);
     }
 
-    free(xbridge->name);
     xbridge->name = xstrdup(name);
 
     xbridge->dpif = dpif;
@@ -394,10 +455,257 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
     xbridge->max_mpls_depth = max_mpls_depth;
 }
 
+static void
+xlate_xbundle_set(struct xbundle *xbundle, const char *name,
+                  enum port_vlan_mode vlan_mode, int vlan,
+                  unsigned long *trunks, bool use_priority_tags,
+                  const struct bond *bond, const struct lacp *lacp,
+                  bool floodable)
+{
+    ovs_assert(xbundle->xbridge);
+
+    xbundle->name = xstrdup(name);
+
+    xbundle->vlan_mode = vlan_mode;
+    xbundle->vlan = vlan;
+    xbundle->trunks = trunks;
+    xbundle->use_priority_tags = use_priority_tags;
+    xbundle->floodable = floodable;
+
+    if (xbundle->bond != bond) {
+        bond_unref(xbundle->bond);
+        xbundle->bond = bond_ref(bond);
+    }
+
+    if (xbundle->lacp != lacp) {
+        lacp_unref(xbundle->lacp);
+        xbundle->lacp = lacp_ref(lacp);
+    }
+}
+
+static void
+xlate_xport_set(struct xport *xport, odp_port_t odp_port,
+                const struct cfm *cfm, const struct bfd *bfd, int stp_port_no,
+                enum ofputil_port_config config, enum ofputil_port_state state,
+                bool is_tunnel, bool may_enable)
+{
+    xport->config = config;
+    xport->state = state;
+    xport->stp_port_no = stp_port_no;
+    xport->is_tunnel = is_tunnel;
+    xport->may_enable = may_enable;
+    xport->odp_port = odp_port;
+
+    if (xport->cfm != cfm) {
+        cfm_unref(xport->cfm);
+        xport->cfm = cfm_ref(cfm);
+    }
+
+    if (xport->bfd != bfd) {
+        bfd_unref(xport->bfd);
+        xport->bfd = bfd_ref(bfd);
+    }
+}
+
+static void
+xlate_xbridge_copy(struct xbridge *xbridge)
+{
+    struct xbundle *xbundle;
+    struct xport *xport;
+    struct xbridge *new_xbridge = xzalloc(sizeof *xbridge);
+    new_xbridge->ofproto = xbridge->ofproto;
+    xlate_xbridge_init(new_xbridge);
+
+    xlate_xbridge_set(new_xbridge, xbridge->name,
+                      xbridge->dpif, xbridge->miss_rule,
+                      xbridge->no_packet_in_rule, xbridge->ml, xbridge->stp,
+                      xbridge->mbridge, xbridge->sflow, xbridge->ipfix,
+                      xbridge->netflow, xbridge->frag, xbridge->forward_bpdu,
+                      xbridge->has_in_band, xbridge->enable_recirc,
+                      xbridge->variable_length_userdata,
+                      xbridge->max_mpls_depth);
+    LIST_FOR_EACH (xbundle, list_node, &xbridge->xbundles) {
+        xlate_xbundle_copy(new_xbridge, xbundle);
+    }
+
+    HMAP_FOR_EACH (xport, ofp_node, &xbridge->xports) {
+        if (!xport->xbundle) {
+            xlate_xport_copy(new_xbridge, NULL, xport);
+        }
+    }
+}
+
+static void
+xlate_xbundle_copy(struct xbridge *xbridge, struct xbundle *xbundle)
+{
+    struct xport *xport;
+    struct xbundle *new_xbundle = xzalloc(sizeof *xbundle);
+    new_xbundle->ofbundle = xbundle->ofbundle;
+    new_xbundle->xbridge = xbridge;
+    xlate_xbundle_init(new_xbundle);
+
+    xlate_xbundle_set(new_xbundle, xbundle->name, xbundle->vlan_mode,
+                      xbundle->vlan, xbundle->trunks,
+                      xbundle->use_priority_tags, xbundle->bond, xbundle->lacp,
+                      xbundle->floodable);
+    LIST_FOR_EACH (xport, bundle_node, &xbundle->xports) {
+        xlate_xport_copy(xbridge, new_xbundle, xport);
+    }
+}
+
+static void
+xlate_xport_copy(struct xbridge *xbridge, struct xbundle *xbundle,
+                 struct xport *xport)
+{
+    struct skb_priority_to_dscp *pdscp, *new_pdscp;
+    struct xport *new_xport = xzalloc(sizeof *xport);
+    new_xport->ofport = xport->ofport;
+    new_xport->ofp_port = xport->ofp_port;
+    new_xport->xbridge = xbridge;
+    xlate_xport_init(new_xport);
+
+    xlate_xport_set(new_xport, xport->odp_port, xport->cfm, xport->bfd,
+                    xport->stp_port_no, xport->config, xport->state,
+                    xport->is_tunnel, xport->may_enable);
+
+    /* Note that RCU read threads do not take a reference to netdev since this
+     * blocks creation of netdevs with the same name after deletion or type
+     * modification of the netdev */
+    new_xport->netdev = xport->netdev;
+
+    if (xport->peer) {
+        struct xport *peer = xport_lookup(xport->peer->ofport, new_xcfg);
+        if (peer) {
+            new_xport->peer = peer;
+            new_xport->peer->peer = new_xport;
+        }
+    }
+
+    if (xbundle) {
+        new_xport->xbundle = xbundle;
+        list_insert(&new_xport->xbundle->xports, &new_xport->bundle_node);
+    }
+
+    HMAP_FOR_EACH (pdscp, hmap_node, &xport->skb_priorities) {
+        new_pdscp = xmalloc(sizeof *pdscp);
+        new_pdscp->skb_priority = pdscp->skb_priority;
+        new_pdscp->dscp = pdscp->dscp;
+        hmap_insert(&new_xport->skb_priorities, &new_pdscp->hmap_node,
+                    hash_int(new_pdscp->skb_priority, 0));
+    }
+}
+
+/* Copies the current xlate configuration in xcfgp to new_xcfg.
+ *
+ * This needs to be called prior to editing the xlate configuration.
+ *
+ * Functions that edit the new xlate configuration are
+ * xlate_<ofport/bundle/ofport>_set and xlate_<ofport/bundle/ofport>_remove.
+ *
+ * A sample workflow:
+ *
+ * xlate_init_new_xcfg();
+ * ...
+ * edit_xlate_configuration();
+ *
+ * xlate_set_new_xcfg(); */
 void
-xlate_remove_ofproto(struct ofproto_dpif *ofproto)
+xlate_init_new_xcfg(void)
+{
+    struct xbridge *xbridge;
+    struct xlate_cfg *xcfg;
+
+    new_xcfg = xmalloc(sizeof *new_xcfg);
+    hmap_init(&new_xcfg->xbridges);
+    hmap_init(&new_xcfg->xbundles);
+    hmap_init(&new_xcfg->xports);
+
+    xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
+    if (!xcfg) {
+        return;
+    }
+
+    HMAP_FOR_EACH (xbridge, hmap_node, &xcfg->xbridges) {
+        xlate_xbridge_copy(xbridge);
+    }
+}
+
+
+static void
+xlate_xcfg_free(struct xlate_cfg *xcfg)
+{
+    struct xbridge *xbridge, *next_xbridge;
+
+    if (!xcfg) {
+        return;
+    }
+
+    HMAP_FOR_EACH_SAFE (xbridge, next_xbridge, hmap_node, &xcfg->xbridges) {
+        xlate_xbridge_remove(xbridge, xcfg);
+    }
+
+    hmap_destroy(&xcfg->xbridges);
+    hmap_destroy(&xcfg->xbundles);
+    hmap_destroy(&xcfg->xports);
+    free(xcfg);
+}
+
+/* Sets the current xlate configuration to new_xcfg and frees the old xlate
+ * configuration in xcfgp.
+ *
+ * This needs to be called after editing the xlate configuration.
+ *
+ * Functions that edit the new xlate configuration are
+ * xlate_<ofport/bundle/ofport>_set and xlate_<ofport/bundle/ofport>_remove.
+ *
+ * A sample workflow:
+ *
+ * xlate_init_new_xcfg();
+ * ...
+ * edit_xlate_configuration();
+ * ...
+ * xlate_set_new_xcfg(); */
+void
+xlate_set_new_xcfg()
+{
+    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
+
+    ovsrcu_set(&xcfgp, new_xcfg);
+    ovsrcu_postpone(xlate_xcfg_free, xcfg);
+}
+
+void
+xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
+                  struct dpif *dpif, struct rule_dpif *miss_rule,
+                  struct rule_dpif *no_packet_in_rule,
+                  const struct mac_learning *ml, struct stp *stp,
+                  const struct mbridge *mbridge,
+                  const struct dpif_sflow *sflow,
+                  const struct dpif_ipfix *ipfix,
+                  const struct netflow *netflow, enum ofp_config_flags frag,
+                  bool forward_bpdu, bool has_in_band,
+                  bool enable_recirc,
+                  bool variable_length_userdata,
+                  size_t max_mpls_depth)
+{
+    struct xbridge *xbridge = xbridge_lookup(ofproto, new_xcfg);
+
+    if (!xbridge) {
+        xbridge = xzalloc(sizeof *xbridge);
+        xbridge->ofproto = ofproto;
+
+        xlate_xbridge_init(xbridge);
+    }
+    free(xbridge->name);
+    xlate_xbridge_set(xbridge, name, dpif, miss_rule,
+                      no_packet_in_rule, ml, stp, mbridge, sflow, ipfix,
+                      netflow, frag, forward_bpdu, has_in_band, enable_recirc,
+                      variable_length_userdata, max_mpls_depth);
+}
+
+static void
+xlate_xbridge_remove(struct xbridge *xbridge, struct xlate_cfg *xcfg)
 {
-    struct xbridge *xbridge = xbridge_lookup(ofproto);
     struct xbundle *xbundle, *next_xbundle;
     struct xport *xport, *next_xport;
 
@@ -406,14 +714,14 @@ xlate_remove_ofproto(struct ofproto_dpif *ofproto)
     }
 
     HMAP_FOR_EACH_SAFE (xport, next_xport, ofp_node, &xbridge->xports) {
-        xlate_ofport_remove(xport->ofport);
+        xlate_xport_remove(xport, xcfg);
     }
 
     LIST_FOR_EACH_SAFE (xbundle, next_xbundle, list_node, &xbridge->xbundles) {
-        xlate_bundle_remove(xbundle->ofbundle);
+        xlate_xbundle_remove(xbundle, xcfg);
     }
 
-    hmap_remove(&xbridges, &xbridge->hmap_node);
+    hmap_remove(&xcfg->xbridges, &xbridge->hmap_node);
     mac_learning_unref(xbridge->ml);
     mbridge_unref(xbridge->mbridge);
     dpif_sflow_unref(xbridge->sflow);
@@ -425,50 +733,37 @@ xlate_remove_ofproto(struct ofproto_dpif *ofproto)
 }
 
 void
+xlate_remove_ofproto(struct ofproto_dpif *ofproto)
+{
+    struct xbridge *xbridge = xbridge_lookup(ofproto, new_xcfg);
+    xlate_xbridge_remove(xbridge, new_xcfg);
+}
+
+void
 xlate_bundle_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
                  const char *name, enum port_vlan_mode vlan_mode, int vlan,
                  unsigned long *trunks, bool use_priority_tags,
                  const struct bond *bond, const struct lacp *lacp,
                  bool floodable)
 {
-    struct xbundle *xbundle = xbundle_lookup(ofbundle);
+    struct xbundle *xbundle = xbundle_lookup(ofbundle, new_xcfg);
 
     if (!xbundle) {
         xbundle = xzalloc(sizeof *xbundle);
         xbundle->ofbundle = ofbundle;
-        xbundle->xbridge = xbridge_lookup(ofproto);
+        xbundle->xbridge = xbridge_lookup(ofproto, new_xcfg);
 
-        hmap_insert(&xbundles, &xbundle->hmap_node, hash_pointer(ofbundle, 0));
-        list_insert(&xbundle->xbridge->xbundles, &xbundle->list_node);
-        list_init(&xbundle->xports);
+        xlate_xbundle_init(xbundle);
     }
 
-    ovs_assert(xbundle->xbridge);
-
     free(xbundle->name);
-    xbundle->name = xstrdup(name);
-
-    xbundle->vlan_mode = vlan_mode;
-    xbundle->vlan = vlan;
-    xbundle->trunks = trunks;
-    xbundle->use_priority_tags = use_priority_tags;
-    xbundle->floodable = floodable;
-
-    if (xbundle->bond != bond) {
-        bond_unref(xbundle->bond);
-        xbundle->bond = bond_ref(bond);
-    }
-
-    if (xbundle->lacp != lacp) {
-        lacp_unref(xbundle->lacp);
-        xbundle->lacp = lacp_ref(lacp);
-    }
+    xlate_xbundle_set(xbundle, name, vlan_mode, vlan, trunks,
+                      use_priority_tags, bond, lacp, floodable);
 }
 
-void
-xlate_bundle_remove(struct ofbundle *ofbundle)
+static void
+xlate_xbundle_remove(struct xbundle *xbundle, struct xlate_cfg *xcfg)
 {
-    struct xbundle *xbundle = xbundle_lookup(ofbundle);
     struct xport *xport, *next;
 
     if (!xbundle) {
@@ -480,7 +775,7 @@ xlate_bundle_remove(struct ofbundle *ofbundle)
         xport->xbundle = NULL;
     }
 
-    hmap_remove(&xbundles, &xbundle->hmap_node);
+    hmap_remove(&xcfg->xbundles, &xbundle->hmap_node);
     list_remove(&xbundle->list_node);
     bond_unref(xbundle->bond);
     lacp_unref(xbundle->lacp);
@@ -489,6 +784,13 @@ xlate_bundle_remove(struct ofbundle *ofbundle)
 }
 
 void
+xlate_bundle_remove(struct ofbundle *ofbundle)
+{
+    struct xbundle *xbundle = xbundle_lookup(ofbundle, new_xcfg);
+    xlate_xbundle_remove(xbundle, new_xcfg);
+}
+
+void
 xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
                  struct ofport_dpif *ofport, ofp_port_t ofp_port,
                  odp_port_t odp_port, const struct netdev *netdev,
@@ -499,49 +801,32 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
                  enum ofputil_port_state state, bool is_tunnel,
                  bool may_enable)
 {
-    struct xport *xport = xport_lookup(ofport);
     size_t i;
+    struct xport *xport = xport_lookup(ofport, new_xcfg);
 
     if (!xport) {
         xport = xzalloc(sizeof *xport);
         xport->ofport = ofport;
-        xport->xbridge = xbridge_lookup(ofproto);
+        xport->xbridge = xbridge_lookup(ofproto, new_xcfg);
         xport->ofp_port = ofp_port;
 
-        hmap_init(&xport->skb_priorities);
-        hmap_insert(&xports, &xport->hmap_node, hash_pointer(ofport, 0));
-        hmap_insert(&xport->xbridge->xports, &xport->ofp_node,
-                    hash_ofp_port(xport->ofp_port));
+        xlate_xport_init(xport);
     }
 
     ovs_assert(xport->ofp_port == ofp_port);
 
-    xport->config = config;
-    xport->state = state;
-    xport->stp_port_no = stp_port_no;
-    xport->is_tunnel = is_tunnel;
-    xport->may_enable = may_enable;
-    xport->odp_port = odp_port;
+    xlate_xport_set(xport, odp_port, cfm, bfd, stp_port_no, config,
+                    state, is_tunnel, may_enable);
 
     if (xport->netdev != netdev) {
         netdev_close(xport->netdev);
         xport->netdev = netdev_ref(netdev);
     }
 
-    if (xport->cfm != cfm) {
-        cfm_unref(xport->cfm);
-        xport->cfm = cfm_ref(cfm);
-    }
-
-    if (xport->bfd != bfd) {
-        bfd_unref(xport->bfd);
-        xport->bfd = bfd_ref(bfd);
-    }
-
     if (xport->peer) {
         xport->peer->peer = NULL;
     }
-    xport->peer = xport_lookup(peer);
+    xport->peer = xport_lookup(peer, new_xcfg);
     if (xport->peer) {
         xport->peer->peer = xport;
     }
@@ -549,7 +834,7 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
     if (xport->xbundle) {
         list_remove(&xport->bundle_node);
     }
-    xport->xbundle = xbundle_lookup(ofbundle);
+    xport->xbundle = xbundle_lookup(ofbundle, new_xcfg);
     if (xport->xbundle) {
         list_insert(&xport->xbundle->xports, &xport->bundle_node);
     }
@@ -572,11 +857,9 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
     }
 }
 
-void
-xlate_ofport_remove(struct ofport_dpif *ofport)
+static void
+xlate_xport_remove(struct xport *xport, struct xlate_cfg *xcfg)
 {
-    struct xport *xport = xport_lookup(ofport);
-
     if (!xport) {
         return;
     }
@@ -593,15 +876,26 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
     clear_skb_priorities(xport);
     hmap_destroy(&xport->skb_priorities);
 
-    hmap_remove(&xports, &xport->hmap_node);
+    hmap_remove(&xcfg->xports, &xport->hmap_node);
     hmap_remove(&xport->xbridge->xports, &xport->ofp_node);
 
-    netdev_close(xport->netdev);
+    /* Only close the netdev if the xlate configuration is being changed. This
+     * is because RCU read threads do not ref the netdev */
+    if (xcfg == new_xcfg) {
+        netdev_close(xport->netdev);
+    }
     cfm_unref(xport->cfm);
     bfd_unref(xport->bfd);
     free(xport);
 }
 
+void
+xlate_ofport_remove(struct ofport_dpif *ofport)
+{
+    struct xport *xport = xport_lookup(ofport, new_xcfg);
+    xlate_xport_remove(xport, new_xcfg);
+}
+
 /* Given a datpath, packet, and flow metadata ('backer', 'packet', and 'key'
  * respectively), populates 'flow' with the result of odp_flow_key_to_flow().
  * Optionally populates 'ofproto' with the ofproto_dpif, 'odp_in_port' with
@@ -634,11 +928,11 @@ xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet,
 {
     const struct xport *xport;
     int error = ENODEV;
+    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
 
-    ovs_rwlock_rdlock(&xlate_rwlock);
     if (odp_flow_key_to_flow(key, key_len, flow) == ODP_FIT_ERROR) {
         error = EINVAL;
-        goto exit;
+        return error;
     }
 
     if (odp_in_port) {
@@ -647,11 +941,12 @@ xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet,
 
     xport = xport_lookup(tnl_port_should_receive(flow)
                          ? tnl_port_receive(flow)
-                         : odp_port_to_ofport(backer, flow->in_port.odp_port));
+                         : odp_port_to_ofport(backer, flow->in_port.odp_port),
+                         xcfg);
 
     flow->in_port.ofp_port = xport ? xport->ofp_port : OFPP_NONE;
     if (!xport) {
-        goto exit;
+        return error;
     }
 
     if (vsp_adjust_flow(xport->xbridge->ofproto, flow)) {
@@ -680,23 +975,23 @@ xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet,
     if (netflow) {
         *netflow = netflow_ref(xport->xbridge->netflow);
     }
-
-exit:
-    ovs_rwlock_unlock(&xlate_rwlock);
     return error;
 }
 
 static struct xbridge *
-xbridge_lookup(const struct ofproto_dpif *ofproto)
+xbridge_lookup(const struct ofproto_dpif *ofproto, struct xlate_cfg *xcfg)
 {
+    struct hmap *xbridges;
     struct xbridge *xbridge;
 
-    if (!ofproto) {
+    if (!ofproto || !xcfg) {
         return NULL;
     }
 
+    xbridges = &xcfg->xbridges;
+
     HMAP_FOR_EACH_IN_BUCKET (xbridge, hmap_node, hash_pointer(ofproto, 0),
-                             &xbridges) {
+                             xbridges) {
         if (xbridge->ofproto == ofproto) {
             return xbridge;
         }
@@ -705,16 +1000,19 @@ xbridge_lookup(const struct ofproto_dpif *ofproto)
 }
 
 static struct xbundle *
-xbundle_lookup(const struct ofbundle *ofbundle)
+xbundle_lookup(const struct ofbundle *ofbundle, struct xlate_cfg *xcfg)
 {
+    struct hmap *xbundles;
     struct xbundle *xbundle;
 
-    if (!ofbundle) {
+    if (!ofbundle || !xcfg) {
         return NULL;
     }
 
+    xbundles = &xcfg->xbundles;
+
     HMAP_FOR_EACH_IN_BUCKET (xbundle, hmap_node, hash_pointer(ofbundle, 0),
-                             &xbundles) {
+                             xbundles) {
         if (xbundle->ofbundle == ofbundle) {
             return xbundle;
         }
@@ -723,16 +1021,19 @@ xbundle_lookup(const struct ofbundle *ofbundle)
 }
 
 static struct xport *
-xport_lookup(const struct ofport_dpif *ofport)
+xport_lookup(const struct ofport_dpif *ofport, struct xlate_cfg *xcfg)
 {
+    struct hmap *xports;
     struct xport *xport;
 
-    if (!ofport) {
+    if (!ofport || !xcfg) {
         return NULL;
     }
 
+    xports = &xcfg->xports;
+
     HMAP_FOR_EACH_IN_BUCKET (xport, hmap_node, hash_pointer(ofport, 0),
-                             &xports) {
+                             xports) {
         if (xport->ofport == ofport) {
             return xport;
         }
@@ -1075,7 +1376,8 @@ add_mirror_actions(struct xlate_ctx *ctx, const struct flow *orig_flow)
         mirrors &= ~dup_mirrors;
         ctx->xout->mirrors |= dup_mirrors;
         if (out) {
-            struct xbundle *out_xbundle = xbundle_lookup(out);
+            struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
+            struct xbundle *out_xbundle = xbundle_lookup(out, xcfg);
             if (out_xbundle) {
                 output_normal(ctx, out_xbundle, vlan);
             }
@@ -1220,6 +1522,7 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
         struct ofport_dpif *ofport;
         struct xlate_recirc *xr = &ctx->recirc;
         struct flow_wildcards *wc = &ctx->xout->wc;
+        struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
 
         if (ctx->xbridge->enable_recirc) {
             ctx->use_recirc = bond_may_recirc(
@@ -1237,7 +1540,7 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
 
         ofport = bond_choose_output_slave(out_xbundle->bond,
                                           &ctx->xin->flow, wc, vid);
-        xport = xport_lookup(ofport);
+        xport = xport_lookup(ofport, xcfg);
 
         if (!xport) {
             /* No slaves enabled, so drop packet. */
@@ -1565,7 +1868,8 @@ xlate_normal(struct xlate_ctx *ctx)
     ovs_rwlock_unlock(&ctx->xbridge->ml->rwlock);
 
     if (mac_port) {
-        struct xbundle *mac_xbundle = xbundle_lookup(mac_port);
+        struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
+        struct xbundle *mac_xbundle = xbundle_lookup(mac_port, xcfg);
         if (mac_xbundle && mac_xbundle != in_xbundle) {
             xlate_report(ctx, "forwarding to learned port");
             output_normal(ctx, mac_xbundle, vlan);
@@ -1889,7 +2193,6 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
             entry->u.dev.rx = netdev_ref(peer->netdev);
             entry->u.dev.bfd = bfd_ref(peer->bfd);
         }
-
         return;
     }
 
@@ -3169,28 +3472,18 @@ actions_output_to_local_port(const struct xlate_ctx *ctx)
     return false;
 }
 
-/* Thread safe call to xlate_actions__(). */
-void
-xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
-    OVS_EXCLUDED(xlate_rwlock)
-{
-    ovs_rwlock_rdlock(&xlate_rwlock);
-    xlate_actions__(xin, xout);
-    ovs_rwlock_unlock(&xlate_rwlock);
-}
-
 /* Translates the 'ofpacts_len' bytes of "struct ofpacts" starting at 'ofpacts'
  * into datapath actions in 'odp_actions', using 'ctx'.
  *
  * The caller must take responsibility for eventually freeing 'xout', with
  * xlate_out_uninit(). */
-static void
-xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
-    OVS_REQ_RDLOCK(xlate_rwlock)
+void
+xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 {
     struct flow_wildcards *wc = &xout->wc;
     struct flow *flow = &xin->flow;
     struct rule_dpif *rule = NULL;
+    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
 
     const struct rule_actions *actions = NULL;
     enum slow_path_reason special;
@@ -3237,7 +3530,7 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
                     sizeof ctx.xout->odp_actions_stub);
     ofpbuf_reserve(&ctx.xout->odp_actions, NL_A_U32_SIZE);
 
-    ctx.xbridge = xbridge_lookup(xin->ofproto);
+    ctx.xbridge = xbridge_lookup(xin->ofproto, xcfg);
     if (!ctx.xbridge) {
         return;
     }
@@ -3475,21 +3768,19 @@ xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
     struct xport *xport;
     struct ofpact_output output;
     struct flow flow;
+    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
 
     ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output);
     /* Use OFPP_NONE as the in_port to avoid special packet processing. */
     flow_extract(packet, NULL, &flow);
     flow.in_port.ofp_port = OFPP_NONE;
 
-    ovs_rwlock_rdlock(&xlate_rwlock);
-    xport = xport_lookup(ofport);
+    xport = xport_lookup(ofport, xcfg);
     if (!xport) {
-        ovs_rwlock_unlock(&xlate_rwlock);
         return EINVAL;
     }
     output.port = xport->ofp_port;
     output.max_len = 0;
-    ovs_rwlock_unlock(&xlate_rwlock);
 
     return ofproto_dpif_execute_actions(xport->xbridge->ofproto, &flow, NULL,
                                         &output.ofpact, sizeof output,
@@ -3536,8 +3827,9 @@ xlate_cache_normal(struct ofproto_dpif *ofproto, struct flow *flow, int vlan)
     struct xbridge *xbridge;
     struct xbundle *xbundle;
     struct flow_wildcards wc;
+    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
 
-    xbridge = xbridge_lookup(ofproto);
+    xbridge = xbridge_lookup(ofproto, xcfg);
     if (!xbridge) {
         return;
     }
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 760736a..f5a0c5c 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -136,8 +136,6 @@ struct xlate_in {
     struct xlate_cache *xcache;
 };
 
-extern struct ovs_rwlock xlate_rwlock;
-
 void xlate_ofproto_set(struct ofproto_dpif *, const char *name,
                        struct dpif *, struct rule_dpif *miss_rule,
                        struct rule_dpif *no_packet_in_rule,
@@ -147,16 +145,15 @@ void xlate_ofproto_set(struct ofproto_dpif *, const char *name,
                        enum ofp_config_flags, bool forward_bpdu,
                        bool has_in_band, bool enable_recirc,
                        bool variable_length_userdata,
-                       size_t mpls_label_stack_length)
-    OVS_REQ_WRLOCK(xlate_rwlock);
-void xlate_remove_ofproto(struct ofproto_dpif *) OVS_REQ_WRLOCK(xlate_rwlock);
+                       size_t mpls_label_stack_length);
+void xlate_remove_ofproto(struct ofproto_dpif *);
 
 void xlate_bundle_set(struct ofproto_dpif *, struct ofbundle *,
                       const char *name, enum port_vlan_mode, int vlan,
                       unsigned long *trunks, bool use_priority_tags,
                       const struct bond *, const struct lacp *,
-                      bool floodable) OVS_REQ_WRLOCK(xlate_rwlock);
-void xlate_bundle_remove(struct ofbundle *) OVS_REQ_WRLOCK(xlate_rwlock);
+                      bool floodable);
+void xlate_bundle_remove(struct ofbundle *);
 
 void xlate_ofport_set(struct ofproto_dpif *, struct ofbundle *,
                       struct ofport_dpif *, ofp_port_t, odp_port_t,
@@ -165,18 +162,16 @@ void xlate_ofport_set(struct ofproto_dpif *, struct ofbundle *,
                       int stp_port_no, const struct ofproto_port_queue *qdscp,
                       size_t n_qdscp, enum ofputil_port_config,
                       enum ofputil_port_state, bool is_tunnel,
-                      bool may_enable) OVS_REQ_WRLOCK(xlate_rwlock);
-void xlate_ofport_remove(struct ofport_dpif *) OVS_REQ_WRLOCK(xlate_rwlock);
+                      bool may_enable);
+void xlate_ofport_remove(struct ofport_dpif *);
 
 int xlate_receive(const struct dpif_backer *, struct ofpbuf *packet,
                   const struct nlattr *key, size_t key_len,
                   struct flow *, struct ofproto_dpif **, struct dpif_ipfix **,
                   struct dpif_sflow **, struct netflow **,
-                  odp_port_t *odp_in_port)
-    OVS_EXCLUDED(xlate_rwlock);
+                  odp_port_t *odp_in_port);
 
-void xlate_actions(struct xlate_in *, struct xlate_out *)
-    OVS_EXCLUDED(xlate_rwlock);
+void xlate_actions(struct xlate_in *, struct xlate_out *);
 void xlate_in_init(struct xlate_in *, struct ofproto_dpif *,
                    const struct flow *, struct rule_dpif *,
                    uint16_t tcp_flags, const struct ofpbuf *packet);
@@ -192,4 +187,7 @@ void xlate_push_stats(struct xlate_cache *, bool may_learn,
 void xlate_cache_clear(struct xlate_cache *);
 void xlate_cache_delete(struct xlate_cache *);
 
+void xlate_init_new_xcfg(void);
+void xlate_set_new_xcfg(void);
+
 #endif /* ofproto-dpif-xlate.h */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index e50b4fe..cd8b199 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -600,7 +600,7 @@ type_run(const char *type)
                 continue;
             }
 
-            ovs_rwlock_wrlock(&xlate_rwlock);
+            xlate_init_new_xcfg();
             xlate_ofproto_set(ofproto, ofproto->up.name,
                               ofproto->backer->dpif, ofproto->miss_rule,
                               ofproto->no_packet_in_rule, ofproto->ml,
@@ -633,7 +633,7 @@ type_run(const char *type)
                                  ofport->up.pp.config, ofport->up.pp.state,
                                  ofport->is_tunnel, ofport->may_enable);
             }
-            ovs_rwlock_unlock(&xlate_rwlock);
+            xlate_set_new_xcfg();
         }
 
         udpif_revalidate(backer->udpif);
@@ -1293,9 +1293,9 @@ destruct(struct ofproto *ofproto_)
     struct list pins;
 
     ofproto->backer->need_revalidate = REV_RECONFIGURE;
-    ovs_rwlock_wrlock(&xlate_rwlock);
+    xlate_init_new_xcfg();
     xlate_remove_ofproto(ofproto);
-    ovs_rwlock_unlock(&xlate_rwlock);
+    xlate_set_new_xcfg();
 
     /* Ensure that the upcall processing threads have no remaining references
      * to the ofproto or anything in it. */
@@ -1646,9 +1646,9 @@ port_destruct(struct ofport *port_)
     const char *dp_port_name;
 
     ofproto->backer->need_revalidate = REV_RECONFIGURE;
-    ovs_rwlock_wrlock(&xlate_rwlock);
+    xlate_init_new_xcfg();
     xlate_ofport_remove(port);
-    ovs_rwlock_unlock(&xlate_rwlock);
+    xlate_set_new_xcfg();
 
     dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf,
                                               sizeof namebuf);
@@ -2291,9 +2291,9 @@ bundle_destroy(struct ofbundle *bundle)
     ofproto = bundle->ofproto;
     mbridge_unregister_bundle(ofproto->mbridge, bundle->aux);
 
-    ovs_rwlock_wrlock(&xlate_rwlock);
+    xlate_init_new_xcfg();
     xlate_bundle_remove(bundle);
-    ovs_rwlock_unlock(&xlate_rwlock);
+    xlate_set_new_xcfg();
 
     LIST_FOR_EACH_SAFE (port, next_port, bundle_node, &bundle->ports) {
         bundle_del_port(port);
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index d4ad624..5167abb 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -53,8 +53,6 @@ enum rule_dpif_lookup_verdict {
                                              * dropped. */
 };
 
-/* For lock annotation below only. */
-extern struct ovs_rwlock xlate_rwlock;
 
 /* Ofproto-dpif -- DPIF based ofproto implementation.
  *
@@ -142,8 +140,7 @@ bool vsp_adjust_flow(const struct ofproto_dpif *, struct flow *);
 
 int ofproto_dpif_execute_actions(struct ofproto_dpif *, const struct flow *,
                                  struct rule_dpif *, const struct ofpact *,
-                                 size_t ofpacts_len, struct ofpbuf *)
-    OVS_EXCLUDED(xlate_rwlock);
+                                 size_t ofpacts_len, struct ofpbuf *);
 void ofproto_dpif_send_packet_in(struct ofproto_dpif *,
                                  struct ofproto_packet_in *);
 bool ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *);
-- 
1.7.9.5




More information about the dev mailing list