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 <b...@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
> 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to