Fix handling of OFPP_ANY in OpenFlow 1.1 and later.

Add OFPP_ANY to include/openflow/openflow-1.1.h, and allow it as a port in
queue stats request. Make ovs_ofctl use OFPP_ANY instead of OFPP_ALL for queue
stats requests on OF 1.1+.

This patch changes "none" ports print out. "none" is still accepted on input
for backwards compatibility, but it prints out as "ANY". To make this less
confusing, I changed the test cases to use "controller" or "any" instead of
"none". The test case that tests for both "none" and "controller" still tests
for them.

Signed-off-by: Jarno Rajahalme <jarno.rajahalme@nsn.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index 9785db456aa32a0a1126311687c9f742385a6d84..c4a5abaa6bf4cd68179b7dcbf28651be37cbd4bc 100644 (file)
--- a/include/openflow/openflow-1.1.h
+++ b/include/openflow/openflow-1.1.h
@@ -70,6 +70,14 @@
 #define OFPP11_MAX    0xffffff00
 #define OFPP11_OFFSET (OFPP11_MAX - OFPP_MAX)
 
+/* Reserved wildcard port used only for flow mod (delete) and flow stats
+ * requests. Selects all flows regardless of output port
+ * (including flows with no output port)
+ *
+ * Define it via OFPP_NONE (0xFFFF) so that OFPP_ANY is still an enum ofp_port
+ */
+#define OFPP_ANY OFPP_NONE
+
 /* OpenFlow 1.1 port config flags are just the common flags. */
 #define OFPPC11_ALL \
     (OFPPC_PORT_DOWN | OFPPC_NO_RECV | OFPPC_NO_FWD | OFPPC_NO_PACKET_IN)

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 054db60e440f73454fb33ca8bd25baed15897589..1d71370d4d0c78455389eba3d8e4537a1af38ec4 100644 (file)
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -829,7 +829,7 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
     fm->idle_timeout = OFP_FLOW_PERMANENT;
     fm->hard_timeout = OFP_FLOW_PERMANENT;
     fm->buffer_id = UINT32_MAX;
