ofp-util: Clean up cookie handling.
authorJustin Pettit <jpettit@nicira.com>
Sat, 24 Mar 2012 08:02:26 +0000 (01:02 -0700)
committerJustin Pettit <jpettit@nicira.com>
Tue, 29 May 2012 08:41:21 +0000 (01:41 -0700)
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>
12 files changed:
DESIGN
include/openflow/nicira-ext.h
lib/learn.c
lib/ofp-parse.c
lib/ofp-print.c
lib/ofp-util.c
lib/ofp-util.h
ofproto/ofproto-dpif.c
ofproto/ofproto.c
tests/ofproto.at
utilities/ovs-ofctl.8.in
utilities/ovs-ofctl.c

diff --git a/DESIGN b/DESIGN
index e59dc6eba94953d7ee07ade4167c648680130014..f9345d16e45485131001248256a18fc176902eeb 100644 (file)
--- a/DESIGN
+++ b/DESIGN
@@ -198,11 +198,16 @@ behavior with the following extensions:
           arbitrary masks.  This is much like the equivalent OpenFlow
           1.1 feature.
 
           arbitrary masks.  This is much like the equivalent OpenFlow
           1.1 feature.
 
-        - However, unlike OpenFlow 1.1, OFPC_MODIFY and
-          OFPFC_MODIFY_STRICT, regardless of whether there was a match
-          based on a cookie or not, always add a new flow if there is
-          no match, and they always update the cookies of flows that
-          they do match.
+        - 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
 
         - NXT_PACKET_IN (the Nicira extended version of
           OFPT_PACKET_IN) reports the cookie of the rule that
@@ -210,6 +215,21 @@ behavior with the following extensions:
           packet.  (Older versions of OVS used all-0-bits instead of
           all-1-bits.)
 
           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
 ======================
 
 Multiple Table Support
 ======================
index 62fc1031323dd467294d4b64fde6cfa2fd4fda57..21888a9d29070f61b8c43854bec2453993c78f83 100644 (file)
@@ -1777,10 +1777,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
 /* 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;
  */
 struct nx_flow_mod {
     struct nicira_header nxh;
index e995c29cf9f35647a7e871f95f86c98abd2b11a9..ba33287a12d6092e1acbe811ad007ac620e863a8 100644 (file)
@@ -204,8 +204,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));
     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);
     fm->table_id = learn->table_id;
     fm->command = OFPFC_MODIFY_STRICT;
     fm->idle_timeout = ntohs(learn->idle_timeout);
index fc86442f7e24bca913fd06c845e64b23e97f14af..73a70c60c053ff13885f53ce54f5e9c8258345fd 100644 (file)
@@ -592,6 +592,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);
     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;
     fm->table_id = 0xff;
     fm->command = command;
     fm->idle_timeout = OFP_FLOW_PERMANENT;
