Looks good,
Ethan
On Thu, Jan 26, 2012 at 15:53, Ben Pfaff <b...@nicira.com> wrote:
> I intend to use OFPFF_SEND_FLOW_REM in upcoming unit tests. I threw in
> OFPFF_CHECK_OVERLAP also because it didn't cost me anything. I omitted
> parsing support for OFPFF_EMERG because we opposed its inclusion from the
> start and it was removed from OpenFlow 1.2.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/ofp-parse.c | 15 ++++++++++-----
> lib/ofp-print.c | 17 ++++++++++++++++-
> tests/ovs-ofctl.at | 8 ++++----
> utilities/ovs-ofctl.8.in | 12 +++++++++++-
> 4 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> index 54622f0..9c82688 100644
> --- a/lib/ofp-parse.c
> +++ b/lib/ofp-parse.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2010, 2011 Nicira Networks.
> + * Copyright (c) 2010, 2011, 2012 Nicira Networks.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -493,7 +493,8 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command,
> const char *str_,
> F_OUT_PORT = 1 << 0,
> F_ACTIONS = 1 << 1,
> F_TIMEOUT = 1 << 3,
> - F_PRIORITY = 1 << 4
> + F_PRIORITY = 1 << 4,
> + F_FLAGS = 1 << 5,
> } fields;
> char *string = xstrdup(str_);
> char *save_ptr = NULL;
> @@ -506,7 +507,7 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command,
> const char *str_,
> break;
>
> case OFPFC_ADD:
> - fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY;
> + fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY | F_FLAGS;
> break;
>
> case OFPFC_DELETE:
> @@ -518,11 +519,11 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command,
> const char *str_,
> break;
>
> case OFPFC_MODIFY:
> - fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY;
> + fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY | F_FLAGS;
> break;
>
> case OFPFC_MODIFY_STRICT:
> - fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY;
> + fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY | F_FLAGS;
> break;
>
> default:
> @@ -562,6 +563,10 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command,
> const char *str_,
> if (p->nw_proto) {
> cls_rule_set_nw_proto(&fm->cr, p->nw_proto);
> }
> + } else if (fields & F_FLAGS && !strcmp(name, "send_flow_rem")) {
> + fm->flags |= OFPFF_SEND_FLOW_REM;
> + } else if (fields & F_FLAGS && !strcmp(name, "check_overlap")) {
> + fm->flags |= OFPFF_CHECK_OVERLAP;
> } else {
> char *value;
>
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index 9112c4b..a0fa7de 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -836,7 +836,22 @@ ofp_print_flow_mod(struct ds *s, const struct ofp_header
> *oh,
> ds_put_format(s, "buf:0x%"PRIx32" ", fm.buffer_id);
> }
> if (fm.flags != 0) {
> - ds_put_format(s, "flags:0x%"PRIx16" ", fm.flags);
> + uint16_t flags = fm.flags;
> +
> + if (flags & OFPFF_SEND_FLOW_REM) {
> + ds_put_cstr(s, "send_flow_rem ");
> + }
> + if (flags & OFPFF_CHECK_OVERLAP) {
> + ds_put_cstr(s, "check_overlap ");
> + }
> + if (flags & OFPFF_EMERG) {
> + ds_put_cstr(s, "emerg ");
> + }
> +
> + flags &= ~(OFPFF_SEND_FLOW_REM | OFPFF_CHECK_OVERLAP | OFPFF_EMERG);
> + if (flags) {
> + ds_put_format(s, "flags:0x%"PRIx16" ", flags);
> + }
> }
>
> ofp_print_actions(s, fm.actions, fm.n_actions);
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 0b58b3d..8d51f4b 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -25,8 +25,8 @@
> actions=bundle_load(symmetric_l4,60,hrw,ofport,NXM_NX_REG0[0..15],slaves:[2,3])
> actions=bundle_load(symmetric_l4,60,hrw,ofport,NXM_NX_REG0[0..30],slaves:)
> actions=output:1,bundle_load(eth_src,0,hrw,ofport,NXM_NX_REG0[16..31],slaves:1),output:2
> actions=resubmit:1,resubmit(2),resubmit(,3),resubmit(2,3)
> -actions=output:1,output:NXM_NX_REG0[],output:2,output:NXM_NX_REG1[16..31],output:3
> -actions=output:1,exit,output:2
> +send_flow_rem,actions=output:1,output:NXM_NX_REG0[],output:2,output:NXM_NX_REG1[16..31],output:3
> +check_overlap,actions=output:1,exit,output:2
> ]])
>
> AT_CHECK([ovs-ofctl parse-flows flows.txt
> @@ -56,8 +56,8 @@ NXT_FLOW_MOD: ADD table:255
> actions=bundle_load(symmetric_l4,60,hrw,ofport,NXM_N
> NXT_FLOW_MOD: ADD table:255
> actions=bundle_load(symmetric_l4,60,hrw,ofport,NXM_NX_REG0[0..30],slaves:)
> NXT_FLOW_MOD: ADD table:255
> actions=output:1,bundle_load(eth_src,0,hrw,ofport,NXM_NX_REG0[16..31],slaves:1),output:2
> NXT_FLOW_MOD: ADD table:255
> actions=resubmit:1,resubmit:2,resubmit(,3),resubmit(2,3)
> -NXT_FLOW_MOD: ADD table:255
> actions=output:1,output:NXM_NX_REG0[],output:2,output:NXM_NX_REG1[16..31],output:3
> -NXT_FLOW_MOD: ADD table:255 actions=output:1,exit,output:2
> +NXT_FLOW_MOD: ADD table:255 send_flow_rem
> actions=output:1,output:NXM_NX_REG0[],output:2,output:NXM_NX_REG1[16..31],output:3
> +NXT_FLOW_MOD: ADD table:255 check_overlap actions=output:1,exit,output:2
> ]])
> AT_CLEANUP
>
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index 78da223..bdfe554 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -992,7 +992,7 @@ support the following additional options. These options
> affect only
> new flows. Thus, for \fBadd\-flow\fR and \fBadd\-flows\fR, these
> options are always significant, but for \fBmod\-flows\fR they are
> significant only if the command creates a new flow, that is, their
> -values do not update existing flows.
> +values do not update or affect existing flows.
> .
> .IP "\fBidle_timeout=\fIseconds\fR"
> Causes the flow to expire after the given number of seconds of
> @@ -1004,6 +1004,16 @@ Causes the flow to expire after the given number of
> seconds,
> regardless of activity. A value of 0 (the default) gives the flow no
> hard expiration deadline.
> .
> +.IP "\fBsend_flow_rem\fR"
> +Marks the flow with a flag that causes the switch to generate a ``flow
> +removed'' message and send it to interested controllers when the flow
> +later expires or is removed.
> +.
> +.IP "\fBcheck_overlap\fR"
> +Forces the switch to check that the flow match does not overlap that
> +of any different flow with the same priority in the same table. (This
> +check is expensive so it is best to avoid it.)
> +.
> .PP
> The \fBdump\-flows\fR, \fBdump\-aggregate\fR, \fBdel\-flow\fR
> and \fBdel\-flows\fR commands support one additional optional field:
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
