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>

11 files changed, 321 insertions, 62 deletions

diff --git a/DESIGN b/DESIGN
index c53a8ea3..fe7a1371 100644
--- a/DESIGN
+++ b/DESIGN
@@ -20,6 +20,91 @@ to 'internal' ports whose port numbers are less than OFPP_MAX, we interpret
 OFPP_LOCAL as a physical port and support OFPAT_ENQUEUE on it as well.
 
 
+Flow Cookies
+============
+
+OpenFlow 1.0 and later versions have the concept of a "flow cookie",
+which is a 64-bit integer value attached to each flow.  The treatment
+of the flow cookie has varied greatly across OpenFlow versions,
+however.
+
+In OpenFlow 1.0:
+
+        - OFPFC_ADD set the cookie in the flow that it added.
+
+        - OFPFC_MODIFY and OFPFC_MODIFY_STRICT updated the cookie for
+          the flow or flows that it modified.
+
+        - OFPST_FLOW messages included the flow cookie.
+
+        - OFPT_FLOW_REMOVED messages reported the cookie of the flow
+          that was removed.
+
+OpenFlow 1.1 made the following changes:
+
+        - Flow mod operations OFPFC_MODIFY, OFPFC_MODIFY_STRICT,
+          OFPFC_DELETE, and OFPFC_DELETE_STRICT, plus flow stats
+          requests and aggregate stats requests, gained the ability to
+          match on flow cookies with an arbitrary mask.
+
+        - OFPFC_MODIFY and OFPFC_MODIFY_STRICT were changed to add a
+          new flow, in the case of no match, only if the flow table
+          modification operation did not match on the cookie field.
+          (In OpenFlow 1.0, modify operations always added a new flow
+          when there was no match.)
+
+        - OFPFC_MODIFY and OFPFC_MODIFY_STRICT no longer updated flow
+          cookies.
+
+OpenFlow 1.2 made the following changes:
+
+        - OFPC_MODIFY and OFPFC_MODIFY_STRICT were changed to never
+          add a new flow, regardless of whether the flow cookie was
+          used for matching.
+
+Open vSwitch support for OpenFlow 1.0 implements the OpenFlow 1.0
+behavior with the following extensions:
+
+        - An NXM extension field NXM_NX_COOKIE(_W) allows the NXM
+          versions of OFPFC_MODIFY, OFPFC_MODIFY_STRICT, OFPFC_DELETE,
+          and OFPFC_DELETE_STRICT flow_mods, plus flow stats requests
+          and aggregate stats requests, to match on flow cookies with
+          arbitrary masks.  This is much like the equivalent OpenFlow
+          1.1 feature.
+
+        - Like OpenFlow 1.1, OFPC_MODIFY and OFPFC_MODIFY_STRICT add a
+          new flow if there is no match and the mask is zero (or not
+          given).
+
+        - The "cookie" field in OFPT_FLOW_MOD and NXT_FLOW_MOD messages
+          is used as the cookie value for OFPFC_ADD commands, as
+          described in OpenFlow 1.0.  For OFPFC_MODIFY and
+          OFPFC_MODIFY_STRICT commands, the "cookie" field is used as a
+          new cookie for flows that match unless it is UINT64_MAX, in
+          which case the flow's cookie is not updated.
+
+        - NXT_PACKET_IN (the Nicira extended version of
+          OFPT_PACKET_IN) reports the cookie of the rule that
+          generated the packet, or all-1-bits if no rule generated the
+          packet.  (Older versions of OVS used all-0-bits instead of
+          all-1-bits.)
+
+The following table shows the handling of different protocols when
+receiving OFPFC_MODIFY and OFPFC_MODIFY_STRICT messages.  A mask of 0
+indicates either an explicit mask of zero or an implicit one by not
+specifying the NXM_NX_COOKIE(_W) field.
+
+                Match   Update   Add on miss   Add on miss
+                cookie  cookie     mask!=0       mask==0
+                ======  ======   ===========   ===========
+OpenFlow 1.0      no     yes        <always add on miss>
+OpenFlow 1.1     yes      no          no           yes
+OpenFlow 1.2     yes      no          no            no
+NXM              yes     yes*         no           yes
+
+* Updates the flow's cookie unless the "cookie" field is UINT64_MAX.
+
+
 Multiple Table Support
 ======================
 
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index f00f9940..160a6349 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -1743,10 +1743,8 @@ OFP_ASSERT(sizeof(struct nx_set_flow_format) == 20);
 /* NXT_FLOW_MOD (analogous to OFPT_FLOW_MOD).
  *
  * It is possible to limit flow deletions and modifications to certain
- * cookies by using the NXM_NX_COOKIE and NXM_NX_COOKIE_W matches.  For
- * these commands, the "cookie" field is always ignored.  Flow additions
- * make use of the "cookie" field and ignore any NXM_NX_COOKIE*
- * definitions.
+ * cookies by using the NXM_NX_COOKIE(_W) matches.  The "cookie" field
+ * is used only to add or modify flow cookies.
  */
 struct nx_flow_mod {
     struct nicira_header nxh;
diff --git a/lib/learn.c b/lib/learn.c
index 284a6cdd..849ea253 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -189,8 +189,9 @@ learn_execute(const struct nx_action_learn *learn, const struct flow *flow,
     struct ofpbuf actions;
 
     cls_rule_init_catchall(&fm->cr, ntohs(learn->priority));
-    fm->cookie = learn->cookie;
-    fm->cookie_mask = htonll(UINT64_MAX);
+    fm->cookie = htonll(0);
+    fm->cookie_mask = htonll(0);
+    fm->new_cookie = learn->cookie;
     fm->table_id = learn->table_id;
     fm->command = OFPFC_MODIFY_STRICT;
     fm->idle_timeout = ntohs(learn->idle_timeout);
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 38c3dabd..6843945a 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -528,6 +528,12 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
     cls_rule_init_catchall(&fm->cr, OFP_DEFAULT_PRIORITY);
     fm->cookie = htonll(0);
     fm->cookie_mask = htonll(0);
+    if (command == OFPFC_MODIFY || command == OFPFC_MODIFY_STRICT) {
+        /* For modify, by default, don't update the cookie. */
+        fm->new_cookie = htonll(UINT64_MAX);
+    } else{
+        fm->new_cookie = htonll(0);
+    }
     fm->table_id = 0xff;
     fm->command = command;
     fm->idle_timeout = OFP_FLOW_PERMANENT;
@@ -578,17 +584,24 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
                 fm->hard_timeout = str_to_u16(value, name);
             } else if (!strcmp(name, "cookie")) {
                 char *mask = strchr(value, '/');
+
                 if (mask) {
+                    /* A mask means we're searching for a cookie. */
                     if (command == OFPFC_ADD) {
                         ofp_fatal(str_, verbose, "flow additions cannot use "
                                   "a cookie mask");
                     }
                     *mask = '\0';
+                    fm->cookie = htonll(str_to_u64(value));
                     fm->cookie_mask = htonll(str_to_u64(mask+1));
                 } else {
-                    fm->cookie_mask = htonll(UINT64_MAX);
+                    /* No mask means that the cookie is being set. */
+                    if (command != OFPFC_ADD && command != OFPFC_MODIFY
+                            && command != OFPFC_MODIFY_STRICT) {
+                        ofp_fatal(str_, verbose, "cannot set cookie");
+                    }
+                    fm->new_cookie = htonll(str_to_u64(value));
                 }
-                fm->cookie = htonll(str_to_u64(value));
             } else if (mf_from_name(name)) {
                 parse_field(mf_from_name(name), value, &fm->cr);
             } else if (!strcmp(name, "duration")
@@ -602,6 +615,13 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
             }
         }
     }
