ofproto-dpif-xlate: Avoid recursive acquisition of xlate_rwlock.

Currently xlate_rwlock is recursively acquired.
(xlate_send_packet -> ofproto_dpif_execute_actions -> xlate_actions)
Due to writer-preference in rwlock implementations, this causes
deadlock if another thread tries to acquire the lock exclusively
behind us.

This change avoids the problem by making xlate_send_packet drop
the lock before calling ofproto_dpif_execute_actions.  This is the
simplest fix but opens a race window against port reconfigurations.
Given the way xlate_send_packet is currently used, the race does not
seem a big problem.  An alternative would be passing down the
"xlate_rwlock is held" info to ofproto_dpif_execute_actions.

Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7e332bd..bb85b89 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3243,7 +3243,6 @@ xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
     struct ofpact_output output;
     struct flow flow;
     union flow_in_port in_port_;
-    int error;
 
     ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output);
     /* Use OFPP_NONE as the in_port to avoid special packet processing. */
@@ -3258,9 +3257,9 @@ xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
     }
     output.port = xport->ofp_port;
     output.max_len = 0;
-    error = ofproto_dpif_execute_actions(xport->xbridge->ofproto, &flow, NULL,
-                                         &output.ofpact, sizeof output,
-                                         packet);
     ovs_rwlock_unlock(&xlate_rwlock);
-    return error;
+
+    return ofproto_dpif_execute_actions(xport->xbridge->ofproto, &flow, NULL,
+                                        &output.ofpact, sizeof output,
+                                        packet);
 }