This patch ensures setting of importance parameter of a flow
everywhere it should be. This also correct formatting issues
as per the coding guidelines.
Signed-off-by: Rishi Bamba <rishi.ba...@tcs.com>
---
include/openflow/openflow-1.1.h | 4 ++--
lib/learn.c | 1 +
lib/learning-switch.c | 1 +
lib/ofp-parse.c | 2 +-
lib/ofp-print.c | 5 ++++-
lib/ofp-util.h | 3 +--
ofproto/ofproto-dpif.c | 1 +
ofproto/ofproto.c | 2 ++
8 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index d881192..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_*. */
- ovs_be16 importance; /* Eviction Precedence */
+ ovs_be16 importance; /* Eviction Precedence */
/* Followed by an ofp11_match structure. */
/* Followed by an instruction set. */
};
@@ -451,7 +451,7 @@ 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*. */
- ovs_be16 importance; /* Eviction Precedence */
+ 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. */
diff --git a/lib/learn.c b/lib/learn.c
index e1c73cb..e4f4a44 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 23626cb..a74ce40 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -248,7 +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_IMPORTANCE = 1 << 2,
F_TIMEOUT = 1 << 3,
F_PRIORITY = 1 << 4,
F_FLAGS = 1 << 5,
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 7f45858..3ca2cae 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,7 +1434,7 @@ 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) {
+ if (fs->importance != OFP_FLOW_PERMANENT) {
ds_put_format(string, "importance=%"PRIu16", ", fs->importance);
}
if (fs->idle_age >= 0) {
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 3173ab4..d50aa4e 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -306,7 +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 */
+ uint16_t importance; /* Eviction Precedence */
struct ofpact *ofpacts; /* Series of "struct ofpact"s. */
size_t ofpacts_len; /* Length of ofpacts, in bytes. */
@@ -393,7 +393,6 @@ 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-dpif.c b/ofproto/ofproto-dpif.c
index d965d38..5b5614d 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.c b/ofproto/ofproto.c
index 63338aa..6f1c781 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1886,6 +1886,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;
@@ -1982,6 +1983,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,
--
2.1.1
Thank You
Regards
Rishi Bamba
----- Original Message -----
From: "Ben Pfaff" <b...@nicira.com>
To: "Rishi Bamba" <rishi.ba...@tcs.com>
Cc: dev@openvswitch.org, "partha datta" <partha.da...@tcs.com>, "deepankar
gupta" <deepankar.gu...@tcs.com>
Sent: Saturday, October 11, 2014 2:01:51 AM
Subject: Re: [PATCH 1/4] ovs-ofctl:To set importance of a rule for
eviction(OF14)
On Fri, Oct 10, 2014 at 07:11:45PM +0530, Rishi Bamba wrote:
> Hi Ben,
>
> # Thank you for reviewing the patch.As per the comments received ,all the
> changes are made and incorporated for this patch i.e Clang
> # compiler doesn't report any error now and all the test cases are successful
> including the one added by us for the same.Also along
> # with this patch sending three other patches related to the addition of
> importance in a rule which includes the changes as asked by
> # you plus replace-flows and diff-flows CLI enhancement after the addition of
> "importance" parameter in a rule as per OF14.
> ---
> 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".
Thanks for the patch. I have some comments.
I'd appreciate it if you would be more careful about formatting your
patch emails. The commit message should be at the top, and any
additional commentary should be below the ---. Only the part before
--- actually goes into the commit message, so this ensures that the
commit message has the content that you intend.
In this hunk, please follow the same formatting pattern as the other
lines:
> @@ -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,
OFP_FLOW_PERMANENT is meant to be used for hard_timeout and
idle_timeout. I don't think that we should reuse it for importance
also (please just write 0):
> + if (fs->importance != OFP_FLOW_PERMANENT) {
>
It looks like some places where 'importance' should be set were
missed. When I grep for ofputil_flow_mod, I see that, for example,
learn_execute() needs to set 'importance'. Please do your own grep
and make sure that importance is set everywhere it should be.
After you fix those problems, I think that this patch will be ready.
Thanks,
Ben.
=====-----=====-----=====
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
