On Wed, Jan 15, 2014 at 12:41:24PM +0900, YAMAMOTO Takashi wrote:
> 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 <yamam...@valinux.co.jp>
Thanks, applied to master and branch-2.1. I changed the patch
slightly to the following because I liked the resulting code a bit
better:
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 0eadf46..4747ea7 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3255,7 +3255,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. */
@@ -3270,9 +3269,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);
}
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
