This no longer applies. I plan to send a fresh version.
On 20 May 2014 09:12, Joe Stringer <joestrin...@nicira.com> wrote: > Rather than setting and resetting the 'mark' field in the ukey, this > patch introduces a seq to track whether a flow has been seen during the > most recent dump. This tidies the code and simplifies the logic for > detecting when flows are duplicated from the datapath. > > Signed-off-by: Joe Stringer <joestrin...@nicira.com> > --- > ofproto/ofproto-dpif-upcall.c | 34 ++++++++++++++++++---------------- > 1 file changed, 18 insertions(+), 16 deletions(-) > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index 522aa8c..6adc729 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -167,8 +167,7 @@ struct udpif_key { > struct ovs_mutex mutex; /* Guards the following. */ > struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/ > long long int created OVS_GUARDED; /* Estimate of creation > time. */ > - bool mark OVS_GUARDED; /* For mark and sweep > garbage > - collection. */ > + uint64_t dump_seq OVS_GUARDED; /* Tracks udpif->dump_seq. > */ > bool flow_exists OVS_GUARDED; /* Ensures flows are only > deleted > once. */ > > @@ -1131,7 +1130,7 @@ ukey_create(const struct nlattr *key, size_t > key_len, long long int used) > ukey->key_len = key_len; > > ovs_mutex_lock(&ukey->mutex); > - ukey->mark = false; > + ukey->dump_seq = 0; > ukey->flow_exists = true; > ukey->created = used ? used : time_msec(); > memset(&ukey->stats, 0, sizeof ukey->stats); > @@ -1441,6 +1440,7 @@ revalidate(struct revalidator *revalidator) > const struct nlattr *key, *mask, *actions; > size_t key_len, mask_len, actions_len; > const struct dpif_flow_stats *stats; > + uint64_t dump_seq; > long long int now; > unsigned int flow_limit; > size_t n_ops; > @@ -1448,13 +1448,14 @@ revalidate(struct revalidator *revalidator) > > n_ops = 0; > now = time_msec(); > + dump_seq = seq_read(udpif->dump_seq); > atomic_read(&udpif->flow_limit, &flow_limit); > > dpif_flow_dump_state_init(udpif->dpif, &state); > while (dpif_flow_dump_next(&udpif->dump, state, &key, &key_len, &mask, > &mask_len, &actions, &actions_len, > &stats)) { > struct udpif_key *ukey; > - bool mark, may_destroy; > + bool keep, may_destroy; > long long int used, max_idle; > uint32_t hash; > size_t n_flows; > @@ -1466,7 +1467,7 @@ revalidate(struct revalidator *revalidator) > if (!used && ukey) { > ovs_mutex_lock(&ukey->mutex); > > - if (ukey->mark || !ukey->flow_exists) { > + if (ukey->dump_seq == dump_seq) { > /* The flow has already been dumped. This can occasionally > * occur if the datapath is changed in the middle of a > flow > * dump. Rather than perform the same work twice, skip the > @@ -1480,6 +1481,7 @@ revalidate(struct revalidator *revalidator) > ovs_mutex_unlock(&ukey->mutex); > } > > + /* Delete flows more aggressively if over the limit. */ > n_flows = udpif_get_n_flows(udpif); > max_idle = ofproto_max_idle; > if (n_flows > flow_limit) { > @@ -1487,7 +1489,7 @@ revalidate(struct revalidator *revalidator) > } > > if ((used && used < now - max_idle) || n_flows > flow_limit * 2) { > - mark = false; > + keep = false; > } else { > if (!ukey) { > ukey = ukey_create(key, key_len, used); > @@ -1500,17 +1502,18 @@ revalidate(struct revalidator *revalidator) > } > } > > - mark = revalidate_ukey(udpif, ukey, mask, mask_len, actions, > + keep = revalidate_ukey(udpif, ukey, mask, mask_len, actions, > actions_len, stats); > } > > if (ukey) { > ovs_mutex_lock(&ukey->mutex); > - ukey->mark = ukey->flow_exists = mark; > + ukey->dump_seq = dump_seq; > + ukey->flow_exists = keep; > ovs_mutex_unlock(&ukey->mutex); > } > > - if (!mark) { > + if (!keep) { > dump_op_init(&ops[n_ops++], key, key_len, ukey); > } > > @@ -1548,22 +1551,21 @@ revalidator_sweep__(struct revalidator > *revalidator, bool purge) > struct dump_op ops[REVALIDATE_MAX_BATCH]; > struct udpif_key *ukey, *next; > size_t n_ops; > + uint64_t dump_seq; > > n_ops = 0; > + dump_seq = seq_read(revalidator->udpif->dump_seq); > > /* During garbage collection, this revalidator completely owns its > ukeys > * map, and therefore doesn't need to do any locking. */ > HMAP_FOR_EACH_SAFE (ukey, next, hmap_node, revalidator->ukeys) { > - if (!purge && ukey->mark) { > - ukey->mark = false; > - } else if (!ukey->flow_exists) { > + if (!ukey->flow_exists) { > ukey_delete(revalidator, ukey); > - } else { > + } else if (purge || ukey->dump_seq != dump_seq) { > struct dump_op *op = &ops[n_ops++]; > > - /* If we have previously seen a flow in the datapath, but > didn't > - * see it during the most recent dump, delete it. This allows > us > - * to clean up the ukey and keep the statistics consistent. */ > + /* If we have previously seen a flow in the datapath, but it > + * hasn't been revalidated with the current seq, delete it. > */ > dump_op_init(op, ukey->key, ukey->key_len, ukey); > if (n_ops == REVALIDATE_MAX_BATCH) { > push_dump_ops(revalidator, ops, n_ops); > -- > 1.7.10.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev