[ovs-dev] [PATCH] ofproto-dpif: Delete un-fit flow from datapath.

Ben Pfaff blp at nicira.com
Wed Nov 30 21:35:30 UTC 2011


This not only improves logging, it's also a nice cleanup.  Thank you, I
didn't expect that part.

I see one nit: in the update_stats() case where we log, please use
VLOG_DROP_WARN() followed by plain VLOG_WARN() to avoid formatting the
log message in the case where it is just going to get discarded.

Thanks,

Ben.

On Wed, Nov 30, 2011 at 01:34:50PM -0800, Pravin B Shelar wrote:
> Fixed according to comments from Ben.
> 
> --8<--------------------------cut here-------------------------->8--
> 
> Treat unexpected and un-fit flow in a similar manner.
> 
> ---
>  ofproto/ofproto-dpif.c |   31 ++++++++++++++++++-------------
>  1 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 14b5447..853a170 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -373,8 +373,7 @@ static struct subfacet *subfacet_create(struct ofproto_dpif *, struct facet *,
>                                          const struct nlattr *key,
>                                          size_t key_len, ovs_be16 initial_tci);
>  static struct subfacet *subfacet_find(struct ofproto_dpif *,
> -                                      const struct nlattr *key, size_t key_len,
> -                                      const struct flow *);
> +                                      const struct nlattr *key, size_t key_len);
>  static void subfacet_destroy(struct ofproto_dpif *, struct subfacet *);
>  static void subfacet_destroy__(struct ofproto_dpif *, struct subfacet *);
>  static void subfacet_reset_dp_stats(struct subfacet *,
> @@ -2782,16 +2781,9 @@ update_stats(struct ofproto_dpif *p)
>  
>      dpif_flow_dump_start(&dump, p->dpif);
>      while (dpif_flow_dump_next(&dump, &key, &key_len, NULL, NULL, &stats)) {
> -        enum odp_key_fitness fitness;
>          struct subfacet *subfacet;
> -        struct flow flow;
>  
> -        fitness = odp_flow_key_to_flow(key, key_len, &flow);
> -        if (fitness == ODP_FIT_ERROR) {
> -            continue;
> -        }
> -
> -        subfacet = subfacet_find(p, key, key_len, &flow);
> +        subfacet = subfacet_find(p, key, key_len);
>          if (subfacet && subfacet->installed) {
>              struct facet *facet = subfacet->facet;
>  
> @@ -2815,8 +2807,15 @@ update_stats(struct ofproto_dpif *p)
>              facet_account(p, facet);
>              facet_push_stats(facet);
>          } else {
> +            struct ds s;
> +
>              /* There's a flow in the datapath that we know nothing about, or a
>               * flow that shouldn't be installed but was anyway.  Delete it. */
> +            ds_init(&s);
> +            odp_flow_key_format(key, key_len, &s);
> +            VLOG_WARN_RL(&rl, "unexpected flow from datapath %s", ds_cstr(&s));
> +            ds_destroy(&s);
> +
>              COVERAGE_INC(facet_unexpected);
>              dpif_flow_del(p->dpif, key, key_len, NULL);
>          }
> @@ -3508,12 +3507,18 @@ subfacet_create(struct ofproto_dpif *ofproto, struct facet *facet,
>   * 'flow'.  Returns the subfacet if one exists, otherwise NULL. */
>  static struct subfacet *
>  subfacet_find(struct ofproto_dpif *ofproto,
> -              const struct nlattr *key, size_t key_len,
> -              const struct flow *flow)
> +              const struct nlattr *key, size_t key_len)
>  {
>      uint32_t key_hash = odp_flow_key_hash(key, key_len);
> +    enum odp_key_fitness fitness;
> +    struct flow flow;
> +
> +    fitness = odp_flow_key_to_flow(key, key_len, &flow);
> +    if (fitness == ODP_FIT_ERROR) {
> +        return NULL;
> +    }
>  
> -    return subfacet_find__(ofproto, key, key_len, key_hash, flow);
> +    return subfacet_find__(ofproto, key, key_len, key_hash, &flow);
>  }
>  
>  /* Uninstalls 'subfacet' from the datapath, if it is installed, removes it from
> -- 
> 1.7.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list