The main effect is to move these functions a little earlier in the file. A secondary effect is to make it possible to use different error codes for different kinds of properties. In particular, table features has different error codes from other properties (yeah, terrible design, tell me about it).
While we're at it, add the error codes that other properties use to ofp-errors.h. Signed-off-by: Ben Pfaff <b...@nicira.com> --- lib/ofp-errors.h | 33 +++++++++++++++++- lib/ofp-util.c | 100 +++++++++++++++++++++++++++++++------------------------ 2 files changed, 89 insertions(+), 44 deletions(-) diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h index b1bcf7cc..2aaf92c 100644 --- a/lib/ofp-errors.h +++ b/lib/ofp-errors.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -557,6 +557,37 @@ enum ofperr { /* OF1.3+(13,5). Permissions error. */ OFPERR_OFPTFFC_EPERM, +/* ## ------------------ ## */ +/* ## OFPET_BAD_PROPERTY ## */ +/* ## ------------------ ## */ + + /* OF1.4+(14,0). Unknown property type. */ + OFPERR_OFPBPC_BAD_TYPE, + + /* OF1.4+(14,1). Length problem in property. */ + OFPERR_OFPBPC_BAD_LEN, + + /* OF1.4+(14,2). Unsupported property value. */ + OFPERR_OFPBPC_BAD_VALUE, + + /* OF1.4+(14,3). Can't handle this many properties. */ + OFPERR_OFPBPC_TOO_MANY, + + /* OF1.4+(14,4). A property type was duplicated. */ + OFPERR_OFPBPC_DUP_TYPE, + + /* OF1.4+(14,5). Unknown experimenter id specified. */ + OFPERR_OFPBPC_BAD_EXPERIMENTER, + + /* OF1.4+(14,6). Unknown exp_type for experimenter id. */ + OFPERR_OFPBPC_BAD_EXP_TYPE, + + /* OF1.4+(14,7). Unknown value for experimenter id. */ + OFPERR_OFPBPC_BAD_EXP_VALUE, + + /* OF1.4+(14,8). Permissions error. */ + OFPERR_OFPBPC_EPERM, + /* ## -------------------- ## */ /* ## OFPET_BUNDLE_FAILED ## */ /* ## -------------------- ## */ diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 2a3c74c..85b2cae 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -50,6 +50,49 @@ VLOG_DEFINE_THIS_MODULE(ofp_util); * in the peer and so there's not much point in showing a lot of them. */ static struct vlog_rate_limit bad_ofmsg_rl = VLOG_RATE_LIMIT_INIT(1, 5); +struct ofp_prop_header { + ovs_be16 type; + ovs_be16 len; +}; + +static enum ofperr +ofputil_pull_property__(struct ofpbuf *msg, enum ofperr bad_len_error, + struct ofpbuf *property, uint16_t *typep) +{ + struct ofp_prop_header *oph; + unsigned int len; + + if (ofpbuf_size(msg) < sizeof *oph) { + return bad_len_error; + } + + oph = ofpbuf_data(msg); + len = ntohs(oph->len); + if (len < sizeof *oph || ROUND_UP(len, 8) > ofpbuf_size(msg)) { + return bad_len_error; + } + + *typep = ntohs(oph->type); + if (property) { + ofpbuf_use_const(property, ofpbuf_data(msg), len); + } + ofpbuf_pull(msg, ROUND_UP(len, 8)); + return 0; +} + + +static void PRINTF_FORMAT(2, 3) +log_property(bool loose, const char *message, ...) +{ + enum vlog_level level = loose ? VLL_DBG : VLL_WARN; + if (!vlog_should_drop(THIS_MODULE, level, &bad_ofmsg_rl)) { + va_list args; + + va_start(args, message); + vlog_valist(THIS_MODULE, level, message, args); + va_end(args); + } +} /* Given the wildcard bit count in the least-significant 6 of 'wcbits', returns * an IP netmask with a 1 in each bit that must match and a 0 in each bit that * is wildcarded. @@ -4204,48 +4247,18 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm, return b; } -struct ofp_prop_header { - ovs_be16 type; - ovs_be16 len; -}; - static enum ofperr -ofputil_pull_property(struct ofpbuf *msg, struct ofpbuf *payload, - uint16_t *typep) +pull_table_feature_property(struct ofpbuf *msg, struct ofpbuf *payload, + uint16_t *typep) { - struct ofp_prop_header *oph; - unsigned int len; - - if (ofpbuf_size(msg) < sizeof *oph) { - return OFPERR_OFPTFFC_BAD_LEN; - } - - oph = ofpbuf_data(msg); - len = ntohs(oph->len); - if (len < sizeof *oph || ROUND_UP(len, 8) > ofpbuf_size(msg)) { - return OFPERR_OFPTFFC_BAD_LEN; - } - - *typep = ntohs(oph->type); - if (payload) { - ofpbuf_use_const(payload, ofpbuf_data(msg), len); - ofpbuf_pull(payload, sizeof *oph); - } - ofpbuf_pull(msg, ROUND_UP(len, 8)); - return 0; -} - -static void PRINTF_FORMAT(2, 3) -log_property(bool loose, const char *message, ...) -{ - enum vlog_level level = loose ? VLL_DBG : VLL_WARN; - if (!vlog_should_drop(THIS_MODULE, level, &bad_ofmsg_rl)) { - va_list args; + enum ofperr error; - va_start(args, message); - vlog_valist(THIS_MODULE, level, message, args); - va_end(args); + error = ofputil_pull_property__(msg, OFPERR_OFPTFFC_BAD_LEN, + payload, typep); + if (!error) { + ofpbuf_pull(payload, sizeof(struct ofp_prop_header)); } + return error; } static enum ofperr @@ -4255,7 +4268,7 @@ parse_table_ids(struct ofpbuf *payload, uint32_t *ids) *ids = 0; while (ofpbuf_size(payload) > 0) { - enum ofperr error = ofputil_pull_property(payload, NULL, &type); + enum ofperr error = pull_table_feature_property(payload, NULL, &type); if (error) { return error; } @@ -4275,7 +4288,7 @@ parse_instruction_ids(struct ofpbuf *payload, bool loose, uint32_t *insts) enum ofperr error; uint16_t ofpit; - error = ofputil_pull_property(payload, NULL, &ofpit); + error = pull_table_feature_property(payload, NULL, &ofpit); if (error) { return error; } @@ -4433,7 +4446,7 @@ ofputil_decode_table_features(struct ofpbuf *msg, enum ofperr error; uint16_t type; - error = ofputil_pull_property(msg, &payload, &type); + error = pull_table_feature_property(msg, &payload, &type); if (error) { return error; } @@ -4496,8 +4509,9 @@ ofputil_decode_table_features(struct ofpbuf *msg, case OFPTFPT13_EXPERIMENTER: case OFPTFPT13_EXPERIMENTER_MISS: - log_property(loose, - "unknown table features experimenter property"); + default: + log_property(loose, "unknown table features property %"PRIu16, + type); error = loose ? 0 : OFPERR_OFPTFFC_BAD_TYPE; break; } -- 1.9.1 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev