ofp-util: Make ofputil_cls_rule_to_match() help with flow cookies too.

This fixes OpenFlow 1.0 flow stats reporting of flows added via NXM.

I noticed this problem while implementing 64-bit tunnel IDs, hence the
positioning.  The following commit adds a test.

Acked-by: Jesse Gross <jesse@nicira.com>

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index cdbbb0dc97f6411a961f862661306dbf9a2def38..1cafd862bac2cb9b8ac2641f105820a57cf48ca0 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -200,16 +200,21 @@ ofputil_cls_rule_from_match(const struct ofp_match *match,
     cls_rule_zero_wildcarded_fields(rule);
 }
 
-/* Extract 'flow' with 'wildcards' into the OpenFlow match structure
- * 'match'.
+/* Convert 'rule' into the OpenFlow match structure 'match'.  'flow_format'
+ * must either NXFF_OPENFLOW10 or NXFF_TUN_ID_FROM_COOKIE.
  *
- * 'flow_format' must either NXFF_OPENFLOW10 or NXFF_TUN_ID_FROM_COOKIE.  In
- * the latter case only, 'match''s NXFW_TUN_ID bit will be filled in; otherwise
- * it is always set to 0. */
+ * The NXFF_TUN_ID_FROM_COOKIE flow format requires modifying the flow cookie.
+ * This function can help with that, if 'cookie_out' is nonnull.  For
+ * NXFF_OPENFLOW10, or if the tunnel ID is wildcarded, 'cookie_in' will be
+ * copied directly to '*cookie_out'.  For NXFF_TUN_ID_FROM_COOKIE when tunnel
+ * ID is matched, 'cookie_in' will be modified appropriately before setting
+ * '*cookie_out'.
+ */
 void
 ofputil_cls_rule_to_match(const struct cls_rule *rule,
                           enum nx_flow_format flow_format,
-                          struct ofp_match *match)
+                          struct ofp_match *match,
+                          ovs_be64 cookie_in, ovs_be64 *cookie_out)
 {
     const struct flow_wildcards *wc = &rule->wc;
     unsigned int ofpfw;
@@ -221,8 +226,19 @@ ofputil_cls_rule_to_match(const struct cls_rule *rule,
     if (wc->wildcards & FWW_NW_TOS) {
         ofpfw |= OFPFW_NW_TOS;
     }
-    if (flow_format == NXFF_TUN_ID_FROM_COOKIE && wc->wildcards & FWW_TUN_ID) {
-        ofpfw |= NXFW_TUN_ID;
+
+    /* Tunnel ID. */
+    if (flow_format == NXFF_TUN_ID_FROM_COOKIE) {
+        if (wc->wildcards & FWW_TUN_ID) {
+            ofpfw |= NXFW_TUN_ID;
+        } else {
+            uint32_t cookie_lo = ntohll(cookie_in);
+            uint32_t cookie_hi = ntohl(rule->flow.tun_id);
+            cookie_in = htonll(cookie_lo | ((uint64_t) cookie_hi << 32));
+        }
+    }
+    if (cookie_out) {
+        *cookie_out = cookie_in;
     }
 
     /* Translate VLANs. */
@@ -979,15 +995,8 @@ ofputil_encode_flow_mod(const struct 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, flow_format, &ofm->match);
-        if (flow_format != NXFF_TUN_ID_FROM_COOKIE
-            || fm->cr.wc.wildcards & FWW_TUN_ID) {
-            ofm->cookie = fm->cookie;
-        } else {
-            uint32_t cookie_lo = ntohll(fm->cookie);
-            uint32_t cookie_hi = ntohl(fm->cr.flow.tun_id);
-            ofm->cookie = htonll(cookie_lo | ((uint64_t) cookie_hi << 32));
-        }
+        ofputil_cls_rule_to_match(&fm->cr, flow_format, &ofm->match,
+                                  fm->cookie, &ofm->cookie);
         ofm->command = htons(fm->command);
         ofm->idle_timeout = htons(fm->idle_timeout);
         ofm->hard_timeout = htons(fm->hard_timeout);
@@ -1124,7 +1133,8 @@ ofputil_encode_flow_stats_request(const struct flow_stats_request *fsr,
 
         type = fsr->aggregate ? OFPST_AGGREGATE : OFPST_FLOW;
         ofsr = ofputil_make_stats_request(sizeof *ofsr, type, &msg);
-        ofputil_cls_rule_to_match(&fsr->match, flow_format, &ofsr->match);
+        ofputil_cls_rule_to_match(&fsr->match, flow_format, &ofsr->match,
+                                  0, NULL);
         ofsr->table_id = fsr->table_id;
         ofsr->out_port = htons(fsr->out_port);
     } else if (flow_format == NXFF_NXM) {
@@ -1427,7 +1437,7 @@ make_flow_mod(uint16_t command, const struct cls_rule *rule,
     ofm->header.length = htons(size);
     ofm->cookie = 0;
     ofm->priority = htons(MIN(rule->priority, UINT16_MAX));
-    ofputil_cls_rule_to_match(rule, NXFF_OPENFLOW10, &ofm->match);
+    ofputil_cls_rule_to_match(rule, NXFF_OPENFLOW10, &ofm->match, 0, NULL);
     ofm->command = htons(command);
     return out;
 }

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index e121002af17415b8a7aa8eee8f8142226eeac12b..42318654b8fa0e33b5473ab8a0428d141dd1675b 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -105,7 +105,8 @@ void ofputil_cls_rule_from_match(const struct ofp_match *,
                                  unsigned int priority, enum nx_flow_format,
                                  ovs_be64 cookie, struct cls_rule *);
 void ofputil_cls_rule_to_match(const struct cls_rule *, enum nx_flow_format,
-                               struct ofp_match *);
+                               struct ofp_match *,
+                               ovs_be64 cookie_in, ovs_be64 *cookie_out);
 void normalize_match(struct ofp_match *);
 char *ofp_match_to_literal_string(const struct ofp_match *match);
 

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 45b9d460fb53030c467c5e40407bb164a8205718..01470f04ed2601c7c75277266ee74bb03ef4c395 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3451,9 +3451,9 @@ put_ofp_flow_stats(struct ofconn *ofconn, struct rule *rule,
     ofs->length = htons(len);
     ofs->table_id = 0;
     ofs->pad = 0;
-    ofputil_cls_rule_to_match(&rule->cr, ofconn->flow_format, &ofs->match);
+    ofputil_cls_rule_to_match(&rule->cr, ofconn->flow_format, &ofs->match,
+                              rule->flow_cookie, &ofs->cookie);
     calc_flow_duration(rule->created, &ofs->duration_sec, &ofs->duration_nsec);
-    ofs->cookie = rule->flow_cookie;
     ofs->priority = htons(rule->cr.priority);
     ofs->idle_timeout = htons(rule->idle_timeout);
     ofs->hard_timeout = htons(rule->hard_timeout);
@@ -4669,8 +4669,8 @@ compose_ofp_flow_removed(struct ofconn *ofconn, const struct rule *rule,
     struct ofpbuf *buf;
 
     ofr = make_openflow(sizeof *ofr, OFPT_FLOW_REMOVED, &buf);
-    ofputil_cls_rule_to_match(&rule->cr, ofconn->flow_format, &ofr->match);
-    ofr->cookie = rule->flow_cookie;
+    ofputil_cls_rule_to_match(&rule->cr, ofconn->flow_format, &ofr->match,
+                              rule->flow_cookie, &ofr->cookie);
     ofr->priority = htons(rule->cr.priority);
     ofr->reason = reason;
     calc_flow_duration(rule->created, &ofr->duration_sec, &ofr->duration_nsec);

diff --git a/tests/test-flows.c b/tests/test-flows.c
index 4ae198b25dd3cfca68a291d2bc1b8ca7c7a228e9..2edfa35fe53588f5cc93159fd0c3970406390a66 100644 (file)
--- a/tests/test-flows.c
+++ b/tests/test-flows.c
@@ -70,7 +70,8 @@ main(int argc OVS_UNUSED, char *argv[])
 
         flow_extract(packet, 0, 1, &flow);
         cls_rule_init_exact(&flow, 0, &rule);
-        ofputil_cls_rule_to_match(&rule, NXFF_OPENFLOW10, &extracted_match);
+        ofputil_cls_rule_to_match(&rule, NXFF_OPENFLOW10, &extracted_match,
+                                  0, NULL);
 
         if (memcmp(&expected_match, &extracted_match, sizeof expected_match)) {
             char *exp_s = ofp_match_to_string(&expected_match, 2);