@@ -646,17 +652,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, '/');
                 fm->hard_timeout = str_to_u16(value, name);
             } else if (!strcmp(name, "cookie")) {
                 char *mask = strchr(value, '/');
+
                 if (mask) {
                 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';
                     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(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")
             } else if (mf_from_name(name)) {
                 parse_field(mf_from_name(name), value, &fm->cr);
             } else if (!strcmp(name, "duration")
@@ -670,6 +683,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;
 
     if (fields & F_ACTIONS) {
         struct ofpbuf actions;
 
index 1757a30c3be522c69f1d1bef60123e90da51c619..9d4396cf4f67f3e6e22334c87901f8faf0fea787 100644 (file)
@@ -977,8 +977,12 @@ ofp_print_flow_mod(struct ds *s, const struct ofp_header *oh,
     if (ds_last(s) != ' ') {
         ds_put_char(s, ' ');
     }
     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);
     }
     if (fm.idle_timeout != OFP_FLOW_PERMANENT) {
         ds_put_format(s, "idle:%"PRIu16" ", fm.idle_timeout);
index 90124ec86af0104bda224608d142f7ab22c44423..c326b727abfdd8d2267eee592a10bea131b7c8a8 100644 (file)
@@ -1402,9 +1402,10 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
         ofputil_normalize_rule(&fm->cr);
 
         /* Translate the message. */
         ofputil_normalize_rule(&fm->cr);
 
         /* Translate the message. */
-        fm->cookie = ofm->cookie;
-        fm->cookie_mask = htonll(UINT64_MAX);
         command = ntohs(ofm->command);
         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);
         fm->idle_timeout = ntohs(ofm->idle_timeout);
         fm->hard_timeout = ntohs(ofm->hard_timeout);
         fm->buffer_id = ntohl(ofm->buffer_id);
@@ -1429,17 +1430,12 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
 
         /* Translate the message. */
         command = ntohs(nfm->command);
 
         /* 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 OFPERR_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 OFPERR_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);
         fm->idle_timeout = ntohs(nfm->idle_timeout);
         fm->hard_timeout = ntohs(nfm->hard_timeout);
         fm->buffer_id = ntohl(nfm->buffer_id);
@@ -1483,7 +1479,7 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm,
         msg = ofpbuf_new(sizeof *ofm + actions_len);
         ofm = put_openflow(sizeof *ofm, OFPT10_FLOW_MOD, msg);
         ofputil_cls_rule_to_match(&fm->cr, &ofm->match);
         msg = ofpbuf_new(sizeof *ofm + actions_len);
         ofm = put_openflow(sizeof *ofm, OFPT10_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);
         ofm->command = htons(command);
         ofm->idle_timeout = htons(fm->idle_timeout);
         ofm->hard_timeout = htons(fm->hard_timeout);
@@ -1499,14 +1495,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);
         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);
         nfm->idle_timeout = htons(fm->idle_timeout);
         nfm->hard_timeout = htons(fm->hard_timeout);
         nfm->priority = htons(fm->cr.priority);
@@ -1545,7 +1535,9 @@ ofputil_flow_mod_usable_protocols(const struct ofputil_flow_mod *fms,
         if (fm->table_id != 0xff) {
             usable_protocols &= OFPUTIL_P_TID;
         }
         if (fm->table_id != 0xff) {
             usable_protocols &= OFPUTIL_P_TID;
         }
-        if (fm->command != OFPFC_ADD && fm->cookie_mask != htonll(0)) {
+
+        /* Matching of the cookie is only supported through NXM. */
+        if (fm->cookie_mask != htonll(0)) {
             usable_protocols &= OFPUTIL_P_NXM_ANY;
         }
     }
             usable_protocols &= OFPUTIL_P_NXM_ANY;
         }
     }
index e6716636a9150409e8347875a88c5a14999ca1b7..3aa1e0952df6b77f3d291f1c9364d1de0ae5bd5e 100644 (file)
@@ -197,11 +197,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);
 
 /* NXT_FLOW_MOD_TABLE_ID extension. */
 struct ofpbuf *ofputil_make_flow_mod_table_id(bool flow_mod_table_id);
 
