From: rishibamba <rishiba...@users.noreply.github.com>
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.
Also changes are made to DESIGN.md and added test cases for add-flow &
replace-flows.
Signed-off-by: Rishi Bamba <rishi.ba...@tcs.com>
---
DESIGN.md | 7 ++++--
include/openflow/openflow-1.1.h | 5 ++--
lib/learn.c | 1 +
lib/learning-switch.c | 1 +
lib/ofp-parse.c | 6 ++++-
lib/ofp-print.c | 6 +++++
lib/ofp-util.c | 32 ++++++++++++++++++++++++
lib/ofp-util.h | 2 ++
ofproto/ofproto-dpif.c | 1 +
ofproto/ofproto-provider.h | 3 +++
ofproto/ofproto.c | 7 +++++-
tests/ovs-ofctl.at | 54 +++++++++++++++++++++++++++++++++++++++++
utilities/ovs-ofctl.c | 8 ++++++
13 files changed, 127 insertions(+), 6 deletions(-)
diff --git a/DESIGN.md b/DESIGN.md
index 6f8d090..bd0ed27 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/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index f87c5cf..d951dc7 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 */
/* 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 */
+ 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 e93015c..7c0d9b7 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 e3b48d5..66dc907 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 35ca316..1b2a483 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 = OFP_FLOW_PERMANENT;
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 8dc7f06..b1e8712 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -807,6 +807,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 != OFP_FLOW_PERMANENT) {
+ ds_put_format(s, "importance:%"PRIu16" ", fm.importance);
+ }
if (fm.priority != OFP_DEFAULT_PRIORITY && need_priority) {
ds_put_format(s, "pri:%"PRIu16" ", fm.priority);
}
@@ -1431,6 +1434,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 != OFP_FLOW_PERMANENT) {
+ 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 2f3dec8..d75c46c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1700,6 +1700,15 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
fm->idle_timeout = ntohs(ofm->idle_timeout);
fm->hard_timeout = ntohs(ofm->hard_timeout);
+ if ((protocol == OFPUTIL_P_OF14_OXM ||
+ protocol == OFPUTIL_P_OF15_OXM)
+ && (oh->version == OFP14_VERSION ||
+ oh->version == OFP15_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 +1747,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 +1775,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 +2259,15 @@ 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 ((protocol == OFPUTIL_P_OF14_OXM ||
+ protocol == OFPUTIL_P_OF15_OXM)
+ && (version == OFP14_VERSION ||
+ version == OFP15_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 +2854,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 || oh->version == OFP15_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 +2900,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 +2936,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 +3004,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 || version == OFP15_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 c9900d8..3c32203 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 9f4dbc8..6a53da8 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5315,6 +5315,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 14f1cba..8204862 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. */
+ /* "Importance" is the eviction precedence of a flow */
+ 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 1233164..6be338a 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 446d8f5..dc29aef 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.c b/utilities/ovs-ofctl.c
index 211c276..67465ce 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2264,6 +2264,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;
@@ -2289,6 +2290,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));
}
@@ -2315,6 +2317,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 != OFP_FLOW_PERMANENT) {
+ ds_put_format(s, " importance=%"PRIu16, version->importance);
+ }
ds_put_cstr(s, " actions=");
ofpacts_format(version->ofpacts, version->ofpacts_len, s);
@@ -2412,6 +2417,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;
@@ -2514,6 +2520,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);
@@ -2541,6 +2548,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;
--
2.1.1
=====-----=====-----=====
Notice: The information contained in this e-mail
message and/or attachments to it may contain
confidential or privileged information. If you are
not the intended recipient, any dissemination, use,
review, distribution, printing or copying of the
information contained in this e-mail message
and/or attachments to it are strictly prohibited. If
you have received this communication in error,
please notify us by reply e-mail or telephone and
immediately and permanently delete the message
and any attachments. Thank you
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
