openflow: Rename OF0.1-1.3 queue property constants.

At first glance, OF1.4 queue properties look a lot like those for OF1.0
to OF1.3, but in fact their different padding makes them incompatible.  In
addition, OF1.4 switches from using regular OpenFlow messages to request
queue properties, to using multipart messages.  Thus, we really need to
use separate code to deal with OF1.4 queues.

OF1.0, OF1.1, and OF1.2 all have slightly different queue config reply
messages, but only OF1.0 and OF1.2 had tests, so this adds tests.  (There
is no test for OF1.3 because it's the same as OF1.2.)

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>

5 files changed, 79 insertions, 37 deletions

diff --git a/include/openflow/openflow-1.0.h b/include/openflow/openflow-1.0.h
index 54b15f258..db629f77c 100644
--- a/include/openflow/openflow-1.0.h
+++ b/include/openflow/openflow-1.0.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -137,6 +137,41 @@ struct ofp10_packet_queue {
 };
 OFP_ASSERT(sizeof(struct ofp10_packet_queue) == 8);
 
+/* Queue properties for OF1.0 to OF1.3.
+ *
+ * OF1.4+ use the same numbers but rename them and change the property formats
+ * in incompatible ways, so there's not much benefit to sharing the names. */
+enum ofp10_queue_properties {
+    /* Introduced in OF1.0. */
+    OFPQT10_MIN_RATE = 1,          /* Minimum datarate guaranteed. */
+
+    /* Introduced in OF1.1. */
+    OFPQT11_MAX_RATE = 2,          /* Maximum guaranteed rate. */
+    OFPQT11_EXPERIMENTER = 0xffff, /* Experimenter defined property. */
+};
+
+/* Description for a queue in OpenFlow 1.0 to 1.3.
+ *
+ * OF1.4+ also use a TLV format but an incompatible one. */
+struct ofp10_queue_prop_header {
+    ovs_be16 property; /* One of OFPQT*. */
+    ovs_be16 len;      /* Length of property, including this header. */
+    uint8_t pad[4];    /* 64-bit alignemnt. */
+};
+OFP_ASSERT(sizeof(struct ofp10_queue_prop_header) == 8);
+
+/* Min-Rate and Max-Rate queue property description (OFPQT10_MIN and
+ * OFPQT11_MAX).
+ *
+ * OF1.4+ use similar TLVs but they are incompatible due to different padding.
+ */
+struct ofp10_queue_prop_rate {
+    struct ofp10_queue_prop_header prop_header;
+    ovs_be16 rate;        /* In 1/10 of a percent; >1000 -> disabled. */
+    uint8_t pad[6];       /* 64-bit alignment */
+};
+OFP_ASSERT(sizeof(struct ofp10_queue_prop_rate) == 16);
+
 /* Query for port queue configuration. */
 struct ofp10_queue_get_config_request {
     ovs_be16 port;          /* Port to be queried. Should refer
diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h
index 81f9120cc..f152718f5 100644
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -209,29 +209,6 @@ enum ofp_port_features {
     OFPPF_10GB_FD    = 1 << 6,  /* 10 Gb full-duplex rate support. */
 };
 
-enum ofp_queue_properties {
-    OFPQT_MIN_RATE = 1,          /* Minimum datarate guaranteed. */
-    OFPQT_MAX_RATE = 2,          /* Maximum guaranteed rate. */
-    OFPQT_EXPERIMENTER = 0xffff, /* Experimenter defined property. */
-};
-
-/* Common description for a queue. */
-struct ofp_queue_prop_header {
-    ovs_be16 property; /* One of OFPQT_. */
-    ovs_be16 len;      /* Length of property, including this header. */
-    uint8_t pad[4];    /* 64-bit alignemnt. */
-};
-OFP_ASSERT(sizeof(struct ofp_queue_prop_header) == 8);
-
-/* Min-Rate and Max-Rate queue property description (OFPQT_MIN and
- * OFPQT_MAX). */
-struct ofp_queue_prop_rate {
-    struct ofp_queue_prop_header prop_header;
-    ovs_be16 rate;        /* In 1/10 of a percent; >1000 -> disabled. */
-    uint8_t pad[6];       /* 64-bit alignment */
-};
-OFP_ASSERT(sizeof(struct ofp_queue_prop_rate) == 16);
-
 /* Switch features. */
 struct ofp_switch_features {
     ovs_be64 datapath_id;   /* Datapath unique ID.  The lower 48-bits are for
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index effd96a30..8fffcecf8 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2554,11 +2554,11 @@ ofputil_encode_queue_get_config_reply(const struct ofp_header *oh)
 }
 
 static void
-put_queue_rate(struct ofpbuf *reply, enum ofp_queue_properties property,
-               uint16_t rate)
+put_ofp10_queue_rate(struct ofpbuf *reply,
+                     enum ofp10_queue_properties property, uint16_t rate)
 {
     if (rate != UINT16_MAX) {
-        struct ofp_queue_prop_rate *oqpr;
+        struct ofp10_queue_prop_rate *oqpr;
 
         oqpr = ofpbuf_put_zeros(reply, sizeof *oqpr);
         oqpr->prop_header.property = htons(property);
@@ -2593,8 +2593,8 @@ ofputil_append_queue_get_config_reply(struct ofpbuf *reply,
         len_ofs = (char *) &opq12->len - (char *) reply->data;
     }
 
-    put_queue_rate(reply, OFPQT_MIN_RATE, oqc->min_rate);
-    put_queue_rate(reply, OFPQT_MAX_RATE, oqc->max_rate);
+    put_ofp10_queue_rate(reply, OFPQT10_MIN_RATE, oqc->min_rate);
+    put_ofp10_queue_rate(reply, OFPQT11_MAX_RATE, oqc->max_rate);
 
     len = ofpbuf_at(reply, len_ofs, sizeof *len);
     *len = htons(reply->size - start_ofs);
@@ -2628,12 +2628,13 @@ ofputil_decode_queue_get_config_reply(struct ofpbuf *reply, ofp_port_t *port)
 }
 
 static enum ofperr
-parse_queue_rate(const struct ofp_queue_prop_header *hdr, uint16_t *rate)
+parse_ofp10_queue_rate(const struct ofp10_queue_prop_header *hdr,
+                       uint16_t *rate)
 {
-    const struct ofp_queue_prop_rate *oqpr;
+    const struct ofp10_queue_prop_rate *oqpr;
 
     if (hdr->len == htons(sizeof *oqpr)) {
-        oqpr = (const struct ofp_queue_prop_rate *) hdr;
+        oqpr = (const struct ofp10_queue_prop_rate *) hdr;
         *rate = ntohs(oqpr->rate);
         return 0;
     } else {
@@ -2692,7 +2693,7 @@ ofputil_pull_queue_get_config_reply(struct ofpbuf *reply,
     len -= opq_len;
 
     while (len > 0) {
-        const struct ofp_queue_prop_header *hdr;
+        const struct ofp10_queue_prop_header *hdr;
         unsigned int property;
         unsigned int prop_len;
         enum ofperr error = 0;
@@ -2705,12 +2706,12 @@ ofputil_pull_queue_get_config_reply(struct ofpbuf *reply,
 
         property = ntohs(hdr->property);
         switch (property) {
-        case OFPQT_MIN_RATE:
-            error = parse_queue_rate(hdr, &queue->min_rate);
+        case OFPQT10_MIN_RATE:
+            error = parse_ofp10_queue_rate(hdr, &queue->min_rate);
             break;
 
-        case OFPQT_MAX_RATE:
-            error = parse_queue_rate(hdr, &queue->max_rate);
+        case OFPQT11_MAX_RATE:
+            error = parse_ofp10_queue_rate(hdr, &queue->max_rate);
             break;
 
         default:
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 6fae7f0ca..24e602e7f 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -2561,6 +2561,21 @@ queue 17476:
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPT_QUEUE_GET_CONFIG_REPLY - OF1.1])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "02 17 00 40 00 00 00 01 \
+00 00 00 01 00 00 00 00 \
+00 00 55 55 00 28 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 00 \
+"], [0], [dnl
+OFPT_QUEUE_GET_CONFIG_REPLY (OF1.1) (xid=0x1): port=1
+queue 21845: min_rate:50.0% max_rate:75.0%
+queue 17476:
+])
+AT_CLEANUP
+
 AT_SETUP([OFPT_QUEUE_GET_CONFIG_REPLY - OF1.2])
 AT_KEYWORDS([ofp-print])
 AT_CHECK([ovs-ofctl ofp-print "03 17 00 50 00 00 00 01 \
diff --git a/tests/ofproto.at b/tests/ofproto.at
index eceff58f4..f29543ecc 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -252,6 +252,20 @@ OFPT_QUEUE_GET_CONFIG_REQUEST (xid=0x2): port=10
 OVS_VSWITCHD_STOP
 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([STRIP_XIDS stdout], [0], [dnl
+OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=1
+])
+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
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto - queue configuration - (OpenFlow 1.2)])
 OVS_VSWITCHD_START
 ADD_OF_PORTS([br0], [1], [2])