[ovs-dev] [PATCH branch-2.3] ofproto-dpif: Use fat_rwlock instead of ovs_rwlock.
Ansis Atteka
aatteka at nicira.com
Tue Apr 7 04:18:58 UTC 2015
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.
The patch fixes this deadlock by using fat_rwlock that still allows
to acquire read lock in recursive manner.
VMware-BZ: #1425671
Reported-by: Scott Hendricks <shendricks at nicira.com>
Signed-off-by: Ansis Atteka <aatteka at nicira.com>
---
ofproto/ofproto-dpif-xlate.c | 16 ++++++++--------
ofproto/ofproto-dpif-xlate.h | 2 +-
ofproto/ofproto-dpif.c | 18 ++++++++++--------
ofproto/ofproto-dpif.h | 2 +-
4 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index b0f3f89..c5dc9e8 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 760736a..2fdc5f1 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 2c150b2..9b87248 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 cee4723..5b0a1a8 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.
*
--
1.9.1
More information about the dev
mailing list