[ovs-dev] [PATCHv2 2/3] revalidator: Replace ukey->mark with dump_seq.

Joe Stringer joestringer at nicira.com
Fri Jun 6 02:27:10 UTC 2014


Recieved a verbal ack, so I applied this to master.


On 5 June 2014 09:26, Joe Stringer <joestringer at nicira.com> wrote:

> udpif->dump_seq already exists in master, and is changed by the leader
> revalidator thread.
>
>
> On 5 June 2014 06:29, Ethan Jackson <ethan at nicira.com> wrote:
>
>> Maybe I'm missing something. But I'm not seeing where the dump seq is
>> incremented
>>
>>
>> On Tuesday, June 3, 2014, Joe Stringer <joestringer at 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 <joestringer at nicira.com>
>>> ---
>>> v2: Rebase
>>> ---
>>>  ofproto/ofproto-dpif-upcall.c |   33 +++++++++++++++++----------------
>>>  1 file changed, 17 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/ofproto/ofproto-dpif-upcall.c
>>> b/ofproto/ofproto-dpif-upcall.c
>>> index 3a75690..90e18e3 100644
>>> --- a/ofproto/ofproto-dpif-upcall.c
>>> +++ b/ofproto/ofproto-dpif-upcall.c
>>> @@ -179,8 +179,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. */
>>>
>>> @@ -1018,7 +1017,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);
>>> @@ -1327,8 +1326,10 @@ revalidate(struct revalidator *revalidator)
>>>  {
>>>      struct udpif *udpif = revalidator->udpif;
>>>      struct dpif_flow_dump_thread *dump_thread;
>>> +    uint64_t dump_seq;
>>>      unsigned int flow_limit;
>>>
>>> +    dump_seq = seq_read(udpif->dump_seq);
>>>      atomic_read(&udpif->flow_limit, &flow_limit);
>>>      dump_thread = dpif_flow_dump_thread_create(udpif->dump);
>>>      for (;;) {
>>> @@ -1370,7 +1371,7 @@ revalidate(struct revalidator *revalidator)
>>>          for (f = flows; f < &flows[n_dumped]; f++) {
>>>              long long int used = f->stats.used;
>>>              struct udpif_key *ukey;
>>> -            bool already_dumped, mark;
>>> +            bool already_dumped, keep;
>>>
>>>              if (!ukey_acquire(udpif, f->key, f->key_len, used, &ukey)) {
>>>                  /* We couldn't acquire the ukey. This means that
>>> @@ -1380,7 +1381,7 @@ revalidate(struct revalidator *revalidator)
>>>                  continue;
>>>              }
>>>
>>> -            already_dumped = ukey->mark || !ukey->flow_exists;
>>> +            already_dumped = ukey->dump_seq == dump_seq;
>>>              if (already_dumped) {
>>>                  /* The flow has already been dumped and handled by
>>> another
>>>                   * revalidator during this flow dump operation. Skip
>>> it. */
>>> @@ -1393,13 +1394,14 @@ revalidate(struct revalidator *revalidator)
>>>                  used = ukey->created;
>>>              }
>>>              if (kill_them_all || (used && used < now - max_idle)) {
>>> -                mark = false;
>>> +                keep = false;
>>>              } else {
>>> -                mark = revalidate_ukey(udpif, ukey, f);
>>> +                keep = revalidate_ukey(udpif, ukey, f);
>>>              }
>>> -            ukey->mark = ukey->flow_exists = mark;
>>> +            ukey->dump_seq = dump_seq;
>>> +            ukey->flow_exists = keep;
>>>
>>> -            if (!mark) {
>>> +            if (!keep) {
>>>                  dump_op_init(&ops[n_ops++], f->key, f->key_len, ukey);
>>>              }
>>>              ovs_mutex_unlock(&ukey->mutex);
>>> @@ -1419,22 +1421,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 seen in the current dump, 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 at openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140606/d51e1dad/attachment-0005.html>


More information about the dev mailing list