From 56085be5fed24dbc44e01b6c72adcafe2328846f Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 7 Jan 2016 08:57:44 -0800 Subject: [PATCH] ofproto: Implement OFPT_QUEUE_GET_CONFIG_REQUEST for OFPP_ANY in OF1.1+. I was not previously aware that this feature was missing. Reported-by: Minoru TAKAHASHI Reported-at: http://openvswitch.org/pipermail/discuss/2015-October/019229.html Signed-off-by: Ben Pfaff Acked-by: Andy Zhou --- NEWS | 3 +++ lib/ofp-util.c | 22 ++++++++++------- lib/ofp-util.h | 1 + ofproto/ofproto.c | 51 ++++++++++++++++++++++++---------------- tests/ofproto.at | 9 +++++++ utilities/ovs-ofctl.8.in | 11 +++++++++ utilities/ovs-ofctl.c | 51 ++++++++++++++++++++++++++++------------ 7 files changed, 104 insertions(+), 44 deletions(-) diff --git a/NEWS b/NEWS index e3461f42a..443332927 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ Post-v2.5.0 --------------------- - ovsdb-server: * New "monitor2" and "update2" extensions to RFC 7047. + - OpenFlow: + * OpenFlow 1.1+ OFPT_QUEUE_GET_CONFIG_REQUEST now supports OFPP_ANY. + v2.5.0 - xx xxx xxxx --------------------- diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 19901d112..4a38121bb 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -2492,14 +2492,23 @@ ofputil_decode_queue_get_config_request(const struct ofp_header *oh, case OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST: qgcr10 = b.data; *port = u16_to_ofp(ntohs(qgcr10->port)); - return 0; + break; case OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST: qgcr11 = b.data; - return ofputil_port_from_ofp11(qgcr11->port, port); + enum ofperr error = ofputil_port_from_ofp11(qgcr11->port, port); + if (error || *port == OFPP_ANY) { + return error; + } + break; + + default: + OVS_NOT_REACHED(); } - OVS_NOT_REACHED(); + return (ofp_to_u16(*port) < ofp_to_u16(OFPP_MAX) + ? 0 + : OFPERR_OFPQOFC_BAD_PORT); } /* Constructs and returns the beginning of a reply to @@ -2576,15 +2585,10 @@ ofputil_append_queue_get_config_reply(struct ofpbuf *reply, opq10->queue_id = htonl(oqc->queue_id); len_ofs = (char *) &opq10->len - (char *) reply->data; } else { - struct ofp11_queue_get_config_reply *qgcr11; struct ofp12_packet_queue *opq12; - ovs_be32 port; - - qgcr11 = reply->msg; - port = qgcr11->port; opq12 = ofpbuf_put_zeros(reply, sizeof *opq12); - opq12->port = port; + opq12->port = ofputil_port_to_ofp11(oqc->port); opq12->queue_id = htonl(oqc->queue_id); len_ofs = (char *) &opq12->len - (char *) reply->data; } diff --git a/lib/ofp-util.h b/lib/ofp-util.h index c0541d45d..21c97af26 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -957,6 +957,7 @@ enum ofperr ofputil_decode_queue_get_config_request(const struct ofp_header *, /* Queue configuration reply. */ struct ofputil_queue_config { + ofp_port_t port; uint32_t queue_id; /* Each of these optional values is expressed in tenths of a percent. diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 4fa045ff4..b211c5ef9 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -6230,30 +6230,12 @@ handle_group_features_stats_request(struct ofconn *ofconn, return 0; } -static enum ofperr -handle_queue_get_config_request(struct ofconn *ofconn, - const struct ofp_header *oh) +static void +put_queue_config(struct ofport *ofport, struct ofpbuf *reply) { - struct ofproto *p = ofconn_get_ofproto(ofconn); struct netdev_queue_dump queue_dump; - struct ofport *ofport; unsigned int queue_id; - struct ofpbuf *reply; struct smap details; - ofp_port_t request; - enum ofperr error; - - error = ofputil_decode_queue_get_config_request(oh, &request); - if (error) { - return error; - } - - ofport = ofproto_get_port(p, request); - if (!ofport) { - return OFPERR_OFPQOFC_BAD_PORT; - } - - reply = ofputil_encode_queue_get_config_reply(oh); smap_init(&details); NETDEV_QUEUE_FOR_EACH (&queue_id, &details, &queue_dump, ofport->netdev) { @@ -6261,13 +6243,42 @@ handle_queue_get_config_request(struct ofconn *ofconn, /* None of the existing queues have compatible properties, so we * hard-code omitting min_rate and max_rate. */ + queue.port = ofport->ofp_port; queue.queue_id = queue_id; queue.min_rate = UINT16_MAX; queue.max_rate = UINT16_MAX; ofputil_append_queue_get_config_reply(reply, &queue); } smap_destroy(&details); +} + +static enum ofperr +handle_queue_get_config_request(struct ofconn *ofconn, + const struct ofp_header *oh) +{ + struct ofproto *ofproto = ofconn_get_ofproto(ofconn); + ofp_port_t port; + enum ofperr error; + error = ofputil_decode_queue_get_config_request(oh, &port); + if (error) { + return error; + } + + struct ofpbuf *reply = ofputil_encode_queue_get_config_reply(oh); + struct ofport *ofport; + if (port == OFPP_ANY) { + HMAP_FOR_EACH (ofport, hmap_node, &ofproto->ports) { + put_queue_config(ofport, reply); + } + } else { + ofport = ofproto_get_port(ofproto, port); + if (!ofport) { + ofpbuf_delete(reply); + return OFPERR_OFPQOFC_BAD_PORT; + } + put_queue_config(ofport, reply); + } ofconn_send_reply(ofconn, reply); return 0; diff --git a/tests/ofproto.at b/tests/ofproto.at index 52e1ab408..b3b8a0f7a 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -240,6 +240,11 @@ AT_CHECK([ovs-ofctl queue-get-config br0 1], [0], [stdout]) AT_CHECK([STRIP_XIDS stdout], [0], [dnl OFPT_QUEUE_GET_CONFIG_REPLY: port=1 ]) +AT_CHECK([ovs-ofctl queue-get-config br0], [0], [stdout]) +AT_CHECK([STRIP_XIDS stdout | sort], [0], [dnl +OFPT_QUEUE_GET_CONFIG_REPLY: port=1 +OFPT_QUEUE_GET_CONFIG_REPLY: port=2 +]) AT_CHECK([ovs-ofctl queue-get-config br0 10], [0], [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_PORT OFPT_QUEUE_GET_CONFIG_REQUEST (xid=0x2): port=10 @@ -254,6 +259,10 @@ AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 1], [0], [stdout]) AT_CHECK([STRIP_XIDS stdout], [0], [dnl OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=1 ]) +AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 ANY], [0], [stdout]) +AT_CHECK([STRIP_XIDS stdout], [0], [dnl +OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=ANY +]) AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 10], [0], [OFPT_ERROR (OF1.2) (xid=0x2): OFPQOFC_BAD_PORT OFPT_QUEUE_GET_CONFIG_REQUEST (OF1.2) (xid=0x2): port=10 diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index 286186b61..d6f310193 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -244,6 +244,17 @@ 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. . +.IP "\fBqueue\-get\-config \fIswitch \fR[\fIport\fR]" +Prints to the console information about all of the queues configured +on \fIport\fR within \fIswitch\fR. If \fIport\fR is \fBANY\fR or if +it is omitted, prints information about queues on every port. The +OpenFlow specification says that only physical ports have queues; in +particular, \fBLOCAL\fR is not valid for \fIport\fR. +.IP +This command has limited usefulness, because ports often have no +configured queues and because the OpenFlow protocol provides only very +limited information about the configuration of a queue. +. .SS "OpenFlow 1.1+ Group Table Commands" . The following commands work only with switches that support OpenFlow diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 0b529e06c..b46fd7296 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -378,7 +378,7 @@ usage(void) " dump-group-features SWITCH print group features\n" " dump-groups SWITCH [GROUP] print group description\n" " dump-group-stats SWITCH [GROUP] print group statistics\n" - " queue-get-config SWITCH PORT print queue information for port\n" + " queue-get-config SWITCH [PORT] print queue config for PORT\n" " add-meter SWITCH METER add meter described by METER\n" " mod-meter SWITCH METER modify specific METER\n" " del-meter SWITCH METER delete METER\n" @@ -1252,19 +1252,40 @@ static void ofctl_queue_get_config(struct ovs_cmdl_context *ctx) { const char *vconn_name = ctx->argv[1]; - const char *port_name = ctx->argv[2]; - enum ofputil_protocol protocol; - enum ofp_version version; - struct ofpbuf *request; - struct vconn *vconn; - ofp_port_t port; - - port = str_to_port_no(vconn_name, port_name); + const char *port_name = ctx->argc >= 3 ? ctx->argv[2] : NULL; + ofp_port_t port = (port_name + ? str_to_port_no(vconn_name, port_name) + : OFPP_ANY); - protocol = open_vconn(vconn_name, &vconn); - version = ofputil_protocol_to_ofp_version(protocol); - request = ofputil_encode_queue_get_config_request(version, port); - dump_transaction(vconn, request); + struct vconn *vconn; + enum ofputil_protocol protocol = open_vconn(vconn_name, &vconn); + enum ofp_version version = ofputil_protocol_to_ofp_version(protocol); + if (port == OFPP_ANY && version == OFP10_VERSION) { + /* The user requested all queues on all ports. OpenFlow 1.0 only + * supports getting queues for an individual port, so to implement the + * user's request we have to get a list of all the ports. + * + * We use a second vconn to avoid having to accumulate a list of all of + * the ports. */ + struct vconn *vconn2; + enum ofputil_protocol protocol2 = open_vconn(vconn_name, &vconn2); + enum ofp_version version2 = ofputil_protocol_to_ofp_version(protocol2); + + struct port_iterator pi; + struct ofputil_phy_port pp; + for (port_iterator_init(&pi, vconn); port_iterator_next(&pi, &pp); ) { + if (ofp_to_u16(pp.port_no) < ofp_to_u16(OFPP_MAX)) { + dump_transaction(vconn2, + ofputil_encode_queue_get_config_request( + version2, pp.port_no)); + } + } + port_iterator_destroy(&pi); + vconn_close(vconn2); + } else { + dump_transaction(vconn, ofputil_encode_queue_get_config_request( + version, port)); + } vconn_close(vconn); } @@ -3870,8 +3891,8 @@ static const struct ovs_cmdl_command all_commands[] = { 1, 2, ofctl_dump_aggregate }, { "queue-stats", "switch [port [queue]]", 1, 3, ofctl_queue_stats }, - { "queue-get-config", "switch port", - 2, 2, ofctl_queue_get_config }, + { "queue-get-config", "switch [port]", + 1, 2, ofctl_queue_get_config }, { "add-flow", "switch flow", 2, 2, ofctl_add_flow }, { "add-flows", "switch file", -- 2.39.5