From 7f05e7abc8fc190c5ff60dd3c3fcd4b9eb5c8dfa Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 26 Nov 2012 18:17:08 +0200 Subject: [PATCH] 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 Signed-off-by: Ben Pfaff --- include/openflow/openflow-1.1.h | 8 ++++++++ lib/ofp-parse.c | 2 +- lib/ofp-print.c | 4 ++-- lib/ofp-util.c | 21 ++++++++++++++++----- lib/ofp-util.h | 2 +- ofproto/ofproto.c | 14 +++++++------- tests/ofp-print.at | 10 +++++----- tests/ofproto.at | 22 +++++++++++----------- utilities/ovs-ofctl.c | 8 ++++---- 9 files changed, 55 insertions(+), 36 deletions(-) diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h index 9785db45..c4a5abaa 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 054db60e..1d71370d 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 a0f04dcb..1e35fba7 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 4facf0a5..b7feff80 100644 --- 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 053cd843..ed1a5382 100644 --- 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 dabb590d..0992fe46 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 bcf9d2c7..7869a867 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 38bc4064..7f468bbf 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 363c0a3a..50373986 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) { -- 2.30.2