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=<optimized out>, 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=<optimized out>) at ../lib/stp.c:1301
stp_received_tcn_bpdu (p=<optimized out>, stp=<optimized out>) at ../lib/stp.c:1353
stp_received_bpdu (p=0x2400c00, bpdu=0x7f980f7f81e9, bpdu_size=<optimized out>) at ../lib/stp.c:771
stp_process_packet (packet=0x7f980f7f80f8, xport=0x24594b0) at ../ofproto/ofproto-dpif-xlate.c:840
process_special (flow=<optimized out>, xport=0x24594b0, packet=0x7f980f7f80f8, ctx=<optimized out>) at ../ofproto/ofproto-dpif-xlate.c:1832
compose_output_action__ (ctx=0x7f980f7e3730, ofp_port=<optimized out>, check_stp=true) at ../ofproto/ofproto-dpif-xlate.c:1894
compose_output_action (ofp_port=<optimized out>, ctx=0x7f980f7e3730) at ../ofproto/ofproto-dpif-xlate.c:2031
output_normal (ctx=0x7f980f7e3730, out_xbundle=0x23d13a0, vlan=<optimized out>) at ../ofproto/ofproto-dpif-xlate.c:1316
xlate_normal (ctx=0x7f980f7e3730) at ../ofproto/ofproto-dpif-xlate.c:1625
xlate_output_action (ctx=0x7f980f7e3730, port=<optimized out>, max_len=<optimized out>, may_packet_in=<optimized out>) at ../ofproto/ofproto-dpif-xlate.c:2540
do_xlate_actions (ofpacts=<optimized out>, ofpacts_len=<optimized out>, 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=<optimized out>) at ../ofproto/ofproto-dpif-xlate.c:3136
handle_upcalls (n_upcalls=50, upcalls=0x7f980f7f3080, misses=0x7f980f7fd890, handler=<optimized out>) at ../ofproto/ofproto-dpif-upcall.c:973
udpif_upcall_handler (arg=0x23e91e0) at ../ofproto/ofproto-dpif-upcall.c:541
ovsthread_wrapper (aux_=<optimized out>) 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 <shendricks@nicira.com>
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
* 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. */
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;
}
exit:
- ovs_rwlock_unlock(&xlate_rwlock);
+ fat_rwlock_unlock(&xlate_rwlock);
return error;
}
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
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,
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,
{
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;
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,
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);
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. */
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);
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);
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.
*