OpenFlow 1.2 protocol fixes: 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+.
Do not check out_group on flow_mod unless the command is DELETE*.
As part of the change to all clients to use OFPP_ANY regardless of version,
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.
include/openflow/openflow-1.1.h | 8 ++++++++
lib/ofp-parse.c | 2 +-
lib/ofp-print.c | 4 ++--
lib/ofp-util.c | 25 +++++++++++++++++++------
ofproto/ofproto.c | 14 +++++++-------
tests/ofp-print.at | 10 +++++-----
tests/ofproto.at | 22 +++++++++++-----------
utilities/ovs-ofctl.c | 8 ++++----
8 files changed, 57 insertions(+), 36 deletions(-)
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index 9785db4..c4a5aba 100644
--- 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 054db60..1d71370 100644
--- 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 a0f04dc..1e35fba 100644
--- 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 4facf0a..bab6b2c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1435,7 +1435,9 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
if (error) {
return error;
}
- if (ofm->out_group != htonl(OFPG_ANY)) {
+ if ((ofm->command == OFPFC_DELETE
+ || ofm->command == OFPFC_DELETE_STRICT)
+ && ofm->out_group != htonl(OFPG_ANY)) {
return OFPERR_OFPFMFC_UNKNOWN;
}
fm->flags = ntohs(ofm->flags);
@@ -3804,6 +3806,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 +3870,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 +4474,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 +4512,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/ofproto/ofproto.c b/ofproto/ofproto.c
index dabb590..0992fe4 100644
--- 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 bcf9d2c..7869a86 100644
--- 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 38bc406..7f468bb 100644
--- 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 363c0a3..5037398 100644
--- 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) {
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
