From 8e97815ea7b8a45ab8c0c2d90ee163f813b0b4b5 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 11 Aug 2014 11:07:53 -0700 Subject: [PATCH] ofp-actions: Add instructions bitmaps and fix related bug. This will allow, later, to centralize all of the knowledge of instruction encoding inside ofp-actions. OFPIT11_ALL and OFPIT13_ALL are no longer used, so this commit removes them. Their definitions were wrong (they did not shift each bit into position correctly), so this commit is also a small bug fix. Signed-off-by: Ben Pfaff Acked-by: Jarno Rajahalme --- include/openflow/openflow-1.1.h | 4 -- include/openflow/openflow-1.3.h | 4 -- lib/ofp-actions.c | 69 +++++++++++++++++++++++++++++++++ lib/ofp-actions.h | 4 ++ lib/ofp-util.c | 6 ++- lib/ofp-util.h | 2 +- ofproto/ofproto-provider.h | 6 +-- ofproto/ofproto.c | 2 +- tests/ofproto.at | 2 +- 9 files changed, 83 insertions(+), 16 deletions(-) diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h index bb6bcb0b3..5e618c9ef 100644 --- a/include/openflow/openflow-1.1.h +++ b/include/openflow/openflow-1.1.h @@ -290,10 +290,6 @@ enum ofp11_instruction_type { OFPIT11_EXPERIMENTER = 0xFFFF /* Experimenter instruction */ }; -#define OFPIT11_ALL (OFPIT11_GOTO_TABLE | OFPIT11_WRITE_METADATA | \ - OFPIT11_WRITE_ACTIONS | OFPIT11_APPLY_ACTIONS | \ - OFPIT11_CLEAR_ACTIONS) - #define OFP11_INSTRUCTION_ALIGN 8 /* Generic ofp_instruction structure. */ diff --git a/include/openflow/openflow-1.3.h b/include/openflow/openflow-1.3.h index cc425f1d1..39de5b3bd 100644 --- a/include/openflow/openflow-1.3.h +++ b/include/openflow/openflow-1.3.h @@ -91,10 +91,6 @@ enum ofp13_instruction_type { OFPIT13_METER = 6 /* Apply meter (rate limiter) */ }; -#define OFPIT13_ALL (OFPIT11_GOTO_TABLE | OFPIT11_WRITE_METADATA | \ - OFPIT11_WRITE_ACTIONS | OFPIT11_APPLY_ACTIONS | \ - OFPIT11_CLEAR_ACTIONS | OFPIT13_METER) - /* Instruction structure for OFPIT_METER */ struct ofp13_instruction_meter { ovs_be16 type; /* OFPIT13_METER */ diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index d6436c28d..3eaf49c73 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -1646,6 +1646,75 @@ OVS_INSTRUCTIONS } } +/* Two-way translation between OVS's internal "OVSINST_*" representation of + * instructions and the "OFPIT_*" representation used in OpenFlow. */ +struct ovsinst_map { + enum ovs_instruction_type ovsinst; /* Internal name for instruction. */ + int ofpit; /* OFPIT_* number from OpenFlow spec. */ +}; + +static const struct ovsinst_map * +get_ovsinst_map(enum ofp_version version) +{ + /* OpenFlow 1.1 and 1.2 instructions. */ + static const struct ovsinst_map of11[] = { + { OVSINST_OFPIT11_GOTO_TABLE, 1 }, + { OVSINST_OFPIT11_WRITE_METADATA, 2 }, + { OVSINST_OFPIT11_WRITE_ACTIONS, 3 }, + { OVSINST_OFPIT11_APPLY_ACTIONS, 4 }, + { OVSINST_OFPIT11_CLEAR_ACTIONS, 5 }, + { 0, -1 }, + }; + + /* OpenFlow 1.3+ instructions. */ + static const struct ovsinst_map of13[] = { + { OVSINST_OFPIT11_GOTO_TABLE, 1 }, + { OVSINST_OFPIT11_WRITE_METADATA, 2 }, + { OVSINST_OFPIT11_WRITE_ACTIONS, 3 }, + { OVSINST_OFPIT11_APPLY_ACTIONS, 4 }, + { OVSINST_OFPIT11_CLEAR_ACTIONS, 5 }, + { OVSINST_OFPIT13_METER, 6 }, + { 0, -1 }, + }; + + return version < OFP13_VERSION ? of11 : of13; +} + +/* Converts 'ovsinst_bitmap', a bitmap whose bits correspond to OVSINST_* + * values, into a bitmap of instructions suitable for OpenFlow 'version' + * (OFP11_VERSION or later), and returns the result. */ +ovs_be32 +ovsinst_bitmap_to_openflow(uint32_t ovsinst_bitmap, enum ofp_version version) +{ + uint32_t ofpit_bitmap = 0; + const struct ovsinst_map *x; + + for (x = get_ovsinst_map(version); x->ofpit >= 0; x++) { + if (ovsinst_bitmap & (1u << x->ovsinst)) { + ofpit_bitmap |= 1u << x->ofpit; + } + } + return htonl(ofpit_bitmap); +} + +/* Converts 'ofpit_bitmap', a bitmap of instructions from an OpenFlow message + * with the given 'version' (OFP11_VERSION or later) into a bitmap whose bits + * correspond to OVSINST_* values, and returns the result. */ +uint32_t +ovsinst_bitmap_from_openflow(ovs_be32 ofpit_bitmap_, enum ofp_version version) +{ + uint32_t ofpit_bitmap = ntohl(ofpit_bitmap_); + uint32_t ovsinst_bitmap = 0; + const struct ovsinst_map *x; + + for (x = get_ovsinst_map(version); x->ofpit >= 0; x++) { + if (ofpit_bitmap & (1u << x->ofpit)) { + ovsinst_bitmap |= 1u << x->ovsinst; + } + } + return ovsinst_bitmap; +} + static inline struct ofp11_instruction * instruction_next(const struct ofp11_instruction *inst) { diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h index ce61f9db4..ff7f3b759 100644 --- a/lib/ofp-actions.h +++ b/lib/ofp-actions.h @@ -761,4 +761,8 @@ enum ovs_instruction_type ovs_instruction_type_from_ofpact_type( enum ofperr ovs_instruction_type_from_inst_type( enum ovs_instruction_type *instruction_type, const uint16_t inst_type); +ovs_be32 ovsinst_bitmap_to_openflow(uint32_t ovsinst_bitmap, enum ofp_version); +uint32_t ovsinst_bitmap_from_openflow(ovs_be32 ofpit_bitmap, + enum ofp_version); + #endif /* ofp-actions.h */ diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 9a8a868c6..86caafe34 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -5083,7 +5083,8 @@ ofputil_put_ofp11_table_stats(const struct ofputil_table_stats *in, ovs_strlcpy(out->name, in->name, sizeof out->name); out->wildcards = fields_to_ofp11_flow_match_fields(&in->wildcards); out->match = fields_to_ofp11_flow_match_fields(&in->match); - out->instructions = htonl(in->instructions); + out->instructions = ovsinst_bitmap_to_openflow(in->ovsinsts, + OFP11_VERSION); out->write_actions = ofpact_bitmap_to_openflow(in->write_ofpacts, OFP11_VERSION); out->apply_actions = ofpact_bitmap_to_openflow(in->apply_ofpacts, @@ -5135,7 +5136,8 @@ ofputil_put_ofp12_table_stats(const struct ofputil_table_stats *in, OFP12_VERSION); out->metadata_match = in->metadata_match; out->metadata_write = in->metadata_write; - out->instructions = htonl(in->instructions & OFPIT11_ALL); + out->instructions = ovsinst_bitmap_to_openflow(in->ovsinsts, + OFP12_VERSION); out->config = htonl(in->config); out->max_entries = htonl(in->max_entries); out->active_count = htonl(in->active_count); diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 38e7006cd..6c7559aaa 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -782,7 +782,7 @@ struct ofputil_table_stats { uint64_t apply_ofpacts; /* OFPACT_* supported on Apply-Actions. */ struct mf_bitmap write_setfields; /* Fields that can be set in W-A. */ struct mf_bitmap apply_setfields; /* Fields that can be set in A-A. */ - uint32_t instructions; /* Bitmap of OFPIT_* values supported. */ + uint32_t ovsinsts; /* Bitmap of OVSINST_* values supported. */ uint32_t active_count; /* Number of active entries. */ uint64_t lookup_count; /* Number of packets looked up in table. */ diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 9c0c94e01..ca173196b 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -797,7 +797,7 @@ struct ofproto_class { * * - 'metadata_match' and 'metadata_write' to OVS_BE64_MAX. * - * - 'instructions' to all instructions. + * - 'ovsinsts' to all instructions. * * - 'config' to OFPTC11_TABLE_MISS_MASK. * @@ -815,8 +815,8 @@ struct ofproto_class { * - 'wildcards' to the set of wildcards actually supported by the table * (if it doesn't support all OpenFlow wildcards). * - * - 'instructions' to set the instructions actually supported by - * the table. + * - 'ovsinsts' to the set of instructions actually supported by the + * table. * * - 'write_actions' to set the write actions actually supported by * the table (if it doesn't support all OpenFlow actions). diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 78b67739a..ac7cb1339 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -3094,7 +3094,7 @@ handle_table_stats_request(struct ofconn *ofconn, stats[i].apply_setfields = rw_fields; stats[i].metadata_match = OVS_BE64_MAX; stats[i].metadata_write = OVS_BE64_MAX; - stats[i].instructions = OFPIT13_ALL; + stats[i].ovsinsts = (1u << N_OVS_INSTRUCTIONS) - 1; stats[i].config = OFPTC11_TABLE_MISS_MASK; stats[i].max_entries = 1000000; /* An arbitrary big number. */ stats[i].active_count = classifier_count(&p->tables[i].cls); diff --git a/tests/ofproto.at b/tests/ofproto.at index 06a7df473..5eef15b70 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -1067,7 +1067,7 @@ OVS_VSWITCHD_START (mid="wild=0x1ffffffffd, max=1000000," tail=" lookup=0, matched=0 - match=0x1ffffffffd, instructions=0x00000007, config=0x00000003 + match=0x1ffffffffd, instructions=0x0000003e, config=0x00000003 write_actions=0x03ff8001, apply_actions=0x03ff8001 write_setfields=0x0000000c0fe7fbdd apply_setfields=0x0000000c0fe7fbdd -- 2.20.1