ofp-util: Meter fixes.

Validate claimed message length for meter stats in ofp-util.c.
Clean up meters in ofp-util.h.
Fix the impossible duration values in ofp-print.at.

Signed-off-by: Jarno Rajahalme <jarno.rajahalme@nsn.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6c58415..028eaeb 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1646,7 +1646,7 @@ ofputil_pull_bands(struct ofpbuf *msg, size_t len, uint16_t *n_bands,
 
     while (len >= sizeof (struct ofp13_meter_band_drop)) {
         size_t ombh_len = ntohs(ombh->len);
-        /* All supported band types have the same length */
+        /* All supported band types have the same length. */
         if (ombh_len != sizeof (struct ofp13_meter_band_drop)) {
             return OFPERR_OFPBRC_BAD_LEN;
         }
@@ -1693,8 +1693,7 @@ ofputil_decode_meter_mod(const struct ofp_header *oh,
         mm->meter.flags = ntohs(omm->flags);
         mm->meter.bands = bands->data;
 
-        error = ofputil_pull_bands(&b, b.size, &mm->meter.n_bands,
-                                   bands);
+        error = ofputil_pull_bands(&b, b.size, &mm->meter.n_bands, bands);
         if (error) {
             return error;
         }
@@ -1748,7 +1747,7 @@ ofputil_put_bands(uint16_t n_bands, const struct ofputil_meter_band *mb,
     uint16_t n = 0;
 
     for (n = 0; n < n_bands; ++n) {
-        /* Currently all band types have same size */
+        /* Currently all band types have same size. */
         struct ofp13_meter_band_dscp_remark *ombh;
         size_t ombh_len = sizeof *ombh;
 
@@ -1842,8 +1841,9 @@ ofputil_decode_meter_config(struct ofpbuf *msg,
 
     omc = ofpbuf_try_pull(msg, sizeof *omc);
     if (!omc) {
-        VLOG_WARN_RL(&bad_ofmsg_rl, "OFPMP_METER_CONFIG reply has %zu "
-                     "leftover bytes at end", msg->size);
+        VLOG_WARN_RL(&bad_ofmsg_rl,
+                     "OFPMP_METER_CONFIG reply has %zu leftover bytes at end",
+                     msg->size);
         return OFPERR_OFPBRC_BAD_LEN;
     }
 
@@ -1868,13 +1868,16 @@ ofputil_pull_band_stats(struct ofpbuf *msg, size_t len, uint16_t *n_bands,
     struct ofputil_meter_band_stats *mbs;
     uint16_t n, i;
 
+    ombs = ofpbuf_try_pull(msg, len);
+    if (!ombs) {
+        return OFPERR_OFPBRC_BAD_LEN;
+    }
+
     n = len / sizeof *ombs;
     if (len != n * sizeof *ombs) {
         return OFPERR_OFPBRC_BAD_LEN;
     }
 
-    ombs = ofpbuf_pull(msg, len);
-
     mbs = ofpbuf_put_uninit(bands, len);
 
     for (i = 0; i < n; ++i) {
@@ -1901,7 +1904,6 @@ ofputil_decode_meter_stats(struct ofpbuf *msg,
                            struct ofpbuf *bands)
 {
     const struct ofp13_meter_stats *oms;
-    uint16_t len;
     enum ofperr err;
 
     /* Pull OpenFlow headers for the first call. */
@@ -1915,15 +1917,15 @@ ofputil_decode_meter_stats(struct ofpbuf *msg,
 
     oms = ofpbuf_try_pull(msg, sizeof *oms);
     if (!oms) {
-        VLOG_WARN_RL(&bad_ofmsg_rl, "OFPMP_METER reply has %zu leftover "
-                     "bytes at end", msg->size);
+        VLOG_WARN_RL(&bad_ofmsg_rl,
+                     "OFPMP_METER reply has %zu leftover bytes at end",
+                     msg->size);
         return OFPERR_OFPBRC_BAD_LEN;
     }
-    len = ntohs(oms->len);
-    len -= sizeof *oms;
 
     ofpbuf_clear(bands);
-    err = ofputil_pull_band_stats(msg, len, &ms->n_bands, bands);
+    err = ofputil_pull_band_stats(msg, ntohs(oms->len) - sizeof *oms,
+                                  &ms->n_bands, bands);
     if (err) {
         return err;
     }

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 57d35c4..39c81be 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -514,10 +514,10 @@ enum ofperr ofputil_decode_port_mod(const struct ofp_header *,
 struct ofpbuf *ofputil_encode_port_mod(const struct ofputil_port_mod *,
                                        enum ofputil_protocol);
 
+/* Meter band configuration for all supported band types. */
 struct ofputil_meter_band {
     uint16_t type;
-    uint8_t pad;
-    uint8_t prec_level;   /* Non-zero if type == OFPMBT_DSCP_REMARK */
+    uint8_t prec_level;         /* Non-zero if type == OFPMBT_DSCP_REMARK. */
     uint32_t rate;
     uint32_t burst_size;
 };
@@ -548,21 +548,21 @@ struct ofputil_meter_stats {
     uint32_t duration_sec;
     uint32_t duration_nsec;
     uint16_t n_bands;
-    struct ofputil_meter_band_stats *bands; /* band stats */
+    struct ofputil_meter_band_stats *bands;
 };
 
 struct ofputil_meter_features {
-    uint32_t max_meters; /* Maximum number of meters */
-    uint32_t band_types; /* Can support max 32 band types */
-    uint32_t capabilities; /* Supported flags */
+    uint32_t max_meters;        /* Maximum number of meters. */
+    uint32_t band_types;        /* Can support max 32 band types. */
+    uint32_t capabilities;      /* Supported flags. */
     uint8_t  max_bands;
     uint8_t  max_color;
 };
 
 enum ofperr ofputil_decode_meter_mod(const struct ofp_header *,
                                      struct ofputil_meter_mod *,
-                                    struct ofpbuf *bands);
-struct ofpbuf *ofputil_encode_meter_mod(enum ofp_version ofp_version,
+                                     struct ofpbuf *bands);
+struct ofpbuf *ofputil_encode_meter_mod(enum ofp_version,
                                         const struct ofputil_meter_mod *);
 
 void ofputil_decode_meter_features(const struct ofp_header *,
@@ -575,10 +575,10 @@ void ofputil_decode_meter_request(const struct ofp_header *,
                                   uint32_t *meter_id);
 
 void ofputil_append_meter_config(struct list *replies,
-                                 const struct ofputil_meter_config *omc);
+                                 const struct ofputil_meter_config *);
 
 void ofputil_append_meter_stats(struct list *replies,
-                                const struct ofputil_meter_stats *oms);
+                                const struct ofputil_meter_stats *);
 
 enum ofputil_meter_request_type {
     OFPUTIL_METER_FEATURES,
@@ -590,15 +590,15 @@ struct ofpbuf *ofputil_encode_meter_request(enum ofp_version,
                                             enum ofputil_meter_request_type,
                                             uint32_t meter_id);
 
-int ofputil_decode_meter_stats(struct ofpbuf *msg,
-                               struct ofputil_meter_stats *ms,
+int ofputil_decode_meter_stats(struct ofpbuf *,
+                               struct ofputil_meter_stats *,
                                struct ofpbuf *bands);
 
-int ofputil_decode_meter_config(struct ofpbuf *msg,
-                                struct ofputil_meter_config *mc,
+int ofputil_decode_meter_config(struct ofpbuf *,
+                                struct ofputil_meter_config *,
                                 struct ofpbuf *bands);
 
-/* Type for meter_id in ofproto provider interface, UINT32_MAX if none */
+/* Type for meter_id in ofproto provider interface, UINT32_MAX if invalid. */
 typedef struct { uint32_t uint32; } ofproto_meter_id;
 
 /* Abstract ofp_role_request and reply. */

diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index a7fef55..f8290a1 100644 (file)
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -1652,20 +1652,20 @@ AT_CHECK([ovs-ofctl ofp-print "\
 04 13 00 90 00 00 00 02 00 09 00 00 00 00 00 00 \
 00 00 00 01 00 48 00 00 00 00 00 00 00 00 00 05 \
 00 00 00 00 00 00 10 00 00 00 00 00 00 02 30 00 \
-00 00 01 8a 9a 6e 23 44 \
+00 00 01 8a 0a 6e 23 44 \
 00 00 00 00 00 00 00 7e 00 00 00 00 00 00 34 33 \
 00 00 00 00 00 00 00 e7 00 00 00 00 00 00 94 2e \
 00 00 00 02 00 38 00 00 00 00 00 00 00 00 00 02 \
 00 00 00 00 00 00 02 00 00 00 00 00 00 00 30 00 \
-00 00 01 87 9a 23 6e 44 \
+00 00 01 87 0a 23 6e 44 \
 00 00 00 00 00 00 00 2a 00 00 00 00 00 00 04 33 \
 "], [0], [dnl
 OFPST_METER reply (OF1.3) (xid=0x2):
-meter:1 flow_count:5 packet_in_count:4096 byte_in_count:143360 duration:394.2590909252s bands:
+meter:1 flow_count:5 packet_in_count:4096 byte_in_count:143360 duration:394.174990148s bands:
 0: packet_count:126 byte_count:13363
 1: packet_count:231 byte_count:37934
 
-meter:2 flow_count:2 packet_in_count:512 byte_in_count:12288 duration:391.2586013252s bands:
+meter:2 flow_count:2 packet_in_count:512 byte_in_count:12288 duration:391.170094148s bands:
 0: packet_count:42 byte_count:1075
 ])
 AT_CLEANUP