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.

    - 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@nicira.com>
Acked-by: Simon Horman <horms@verge.net.au>

diff --git a/NEWS b/NEWS
index cbc5c58..38e5129 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;