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 <jpet...@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 <rzh...@vmware.com>
> Signed-off-by: Justin Pettit <jpet...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
X-CudaMail-Whitelist-To: dev@openvswitch.org
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
