From ff9d38264c74e2e807ba0fd759e44116d1203670 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 9 Dec 2010 14:19:51 -0800 Subject: [PATCH] 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 --- lib/ofp-util.c | 48 ++++++++++++++++++++++++++++------------------ lib/ofp-util.h | 3 ++- ofproto/ofproto.c | 8 ++++---- tests/test-flows.c | 3 ++- 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/lib/ofp-util.c b/lib/ofp-util.c index cdbbb0dc9..1cafd862b 100644 --- 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 e121002af..42318654b 100644 --- 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 45b9d460f..01470f04e 100644 --- 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 4ae198b25..2edfa35fe 100644 --- 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); -- 2.43.0