On Mon, Sep 17, 2012 at 03:53:28PM -0700, Ben Pfaff wrote:
> On Mon, Sep 17, 2012 at 09:26:48AM +0900, Simon Horman wrote:
> > Open Flow 1.1 and 1.2 make use of 32 bit ports, however Open vSwtich maps
> > them to 16 bits. Make ovs-ofputl aware of this.
> >
> > Also, only accept ports that fit into 16 bits for Open Flow 1.0.
> >
> > Signed-off-by: Simon Horman <ho...@verge.net.au>
>
> This makes the acceptable port numbers a function of the protocol we
> end up with, but the ovs-ofctl philosophy has always been that you
> tell it what you want and it'll pick an acceptable protocol for doing
> what you asked for. There's also an issue of some confusion over
> whether, say, port 65535 is OFPP_NONE (OF1.0) or just an
> undistinguished "physical" port (OF1.1). I think we can do better.
>
> Here's a counterproposal. What do you think?
>
> Thanks,
>
> Ben.
>
> --8<--------------------------cut here-------------------------->8--
>
> From: Ben Pfaff <b...@nicira.com>
> Date: Mon, 17 Sep 2012 15:49:59 -0700
> Subject: [PATCH] ovs-ofctl: Accept port keywords, OF1.1 port numbers, reject
> port number 0.
>
> OpenFlow 1.0 has special reserved ports in the range 0xfff8 to 0xffff.
> OpenFlow 1.1 and later has the same ports in the range 0xfffffff8 to
> 0xffffffff and allows the OF1.0 range to be used for ordinary ("physical")
> switch ports. This means that, naively, the meaning of a port number in
> the range 0xfff8 to 0xffff given on the ovs-ofctl command line depends on
> the protocol in use. This commit implements something a little smarter:
>
> - Accept keyword names (e.g. LOCAL) for special reserved ports
> everywhere that such a port can plausibly be used (previously they
> were only accepted in some places).
>
> - Translate 0xfff8...0xffff to 0xfffffff8...0xffffffff for now, since
> OF1.1+ isn't in widespread use and those particular ports aren't
> likely to be in use in OF1.1+ anyway.
I don't really like the above assumption, 0xfff8...0xffff
are valid OF1.1+ port numbers, it seems that it would
cause rather a surprise if they were used as non-reserved ports
but Open vSwtich interpreted them as reserved ports.
I am prepared to live with it, but I don't like it.
> - Log warnings about those ports when they are specified by number, to
> allow users to fix their invocations.
>
> Also:
>
> - Accept the OF1.1+ port numbers for these ports, without warning, for
> compatibility with the upcoming OF1.1+ support.
>
> - Stop accepting port number 0, which has never been a valid port
> number in OpenFlow 1.0 and later. (This required fixing some tests
> that inadvertently used this port number).
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> NEWS | 8 ++
> include/openflow/openflow-1.0.h | 16 +++-
> lib/autopath.c | 9 +--
> lib/bundle.c | 9 ++-
> lib/meta-flow.c | 3 +-
> lib/ofp-actions.c | 7 ++-
> lib/ofp-parse.c | 25 ++++---
> lib/ofp-util.c | 84 ++++++++++++++++------
> lib/ofp-util.h | 2 +-
> tests/autopath.at | 2 +-
> tests/ofproto.at | 148 +++++++++++++++++++-------------------
> utilities/ovs-ofctl.8.in | 79 ++++++++++++---------
> utilities/ovs-ofctl.c | 9 +--
> 13 files changed, 234 insertions(+), 167 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index cbc5c58..38e5129 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -9,6 +9,14 @@ post-v1.8.0
> - OpenFlow:
> - Allow bitwise masking for SHA and THA fields in ARP, SLL and TLL
> fields in IPv6 neighbor discovery messages, and IPv6 flow label.
> + - ovs-ofctl:
> + - Commands and actions that accept port numbers now also accept
> keywords
> + that represent those ports (such as LOCAL, NONE, and ALL). This is
> + also the recommended way to specify these ports, for compatibility
> + with OpenFlow 1.1 and later (which use the OpenFlow 1.0 numbers
> + for these ports for different purposes).
> + - Commands and actions that accept port numbers no longer accept port
> 0,
> + which is not a valid port number in OpenFlow 1.0 and later.
> - ovs-dpctl:
> - Support requesting the port number with the "port_no" option in
> the "add-if" command.
> diff --git a/include/openflow/openflow-1.0.h b/include/openflow/openflow-1.0.h
> index 56af4c5..9af7740 100644
> --- a/include/openflow/openflow-1.0.h
> +++ b/include/openflow/openflow-1.0.h
> @@ -21,12 +21,20 @@
>
> #include "openflow/openflow-common.h"
>
> -/* Port numbering. Physical ports are numbered starting from 1. */
> +/* Port number(s) meaning
> + * --------------- --------------------------------------
> + * 0x0000 not assigned a meaning by OpenFlow 1.0
> + * 0x0001...0xfeff "physical" ports
> + * 0xff00...0xfff7 "reserved" but not assigned a meaning by OpenFlow 1.0
> + * 0xfff8...0xffff "reserved" OFPP_* ports with assigned meanings
> + */
> enum ofp_port {
> - /* Maximum number of physical switch ports. */
> - OFPP_MAX = 0xff00,
> + /* Ranges. */
> + OFPP_MAX = 0xff00, /* Maximum number of physical switch ports.
> */
> + OFPP_FIRST_RESV = 0xfff8, /* First assigned reserved port number. */
> + OFPP_LAST_RESV = 0xffff, /* Last assigned reserved port number. */
>
> - /* Fake output "ports". */
> + /* Reserved output "ports". */
> OFPP_IN_PORT = 0xfff8, /* Send the packet out the input port. This
> virtual port must be explicitly used
> in order to send back out of the input
> diff --git a/lib/autopath.c b/lib/autopath.c
> index ae8007d..b204e84 100644
> --- a/lib/autopath.c
> +++ b/lib/autopath.c
> @@ -36,7 +36,6 @@ void
> autopath_parse(struct ofpact_autopath *ap, const char *s_)
> {
> char *s;
> - int id_int;
> char *id_str, *dst, *save_ptr;
>
> ofpact_init_AUTOPATH(ap);
> @@ -50,12 +49,10 @@ autopath_parse(struct ofpact_autopath *ap, const char *s_)
> ovs_fatal(0, "%s: not enough arguments to autopath action", s_);
> }
>
> - id_int = atoi(id_str);
> - if (id_int < 1 || id_int > UINT32_MAX) {
> - ovs_fatal(0, "%s: autopath id %d is not in valid range "
> - "1 to %"PRIu32, s_, id_int, UINT32_MAX);
> + ap->port = ofputil_port_from_string(id_str);
> + if (!ap->port) {
> + ovs_fatal(0, "%s: bad port number", s_);
> }
> - ap->port = id_int;
>
> mf_parse_subfield(&ap->dst, dst);
> if (ap->dst.n_bits < 16) {
> diff --git a/lib/bundle.c b/lib/bundle.c
> index e0f8e6b..b68ebab 100644
> --- a/lib/bundle.c
> +++ b/lib/bundle.c
> @@ -267,12 +267,15 @@ bundle_parse__(const char *s, char **save_ptr,
> uint16_t slave_port;
> char *slave;
>
> - slave = strtok_r(NULL, ", [", save_ptr);
> + slave = strtok_r(NULL, ", []", save_ptr);
> if (!slave || bundle->n_slaves >= BUNDLE_MAX_SLAVES) {
> break;
> }
>
> - slave_port = atoi(slave);
> + slave_port = ofputil_port_from_string(slave);
> + if (!slave_port) {
> + ovs_fatal(0, "%s: bad port number", slave);
> + }
> ofpbuf_put(ofpacts, &slave_port, sizeof slave_port);
>
> bundle = ofpacts->l2;
> @@ -387,7 +390,7 @@ bundle_format(const struct ofpact_bundle *bundle, struct
> ds *s)
> ds_put_cstr(s, ",");
> }
>
> - ds_put_format(s, "%"PRIu16, bundle->slaves[i]);
> + ofputil_format_port(bundle->slaves[i], s);
> }
>
> ds_put_cstr(s, ")");
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index aa44ce8..38c9a27 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -1941,7 +1941,8 @@ mf_from_ofp_port_string(const struct mf_field *mf,
> const char *s,
> uint16_t port;
>
> assert(mf->n_bytes == sizeof(ovs_be16));
> - if (ofputil_port_from_string(s, &port)) {
> + port = ofputil_port_from_string(s);
> + if (port) {
> *valuep = htons(port);
> *maskp = htons(UINT16_MAX);
> return NULL;
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index 210f4ce..19ed7a0 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -1770,7 +1770,8 @@ ofpact_format(const struct ofpact *a, struct ds *s)
> case OFPACT_RESUBMIT:
> resubmit = ofpact_get_RESUBMIT(a);
> if (resubmit->in_port != OFPP_IN_PORT && resubmit->table_id == 255) {
> - ds_put_format(s, "resubmit:%"PRIu16, resubmit->in_port);
> + ds_put_cstr(s, "resubmit:");
> + ofputil_format_port(resubmit->in_port, s);
> } else {
> ds_put_format(s, "resubmit(");
> if (resubmit->in_port != OFPP_IN_PORT) {
> @@ -1794,7 +1795,9 @@ ofpact_format(const struct ofpact *a, struct ds *s)
>
> case OFPACT_AUTOPATH:
> autopath = ofpact_get_AUTOPATH(a);
> - ds_put_format(s, "autopath(%u,", autopath->port);
> + ds_put_cstr(s, "autopath(");
> + ofputil_format_port(autopath->port, s);
> + ds_put_char(s, ',');
> mf_format_subfield(&autopath->dst, s);
> ds_put_char(s, ')');
> break;
> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> index 165a36e..e3b7dc1 100644
> --- a/lib/ofp-parse.c
> +++ b/lib/ofp-parse.c
> @@ -168,8 +168,9 @@ parse_resubmit(char *arg, struct ofpbuf *ofpacts)
>
> in_port_s = strsep(&arg, ",");
> if (in_port_s && in_port_s[0]) {
> - if (!ofputil_port_from_string(in_port_s, &resubmit->in_port)) {
> - resubmit->in_port = str_to_u32(in_port_s);
> + resubmit->in_port = ofputil_port_from_string(in_port_s);
> + if (!resubmit->in_port) {
> + ovs_fatal(0, "%s: resubmit to unknown port", in_port_s);
> }
> } else {
> resubmit->in_port = OFPP_IN_PORT;
> @@ -488,10 +489,7 @@ str_to_ofpacts(const struct flow *flow, char *str,
> struct ofpbuf *ofpacts)
> pos = str;
> n_actions = 0;
> while (ofputil_parse_key_value(&pos, &act, &arg)) {
> - uint16_t port;
> - int code;
> -
> - code = ofputil_action_code_from_name(act);
> + int code = ofputil_action_code_from_name(act);
> if (code >= 0) {
> parse_named_action(code, flow, arg, ofpacts);
> } else if (!strcasecmp(act, "drop")) {
> @@ -503,10 +501,13 @@ str_to_ofpacts(const struct flow *flow, char *str,
> struct ofpbuf *ofpacts)
> "actions");
> }
> break;
> - } else if (ofputil_port_from_string(act, &port)) {
> - ofpact_put_OUTPUT(ofpacts)->port = port;
> } else {
> - ovs_fatal(0, "Unknown action: %s", act);
> + uint16_t port = ofputil_port_from_string(act);
> + if (port) {
> + ofpact_put_OUTPUT(ofpacts)->port = port;
> + } else {
> + ovs_fatal(0, "Unknown action: %s", act);
> + }
> }
> n_actions++;
> }
> @@ -681,7 +682,11 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command,
> const char *str_,
> if (!strcmp(name, "table")) {
> fm->table_id = str_to_table_id(value);
> } else if (!strcmp(name, "out_port")) {
> - fm->out_port = atoi(value);
> + fm->out_port = ofputil_port_from_string(name);
> + if (!fm->out_port) {
> + ofp_fatal(str_, verbose, "%s is not a valid OpenFlow
> port",
> + name);
> + }
> } else if (fields & F_PRIORITY && !strcmp(name, "priority")) {
> fm->priority = str_to_u16(value, name);
> } else if (fields & F_TIMEOUT && !strcmp(name, "idle_timeout")) {
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 300ef4c..fc98894 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -3536,36 +3536,72 @@ ofputil_check_output_port(uint16_t port, int
> max_ports)
> OFPUTIL_NAMED_PORT(LOCAL) \
> OFPUTIL_NAMED_PORT(NONE)
>
> -/* Checks whether 's' is the string representation of an OpenFlow port
> number,
> - * either as an integer or a string name (e.g. "LOCAL"). If it is, stores
> the
> - * number in '*port' and returns true. Otherwise, returns false. */
> -bool
> -ofputil_port_from_string(const char *name, uint16_t *port)
> +/* Returns the port number represented by 's', which may be an integer or,
> for
> + * reserved ports, the standard OpenFlow name for the port (e.g. "LOCAL").
> + *
> + * Returns 0 if 's' is not a valid OpenFlow port number or name. The caller
> + * should issue an error message in this case, because this function usually
> + * does not. (This gives the caller an opportunity to look up the port name
> + * another way, e.g. by contacting the switch and listing the names of all
> its
> + * ports).
> + *
> + * This function accepts OpenFlow 1.0 port numbers. It also accepts a subset
> + * of OpenFlow 1.1+ port numbers, mapping those port numbers into the 16-bit
> + * range as described in include/openflow/openflow-1.1.h. */
> +uint16_t
> +ofputil_port_from_string(const char *s)
> {
> - struct pair {
> - const char *name;
> - uint16_t value;
> - };
> - static const struct pair pairs[] = {
> + unsigned int port32;
> +
> + if (str_to_uint(s, 10, &port32)) {
> + if (port32 == 0) {
> + VLOG_WARN("port 0 is not a valid OpenFlow port number");
> + return 0;
> + } else if (port32 < OFPP_MAX) {
> + return port32;
> + } else if (port32 < OFPP_FIRST_RESV) {
> + VLOG_WARN("port %u is a reserved OF1.0 port number that will "
> + "be translated to %u when talking to an OF1.1 or "
> + "later controller", port32, port32 + OFPP11_OFFSET);
> + return port32;
> + } else if (port32 <= OFPP_LAST_RESV) {
> + struct ds s;
> +
> + ds_init(&s);
> + ofputil_format_port(port32, &s);
> + VLOG_WARN("port %u is better referred to as %s, for
> compatibility "
> + "with future versions of OpenFlow",
> + port32, ds_cstr(&s));
> + ds_destroy(&s);
> +
> + return port32;
> + } else if (port32 < OFPP11_MAX) {
> + VLOG_WARN("port %u is outside the supported range 0 through "
> + "%"PRIx16"or 0x%x through 0x%"PRIx32, port32,
> + UINT16_MAX, (unsigned int) OFPP11_MAX, UINT32_MAX);
> + return 0;
> + } else {
> + return port32 - OFPP11_OFFSET;
> + }
> + } else {
> + struct pair {
> + const char *name;
> + uint16_t value;
> + };
> + static const struct pair pairs[] = {
> #define OFPUTIL_NAMED_PORT(NAME) {#NAME, OFPP_##NAME},
> - OFPUTIL_NAMED_PORTS
> + OFPUTIL_NAMED_PORTS
> #undef OFPUTIL_NAMED_PORT
> - };
> - static const int n_pairs = ARRAY_SIZE(pairs);
> - int i;
> -
> - if (str_to_int(name, 0, &i) && i >= 0 && i < UINT16_MAX) {
> - *port = i;
> - return true;
> - }
> + };
> + const struct pair *p;
>
> - for (i = 0; i < n_pairs; i++) {
> - if (!strcasecmp(name, pairs[i].name)) {
> - *port = pairs[i].value;
> - return true;
> + for (p = pairs; p < &pairs[ARRAY_SIZE(pairs)]; p++) {
> + if (!strcasecmp(s, p->name)) {
> + return p->value;
> + }
> }
> + return 0;
> }
> - return false;
> }
>
> /* Appends to 's' a string representation of the OpenFlow port number 'port'.
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index a860d87..4e9f946 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -36,7 +36,7 @@ enum ofperr ofputil_port_from_ofp11(ovs_be32 ofp11_port,
> uint16_t *ofp10_port);
> ovs_be32 ofputil_port_to_ofp11(uint16_t ofp10_port);
>
> enum ofperr ofputil_check_output_port(uint16_t ofp_port, int max_ports);
> -bool ofputil_port_from_string(const char *, uint16_t *port);
> +uint16_t ofputil_port_from_string(const char *);
> void ofputil_format_port(uint16_t port, struct ds *);
>
> /* Converting OFPFW10_NW_SRC_MASK and OFPFW10_NW_DST_MASK wildcard bit counts
> diff --git a/tests/autopath.at b/tests/autopath.at
> index 634d46f..557c92c 100644
> --- a/tests/autopath.at
> +++ b/tests/autopath.at
> @@ -24,7 +24,7 @@ AT_CLEANUP
>
> AT_SETUP([autopath action bad port])
> AT_CHECK([ovs-ofctl parse-flow 'actions=autopath(bad, NXM_NX_REG0[[]])'],
> [1], [],
> - [ovs-ofctl: bad, NXM_NX_REG0[[]]: autopath id 0 is not in valid range 1 to
> 4294967295
> + [ovs-ofctl: bad, NXM_NX_REG0[[]]: bad port number
> ])
> AT_CLEANUP
>
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index a055851..827daa2 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -108,13 +108,13 @@ AT_SETUP([ofproto - basic flow_mod commands (NXM)])
> OVS_VSWITCHD_START
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip], [0], [NXST_FLOW reply:
> ])
> -AT_CHECK([echo 'in_port=1,actions=0' | ovs-ofctl add-flows br0 -])
> -AT_CHECK([ovs-ofctl add-flow br0 in_port=0,actions=1])
> -AT_CHECK([ovs-ofctl -F nxm add-flow br0 table=1,in_port=3,actions=2])
> +AT_CHECK([echo 'in_port=2,actions=1' | ovs-ofctl add-flows br0 -])
> +AT_CHECK([ovs-ofctl add-flow br0 in_port=1,actions=2])
> +AT_CHECK([ovs-ofctl -F nxm add-flow br0 table=1,in_port=4,actions=3])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - in_port=0 actions=output:1
> - in_port=1 actions=output:0
> - table=1, in_port=3 actions=output:2
> + in_port=1 actions=output:2
> + in_port=2 actions=output:1
> + table=1, in_port=4 actions=output:3
> NXST_FLOW reply:
> ])
> AT_CHECK([ovs-ofctl dump-aggregate br0 table=0 | STRIP_XIDS], [0], [dnl
> @@ -130,13 +130,13 @@ AT_SETUP([ofproto - basic flow_mod commands (OpenFlow
> 1.0)])
> OVS_VSWITCHD_START
> AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | ofctl_strip], [0],
> [OFPST_FLOW reply:
> ])
> -AT_CHECK([echo 'in_port=1,actions=0' | ovs-ofctl -F openflow10 add-flows br0
> -])
> -AT_CHECK([ovs-ofctl -F openflow10 add-flow br0 in_port=0,actions=1])
> -AT_CHECK([ovs-ofctl -F openflow10 add-flow br0 table=1,in_port=3,actions=2])
> +AT_CHECK([echo 'in_port=2,actions=1' | ovs-ofctl -F openflow10 add-flows br0
> -])
> +AT_CHECK([ovs-ofctl -F openflow10 add-flow br0 in_port=1,actions=2])
> +AT_CHECK([ovs-ofctl -F openflow10 add-flow br0 table=1,in_port=4,actions=3])
> AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | ofctl_strip | sort], [0],
> [dnl
> - in_port=0 actions=output:1
> - in_port=1 actions=output:0
> - table=1, in_port=3 actions=output:2
> + in_port=1 actions=output:2
> + in_port=2 actions=output:1
> + table=1, in_port=4 actions=output:3
> OFPST_FLOW reply:
> ])
> AT_CHECK([ovs-ofctl -F openflow10 dump-aggregate br0 table=0 | STRIP_XIDS],
> [0], [dnl
> @@ -150,20 +150,20 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - dump flows with cookie])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=0])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=1])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> - cookie=0x2, in_port=2 actions=output:0
> - cookie=0x3, in_port=3 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> + cookie=0x2, in_port=2 actions=output:1
> + cookie=0x3, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
> AT_CHECK([ovs-ofctl dump-aggregate br0 table=0 | STRIP_XIDS], [0], [dnl
> NXST_AGGREGATE reply: packet_count=0 byte_count=0 flow_count=3
> ])
> AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x3/-1 | ofctl_strip | sort], [0],
> [dnl
> - cookie=0x3, in_port=3 actions=output:0
> + cookie=0x3, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
> AT_CHECK([ovs-ofctl dump-aggregate br0 cookie=0x3/-1 | STRIP_XIDS], [0], [dnl
> @@ -174,21 +174,21 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - dump flows with cookie mask])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=0])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=1])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> - cookie=0x2, in_port=2 actions=output:0
> - cookie=0x3, in_port=3 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> + cookie=0x2, in_port=2 actions=output:1
> + cookie=0x3, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
> AT_CHECK([ovs-ofctl dump-aggregate br0 table=0 | STRIP_XIDS], [0], [dnl
> NXST_AGGREGATE reply: packet_count=0 byte_count=0 flow_count=3
> ])
> AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x3/0x1 | ofctl_strip | sort],
> [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> - cookie=0x3, in_port=3 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> + cookie=0x3, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
> AT_CHECK([ovs-ofctl dump-aggregate br0 cookie=0x3/0x1 | STRIP_XIDS], [0],
> [dnl
> @@ -199,15 +199,15 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - mod flow with cookie change (OpenFlow 1.0)])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl -F openflow10 add-flow br0
> cookie=0x1,in_port=1,actions=0])
> +AT_CHECK([ovs-ofctl -F openflow10 add-flow br0
> cookie=0x1,in_port=1,actions=1])
> AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | ofctl_strip | sort], [0],
> [dnl
> - cookie=0x1, in_port=1 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> OFPST_FLOW reply:
> ])
>
> -AT_CHECK([ovs-ofctl -F openflow10 mod-flows br0
> cookie=0x2,in_port=1,actions=0])
> +AT_CHECK([ovs-ofctl -F openflow10 mod-flows br0
> cookie=0x2,in_port=1,actions=1])
> AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | ofctl_strip | sort], [0],
> [dnl
> - cookie=0x2, in_port=1 actions=output:0
> + cookie=0x2, in_port=1 actions=output:1
> OFPST_FLOW reply:
> ])
> OVS_VSWITCHD_STOP
> @@ -215,15 +215,15 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - mod flow with cookie change (NXM)])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl -F nxm add-flow br0 cookie=0x1,in_port=1,actions=0])
> +AT_CHECK([ovs-ofctl -F nxm add-flow br0 cookie=0x1,in_port=1,actions=1])
> AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> NXST_FLOW reply:
> ])
>
> -AT_CHECK([ovs-ofctl -F nxm mod-flows br0 cookie=0x2,in_port=1,actions=0])
> +AT_CHECK([ovs-ofctl -F nxm mod-flows br0 cookie=0x2,in_port=1,actions=1])
> AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x2, in_port=1 actions=output:0
> + cookie=0x2, in_port=1 actions=output:1
> NXST_FLOW reply:
> ])
> OVS_VSWITCHD_STOP
> @@ -231,13 +231,13 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - mod flows based on cookie mask])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=2,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=3,actions=0])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=2,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=3,actions=1])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> - cookie=0x1, in_port=2 actions=output:0
> - cookie=0x2, in_port=3 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> + cookie=0x1, in_port=2 actions=output:1
> + cookie=0x2, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
>
> @@ -245,7 +245,7 @@ AT_CHECK([ovs-ofctl -F nxm mod-flows br0
> cookie=0x1/0xff,actions=4])
> AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
> cookie=0x1, in_port=1 actions=output:4
> cookie=0x1, in_port=2 actions=output:4
> - cookie=0x2, in_port=3 actions=output:0
> + cookie=0x2, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
> OVS_VSWITCHD_STOP
> @@ -253,19 +253,19 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - mod flows based on cookie mask with cookie change])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=2,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=3,actions=0])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=2,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=3,actions=1])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> - cookie=0x1, in_port=2 actions=output:0
> - cookie=0x2, in_port=3 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> + cookie=0x1, in_port=2 actions=output:1
> + cookie=0x2, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
>
> AT_CHECK([ovs-ofctl -F nxm mod-flows br0 cookie=1/-1,cookie=4,actions=4])
> AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x2, in_port=3 actions=output:0
> + cookie=0x2, in_port=3 actions=output:1
> cookie=0x4, in_port=1 actions=output:4
> cookie=0x4, in_port=2 actions=output:4
> NXST_FLOW reply:
> @@ -275,9 +275,9 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - mod flow with cookie miss (mask==0)])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl -F nxm mod-flows br0 in_port=1,actions=0])
> +AT_CHECK([ovs-ofctl -F nxm mod-flows br0 in_port=1,actions=1])
> AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - in_port=1 actions=output:0
> + in_port=1 actions=output:1
> NXST_FLOW reply:
> ])
> OVS_VSWITCHD_STOP
> @@ -285,7 +285,7 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - mod flow with cookie miss (mask!=0)])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl -F nxm mod-flows br0 cookie=1/1,in_port=1,actions=0])
> +AT_CHECK([ovs-ofctl -F nxm mod-flows br0 cookie=1/1,in_port=1,actions=1])
> AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
> NXST_FLOW reply:
> ])
> @@ -294,13 +294,13 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - del flows with cookies])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=0])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=1])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> - cookie=0x2, in_port=2 actions=output:0
> - cookie=0x3, in_port=3 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> + cookie=0x2, in_port=2 actions=output:1
> + cookie=0x3, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
>
> @@ -313,20 +313,20 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - del flows based on cookie])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=0])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=1])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> - cookie=0x2, in_port=2 actions=output:0
> - cookie=0x3, in_port=3 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> + cookie=0x2, in_port=2 actions=output:1
> + cookie=0x3, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
>
> AT_CHECK([ovs-ofctl del-flows br0 cookie=0x3/-1])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> - cookie=0x2, in_port=2 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> + cookie=0x2, in_port=2 actions=output:1
> NXST_FLOW reply:
> ])
> OVS_VSWITCHD_STOP
> @@ -334,18 +334,18 @@ AT_CLEANUP
>
> AT_SETUP([ofproto - del flows based on cookie mask])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=0])
> -AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=0])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=1])
> +AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=1])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x1, in_port=1 actions=output:0
> - cookie=0x2, in_port=2 actions=output:0
> - cookie=0x3, in_port=3 actions=output:0
> + cookie=0x1, in_port=1 actions=output:1
> + cookie=0x2, in_port=2 actions=output:1
> + cookie=0x3, in_port=3 actions=output:1
> NXST_FLOW reply:
> ])
> AT_CHECK([ovs-ofctl del-flows br0 cookie=0x3/0x1])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - cookie=0x2, in_port=2 actions=output:0
> + cookie=0x2, in_port=2 actions=output:1
> NXST_FLOW reply:
> ])
> OVS_VSWITCHD_STOP
> @@ -692,7 +692,7 @@ AT_CAPTURE_FILE([monitor.log])
>
> # Send some packet-outs with OFPP_NONE and OFPP_CONTROLLER (65533) as
> in_port.
> AT_CHECK([ovs-ofctl packet-out br0 none controller
> '0001020304050010203040501234'])
> -AT_CHECK([ovs-ofctl packet-out br0 65533 controller
> '0001020304050010203040505678'])
> +AT_CHECK([ovs-ofctl packet-out br0 controller controller
> '0001020304050010203040505678'])
>
> # Stop the monitor and check its output.
> ovs-appctl -t ovs-ofctl ofctl/barrier
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index 3407050..be71f18 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -71,12 +71,11 @@ Prints to the console detailed information about network
> devices
> associated with \fIswitch\fR (version 1.7 or later). This is a subset
> of the information provided by the \fBshow\fR command.
> .
> -.TP
> -\fBmod\-port \fIswitch\fR \fInetdev\fR \fIaction\fR
> -Modify characteristics of an interface monitored by \fIswitch\fR.
> -\fInetdev\fR can be referred to by its OpenFlow assigned port number or
> -the device name, e.g. \fBeth0\fR. The \fIaction\fR may be any one of the
> -following:
> +.IP "\fBmod\-port \fIswitch\fR \fIport\fR \fIaction\fR"
> +Modify characteristics of port \fBport\fR in \fIswitch\fR. \fIport\fR
> +may be an OpenFlow port number or name or the keyword \fBLOCAL\fR (the
> +preferred way to refer to the OpenFlow local port). The \fIaction\fR
> +may be any one of the following:
> .
> .RS
> .IQ \fBup\fR
> @@ -180,12 +179,15 @@ The output format is described in \fBTable Entry
> Output\fR.
> .
> .IP "\fBqueue\-stats \fIswitch \fR[\fIport \fR[\fIqueue\fR]]"
> Prints to the console statistics for the specified \fIqueue\fR on
> -\fIport\fR within \fIswitch\fR. Either of \fIport\fR or \fIqueue\fR
> -or both may be omitted (or equivalently specified as \fBALL\fR). If
> -both are omitted, statistics are printed for all queues on all ports.
> -If only \fIqueue\fR is omitted, then statistics are printed for all
> -queues on \fIport\fR; if only \fIport\fR is omitted, then statistics
> -are printed for \fIqueue\fR on every port where it exists.
> +\fIport\fR within \fIswitch\fR. \fIport\fR can be an OpenFlow port
> +number or name, the keyword \fBLOCAL\fR (the preferred way to refer to
> +the OpenFlow local port), or the keyword \fBALL\fR. Either of
> +\fIport\fR or \fIqueue\fR or both may be omitted (or equivalently the
> +keyword \fBALL\fR). If both are omitted, statistics are printed for
> +all queues on all ports. If only \fIqueue\fR is omitted, then
> +statistics are printed for all queues on \fIport\fR; if only
> +\fIport\fR is omitted, then statistics are printed for \fIqueue\fR on
> +every port where it exists.
> .
> .SS "OpenFlow Switch Flow Table Commands"
> .
> @@ -250,10 +252,10 @@ differences were found.
> Connects to \fIswitch\fR and instructs it to execute the OpenFlow
> \fIactions\fR on each \fIpacket\fR. For the purpose of executing the
> actions, the packets are considered to have arrived on \fIin_port\fR,
> -which may be an OpenFlow assigned port number, an OpenFlow port name
> -(e.g. \fBeth0\fR), the keyword \fBlocal\fR for the OpenFlow ``local''
> -port \fBOFPP_LOCAL\fR, or the keyword \fBnone\fR to indicate that the
> -packet was generated by the switch itself.
> +which may be an OpenFlow port number or name (e.g. \fBeth0\fR), the
> +keyword \fBLOCAL\fR (the preferred way to refer to the OpenFlow
> +``local'' port), or the keyword \fBNONE\fR to indicate that the packet
> +was generated by the switch itself.
> .
> .SS "OpenFlow Switch Monitoring Commands"
> .
> @@ -325,7 +327,9 @@ Do not report actions as part of flow updates.
> Limits the monitoring to the table with the given \fInumber\fR between
> 0 and 254. By default, all tables are monitored.
> .IP "\fBout_port=\fIport\fR"
> -If set, only flows that output to \fIport\fR are monitored.
> +If set, only flows that output to \fIport\fR are monitored. The
> +\fIport\fR may be an OpenFlow port number or keyword
> +(e.g. \fBLOCAL\fR).
> .IP "\fIfield\fB=\fIvalue\fR"
> Monitors only flows that have \fIfield\fR specified as the given
> \fIvalue\fR. Any syntax valid for matching on \fBdump\-flows\fR may
> @@ -392,9 +396,10 @@ resulting flow matches all packets. The string \fB*\fR
> or \fBANY\fR
> may be specified to explicitly mark any of these fields as a wildcard.
> (\fB*\fR should be quoted to protect it from shell expansion.)
> .
> -.IP \fBin_port=\fIport_no\fR
> -Matches OpenFlow port \fIport_no\fR. Ports are numbered as
> -displayed by \fBovs\-ofctl show\fR.
> +.IP \fBin_port=\fIport\fR
> +Matches OpenFlow port \fIport\fR, which may be an OpenFlow port number
> +or keyword (e.g. \fBLOCAL\fR).
> +\fBovs\-ofctl show\fR.
> .IP
> (The \fBresubmit\fR action can search OpenFlow flow tables with
> arbitrary \fBin_port\fR values, so flows that match port numbers that
> @@ -799,25 +804,28 @@ require an additional field, which must be the final
> field specified:
> .IP \fBactions=\fR[\fItarget\fR][\fB,\fItarget\fR...]\fR
> Specifies a comma-separated list of actions to take on a packet when the
> flow entry matches. If no \fItarget\fR is specified, then packets
> -matching the flow are dropped. The \fItarget\fR may be a decimal port
> +matching the flow are dropped. The \fItarget\fR may be an OpenFlow port
> number designating the physical port on which to output the packet, or one
> of the following keywords:
> .
> .RS
> -.IP \fBoutput\fR:\fIport\fR
> -.IQ \fBoutput\fR:\fIsrc\fB[\fIstart\fB..\fIend\fB]
> -Outputs the packet. If \fIport\fR is an OpenFlow port number, outputs
> directly
> -to it. Otherwise, outputs to the OpenFlow port number read from \fIsrc\fR
> -which must be an NXM field as described above. Outputting to an NXM field is
> -an OpenFlow extension which is not supported by standard OpenFlow switches.
> -.IP
> -Example: \fBoutput:NXM_NX_REG0[16..31]\fR outputs to the OpenFlow port number
> -written in the upper half of register 0.
> -.
> -.IP \fBenqueue\fR:\fIport\fB:\fIqueue\fR
> +.IP \fBoutput:\fIport\fR
> +Outputs the packet to \fIport\fR, which must be an OpenFlow port
> +number or keyword (e.g. \fBLOCAL\fR).
> +.
> +.IP \fBoutput:\fIsrc\fB[\fIstart\fB..\fIend\fB]
> +Outputs the packet to the OpenFlow port number read from \fIsrc\fR,
> +which must be an NXM field as described above. For example,
> +\fBoutput:NXM_NX_REG0[16..31]\fR outputs to the OpenFlow port number
> +written in the upper half of register 0. This form of \fBoutput\fR
> +uses an OpenFlow extension that is not supported by standard OpenFlow
> +switches.
> +.
> +.IP \fBenqueue:\fIport\fB:\fIqueue\fR
> Enqueues the packet on the specified \fIqueue\fR within port
> -\fIport\fR. The number of supported queues depends on the switch;
> -some OpenFlow implementations do not support queuing at all.
> +\fIport\fR, which must be an OpenFlow port number or keyword
> +(e.g. \fBLOCAL\fR).. The number of supported queues depends on the
> +switch; some OpenFlow implementations do not support queuing at all.
> .
> .IP \fBnormal\fR
> Subjects the packet to the device's normal L2/L3 processing. (This
> @@ -1245,7 +1253,8 @@ and \fBdel\-flows\fR commands support one additional
> optional field:
> .
> .TP
> \fBout_port=\fIport\fR
> -If set, a matching flow must include an output action to \fIport\fR.
> +If set, a matching flow must include an output action to \fIport\fR,
> +which must an OpenFlow port number or name (e.g. \fBlocal\fR).
> .
> .SS "Table Entry Output"
> .
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 5f61fd6..dea8878 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -741,9 +741,8 @@ fetch_ofputil_phy_port(const char *vconn_name, const char
> *port_name,
> static uint16_t
> str_to_port_no(const char *vconn_name, const char *port_name)
> {
> - unsigned int port_no;
> -
> - if (str_to_uint(port_name, 10, &port_no)) {
> + uint16_t port_no = ofputil_port_from_string(port_name);
> + if (port_no) {
> return port_no;
> } else {
> struct ofputil_phy_port pp;
> @@ -1466,9 +1465,7 @@ ofctl_packet_out(int argc, char *argv[])
> parse_ofpacts(argv[3], &ofpacts);
>
> po.buffer_id = UINT32_MAX;
> - po.in_port = (!strcasecmp(argv[2], "none") ? OFPP_NONE
> - : !strcasecmp(argv[2], "local") ? OFPP_LOCAL
> - : str_to_port_no(argv[1], argv[2]));
> + po.in_port = str_to_port_no(argv[1], argv[2]);
> po.ofpacts = ofpacts.data;
> po.ofpacts_len = ofpacts.size;
>
> --
> 1.7.2.5
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
