[ovs-dev] [error reporting 1/6] ofp-util: Avoid linear search in OpenFlow message type decoding.
Ethan Jackson
ethan at nicira.com
Thu Oct 27 20:01:35 UTC 2011
> I think this patch isn't necessary right now. I'm dropping it.
Sounds good, I was wondering about it as I reviewed it which is why I
asked you to explain it in the commit message. Dropping it is fine
too though.
Ethan
>
> The second patch will need to be revised, stand by.
>
> On Fri, Oct 21, 2011 at 02:34:50PM -0700, Ethan Jackson wrote:
>> 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 <blp at 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 at openvswitch.org
>> > http://openvswitch.org/mailman/listinfo/dev
>> >
>
More information about the dev
mailing list