From 0445637d8a3ac8c5a5ccf4958b7e8e282f297e98 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Thu, 27 Jun 2013 12:10:42 +0300 Subject: [PATCH] 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 Signed-off-by: Ben Pfaff --- lib/ofp-util.c | 30 ++++++++++++++++-------------- lib/ofp-util.h | 30 +++++++++++++++--------------- tests/ofp-print.at | 8 ++++---- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 6c584157a..028eaeba7 100644 --- 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 57d35c4de..39c81be5a 100644 --- 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 a7fef554f..f8290a16c 100644 --- 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 -- 2.43.0