From: Ben Pfaff Date: Mon, 27 Aug 2018 22:40:35 +0000 (-0700) Subject: ofp-table: Ignore bits that have to change according to OpenFlow. X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=dccab9b15e656c6c1db1b8e94da1e1f234811506;p=ovs.git ofp-table: Ignore bits that have to change according to OpenFlow. OpenFlow table feature replies contain a per-table bitmap that indicates which tables a flow can point to in goto_table actions. OpenFlow requires that a table only be able to go to higher-numbered tables. This means that a switch that is general as possible will always have different features for every table, since each one will have a different bitmap. This makes the output of "ovs-ofctl dump-table-features" pretty long and ugly because it has about 250 entries like this: table %d: metadata: match=0xffffffffffffffff write=0xffffffffffffffff max_entries=%d instructions (table miss and others): next tables: %d-253 (same instructions) (same actions) (same matching) This commit changes the logic that prints table features messages so that it considers two sequentially numbered tables to be the same if only the bit that necessarily must be tunred off changes. This reduces the hundreds of entries above to just: tables 1...253: ditto which is so much more readable. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- diff --git a/lib/ofp-table.c b/lib/ofp-table.c index 71197f644..e5c824771 100644 --- a/lib/ofp-table.c +++ b/lib/ofp-table.c @@ -1329,12 +1329,47 @@ print_table_instruction_features( } } +/* Compares bitmaps of next tables 'a' and 'b', for tables 'a_table_id' and + * 'b_table_id', respectively. Returns true if the bitmaps are equal. + * + * The bitmaps are considered equal if b_table_id == a_table_id + 1 and the bit + * for 'b_table_id' is set in 'a' but not in 'b'. This is because OpenFlow + * requires that a table not be able to do a goto_table back to its own table + * or an earlier one. Without considering these equivalent, every table will + * be different from every one in some way, which just isn't useful in printing + * table features. */ +static bool +table_instruction_features_next_equal(const unsigned long *a, int a_table_id, + const unsigned long *b, int b_table_id) +{ + if (b_table_id == a_table_id + 1 + && bitmap_is_set(a, b_table_id) + && !bitmap_is_set(b, b_table_id)) { + for (size_t i = 0; i < BITMAP_N_LONGS(255); i++) { + unsigned long diff = a[i] ^ b[i]; + if (i == b_table_id / BITMAP_ULONG_BITS) { + diff &= ~bitmap_bit__(b_table_id); + } + if (diff) { + return false; + } + } + return true; + } else if (a_table_id == b_table_id + 1) { + return table_instruction_features_next_equal(b, b_table_id, + a, a_table_id); + } else { + return bitmap_equal(a, b, 255); + } +} + static bool table_instruction_features_equal( - const struct ofputil_table_instruction_features *a, - const struct ofputil_table_instruction_features *b) + const struct ofputil_table_instruction_features *a, int a_table_id, + const struct ofputil_table_instruction_features *b, int b_table_id) { - return (bitmap_equal(a->next, b->next, 255) + return (table_instruction_features_next_equal(a->next, a_table_id, + b->next, b_table_id) && a->instructions == b->instructions && table_action_features_equal(&a->write, &b->write) && table_action_features_equal(&a->apply, &b->apply)); @@ -1360,8 +1395,10 @@ table_features_equal(const struct ofputil_table_features *a, && a->supports_eviction == b->supports_eviction && a->supports_vacancy_events == b->supports_vacancy_events && a->max_entries == b->max_entries - && table_instruction_features_equal(&a->nonmiss, &b->nonmiss) - && table_instruction_features_equal(&a->miss, &b->miss) + && table_instruction_features_equal(&a->nonmiss, a->table_id, + &b->nonmiss, b->table_id) + && table_instruction_features_equal(&a->miss, a->table_id, + &b->miss, b->table_id) && bitmap_equal(a->match.bm, b->match.bm, MFF_N_IDS)); } @@ -1398,22 +1435,25 @@ ofputil_table_features_format( const struct ofputil_table_map *table_map, int *first_ditto, int *last_ditto) { + int table = features->table_id; + int prev_table = prev_features ? prev_features->table_id : 0; + bool same_stats = !stats || (prev_stats && table_stats_equal(stats, prev_stats)); bool same_features = prev_features && table_features_equal(features, prev_features); if (same_stats && same_features && !features->name[0]) { if (*first_ditto < 0) { - *first_ditto = features->table_id; + *first_ditto = table; } - *last_ditto = features->table_id; + *last_ditto = table; return; } ofputil_table_features_format_finish(s, *first_ditto, *last_ditto); *first_ditto = -1; ds_put_format(s, "\n table "); - ofputil_format_table(features->table_id, table_map, s); + ofputil_format_table(table, table_map, s); if (features->name[0]) { ds_put_format(s, " (\"%s\")", features->name); } @@ -1466,13 +1506,15 @@ ofputil_table_features_format( const struct ofputil_table_instruction_features *prev_miss = prev_features ? &prev_features->miss : NULL; if (prev_features - && table_instruction_features_equal(&features->nonmiss, prev_nonmiss) - && table_instruction_features_equal(&features->miss, prev_miss)) { + && table_instruction_features_equal(&features->nonmiss, table, + prev_nonmiss, prev_table) + && table_instruction_features_equal(&features->miss, table, + prev_miss, prev_table)) { if (!table_instruction_features_empty(&features->nonmiss)) { ds_put_cstr(s, " (same instructions)\n"); } - } else if (!table_instruction_features_equal(&features->nonmiss, - &features->miss)) { + } else if (!table_instruction_features_equal(&features->nonmiss, table, + &features->miss, table)) { ds_put_cstr(s, " instructions (other than table miss):\n"); print_table_instruction_features(s, &features->nonmiss, prev_nonmiss); ds_put_cstr(s, " instructions (table miss):\n"); diff --git a/tests/ofproto.at b/tests/ofproto.at index 16700223d..6a2cf27c0 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -2624,29 +2624,8 @@ metadata in_port in_port_oxm pkt_mark ct_mark ct_label reg0 reg1 reg2 reg3 reg4 ' "$1" } -ditto() { - printf ' table %d: - metadata: match=0xffffffffffffffff write=0xffffffffffffffff - max_entries=%d - instructions (table miss and others): - next tables: %d-253 - (same instructions) - (same actions) - (same matching) - -' $1 $2 `expr $1 + 1` -} tail_tables() { -echo ' table 252: - metadata: match=0xffffffffffffffff write=0xffffffffffffffff - max_entries=1000000 - instructions (table miss and others): - next tables: 253 - (same instructions) - (same actions) - (same matching) - - table 253: +echo ' table 253: metadata: match=0xffffffffffffffff write=0xffffffffffffffff max_entries=1000000 instructions (table miss and others): @@ -2656,9 +2635,7 @@ echo ' table 252: ' } (head_table - for i in `seq 1 251`; do - ditto $i 1000000 - done + printf ' tables 1...252: ditto\n\n' tail_tables) > expout AT_CHECK([ovs-ofctl -O OpenFlow13 dump-table-features br0], [0], [expout]) # Change the configuration. @@ -2673,10 +2650,20 @@ AT_CHECK( ]) # Check that the configuration was updated. (head_table ' ("main")' - ditto 1 1024 - for i in `seq 2 251`; do - ditto $i 1000000 - done + echo ' table 1: + metadata: match=0xffffffffffffffff write=0xffffffffffffffff + max_entries=1024 + (same instructions) + (same matching) + + table 2: + metadata: match=0xffffffffffffffff write=0xffffffffffffffff + max_entries=1000000 + (same instructions) + (same matching) + + tables 3...252: ditto +' tail_tables) > expout AT_CHECK([ovs-ofctl -O OpenFlow13 dump-table-features br0], [0], [expout]) OVS_VSWITCHD_STOP