Parsing of actions was wrong because OF1.3 says that non-experimenter actions are 4 bytes and don't include padding. This fixes the problem.
Parsing of instructions seems wrong too because OF1.3 says that non-experimenter instructions are 4 bytes (though it is not explicit about padding as it is for actions). This fixes the problem there too. Signed-off-by: Ben Pfaff <b...@nicira.com> --- lib/ofp-util.c | 47 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 1e641bd..1ba3970 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -61,12 +61,17 @@ struct ofp_prop_header { /* Pulls a property, beginning with struct ofp_prop_header, from the beginning * of 'msg'. Stores the type of the property in '*typep' and, if 'property' is * nonnull, the entire property, including the header, in '*property'. Returns - * 0 if successful, otherwise an error code. */ + * 0 if successful, otherwise an error code. + * + * This function pulls the property's stated size padded out to a multiple of + * 'alignment' bytes. The common case in OpenFlow is an 'alignment' of 8, so + * you can use ofputil_pull_property() for that case. */ static enum ofperr -ofputil_pull_property(struct ofpbuf *msg, struct ofpbuf *property, - uint16_t *typep) +ofputil_pull_property__(struct ofpbuf *msg, struct ofpbuf *property, + unsigned int alignment, uint16_t *typep) { struct ofp_prop_header *oph; + unsigned int padded_len; unsigned int len; if (ofpbuf_size(msg) < sizeof *oph) { @@ -75,7 +80,8 @@ ofputil_pull_property(struct ofpbuf *msg, struct ofpbuf *property, oph = ofpbuf_data(msg); len = ntohs(oph->len); - if (len < sizeof *oph || ROUND_UP(len, 8) > ofpbuf_size(msg)) { + padded_len = ROUND_UP(len, alignment); + if (len < sizeof *oph || padded_len > ofpbuf_size(msg)) { return OFPERR_OFPBPC_BAD_LEN; } @@ -83,10 +89,24 @@ ofputil_pull_property(struct ofpbuf *msg, struct ofpbuf *property, if (property) { ofpbuf_use_const(property, ofpbuf_data(msg), len); } - ofpbuf_pull(msg, ROUND_UP(len, 8)); + ofpbuf_pull(msg, padded_len); return 0; } +/* Pulls a property, beginning with struct ofp_prop_header, from the beginning + * of 'msg'. Stores the type of the property in '*typep' and, if 'property' is + * nonnull, the entire property, including the header, in '*property'. Returns + * 0 if successful, otherwise an error code. + * + * This function pulls the property's stated size padded out to a multiple of + * 8 bytes, which is the common case for OpenFlow properties. */ +static enum ofperr +ofputil_pull_property(struct ofpbuf *msg, struct ofpbuf *property, + uint16_t *typep) +{ + return ofputil_pull_property__(msg, property, 8, typep); +} + static void PRINTF_FORMAT(2, 3) log_property(bool loose, const char *message, ...) { @@ -4443,10 +4463,12 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm, return b; } + +/* Table features. */ static enum ofperr pull_table_feature_property(struct ofpbuf *msg, struct ofpbuf *payload, - uint16_t *typep) + uint16_t *typep) { enum ofperr error; @@ -4467,7 +4489,7 @@ parse_action_bitmap(struct ofpbuf *payload, enum ofp_version ofp_version, uint16_t type; enum ofperr error; - error = pull_table_feature_property(payload, NULL, &type); + error = ofputil_pull_property__(payload, NULL, 1, &type); if (error) { return error; } @@ -4489,7 +4511,16 @@ parse_instruction_ids(struct ofpbuf *payload, bool loose, uint32_t *insts) enum ofperr error; uint16_t ofpit; - error = pull_table_feature_property(payload, NULL, &ofpit); + /* OF1.3 and OF1.4 aren't clear about padding in the instruction IDs. + * It seems clear that they aren't padded to 8 bytes, though, because + * both standards say that "non-experimenter instructions are 4 bytes" + * and do not mention any padding before the first instruction ID. + * (There wouldn't be any point in padding to 8 bytes if the IDs were + * aligned on an odd 4-byte boundary.) + * + * Anyway, we just assume they're all glommed together on byte + * boundaries. */ + error = ofputil_pull_property__(payload, NULL, 1, &ofpit); if (error) { return error; } -- 1.7.10.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev