On Thu, Sep 12, 2013 at 12:56:53AM +0300, Daniel Baluta wrote:
> This is a first step in implementing 'on demand flow counters'.
>
> We save relevant flow_mod flags (OFPUTIL_FF_SEND_FLOW_REM,
> OFPUTIL_FF_NO_PKT_COUNTS, OFPUTIL_FF_NO_BYT_COUNTS) into newly created rule
> when a new flow is added, and echo them back in the flow stats request.
>
> Notice that we remove send_flow_removed flag from struct rule/struct
> ofoperation
> and we replace it with an enum tracking all ofputil_flow_mod_flags.
>
> Signed-off-by: Daniel Baluta <[email protected]>
Seems reasonable, thanks. I applied this.
If you'll tell me a mailing address (off-list presumably) then I'll send
you one of the hackathon stickers in the mail.
I noticed a way to simplify it and make it less redundant, so I folded
this in:
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 0ca483c..d5f34d7 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -227,12 +227,20 @@ struct ofpbuf *ofputil_make_flow_mod_table_id(bool
flow_mod_table_id);
/* Protocol-independent flow_mod flags. */
enum ofputil_flow_mod_flags {
+ /* Flags that are maintained with a flow as part of its state.
+ *
+ * (OFPUTIL_FF_EMERG would be here too, if OVS supported it.) */
OFPUTIL_FF_SEND_FLOW_REM = 1 << 0, /* All versions. */
- OFPUTIL_FF_CHECK_OVERLAP = 1 << 1, /* All versions. */
- OFPUTIL_FF_EMERG = 1 << 2, /* OpenFlow 1.0 only. */
- OFPUTIL_FF_RESET_COUNTS = 1 << 3, /* OpenFlow 1.2+. */
- OFPUTIL_FF_NO_PKT_COUNTS = 1 << 4, /* OpenFlow 1.3+. */
- OFPUTIL_FF_NO_BYT_COUNTS = 1 << 5 /* OpenFlow 1.3+. */
+ OFPUTIL_FF_NO_PKT_COUNTS = 1 << 1, /* OpenFlow 1.3+. */
+ OFPUTIL_FF_NO_BYT_COUNTS = 1 << 2, /* OpenFlow 1.3+. */
+#define OFPUTIL_FF_STATE (OFPUTIL_FF_SEND_FLOW_REM \
+ | OFPUTIL_FF_NO_PKT_COUNTS \
+ | OFPUTIL_FF_NO_BYT_COUNTS)
+
+ /* Flags that affect flow_mod behavior but are not part of flow state. */
+ OFPUTIL_FF_CHECK_OVERLAP = 1 << 3, /* All versions. */
+ OFPUTIL_FF_EMERG = 1 << 4, /* OpenFlow 1.0 only. */
+ OFPUTIL_FF_RESET_COUNTS = 1 << 5, /* OpenFlow 1.2+. */
};
/* Protocol-independent flow_mod.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f718881..9605baa 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3155,16 +3155,7 @@ handle_flow_stats_request(struct ofconn *ofconn,
fs.hard_timeout = rule->hard_timeout;
ovs_mutex_unlock(&rule->timeout_mutex);
- fs.flags = 0;
- if (rule->flags & OFPUTIL_FF_SEND_FLOW_REM) {
- fs.flags |= OFPUTIL_FF_SEND_FLOW_REM;
- }
- if (rule->flags & OFPUTIL_FF_NO_PKT_COUNTS) {
- fs.flags |= OFPUTIL_FF_NO_PKT_COUNTS;
- }
- if (rule->flags & OFPUTIL_FF_NO_BYT_COUNTS) {
- fs.flags |= OFPUTIL_FF_NO_BYT_COUNTS;
- }
+ fs.flags = rule->flags;
ofputil_append_flow_stats_reply(&fs, &replies);
}
@@ -3581,16 +3572,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
ovs_mutex_unlock(&rule->timeout_mutex);
rule->table_id = table - ofproto->tables;
-
- if (fm->flags & OFPUTIL_FF_SEND_FLOW_REM) {
- rule->flags |= OFPUTIL_FF_SEND_FLOW_REM;
- }
- if (fm->flags & OFPUTIL_FF_NO_PKT_COUNTS) {
- rule->flags |= OFPUTIL_FF_NO_PKT_COUNTS;
- }
- if (fm->flags & OFPUTIL_FF_NO_BYT_COUNTS) {
- rule->flags |= OFPUTIL_FF_NO_BYT_COUNTS;
- }
+ rule->flags = fm->flags & OFPUTIL_FF_STATE;
rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
rule->ofpacts_len = fm->ofpacts_len;
@@ -3678,16 +3660,7 @@ modify_flows__(struct ofproto *ofproto, struct ofconn
*ofconn,
rule->hard_timeout = fm->hard_timeout;
ovs_mutex_unlock(&rule->timeout_mutex);
- if (fm->flags & OFPUTIL_FF_SEND_FLOW_REM) {
- rule->flags |= OFPUTIL_FF_SEND_FLOW_REM;
- }
- if (fm->flags & OFPUTIL_FF_NO_PKT_COUNTS) {
- rule->flags |= OFPUTIL_FF_NO_PKT_COUNTS;
- }
- if (fm->flags & OFPUTIL_FF_NO_BYT_COUNTS) {
- rule->flags |= OFPUTIL_FF_NO_BYT_COUNTS;
- }
-
+ rule->flags = fm->flags & OFPUTIL_FF_STATE;
if (fm->idle_timeout || fm->hard_timeout) {
if (!rule->eviction_group) {
eviction_group_add_rule(rule);
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
