From: Ansis Atteka Date: Tue, 7 Apr 2015 02:01:59 +0000 (-0700) Subject: ofproto-dpif: Use fat_rwlock instead of ovs_rwlock. X-Git-Tag: v2.3.2~16 X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=d91f11d0e0812ef96f359348cbe48e9c48d9152f ofproto-dpif: Use fat_rwlock instead of ovs_rwlock. This patch fixes a deadlock introduced by commit 6b59b543 (ovs-thread: Use fair (but nonrecursive) rwlocks on glibc.) If STP is enabled, then a handler thread could have already had acquired "xlate_rwlock" in xlate_actions() and then might have attempt to acquire it again in xlate_send_packet() leading to a deadlock: pthread_rwlock_rdlock () from /lib/x86_64-linux-gnu/libpthread.so.0 ovs_rwlock_rdlock_at (l_=0x769cc0, where=0x4f4568 "../ofproto/ofproto-dpif-xlate.c:3600") at ../lib/ovs-thread.c:71 xlate_send_packet (ofport=0x23b6400, packet=0x7f980400a8d0) at ../ofproto/ofproto-dpif-xlate.c:3600 ofproto_dpif_send_packet (ofport=, packet=0x7f980400a8d0) at ../ofproto/ofproto-dpif.c:3684 send_bpdu_cb (pkt=0x7f980400a8d0, port_num=0, ofproto_=0x229a410) at ../ofproto/ofproto-dpif.c:1927 stp_send_bpdu (p=0x2400c00, bpdu=0x7f980f7e3080, bpdu_size=35) at ../lib/stp.c:1558 stp_transmit_config (p=0x2400c00) at ../lib/stp.c:1052 stp_acknowledge_topology_change (p=) at ../lib/stp.c:1301 stp_received_tcn_bpdu (p=, stp=) at ../lib/stp.c:1353 stp_received_bpdu (p=0x2400c00, bpdu=0x7f980f7f81e9, bpdu_size=) at ../lib/stp.c:771 stp_process_packet (packet=0x7f980f7f80f8, xport=0x24594b0) at ../ofproto/ofproto-dpif-xlate.c:840 process_special (flow=, xport=0x24594b0, packet=0x7f980f7f80f8, ctx=) at ../ofproto/ofproto-dpif-xlate.c:1832 compose_output_action__ (ctx=0x7f980f7e3730, ofp_port=, check_stp=true) at ../ofproto/ofproto-dpif-xlate.c:1894 compose_output_action (ofp_port=, ctx=0x7f980f7e3730) at ../ofproto/ofproto-dpif-xlate.c:2031 output_normal (ctx=0x7f980f7e3730, out_xbundle=0x23d13a0, vlan=) at ../ofproto/ofproto-dpif-xlate.c:1316 xlate_normal (ctx=0x7f980f7e3730) at ../ofproto/ofproto-dpif-xlate.c:1625 xlate_output_action (ctx=0x7f980f7e3730, port=, max_len=, may_packet_in=) at ../ofproto/ofproto-dpif-xlate.c:2540 do_xlate_actions (ofpacts=, ofpacts_len=, ctx=0x7f980f7e3730) at ../ofproto/ofproto-dpif-xlate.c:2833 xlate_actions__ (xin=0x7f980f7fda40, xout=0x7f980f7e41f0) at ../ofproto/ofproto-dpif-xlate.c:3485 xlate_actions (xin=0x7f980f7fda40, xout=0x7f980f7e41f0) at ../ofproto/ofproto-dpif-xlate.c:3223 xlate_actions_for_side_effects (xin=) at ../ofproto/ofproto-dpif-xlate.c:3136 handle_upcalls (n_upcalls=50, upcalls=0x7f980f7f3080, misses=0x7f980f7fd890, handler=) at ../ofproto/ofproto-dpif-upcall.c:973 udpif_upcall_handler (arg=0x23e91e0) at ../ofproto/ofproto-dpif-upcall.c:541 ovsthread_wrapper (aux_=) at ../lib/ovs-thread.c:322 start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 clone () from /lib/x86_64-linux-gnu/libc.so.6 ?? () The patch fixes this deadlock by using fat_rwlock that still allows to acquire read lock in a recursive manner. This bug is not present in master branch because commit 84f0f298 (ofproto-dpif-xlate: Implement RCU locking in ofproto-dpif-xlate) removed xlate_rwlock. VMware-BZ: #1425671 Reported-by: Scott Hendricks Signed-off-by: Ansis Atteka Acked-by: Ben Pfaff Acked-by: Alex Wang --- diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index b0f3f8991..c5dc9e825 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -66,7 +66,7 @@ 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 fat_rwlock xlate_rwlock; struct xbridge { struct hmap_node hmap_node; /* Node in global 'xbridges' map. */ @@ -639,7 +639,7 @@ xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet, const struct xport *xport; int error = ENODEV; - ovs_rwlock_rdlock(&xlate_rwlock); + fat_rwlock_rdlock(&xlate_rwlock); if (odp_flow_key_to_flow(key, key_len, flow) == ODP_FIT_ERROR) { error = EINVAL; goto exit; @@ -721,7 +721,7 @@ xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet, } exit: - ovs_rwlock_unlock(&xlate_rwlock); + fat_rwlock_unlock(&xlate_rwlock); return error; } @@ -3231,9 +3231,9 @@ void xlate_actions(struct xlate_in *xin, struct xlate_out *xout) OVS_EXCLUDED(xlate_rwlock) { - ovs_rwlock_rdlock(&xlate_rwlock); + fat_rwlock_rdlock(&xlate_rwlock); xlate_actions__(xin, xout); - ovs_rwlock_unlock(&xlate_rwlock); + fat_rwlock_unlock(&xlate_rwlock); } /* Returns the maximum number of packets that the Linux kernel is willing to @@ -3609,15 +3609,15 @@ xlate_send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet) flow_extract(packet, NULL, &flow); flow.in_port.ofp_port = OFPP_NONE; - ovs_rwlock_rdlock(&xlate_rwlock); + fat_rwlock_rdlock(&xlate_rwlock); xport = xport_lookup(ofport); if (!xport) { - ovs_rwlock_unlock(&xlate_rwlock); + fat_rwlock_unlock(&xlate_rwlock); return EINVAL; } output.port = xport->ofp_port; output.max_len = 0; - ovs_rwlock_unlock(&xlate_rwlock); + fat_rwlock_unlock(&xlate_rwlock); return ofproto_dpif_execute_actions(xport->xbridge->ofproto, &flow, NULL, &output.ofpact, sizeof output, diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h index 760736a64..2fdc5f118 100644 --- a/ofproto/ofproto-dpif-xlate.h +++ b/ofproto/ofproto-dpif-xlate.h @@ -136,7 +136,7 @@ struct xlate_in { struct xlate_cache *xcache; }; -extern struct ovs_rwlock xlate_rwlock; +extern struct fat_rwlock xlate_rwlock; void xlate_ofproto_set(struct ofproto_dpif *, const char *name, struct dpif *, struct rule_dpif *miss_rule, diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 2c150b2e2..9b872484c 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -410,6 +410,8 @@ init(const struct shash *iface_hints) { struct shash_node *node; + fat_rwlock_init(&xlate_rwlock); + /* Make a local copy, since we don't own 'iface_hints' elements. */ SHASH_FOR_EACH(node, iface_hints) { const struct iface_hint *orig_hint = node->data; @@ -598,7 +600,7 @@ type_run(const char *type) continue; } - ovs_rwlock_wrlock(&xlate_rwlock); + fat_rwlock_wrlock(&xlate_rwlock); xlate_ofproto_set(ofproto, ofproto->up.name, ofproto->backer->dpif, ofproto->miss_rule, ofproto->no_packet_in_rule, ofproto->ml, @@ -631,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); + fat_rwlock_unlock(&xlate_rwlock); } udpif_revalidate(backer->udpif); @@ -1311,9 +1313,9 @@ destruct(struct ofproto *ofproto_) struct list pins; ofproto->backer->need_revalidate = REV_RECONFIGURE; - ovs_rwlock_wrlock(&xlate_rwlock); + fat_rwlock_wrlock(&xlate_rwlock); xlate_remove_ofproto(ofproto); - ovs_rwlock_unlock(&xlate_rwlock); + fat_rwlock_unlock(&xlate_rwlock); /* Ensure that the upcall processing threads have no remaining references * to the ofproto or anything in it. */ @@ -1666,9 +1668,9 @@ port_destruct(struct ofport *port_) const char *dp_port_name; ofproto->backer->need_revalidate = REV_RECONFIGURE; - ovs_rwlock_wrlock(&xlate_rwlock); + fat_rwlock_wrlock(&xlate_rwlock); xlate_ofport_remove(port); - ovs_rwlock_unlock(&xlate_rwlock); + fat_rwlock_unlock(&xlate_rwlock); dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf, sizeof namebuf); @@ -2315,9 +2317,9 @@ bundle_destroy(struct ofbundle *bundle) ofproto = bundle->ofproto; mbridge_unregister_bundle(ofproto->mbridge, bundle); - ovs_rwlock_wrlock(&xlate_rwlock); + fat_rwlock_wrlock(&xlate_rwlock); xlate_bundle_remove(bundle); - ovs_rwlock_unlock(&xlate_rwlock); + fat_rwlock_unlock(&xlate_rwlock); 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 cee472313..5b0a1a8a6 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -59,7 +59,7 @@ enum { TBL_INTERNAL = N_TABLES - 1 }; /* Used for internal hidden rules. */ BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255); /* For lock annotation below only. */ -extern struct ovs_rwlock xlate_rwlock; +extern struct fat_rwlock xlate_rwlock; /* Ofproto-dpif -- DPIF based ofproto implementation. *