[ovs-dev] [slow path 08/11] ofproto-dpif: Move code closer to left margin in facet_check_consistency().

Ethan Jackson ethan at nicira.com
Tue May 8 20:50:53 UTC 2012


Looks good, thanks.

Ethan

On Sat, May 5, 2012 at 11:10 AM, Ben Pfaff <blp at nicira.com> wrote:
> This makes an upcoming commit break up fewer lines.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  ofproto/ofproto-dpif.c |   82 +++++++++++++++++++++++++-----------------------
>  1 files changed, 43 insertions(+), 39 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 62cd768..3df6f27 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3741,9 +3741,12 @@ facet_check_consistency(struct facet *facet)
>     /* Check the datapath actions for consistency. */
>     ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof odp_actions_stub);
>     LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) {
> +        struct odputil_keybuf keybuf;
>         struct action_xlate_ctx ctx;
>         bool actions_changed;
>         bool should_install;
> +        struct ofpbuf key;
> +        struct ds s;
>
>         action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
>                               subfacet->initial_tci, rule, 0, NULL);
> @@ -3761,48 +3764,49 @@ facet_check_consistency(struct facet *facet)
>         actions_changed = (subfacet->actions_len != odp_actions.size
>                            || memcmp(subfacet->actions, odp_actions.data,
>                                      subfacet->actions_len));
> -        if (should_install != subfacet->installed || actions_changed) {
> -            if (ok) {
> -                may_log = !VLOG_DROP_WARN(&rl);
> -                ok = false;
> -            }
> -
> -            if (may_log) {
> -                struct odputil_keybuf keybuf;
> -                struct ofpbuf key;
> -                struct ds s;
> -
> -                ds_init(&s);
> -                subfacet_get_key(subfacet, &keybuf, &key);
> -                odp_flow_key_format(key.data, key.size, &s);
> +        if (should_install == subfacet->installed && !actions_changed) {
> +            continue;
> +        }
>
> -                ds_put_cstr(&s, ": inconsistency in subfacet");
> -                if (should_install != subfacet->installed) {
> -                    enum odp_key_fitness fitness = subfacet->key_fitness;
> +        /* Inconsistency! */
> +        if (ok) {
> +            may_log = !VLOG_DROP_WARN(&rl);
> +            ok = false;
> +        }
> +        if (!may_log) {
> +            /* Rate-limited, skip reporting. */
> +            continue;
> +        }
>
> -                    ds_put_format(&s, " (should%s have been installed)",
> -                                  should_install ? "" : " not");
> -                    ds_put_format(&s, " (may_set_up_flow=%s, fitness=%s)",
> -                                  ctx.may_set_up_flow ? "true" : "false",
> -                                  odp_key_fitness_to_string(fitness));
> -                }
> -                if (actions_changed) {
> -                    ds_put_cstr(&s, " (actions were: ");
> -                    format_odp_actions(&s, subfacet->actions,
> -                                       subfacet->actions_len);
> -                    ds_put_cstr(&s, ") (correct actions: ");
> -                    format_odp_actions(&s, odp_actions.data, odp_actions.size);
> -                    ds_put_char(&s, ')');
> -                } else {
> -                    ds_put_cstr(&s, " (actions: ");
> -                    format_odp_actions(&s, subfacet->actions,
> -                                       subfacet->actions_len);
> -                    ds_put_char(&s, ')');
> -                }
> -                VLOG_WARN("%s", ds_cstr(&s));
> -                ds_destroy(&s);
> -            }
> +        ds_init(&s);
> +        subfacet_get_key(subfacet, &keybuf, &key);
> +        odp_flow_key_format(key.data, key.size, &s);
> +
> +        ds_put_cstr(&s, ": inconsistency in subfacet");
> +        if (should_install != subfacet->installed) {
> +            enum odp_key_fitness fitness = subfacet->key_fitness;
> +
> +            ds_put_format(&s, " (should%s have been installed)",
> +                          should_install ? "" : " not");
> +            ds_put_format(&s, " (may_set_up_flow=%s, fitness=%s)",
> +                          ctx.may_set_up_flow ? "true" : "false",
> +                          odp_key_fitness_to_string(fitness));
> +        }
> +        if (actions_changed) {
> +            ds_put_cstr(&s, " (actions were: ");
> +            format_odp_actions(&s, subfacet->actions,
> +                               subfacet->actions_len);
> +            ds_put_cstr(&s, ") (correct actions: ");
> +            format_odp_actions(&s, odp_actions.data, odp_actions.size);
> +            ds_put_char(&s, ')');
> +        } else {
> +            ds_put_cstr(&s, " (actions: ");
> +            format_odp_actions(&s, subfacet->actions,
> +                               subfacet->actions_len);
> +            ds_put_char(&s, ')');
>         }
> +        VLOG_WARN("%s", ds_cstr(&s));
> +        ds_destroy(&s);
>     }
>     ofpbuf_uninit(&odp_actions);
>
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list