Looks good to me.
I think it would be useful to explain why this was done in the commit message.
I think the MSG_CASE family of macros could benefit from a comment.
I think lining up the sizeof's in the switch statements is going to be
a bit annoying to maintain, I don't feel strongly about it though.
Ethan
On Thu, Sep 8, 2011 at 12:36, Ben Pfaff <b...@nicira.com> wrote:
> ---
> lib/ofp-util.c | 536 +++++++++++++++++++++------------------------------
> tests/ofp-print.at | 2 +-
> 2 files changed, 222 insertions(+), 316 deletions(-)
>
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 0a020d3..1e95d04 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -282,111 +282,80 @@ struct ofputil_msg_type {
> unsigned int extra_multiple;
> };
>
> -struct ofputil_msg_category {
> - const char *name; /* e.g. "OpenFlow message" */
> - const struct ofputil_msg_type *types;
> - size_t n_types;
> - int missing_error; /* ofp_mkerr() value for missing type. */
> +/* Represents a malformed OpenFlow message. */
> +static const struct ofputil_msg_type ofputil_invalid_type = {
> + OFPUTIL_MSG_INVALID, 0, "OFPUTIL_MSG_INVALID", 0, 0
> };
>
> -static bool
> -ofputil_length_ok(const struct ofputil_msg_category *cat,
> - const struct ofputil_msg_type *type,
> - unsigned int size)
> +#define MSG_CASE__(ENUM, ENUM_SUFFIX, NAME_SUFFIX, MIN_SIZE, EXTRA_MULTIPLE)
> \
> + case ENUM: { \
> + static const struct ofputil_msg_type this_type = { \
> + OFPUTIL_##ENUM##ENUM_SUFFIX, \
> + ENUM, \
> + #ENUM NAME_SUFFIX, \
> + MIN_SIZE, \
> + EXTRA_MULTIPLE \
> + }; \
> + type = &this_type; \
> + break; \
> + }
> +#define MSG_CASE(ENUM, MIN_SIZE, EXTRA_MULTIPLE) \
> + MSG_CASE__(ENUM,,, MIN_SIZE, EXTRA_MULTIPLE)
> +#define REQ_CASE(ENUM, MIN_SIZE, EXTRA_MULTIPLE) \
> + MSG_CASE__(ENUM, _REQUEST, " request", MIN_SIZE, EXTRA_MULTIPLE)
> +#define RPY_CASE(ENUM, MIN_SIZE, EXTRA_MULTIPLE) \
> + MSG_CASE__(ENUM, _REPLY, " reply", MIN_SIZE, EXTRA_MULTIPLE)
> +
> +static int
> +ofputil_check_length(const struct ofputil_msg_type *type, unsigned int size)
> {
> switch (type->extra_multiple) {
> case 0:
> if (size != type->min_size) {
> - VLOG_WARN_RL(&bad_ofmsg_rl, "received %s %s with incorrect "
> + VLOG_WARN_RL(&bad_ofmsg_rl, "received %s with incorrect "
> "length %u (expected length %u)",
> - cat->name, type->name, size, type->min_size);
> - return false;
> + type->name, size, type->min_size);
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
> }
> - return true;
> + return 0;
>
> case 1:
> if (size < type->min_size) {
> - VLOG_WARN_RL(&bad_ofmsg_rl, "received %s %s with incorrect "
> + VLOG_WARN_RL(&bad_ofmsg_rl, "received %s with incorrect "
> "length %u (expected length at least %u bytes)",
> - cat->name, type->name, size, type->min_size);
> - return false;
> + type->name, size, type->min_size);
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
> }
> - return true;
> + return 0;
>
> default:
> if (size < type->min_size
> || (size - type->min_size) % type->extra_multiple) {
> - VLOG_WARN_RL(&bad_ofmsg_rl, "received %s %s with incorrect "
> + VLOG_WARN_RL(&bad_ofmsg_rl, "received %s with incorrect "
> "length %u (must be exactly %u bytes or longer "
> "by an integer multiple of %u bytes)",
> - cat->name, type->name, size,
> + type->name, size,
> type->min_size, type->extra_multiple);
> - return false;
> - }
> - return true;
> - }
> -}
> -
> -static int
> -ofputil_lookup_openflow_message(const struct ofputil_msg_category *cat,
> - uint32_t value, unsigned int size,
> - const struct ofputil_msg_type **typep)
> -{
> - const struct ofputil_msg_type *type;
> -
> - for (type = cat->types; type < &cat->types[cat->n_types]; type++) {
> - if (type->value == value) {
> - if (!ofputil_length_ok(cat, type, size)) {
> - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
> - }
> - *typep = type;
> - return 0;
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
> }
> + return 0;
> }
> -
> - VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu32,
> - cat->name, value);
> - return cat->missing_error;
> }
>
> static int
> ofputil_decode_vendor(const struct ofp_header *oh,
> const struct ofputil_msg_type **typep)
> {
> - static const struct ofputil_msg_type nxt_messages[] = {
> - { OFPUTIL_NXT_ROLE_REQUEST,
> - NXT_ROLE_REQUEST, "NXT_ROLE_REQUEST",
> - sizeof(struct nx_role_request), 0 },
> -
> - { OFPUTIL_NXT_ROLE_REPLY,
> - NXT_ROLE_REPLY, "NXT_ROLE_REPLY",
> - sizeof(struct nx_role_request), 0 },
> -
> - { OFPUTIL_NXT_SET_FLOW_FORMAT,
> - NXT_SET_FLOW_FORMAT, "NXT_SET_FLOW_FORMAT",
> - sizeof(struct nxt_set_flow_format), 0 },
> -
> - { OFPUTIL_NXT_FLOW_MOD,
> - NXT_FLOW_MOD, "NXT_FLOW_MOD",
> - sizeof(struct nx_flow_mod), 8 },
> -
> - { OFPUTIL_NXT_FLOW_REMOVED,
> - NXT_FLOW_REMOVED, "NXT_FLOW_REMOVED",
> - sizeof(struct nx_flow_removed), 8 },
> -
> - { OFPUTIL_NXT_FLOW_MOD_TABLE_ID,
> - NXT_FLOW_MOD_TABLE_ID, "NXT_FLOW_MOD_TABLE_ID",
> - sizeof(struct nxt_flow_mod_table_id), 0 },
> - };
> -
> - static const struct ofputil_msg_category nxt_category = {
> - "Nicira extension message",
> - nxt_messages, ARRAY_SIZE(nxt_messages),
> - OFP_MKERR(OFPET_BAD_REQUEST, OFPBRC_BAD_SUBTYPE)
> - };
> -
> + const char *category = "Nicira extension message";
> const struct ofp_vendor_header *ovh;
> + const struct ofputil_msg_type *type;
> const struct nicira_header *nh;
> + size_t length = ntohs(oh->length);
> +
> + if (length < sizeof(struct ofp_vendor_header)) {
> + VLOG_WARN_RL(&bad_ofmsg_rl, "truncated vendor message");
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
> + }
>
> ovh = (const struct ofp_vendor_header *) oh;
> if (ovh->vendor != htonl(NX_VENDOR_ID)) {
> @@ -395,32 +364,52 @@ ofputil_decode_vendor(const struct ofp_header *oh,
> return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_VENDOR);
> }
>
> - if (ntohs(ovh->header.length) < sizeof(struct nicira_header)) {
> + if (length < sizeof(struct nicira_header)) {
> VLOG_WARN_RL(&bad_ofmsg_rl, "received Nicira vendor message of "
> - "length %u (expected at least %zu)",
> - ntohs(ovh->header.length), sizeof(struct
> nicira_header));
> + "length %zu (expected at least %zu)",
> + length, sizeof(struct nicira_header));
> return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
> }
>
> nh = (const struct nicira_header *) oh;
> - return ofputil_lookup_openflow_message(&nxt_category, ntohl(nh->subtype),
> - ntohs(oh->length), typep);
> + switch (ntohl(nh->subtype)) {
> + MSG_CASE(NXT_ROLE_REQUEST, sizeof(struct nx_role_request), 0);
> + MSG_CASE(NXT_ROLE_REPLY, sizeof(struct nx_role_request), 0);
> + MSG_CASE(NXT_SET_FLOW_FORMAT, sizeof(struct nxt_set_flow_format),
> 0);
> + MSG_CASE(NXT_FLOW_MOD, sizeof(struct nx_flow_mod), 8);
> + MSG_CASE(NXT_FLOW_REMOVED, sizeof(struct nx_flow_removed), 8);
> + MSG_CASE(NXT_FLOW_MOD_TABLE_ID, sizeof(struct nxt_flow_mod_table_id),
> + 0);
> +
> + default:
> + VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu32,
> + category, ntohl(nh->subtype));
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_SUBTYPE);
> + }
> +
> + *typep = type;
> + return 0;
> }
>
> static int
> check_nxstats_msg(const struct ofp_header *oh)
> {
> - const struct ofp_stats_msg *osm = (const struct ofp_stats_msg *) oh;
> - ovs_be32 vendor;
> + const struct ofp_vendor_stats_msg *ovsm;
> + size_t length = ntohs(oh->length);
> +
> + if (length < sizeof(struct ofp_vendor_stats_msg)) {
> + VLOG_WARN_RL(&bad_ofmsg_rl, "truncated vendor stats message");
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
> + }
>
> - memcpy(&vendor, osm + 1, sizeof vendor);
> - if (vendor != htonl(NX_VENDOR_ID)) {
> + ovsm = (const struct ofp_vendor_stats_msg *) oh;
> + if (ovsm->vendor != htonl(NX_VENDOR_ID)) {
> VLOG_WARN_RL(&bad_ofmsg_rl, "received vendor stats message for "
> - "unknown vendor %"PRIx32, ntohl(vendor));
> + "unknown vendor %"PRIx32, ntohl(ovsm->vendor));
> return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_VENDOR);
> }
>
> - if (ntohs(osm->header.length) < sizeof(struct nicira_stats_msg)) {
> + if (length < sizeof(struct nicira_stats_msg)) {
> VLOG_WARN_RL(&bad_ofmsg_rl, "truncated Nicira stats message");
> return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
> }
> @@ -432,22 +421,8 @@ static int
> ofputil_decode_nxst_request(const struct ofp_header *oh,
> const struct ofputil_msg_type **typep)
> {
> - static const struct ofputil_msg_type nxst_requests[] = {
> - { OFPUTIL_NXST_FLOW_REQUEST,
> - NXST_FLOW, "NXST_FLOW request",
> - sizeof(struct nx_flow_stats_request), 8 },
> -
> - { OFPUTIL_NXST_AGGREGATE_REQUEST,
> - NXST_AGGREGATE, "NXST_AGGREGATE request",
> - sizeof(struct nx_aggregate_stats_request), 8 },
> - };
> -
> - static const struct ofputil_msg_category nxst_request_category = {
> - "Nicira extension statistics request",
> - nxst_requests, ARRAY_SIZE(nxst_requests),
> - OFP_MKERR(OFPET_BAD_REQUEST, OFPBRC_BAD_SUBTYPE)
> - };
> -
> + const char *category = "Nicira extension statistics request";
> + const struct ofputil_msg_type *type;
> const struct nicira_stats_msg *nsm;
> int error;
>
> @@ -457,31 +432,26 @@ ofputil_decode_nxst_request(const struct ofp_header *oh,
> }
>
> nsm = (struct nicira_stats_msg *) oh;
> - return ofputil_lookup_openflow_message(&nxst_request_category,
> - ntohl(nsm->subtype),
> - ntohs(oh->length), typep);
> + switch (ntohl(nsm->subtype)) {
> + REQ_CASE(NXST_FLOW, sizeof(struct nx_flow_stats_request), 8);
> + REQ_CASE(NXST_AGGREGATE, sizeof(struct nx_aggregate_stats_request),
> 8);
> +
> + default:
> + VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu32,
> + category, ntohl(nsm->subtype));
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_SUBTYPE);
> + }
> +
> + *typep = type;
> + return 0;
> }
>
> static int
> ofputil_decode_nxst_reply(const struct ofp_header *oh,
> const struct ofputil_msg_type **typep)
> {
> - static const struct ofputil_msg_type nxst_replies[] = {
> - { OFPUTIL_NXST_FLOW_REPLY,
> - NXST_FLOW, "NXST_FLOW reply",
> - sizeof(struct nicira_stats_msg), 8 },
> -
> - { OFPUTIL_NXST_AGGREGATE_REPLY,
> - NXST_AGGREGATE, "NXST_AGGREGATE reply",
> - sizeof(struct nx_aggregate_stats_reply), 0 },
> - };
> -
> - static const struct ofputil_msg_category nxst_reply_category = {
> - "Nicira extension statistics reply",
> - nxst_replies, ARRAY_SIZE(nxst_replies),
> - OFP_MKERR(OFPET_BAD_REQUEST, OFPBRC_BAD_SUBTYPE)
> - };
> -
> + const char *category = "Nicira extension statistics reply";
> + const struct ofputil_msg_type *type;
> const struct nicira_stats_msg *nsm;
> int error;
>
> @@ -491,115 +461,156 @@ ofputil_decode_nxst_reply(const struct ofp_header *oh,
> }
>
> nsm = (struct nicira_stats_msg *) oh;
> - return ofputil_lookup_openflow_message(&nxst_reply_category,
> - ntohl(nsm->subtype),
> - ntohs(oh->length), typep);
> + switch (ntohl(nsm->subtype)) {
> + RPY_CASE(NXST_FLOW, sizeof(struct nicira_stats_msg), 8);
> + RPY_CASE(NXST_AGGREGATE, sizeof(struct nx_aggregate_stats_reply), 0);
> +
> + default:
> + VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu32,
> + category, ntohl(nsm->subtype));
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_SUBTYPE);
> + }
> +
> + *typep = type;
> + return 0;
> }
>
> static int
> -ofputil_decode_ofpst_request(const struct ofp_header *oh,
> - const struct ofputil_msg_type **typep)
> +check_stats_msg(const struct ofp_header *oh)
> {
> - static const struct ofputil_msg_type ofpst_requests[] = {
> - { OFPUTIL_OFPST_DESC_REQUEST,
> - OFPST_DESC, "OFPST_DESC request",
> - sizeof(struct ofp_stats_msg), 0 },
> -
> - { OFPUTIL_OFPST_FLOW_REQUEST,
> - OFPST_FLOW, "OFPST_FLOW request",
> - sizeof(struct ofp_flow_stats_request), 0 },
> + size_t length = ntohs(oh->length);
>
> - { OFPUTIL_OFPST_AGGREGATE_REQUEST,
> - OFPST_AGGREGATE, "OFPST_AGGREGATE request",
> - sizeof(struct ofp_flow_stats_request), 0 },
> -
> - { OFPUTIL_OFPST_TABLE_REQUEST,
> - OFPST_TABLE, "OFPST_TABLE request",
> - sizeof(struct ofp_stats_msg), 0 },
> + if (length < sizeof(struct ofp_stats_msg)) {
> + VLOG_WARN_RL(&bad_ofmsg_rl, "truncated stats message");
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
> + }
>
> - { OFPUTIL_OFPST_PORT_REQUEST,
> - OFPST_PORT, "OFPST_PORT request",
> - sizeof(struct ofp_port_stats_request), 0 },
> + return 0;
> +}
>
> - { OFPUTIL_OFPST_QUEUE_REQUEST,
> - OFPST_QUEUE, "OFPST_QUEUE request",
> - sizeof(struct ofp_queue_stats_request), 0 },
> +static int
> +ofputil_decode_ofpst_request(const struct ofp_header *oh,
> + const struct ofputil_msg_type **typep)
> +{
> + const char *category = "OpenFlow statistics";
> + const struct ofp_stats_msg *request = (const struct ofp_stats_msg *) oh;
> + const struct ofputil_msg_type *type;
> + int error;
>
> - { 0,
> - OFPST_VENDOR, "OFPST_VENDOR request",
> - sizeof(struct ofp_vendor_stats_msg), 1 },
> - };
> + error = check_stats_msg(oh);
> + if (error) {
> + return error;
> + }
>
> - static const struct ofputil_msg_category ofpst_request_category = {
> - "OpenFlow statistics",
> - ofpst_requests, ARRAY_SIZE(ofpst_requests),
> - OFP_MKERR(OFPET_BAD_REQUEST, OFPBRC_BAD_STAT)
> - };
> + switch (ntohs(request->type)) {
> + REQ_CASE(OFPST_DESC, sizeof(struct ofp_stats_msg), 0);
> + REQ_CASE(OFPST_FLOW, sizeof(struct ofp_flow_stats_request), 0);
> + REQ_CASE(OFPST_AGGREGATE, sizeof(struct ofp_flow_stats_request), 0);
> + REQ_CASE(OFPST_TABLE, sizeof(struct ofp_stats_msg), 0);
> + REQ_CASE(OFPST_PORT, sizeof(struct ofp_port_stats_request), 0);
> + REQ_CASE(OFPST_QUEUE, sizeof(struct ofp_queue_stats_request), 0);
>
> - const struct ofp_stats_msg *request = (const struct ofp_stats_msg *) oh;
> - int error;
> + case OFPST_VENDOR:
> + return ofputil_decode_nxst_request(oh, typep);
>
> - error = ofputil_lookup_openflow_message(&ofpst_request_category,
> - ntohs(request->type),
> - ntohs(oh->length), typep);
> - if (!error && request->type == htons(OFPST_VENDOR)) {
> - error = ofputil_decode_nxst_request(oh, typep);
> + default:
> + VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu16,
> + category, ntohs(request->type));
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_STAT);
> }
> - return error;
> +
> + *typep = type;
> + return 0;
> }
>
> static int
> ofputil_decode_ofpst_reply(const struct ofp_header *oh,
> const struct ofputil_msg_type **typep)
> {
> - static const struct ofputil_msg_type ofpst_replies[] = {
> - { OFPUTIL_OFPST_DESC_REPLY,
> - OFPST_DESC, "OFPST_DESC reply",
> - sizeof(struct ofp_desc_stats), 0 },
> -
> - { OFPUTIL_OFPST_FLOW_REPLY,
> - OFPST_FLOW, "OFPST_FLOW reply",
> - sizeof(struct ofp_stats_msg), 1 },
> -
> - { OFPUTIL_OFPST_AGGREGATE_REPLY,
> - OFPST_AGGREGATE, "OFPST_AGGREGATE reply",
> - sizeof(struct ofp_aggregate_stats_reply), 0 },
> + const char *category = "OpenFlow statistics";
> + const struct ofp_stats_msg *reply = (const struct ofp_stats_msg *) oh;
> + const struct ofputil_msg_type *type;
> + int error;
>
> - { OFPUTIL_OFPST_TABLE_REPLY,
> - OFPST_TABLE, "OFPST_TABLE reply",
> - sizeof(struct ofp_stats_msg), sizeof(struct ofp_table_stats) },
> + error = check_stats_msg(oh);
> + if (error) {
> + return error;
> + }
>
> - { OFPUTIL_OFPST_PORT_REPLY,
> - OFPST_PORT, "OFPST_PORT reply",
> - sizeof(struct ofp_stats_msg), sizeof(struct ofp_port_stats) },
> + switch (ntohs(reply->type)) {
> + RPY_CASE(OFPST_DESC, sizeof(struct ofp_desc_stats), 0);
> + RPY_CASE(OFPST_FLOW, sizeof(struct ofp_stats_msg), 1);
> + RPY_CASE(OFPST_AGGREGATE, sizeof(struct ofp_aggregate_stats_reply),
> 0);
> + RPY_CASE(OFPST_TABLE, sizeof(struct ofp_stats_msg),
> + sizeof(struct ofp_table_stats));
> + RPY_CASE(OFPST_PORT, sizeof(struct ofp_stats_msg),
> + sizeof(struct ofp_port_stats));
> + RPY_CASE(OFPST_QUEUE, sizeof(struct ofp_stats_msg),
> + sizeof(struct ofp_queue_stats));
>
> - { OFPUTIL_OFPST_QUEUE_REPLY,
> - OFPST_QUEUE, "OFPST_QUEUE reply",
> - sizeof(struct ofp_stats_msg), sizeof(struct ofp_queue_stats) },
> + case OFPST_VENDOR:
> + return ofputil_decode_nxst_reply(oh, typep);
>
> - { 0,
> - OFPST_VENDOR, "OFPST_VENDOR reply",
> - sizeof(struct ofp_vendor_stats_msg), 1 },
> - };
> + default:
> + VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu16,
> + category, ntohs(reply->type));
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_STAT);
> + }
>
> - static const struct ofputil_msg_category ofpst_reply_category = {
> - "OpenFlow statistics",
> - ofpst_replies, ARRAY_SIZE(ofpst_replies),
> - OFP_MKERR(OFPET_BAD_REQUEST, OFPBRC_BAD_STAT)
> - };
> + *typep = type;
> + return 0;
> +}
>
> - const struct ofp_stats_msg *reply = (const struct ofp_stats_msg *) oh;
> +static int
> +ofputil_decode_msg_type__(const struct ofp_header *oh,
> + const struct ofputil_msg_type **typep)
> +{
> + const char *category = "OpenFlow message";
> + const struct ofputil_msg_type *type;
> int error;
>
> - error = ofputil_lookup_openflow_message(&ofpst_reply_category,
> - ntohs(reply->type),
> - ntohs(oh->length), typep);
> - if (!error && reply->type == htons(OFPST_VENDOR)) {
> - error = ofputil_decode_nxst_reply(oh, typep);
> + error = 0;
> + switch (oh->type) {
> + MSG_CASE(OFPT_HELLO, sizeof(struct ofp_hello), 1);
> + MSG_CASE(OFPT_ERROR, sizeof(struct ofp_error_msg), 1);
> + MSG_CASE(OFPT_ECHO_REQUEST, sizeof(struct ofp_header), 1);
> + MSG_CASE(OFPT_ECHO_REPLY, sizeof(struct ofp_header), 1);
> + MSG_CASE(OFPT_FEATURES_REQUEST, sizeof(struct ofp_header), 0);
> + MSG_CASE(OFPT_FEATURES_REPLY, sizeof(struct ofp_switch_features),
> + sizeof(struct ofp_phy_port));
> + MSG_CASE(OFPT_GET_CONFIG_REQUEST, sizeof(struct ofp_header), 0);
> + MSG_CASE(OFPT_GET_CONFIG_REPLY, sizeof(struct ofp_switch_config),
> 0);
> + MSG_CASE(OFPT_SET_CONFIG, sizeof(struct ofp_switch_config),
> 0);
> + MSG_CASE(OFPT_PACKET_IN, offsetof(struct ofp_packet_in,
> data),
> + 1);
> + MSG_CASE(OFPT_FLOW_REMOVED, sizeof(struct ofp_flow_removed),
> 0);
> + MSG_CASE(OFPT_PORT_STATUS, sizeof(struct ofp_port_status), 0);
> + MSG_CASE(OFPT_PACKET_OUT, sizeof(struct ofp_packet_out), 1);
> + MSG_CASE(OFPT_FLOW_MOD, sizeof(struct ofp_flow_mod), 1);
> + MSG_CASE(OFPT_PORT_MOD, sizeof(struct ofp_port_mod), 0);
> + MSG_CASE(OFPT_BARRIER_REQUEST, sizeof(struct ofp_header), 0);
> + MSG_CASE(OFPT_BARRIER_REPLY, sizeof(struct ofp_header), 0);
> +
> + case OFPT_STATS_REQUEST:
> + return ofputil_decode_ofpst_request(oh, typep);
> +
> + case OFPT_STATS_REPLY:
> + return ofputil_decode_ofpst_reply(oh, typep);
> +
> + case OFPT_VENDOR:
> + return ofputil_decode_vendor(oh, typep);
> +
> + default:
> + VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu8,
> + category, oh->type);
> + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_TYPE);
> }
> - return error;
> +
> + *typep = type;
> + return 0;
> }
>
> +
> /* Decodes the message type represented by 'oh'. Returns 0 if successful or
> * an OpenFlow error code constructed with ofp_mkerr() on failure. Either
> * way, stores in '*typep' a type structure that can be inspected with the
> @@ -614,114 +625,9 @@ int
> ofputil_decode_msg_type(const struct ofp_header *oh,
> const struct ofputil_msg_type **typep)
> {
> - static const struct ofputil_msg_type ofpt_messages[] = {
> - { OFPUTIL_OFPT_HELLO,
> - OFPT_HELLO, "OFPT_HELLO",
> - sizeof(struct ofp_hello), 1 },
> -
> - { OFPUTIL_OFPT_ERROR,
> - OFPT_ERROR, "OFPT_ERROR",
> - sizeof(struct ofp_error_msg), 1 },
> -
> - { OFPUTIL_OFPT_ECHO_REQUEST,
> - OFPT_ECHO_REQUEST, "OFPT_ECHO_REQUEST",
> - sizeof(struct ofp_header), 1 },
> -
> - { OFPUTIL_OFPT_ECHO_REPLY,
> - OFPT_ECHO_REPLY, "OFPT_ECHO_REPLY",
> - sizeof(struct ofp_header), 1 },
> -
> - { OFPUTIL_OFPT_FEATURES_REQUEST,
> - OFPT_FEATURES_REQUEST, "OFPT_FEATURES_REQUEST",
> - sizeof(struct ofp_header), 0 },
> -
> - { OFPUTIL_OFPT_FEATURES_REPLY,
> - OFPT_FEATURES_REPLY, "OFPT_FEATURES_REPLY",
> - sizeof(struct ofp_switch_features), sizeof(struct ofp_phy_port) },
> -
> - { OFPUTIL_OFPT_GET_CONFIG_REQUEST,
> - OFPT_GET_CONFIG_REQUEST, "OFPT_GET_CONFIG_REQUEST",
> - sizeof(struct ofp_header), 0 },
> -
> - { OFPUTIL_OFPT_GET_CONFIG_REPLY,
> - OFPT_GET_CONFIG_REPLY, "OFPT_GET_CONFIG_REPLY",
> - sizeof(struct ofp_switch_config), 0 },
> -
> - { OFPUTIL_OFPT_SET_CONFIG,
> - OFPT_SET_CONFIG, "OFPT_SET_CONFIG",
> - sizeof(struct ofp_switch_config), 0 },
> -
> - { OFPUTIL_OFPT_PACKET_IN,
> - OFPT_PACKET_IN, "OFPT_PACKET_IN",
> - offsetof(struct ofp_packet_in, data), 1 },
> -
> - { OFPUTIL_OFPT_FLOW_REMOVED,
> - OFPT_FLOW_REMOVED, "OFPT_FLOW_REMOVED",
> - sizeof(struct ofp_flow_removed), 0 },
> -
> - { OFPUTIL_OFPT_PORT_STATUS,
> - OFPT_PORT_STATUS, "OFPT_PORT_STATUS",
> - sizeof(struct ofp_port_status), 0 },
> -
> - { OFPUTIL_OFPT_PACKET_OUT,
> - OFPT_PACKET_OUT, "OFPT_PACKET_OUT",
> - sizeof(struct ofp_packet_out), 1 },
> -
> - { OFPUTIL_OFPT_FLOW_MOD,
> - OFPT_FLOW_MOD, "OFPT_FLOW_MOD",
> - sizeof(struct ofp_flow_mod), 1 },
> -
> - { OFPUTIL_OFPT_PORT_MOD,
> - OFPT_PORT_MOD, "OFPT_PORT_MOD",
> - sizeof(struct ofp_port_mod), 0 },
> -
> - { 0,
> - OFPT_STATS_REQUEST, "OFPT_STATS_REQUEST",
> - sizeof(struct ofp_stats_msg), 1 },
> -
> - { 0,
> - OFPT_STATS_REPLY, "OFPT_STATS_REPLY",
> - sizeof(struct ofp_stats_msg), 1 },
> -
> - { OFPUTIL_OFPT_BARRIER_REQUEST,
> - OFPT_BARRIER_REQUEST, "OFPT_BARRIER_REQUEST",
> - sizeof(struct ofp_header), 0 },
> -
> - { OFPUTIL_OFPT_BARRIER_REPLY,
> - OFPT_BARRIER_REPLY, "OFPT_BARRIER_REPLY",
> - sizeof(struct ofp_header), 0 },
> -
> - { 0,
> - OFPT_VENDOR, "OFPT_VENDOR",
> - sizeof(struct ofp_vendor_header), 1 },
> - };
> -
> - static const struct ofputil_msg_category ofpt_category = {
> - "OpenFlow message",
> - ofpt_messages, ARRAY_SIZE(ofpt_messages),
> - OFP_MKERR(OFPET_BAD_REQUEST, OFPBRC_BAD_TYPE)
> - };
> -
> - int error;
> -
> - error = ofputil_lookup_openflow_message(&ofpt_category, oh->type,
> - ntohs(oh->length), typep);
> + int error = ofputil_decode_msg_type__(oh, typep);
> if (!error) {
> - switch (oh->type) {
> - case OFPT_VENDOR:
> - error = ofputil_decode_vendor(oh, typep);
> - break;
> -
> - case OFPT_STATS_REQUEST:
> - error = ofputil_decode_ofpst_request(oh, typep);
> - break;
> -
> - case OFPT_STATS_REPLY:
> - error = ofputil_decode_ofpst_reply(oh, typep);
> -
> - default:
> - break;
> - }
> + error = ofputil_check_length(*typep, ntohs(oh->length));
> }
> if (error) {
> static const struct ofputil_msg_type ofputil_invalid_type = {
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index c2018c7..d293348 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -197,7 +197,7 @@ ff fe 50 54 00 00 00 01 62 72 30 00 00 00 00 00 \
> 000000d0 00 00 02 08 00 00 02 8f-00 00 02 8f |............ |
> ], [stderr])
> AT_CHECK([sed 's/.*|//' stderr], [0], [dnl
> -received OpenFlow message OFPT_FEATURES_REPLY with incorrect length 220
> (must be exactly 32 bytes or longer by an integer multiple of 48 bytes)
> +received OFPT_FEATURES_REPLY with incorrect length 220 (must be exactly 32
> bytes or longer by an integer multiple of 48 bytes)
> ])
> AT_CLEANUP
>
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
