[ovs-dev] [PATCH] ovs-ofctl:To set importance of a rule for eviction(OF14)
Rishi Bamba
rishi.bamba at tcs.com
Fri Sep 26 10:57:06 UTC 2014
This patch enables a user to set importance for a new rule via add-flow
OF1.1+ in the OVS and display the same via dump-flows command OF1.1+ .
The changes are made in accordance with OpenFlow 1.4 specs to implement
Eviction on the basis of "importance".
Signed-off-by: Rishi Bamba <rishi.bamba at tcs.com>
---
include/openflow/openflow-1.1.h | 5 +++--
lib/ofp-parse.c | 6 +++++-
lib/ofp-print.c | 3 +++
lib/ofp-util.c | 5 +++++
lib/ofp-util.h | 3 +++
ofproto/ofproto-provider.h | 3 +++
ofproto/ofproto.c | 5 ++++-
7 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index f87c5cf..d881192 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/ofp-parse.c b/lib/ofp-parse.c
index eed5a08..23626cb 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 43bfa17..7f45858 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1431,6 +1431,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 c8d38e8..9c3f219 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1700,6 +1700,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
fm->idle_timeout = ntohs(ofm->idle_timeout);
fm->hard_timeout = ntohs(ofm->hard_timeout);
+ fm->importance = ntohs(ofm->importance);
fm->buffer_id = ntohl(ofm->buffer_id);
error = ofputil_port_from_ofp11(ofm->out_port, &fm->out_port);
if (error) {
@@ -1738,6 +1739,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;
@@ -2248,6 +2250,7 @@ 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;
+ ofm->importance = htons(fm->importance);
ofputil_put_ofp11_match(msg, &fm->match, protocol);
ofpacts_put_openflow_instructions(fm->ofpacts, fm->ofpacts_len, msg,
version);
@@ -2834,6 +2837,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 = ntohs(ofs->importance);
if (raw == OFPRAW_OFPST13_FLOW_REPLY) {
error = ofputil_decode_flow_mod_flags(ofs->flags, -1, oh->version,
&fs->flags);
@@ -2977,6 +2981,7 @@ 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);
+ ofs->importance = htons(fs->importance);
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 af1a2a3..3173ab4 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 *,
@@ -391,6 +393,7 @@ struct ofputil_flow_removed {
uint16_t hard_timeout;
uint64_t packet_count; /* Packet count, UINT64_MAX if unknown. */
uint64_t byte_count; /* Byte count, UINT64_MAX if unknown. */
+ uint16_t importance; /* Eviction Precedence */
};
enum ofperr ofputil_decode_flow_removed(struct ofputil_flow_removed *,
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 158f86e..106d4e6 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 5233a4d..f2835f7 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3827,6 +3827,7 @@ handle_flow_stats_request(struct ofconn *ofconn,
fs.ofpacts_len = actions->ofpacts_len;
fs.flags = flags;
+ fs.importance=rule->importance;
ofputil_append_flow_stats_reply(&fs, &replies);
}
@@ -4237,6 +4238,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;
@@ -4354,6 +4356,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;
}
@@ -4367,7 +4370,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);
}
--
2.1.1
Thank You
Regards
Rishi
----- Original Message -----
From: "Ben Pfaff" <blp at nicira.com>
To: "Rishi Bamba" <rishi.bamba at tcs.com>
Cc: dev at openvswitch.org
Sent: Monday, September 22, 2014 10:31:37 PM
Subject: Re: [ovs-dev] Addition of importance parameter in flow_mod structure for implementation of eviction according to OF1.4
It would save a lot of time for both of us if you would just post your
patches.
On Mon, Sep 22, 2014 at 08:09:03PM +0530, Rishi Bamba wrote:
> Hi Ben,
>
> In reference to the mail dated 15.09.2014 I would like to thank you for your continued support to the team.Attached is the mail for reference.
>
> In progress to the same ,so far we have achieved the below mentioned.Wanted to share the understanding and confirmation on the same.
>
>
> Case 1: Flow Added with default active version on OpenvSwitch ( OF10 ) alongwith importance
> ( Eviction implementation in accordance with OF1.4 ).
>
> Add Flow Command(OF10) : ovs-ofctl add-flow br0 priority=21,importance=21,action=normal
> Output: No Error , rule saved but with default importance( i.e zero ) rather than with the specified value by user.
>
>
> Dump-Flow Command(OF 10) : ovs-ofctl dump-flows br0
> Output: cookie=0x0, duration=8.736s, table=0, n_packets=0, n_bytes=0, idle_age=8, priority=21 actions=NORMAL
>
> Dump-Flow Command(OF 11+) : ovs-ofctl -O OpenFlow13 dump-flows br0
> Output: cookie=0x0, duration=116.980s, table=0, n_packets=6, n_bytes=546, priority=21 actions=NORMAL
>
> The understanding is ,if the flow is added by the default active version i.e OF10 , the added flow will have importance set as zero whether provided by the user or not.The same is reflected in the dump flows either via OF10 or OF11+ . If importance is zero it will not be visible in that case.
>
> Query 1: Is this handling correct or we need to throw a message to the user that "importance" is supported by a particular version only rather than making it zero. Also as we are using OF11 flow mod structure as mentioned in previous mail, will importance parameter in add-flow should be supported on 1.1+ or we have to make it OF1.4 specific only.
>
>
>
> Case 2: Flow Added with specific version on OpenvSwitch ( OF11+ ) alongwith importance
> (Eviction implementation in accordance with OF1.4 ).
>
> Add Flow Command(OF11+) : ovs-ofctl -O OpenFlow13 add-flow br0 priority=22,importance=22,action=drop
> Output: No Error , rule saved with the value provided by the user and else zero if not provided by the user.
>
>
> Dump-Flow Command(OF 10) : ovs-ofctl dump-flows br0
> Output: (IMPORTANCE NOT VISIBLE)
> cookie=0x0, duration=47.841s, table=0, n_packets=2, n_bytes=182, idle_age=24, priority=22 actions=drop
> cookie=0x0, duration=243.481s, table=0, n_packets=6, n_bytes=546, idle_age=152, priority=21 actions=NORMAL
>
> Dump-Flow Command(OF 11+) : ovs-ofctl -O OpenFlow13 dump-flows br0
> Output: (IMPORTANCE VISIBLE FOR THE FLOWS HAVING VALUE SET FOR THE FIELD)
> cookie=0x0, duration=141.275s, table=0, n_packets=2, n_bytes=182, importance=22, priority=22 actions=drop
> cookie=0x0, duration=336.915s, table=0, n_packets=6, n_bytes=546, priority=21 actions=NORMAL
>
> The understanding is,if the flow is added by a specific version (OF11+), the added flow will have importance set as the value provided by the user else zero.Now if the user dump-flows with the default version (OF10) none of the flows importance parameter will be visible even for the set one's else if dump-flows via OF11+ , the importance parameter will be visible for the flows which have the parameter set of.
>
> Query 2: Do we need to show the importance field if a flow is added by OF11+ and dump-flows via OF10. Currently we are not showing as mentioned above.Also this should also be only 1.4 specific or 1.1+ is fine.
>
>
> Note: On finalizing the the above mentioned approach we will be submitting the patch for the same at the earliest. Need your confirmation on the above working.
>
>
> Thank You
> Regards
> Rishi
>
> =====-----=====-----=====
> 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
>
>
> Date: Sun, 14 Sep 2014 13:50:23 -0700
> From: Ben Pfaff <blp at nicira.com>
> To: Rishi Bamba <rishi.bamba at tcs.com>
> Cc: dev at openvswitch.org
> Subject: Re: [ovs-dev] Addition of importance parameter in flow_mod
> structure for implementation of eviction according to OF1.4
> User-Agent: Mutt/1.5.21 (2010-09-15)
>
> On Fri, Sep 12, 2014 at 05:55:18PM +0530, Rishi Bamba wrote:
> > 1. Currently we have modified ofp11_flow_mod in openflow-1.1.h for
> > addition of "ovs_be16 importance" parameter.
> >
> > Query: Do we need to create ofp14_flow_mod as the structure is
> > currently not present in openflow-1.4.h or the enhancement to
> > ofp11_flow_mod in openflow-1.1.h is justified.
>
> It's OK to update ofp11_flow_mod because the new member just assigns a
> meaning to a field previously used for padding.
>
> > 2. We noticed that when using "add-flow" command with argument "-O
> > OpenFlow14" a error message is thrown
>
> You should enable OF1.4. See the FAQ.
>
> Q: What versions of OpenFlow does Open vSwitch support?
>
> A: The following table lists the versions of OpenFlow supported by
> each version of Open vSwitch:
>
> Open vSwitch OF1.0 OF1.1 OF1.2 OF1.3 OF1.4 OF1.5
> =============== ===== ===== ===== ===== ===== =====
> 1.9 and earlier yes --- --- --- --- ---
> 1.10 yes --- [*] [*] --- ---
> 1.11 yes --- [*] [*] --- ---
> 2.0 yes [*] [*] [*] --- ---
> 2.1 yes [*] [*] [*] --- ---
> 2.2 yes [*] [*] [*] [%] [*]
> 2.3 yes yes yes yes [*] [*]
>
> [*] Supported, with one or more missing features.
> [%] Experimental, unsafe implementation.
>
> Open vSwitch 2.3 enables OpenFlow 1.0, 1.1, 1.2, and 1.3 by default
> in ovs-vswitchd. In Open vSwitch 1.10 through 2.2, OpenFlow 1.1,
> 1.2, and 1.3 must be enabled manually in ovs-vswitchd. OpenFlow
> 1.4 and 1.5 are also supported, with missing features, in Open
> vSwitch 2.3 and later, but not enabled by default. In any case,
> the user may override the default:
>
> - To enable OpenFlow 1.0, 1.1, 1.2, and 1.3 on bridge br0:
>
> ovs-vsctl set bridge br0 protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13
>
> - To enable OpenFlow 1.0, 1.1, 1.2, 1.3, 1.4, and 1.5 on bridge br0:
>
> ovs-vsctl set bridge br0 protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15
>
> - To enable only OpenFlow 1.0 on bridge br0:
>
> ovs-vsctl set bridge br0 protocols=OpenFlow10
>
> All current versions of ovs-ofctl enable only OpenFlow 1.0 by
> default. Use the -O option to enable support for later versions of
> OpenFlow in ovs-ofctl. For example:
>
> ovs-ofctl -O OpenFlow13 dump-flows br0
>
> (Open vSwitch 2.2 had an experimental implementation of OpenFlow
> 1.4 that could cause crashes. We don't recommend enabling it.)
>
> OPENFLOW-1.1+ in the Open vSwitch source tree tracks support for
> OpenFlow 1.1 and later features. When support for OpenFlow 1.4 and
> 1.5 is solidly implemented, Open vSwitch will enable those version
> by default. Also, the OpenFlow 1.5 specification is still under
> development and thus subject to change.
More information about the dev
mailing list