From patchwork Thu Aug 30 20:00:53 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 964168 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 421YMP3vvhz9rvt for ; Fri, 31 Aug 2018 06:03:13 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 5ACCECFD; Thu, 30 Aug 2018 20:01:14 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 06B86CDB for ; Thu, 30 Aug 2018 20:01:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id F310E466 for ; Thu, 30 Aug 2018 20:01:10 +0000 (UTC) X-Originating-IP: 173.228.112.177 Received: from sigabrt.gateway.sonic.net (173-228-112-177.dsl.dynamic.fusionbroadband.com [173.228.112.177]) (Authenticated sender: blp@ovn.org) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id B7BF220006; Thu, 30 Aug 2018 20:01:08 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 30 Aug 2018 13:00:53 -0700 Message-Id: <20180830200056.15484-5-blp@ovn.org> X-Mailer: git-send-email 2.16.1 In-Reply-To: <20180830200056.15484-1-blp@ovn.org> References: <20180830200056.15484-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 5/8] ofp-table: Parse table features messages more carefully. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- include/openflow/openflow-1.3.h | 24 +++++++++++- include/openvswitch/ofp-table.h | 14 ++++++- lib/ofp-table.c | 83 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 3 deletions(-) diff --git a/include/openflow/openflow-1.3.h b/include/openflow/openflow-1.3.h index a521995da339..c48a8ea7f3e9 100644 --- a/include/openflow/openflow-1.3.h +++ b/include/openflow/openflow-1.3.h @@ -215,13 +215,24 @@ struct ofp13_table_stats { }; OFP_ASSERT(sizeof(struct ofp13_table_stats) == 24); +enum ofp15_table_features_command { + OFPTFC15_REPLACE = 0, /* Replace full pipeline. */ + OFPTFC15_MODIFY = 1, /* Modify flow tables capabilities. */ + OFPTFC15_ENABLE = 2, /* Enable flow tables in the pipeline. */ + OFPTFC15_DISABLE = 3, /* Disable flow tables in pipeline. */ +}; + /* Body for ofp_multipart_request of type OFPMP_TABLE_FEATURES./ * Body of reply to OFPMP_TABLE_FEATURES request. */ struct ofp13_table_features { ovs_be16 length; /* Length is padded to 64 bits. */ uint8_t table_id; /* Identifier of table. Lower numbered tables are consulted first. */ - uint8_t pad[5]; /* Align to 64-bits. */ + + /* Added in OF1.5. Earlier versions acted like OFPTFC15_REPLACE. */ + uint8_t command; /* One of OFPTFC15_*. */ + + uint8_t pad[4]; /* Align to 64-bits. */ char name[OFP_MAX_TABLE_NAME_LEN]; ovs_be64 metadata_match; /* Bits of metadata table can match. */ ovs_be64 metadata_write; /* Bits of metadata table can write. */ @@ -260,6 +271,17 @@ enum ofp13_table_feature_prop_type { OFPTFPT13_APPLY_SETFIELD_MISS = 15, /* Apply Set-Field for table-miss. */ OFPTFPT13_EXPERIMENTER = 0xFFFE, /* Experimenter property. */ OFPTFPT13_EXPERIMENTER_MISS = 0xFFFF, /* Experimenter for table-miss. */ + + /* OpenFlow says that each of these properties must occur exactly once. */ +#define OFPTFPT13_REQUIRED ((1u << OFPTFPT13_INSTRUCTIONS) | \ + (1u << OFPTFPT13_NEXT_TABLES) | \ + (1u << OFPTFPT13_WRITE_ACTIONS) | \ + (1u << OFPTFPT13_APPLY_ACTIONS) | \ + (1u << OFPTFPT13_MATCH) | \ + (1u << OFPTFPT13_WILDCARDS) | \ + (1u << OFPTFPT13_WRITE_SETFIELD) | \ + (1u << OFPTFPT13_APPLY_SETFIELD)) + }; /* Body of reply to OFPMP13_PORT request. If a counter is unsupported, set diff --git a/include/openvswitch/ofp-table.h b/include/openvswitch/ofp-table.h index 713ce26d014d..370ec85aec55 100644 --- a/include/openvswitch/ofp-table.h +++ b/include/openvswitch/ofp-table.h @@ -187,13 +187,23 @@ void ofputil_table_desc_format(struct ds *, * include support for a subset of ofp_table_features through OFPST_TABLE (aka * OFPMP_TABLE). */ struct ofputil_table_features { - uint8_t table_id; /* Identifier of table. Lower numbered tables - are consulted first. */ + /* Only for OFPT_TABLE_FEATURES requests and only the first table_features + * in such a request. */ + enum ofp15_table_features_command command; + + /* The following are always present in table features requests and + * replies. */ + uint8_t table_id; char name[OFP_MAX_TABLE_NAME_LEN]; ovs_be64 metadata_match; /* Bits of metadata table can match. */ ovs_be64 metadata_write; /* Bits of metadata table can write. */ uint32_t max_entries; /* Max number of entries supported. */ + /* True if the message included any properties. This is important for + * OFPT_TABLE_FEATURES requests, which change table properties only if any + * are included. */ + bool any_properties; + /* Flags. * * 'miss_config' is relevant for OpenFlow 1.1 and 1.2 only, because those diff --git a/lib/ofp-table.c b/lib/ofp-table.c index 88fc322408ee..afa9b9103b88 100644 --- a/lib/ofp-table.c +++ b/lib/ofp-table.c @@ -74,6 +74,33 @@ ofputil_table_vacancy_to_string(enum ofputil_table_vacancy vacancy) default: return "***error***"; } } + +static bool +ofp15_table_features_command_is_valid(enum ofp15_table_features_command cmd) +{ + switch (cmd) { + case OFPTFC15_REPLACE: + case OFPTFC15_MODIFY: + case OFPTFC15_ENABLE: + case OFPTFC15_DISABLE: + return true; + + default: + return false; + } +} + +static const char * +ofp15_table_features_command_to_string(enum ofp15_table_features_command cmd) +{ + switch (cmd) { + case OFPTFC15_REPLACE: return "replace"; + case OFPTFC15_MODIFY: return "modify"; + case OFPTFC15_ENABLE: return "enable"; + case OFPTFC15_DISABLE: return "disable"; + default: return "***bad command***"; + } +} /* ofputil_table_map. */ @@ -361,6 +388,15 @@ ofputil_decode_table_features(struct ofpbuf *msg, return OFPERR_OFPBPC_BAD_LEN; } + if (oh->version >= OFP15_VERSION) { + if (!ofp15_table_features_command_is_valid(otf->command)) { + return OFPERR_OFPTFFC_BAD_COMMAND; + } + tf->command = otf->command; + } else { + tf->command = OFPTFC15_REPLACE; + } + tf->table_id = otf->table_id; if (tf->table_id == OFPTT_ALL) { return OFPERR_OFPTFFC_BAD_TABLE; @@ -382,7 +418,9 @@ ofputil_decode_table_features(struct ofpbuf *msg, struct ofpbuf properties = ofpbuf_const_initializer(ofpbuf_pull(msg, len), len); + tf->any_properties = properties.size > 0; ofpbuf_pull(&properties, sizeof *otf); + uint32_t seen = 0; while (properties.size > 0) { struct ofpbuf payload; enum ofperr error; @@ -393,6 +431,14 @@ ofputil_decode_table_features(struct ofpbuf *msg, return error; } + if (type < 32) { + uint32_t bit = 1u << type; + if (seen & bit) { + return OFPERR_OFPTFFC_BAD_FEATURES; + } + seen |= bit; + } + switch ((enum ofp13_table_feature_prop_type) type) { case OFPTFPT13_INSTRUCTIONS: error = parse_instruction_ids(&payload, loose, @@ -464,6 +510,36 @@ ofputil_decode_table_features(struct ofpbuf *msg, } } + /* OpenFlow 1.3 and 1.4 always require all of the required properties. + * OpenFlow 1.5 requires all of them if any property is present. */ + if ((seen & OFPTFPT13_REQUIRED) != OFPTFPT13_REQUIRED + && (tf->any_properties || oh->version < OFP15_VERSION)) { + VLOG_WARN_RL(&rl, "table features message missing required property"); + return OFPERR_OFPTFFC_BAD_FEATURES; + } + + /* Copy nonmiss to miss when appropriate. */ + if (tf->any_properties) { + if (!(seen & (1u << OFPTFPT13_INSTRUCTIONS_MISS))) { + tf->miss.instructions = tf->nonmiss.instructions; + } + if (!(seen & (1u << OFPTFPT13_NEXT_TABLES_MISS))) { + memcpy(tf->miss.next, tf->nonmiss.next, sizeof tf->miss.next); + } + if (!(seen & (1u << OFPTFPT13_WRITE_ACTIONS_MISS))) { + tf->miss.write.ofpacts = tf->nonmiss.write.ofpacts; + } + if (!(seen & (1u << OFPTFPT13_APPLY_ACTIONS_MISS))) { + tf->miss.apply.ofpacts = tf->nonmiss.apply.ofpacts; + } + if (!(seen & (1u << OFPTFPT13_WRITE_SETFIELD_MISS))) { + tf->miss.write.set_fields = tf->nonmiss.write.set_fields; + } + if (!(seen & (1u << OFPTFPT13_APPLY_SETFIELD_MISS))) { + tf->miss.apply.set_fields = tf->nonmiss.apply.set_fields; + } + } + /* Fix inconsistencies: * * - Turn on 'match' bits that are set in 'mask', because maskable @@ -577,6 +653,7 @@ ofputil_append_table_features_reply(const struct ofputil_table_features *tf, otf = ofpbuf_put_zeros(reply, sizeof *otf); otf->table_id = tf->table_id; + otf->command = version >= OFP15_VERSION ? tf->command : 0; ovs_strlcpy_arrays(otf->name, tf->name); otf->metadata_match = tf->metadata_match; otf->metadata_write = tf->metadata_write; @@ -1434,6 +1511,12 @@ ofputil_table_features_format( const struct ofputil_table_stats *prev_stats, int *first_ditto, int *last_ditto) { + if (!prev_features && features->command != OFPTFC15_REPLACE) { + ds_put_format(s, "\n command: %s", + ofp15_table_features_command_to_string( + features->command)); + } + int table = features->table_id; int prev_table = prev_features ? prev_features->table_id : 0;