Acked-by: Jarno Rajahalme <ja...@ovn.org>
> On Jan 18, 2016, at 11:27 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> OpenFlow 1.0 through 1.3 have a message OFPT_QUEUE_GET_CONFIG_REQUEST and
> its corresponding reply, for fetching a description of the queues
> configured on a given port. OpenFlow 1.4 changes this message to a
> multipart message OFPMP_QUEUE_DESC, which Open vSwitch has not until now
> implemented. This commit adds an implemntation of that message. Because
> the message is a replacement for the former one, this commit implements it
> using the same ofp-util functions as the former message, so that the client
> code doesn't have to distinguish a difference between versions.
>
> The ovs-ofctl command queue-get-config was previously undocumented (due
> only to an oversight). This commit corrects that and documents the new
> feature available with OpenFlow 1.4.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> NEWS | 4 +-
> OPENFLOW-1.1+.md | 4 +-
> include/openflow/openflow-1.4.h | 25 ++++
> lib/ofp-errors.h | 3 +
> lib/ofp-msgs.h | 15 +-
> lib/ofp-print.c | 32 +++--
> lib/ofp-util.c | 305 +++++++++++++++++++++++++++-------------
> lib/ofp-util.h | 16 +--
> ofproto/ofproto.c | 116 +++++++++------
> tests/ofp-print.at | 44 ++++++
> tests/ofproto.at | 46 ++++--
> utilities/ovs-ofctl.8.in | 12 +-
> utilities/ovs-ofctl.c | 20 +--
> 13 files changed, 453 insertions(+), 189 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 4433329..5c18867 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,7 +4,9 @@ Post-v2.5.0
> * New "monitor2" and "update2" extensions to RFC 7047.
> - OpenFlow:
> * OpenFlow 1.1+ OFPT_QUEUE_GET_CONFIG_REQUEST now supports OFPP_ANY.
> -
> + * OpenFlow 1.4+ OFPMP_QUEUE_DESC is now supported.
> + - ovs-ofctl:
> + * queue-get-config command now allows a queue ID to be specified.
>
> v2.5.0 - xx xxx xxxx
> ---------------------
> diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md
> index 0b2f0f9..537f660 100644
> --- a/OPENFLOW-1.1+.md
> +++ b/OPENFLOW-1.1+.md
> @@ -194,8 +194,8 @@ OpenFlow 1.4 features are listed in the previous section.
> Many on-wire structures got TLVs.
> Already implemented: port desc properties, port mod properties,
> port stats properties, table mod properties,
> - queue stats, unified property errors.
> - Remaining required: set-async, queue desc
> + queue stats, unified property errors, queue desc.
> + Remaining required: set-async
> Remaining optional: table desc, table-status
> [EXT-262]
> [required for OF1.4+]
> diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
> index bbdd54f..bce1598 100644
> --- a/include/openflow/openflow-1.4.h
> +++ b/include/openflow/openflow-1.4.h
> @@ -219,6 +219,31 @@ struct ofp14_queue_stats {
> OFP_ASSERT(sizeof(struct ofp14_queue_stats) == 48);
>
>
> +/* ## ---------------- ## */
> +/* ## ofp14_queue_desc ## */
> +/* ## ---------------- ## */
> +
> +struct ofp14_queue_desc_request {
> + ovs_be32 port; /* All ports if OFPP_ANY. */
> + ovs_be32 queue; /* All queues if OFPQ_ALL. */
> +};
> +OFP_ASSERT(sizeof(struct ofp14_queue_desc_request) == 8);
> +
> +/* Body of reply to OFPMP_QUEUE_DESC request. */
> +struct ofp14_queue_desc {
> + ovs_be32 port_no; /* Port this queue is attached to. */
> + ovs_be32 queue_id; /* ID for the specific queue. */
> + ovs_be16 len; /* Length in bytes of this queue desc. */
> + uint8_t pad[6]; /* 64-bit alignment. */
> +};
> +OFP_ASSERT(sizeof(struct ofp14_queue_desc) == 16);
> +
> +enum ofp14_queue_desc_prop_type {
> + OFPQDPT14_MIN_RATE = 1,
> + OFPQDPT14_MAX_RATE = 2,
> + OFPQDPT14_EXPERIMENTER = 0xffff
> +};
> +
> /* ## -------------- ## */
> /* ## Miscellaneous. ## */
> /* ## -------------- ## */
> diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h
> index 4f59acf..8e13873 100644
> --- a/lib/ofp-errors.h
> +++ b/lib/ofp-errors.h
> @@ -504,6 +504,9 @@ enum ofperr {
> /* OF1.0(5,2), OF1.1+(9,2). Permissions error. */
> OFPERR_OFPQOFC_EPERM,
>
> + /* NX1.4+(23). System error retrieving queue details. */
> + OFPERR_NXQOFC_QUEUE_ERROR,
> +
> /* ## -------------------------- ## */
> /* ## OFPET_SWITCH_CONFIG_FAILED ## */
> /* ## -------------------------- ## */
> diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
> index 2c4a916..54018b4 100644
> --- a/lib/ofp-msgs.h
> +++ b/lib/ofp-msgs.h
> @@ -208,12 +208,12 @@ enum ofpraw {
>
> /* OFPT 1.0 (20): struct ofp10_queue_get_config_request. */
> OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST,
> - /* OFPT 1.1+ (22): struct ofp11_queue_get_config_request. */
> + /* OFPT 1.1-1.3 (22): struct ofp11_queue_get_config_request. */
> OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST,
>
> /* OFPT 1.0 (21): struct ofp10_queue_get_config_reply, uint8_t[8][]. */
> OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY,
> - /* OFPT 1.1+ (23): struct ofp11_queue_get_config_reply, uint8_t[8][]. */
> + /* OFPT 1.1-1.3 (23): struct ofp11_queue_get_config_reply, uint8_t[8][].
> */
> OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY,
>
> /* OFPT 1.2+ (24): struct ofp12_role_request. */
> @@ -396,6 +396,11 @@ enum ofpraw {
> /* OFPST 1.4+ (13): uint8_t[8][]. */
> OFPRAW_OFPST14_PORT_DESC_REPLY,
>
> + /* OFPST 1.4+ (15): struct ofp14_queue_desc_request. */
> + OFPRAW_OFPST14_QUEUE_DESC_REQUEST,
> + /* OFPST 1.4+ (15): uint8_t[8][]. */
> + OFPRAW_OFPST14_QUEUE_DESC_REPLY,
> +
> /* OFPST 1.4+ (16): uint8_t[8][]. */
> OFPRAW_OFPST14_FLOW_MONITOR_REQUEST,
> /* NXST 1.0 (2): uint8_t[8][]. */
> @@ -537,9 +542,11 @@ enum ofptype {
>
> /* Queue Configuration messages. */
> OFPTYPE_QUEUE_GET_CONFIG_REQUEST, /*
> OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST.
> - *
> OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST. */
> + *
> OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST.
> + * OFPRAW_OFPST14_QUEUE_DESC_REQUEST.
> */
> OFPTYPE_QUEUE_GET_CONFIG_REPLY, /* OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY.
> - * OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY.
> */
> + * OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY.
> + * OFPRAW_OFPST14_QUEUE_DESC_REPLY. */
>
> /* Controller role change request messages. */
> OFPTYPE_ROLE_REQUEST, /* OFPRAW_OFPT12_ROLE_REQUEST.
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index af56e9b..42e822b 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -1086,8 +1086,9 @@ ofp_print_queue_get_config_request(struct ds *string,
> {
> enum ofperr error;
> ofp_port_t port;
> + uint32_t queue;
>
> - error = ofputil_decode_queue_get_config_request(oh, &port);
> + error = ofputil_decode_queue_get_config_request(oh, &port, &queue);
> if (error) {
> ofp_print_error(string, error);
> return;
> @@ -1095,6 +1096,11 @@ ofp_print_queue_get_config_request(struct ds *string,
>
> ds_put_cstr(string, " port=");
> ofputil_format_port(port, string);
> +
> + if (queue != OFPQ_ALL) {
> + ds_put_cstr(string, " queue=");
> + ofp_print_queue_name(string, queue);
> + }
> }
>
> static void
> @@ -1111,21 +1117,12 @@ static void
> ofp_print_queue_get_config_reply(struct ds *string,
> const struct ofp_header *oh)
> {
> - enum ofperr error;
> struct ofpbuf b;
> - ofp_port_t port;
> + ofp_port_t port = 0;
>
> ofpbuf_use_const(&b, oh, ntohs(oh->length));
> - error = ofputil_decode_queue_get_config_reply(&b, &port);
> - if (error) {
> - ofp_print_error(string, error);
> - return;
> - }
> -
> - ds_put_cstr(string, " port=");
> - ofputil_format_port(port, string);
> - ds_put_char(string, '\n');
>
> + ds_put_char(string, ' ');
> for (;;) {
> struct ofputil_queue_config queue;
> int retval;
> @@ -1135,10 +1132,19 @@ ofp_print_queue_get_config_reply(struct ds *string,
> if (retval != EOF) {
> ofp_print_error(string, retval);
> }
> + ds_chomp(string, ' ');
> break;
> }
>
> - ds_put_format(string, "queue %"PRIu32":", queue.queue_id);
> + if (queue.port != port) {
> + port = queue.port;
> +
> + ds_put_cstr(string, "port=");
> + ofputil_format_port(port, string);
> + ds_put_char(string, '\n');
> + }
> +
> + ds_put_format(string, "queue %"PRIu32":", queue.queue);
> print_queue_rate(string, "min_rate", queue.min_rate);
> print_queue_rate(string, "max_rate", queue.max_rate);
> ds_put_char(string, '\n');
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 1225664..6f9aeb7 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -2343,10 +2343,14 @@ ofputil_decode_nxst_flow_request(struct
> ofputil_flow_stats_request *fsr,
> }
>
> /* Constructs and returns an OFPT_QUEUE_GET_CONFIG request for the specified
> - * 'port', suitable for OpenFlow version 'version'. */
> + * 'port' and 'queue', suitable for OpenFlow version 'version'.
> + *
> + * 'queue' is honored only for OpenFlow 1.4 and later; older versions always
> + * request all queues. */
> struct ofpbuf *
> ofputil_encode_queue_get_config_request(enum ofp_version version,
> - ofp_port_t port)
> + ofp_port_t port,
> + uint32_t queue)
> {
> struct ofpbuf *request;
>
> @@ -2357,13 +2361,21 @@ ofputil_encode_queue_get_config_request(enum
> ofp_version version,
> version, 0);
> qgcr10 = ofpbuf_put_zeros(request, sizeof *qgcr10);
> qgcr10->port = htons(ofp_to_u16(port));
> - } else {
> + } else if (version < OFP14_VERSION) {
> struct ofp11_queue_get_config_request *qgcr11;
>
> request = ofpraw_alloc(OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST,
> version, 0);
> qgcr11 = ofpbuf_put_zeros(request, sizeof *qgcr11);
> qgcr11->port = ofputil_port_to_ofp11(port);
> + } else {
> + struct ofp14_queue_desc_request *qdr14;
> +
> + request = ofpraw_alloc(OFPRAW_OFPST14_QUEUE_DESC_REQUEST,
> + version, 0);
> + qdr14 = ofpbuf_put_zeros(request, sizeof *qdr14);
> + qdr14->port = ofputil_port_to_ofp11(port);
> + qdr14->queue = htonl(queue);
> }
>
> return request;
> @@ -2374,10 +2386,11 @@ ofputil_encode_queue_get_config_request(enum
> ofp_version version,
> * code. */
> enum ofperr
> ofputil_decode_queue_get_config_request(const struct ofp_header *oh,
> - ofp_port_t *port)
> + ofp_port_t *port, uint32_t *queue)
> {
> const struct ofp10_queue_get_config_request *qgcr10;
> const struct ofp11_queue_get_config_request *qgcr11;
> + const struct ofp14_queue_desc_request *qdr14;
> enum ofpraw raw;
> struct ofpbuf b;
>
> @@ -2388,16 +2401,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));
> + *queue = OFPQ_ALL;
> break;
>
> case OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST:
> qgcr11 = b.data;
> + *queue = OFPQ_ALL;
> enum ofperr error = ofputil_port_from_ofp11(qgcr11->port, port);
> if (error || *port == OFPP_ANY) {
> return error;
> }
> break;
>
> + case OFPRAW_OFPST14_QUEUE_DESC_REQUEST:
> + qdr14 = b.data;
> + *queue = ntohl(qdr14->queue);
> + return ofputil_port_from_ofp11(qdr14->port, port);
> +
> default:
> OVS_NOT_REACHED();
> }
> @@ -2408,45 +2428,49 @@ ofputil_decode_queue_get_config_request(const struct
> ofp_header *oh,
> }
>
> /* Constructs and returns the beginning of a reply to
> - * OFPT_QUEUE_GET_CONFIG_REQUEST 'oh'. The caller may append information
> about
> - * individual queues with ofputil_append_queue_get_config_reply(). */
> -struct ofpbuf *
> -ofputil_encode_queue_get_config_reply(const struct ofp_header *oh)
> + * OFPT_QUEUE_GET_CONFIG_REQUEST or OFPMP_QUEUE_DESC request 'oh'. The
> caller
> + * may append information about individual queues with
> + * ofputil_append_queue_get_config_reply(). */
> +void
> +ofputil_start_queue_get_config_reply(const struct ofp_header *request,
> + struct ovs_list *replies)
> {
> - struct ofp10_queue_get_config_reply *qgcr10;
> - struct ofp11_queue_get_config_reply *qgcr11;
> struct ofpbuf *reply;
> enum ofperr error;
> - struct ofpbuf b;
> - enum ofpraw raw;
> ofp_port_t port;
> + uint32_t queue;
>
> - error = ofputil_decode_queue_get_config_request(oh, &port);
> + error = ofputil_decode_queue_get_config_request(request, &port, &queue);
> ovs_assert(!error);
>
> - ofpbuf_use_const(&b, oh, ntohs(oh->length));
> - raw = ofpraw_pull_assert(&b);
> -
> + enum ofpraw raw = ofpraw_decode_assert(request);
> switch ((int) raw) {
> case OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST:
> reply = ofpraw_alloc_reply(OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY,
> - oh, 0);
> - qgcr10 = ofpbuf_put_zeros(reply, sizeof *qgcr10);
> + request, 0);
> + struct ofp10_queue_get_config_reply *qgcr10
> + = ofpbuf_put_zeros(reply, sizeof *qgcr10);
> qgcr10->port = htons(ofp_to_u16(port));
> break;
>
> case OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST:
> reply = ofpraw_alloc_reply(OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY,
> - oh, 0);
> - qgcr11 = ofpbuf_put_zeros(reply, sizeof *qgcr11);
> + request, 0);
> + struct ofp11_queue_get_config_reply *qgcr11
> + = ofpbuf_put_zeros(reply, sizeof *qgcr11);
> qgcr11->port = ofputil_port_to_ofp11(port);
> break;
>
> + case OFPRAW_OFPST14_QUEUE_DESC_REQUEST:
> + reply = ofpraw_alloc_stats_reply(request, 0);
> + break;
> +
> default:
> OVS_NOT_REACHED();
> }
>
> - return reply;
> + list_init(replies);
> + list_push_back(replies, &reply->list_node);
> }
>
> static void
> @@ -2463,64 +2487,58 @@ put_ofp11_queue_rate(struct ofpbuf *reply,
> }
> }
>
> -/* Appends a queue description for 'queue_id' to the
> - * OFPT_QUEUE_GET_CONFIG_REPLY already in 'oh'. */
> +static void
> +put_ofp14_queue_rate(struct ofpbuf *reply,
> + enum ofp14_queue_desc_prop_type type, uint16_t rate)
> +{
> + if (rate != UINT16_MAX) {
> + ofpprop_put_u16(reply, type, rate);
> + }
> +}
> +
> void
> -ofputil_append_queue_get_config_reply(struct ofpbuf *reply,
> - const struct ofputil_queue_config *oqc)
> +ofputil_append_queue_get_config_reply(const struct ofputil_queue_config *qc,
> + struct ovs_list *replies)
> {
> - const struct ofp_header *oh = reply->data;
> - size_t start_ofs, len_ofs;
> + enum ofp_version ofp_version = ofpmp_version(replies);
> + struct ofpbuf *reply = ofpbuf_from_list(list_back(replies));
> + size_t start_ofs = reply->size;
> + size_t len_ofs;
> ovs_be16 *len;
>
> - start_ofs = reply->size;
> - if (oh->version < OFP12_VERSION) {
> - struct ofp10_packet_queue *opq10;
> + if (ofp_version < OFP14_VERSION) {
> + if (ofp_version < OFP12_VERSION) {
> + struct ofp10_packet_queue *opq10;
>
> - opq10 = ofpbuf_put_zeros(reply, sizeof *opq10);
> - opq10->queue_id = htonl(oqc->queue_id);
> - len_ofs = (char *) &opq10->len - (char *) reply->data;
> - } else {
> - struct ofp12_packet_queue *opq12;
> + opq10 = ofpbuf_put_zeros(reply, sizeof *opq10);
> + opq10->queue_id = htonl(qc->queue);
> + len_ofs = (char *) &opq10->len - (char *) reply->data;
> + } else {
> + struct ofp12_packet_queue *opq12;
>
> - opq12 = ofpbuf_put_zeros(reply, sizeof *opq12);
> - opq12->port = ofputil_port_to_ofp11(oqc->port);
> - opq12->queue_id = htonl(oqc->queue_id);
> - len_ofs = (char *) &opq12->len - (char *) reply->data;
> - }
> + opq12 = ofpbuf_put_zeros(reply, sizeof *opq12);
> + opq12->port = ofputil_port_to_ofp11(qc->port);
> + opq12->queue_id = htonl(qc->queue);
> + len_ofs = (char *) &opq12->len - (char *) reply->data;
> + }
>
> - put_ofp11_queue_rate(reply, OFPQT10_MIN_RATE, oqc->min_rate);
> - put_ofp11_queue_rate(reply, OFPQT11_MAX_RATE, oqc->max_rate);
> + put_ofp11_queue_rate(reply, OFPQT10_MIN_RATE, qc->min_rate);
> + put_ofp11_queue_rate(reply, OFPQT11_MAX_RATE, qc->max_rate);
> + } else {
> + struct ofp14_queue_desc *oqd = ofpbuf_put_zeros(reply, sizeof *oqd);
> + oqd->port_no = ofputil_port_to_ofp11(qc->port);
> + oqd->queue_id = htonl(qc->queue);
> + len_ofs = (char *) &oqd->len - (char *) reply->data;
> + put_ofp14_queue_rate(reply, OFPQDPT14_MIN_RATE, qc->min_rate);
> + put_ofp14_queue_rate(reply, OFPQDPT14_MAX_RATE, qc->max_rate);
> + }
>
> len = ofpbuf_at(reply, len_ofs, sizeof *len);
> *len = htons(reply->size - start_ofs);
> -}
> -
> -/* Decodes the initial part of an OFPT_QUEUE_GET_CONFIG_REPLY from 'reply'
> and
> - * stores in '*port' the port that the reply is about. The caller may call
> - * ofputil_pull_queue_get_config_reply() to obtain information about
> individual
> - * queues included in the reply. Returns 0 if successful, otherwise an
> - * ofperr.*/
> -enum ofperr
> -ofputil_decode_queue_get_config_reply(struct ofpbuf *reply, ofp_port_t *port)
> -{
> - const struct ofp10_queue_get_config_reply *qgcr10;
> - const struct ofp11_queue_get_config_reply *qgcr11;
> - enum ofpraw raw;
> -
> - raw = ofpraw_pull_assert(reply);
> - switch ((int) raw) {
> - case OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY:
> - qgcr10 = ofpbuf_pull(reply, sizeof *qgcr10);
> - *port = u16_to_ofp(ntohs(qgcr10->port));
> - return 0;
>
> - case OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY:
> - qgcr11 = ofpbuf_pull(reply, sizeof *qgcr11);
> - return ofputil_port_from_ofp11(qgcr11->port, port);
> + if (ofp_version >= OFP14_VERSION) {
> + ofpmp_postappend(replies, start_ofs);
> }
> -
> - OVS_NOT_REACHED();
> }
>
> static enum ofperr
> @@ -2538,65 +2556,65 @@ parse_ofp11_queue_rate(const struct
> ofp10_queue_prop_header *hdr,
> }
> }
>
> -/* Decodes information about a queue from the OFPT_QUEUE_GET_CONFIG_REPLY in
> - * 'reply' and stores it in '*queue'.
> ofputil_decode_queue_get_config_reply()
> - * must already have pulled off the main header.
> - *
> - * This function returns EOF if the last queue has already been decoded, 0
> if a
> - * queue was successfully decoded into '*queue', or an ofperr if there was a
> - * problem decoding 'reply'. */
> -int
> -ofputil_pull_queue_get_config_reply(struct ofpbuf *reply,
> - struct ofputil_queue_config *queue)
> +static int
> +ofputil_pull_queue_get_config_reply10(struct ofpbuf *msg,
> + struct ofputil_queue_config *queue)
> {
> - const struct ofp_header *oh;
> - unsigned int opq_len;
> - unsigned int len;
> + const struct ofp_header *oh = msg->header;
> + unsigned int opq_len; /* Length of protocol-specific queue header.
> */
> + unsigned int len; /* Total length of queue + properties. */
>
> - if (!reply->size) {
> - return EOF;
> + /* Obtain the port number from the message header. */
> + if (oh->version == OFP10_VERSION) {
> + const struct ofp10_queue_get_config_reply *oqgcr10 = msg->msg;
> + queue->port = u16_to_ofp(ntohs(oqgcr10->port));
> + } else {
> + const struct ofp11_queue_get_config_reply *oqgcr11 = msg->msg;
> + enum ofperr error = ofputil_port_from_ofp11(oqgcr11->port,
> + &queue->port);
> + if (error) {
> + return error;
> + }
> }
>
> - queue->min_rate = UINT16_MAX;
> - queue->max_rate = UINT16_MAX;
> -
> - oh = reply->header;
> + /* Pull off the queue header and get the queue number and length. */
> if (oh->version < OFP12_VERSION) {
> const struct ofp10_packet_queue *opq10;
> -
> - opq10 = ofpbuf_try_pull(reply, sizeof *opq10);
> + opq10 = ofpbuf_try_pull(msg, sizeof *opq10);
> if (!opq10) {
> return OFPERR_OFPBRC_BAD_LEN;
> }
> - queue->queue_id = ntohl(opq10->queue_id);
> + queue->queue = ntohl(opq10->queue_id);
> len = ntohs(opq10->len);
> opq_len = sizeof *opq10;
> } else {
> const struct ofp12_packet_queue *opq12;
> -
> - opq12 = ofpbuf_try_pull(reply, sizeof *opq12);
> + opq12 = ofpbuf_try_pull(msg, sizeof *opq12);
> if (!opq12) {
> return OFPERR_OFPBRC_BAD_LEN;
> }
> - queue->queue_id = ntohl(opq12->queue_id);
> + queue->queue = ntohl(opq12->queue_id);
> len = ntohs(opq12->len);
> opq_len = sizeof *opq12;
> }
>
> - if (len < opq_len || len > reply->size + opq_len || len % 8) {
> + /* Length check. */
> + if (len < opq_len || len > msg->size + opq_len || len % 8) {
> return OFPERR_OFPBRC_BAD_LEN;
> }
> len -= opq_len;
>
> + /* Pull properties. The format of these properties differs from used in
> + * OF1.4+ so we can't use the common property functions. */
> while (len > 0) {
> const struct ofp10_queue_prop_header *hdr;
> unsigned int property;
> unsigned int prop_len;
> enum ofperr error = 0;
>
> - hdr = ofpbuf_at_assert(reply, 0, sizeof *hdr);
> + hdr = ofpbuf_at_assert(msg, 0, sizeof *hdr);
> prop_len = ntohs(hdr->len);
> - if (prop_len < sizeof *hdr || prop_len > reply->size || prop_len %
> 8) {
> + if (prop_len < sizeof *hdr || prop_len > msg->size || prop_len % 8) {
> return OFPERR_OFPBRC_BAD_LEN;
> }
>
> @@ -2618,12 +2636,107 @@ ofputil_pull_queue_get_config_reply(struct ofpbuf
> *reply,
> return error;
> }
>
> - ofpbuf_pull(reply, prop_len);
> + ofpbuf_pull(msg, prop_len);
> len -= prop_len;
> }
> return 0;
> }
>
> +static int
> +ofputil_pull_queue_get_config_reply14(struct ofpbuf *msg,
> + struct ofputil_queue_config *queue)
> +{
> + struct ofp14_queue_desc *oqd14 = ofpbuf_try_pull(msg, sizeof *oqd14);
> + if (!oqd14) {
> + return OFPERR_OFPBRC_BAD_LEN;
> + }
> + enum ofperr error = ofputil_port_from_ofp11(oqd14->port_no,
> &queue->port);
> + if (error) {
> + return error;
> + }
> + queue->queue = ntohl(oqd14->queue_id);
> +
> + /* Length check. */
> + unsigned int len = ntohs(oqd14->len);
> + if (len < sizeof *oqd14 || len > msg->size + sizeof *oqd14 || len % 8) {
> + return OFPERR_OFPBRC_BAD_LEN;
> + }
> + len -= sizeof *oqd14;
> +
> + struct ofpbuf properties;
> + ofpbuf_use_const(&properties, ofpbuf_pull(msg, len), len);
> + while (properties.size > 0) {
> + struct ofpbuf payload;
> + uint64_t type;
> +
> + error = ofpprop_pull(&properties, &payload, &type);
> + if (error) {
> + return error;
> + }
> +
> + switch (type) {
> + case OFPQDPT14_MIN_RATE:
> + error = ofpprop_parse_u16(&payload, &queue->min_rate);
> + break;
> +
> + case OFPQDPT14_MAX_RATE:
> + error = ofpprop_parse_u16(&payload, &queue->max_rate);
> + break;
> +
> + default:
> + error = OFPPROP_UNKNOWN(true, "queue desc", type);
> + break;
> + }
> +
> + if (error) {
> + return error;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* Decodes information about a queue from the OFPT_QUEUE_GET_CONFIG_REPLY in
> + * 'reply' and stores it in '*queue'.
> ofputil_decode_queue_get_config_reply()
> + * must already have pulled off the main header.
> + *
> + * This function returns EOF if the last queue has already been decoded, 0
> if a
> + * queue was successfully decoded into '*queue', or an ofperr if there was a
> + * problem decoding 'reply'. */
> +int
> +ofputil_pull_queue_get_config_reply(struct ofpbuf *msg,
> + struct ofputil_queue_config *queue)
> +{
> + enum ofpraw raw;
> + if (!msg->header) {
> + /* Pull OpenFlow header. */
> + raw = ofpraw_pull_assert(msg);
> +
> + /* Pull protocol-specific ofp_queue_get_config_reply header (OF1.4
> + * doesn't have one at all). */
> + if (raw == OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY) {
> + ofpbuf_pull(msg, sizeof(struct ofp10_queue_get_config_reply));
> + } else if (raw == OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY) {
> + ofpbuf_pull(msg, sizeof(struct ofp11_queue_get_config_reply));
> + } else {
> + ovs_assert(raw == OFPRAW_OFPST14_QUEUE_DESC_REPLY);
> + }
> + } else {
> + raw = ofpraw_decode_assert(msg->header);
> + }
> +
> + queue->min_rate = UINT16_MAX;
> + queue->max_rate = UINT16_MAX;
> +
> + if (!msg->size) {
> + return EOF;
> + } else if (raw == OFPRAW_OFPST14_QUEUE_DESC_REPLY) {
> + return ofputil_pull_queue_get_config_reply14(msg, queue);
> + } else {
> + return ofputil_pull_queue_get_config_reply10(msg, queue);
> + }
> +}
> +
> /* Converts an OFPST_FLOW, OFPST_AGGREGATE, NXST_FLOW, or NXST_AGGREGATE
> * request 'oh', into an abstract flow_stats_request in 'fsr'. Returns 0 if
> * successful, otherwise an OpenFlow error code. */
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index f411651..89f3d95 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -986,14 +986,16 @@ int ofputil_decode_table_stats_reply(struct ofpbuf
> *reply,
>
> /* Queue configuration request. */
> struct ofpbuf *ofputil_encode_queue_get_config_request(enum ofp_version,
> - ofp_port_t port);
> + ofp_port_t port,
> + uint32_t queue);
> enum ofperr ofputil_decode_queue_get_config_request(const struct ofp_header *,
> - ofp_port_t *port);
> + ofp_port_t *port,
> + uint32_t *queue);
>
> /* Queue configuration reply. */
> struct ofputil_queue_config {
> ofp_port_t port;
> - uint32_t queue_id;
> + uint32_t queue;
>
> /* Each of these optional values is expressed in tenths of a percent.
> * Values greater than 1000 indicate that the feature is disabled.
> @@ -1002,13 +1004,11 @@ struct ofputil_queue_config {
> uint16_t max_rate;
> };
>
> -struct ofpbuf *ofputil_encode_queue_get_config_reply(
> - const struct ofp_header *request);
> +void ofputil_start_queue_get_config_reply(const struct ofp_header *request,
> + struct ovs_list *replies);
> void ofputil_append_queue_get_config_reply(
> - struct ofpbuf *reply, const struct ofputil_queue_config *);
> + const struct ofputil_queue_config *, struct ovs_list *replies);
>
> -enum ofperr ofputil_decode_queue_get_config_reply(struct ofpbuf *reply,
> - ofp_port_t *);
> int ofputil_pull_queue_get_config_reply(struct ofpbuf *reply,
> struct ofputil_queue_config *);
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 386009e..8dcfafe 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -6227,57 +6227,89 @@ handle_group_features_stats_request(struct ofconn
> *ofconn,
> }
>
> static void
> -put_queue_config(struct ofport *ofport, struct ofpbuf *reply)
> +put_queue_get_config_reply(struct ofport *port, uint32_t queue,
> + struct ovs_list *replies)
> {
> - struct netdev_queue_dump queue_dump;
> - unsigned int queue_id;
> - struct smap details;
> + struct ofputil_queue_config qc;
>
> - smap_init(&details);
> - NETDEV_QUEUE_FOR_EACH (&queue_id, &details, &queue_dump, ofport->netdev) {
> - struct ofputil_queue_config queue;
> + /* None of the existing queues have compatible properties, so we
> hard-code
> + * omitting min_rate and max_rate. */
> + qc.port = port->ofp_port;
> + qc.queue = queue;
> + qc.min_rate = UINT16_MAX;
> + qc.max_rate = UINT16_MAX;
> + ofputil_append_queue_get_config_reply(&qc, replies);
> +}
>
> - /* 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 int
> +handle_queue_get_config_request_for_port(struct ofport *port, uint32_t queue,
> + struct ovs_list *replies)
> +{
> + struct smap details = SMAP_INITIALIZER(&details);
> + if (queue != OFPQ_ALL) {
> + int error = netdev_get_queue(port->netdev, queue, &details);
> + switch (error) {
> + case 0:
> + put_queue_get_config_reply(port, queue, replies);
> + break;
> + case EOPNOTSUPP:
> + case EINVAL:
> + return OFPERR_OFPQOFC_BAD_QUEUE;
> + default:
> + return OFPERR_NXQOFC_QUEUE_ERROR;
> + }
> + } else {
> + struct netdev_queue_dump queue_dump;
> + uint32_t queue_id;
> +
> + NETDEV_QUEUE_FOR_EACH (&queue_id, &details, &queue_dump,
> + port->netdev) {
> + put_queue_get_config_reply(port, queue_id, replies);
> + }
> + }
> + smap_destroy(&details);
> + return 0;
> }
>
> 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;
> + struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
> + struct ovs_list replies;
> + struct ofport *port;
> + ofp_port_t req_port;
> + uint32_t req_queue;
> + enum ofperr error;
> +
> + error = ofputil_decode_queue_get_config_request(oh, &req_port,
> &req_queue);
> + if (error) {
> + return error;
> + }
> +
> + ofputil_start_queue_get_config_reply(oh, &replies);
> + if (req_port == OFPP_ANY) {
> + error = OFPERR_OFPQOFC_BAD_QUEUE;
> + HMAP_FOR_EACH (port, hmap_node, &ofproto->ports) {
> + if (!handle_queue_get_config_request_for_port(port, req_queue,
> + &replies)) {
> + error = 0;
> + }
> + }
> + } else {
> + port = ofproto_get_port(ofproto, req_port);
> + error = (port
> + ? handle_queue_get_config_request_for_port(port, req_queue,
> + &replies)
> + : OFPERR_OFPQOFC_BAD_PORT);
> + }
> + if (!error) {
> + ofconn_send_replies(ofconn, &replies);
> + } else {
> + ofpbuf_list_delete(&replies);
> + }
> +
> + return error;
> }
>
> static enum ofperr
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index 24e602e..6f20fac 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -2546,6 +2546,15 @@ OFPT_QUEUE_GET_CONFIG_REQUEST (OF1.2) (xid=0x1): port=1
> ])
> AT_CLEANUP
>
> +AT_SETUP([OFPST_QUEUE_DESC request - OF1.4])
> +AT_KEYWORDS([ofp-print OFPT_QUEUE_GET_CONFIG_REQUEST])
> +AT_CHECK([ovs-ofctl ofp-print "\
> +05 12 00 18 00 00 00 01 00 0f 00 00 00 00 00 00 \
> +00 00 00 01 00 00 00 02"], [0],
> + [OFPST_QUEUE_DESC request (OF1.4) (xid=0x1): port=1 queue=2
> +])
> +AT_CLEANUP
> +
> AT_SETUP([OFPT_QUEUE_GET_CONFIG_REPLY - OF1.0])
> AT_KEYWORDS([ofp-print])
> AT_CHECK([ovs-ofctl ofp-print "01 15 00 40 00 00 00 01 \
> @@ -2591,6 +2600,41 @@ queue 17476:
> ])
> AT_CLEANUP
>
> +AT_SETUP([OFPT_QUEUE_GET_CONFIG_REPLY - OF1.3])
> +AT_KEYWORDS([ofp-print])
> +AT_CHECK([ovs-ofctl ofp-print "04 17 00 50 00 00 00 01 \
> +00 00 00 01 00 00 00 00 \
> +00 00 55 55 00 00 00 01 00 30 00 00 00 00 00 00 \
> +00 01 00 10 00 00 00 00 01 f4 00 00 00 00 00 00 \
> +00 02 00 10 00 00 00 00 02 ee 00 00 00 00 00 00 \
> +00 00 44 44 00 08 00 01 00 10 00 00 00 00 00 00 \
> +"], [0], [dnl
> +OFPT_QUEUE_GET_CONFIG_REPLY (OF1.3) (xid=0x1): port=1
> +queue 21845: min_rate:50.0% max_rate:75.0%
> +queue 17476:
> +])
> +AT_CLEANUP
> +
> +# OF1.4 renamed OFPT_QUEUE_GET_CONFIG_REPLY to OFPST_QUEUE_DESC.
> +AT_SETUP([OFPST_QUEUE_DESC reply - OF1.4])
> +AT_KEYWORDS([ofp-print OFPT_QUEUE_GET_CONFIG_REPLY])
> +AT_CHECK([ovs-ofctl ofp-print "\
> +05 13 00 48 00 00 00 01 00 0f 00 00 00 00 00 00 \
> +
> +00 00 00 01 00 00 55 55 00 20 00 00 00 00 00 00 \
> +00 01 00 08 01 f4 00 00 \
> +00 02 00 08 02 ee 00 00 \
> +
> +00 00 00 02 00 00 44 44 00 18 00 00 00 00 00 00 \
> +00 02 00 08 00 64 00 00 \
> +"], [0], [dnl
> +OFPST_QUEUE_DESC reply (OF1.4) (xid=0x1): port=1
> +queue 21845: min_rate:50.0% max_rate:75.0%
> +port=2
> +queue 17476: max_rate:10.0%
> +])
> +AT_CLEANUP
> +
> AT_SETUP([OFPT_SET_ASYNC - OF1.3])
> AT_KEYWORDS([ofp-print])
> AT_CHECK([ovs-ofctl ofp-print "\
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index a4064d4..ab4d254 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -253,14 +253,14 @@ AT_CLEANUP
> AT_SETUP([ofproto - queue configuration - (OpenFlow 1.1)])
> OVS_VSWITCHD_START
> ADD_OF_PORTS([br0], [1], [2])
> -AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 1], [0], [stdout])
> +AT_CHECK([ovs-ofctl -O OpenFlow11 queue-get-config br0 1], [0], [stdout])
> AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> -OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=1
> +OFPT_QUEUE_GET_CONFIG_REPLY (OF1.1): port=1
> queue 0:
> ])
> -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
> +AT_CHECK([ovs-ofctl -O OpenFlow11 queue-get-config br0 10 | STRIP_XIDS], [0],
> + [OFPT_ERROR (OF1.1): OFPQOFC_BAD_PORT
> +OFPT_QUEUE_GET_CONFIG_REQUEST (OF1.1): port=10
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> @@ -280,9 +280,39 @@ queue 0:
> queue 0:
> queue 0:
> ])
> -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
> +AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 10 | STRIP_XIDS], [0],
> + [OFPT_ERROR (OF1.2): OFPQOFC_BAD_PORT
> +OFPT_QUEUE_GET_CONFIG_REQUEST (OF1.2): port=10
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> +AT_SETUP([ofproto - queue configuration - (OpenFlow 1.4)])
> +OVS_VSWITCHD_START
> +ADD_OF_PORTS([br0], [1], [2])
> +
> +AT_CHECK([ovs-ofctl -O OpenFlow14 queue-get-config br0 any | STRIP_XIDS],
> [0],
> + [OFPST_QUEUE_DESC reply (OF1.4): port=1
> +queue 0:
> +port=LOCAL
> +queue 0:
> +port=2
> +queue 0:
> +])
> +
> +AT_CHECK([ovs-ofctl -O OpenFlow14 queue-get-config br0 1 | STRIP_XIDS], [0],
> + [OFPST_QUEUE_DESC reply (OF1.4): port=1
> +queue 0:
> +])
> +
> +AT_CHECK([ovs-ofctl -O OpenFlow14 queue-get-config br0 10 | STRIP_XIDS], [0],
> + [OFPT_ERROR (OF1.4): OFPQOFC_BAD_PORT
> +OFPST_QUEUE_DESC request (OF1.4): port=10
> +])
> +
> +AT_CHECK([ovs-ofctl -O OpenFlow14 queue-get-config br0 1 2 | STRIP_XIDS],
> [0],
> + [OFPT_ERROR (OF1.4): OFPQOFC_BAD_QUEUE
> +OFPST_QUEUE_DESC request (OF1.4): port=1 queue=2
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index 454f6d5..d2fbf49 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -244,12 +244,12 @@ 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 "\fBqueue\-get\-config \fIswitch [\fIport \fR[\fIqueue\fR]]"
> +Prints to the console the configuration of \fIqueue\fR on \fIport\fR
> +in \fIswitch\fR. If \fIport\fR is omitted or \fBANY\fR, reports
> +queues for all port. If \fIqueue\fR is omitted or \fBANY\fR, reports
> +all queues. For OpenFlow 1.3 and earlier, the output always includes
> +all queues, ignoring \fIqueue\fR if specified.
> .IP
> This command has limited usefulness, because ports often have no
> configured queues and because the OpenFlow protocol provides only very
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 4dbd4ed..1ad48c3 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -1232,12 +1232,14 @@ static void
> ofctl_queue_get_config(struct ovs_cmdl_context *ctx)
> {
> const char *vconn_name = ctx->argv[1];
> - 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);
> -
> + const char *port_name = ctx->argc > 2 ? ctx->argv[2] : "any";
> + ofp_port_t port = str_to_port_no(vconn_name, port_name);
> + const char *queue_name = ctx->argc > 3 ? ctx->argv[3] : "all";
> + uint32_t queue = (!strcasecmp(queue_name, "all")
> + ? OFPQ_ALL
> + : atoi(queue_name));
> 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) {
> @@ -1257,14 +1259,14 @@ ofctl_queue_get_config(struct ovs_cmdl_context *ctx)
> 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));
> + version2, pp.port_no, queue));
> }
> }
> port_iterator_destroy(&pi);
> vconn_close(vconn2);
> } else {
> dump_transaction(vconn, ofputil_encode_queue_get_config_request(
> - version, port));
> + version, port, queue));
> }
> vconn_close(vconn);
> }
> @@ -3883,8 +3885,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]",
> - 1, 2, ofctl_queue_get_config },
> + { "queue-get-config", "switch [port [queue]]",
> + 1, 3, ofctl_queue_get_config },
> { "add-flow", "switch flow",
> 2, 2, ofctl_add_flow },
> { "add-flows", "switch file",
> --
> 2.1.3
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
