ofp-util: Clean up cookie handling.

Commit e72e793 (Add ability to restrict flow mods and flow stats
requests to cookies.) modified cookie handling.  Some of its behavior
was unintuitive and there was at least one bug (described below).
Commit f66b87d (DESIGN: Document uses for flow cookies.) attempted to
document a clean design for cookie handling.  This commit updates the
DESIGN document and brings the implementation in line with it.

In commit e72e793, the code that handled processing OpenFlow flow
modification requests set the cookie mask to exact-match.  This seems
reasonable for adding flows, but is not correct for matching, since
OpenFlow 1.0 doesn't support matching based on the cookie.  This commit
changes to cookie mask to fully wildcarded, which is the correct
behavior for modifications and deletions.  It doesn't cause any problems
for flow additions, since the mask is ignored for that operation.

Bug #9742

Reported-by: Luca Giraudo <lgiraudo@nicira.com>
Reported-by: Paul Ingram <paul@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>

1 files changed, 119 insertions, 5 deletions

diff --git a/tests/ofproto.at b/tests/ofproto.at
index 84868d71..5fc4968c 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -147,11 +147,11 @@ NXST_FLOW reply:
 AT_CHECK([ovs-ofctl dump-aggregate br0 table=0 | STRIP_XIDS], [0], [dnl
 NXST_AGGREGATE reply: packet_count=0 byte_count=0 flow_count=3
 ])
-AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x3 | ofctl_strip | sort], [0], [dnl
+AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x3/-1 | ofctl_strip | sort], [0], [dnl
  cookie=0x3, in_port=3 actions=output:0
 NXST_FLOW reply:
 ])
-AT_CHECK([ovs-ofctl dump-aggregate br0 cookie=0x3 | STRIP_XIDS], [0], [dnl
+AT_CHECK([ovs-ofctl dump-aggregate br0 cookie=0x3/-1 | STRIP_XIDS], [0], [dnl
 NXST_AGGREGATE reply: packet_count=0 byte_count=0 flow_count=1
 ])
 OVS_VSWITCHD_STOP
@@ -182,7 +182,121 @@ NXST_AGGREGATE reply: packet_count=0 byte_count=0 flow_count=2
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([ofproto - del flows with cookie])
+AT_SETUP([ofproto - mod flow with cookie change (OpenFlow 1.0)])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl -F openflow10 add-flow br0 cookie=0x1,in_port=1,actions=0])
+AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0x1, in_port=1 actions=output:0
+OFPST_FLOW reply:
+])
+
+AT_CHECK([ovs-ofctl -F openflow10 mod-flows br0 cookie=0x2,in_port=1,actions=0])
+AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0x2, in_port=1 actions=output:0
+OFPST_FLOW reply:
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - mod flow with cookie change (NXM)])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl -F nxm add-flow br0 cookie=0x1,in_port=1,actions=0])
+AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0x1, in_port=1 actions=output:0
+NXST_FLOW reply:
+])
+
+AT_CHECK([ovs-ofctl -F nxm mod-flows br0 cookie=0x2,in_port=1,actions=0])
+AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0x2, in_port=1 actions=output:0
+NXST_FLOW reply:
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - mod flows based on cookie mask])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
+AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=2,actions=0])
+AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=3,actions=0])
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0x1, in_port=1 actions=output:0
+ cookie=0x1, in_port=2 actions=output:0
+ cookie=0x2, in_port=3 actions=output:0
+NXST_FLOW reply:
+])
+
+AT_CHECK([ovs-ofctl -F nxm mod-flows br0 cookie=0x1/0xff,actions=4])
+AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0x1, in_port=1 actions=output:4
+ cookie=0x1, in_port=2 actions=output:4
+ cookie=0x2, in_port=3 actions=output:0
+NXST_FLOW reply:
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - mod flows based on cookie mask with cookie change])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
+AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=2,actions=0])
+AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=3,actions=0])
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0x1, in_port=1 actions=output:0
+ cookie=0x1, in_port=2 actions=output:0
+ cookie=0x2, in_port=3 actions=output:0
+NXST_FLOW reply:
+])
+
+AT_CHECK([ovs-ofctl -F nxm mod-flows br0 cookie=1/-1,cookie=4,actions=4])
+AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0x2, in_port=3 actions=output:0
+ cookie=0x4, in_port=1 actions=output:4
+ cookie=0x4, in_port=2 actions=output:4
+NXST_FLOW reply:
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - mod flow with cookie miss (mask==0)])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl -F nxm mod-flows br0 in_port=1,actions=0])
+AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ in_port=1 actions=output:0
+NXST_FLOW reply:
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - mod flow with cookie miss (mask!=0)])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl -F nxm mod-flows br0 cookie=1/1,in_port=1,actions=0])
+AT_CHECK([ovs-ofctl -F nxm dump-flows br0 | ofctl_strip | sort], [0], [dnl
+NXST_FLOW reply:
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - del flows with cookies])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
+AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=0])
+AT_CHECK([ovs-ofctl add-flow br0 cookie=0x3,in_port=3,actions=0])
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ cookie=0x1, in_port=1 actions=output:0
+ cookie=0x2, in_port=2 actions=output:0
+ cookie=0x3, in_port=3 actions=output:0
+NXST_FLOW reply:
+])
+
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+NXST_FLOW reply:
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - del flows based on cookie])
 OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
 AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=0])
@@ -194,7 +308,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
 NXST_FLOW reply:
 ])
 
-AT_CHECK([ovs-ofctl del-flows br0 cookie=0x3])
+AT_CHECK([ovs-ofctl del-flows br0 cookie=0x3/-1])
 AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
  cookie=0x1, in_port=1 actions=output:0
  cookie=0x2, in_port=2 actions=output:0
@@ -203,7 +317,7 @@ NXST_FLOW reply:
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([ofproto - del flows with cookie mask])
+AT_SETUP([ofproto - del flows based on cookie mask])
 OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl add-flow br0 cookie=0x1,in_port=1,actions=0])
 AT_CHECK([ovs-ofctl add-flow br0 cookie=0x2,in_port=2,actions=0])