[ovs-dev] [PATCH 01/12] ovs-ofputil: Make str_to_port_no() aware of invalid ports

Simon Horman horms at verge.net.au
Mon Sep 17 23:58:14 UTC 2012


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 <horms at 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 <blp at 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 <blp at 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
> 



More information about the dev mailing list