<div dir="ltr">Thanks Ryan, it is pretty close.  Please see my comments below:<br><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">One high level comment is that, we assume new_xcfg is non-null.  Could we add assertion to the</div>


<div class="gmail_extra">external functions? e.g. xlate_ofport_remove()</div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra"><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">


<div><div>  <br></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">@@ -324,10 +332,76 @@ static bool dscp_from_skb_priority(const struct xport *, uint32_t skb_priority,<br>



<div><div><br>
 static struct xc_entry *xlate_cache_add_entry(struct xlate_cache *xc,<br>
                                               enum xc_type type);<br>
+static void xlate_xbridge_init(struct xbridge *);<br>
+static void xlate_xbundle_init(struct xbundle *);<br>
+static void xlate_xport_init(struct xport *);<br></div></div></blockquote><div><br></div><div><br></div><div><br></div><div>Could we add the xlate_cfg as input argument here?  Even though we always insert to the new_xcfg,</div>


<div>I think adding it makes it clear. </div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">


<div><div><span style="color:rgb(34,34,34)">@@ -339,17 +413,6 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,</span><br></div></div>
<div>                   bool variable_length_userdata,<br>
                   size_t max_mpls_depth)<br>
 {<br>
-    struct xbridge *xbridge = xbridge_lookup(ofproto);<br>
-<br>
-    if (!xbridge) {<br>
-        xbridge = xzalloc(sizeof *xbridge);<br>
-        xbridge-&gt;ofproto = ofproto;<br>
-<br>
-        hmap_insert(&amp;xbridges, &amp;xbridge-&gt;hmap_node, hash_pointer(ofproto, 0));<br>
-        hmap_init(&amp;xbridge-&gt;xports);<br>
-        list_init(&amp;xbridge-&gt;xbundles);<br>
-    }<br>
-<br>
     if (xbridge-&gt;ml != ml) {<br>
         mac_learning_unref(xbridge-&gt;ml);<br>
         xbridge-&gt;ml = mac_learning_ref(ml);<br>
</div>@@ -380,7 +443,6 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,<br>
<div>         xbridge-&gt;netflow = netflow_ref(netflow);<br>
     }<br>
<br>
-    free(xbridge-&gt;name);<br>
     xbridge-&gt;name = xstrdup(name);<br>
<br></div></blockquote><div><br></div><div><br></div><div><br></div><div>I think it is better to keep the name copy and free together.  Either inside or outside the xlate_x*_set().</div><div><br></div><div><br></div><div>


<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div>   <span style="color:rgb(80,0,80)">static void</span></div>


<div><div>
+xlate_xbridge_copy(struct xbridge *xbridge)<br>
+{<br>
+    struct xbundle *xbundle;<br>
+    struct xport *xport;<br>
+    struct xbridge *new_xbridge = xzalloc(sizeof *xbridge);<br>
+    new_xbridge-&gt;ofproto = xbridge-&gt;ofproto;<br>
+    xlate_xbridge_init(new_xbridge);<br>
+<br>
+    xlate_xbridge_set(new_xbridge, xbridge-&gt;name,<br>
+                      xbridge-&gt;dpif, xbridge-&gt;miss_rule,<br>
+                      xbridge-&gt;no_packet_in_rule, xbridge-&gt;ml, xbridge-&gt;stp,<br>
+                      xbridge-&gt;mbridge, xbridge-&gt;sflow, xbridge-&gt;ipfix,<br>
+                      xbridge-&gt;netflow, xbridge-&gt;frag, xbridge-&gt;forward_bpdu,<br>
+                      xbridge-&gt;has_in_band, xbridge-&gt;enable_recirc,<br>
+                      xbridge-&gt;variable_length_userdata,<br>
+                      xbridge-&gt;max_mpls_depth);<br>
+    LIST_FOR_EACH (xbundle, list_node, &amp;xbridge-&gt;xbundles) {<br>
+        xlate_xbundle_copy(new_xbridge, xbundle);<br>
+    }<br>
+<br>
+    HMAP_FOR_EACH (xport, ofp_node, &amp;xbridge-&gt;xports) {<br>
+        if (!xport-&gt;xbundle) {<br>
+            xlate_xport_copy(new_xbridge, NULL, xport);<br>
+        }<br>
+    }<br>
+}<br>
+<br></div></div></blockquote><div><br></div><div><br></div><div><br></div><div>maybe a comment here explaining why we need to check &quot;!<span style="color:rgb(80,0,80)">xport-&gt;xbundle</span>&quot;?</div><div><br></div>


<div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>
+<br>
+/* Sets the current xlate configuration to new_xcfg and frees the old xlate<br>
+ * configuration in xcfgp.<br>
+ *<br>
+ * This needs to be called after editing the xlate configuration.<br>
+ *<br>
+ * Functions that edit the new xlate configuration are<br>
+ * xlate_&lt;ofport/bundle/ofport&gt;_set and xlate_&lt;ofport/bundle/ofport&gt;_remove.<br>
+ *<br>
+ * A sample workflow:<br>
+ *<br>
+ * xlate_init_new_xcfg();<br>
+ * ...<br>
+ * edit_xlate_configuration();<br>
+ * ...<br>
+ * xlate_set_new_xcfg(); */<br>
+void<br>
+xlate_set_new_xcfg()<br>
+{<br>
+    struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &amp;xcfgp);<br>
+<br>
+    ovsrcu_set(&amp;xcfgp, new_xcfg);<br>
+    ovsrcu_postpone(xlate_xcfg_free, xcfg);<br>
+}<br></div></div></blockquote><div><br></div><div><br></div><div><br></div><div>Maybe nitpicking, could we move <span style="color:rgb(80,0,80)">xlate_set_new_xcfg() after </span><span style="color:rgb(80,0,80)">xlate_init_new_xcfg()?  And avoid the</span></div>


<div><span style="color:rgb(80,0,80)">comment duplication?</span></div><div><span style="color:rgb(80,0,80)"><br></span></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">


<div>
--<br>
1.7.9.5<br>
<br>
</div><div>_______________________________________________<br>
dev mailing list<br>
<a href="mailto:dev@openvswitch.org" target="_blank">dev@openvswitch.org</a><br>
</div><a href="http://openvswitch.org/mailman/listinfo/dev" target="_blank">http://openvswitch.org/mailman/listinfo/dev</a></blockquote><div><br></div><div><br></div><div>Again, it is really glad to see the xlate_rwlock gone.!</div>


<div><br></div><div><br></div></div><span style="font-family:arial,sans-serif;font-size:12.800000190734863px">Acked-by: Alex Wang &lt;</span><a href="mailto:alexw@nicira.com" target="_blank" style="font-family:arial,sans-serif;font-size:12.800000190734863px">alexw@nicira.com</a><span style="font-family:arial,sans-serif;font-size:12.800000190734863px">&gt;</span><br>

</div></div>