[ovs-dev] [PATCH] ofp-util: Add "check_overlap" and "reset_counts" to stateful flags.

[Ben Pfaff]

The OpenFlow specification implies that every flag is part of the flow
state, even though that isn't really meaningful for OFPFF_CHECK_OVERLAP
or OFPFF_RESET_COUNTS.  This commit adds them to the flow state (reported
in flow stats replies).

Found by OFTest.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/ofp-util.h     | 20 ++++++++++++++------
 tests/ofproto.at   | 19 +++++++++++++++++++
 tests/ovs-ofctl.at |  2 +-
 3 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 8914342..f27f180 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -249,14 +249,22 @@ enum ofputil_flow_mod_flags {
     OFPUTIL_FF_SEND_FLOW_REM = 1 << 0, /* All versions. */
     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. */
+    /* These flags primarily affects flow_mod behavior.  They are not
+     * particularly useful as part of flow state.  We include them in flow
+     * state only because OpenFlow implies that they should be. */
     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+. */
+    OFPUTIL_FF_RESET_COUNTS  = 1 << 4, /* OpenFlow 1.2+. */
+
+    /* Not supported by OVS. */
+    OFPUTIL_FF_EMERG         = 1 << 5, /* OpenFlow 1.0 only. */
+
+    /* The set of flags maintained as part of a flow table entry. */
+#define OFPUTIL_FF_STATE (OFPUTIL_FF_SEND_FLOW_REM      \
+                          | OFPUTIL_FF_NO_PKT_COUNTS    \
+                          | OFPUTIL_FF_NO_BYT_COUNTS    \
+                          | OFPUTIL_FF_CHECK_OVERLAP    \
+                          | OFPUTIL_FF_RESET_COUNTS)
 
     /* Flags that are only set by OVS for its internal use.  Cannot be set via
      * OpenFlow. */
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 1ad80d3..065799a 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -833,6 +833,25 @@ AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | ofctl_strip], [0], [OFPST_FLO
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+# It's really dumb that check_overlap and reset_counts are considered
+# part of flow state, but OpenFlow implies that it is, and OFTest and
+# some users insist on it.
+AT_SETUP([ofproto - add-flow and flags])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl -F openflow10 add-flow br0 check_overlap,in_port=1,actions=drop])
+# Prior to OF1.4, flow dumps didn't include a "flags" field.
+AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | ofctl_strip], [0], [dnl
+OFPST_FLOW reply:
+ in_port=1 actions=drop
+])
+# OF1.4 makes the flags visible.
+AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip], [0], [dnl
+OFPST_FLOW reply (OF1.4):
+ check_overlap reset_counts in_port=1 actions=drop
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto - basic flow_mod commands (OpenFlow 1.1)])
 OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip], [0], [OFPST_FLOW reply (OF1.1):
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index c6f6bca..a851561 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -2770,7 +2770,7 @@ 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 | sed '/ST_FLOW reply/d' | sort], [0], [dnl
  importance=21, priority=21 actions=NORMAL
- priority=22 actions=NORMAL
+ reset_counts priority=22 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+
-- 
2.1.3