Bridges might get deleted while revalidation is going through the
xlate cache entries. Thus we need to do the xbridge lookup before we
use the xlate cache, and use uuid's instead of pointers on xlate cache
entries that might refer to other bridges.
This has the side effect of not updating the learned flow or mac table
on a peer bridge if the first bridge is deleted. Such cases should be
very rare though, as no-one has reported this bug so far. So it
appears that this corner-case is not work the code complication
covering it would cause.
Found by inspection.
Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
---
ofproto/ofproto-dpif-upcall.c | 12 ++++++------
ofproto/ofproto-dpif-xlate.c | 43 +++++++++++++++++++++++++++++++++----------
2 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 565de5b..0c6c354 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1883,12 +1883,6 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
goto exit;
}
- if (ukey->xcache && !need_revalidate) {
- xlate_push_stats(ukey->xcache, &push);
- result = UKEY_KEEP;
- goto exit;
- }
-
if (odp_flow_key_to_flow(ukey->key, ukey->key_len, &flow)
== ODP_FIT_ERROR) {
goto exit;
@@ -1902,7 +1896,13 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
if (need_revalidate) {
xlate_cache_clear(ukey->xcache);
+ } else if (ukey->xcache) {
+ /* Use xcache only after we have verified the switch exists. */
+ xlate_push_stats(ukey->xcache, &push);
+ result = UKEY_KEEP;
+ goto exit;
}
+
if (!ukey->xcache) {
ukey->xcache = xlate_cache_new();
}
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 1e2957c..f76cda6 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -490,12 +490,12 @@ struct xc_entry {
uint16_t vid;
} bond;
struct {
- struct ofproto_dpif *ofproto;
+ struct uuid ofproto_uuid;
struct ofputil_flow_mod *fm;
struct ofpbuf *ofpacts;
} learn;
struct {
- struct ofproto_dpif *ofproto;
+ struct uuid ofproto_uuid;
struct flow *flow;
int vlan;
} normal;
@@ -2488,7 +2488,8 @@ xlate_normal(struct xlate_ctx *ctx)
/* Save enough info to update mac learning table later. */
entry = xlate_cache_add_entry(ctx->xin->xcache, XC_NORMAL);
- entry->u.normal.ofproto = ctx->xbridge->ofproto;
+ entry->u.normal.ofproto_uuid =
+ *ofproto_dpif_get_uuid(ctx->xbridge->ofproto);
entry->u.normal.flow = xmemdup(flow, sizeof *flow);
entry->u.normal.vlan = vlan;
}
@@ -4165,7 +4166,8 @@ xlate_learn_action(struct xlate_ctx *ctx, const struct
ofpact_learn *learn)
struct xc_entry *entry;
entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN);
- entry->u.learn.ofproto = ctx->xbridge->ofproto;
+ entry->u.learn.ofproto_uuid =
+ *ofproto_dpif_get_uuid(ctx->xbridge->ofproto);
entry->u.learn.fm = xmalloc(sizeof *entry->u.learn.fm);
entry->u.learn.ofpacts = ofpbuf_new(64);
xlate_learn_action__(ctx, learn, entry->u.learn.fm,
@@ -5821,7 +5823,9 @@ xlate_cache_normal(struct ofproto_dpif *ofproto, struct
flow *flow, int vlan)
update_learning_table(xbridge, flow, &wc, vlan, xbundle);
}
-/* Push stats and perform side effects of flow translation. */
+/* Push stats and perform side effects of flow translation. Caller is
+ * responsible for making sure the bridge for which this cache was generated
+ * still exist. */
void
xlate_push_stats(struct xlate_cache *xcache,
const struct dpif_flow_stats *stats)
@@ -5855,13 +5859,32 @@ xlate_push_stats(struct xlate_cache *xcache,
entry->u.mirror.mirrors,
stats->n_packets, stats->n_bytes);
break;
- case XC_LEARN:
- ofproto_dpif_flow_mod(entry->u.learn.ofproto, entry->u.learn.fm);
+ case XC_LEARN: {
+ struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
+ struct xbridge *xbridge;
+
+ /* Check that the bridge still exists. */
+ xbridge = xbridge_lookup_by_uuid(xcfg,
+ &entry->u.learn.ofproto_uuid);
+
+ if (xbridge) {
+ ofproto_dpif_flow_mod(xbridge->ofproto, entry->u.learn.fm);
+ }
break;
- case XC_NORMAL:
- xlate_cache_normal(entry->u.normal.ofproto, entry->u.normal.flow,
- entry->u.normal.vlan);
+ }
+ case XC_NORMAL: {
+ struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
+ struct xbridge *xbridge;
+
+ /* Check that the bridge still exists. */
+ xbridge = xbridge_lookup_by_uuid(xcfg,
+ &entry->u.normal.ofproto_uuid);
+ if (xbridge) {
+ xlate_cache_normal(xbridge->ofproto, entry->u.normal.flow,
+ entry->u.normal.vlan);
+ }
break;
+ }
case XC_FIN_TIMEOUT:
xlate_fin_timeout__(entry->u.fin.rule, stats->tcp_flags,
entry->u.fin.idle, entry->u.fin.hard);
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
