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