-/* Protocol-independent flow_mod. */
+/* Protocol-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;
 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;
     uint8_t table_id;
     uint16_t command;
     uint16_t idle_timeout;
index f2c2ca9655050444e946f3947ca0f57353ab306a..e3efed7d2c4755e13821423e5bdd2683ce0cae92 100644 (file)
@@ -791,6 +791,7 @@ add_internal_flow(struct ofproto_dpif *ofproto, int id,
 
     cls_rule_init_catchall(&fm.cr, 0);
     cls_rule_set_reg(&fm.cr, 0, id);
 
     cls_rule_init_catchall(&fm.cr, 0);
     cls_rule_set_reg(&fm.cr, 0, id);
+    fm.new_cookie = htonll(0);
     fm.cookie = htonll(0);
     fm.cookie_mask = htonll(0);
     fm.table_id = TBL_INTERNAL;
     fm.cookie = htonll(0);
     fm.cookie_mask = htonll(0);
     fm.table_id = TBL_INTERNAL;
index 0bda06ac80d6061b6b0a19c2398022ee5fd6e2f1..0c24314f9c4a3a14c7a03c45f4b3efd59e69e482 100644 (file)
@@ -2785,7 +2785,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     rule->ofproto = ofproto;
     rule->cr = fm->cr;
     rule->pending = NULL;
     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 = rule->used = time_msec();
     rule->idle_timeout = fm->idle_timeout;
     rule->hard_timeout = fm->hard_timeout;
     rule->created = rule->modified = rule->used = time_msec();
     rule->idle_timeout = fm->idle_timeout;
     rule->hard_timeout = fm->hard_timeout;
@@ -2885,7 +2885,9 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
         } else {
             rule->modified = time_msec();
         }
         } 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);
 
     }
     ofopgroup_submit(group);
 
@@ -2908,9 +2910,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);
     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
 }
 
 /* Implements OFPFC_MODIFY_STRICT.  Returns 0 on success or an OpenFlow error
@@ -2929,11 +2935,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);
     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;
+    }
 }
 \f
 /* OFPFC_DELETE implementation. */
 }
 \f
 /* OFPFC_DELETE implementation. */
index 98942e58b9ec3a144564cdb9bb827328839f7650..dbe31c4c1a66326c46af9b27165c22befad4c6ba 100644 (file)
@@ -154,11 +154,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-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:
 ])
  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
 NXST_AGGREGATE reply: packet_count=0 byte_count=0 flow_count=1
 ])
 OVS_VSWITCHD_STOP
@@ -189,7 +189,121 @@ NXST_AGGREGATE reply: packet_count=0 byte_count=0 flow_count=2
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
 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])
 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])
@@ -201,7 +315,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
 NXST_FLOW reply:
 ])
 
 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
 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
@@ -210,7 +324,7 @@ NXST_FLOW reply:
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
 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])
 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])
index a9398e380ec2dfd858b5df3d7fc67429d1e3ff2a..67cc3e95f3a34597957f96fe428fb81f0bce11dd 100644 (file)
@@ -1094,26 +1094,42 @@ 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:
 .
 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,
 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
 the \fBadd\-flow\fR and \fBadd\-flows\fR commands.  For
 \fBmod\-flows\fR and \fBdel\-flows\fR when \fB\-\-strict\fR is
 specified, priority must match along with the rest of the flow
 .
 .PP
 The following additional field sets the priority for flows added by
 the \fBadd\-flow\fR and \fBadd\-flows\fR commands.  For
 \fBmod\-flows\fR and \fBdel\-flows\fR when \fB\-\-strict\fR is
 specified, priority must match along with the rest of the flow
-specification.  For \fBmod\-flows\fR without \fB\-\-strict\fR,
+specification.  For \fBmod-flows\fR without \fB\-\-strict\fR,
 priority is only significant if the command creates a new flow, that
 is, non-strict \fBmod\-flows\fR does not match on priority and will
 not change the priority of existing flows.  Other commands do not
 priority is only significant if the command creates a new flow, that
 is, non-strict \fBmod\-flows\fR does not match on priority and will
 not change the priority of existing flows.  Other commands do not
index 834dff54fa6cbdb571f8171b7d17ab537a71ee5b..f20727227eccef0a54cb006b92e6e9c363d2557d 100644 (file)
@@ -1613,7 +1613,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);
         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);
         version->idle_timeout = fm.idle_timeout;
         version->hard_timeout = fm.hard_timeout;
         version->flags = fm.flags & (OFPFF_SEND_FLOW_REM | OFPFF_EMERG);
@@ -1722,7 +1722,9 @@ fte_make_flow_mod(const struct fte *fte, int index, uint16_t command,
     struct ofpbuf *ofm;
 
     fm.cr = fte->rule;
     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;
     fm.table_id = 0xff;
     fm.command = command;
     fm.idle_timeout = version->idle_timeout;