From 00da1ac5299c054fe0bd41a4e0113fc36d0c3be8 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Mon, 30 Nov 2015 16:12:11 -0800 Subject: [PATCH] ovs-ofctl: Support multiple tables in replace-flows and diff-flows. Currently ovs-ofctl replace-flows and diff-flows commands only support flows in table 0. Extend this to cover all possible tables. Signed-off-by: Jarno Rajahalme Acked-by: Ben Pfaff --- tests/ovs-ofctl.at | 60 ++++++------- utilities/ovs-ofctl.c | 194 +++++++++++++++++++++++++++--------------- 2 files changed, 157 insertions(+), 97 deletions(-) diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 78f31bdaa..2923cf2dd 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -2891,11 +2891,11 @@ OVS_VSWITCHD_START AT_CHECK([ovs-appctl vlog/set vconn:dbg]) dnl Add flows to br0 with importance via OF1.4+, using an OF1.4+ bundle. For more details refer "ovs-ofctl rule with importance" test case. -for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i,actions=drop"; done > add-flows.txt +for i in 1 2 3 4 5 6 7 8; do echo "table=$i,dl_vlan=$i,importance=$i,actions=drop"; done > add-flows.txt AT_CHECK([ovs-ofctl --bundle add-flows br0 add-flows.txt]) dnl Replace some flows in the bridge. -for i in 1 3 5 7; do echo " importance=`expr $i + 10`, dl_vlan=$i actions=drop"; done > replace-flows.txt +for i in 1 3 5 7; do echo " table=$i, importance=`expr $i + 10`, dl_vlan=$i actions=drop"; done > replace-flows.txt AT_CHECK([ovs-ofctl --bundle replace-flows br0 replace-flows.txt]) dnl Dump them and compare the dump flows output against the expected output. @@ -2919,28 +2919,28 @@ vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REPLY flags=0 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:1 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:1 dl_vlan=1 importance:1 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=2 importance:2 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:2 dl_vlan=2 importance:2 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=3 importance:3 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:3 dl_vlan=3 importance:3 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=4 importance:4 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:4 dl_vlan=4 importance:4 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=5 importance:5 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:5 dl_vlan=5 importance:5 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=6 importance:6 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:6 dl_vlan=6 importance:6 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=7 importance:7 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:7 dl_vlan=7 importance:7 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=8 importance:8 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:8 dl_vlan=8 importance:8 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): @@ -2952,42 +2952,42 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4): vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): - importance=1, dl_vlan=1 actions=drop - importance=2, dl_vlan=2 actions=drop - importance=3, dl_vlan=3 actions=drop - importance=4, dl_vlan=4 actions=drop - importance=5, dl_vlan=5 actions=drop - importance=6, dl_vlan=6 actions=drop - importance=7, dl_vlan=7 actions=drop - importance=8, dl_vlan=8 actions=drop + table=1, importance=1, dl_vlan=1 actions=drop + table=2, importance=2, dl_vlan=2 actions=drop + table=3, importance=3, dl_vlan=3 actions=drop + table=4, importance=4, dl_vlan=4 actions=drop + table=5, importance=5, dl_vlan=5 actions=drop + table=6, importance=6, dl_vlan=6 actions=drop + table=7, importance=7, dl_vlan=7 actions=drop + table=8, importance=8, dl_vlan=8 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REPLY flags=0 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=2 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:1 dl_vlan=1 importance:11 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=4 actions=drop +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:2 dl_vlan=2 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=6 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:3 dl_vlan=3 importance:13 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=8 actions=drop +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:4 dl_vlan=4 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:11 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:5 dl_vlan=5 importance:15 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=3 importance:13 actions=drop +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:6 dl_vlan=6 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=5 importance:15 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:7 dl_vlan=7 importance:17 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=7 importance:17 actions=drop +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:8 dl_vlan=8 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): @@ -2999,10 +2999,10 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4): vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): - importance=11, dl_vlan=1 actions=drop - importance=13, dl_vlan=3 actions=drop - importance=15, dl_vlan=5 actions=drop - importance=17, dl_vlan=7 actions=drop + table=1, importance=11, dl_vlan=1 actions=drop + table=3, importance=13, dl_vlan=3 actions=drop + table=5, importance=15, dl_vlan=5 actions=drop + table=7, importance=17, dl_vlan=7 actions=drop ]) OVS_VSWITCHD_STOP diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index bc37e77b3..685c3504c 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -2558,6 +2558,45 @@ ofctl_list_commands(struct ovs_cmdl_context *ctx OVS_UNUSED) /* replace-flows and diff-flows commands. */ +struct flow_tables { + struct classifier tables[OFPTT_MAX + 1]; +}; + +#define FOR_EACH_TABLE(CLS, TABLES) \ + for ((CLS) = (TABLES)->tables; \ + (CLS) < &(TABLES)->tables[ARRAY_SIZE((TABLES)->tables)]; \ + (CLS)++) + +static void +flow_tables_init(struct flow_tables *tables) +{ + struct classifier *cls; + + FOR_EACH_TABLE (cls, tables) { + classifier_init(cls, NULL); + } +} + +static void +flow_tables_defer(struct flow_tables *tables) +{ + struct classifier *cls; + + FOR_EACH_TABLE (cls, tables) { + classifier_defer(cls); + } +} + +static void +flow_tables_publish(struct flow_tables *tables) +{ + struct classifier *cls; + + FOR_EACH_TABLE (cls, tables) { + classifier_publish(cls); + } +} + /* A flow table entry, possibly with two different versions. */ struct fte { struct cls_rule rule; /* Within a "struct classifier". */ @@ -2573,6 +2612,7 @@ struct fte_version { uint16_t flags; struct ofpact *ofpacts; size_t ofpacts_len; + uint8_t table_id; }; /* Frees 'version' and the data that it owns. */ @@ -2596,6 +2636,7 @@ fte_version_equals(const struct fte_version *a, const struct fte_version *b) && a->idle_timeout == b->idle_timeout && a->hard_timeout == b->hard_timeout && a->importance == b->importance + && a->table_id == b->table_id && ofpacts_equal(a->ofpacts, a->ofpacts_len, b->ofpacts, b->ofpacts_len)); } @@ -2612,6 +2653,9 @@ fte_version_format(const struct fte *fte, int index, struct ds *s) return; } + if (version->table_id) { + ds_put_format(s, "table=%"PRIu8" ", version->table_id); + } cls_rule_format(&fte->rule, s); if (version->cookie != htonll(0)) { ds_put_format(s, " cookie=0x%"PRIx64, ntohll(version->cookie)); @@ -2650,29 +2694,34 @@ fte_free(struct fte *fte) } } -/* Frees all of the FTEs within 'cls'. */ +/* Frees all of the FTEs within 'tables'. */ static void -fte_free_all(struct classifier *cls) +fte_free_all(struct flow_tables *tables) { - struct fte *fte; + struct classifier *cls; + + FOR_EACH_TABLE (cls, tables) { + struct fte *fte; - classifier_defer(cls); - CLS_FOR_EACH (fte, rule, cls) { - classifier_remove(cls, &fte->rule); - ovsrcu_postpone(fte_free, fte); + classifier_defer(cls); + CLS_FOR_EACH (fte, rule, cls) { + classifier_remove(cls, &fte->rule); + ovsrcu_postpone(fte_free, fte); + } + classifier_destroy(cls); } - classifier_destroy(cls); } -/* Searches 'cls' for an FTE matching 'rule', inserting a new one if +/* Searches 'tables' for an FTE matching 'rule', inserting a new one if * necessary. Sets 'version' as the version of that rule with the given * 'index', replacing any existing version, if any. * * Takes ownership of 'version'. */ static void -fte_insert(struct classifier *cls, const struct match *match, +fte_insert(struct flow_tables *tables, const struct match *match, int priority, struct fte_version *version, int index) { + struct classifier *cls = &tables->tables[version->table_id]; struct fte *old, *fte; fte = xzalloc(sizeof *fte); @@ -2689,11 +2738,12 @@ fte_insert(struct classifier *cls, const struct match *match, } } -/* Reads the flows in 'filename' as flow table entries in 'cls' for the version - * with the specified 'index'. Returns the flow formats able to represent the - * flows that were read. */ +/* Reads the flows in 'filename' as flow table entries in 'tables' for the + * version with the specified 'index'. Returns the flow formats able to + * represent the flows that were read. */ static enum ofputil_protocol -read_flows_from_file(const char *filename, struct classifier *cls, int index) +read_flows_from_file(const char *filename, struct flow_tables *tables, + int index) { enum ofputil_protocol usable_protocols; int line_number; @@ -2708,7 +2758,7 @@ read_flows_from_file(const char *filename, struct classifier *cls, int index) ds_init(&s); usable_protocols = OFPUTIL_P_ANY; line_number = 0; - classifier_defer(cls); + flow_tables_defer(tables); while (!ds_get_preprocessed_line(&s, file, &line_number)) { struct fte_version *version; struct ofputil_flow_mod fm; @@ -2730,10 +2780,11 @@ read_flows_from_file(const char *filename, struct classifier *cls, int index) | OFPUTIL_FF_EMERG); version->ofpacts = fm.ofpacts; version->ofpacts_len = fm.ofpacts_len; + version->table_id = fm.table_id != OFPTT_ALL ? fm.table_id : 0; - fte_insert(cls, &fm.match, fm.priority, version, index); + fte_insert(tables, &fm.match, fm.priority, version, index); } - classifier_publish(cls); + flow_tables_publish(tables); ds_destroy(&s); if (file != stdin) { @@ -2797,12 +2848,12 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32 send_xid, } /* Reads the OpenFlow flow table from 'vconn', which has currently active flow - * format 'protocol', and adds them as flow table entries in 'cls' for the + * format 'protocol', and adds them as flow table entries in 'tables' for the * version with the specified 'index'. */ static void read_flows_from_switch(struct vconn *vconn, enum ofputil_protocol protocol, - struct classifier *cls, int index) + struct flow_tables *tables, int index) { struct ofputil_flow_stats_request fsr; struct ofputil_flow_stats fs; @@ -2823,7 +2874,7 @@ read_flows_from_switch(struct vconn *vconn, reply = NULL; ofpbuf_init(&ofpacts, 0); - classifier_defer(cls); + flow_tables_defer(tables); while (recv_flow_stats_reply(vconn, send_xid, &reply, &fs, &ofpacts)) { struct fte_version *version; @@ -2835,10 +2886,11 @@ read_flows_from_switch(struct vconn *vconn, version->flags = 0; version->ofpacts_len = fs.ofpacts_len; version->ofpacts = xmemdup(fs.ofpacts, fs.ofpacts_len); + version->table_id = fs.table_id; - fte_insert(cls, &fs.match, fs.priority, version, index); + fte_insert(tables, &fs.match, fs.priority, version, index); } - classifier_publish(cls); + flow_tables_publish(tables); ofpbuf_uninit(&ofpacts); } @@ -2856,7 +2908,7 @@ fte_make_flow_mod(const struct fte *fte, int index, uint16_t command, fm.cookie_mask = htonll(0); fm.new_cookie = version->cookie; fm.modify_cookie = true; - fm.table_id = 0xff; + fm.table_id = version->table_id; fm.command = command; fm.idle_timeout = version->idle_timeout; fm.hard_timeout = version->hard_timeout; @@ -2884,41 +2936,45 @@ ofctl_replace_flows(struct ovs_cmdl_context *ctx) { enum { FILE_IDX = 0, SWITCH_IDX = 1 }; enum ofputil_protocol usable_protocols, protocol; - struct classifier cls; + struct flow_tables tables; + struct classifier *cls; struct ovs_list requests; struct vconn *vconn; struct fte *fte; - classifier_init(&cls, NULL); - usable_protocols = read_flows_from_file(ctx->argv[2], &cls, FILE_IDX); + flow_tables_init(&tables); + usable_protocols = read_flows_from_file(ctx->argv[2], &tables, FILE_IDX); protocol = open_vconn(ctx->argv[1], &vconn); protocol = set_protocol_for_flow_dump(vconn, protocol, usable_protocols); - read_flows_from_switch(vconn, protocol, &cls, SWITCH_IDX); + read_flows_from_switch(vconn, protocol, &tables, SWITCH_IDX); list_init(&requests); - /* Delete flows that exist on the switch but not in the file. */ - CLS_FOR_EACH (fte, rule, &cls) { - struct fte_version *file_ver = fte->versions[FILE_IDX]; - struct fte_version *sw_ver = fte->versions[SWITCH_IDX]; + FOR_EACH_TABLE (cls, &tables) { + /* Delete flows that exist on the switch but not in the file. */ + CLS_FOR_EACH (fte, rule, cls) { + struct fte_version *file_ver = fte->versions[FILE_IDX]; + struct fte_version *sw_ver = fte->versions[SWITCH_IDX]; - if (sw_ver && !file_ver) { - fte_make_flow_mod(fte, SWITCH_IDX, OFPFC_DELETE_STRICT, - protocol, &requests); + if (sw_ver && !file_ver) { + fte_make_flow_mod(fte, SWITCH_IDX, OFPFC_DELETE_STRICT, + protocol, &requests); + } } - } - /* Add flows that exist in the file but not on the switch. - * Update flows that exist in both places but differ. */ - CLS_FOR_EACH (fte, rule, &cls) { - struct fte_version *file_ver = fte->versions[FILE_IDX]; - struct fte_version *sw_ver = fte->versions[SWITCH_IDX]; + /* Add flows that exist in the file but not on the switch. + * Update flows that exist in both places but differ. */ + CLS_FOR_EACH (fte, rule, cls) { + struct fte_version *file_ver = fte->versions[FILE_IDX]; + struct fte_version *sw_ver = fte->versions[SWITCH_IDX]; - if (file_ver - && (readd || !sw_ver || !fte_version_equals(sw_ver, file_ver))) { - fte_make_flow_mod(fte, FILE_IDX, OFPFC_ADD, protocol, &requests); + if (file_ver && + (readd || !sw_ver || !fte_version_equals(sw_ver, file_ver))) { + fte_make_flow_mod(fte, FILE_IDX, OFPFC_ADD, protocol, + &requests); + } } } if (bundle) { @@ -2928,24 +2984,25 @@ ofctl_replace_flows(struct ovs_cmdl_context *ctx) } vconn_close(vconn); - fte_free_all(&cls); + fte_free_all(&tables); } static void -read_flows_from_source(const char *source, struct classifier *cls, int index) +read_flows_from_source(const char *source, struct flow_tables *tables, + int index) { struct stat s; if (source[0] == '/' || source[0] == '.' || (!strchr(source, ':') && !stat(source, &s))) { - read_flows_from_file(source, cls, index); + read_flows_from_file(source, tables, index); } else { enum ofputil_protocol protocol; struct vconn *vconn; protocol = open_vconn(source, &vconn); protocol = set_protocol_for_flow_dump(vconn, protocol, OFPUTIL_P_ANY); - read_flows_from_switch(vconn, protocol, cls, index); + read_flows_from_switch(vconn, protocol, tables, index); vconn_close(vconn); } } @@ -2954,32 +3011,35 @@ static void ofctl_diff_flows(struct ovs_cmdl_context *ctx) { bool differences = false; - struct classifier cls; + struct flow_tables tables; + struct classifier *cls; struct ds a_s, b_s; struct fte *fte; - classifier_init(&cls, NULL); - read_flows_from_source(ctx->argv[1], &cls, 0); - read_flows_from_source(ctx->argv[2], &cls, 1); + flow_tables_init(&tables); + read_flows_from_source(ctx->argv[1], &tables, 0); + read_flows_from_source(ctx->argv[2], &tables, 1); ds_init(&a_s); ds_init(&b_s); - CLS_FOR_EACH (fte, rule, &cls) { - struct fte_version *a = fte->versions[0]; - struct fte_version *b = fte->versions[1]; - - if (!a || !b || !fte_version_equals(a, b)) { - fte_version_format(fte, 0, &a_s); - fte_version_format(fte, 1, &b_s); - if (strcmp(ds_cstr(&a_s), ds_cstr(&b_s))) { - if (a_s.length) { - printf("-%s", ds_cstr(&a_s)); - } - if (b_s.length) { - printf("+%s", ds_cstr(&b_s)); + FOR_EACH_TABLE (cls, &tables) { + CLS_FOR_EACH (fte, rule, cls) { + struct fte_version *a = fte->versions[0]; + struct fte_version *b = fte->versions[1]; + + if (!a || !b || !fte_version_equals(a, b)) { + fte_version_format(fte, 0, &a_s); + fte_version_format(fte, 1, &b_s); + if (strcmp(ds_cstr(&a_s), ds_cstr(&b_s))) { + if (a_s.length) { + printf("-%s", ds_cstr(&a_s)); + } + if (b_s.length) { + printf("+%s", ds_cstr(&b_s)); + } + differences = true; } - differences = true; } } } @@ -2987,7 +3047,7 @@ ofctl_diff_flows(struct ovs_cmdl_context *ctx) ds_destroy(&a_s); ds_destroy(&b_s); - fte_free_all(&cls); + fte_free_all(&tables); if (differences) { exit(2); @@ -3334,7 +3394,7 @@ ofctl_parse_actions__(const char *version_s, bool instructions) error = ofpacts_check_consistency(ofpacts.data, ofpacts.size, &flow, OFPP_MAX, table_id ? atoi(table_id) : 0, - 255, protocol); + OFPTT_MAX + 1, protocol); } if (error) { printf("bad %s %s: %s\n\n", -- 2.20.1