From: Rishi Bamba Date: Fri, 7 Nov 2014 12:48:48 +0000 (+0530) Subject: Add support for OpenFlow 1.4+ "importance" values. X-Git-Tag: v2.4.0~1000 X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=ca26eb4437120e3b95b3727ccc6037dfa4e4065d Add support for OpenFlow 1.4+ "importance" values. This patch enables a user to set importance for a new rule via add-flow OF1.4+ in the OVS and display the same via dump-flows command OF1.4+. The changes are made in accordance with OpenFlow 1.4 specs to implement eviction on the basis of "importance". This patch also enhances the diff-flows & replace-flows CLI for addition of importance parameter in a rule. This doesn't actually implement eviction on the basis of importance, which will happen in a later patch. Signed-off-by: Rishi Bamba Signed-off-by: Ben Pfaff --- diff --git a/AUTHORS b/AUTHORS index 135470392..16e27e25f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -122,6 +122,7 @@ Ravi Kerur Ravi.Kerur@telekom.com Reid Price reid@nicira.com Remko Tronçon git@el-tramo.be Rich Lane rlane@bigswitch.com +Rishi Bamba rishi.bamba@tcs.com Rob Hoes rob.hoes@citrix.com Romain Lenglet romain.lenglet@berabera.info Ryan Wilson wryan@nicira.com diff --git a/DESIGN.md b/DESIGN.md index 6f8d09072..bd0ed272d 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -277,8 +277,11 @@ The table for 1.3 is the same as the one shown above for 1.2. OpenFlow 1.4 ------------ -OpenFlow 1.4 does not change flow_mod semantics. - +OpenFlow 1.4 adds the "importance" field to flow_mods, but it does not +explicitly specify which kinds of flow_mods set the importance.For +consistency, Open vSwitch uses the same rule for importance as for +idle_timeout and hard_timeout, that is, only an "ADD" flow_mod sets +the importance. (This issue has been filed with the ONF as EXT-496.) OFPT_PACKET_IN ============== diff --git a/NEWS b/NEWS index d487e328b..bc0ff5f4e 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,7 @@ Post-v2.3.0 * "resubmit" actions may now be included in action sets. The resubmit is executed last, and only if the action set has no "output" or "group" action. + * OpenFlow 1.4+ flow "importance" is now maintained in the flow table. - ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because MD5 is no longer secure and some operating systems have started to disable it in OpenSSL. diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h index f87c5cfb3..8b52b251f 100644 --- a/include/openflow/openflow-1.1.h +++ b/include/openflow/openflow-1.1.h @@ -340,7 +340,7 @@ struct ofp11_flow_mod { output group. A value of OFPG11_ANY indicates no restriction. */ ovs_be16 flags; /* One of OFPFF_*. */ - uint8_t pad[2]; + ovs_be16 importance; /* Eviction precedence (OF1.4+). */ /* Followed by an ofp11_match structure. */ /* Followed by an instruction set. */ }; @@ -451,7 +451,8 @@ struct ofp11_flow_stats { ovs_be16 idle_timeout; /* Number of seconds idle before expiration. */ ovs_be16 hard_timeout; /* Number of seconds before expiration. */ ovs_be16 flags; /* OF 1.3: Set of OFPFF*. */ - uint8_t pad2[4]; /* Align to 64-bits. */ + ovs_be16 importance; /* Eviction precedence (OF1.4+). */ + uint8_t pad2[2]; /* Align to 64-bits. */ ovs_be64 cookie; /* Opaque controller-issued identifier. */ ovs_be64 packet_count; /* Number of packets in flow. */ ovs_be64 byte_count; /* Number of bytes in flow. */ diff --git a/lib/learn.c b/lib/learn.c index 50ffd6f61..8a17a0d2e 100644 --- a/lib/learn.c +++ b/lib/learn.c @@ -101,6 +101,7 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow, fm->command = OFPFC_MODIFY_STRICT; fm->idle_timeout = learn->idle_timeout; fm->hard_timeout = learn->hard_timeout; + fm->importance = 0; fm->buffer_id = UINT32_MAX; fm->out_port = OFPP_NONE; fm->flags = 0; diff --git a/lib/learning-switch.c b/lib/learning-switch.c index 2b7495cad..af72ae10a 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -202,6 +202,7 @@ lswitch_handshake(struct lswitch *sw) fm.command = OFPFC_ADD; fm.idle_timeout = 0; fm.hard_timeout = 0; + fm.importance = 0; fm.buffer_id = UINT32_MAX; fm.out_port = OFPP_NONE; fm.out_group = OFPG_ANY; diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 35ca3169b..5064c7206 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -248,6 +248,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string, enum { F_OUT_PORT = 1 << 0, F_ACTIONS = 1 << 1, + F_IMPORTANCE = 1 << 2, F_TIMEOUT = 1 << 3, F_PRIORITY = 1 << 4, F_FLAGS = 1 << 5, @@ -264,7 +265,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string, break; case OFPFC_ADD: - fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY | F_FLAGS; + fields = F_ACTIONS | F_TIMEOUT | F_PRIORITY | F_FLAGS | F_IMPORTANCE; break; case OFPFC_DELETE: @@ -305,6 +306,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string, fm->buffer_id = UINT32_MAX; fm->out_port = OFPP_ANY; fm->flags = 0; + fm->importance = 0; fm->out_group = OFPG11_ANY; fm->delete_reason = OFPRR_DELETE; if (fields & F_ACTIONS) { @@ -366,6 +368,8 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string, error = str_to_u16(value, name, &fm->idle_timeout); } else if (fields & F_TIMEOUT && !strcmp(name, "hard_timeout")) { error = str_to_u16(value, name, &fm->hard_timeout); + } else if (fields & F_IMPORTANCE && !strcmp(name, "importance")) { + error = str_to_u16(value, name, &fm->importance); } else if (!strcmp(name, "cookie")) { char *mask = strchr(value, '/'); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index d27d8a757..e89a372eb 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -805,6 +805,9 @@ ofp_print_flow_mod(struct ds *s, const struct ofp_header *oh, int verbosity) if (fm.hard_timeout != OFP_FLOW_PERMANENT) { ds_put_format(s, "hard:%"PRIu16" ", fm.hard_timeout); } + if (fm.importance != 0) { + ds_put_format(s, "importance:%"PRIu16" ", fm.importance); + } if (fm.priority != OFP_DEFAULT_PRIORITY && need_priority) { ds_put_format(s, "pri:%"PRIu16" ", fm.priority); } @@ -1429,6 +1432,9 @@ ofp_print_flow_stats(struct ds *string, struct ofputil_flow_stats *fs) if (fs->flags) { ofp_print_flow_flags(string, fs->flags); } + if (fs->importance != 0) { + ds_put_format(string, "importance=%"PRIu16", ", fs->importance); + } if (fs->idle_age >= 0) { ds_put_format(string, "idle_age=%d, ", fs->idle_age); } diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 2f3dec895..60dc0e984 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -1700,6 +1700,11 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, fm->idle_timeout = ntohs(ofm->idle_timeout); fm->hard_timeout = ntohs(ofm->hard_timeout); + if (oh->version >= OFP14_VERSION && ofm->command == OFPFC_ADD) { + fm->importance = ntohs(ofm->importance); + } else { + fm->importance = 0; + } fm->buffer_id = ntohl(ofm->buffer_id); error = ofputil_port_from_ofp11(ofm->out_port, &fm->out_port); if (error) { @@ -1738,6 +1743,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, fm->new_cookie = ofm->cookie; fm->idle_timeout = ntohs(ofm->idle_timeout); fm->hard_timeout = ntohs(ofm->hard_timeout); + fm->importance = 0; fm->buffer_id = ntohl(ofm->buffer_id); fm->out_port = u16_to_ofp(ntohs(ofm->out_port)); fm->out_group = OFPG11_ANY; @@ -1765,6 +1771,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, fm->new_cookie = nfm->cookie; fm->idle_timeout = ntohs(nfm->idle_timeout); fm->hard_timeout = ntohs(nfm->hard_timeout); + fm->importance = 0; fm->buffer_id = ntohl(nfm->buffer_id); fm->out_port = u16_to_ofp(ntohs(nfm->out_port)); fm->out_group = OFPG11_ANY; @@ -2248,6 +2255,11 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm, ofm->out_port = ofputil_port_to_ofp11(fm->out_port); ofm->out_group = htonl(fm->out_group); ofm->flags = raw_flags; + if (version >= OFP14_VERSION && fm->command == OFPFC_ADD) { + ofm->importance = htons(fm->importance); + } else { + ofm->importance = 0; + } ofputil_put_ofp11_match(msg, &fm->match, protocol); ofpacts_put_openflow_instructions(fm->ofpacts, fm->ofpacts_len, msg, version); @@ -2834,6 +2846,11 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, fs->duration_nsec = ntohl(ofs->duration_nsec); fs->idle_timeout = ntohs(ofs->idle_timeout); fs->hard_timeout = ntohs(ofs->hard_timeout); + if (oh->version >= OFP14_VERSION) { + fs->importance = ntohs(ofs->importance); + } else { + fs->importance = 0; + } if (raw == OFPRAW_OFPST13_FLOW_REPLY) { error = ofputil_decode_flow_mod_flags(ofs->flags, -1, oh->version, &fs->flags); @@ -2875,6 +2892,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, fs->duration_nsec = ntohl(ofs->duration_nsec); fs->idle_timeout = ntohs(ofs->idle_timeout); fs->hard_timeout = ntohs(ofs->hard_timeout); + fs->importance = 0; fs->idle_age = -1; fs->hard_age = -1; fs->packet_count = ntohll(get_32aligned_be64(&ofs->packet_count)); @@ -2910,6 +2928,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, fs->priority = ntohs(nfs->priority); fs->idle_timeout = ntohs(nfs->idle_timeout); fs->hard_timeout = ntohs(nfs->hard_timeout); + fs->importance = 0; fs->idle_age = -1; fs->hard_age = -1; if (flow_age_extension) { @@ -2977,6 +2996,11 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs, ofs->priority = htons(fs->priority); ofs->idle_timeout = htons(fs->idle_timeout); ofs->hard_timeout = htons(fs->hard_timeout); + if (version >= OFP14_VERSION) { + ofs->importance = htons(fs->importance); + } else { + ofs->importance = 0; + } if (raw == OFPRAW_OFPST13_FLOW_REPLY) { ofs->flags = ofputil_encode_flow_mod_flags(fs->flags, version); } else { diff --git a/lib/ofp-util.h b/lib/ofp-util.h index c9900d8ac..a5cc55a8c 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -306,6 +306,7 @@ struct ofputil_flow_mod { ofp_port_t out_port; uint32_t out_group; enum ofputil_flow_mod_flags flags; + uint16_t importance; /* Eviction precedence. */ struct ofpact *ofpacts; /* Series of "struct ofpact"s. */ size_t ofpacts_len; /* Length of ofpacts, in bytes. */ @@ -355,6 +356,7 @@ struct ofputil_flow_stats { const struct ofpact *ofpacts; size_t ofpacts_len; enum ofputil_flow_mod_flags flags; + uint16_t importance; /* Eviction precedence. */ }; int ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *, diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 0d0649c2f..db306c544 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -5313,6 +5313,7 @@ ofproto_dpif_add_internal_flow(struct ofproto_dpif *ofproto, fm.command = OFPFC_ADD; fm.idle_timeout = idle_timeout; fm.hard_timeout = 0; + fm.importance = 0; fm.buffer_id = 0; fm.out_port = 0; fm.flags = OFPUTIL_FF_HIDDEN_FIELDS | OFPUTIL_FF_NO_READONLY; diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 1f7f7117b..94dbbe9ac 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -346,6 +346,9 @@ struct rule { uint16_t hard_timeout OVS_GUARDED; /* In seconds from ->modified. */ uint16_t idle_timeout OVS_GUARDED; /* In seconds from ->used. */ + /* Eviction precedence. */ + uint16_t importance OVS_GUARDED; + /* Eviction groups (see comment on struct eviction_group for explanation) . * * 'eviction_group' is this rule's eviction group, or NULL if it is not in diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 12331646f..6be338a29 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1895,6 +1895,7 @@ flow_mod_init(struct ofputil_flow_mod *fm, fm->command = command; fm->idle_timeout = 0; fm->hard_timeout = 0; + fm->importance = 0; fm->buffer_id = UINT32_MAX; fm->out_port = OFPP_ANY; fm->out_group = OFPG_ANY; @@ -1991,6 +1992,7 @@ ofproto_flow_mod(struct ofproto *ofproto, struct ofputil_flow_mod *fm) actions = rule_get_actions(rule); if (rule->idle_timeout == fm->idle_timeout && rule->hard_timeout == fm->hard_timeout + && rule->importance == fm->importance && rule->flags == (fm->flags & OFPUTIL_FF_STATE) && (!fm->modify_cookie || (fm->new_cookie == rule->flow_cookie)) && ofpacts_equal(fm->ofpacts, fm->ofpacts_len, @@ -3809,6 +3811,7 @@ handle_flow_stats_request(struct ofconn *ofconn, fs.cookie = rule->flow_cookie; fs.idle_timeout = rule->idle_timeout; fs.hard_timeout = rule->hard_timeout; + fs.importance = rule->importance; created = rule->created; modified = rule->modified; actions = rule_get_actions(rule); @@ -4238,6 +4241,7 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm, ovs_mutex_lock(&rule->mutex); rule->idle_timeout = fm->idle_timeout; rule->hard_timeout = fm->hard_timeout; + rule->importance = fm->importance; ovs_mutex_unlock(&rule->mutex); *CONST_CAST(uint8_t *, &rule->table_id) = table - ofproto->tables; @@ -4353,6 +4357,7 @@ modify_flows__(struct ofproto *ofproto, struct ofputil_flow_mod *fm, if (fm->command == OFPFC_ADD) { rule->idle_timeout = fm->idle_timeout; rule->hard_timeout = fm->hard_timeout; + rule->importance = fm->importance; rule->flags = fm->flags & OFPUTIL_FF_STATE; rule->created = now; } @@ -4366,7 +4371,7 @@ modify_flows__(struct ofproto *ofproto, struct ofputil_flow_mod *fm, cookies_insert(ofproto, rule); } if (fm->command == OFPFC_ADD) { - if (fm->idle_timeout || fm->hard_timeout) { + if (fm->idle_timeout || fm->hard_timeout || fm->importance) { if (!rule->eviction_group) { eviction_group_add_rule(rule); } diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 446d8f58e..dc29aefb8 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -2751,3 +2751,57 @@ AT_CHECK([tail -1 stdout], [0], OVS_VSWITCHD_STOP AT_CLEANUP + +dnl Check importance parameter added in OF1.4. +dnl It validates whether importance set via add-flow via OpenFlow1.4+ gets +dnl set or not by validating it againt the dump-flows output via OpenFlow1.4+ +dnl If add-flow or dump-flows is used with later version of OpenFlow prior to 1.4+ +dnl then the importance will be considered zero whether provided as an argument. + +AT_SETUP([ovs-ofctl rule with importance]) +OVS_VSWITCHD_START +dnl Flow with importance parameter added via OF1.4+ and later version +AT_CHECK([ovs-ofctl -O OpenFlow14 add-flow br0 priority=21,importance=21,actions=normal]) +AT_CHECK([ovs-ofctl add-flow br0 priority=22,importance=22,actions=normal]) + +dnl Importance parameter will only be visible of flows that are added via OF1.4+ if dumped via OF1.4+ +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip], [0], [dnl +OFPST_FLOW reply (OF1.4): + priority=22 actions=NORMAL + importance=21, priority=21 actions=NORMAL +]) + +dnl Importance parameter will not be visible if flow is dumped with previous version prior to OF1.4+ whether added via OF1.4+ +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip], [0], [dnl +NXST_FLOW reply: + priority=22 actions=NORMAL + priority=21 actions=NORMAL +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + + +dnl Importance parameter added in OF1.4. +dnl This validates whether flows with importance +dnl parameter are getting replaced with "replace-flows" or +dnl not by validating dump-flows output after replace with the expected output. + +AT_SETUP([ovs-ofctl replace-flows with importance]) +OVS_VSWITCHD_START + +dnl Add flows to br0 with importance via OF1.4+. For more details refer "ovs-ofctl rule with importance" test case. +for i in {1..8}; do echo "dl_vlan=$i,importance=$i,actions=drop"; done > add-flows.txt +AT_CHECK([ovs-ofctl -O OpenFlow14 add-flows br0 add-flows.txt]) + +dnl Replace some flows in the bridge. +for i in {1..8..2}; do echo "dl_vlan=$i,importance=`expr $i + 10`,actions=drop"; done > replace-flows.txt +AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 replace-flows.txt]) + +dnl Dump them and compare the dump flows output against the expected output. +for i in `seq 1 8`; do if [[ `expr $i % 2` -eq 1 ]]; then importance=`expr $i + 10`; else importance=$i; fi; echo " importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed '/OFPST_FLOW/d' | sort], + [0], [expout]) + +OVS_VSWITCHD_STOP +AT_CLEANUP diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index b8d41fbe5..0fe2bd4d0 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1847,6 +1847,14 @@ Causes the flow to expire after the given number of seconds, regardless of activity. A value of 0 (the default) gives the flow no hard expiration deadline. . +.IP "\fBimportance=\fIvalue\fR" +Sets the importance of a flow. The flow entry eviction mechanism can +use importance as a factor in deciding which flow to evict. A value +of 0 (the default) makes the flow non-evictable on the basis of +importance. Specify a value between 0 and 65535. +.IP +Only OpenFlow 1.4 and later support \fBimportance\fR. +. .IP "\fBsend_flow_rem\fR" Marks the flow with a flag that causes the switch to generate a ``flow removed'' message and send it to interested controllers when the flow diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 5835a7034..b5b90646e 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -2267,6 +2267,7 @@ struct fte_version { ovs_be64 cookie; uint16_t idle_timeout; uint16_t hard_timeout; + uint16_t importance; uint16_t flags; struct ofpact *ofpacts; size_t ofpacts_len; @@ -2292,6 +2293,7 @@ fte_version_equals(const struct fte_version *a, const struct fte_version *b) return (a->cookie == b->cookie && a->idle_timeout == b->idle_timeout && a->hard_timeout == b->hard_timeout + && a->importance == b->importance && ofpacts_equal(a->ofpacts, a->ofpacts_len, b->ofpacts, b->ofpacts_len)); } @@ -2318,6 +2320,9 @@ fte_version_format(const struct fte *fte, int index, struct ds *s) if (version->hard_timeout != OFP_FLOW_PERMANENT) { ds_put_format(s, " hard_timeout=%"PRIu16, version->hard_timeout); } + if (version->importance != 0) { + ds_put_format(s, " importance=%"PRIu16, version->importance); + } ds_put_cstr(s, " actions="); ofpacts_format(version->ofpacts, version->ofpacts_len, s); @@ -2415,6 +2420,7 @@ read_flows_from_file(const char *filename, struct classifier *cls, int index) version->cookie = fm.new_cookie; version->idle_timeout = fm.idle_timeout; version->hard_timeout = fm.hard_timeout; + version->importance = fm.importance; version->flags = fm.flags & (OFPUTIL_FF_SEND_FLOW_REM | OFPUTIL_FF_EMERG); version->ofpacts = fm.ofpacts; @@ -2517,6 +2523,7 @@ read_flows_from_switch(struct vconn *vconn, version->cookie = fs.cookie; version->idle_timeout = fs.idle_timeout; version->hard_timeout = fs.hard_timeout; + version->importance = fs.importance; version->flags = 0; version->ofpacts_len = fs.ofpacts_len; version->ofpacts = xmemdup(fs.ofpacts, fs.ofpacts_len); @@ -2544,6 +2551,7 @@ fte_make_flow_mod(const struct fte *fte, int index, uint16_t command, fm.command = command; fm.idle_timeout = version->idle_timeout; fm.hard_timeout = version->hard_timeout; + fm.importance = version->importance; fm.buffer_id = UINT32_MAX; fm.out_port = OFPP_ANY; fm.flags = version->flags;