-    fm->out_port = OFPP_NONE;
+    fm->out_port = OFPP_ANY;
     fm->flags = 0;
     if (fields & F_ACTIONS) {
         act_str = strstr(string, "action");

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index a0f04dcb64634d734e06029a92149f553d56d692..1e35fba7937b05e21fefbcd146838463859bc4b4 100644 (file)
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -765,7 +765,7 @@ ofp_print_flow_mod(struct ds *s, const struct ofp_header *oh, int verbosity)
     if (fm.buffer_id != UINT32_MAX) {
         ds_put_format(s, "buf:0x%"PRIx32" ", fm.buffer_id);
     }
-    if (fm.out_port != OFPP_NONE) {
+    if (fm.out_port != OFPP_ANY) {
         ds_put_format(s, "out_port:");
         ofputil_format_port(fm.out_port, s);
         ds_put_char(s, ' ');
@@ -1003,7 +1003,7 @@ ofp_print_flow_stats_request(struct ds *string, const struct ofp_header *oh)
         ds_put_format(string, " table=%"PRIu8, fsr.table_id);
     }
 
-    if (fsr.out_port != OFPP_NONE) {
+    if (fsr.out_port != OFPP_ANY) {
         ds_put_cstr(string, " out_port=");
         ofputil_format_port(fsr.out_port, string);
     }

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 4facf0a579b631a876ad85de278f61a8f0ce8c98..b7feff800197c5377895d1593d1987b4845de965 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3804,6 +3804,11 @@ ofputil_check_output_port(uint16_t port, int max_ports)
         OFPUTIL_NAMED_PORT(ALL)                 \
         OFPUTIL_NAMED_PORT(CONTROLLER)          \
         OFPUTIL_NAMED_PORT(LOCAL)               \
+        OFPUTIL_NAMED_PORT(ANY)
+
+/* For backwards compatibility, so that "none" is recognized as OFPP_ANY */
+#define OFPUTIL_NAMED_PORTS_WITH_NONE           \
+        OFPUTIL_NAMED_PORTS                     \
         OFPUTIL_NAMED_PORT(NONE)
 
 /* Stores the port number represented by 's' into '*portp'.  's' may be an
@@ -3863,7 +3868,7 @@ ofputil_port_from_string(const char *s, uint16_t *portp)
         };
         static const struct pair pairs[] = {
 #define OFPUTIL_NAMED_PORT(NAME) {#NAME, OFPP_##NAME},
-            OFPUTIL_NAMED_PORTS
+            OFPUTIL_NAMED_PORTS_WITH_NONE
 #undef OFPUTIL_NAMED_PORT
         };
         const struct pair *p;
@@ -4467,9 +4472,13 @@ ofputil_decode_queue_stats_request(const struct ofp_header *request,
     }
 
     case OFP10_VERSION: {
-        const struct ofp10_queue_stats_request *qsr11 = ofpmsg_body(request);
-        oqsr->queue_id = ntohl(qsr11->queue_id);
-        oqsr->port_no = ntohs(qsr11->port_no);
+        const struct ofp10_queue_stats_request *qsr10 = ofpmsg_body(request);
+        oqsr->queue_id = ntohl(qsr10->queue_id);
+        oqsr->port_no = ntohs(qsr10->port_no);
+        /* OF 1.0 uses OFPP_ALL for OFPP_ANY */
+        if (oqsr->port_no == OFPP_ALL) {
+            oqsr->port_no = OFPP_ANY;
+        }
         return 0;
     }
 
@@ -4501,7 +4510,9 @@ ofputil_encode_queue_stats_request(enum ofp_version ofp_version,
         struct ofp10_queue_stats_request *req;
         request = ofpraw_alloc(OFPRAW_OFPST10_QUEUE_REQUEST, ofp_version, 0);
         req = ofpbuf_put_zeros(request, sizeof *req);
-        req->port_no = htons(oqsr->port_no);
+        /* OpenFlow 1.0 needs OFPP_ALL instead of OFPP_ANY */
+        req->port_no = htons(oqsr->port_no == OFPP_ANY
+                             ? OFPP_ALL : oqsr->port_no);
         req->queue_id = htonl(oqsr->queue_id);
         break;
     }

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 053cd843adbc35bb2bf469ba2824e616bb059820..ed1a5382cb827ddd63133b1a1a6cc4fddfa59b73 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -686,7 +686,7 @@ enum ofperr ofputil_decode_port_stats_request(const struct ofp_header *request,
                                               uint16_t *ofp10_port);
 
 struct ofputil_queue_stats_request {
-    uint16_t port_no;
+    uint16_t port_no;           /* OFPP_ANY means "all ports". */
     uint32_t queue_id;
 };
 

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index dabb590d3e55027219feb9fca6275291d91e9ffc..0992fe46697b8b8f9ad4ccdb8db6d4b9acf167e2 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2129,7 +2129,7 @@ ofproto_rule_destroy(struct rule *rule)
 bool
 ofproto_rule_has_out_port(const struct rule *rule, uint16_t port)
 {
-    return (port == OFPP_NONE
+    return (port == OFPP_ANY
             || ofpacts_output_to_port(rule->ofpacts, rule->ofpacts_len, port));
 }
 
@@ -2542,7 +2542,7 @@ handle_port_stats_request(struct ofconn *ofconn,
     }
 
     ofpmp_init(&replies, request);
-    if (port_no != OFPP_NONE) {
+    if (port_no != OFPP_ANY) {
         port = ofproto_get_port(p, port_no);
         if (port) {
             append_port_stat(port, &replies);
@@ -2659,7 +2659,7 @@ next_matching_table(const struct ofproto *ofproto,
  * OpenFlow OFPFC_MODIFY and OFPFC_DELETE requests and puts them on list
  * 'rules'.
  *
- * If 'out_port' is anything other than OFPP_NONE, then only rules that output
+ * If 'out_port' is anything other than OFPP_ANY, then only rules that output
  * to 'out_port' are included.
  *
  * Hidden rules are always omitted.
@@ -2710,7 +2710,7 @@ exit:
  * OpenFlow OFPFC_MODIFY_STRICT and OFPFC_DELETE_STRICT requests and puts them
  * on list 'rules'.
  *
- * If 'out_port' is anything other than OFPP_NONE, then only rules that output
+ * If 'out_port' is anything other than OFPP_ANY, then only rules that output
  * to 'out_port' are included.
  *
  * Hidden rules are always omitted.
@@ -3054,7 +3054,7 @@ handle_queue_stats_request(struct ofconn *ofconn,
         return error;
     }
 
-    if (oqsr.port_no == OFPP_ALL) {
+    if (oqsr.port_no == OFPP_ANY) {
         error = OFPERR_OFPQOFC_BAD_QUEUE;
         HMAP_FOR_EACH (port, hmap_node, &ofproto->ports) {
             if (!handle_queue_stats_for_port(port, oqsr.queue_id, &cbdata)) {
@@ -3327,7 +3327,7 @@ modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
 
     error = collect_rules_loose(ofproto, fm->table_id, &fm->match,
                                 fm->cookie, fm->cookie_mask,
-                                OFPP_NONE, &rules);
+                                OFPP_ANY, &rules);
     if (error) {
         return error;
     } else if (list_is_empty(&rules)) {
@@ -3352,7 +3352,7 @@ modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
 
     error = collect_rules_strict(ofproto, fm->table_id, &fm->match,
                                  fm->priority, fm->cookie, fm->cookie_mask,
-                                 OFPP_NONE, &rules);
+                                 OFPP_ANY, &rules);
 
     if (error) {
         return error;

diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index bcf9d2c70e472787d24c7fffb9cf8275faf4c80a..7869a8670a8e6bf25ed03e095cd68f6efd63bf7b 100644 (file)
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -1149,7 +1149,7 @@ AT_CHECK([ovs-ofctl ofp-print "\
 01 10 00 14 00 00 00 01 00 05 00 00 ff fc 00 00 \
 ff ff ff ff \
 "], [0], [dnl
-OFPST_QUEUE request (xid=0x1):port=ALL queue=ALL
+OFPST_QUEUE request (xid=0x1):port=ANY queue=ALL
 ])
 AT_CLEANUP
 
@@ -1157,9 +1157,9 @@ AT_SETUP([OFPST_QUEUE request - OF1.1])
 AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
 AT_CHECK([ovs-ofctl ofp-print "\
 02 12 00 18 00 00 00 02 00 05 00 00 00 00 00 00 \
-ff ff ff fc ff ff ff ff \
+ff ff ff ff ff ff ff ff \
 "], [0], [dnl
-OFPST_QUEUE request (OF1.1) (xid=0x2):port=ALL queue=ALL
+OFPST_QUEUE request (OF1.1) (xid=0x2):port=ANY queue=ALL
 ])
 AT_CLEANUP
 
@@ -1167,9 +1167,9 @@ AT_SETUP([OFPST_QUEUE request - OF1.2])
 AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
 AT_CHECK([ovs-ofctl ofp-print "\
 03 12 00 18 00 00 00 02 00 05 00 00 00 00 00 00 \
-ff ff ff fc ff ff ff ff \
+ff ff ff ff ff ff ff ff \
 "], [0], [dnl
-OFPST_QUEUE request (OF1.2) (xid=0x2):port=ALL queue=ALL
+OFPST_QUEUE request (OF1.2) (xid=0x2):port=ANY queue=ALL
 ])
 AT_CLEANUP
 

diff --git a/tests/ofproto.at b/tests/ofproto.at
index 38bc406428021cdc4ac74d129aca8e5d152f5072..7f468bbf81c894e0ec4bda5eba5985e35d06f7fe 100644 (file)
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -90,9 +90,9 @@ AT_CHECK([ovs-ofctl -vwarn queue-stats br0], [0], [stdout])
 AT_CHECK([STRIP_XIDS stdout], [0], [dnl
 OFPST_QUEUE reply: 0 queues
 ])
-AT_CHECK([ovs-ofctl -vwarn queue-stats br0 ALL 5], [0],
+AT_CHECK([ovs-ofctl -vwarn queue-stats br0 ANY 5], [0],
   [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_QUEUE
-OFPST_QUEUE request (xid=0x2):port=ALL queue=5
+OFPST_QUEUE request (xid=0x2):port=ANY queue=5
 ])
 AT_CHECK([ovs-ofctl -vwarn queue-stats br0 10], [0],
   [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_PORT
@@ -657,23 +657,23 @@ check_async () {
     : > expout
 
     # OFPT_PACKET_IN, OFPR_ACTION (controller_id=0)
-    ovs-ofctl -v packet-out br0 none controller '0001020304050010203040501234'
+    ovs-ofctl -v packet-out br0 controller controller '0001020304050010203040501234'
     if test X"$1" = X"OFPR_ACTION"; then shift;
-        echo >>expout "OFPT_PACKET_IN: total_len=14 in_port=NONE (via action) data_len=14 (unbuffered)
+        echo >>expout "OFPT_PACKET_IN: total_len=14 in_port=CONTROLLER (via action) data_len=14 (unbuffered)
 priority=0,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
     fi
 
     # OFPT_PACKET_IN, OFPR_NO_MATCH (controller_id=123)
-    ovs-ofctl -v packet-out br0 none 'controller(reason=no_match,id=123)' '0001020304050010203040501234'
+    ovs-ofctl -v packet-out br0 controller 'controller(reason=no_match,id=123)' '0001020304050010203040501234'
     if test X"$1" = X"OFPR_NO_MATCH"; then shift;
-        echo >>expout "OFPT_PACKET_IN: total_len=14 in_port=NONE (via no_match) data_len=14 (unbuffered)
+        echo >>expout "OFPT_PACKET_IN: total_len=14 in_port=CONTROLLER (via no_match) data_len=14 (unbuffered)
 priority=0,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
     fi
 
     # OFPT_PACKET_IN, OFPR_INVALID_TTL (controller_id=0)
-    ovs-ofctl packet-out br0 none dec_ttl '002583dfb4000026b98cb0f908004500003fb7e200000011339bac11370dac100002d7730035002b8f6d86fb0100000100000000000006626c702d7873066e696369726103636f6d00000f00'
+    ovs-ofctl packet-out br0 controller dec_ttl '002583dfb4000026b98cb0f908004500003fb7e200000011339bac11370dac100002d7730035002b8f6d86fb0100000100000000000006626c702d7873066e696369726103636f6d00000f00'
     if test X"$1" = X"OFPR_INVALID_TTL"; then shift;
-        echo >>expout "OFPT_PACKET_IN: total_len=76 in_port=NONE (via invalid_ttl) data_len=76 (unbuffered)
+        echo >>expout "OFPT_PACKET_IN: total_len=76 in_port=CONTROLLER (via invalid_ttl) data_len=76 (unbuffered)
 priority=0,udp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_dst=00:25:83:df:b4:00,nw_src=172.17.55.13,nw_dst=172.16.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=55155,tp_dst=53 udp_csum:8f6d"
     fi
 
@@ -771,7 +771,7 @@ ovs-appctl -t ovs-ofctl ofctl/barrier
 ovs-appctl -t ovs-ofctl exit
 
 AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//' monitor.log], [0], [dnl
-OFPT_PACKET_IN: total_len=14 in_port=NONE (via action) data_len=14 (unbuffered)
+OFPT_PACKET_IN: total_len=14 in_port=ANY (via action) data_len=14 (unbuffered)
 priority=0,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
 OFPT_PACKET_IN: total_len=14 in_port=CONTROLLER (via action) data_len=14 (unbuffered)
 priority=0,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x5678
@@ -794,14 +794,14 @@ ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
 AT_CAPTURE_FILE([monitor.log])
 
 # Send a packet-out with a load action to set some metadata, and forward to controller
-AT_CHECK([ovs-ofctl packet-out br0 none 'load(0xfafafafa5a5a5a5a->OXM_OF_METADATA[[0..63]]), controller' '0001020304050010203040501234'])
+AT_CHECK([ovs-ofctl packet-out br0 controller 'load(0xfafafafa5a5a5a5a->OXM_OF_METADATA[[0..63]]), controller' '0001020304050010203040501234'])
 
 # Stop the monitor and check its output.
 ovs-appctl -t ovs-ofctl ofctl/barrier
 ovs-appctl -t ovs-ofctl exit
 
 AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//' monitor.log], [0], [dnl
-NXT_PACKET_IN: total_len=14 in_port=NONE metadata=0xfafafafa5a5a5a5a (via action) data_len=14 (unbuffered)
+NXT_PACKET_IN: total_len=14 in_port=CONTROLLER metadata=0xfafafafa5a5a5a5a (via action) data_len=14 (unbuffered)
 priority=0,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
 OFPT_BARRIER_REPLY:
 ])

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 363c0a3a3d44ad18be52c96a3c655216b585563d..5037398686b4d51c331813a52146a609ea94507a 100644 (file)
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -968,7 +968,7 @@ ofctl_queue_stats(int argc, char *argv[])
     if (argc > 2 && argv[2][0] && strcasecmp(argv[2], "all")) {
         oqs.port_no = str_to_port_no(argv[1], argv[2]);
     } else {
-        oqs.port_no = OFPP_ALL;
+        oqs.port_no = OFPP_ANY;
     }
     if (argc > 3 && argv[3][0] && strcasecmp(argv[3], "all")) {
         oqs.queue_id = atoi(argv[3]);
@@ -1430,7 +1430,7 @@ ofctl_dump_ports(int argc, char *argv[])
     uint16_t port;
 
     open_vconn(argv[1], &vconn);
-    port = argc > 2 ? str_to_port_no(argv[1], argv[2]) : OFPP_NONE;
+    port = argc > 2 ? str_to_port_no(argv[1], argv[2]) : OFPP_ANY;
     request = ofputil_encode_dump_ports_request(vconn_get_version(vconn), port);
     dump_stats_transaction(vconn, request);
     vconn_close(vconn);
@@ -1940,7 +1940,7 @@ read_flows_from_switch(struct vconn *vconn,
 
     fsr.aggregate = false;
     match_init_catchall(&fsr.match);
-    fsr.out_port = OFPP_NONE;
+    fsr.out_port = OFPP_ANY;
     fsr.table_id = 0xff;
     fsr.cookie = fsr.cookie_mask = htonll(0);
     request = ofputil_encode_flow_stats_request(&fsr, protocol);
@@ -1983,7 +1983,7 @@ fte_make_flow_mod(const struct fte *fte, int index, uint16_t command,
     fm.idle_timeout = version->idle_timeout;
     fm.hard_timeout = version->hard_timeout;
     fm.buffer_id = UINT32_MAX;
-    fm.out_port = OFPP_NONE;
+    fm.out_port = OFPP_ANY;
     fm.flags = version->flags;
     if (command == OFPFC_ADD || command == OFPFC_MODIFY ||
         command == OFPFC_MODIFY_STRICT) {