[ovs-dev] [PATCH 2/2] ofproto-dpif: Add ability to disable megaflows.

Ethan Jackson ethan at nicira.com
Sat Jun 29 00:37:49 UTC 2013


Please put a period after the Feature number.  Not sure why, but
that's been the convention.

Instead of calling the variable "disable_megaflows", I think the code
would be a bit easier to read if we called it "megaflows" and
initialize it to true.  Negative flags are a bit confusing.

I don't think we need to bother making this a per-ofproto switch.  Any
reason not to make it a static global?  I think it'd be easier to use
that way.

Ethan


On Fri, Jun 28, 2013 at 5:21 PM, Justin Pettit <jpettit at nicira.com> wrote:
> Add new "dpif/disable-megaflows" and "dpif/enable-megaflows" commands to
> ovs-appctl to disable and enable megaflows, respectively.  By default,
> megaflows are enabled, and these commands should only be used for
> debugging.
>
> Feature #18265
>
> Requested-by: Ronghua Zhang <rzhang at vmware.com>
> Signed-off-by: Justin Pettit <jpettit at nicira.com>
> ---
>  ofproto/ofproto-dpif.c |   64 +++++++++++++++++++++++++++++++++++++++++++++---
>  ofproto/ofproto-dpif.h |    4 +++
>  2 files changed, 64 insertions(+), 4 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 5ca16b7..92da91d 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1080,6 +1080,8 @@ construct(struct ofproto *ofproto_)
>      ofproto->n_hit = 0;
>      ofproto->n_missed = 0;
>
> +    ofproto->disable_megaflows = false;
> +
>      return error;
>  }
>
> @@ -3476,8 +3478,10 @@ handle_flow_miss_with_facet(struct flow_miss *miss, struct facet *facet,
>          subfacet->path = want_path;
>
>          ofpbuf_use_stack(&op->mask, &op->maskbuf, sizeof op->maskbuf);
> -        odp_flow_key_from_mask(&op->mask, &facet->xout.wc.masks,
> -                               &miss->flow, UINT32_MAX);
> +        if (!ofproto->disable_megaflows) {
> +            odp_flow_key_from_mask(&op->mask, &facet->xout.wc.masks,
> +                                   &miss->flow, UINT32_MAX);
> +        }
>
>          op->xout_garbage = false;
>          op->dpif_op.type = DPIF_OP_FLOW_PUT;
> @@ -5064,8 +5068,10 @@ subfacet_install(struct subfacet *subfacet, const struct ofpbuf *odp_actions,
>      }
>
>      ofpbuf_use_stack(&mask, &maskbuf, sizeof maskbuf);
> -    odp_flow_key_from_mask(&mask, &facet->xout.wc.masks,
> -                           &facet->flow, UINT32_MAX);
> +    if (!ofproto->disable_megaflows) {
> +        odp_flow_key_from_mask(&mask, &facet->xout.wc.masks,
> +                               &facet->flow, UINT32_MAX);
> +    }
>
>      ret = dpif_flow_put(subfacet->backer->dpif, flags, subfacet->key,
>                          subfacet->key_len,  mask.data, mask.size,
> @@ -6388,6 +6394,52 @@ ofproto_unixctl_dpif_dump_megaflows(struct unixctl_conn *conn,
>      ds_destroy(&ds);
>  }
>
> +/* Disable using the megaflows.
> + *
> + * This command is only needed for advanced debugging, so it's not
> + * documented in the man page. */
> +static void
> +ofproto_unixctl_dpif_disable_megaflows(struct unixctl_conn *conn,
> +                                       int argc OVS_UNUSED, const char *argv[],
> +                                       void *aux OVS_UNUSED)
> +{
> +    struct ofproto_dpif *ofproto;
> +
> +    ofproto = ofproto_dpif_lookup(argv[1]);
> +    if (!ofproto) {
> +        unixctl_command_reply_error(conn, "no such bridge");
> +        return;
> +    }
> +
> +    ofproto->disable_megaflows = true;
> +    flush(&ofproto->up);
> +
> +    unixctl_command_reply(conn, "megaflows disabled");
> +}
> +
> +/* Re-enable using megaflows.
> + *
> + * This command is only needed for advanced debugging, so it's not
> + * documented in the man page. */
> +static void
> +ofproto_unixctl_dpif_enable_megaflows(struct unixctl_conn *conn,
> +                                      int argc OVS_UNUSED, const char *argv[],
> +                                      void *aux OVS_UNUSED)
> +{
> +    struct ofproto_dpif *ofproto;
> +
> +    ofproto = ofproto_dpif_lookup(argv[1]);
> +    if (!ofproto) {
> +        unixctl_command_reply_error(conn, "no such bridge");
> +        return;
> +    }
> +
> +    ofproto->disable_megaflows = false;
> +    flush(&ofproto->up);
> +
> +    unixctl_command_reply(conn, "megaflows enabled");
> +}
> +
>  static void
>  ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn,
>                                  int argc OVS_UNUSED, const char *argv[],
> @@ -6501,6 +6553,10 @@ ofproto_dpif_unixctl_init(void)
>                               ofproto_unixctl_dpif_del_flows, NULL);
>      unixctl_command_register("dpif/dump-megaflows", "bridge", 1, 1,
>                               ofproto_unixctl_dpif_dump_megaflows, NULL);
> +    unixctl_command_register("dpif/disable-megaflows", "bridge", 1, 1,
> +                             ofproto_unixctl_dpif_disable_megaflows, NULL);
> +    unixctl_command_register("dpif/enable-megaflows", "bridge", 1, 1,
> +                             ofproto_unixctl_dpif_enable_megaflows, NULL);
>  }
>
>  /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
> index 0704297..eeb41c3 100644
> --- a/ofproto/ofproto-dpif.h
> +++ b/ofproto/ofproto-dpif.h
> @@ -120,6 +120,10 @@ struct ofproto_dpif {
>      /* Per ofproto's dpif stats. */
>      uint64_t n_hit;
>      uint64_t n_missed;
> +
> +    /* Disable wildcarding the datapath's flows (megaflows) when
> +     * "ovs-appctl dpif/disable-megaflows <br>" is called. */
> +    bool disable_megaflows;
>  };
>
>  struct ofport_dpif {
> --
> 1.7.5.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
X-CudaMail-Whitelist-To: dev at openvswitch.org



More information about the dev mailing list