+    if (!fm->cookie_mask && fm->new_cookie == htonll(UINT64_MAX)
+            && (command == OFPFC_MODIFY || command == OFPFC_MODIFY_STRICT)) {
+        /* On modifies without a mask, we are supposed to add a flow if
+         * one does not exist.  If a cookie wasn't been specified, use a
+         * default of zero. */
+        fm->new_cookie = htonll(0);
+    }
     if (fields & F_ACTIONS) {
         struct ofpbuf actions;
 
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 89267d87..b35ac681 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -806,8 +806,12 @@ ofp_print_flow_mod(struct ds *s, const struct ofp_header *oh,
     if (ds_last(s) != ' ') {
         ds_put_char(s, ' ');
     }
-    if (fm.cookie != htonll(0)) {
-        ds_put_format(s, "cookie:0x%"PRIx64" ", ntohll(fm.cookie));
+    if (fm.new_cookie != htonll(0)) {
+        ds_put_format(s, "cookie:0x%"PRIx64" ", ntohll(fm.new_cookie));
+    }
+    if (fm.cookie_mask != htonll(0)) {
+        ds_put_format(s, "cookie:0x%"PRIx64"/0x%"PRIx64" ",
+                ntohll(fm.cookie), ntohll(fm.cookie_mask));
     }
     if (fm.idle_timeout != OFP_FLOW_PERMANENT) {
         ds_put_format(s, "idle:%"PRIu16" ", fm.idle_timeout);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index a00939dc..fac85d64 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1043,9 +1043,10 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
         ofputil_normalize_rule(&fm->cr, NXFF_OPENFLOW10);
 
         /* Translate the message. */
-        fm->cookie = ofm->cookie;
-        fm->cookie_mask = htonll(UINT64_MAX);
         command = ntohs(ofm->command);
+        fm->cookie = htonll(0);
+        fm->cookie_mask = htonll(0);
+        fm->new_cookie = ofm->cookie;
         fm->idle_timeout = ntohs(ofm->idle_timeout);
         fm->hard_timeout = ntohs(ofm->hard_timeout);
         fm->buffer_id = ntohl(ofm->buffer_id);
@@ -1070,17 +1071,12 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
 
         /* Translate the message. */
         command = ntohs(nfm->command);
-        if (command == OFPFC_ADD) {
-            if (fm->cookie_mask) {
-                /* The "NXM_NX_COOKIE*" matches are not valid for flow
-                 * additions.  Additions must use the "cookie" field of
-                 * the "nx_flow_mod" structure. */
-                return ofp_mkerr(OFPET_BAD_REQUEST, NXBRC_NXM_INVALID);
-            } else {
-                fm->cookie = nfm->cookie;
-                fm->cookie_mask = htonll(UINT64_MAX);
-            }
+        if ((command & 0xff) == OFPFC_ADD && fm->cookie_mask) {
+            /* Flow additions may only set a new cookie, not match an
+             * existing cookie. */
+            return NXBRC_NXM_INVALID;
         }
+        fm->new_cookie = nfm->cookie;
         fm->idle_timeout = ntohs(nfm->idle_timeout);
         fm->hard_timeout = ntohs(nfm->hard_timeout);
         fm->buffer_id = ntohl(nfm->buffer_id);
@@ -1125,7 +1121,7 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm,
         msg = ofpbuf_new(sizeof *ofm + actions_len);
         ofm = put_openflow(sizeof *ofm, OFPT_FLOW_MOD, msg);
         ofputil_cls_rule_to_match(&fm->cr, &ofm->match);
-        ofm->cookie = fm->cookie;
+        ofm->cookie = fm->new_cookie;
         ofm->command = htons(command);
         ofm->idle_timeout = htons(fm->idle_timeout);
         ofm->hard_timeout = htons(fm->hard_timeout);
@@ -1141,14 +1137,8 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm,
         put_nxmsg(sizeof *nfm, NXT_FLOW_MOD, msg);
         nfm = msg->data;
         nfm->command = htons(command);
-        if (command == OFPFC_ADD) {
-            nfm->cookie = fm->cookie;
-            match_len = nx_put_match(msg, &fm->cr, 0, 0);
-        } else {
-            nfm->cookie = 0;
-            match_len = nx_put_match(msg, &fm->cr,
-                                     fm->cookie, fm->cookie_mask);
-        }
+        nfm->cookie = fm->new_cookie;
+        match_len = nx_put_match(msg, &fm->cr, fm->cookie, fm->cookie_mask);
         nfm->idle_timeout = htons(fm->idle_timeout);
         nfm->hard_timeout = htons(fm->hard_timeout);
         nfm->priority = htons(fm->cr.priority);
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 971331a5..44fe7854 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -135,11 +135,29 @@ struct ofpbuf *ofputil_make_set_packet_in_format(enum nx_packet_in_format);
 /* NXT_FLOW_MOD_TABLE_ID extension. */
 struct ofpbuf *ofputil_make_flow_mod_table_id(bool flow_mod_table_id);
 
-/* Flow format independent flow_mod. */
+/* Flow format independent flow_mod.
+ *
+ * The handling of cookies across multiple versions of OpenFlow is a bit
+ * confusing.  A full description of Open vSwitch's cookie handling is
+ * in the DESIGN file.  The following table shows the expected values of
+ * the cookie-related fields for the different flow_mod commands in
+ * OpenFlow 1.0 ("OF10") and NXM.  "<used>" and "-" indicate a value
+ * that may be populated and an ignored field, respectively.
+ *
+ *               cookie  cookie_mask  new_cookie
+ *               ======  ===========  ==========
+ * OF10 Add        -          0         <used>
+ * OF10 Modify     -          0         <used>
+ * OF10 Delete     -          0           -
+ * NXM Add         -          0         <used>
+ * NXM Modify    <used>     <used>      <used>
+ * NXM Delete    <used>     <used>        -
+ */
 struct ofputil_flow_mod {
     struct cls_rule cr;
-    ovs_be64 cookie;
-    ovs_be64 cookie_mask;
+    ovs_be64 cookie;         /* Cookie bits to match. */
+    ovs_be64 cookie_mask;    /* 1-bit in each 'cookie' bit to match. */
+    ovs_be64 new_cookie;     /* New cookie to install or -1. */
     uint8_t table_id;
     uint16_t command;
     uint16_t idle_timeout;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index bdf82a18..2cf57c52 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2537,7 +2537,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     rule->ofproto = ofproto;
     rule->cr = fm->cr;
     rule->pending = NULL;
-    rule->flow_cookie = fm->cookie;
+    rule->flow_cookie = fm->new_cookie;
     rule->created = rule->modified = time_msec();
     rule->idle_timeout = fm->idle_timeout;
     rule->hard_timeout = fm->hard_timeout;
@@ -2604,7 +2604,9 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
         } else {
             rule->modified = time_msec();
         }
-        rule->flow_cookie = fm->cookie;
+        if (fm->new_cookie != htonll(UINT64_MAX)) {
+            rule->flow_cookie = fm->new_cookie;
+        }
     }
     ofopgroup_submit(group);
 
@@ -2627,9 +2629,13 @@ modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
     error = collect_rules_loose(ofproto, fm->table_id, &fm->cr,
                                 fm->cookie, fm->cookie_mask,
                                 OFPP_NONE, &rules);
-    return (error ? error
-            : list_is_empty(&rules) ? add_flow(ofproto, ofconn, fm, request)
-            : modify_flows__(ofproto, ofconn, fm, request, &rules));
+    if (error) {
+        return error;
+    } else if (list_is_empty(&rules)) {
+        return fm->cookie_mask ? 0 : add_flow(ofproto, ofconn, fm, request);
+    } else {
+        return modify_flows__(ofproto, ofconn, fm, request, &rules);
+    }
 }
 
 /* Implements OFPFC_MODIFY_STRICT.  Returns 0 on success or an OpenFlow error
@@ -2648,11 +2654,16 @@ modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
     error = collect_rules_strict(ofproto, fm->table_id, &fm->cr,
                                  fm->cookie, fm->cookie_mask,
                                  OFPP_NONE, &rules);
-    return (error ? error
-            : list_is_empty(&rules) ? add_flow(ofproto, ofconn, fm, request)
-            : list_is_singleton(&rules) ? modify_flows__(ofproto, ofconn,
-                                                         fm, request, &rules)
-            : 0);
+
+    if (error) {
+        return error;
+    } else if (list_is_empty(&rules)) {
+        return fm->cookie_mask ? 0 : add_flow(ofproto, ofconn, fm, request);
+    } else {
+        return list_is_singleton(&rules) ? modify_flows__(ofproto, ofconn,
+                                                          fm, request, &rules)
+                                         : 0;
+    }
 }
 
 /* OFPFC_DELETE implementation. */
diff --git a/tests/ofproto.at b/tests/ofproto.at
index b54d1dd1..cfa0d091 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -128,11 +128,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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+AT_CHECK([ovs-ofctl dump-flows br0 cookie=0x3/-1 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
  cookie=0x3, duration=?s, table=0, n_packets=0, n_bytes=0, 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
@@ -163,7 +163,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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x2, duration=?s, table=0, n_packets=0, n_bytes=0, 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x2, duration=?s, table=0, n_packets=0, n_bytes=0, 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=1 actions=output:0
+ cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=2 actions=output:0
+ cookie=0x2, duration=?s, table=0, n_packets=0, n_bytes=0, 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=1 actions=output:4
+ cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=2 actions=output:4
+ cookie=0x2, duration=?s, table=0, n_packets=0, n_bytes=0, 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=1 actions=output:0
+ cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=2 actions=output:0
+ cookie=0x2, duration=?s, table=0, n_packets=0, n_bytes=0, 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x2, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=3 actions=output:0
+ cookie=0x4, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=1 actions=output:4
+ cookie=0x4, duration=?s, table=0, n_packets=0, n_bytes=0, 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x0, duration=?s, table=0, n_packets=0, n_bytes=0, 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 | STRIP_XIDS | STRIP_DURATION | 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
+ cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=1 actions=output:0
+ cookie=0x2, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=2 actions=output:0
+ cookie=0x3, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=3 actions=output:0
+NXST_FLOW reply:
+])
+
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS | STRIP_DURATION | 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])
@@ -174,7 +288,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS | STRIP_DURATION | sort], [0], [
  cookie=0x3, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=3 actions=output:0
 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 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl
  cookie=0x1, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=1 actions=output:0
  cookie=0x2, duration=?s, table=0, n_packets=0, n_bytes=0, in_port=2 actions=output:0
@@ -183,7 +297,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])
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index cacf0390..02401813 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -936,19 +936,35 @@ levels of the \fBresubmit\fR call stack, are ignored.
 An opaque identifier called a cookie can be used as a handle to identify
 a set of flows:
 .
