[ovs-dev] [PATCH 1/6] ofproto: Hold ofproto_mutex when enabling or disabling eviction.

Ben Pfaff blp at nicira.com
Wed Jul 1 21:06:20 UTC 2015


Jarno, this patch is probably a good one for you to look at.  It's a
possible important bug fix and I know that you're knowledgeable about
the mutex in question.

(If you wanted to look at the rest of the series that would be nice too
but this patch in particular may be important.)

On Wed, Jun 24, 2015 at 10:57:01AM -0700, Ben Pfaff wrote:
> ofproto_enable_eviction() and ofproto_disable_eviction() require
> ofproto_mutex (and they were even annotated that way, though not on their
> prototypes but only at definition), but it wasn't being held.  This fixes
> the problem.
> 
> Found by inspection.
> 
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  ofproto/ofproto.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 08ba043..278c97f 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -83,10 +83,12 @@ static void oftable_set_name(struct oftable *, const char *name);
>  
>  static enum ofperr evict_rules_from_table(struct oftable *)
>      OVS_REQUIRES(ofproto_mutex);
> -static void oftable_disable_eviction(struct oftable *);
> +static void oftable_disable_eviction(struct oftable *)
> +    OVS_REQUIRES(ofproto_mutex);
>  static void oftable_enable_eviction(struct oftable *,
>                                      const struct mf_subfield *fields,
> -                                    size_t n_fields);
> +                                    size_t n_fields)
> +    OVS_REQUIRES(ofproto_mutex);
>  
>  /* A set of rules within a single OpenFlow table (oftable) that have the same
>   * values for the oftable's eviction_fields.  A rule to be evicted, when one is
> @@ -1419,13 +1421,6 @@ ofproto_configure_table(struct ofproto *ofproto, int table_id,
>          return;
>      }
>  
> -    if (s->groups) {
> -        oftable_enable_eviction(table, s->groups, s->n_groups);
> -    } else {
> -        oftable_disable_eviction(table);
> -    }
> -
> -    table->max_flows = s->max_flows;
>  
>      if (classifier_set_prefix_fields(&table->cls,
>                                       s->prefix_fields, s->n_prefix_fields)) {
> @@ -1433,6 +1428,12 @@ ofproto_configure_table(struct ofproto *ofproto, int table_id,
>      }
>  
>      ovs_mutex_lock(&ofproto_mutex);
> +    if (s->groups) {
> +        oftable_enable_eviction(table, s->groups, s->n_groups);
> +    } else {
> +        oftable_disable_eviction(table);
> +    }
> +    table->max_flows = s->max_flows;
>      evict_rules_from_table(table);
>      ovs_mutex_unlock(&ofproto_mutex);
>  }
> @@ -7431,7 +7432,9 @@ static void
>  oftable_destroy(struct oftable *table)
>  {
>      ovs_assert(classifier_is_empty(&table->cls));
> +    ovs_mutex_lock(&ofproto_mutex);
>      oftable_disable_eviction(table);
> +    ovs_mutex_unlock(&ofproto_mutex);
>      classifier_destroy(&table->cls);
>      free(table->name);
>  }
> -- 
> 2.1.3
> 



More information about the dev mailing list