[ovs-dev] [PATCH repost 2/2] ofproto: Implement OFPT_QUEUE_GET_CONFIG_REQUEST for OFPP_ANY in OF1.1+.
Ben Pfaff
blp at ovn.org
Mon Dec 21 23:26:35 UTC 2015
From: Ben Pfaff <blp at nicira.com>
I was not previously aware that this feature was missing.
Reported-by: Minoru TAKAHASHI <takahashi.minoru7 at gmail.com>
Reported-at: http://openvswitch.org/pipermail/discuss/2015-October/019229.html
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
NEWS | 1 +
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, 102 insertions(+), 44 deletions(-)
diff --git a/NEWS b/NEWS
index 81f49e2..54570e7 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,7 @@ v2.5.0 - xx xxx xxxx
- OpenFlow:
* Group chaining (where one OpenFlow group triggers another) is
now supported.
+ * OpenFlow 1.1+ OFPT_QUEUE_GET_CONFIG_REQUEST now supports OFPP_ANY.
* OpenFlow 1.4+ "importance" is now considered for flow eviction.
* OpenFlow 1.4+ OFPTC_EVICTION is now implemented.
* OpenFlow 1.4+ OFPTC_VACANCY_EVENTS is now implemented.
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index d7e9ccc..74fe413 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2490,14 +2490,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
@@ -2574,15 +2583,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 c0541d4..21c97af 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 c5bd949..214c009 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -6236,30 +6236,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) {
@@ -6267,13 +6249,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 c22d79f..b3aae98 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 0f0b1f7..4852759 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 931dda2..f55903e 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);
}
@@ -3868,8 +3889,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.1.3
More information about the dev
mailing list