-.IP \fBcookie=\fIvalue\fR[\fB/\fImask\fR]
+.IP \fBcookie=\fIvalue\fR
+.
+A cookie can be associated with a flow using the \fBadd\-flow\fR,
+\fBadd\-flows\fR, and \fBmod\-flows\fR commands.  \fIvalue\fR can be any
+64-bit number and need not be unique among flows.  If this field is
+omitted, a default cookie value of 0 is used.
+.
+.IP \fBcookie=\fIvalue\fR\fB/\fImask\fR
 .
-A cookie can be associated with a flow using the \fBadd-flow\fR and
-\fBadd-flows\fR commands.  \fIvalue\fR can be any 64-bit number and need
-not be unique among flows.  If this field is omitted, a default cookie
-value of 0 is used.
-.IP
 When using NXM, the cookie can be used as a handle for querying,
-modifying, and deleting flows.  In addition to \fIvalue\fR, an optional
-\fImask\fR may be supplied for the \fBdel-flows\fR, \fBmod-flows\fR,
-\fBdump-flows\fR, and \fBdump-aggregate\fR commands to limit matching
-cookies.  A 1-bit in \fImask\fR indicates that the corresponding bit in
-\fIcookie\fR must match exactly, and a 0-bit wildcards that bit.
+modifying, and deleting flows.  \fIvalue\fR and \fImask\fR may be
+supplied for the \fBdel\-flows\fR, \fBmod\-flows\fR, \fBdump\-flows\fR, and
+\fBdump\-aggregate\fR commands to limit matching cookies.  A 1-bit in
+\fImask\fR indicates that the corresponding bit in \fIcookie\fR must
+match exactly, and a 0-bit wildcards that bit.  A mask of \-1 may be used
+to exactly match a cookie.
+.IP
+The \fBmod\-flows\fR command can update the cookies of flows that
+match a cookie by specifying the \fIcookie\fR field twice (once with a
+mask for matching and once without to indicate the new value):
+.RS
+.IP "\fBovs\-ofctl mod\-flows br0 cookie=1,actions=normal\fR"
+Change all flows' cookies to 1 and change their actions to \fBnormal\fR.
+.IP "\fBovs\-ofctl mod\-flows br0 cookie=1/\-1,cookie=2,actions=normal\fR"
+Update cookies with a value of 1 to 2 and change their actions to
+\fBnormal\fR.
+.RE
+.IP
+The ability to match on cookies was added in Open vSwitch 1.5.0.
 .
 .PP
 The following additional field sets the priority for flows added by
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index d9d1d193..8c80fbb4 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1244,7 +1244,7 @@ read_flows_from_file(const char *filename, struct classifier *cls, int index)
         parse_ofp_str(&fm, OFPFC_ADD, ds_cstr(&s), true);
 
         version = xmalloc(sizeof *version);
-        version->cookie = fm.cookie;
+        version->cookie = fm.new_cookie;
         version->idle_timeout = fm.idle_timeout;
         version->hard_timeout = fm.hard_timeout;
         version->flags = fm.flags & (OFPFF_SEND_FLOW_REM | OFPFF_EMERG);
@@ -1354,7 +1354,9 @@ fte_make_flow_mod(const struct fte *fte, int index, uint16_t command,
     struct ofpbuf *ofm;
 
     fm.cr = fte->rule;
-    fm.cookie = version->cookie;
+    fm.cookie = htonll(0);
+    fm.cookie_mask = htonll(0);
+    fm.new_cookie = version->cookie;
     fm.table_id = 0xff;
     fm.command = command;
     fm.idle_timeout = version->idle_timeout;