From b78f6b77bcaefc7b1e480aa6063091cb9ad50ad2 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 26 Apr 2011 09:42:18 -0700 Subject: [PATCH] Remove support for obsolete "tun_id_from_cookie" extension. The "tun_id_from_cookie" OpenFlow extension predated NXM and supports only a fraction of its features. Nothing (at Nicira, anyway) uses it any longer. Support for it had been broken since January and it took until a few days ago for anyone to complain, so it cannot be too important. This commit removes it. --- ChangeLog | 6 + include/openflow/nicira-ext.h | 23 +--- lib/learning-switch.c | 1 - lib/ofp-parse.c | 2 +- lib/ofp-print.c | 22 +--- lib/ofp-util.c | 233 +++++++--------------------------- lib/ofp-util.h | 24 +--- ofproto/ofproto.c | 34 +---- tests/ofp-print.at | 10 -- tests/ofproto.at | 4 - tests/ovs-ofctl.at | 17 +-- tests/test-flows.c | 5 +- utilities/ovs-ofctl.8.in | 5 - utilities/ovs-ofctl.c | 9 +- 14 files changed, 82 insertions(+), 313 deletions(-) diff --git a/ChangeLog b/ChangeLog index 76d8e298..18f92280 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +post v1.1.0 +------------------------ + - Feature removals: + - Dropped support for "tun_id_from_cookie" OpenFlow extension. + (Use the extensible match extensions instead.) + v1.1.0 - 05 Apr 2011 ------------------------ - Ability to define policies over IPv6 diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index 292a9873..241cfe99 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -136,10 +136,7 @@ enum nicira_type { NXT_FLOW_END_CONFIG__OBSOLETE, NXT_FLOW_END__OBSOLETE, NXT_MGMT__OBSOLETE, - - /* Use the high 32 bits of the cookie field as the tunnel ID in the flow - * match. */ - NXT_TUN_ID_FROM_COOKIE, + NXT_TUN_ID_FROM_COOKIE__OBSOLETE, /* Controller role support. The request body is struct nx_role_request. * The reply echos the request. */ @@ -170,16 +167,6 @@ enum nicira_stats_type { NXST_AGGREGATE /* Analogous to OFPST_AGGREGATE. */ }; -/* NXT_TUN_ID_FROM_COOKIE request. */ -struct nxt_tun_id_cookie { - struct ofp_header header; - ovs_be32 vendor; /* NX_VENDOR_ID. */ - ovs_be32 subtype; /* NXT_TUN_ID_FROM_COOKIE */ - uint8_t set; /* Nonzero to enable, zero to disable. */ - uint8_t pad[7]; -}; -OFP_ASSERT(sizeof(struct nxt_tun_id_cookie) == 24); - /* Configures the "role" of the sending controller. The default role is: * * - Other (NX_ROLE_OTHER), which allows the controller access to all @@ -604,12 +591,6 @@ enum nx_mp_algorithm { */ NX_MP_ALG_ITER_HASH /* Iterative Hash. */ }; - -/* Wildcard for tunnel ID. */ -#define NXFW_TUN_ID (1 << 25) - -#define NXFW_ALL NXFW_TUN_ID -#define OVSFW_ALL (OFPFW_ALL | NXFW_ALL) /* Action structure for NXAST_AUTOPATH. * @@ -1165,8 +1146,6 @@ OFP_ASSERT(sizeof(struct nx_action_autopath) == 24); enum nx_flow_format { NXFF_OPENFLOW10 = 0, /* Standard OpenFlow 1.0 compatible. */ - NXFF_TUN_ID_FROM_COOKIE = 1, /* OpenFlow 1.0, plus obtain tunnel ID from - * cookie. */ NXFF_NXM = 2 /* Nicira extended match. */ }; diff --git a/lib/learning-switch.c b/lib/learning-switch.c index dc9af773..e003b8f9 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -238,7 +238,6 @@ lswitch_process_packet(struct lswitch *sw, struct rconn *rconn, case OFPUTIL_OFPST_PORT_REPLY: case OFPUTIL_OFPST_TABLE_REPLY: case OFPUTIL_OFPST_AGGREGATE_REPLY: - case OFPUTIL_NXT_TUN_ID_FROM_COOKIE: case OFPUTIL_NXT_ROLE_REQUEST: case OFPUTIL_NXT_ROLE_REPLY: case OFPUTIL_NXT_SET_FLOW_FORMAT: diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 7e9a9653..f45c450c 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -859,7 +859,7 @@ parse_ofp_flow_mod_str(struct list *packets, enum nx_flow_format *cur_format, parse_ofp_str(&fm, NULL, is_del ? NULL : &actions, string); fm.command = command; - min_format = ofputil_min_flow_format(&fm.cr, true, fm.cookie); + min_format = ofputil_min_flow_format(&fm.cr); next_format = MAX(*cur_format, min_format); if (next_format != *cur_format) { struct ofpbuf *sff = ofputil_make_set_flow_format(next_format); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 30f6d374..65dea728 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -780,9 +780,6 @@ ofp_match_to_string(const struct ofp_match *om, int verbosity) skip_type = false; } } - if (w & NXFW_TUN_ID) { - ds_put_cstr(&f, "tun_id_wild,"); - } print_wild(&f, "in_port=", w & OFPFW_IN_PORT, verbosity, "%d", ntohs(om->in_port)); print_wild(&f, "dl_vlan=", w & OFPFW_DL_VLAN, verbosity, @@ -837,7 +834,7 @@ ofp_print_flow_mod(struct ds *s, const struct ofp_header *oh, bool need_priority; int error; - error = ofputil_decode_flow_mod(&fm, oh, NXFF_OPENFLOW10); + error = ofputil_decode_flow_mod(&fm, oh); if (error) { ofp_print_error(s, error); return; @@ -934,7 +931,7 @@ ofp_print_flow_removed(struct ds *string, const struct ofp_header *oh) struct ofputil_flow_removed fr; int error; - error = ofputil_decode_flow_removed(&fr, oh, NXFF_OPENFLOW10); + error = ofputil_decode_flow_removed(&fr, oh); if (error) { ofp_print_error(string, error); return; @@ -1070,7 +1067,7 @@ ofp_print_flow_stats_request(struct ds *string, const struct ofp_header *oh) struct flow_stats_request fsr; int error; - error = ofputil_decode_flow_stats_request(&fsr, oh, NXFF_OPENFLOW10); + error = ofputil_decode_flow_stats_request(&fsr, oh); if (error) { ofp_print_error(string, error); return; @@ -1103,7 +1100,7 @@ ofp_print_flow_stats_reply(struct ds *string, const struct ofp_header *oh) struct ofputil_flow_stats fs; int retval; - retval = ofputil_decode_flow_stats_reply(&fs, &b, NXFF_OPENFLOW10); + retval = ofputil_decode_flow_stats_reply(&fs, &b); if (retval) { if (retval != EOF) { ds_put_cstr(string, " ***parse error***"); @@ -1331,13 +1328,6 @@ ofp_print_echo(struct ds *string, const struct ofp_header *oh, int verbosity) } } -static void -ofp_print_nxt_tun_id_from_cookie(struct ds *string, - const struct nxt_tun_id_cookie *ntic) -{ - ds_put_format(string, " set=%"PRIu8, ntic->set); -} - static void ofp_print_nxt_role_message(struct ds *string, const struct nx_role_request *nrr) @@ -1507,10 +1497,6 @@ ofp_to_string__(const struct ofp_header *oh, ofp_print_ofpst_aggregate_reply(string, oh); break; - case OFPUTIL_NXT_TUN_ID_FROM_COOKIE: - ofp_print_nxt_tun_id_from_cookie(string, msg); - break; - case OFPUTIL_NXT_ROLE_REQUEST: case OFPUTIL_NXT_ROLE_REPLY: ofp_print_nxt_role_message(string, msg); diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 9541d697..5aa2b825 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -102,25 +102,17 @@ enum { }; /* Converts the ofp_match in 'match' into a cls_rule in 'rule', with the given - * 'priority'. - * - * 'flow_format' must either NXFF_OPENFLOW10 or NXFF_TUN_ID_FROM_COOKIE. In - * the latter case only, 'flow''s tun_id field will be taken from the high bits - * of 'cookie', if 'match''s wildcards do not indicate that tun_id is - * wildcarded. */ + * 'priority'. */ void ofputil_cls_rule_from_match(const struct ofp_match *match, - unsigned int priority, - enum nx_flow_format flow_format, - ovs_be64 cookie, struct cls_rule *rule) + unsigned int priority, struct cls_rule *rule) { struct flow_wildcards *wc = &rule->wc; unsigned int ofpfw; ovs_be16 vid, pcp; /* Initialize rule->priority. */ - ofpfw = ntohl(match->wildcards); - ofpfw &= flow_format == NXFF_TUN_ID_FROM_COOKIE ? OVSFW_ALL : OFPFW_ALL; + ofpfw = ntohl(match->wildcards) & OFPFW_ALL; rule->priority = !ofpfw ? UINT16_MAX : priority; /* Initialize most of rule->wc. */ @@ -136,10 +128,6 @@ ofputil_cls_rule_from_match(const struct ofp_match *match, wc->nw_src_mask = ofputil_wcbits_to_netmask(ofpfw >> OFPFW_NW_SRC_SHIFT); wc->nw_dst_mask = ofputil_wcbits_to_netmask(ofpfw >> OFPFW_NW_DST_SHIFT); - if (flow_format == NXFF_TUN_ID_FROM_COOKIE && !(ofpfw & NXFW_TUN_ID)) { - cls_rule_set_tun_id(rule, htonll(ntohll(cookie) >> 32)); - } - if (ofpfw & OFPFW_DL_DST) { /* OpenFlow 1.0 OFPFW_DL_DST covers the whole Ethernet destination, but * Open vSwitch breaks the Ethernet destination into bits as FWW_DL_DST @@ -207,21 +195,9 @@ ofputil_cls_rule_from_match(const struct ofp_match *match, cls_rule_zero_wildcarded_fields(rule); } -/* Convert 'rule' into the OpenFlow match structure 'match'. 'flow_format' - * must either NXFF_OPENFLOW10 or NXFF_TUN_ID_FROM_COOKIE. - * - * 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'. - */ +/* Convert 'rule' into the OpenFlow match structure 'match'. */ void -ofputil_cls_rule_to_match(const struct cls_rule *rule, - enum nx_flow_format flow_format, - struct ofp_match *match, - ovs_be64 cookie_in, ovs_be64 *cookie_out) +ofputil_cls_rule_to_match(const struct cls_rule *rule, struct ofp_match *match) { const struct flow_wildcards *wc = &rule->wc; unsigned int ofpfw; @@ -234,20 +210,6 @@ ofputil_cls_rule_to_match(const struct cls_rule *rule, ofpfw |= OFPFW_NW_TOS; } - /* Tunnel ID. */ - if (flow_format == NXFF_TUN_ID_FROM_COOKIE) { - if (wc->tun_id_mask == htonll(0)) { - ofpfw |= NXFW_TUN_ID; - } else { - uint32_t cookie_lo = ntohll(cookie_in); - uint32_t cookie_hi = ntohll(rule->flow.tun_id); - cookie_in = htonll(cookie_lo | ((uint64_t) cookie_hi << 32)); - } - } - if (cookie_out) { - *cookie_out = cookie_in; - } - /* Translate VLANs. */ match->dl_vlan = htons(0); match->dl_vlan_pcp = 0; @@ -400,10 +362,6 @@ ofputil_decode_vendor(const struct ofp_header *oh, const struct ofputil_msg_type **typep) { static const struct ofputil_msg_type nxt_messages[] = { - { OFPUTIL_NXT_TUN_ID_FROM_COOKIE, - NXT_TUN_ID_FROM_COOKIE, "NXT_TUN_ID_FROM_COOKIE", - sizeof(struct nxt_tun_id_cookie), 0 }, - { OFPUTIL_NXT_ROLE_REQUEST, NXT_ROLE_REQUEST, "NXT_ROLE_REQUEST", sizeof(struct nx_role_request), 0 }, @@ -800,7 +758,6 @@ ofputil_flow_format_is_valid(enum nx_flow_format flow_format) { switch (flow_format) { case NXFF_OPENFLOW10: - case NXFF_TUN_ID_FROM_COOKIE: case NXFF_NXM: return true; } @@ -814,8 +771,6 @@ ofputil_flow_format_to_string(enum nx_flow_format flow_format) switch (flow_format) { case NXFF_OPENFLOW10: return "openflow10"; - case NXFF_TUN_ID_FROM_COOKIE: - return "tun_id_from_cookie"; case NXFF_NXM: return "nxm"; default: @@ -827,7 +782,6 @@ int ofputil_flow_format_from_string(const char *s) { return (!strcmp(s, "openflow10") ? NXFF_OPENFLOW10 - : !strcmp(s, "tun_id_from_cookie") ? NXFF_TUN_ID_FROM_COOKIE : !strcmp(s, "nxm") ? NXFF_NXM : -1); } @@ -845,100 +799,43 @@ regs_fully_wildcarded(const struct flow_wildcards *wc) return true; } -static inline bool -is_nxm_required(const struct cls_rule *rule, bool cookie_support, - ovs_be64 cookie) +/* Returns the minimum nx_flow_format to use for sending 'rule' to a switch + * (e.g. to add or remove a flow). Only NXM can handle tunnel IDs, registers, + * or fixing the Ethernet multicast bit. Otherwise, it's better to use + * NXFF_OPENFLOW10 for backward compatibility. */ +enum nx_flow_format +ofputil_min_flow_format(const struct cls_rule *rule) { const struct flow_wildcards *wc = &rule->wc; - uint32_t cookie_hi; - uint64_t tun_id; /* Only NXM supports separately wildcards the Ethernet multicast bit. */ if (!(wc->wildcards & FWW_DL_DST) != !(wc->wildcards & FWW_ETH_MCAST)) { - return true; + return NXFF_NXM; } /* Only NXM supports matching ARP hardware addresses. */ if (!(wc->wildcards & FWW_ARP_SHA) || !(wc->wildcards & FWW_ARP_THA)) { - return true; + return NXFF_NXM; } /* Only NXM supports matching IPv6 traffic. */ if (!(wc->wildcards & FWW_DL_TYPE) && (rule->flow.dl_type == htons(ETH_TYPE_IPV6))) { - return true; + return NXFF_NXM; } /* Only NXM supports matching registers. */ if (!regs_fully_wildcarded(wc)) { - return true; + return NXFF_NXM; } - switch (wc->tun_id_mask) { - case CONSTANT_HTONLL(0): - /* Other formats can fully wildcard tun_id. */ - break; - - case CONSTANT_HTONLL(UINT64_MAX): - /* Only NXM supports tunnel ID matching without a cookie. */ - if (!cookie_support) { - return true; - } - - /* Only NXM supports 64-bit tunnel IDs. */ - tun_id = ntohll(rule->flow.tun_id); - if (tun_id > UINT32_MAX) { - return true; - } - - /* Only NXM supports a cookie whose top 32 bits conflict with the - * tunnel ID. */ - cookie_hi = ntohll(cookie) >> 32; - if (cookie_hi && cookie_hi != tun_id) { - return true; - } - break; - - default: - /* Only NXM supports partial matches on tunnel ID. */ - return true; + /* Only NXM supports matching tun_id. */ + if (wc->tun_id_mask != htonll(0)) { + return NXFF_NXM; } /* Other formats can express this rule. */ - return false; -} - -/* Returns the minimum nx_flow_format to use for sending 'rule' to a switch - * (e.g. to add or remove a flow). 'cookie_support' should be true if the - * command to be sent includes a flow cookie (as OFPT_FLOW_MOD does, for - * example) or false if the command does not (OFPST_FLOW and OFPST_AGGREGATE do - * not, for example). If 'cookie_support' is true, then 'cookie' should be the - * cookie to be sent; otherwise its value is ignored. - * - * The "best" flow format is chosen on this basis: - * - * - It must be capable of expressing the rule. NXFF_OPENFLOW10 flows can't - * handle tunnel IDs. NXFF_TUN_ID_FROM_COOKIE flows can't handle registers - * or fixing the Ethernet multicast bit, and can't handle tunnel IDs that - * conflict with the high 32 bits of the cookie or commands that don't - * support cookies. - * - * - Otherwise, the chosen format should be as backward compatible as - * possible. (NXFF_OPENFLOW10 is more backward compatible than - * NXFF_TUN_ID_FROM_COOKIE, which is more backward compatible than - * NXFF_NXM.) - */ -enum nx_flow_format -ofputil_min_flow_format(const struct cls_rule *rule, bool cookie_support, - ovs_be64 cookie) -{ - if (is_nxm_required(rule, cookie_support, cookie)) { - return NXFF_NXM; - } else if (rule->wc.tun_id_mask != htonll(0)) { - return NXFF_TUN_ID_FROM_COOKIE; - } else { - return NXFF_OPENFLOW10; - } + return NXFF_OPENFLOW10; } /* Returns an OpenFlow message that can be used to set the flow format to @@ -946,20 +843,11 @@ ofputil_min_flow_format(const struct cls_rule *rule, bool cookie_support, struct ofpbuf * ofputil_make_set_flow_format(enum nx_flow_format flow_format) { + struct nxt_set_flow_format *sff; struct ofpbuf *msg; - if (flow_format == NXFF_OPENFLOW10 - || flow_format == NXFF_TUN_ID_FROM_COOKIE) { - struct nxt_tun_id_cookie *tic; - - tic = make_nxmsg(sizeof *tic, NXT_TUN_ID_FROM_COOKIE, &msg); - tic->set = flow_format == NXFF_TUN_ID_FROM_COOKIE; - } else { - struct nxt_set_flow_format *sff; - - sff = make_nxmsg(sizeof *sff, NXT_SET_FLOW_FORMAT, &msg); - sff->format = htonl(flow_format); - } + sff = make_nxmsg(sizeof *sff, NXT_SET_FLOW_FORMAT, &msg); + sff->format = htonl(flow_format); return msg; } @@ -968,14 +856,9 @@ ofputil_make_set_flow_format(enum nx_flow_format flow_format) * flow_mod in 'fm'. Returns 0 if successful, otherwise an OpenFlow error * code. * - * For OFPT_FLOW_MOD messages, 'flow_format' should be the current flow format - * at the time when the message was received. Otherwise 'flow_format' is - * ignored. - * * Does not validate the flow_mod actions. */ int -ofputil_decode_flow_mod(struct flow_mod *fm, const struct ofp_header *oh, - enum nx_flow_format flow_format) +ofputil_decode_flow_mod(struct flow_mod *fm, const struct ofp_header *oh) { const struct ofputil_msg_type *type; struct ofpbuf b; @@ -1015,8 +898,7 @@ ofputil_decode_flow_mod(struct flow_mod *fm, const struct ofp_header *oh, } /* Translate the message. */ - ofputil_cls_rule_from_match(&match, ntohs(ofm->priority), flow_format, - ofm->cookie, &fm->cr); + ofputil_cls_rule_from_match(&match, ntohs(ofm->priority), &fm->cr); fm->cookie = ofm->cookie; fm->command = ntohs(ofm->command); fm->idle_timeout = ntohs(ofm->idle_timeout); @@ -1065,14 +947,13 @@ ofputil_encode_flow_mod(const struct flow_mod *fm, size_t actions_len = fm->n_actions * sizeof *fm->actions; struct ofpbuf *msg; - if (flow_format == NXFF_OPENFLOW10 - || flow_format == NXFF_TUN_ID_FROM_COOKIE) { + if (flow_format == NXFF_OPENFLOW10) { struct ofp_flow_mod *ofm; 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, - fm->cookie, &ofm->cookie); + ofputil_cls_rule_to_match(&fm->cr, &ofm->match); + ofm->cookie = fm->cookie; ofm->command = htons(fm->command); ofm->idle_timeout = htons(fm->idle_timeout); ofm->hard_timeout = htons(fm->hard_timeout); @@ -1110,13 +991,12 @@ ofputil_encode_flow_mod(const struct flow_mod *fm, static int ofputil_decode_ofpst_flow_request(struct flow_stats_request *fsr, const struct ofp_header *oh, - enum nx_flow_format flow_format, bool aggregate) { const struct ofp_flow_stats_request *ofsr = ofputil_stats_body(oh); fsr->aggregate = aggregate; - ofputil_cls_rule_from_match(&ofsr->match, 0, flow_format, 0, &fsr->match); + ofputil_cls_rule_from_match(&ofsr->match, 0, &fsr->match); fsr->out_port = ntohs(ofsr->out_port); fsr->table_id = ofsr->table_id; @@ -1151,17 +1031,11 @@ ofputil_decode_nxst_flow_request(struct flow_stats_request *fsr, } /* Converts an OFPST_FLOW, OFPST_AGGREGATE, NXST_FLOW, or NXST_AGGREGATE - * request 'oh', received when the current flow format was 'flow_format', into - * an abstract flow_stats_request in 'fsr'. Returns 0 if successful, otherwise - * an OpenFlow error code. - * - * For OFPST_FLOW and OFPST_AGGREGATE messages, 'flow_format' should be the - * current flow format at the time when the message was received. Otherwise - * 'flow_format' is ignored. */ + * request 'oh', into an abstract flow_stats_request in 'fsr'. Returns 0 if + * successful, otherwise an OpenFlow error code. */ int ofputil_decode_flow_stats_request(struct flow_stats_request *fsr, - const struct ofp_header *oh, - enum nx_flow_format flow_format) + const struct ofp_header *oh) { const struct ofputil_msg_type *type; struct ofpbuf b; @@ -1173,10 +1047,10 @@ ofputil_decode_flow_stats_request(struct flow_stats_request *fsr, code = ofputil_msg_type_code(type); switch (code) { case OFPUTIL_OFPST_FLOW_REQUEST: - return ofputil_decode_ofpst_flow_request(fsr, oh, flow_format, false); + return ofputil_decode_ofpst_flow_request(fsr, oh, false); case OFPUTIL_OFPST_AGGREGATE_REQUEST: - return ofputil_decode_ofpst_flow_request(fsr, oh, flow_format, true); + return ofputil_decode_ofpst_flow_request(fsr, oh, true); case OFPUTIL_NXST_FLOW_REQUEST: return ofputil_decode_nxst_flow_request(fsr, oh, false); @@ -1199,8 +1073,7 @@ ofputil_encode_flow_stats_request(const struct flow_stats_request *fsr, { struct ofpbuf *msg; - if (flow_format == NXFF_OPENFLOW10 - || flow_format == NXFF_TUN_ID_FROM_COOKIE) { + if (flow_format == NXFF_OPENFLOW10) { struct ofp_flow_stats_request *ofsr; int type; @@ -1209,8 +1082,7 @@ 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, - 0, NULL); + ofputil_cls_rule_to_match(&fsr->match, &ofsr->match); ofsr->table_id = fsr->table_id; ofsr->out_port = htons(fsr->out_port); } else if (flow_format == NXFF_NXM) { @@ -1234,9 +1106,7 @@ ofputil_encode_flow_stats_request(const struct flow_stats_request *fsr, } /* Converts an OFPST_FLOW or NXST_FLOW reply in 'msg' into an abstract - * ofputil_flow_stats in 'fs'. For OFPST_FLOW messages, 'flow_format' should - * be the current flow format at the time when the request corresponding to the - * reply in 'msg' was sent. Otherwise 'flow_format' is ignored. + * ofputil_flow_stats in 'fs'. * * Multiple OFPST_FLOW or NXST_FLOW replies can be packed into a single * OpenFlow message. Calling this function multiple times for a single 'msg' @@ -1247,8 +1117,7 @@ ofputil_encode_flow_stats_request(const struct flow_stats_request *fsr, * otherwise a positive errno value. */ int ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, - struct ofpbuf *msg, - enum nx_flow_format flow_format) + struct ofpbuf *msg) { const struct ofputil_msg_type *type; int code; @@ -1293,7 +1162,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, fs->cookie = get_32aligned_be64(&ofs->cookie); ofputil_cls_rule_from_match(&ofs->match, ntohs(ofs->priority), - flow_format, fs->cookie, &fs->rule); + &fs->rule); fs->table_id = ofs->table_id; fs->duration_sec = ntohl(ofs->duration_sec); fs->duration_nsec = ntohl(ofs->duration_nsec); @@ -1344,18 +1213,12 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, return 0; } -/* Converts an OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED message 'oh', received - * when the current flow format was 'flow_format', into an abstract - * ofputil_flow_removed in 'fr'. Returns 0 if successful, otherwise an - * OpenFlow error code. - * - * For OFPT_FLOW_REMOVED messages, 'flow_format' should be the current flow - * format at the time when the message was received. Otherwise 'flow_format' - * is ignored. */ +/* Converts an OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED message 'oh' into an + * abstract ofputil_flow_removed in 'fr'. Returns 0 if successful, otherwise + * an OpenFlow error code. */ int ofputil_decode_flow_removed(struct ofputil_flow_removed *fr, - const struct ofp_header *oh, - enum nx_flow_format flow_format) + const struct ofp_header *oh) { const struct ofputil_msg_type *type; enum ofputil_msg_code code; @@ -1367,7 +1230,7 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed *fr, ofr = (const struct ofp_flow_removed *) oh; ofputil_cls_rule_from_match(&ofr->match, ntohs(ofr->priority), - flow_format, ofr->cookie, &fr->rule); + &fr->rule); fr->cookie = ofr->cookie; fr->reason = ofr->reason; fr->duration_sec = ntohl(ofr->duration_sec); @@ -1415,14 +1278,12 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr, { struct ofpbuf *msg; - if (flow_format == NXFF_OPENFLOW10 - || flow_format == NXFF_TUN_ID_FROM_COOKIE) { + if (flow_format == NXFF_OPENFLOW10) { struct ofp_flow_removed *ofr; ofr = make_openflow_xid(sizeof *ofr, OFPT_FLOW_REMOVED, htonl(0), &msg); - ofputil_cls_rule_to_match(&fr->rule, flow_format, &ofr->match, - fr->cookie, &ofr->cookie); + ofputil_cls_rule_to_match(&fr->rule, &ofr->match); ofr->priority = htons(fr->rule.priority); ofr->reason = fr->reason; ofr->duration_sec = htonl(fr->duration_sec); @@ -1717,7 +1578,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, 0, NULL); + ofputil_cls_rule_to_match(rule, &ofm->match); ofm->command = htons(command); return out; } @@ -2175,7 +2036,7 @@ normalize_match(struct ofp_match *m) enum { OFPFW_TP = OFPFW_TP_SRC | OFPFW_TP_DST }; uint32_t wc; - wc = ntohl(m->wildcards) & OVSFW_ALL; + wc = ntohl(m->wildcards) & OFPFW_ALL; if (wc & OFPFW_DL_TYPE) { m->dl_type = 0; diff --git a/lib/ofp-util.h b/lib/ofp-util.h index fdeb9d9e..6e69bae0 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -71,7 +71,6 @@ enum ofputil_msg_code { OFPUTIL_OFPST_AGGREGATE_REPLY, /* NXT_* messages. */ - OFPUTIL_NXT_TUN_ID_FROM_COOKIE, OFPUTIL_NXT_ROLE_REQUEST, OFPUTIL_NXT_ROLE_REPLY, OFPUTIL_NXT_SET_FLOW_FORMAT, @@ -100,11 +99,8 @@ int ofputil_netmask_to_wcbits(ovs_be32 netmask); /* Work with OpenFlow 1.0 ofp_match. */ 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 *, - ovs_be64 cookie_in, ovs_be64 *cookie_out); + unsigned int priority, struct cls_rule *); +void ofputil_cls_rule_to_match(const struct cls_rule *, struct ofp_match *); void normalize_match(struct ofp_match *); char *ofp_match_to_literal_string(const struct ofp_match *match); @@ -116,9 +112,7 @@ ovs_be16 ofputil_dl_type_from_openflow(ovs_be16 ofp_dl_type); bool ofputil_flow_format_is_valid(enum nx_flow_format); const char *ofputil_flow_format_to_string(enum nx_flow_format); int ofputil_flow_format_from_string(const char *); -enum nx_flow_format ofputil_min_flow_format(const struct cls_rule *, - bool cookie_support, - ovs_be64 cookie); +enum nx_flow_format ofputil_min_flow_format(const struct cls_rule *); struct ofpbuf *ofputil_make_set_flow_format(enum nx_flow_format); @@ -136,8 +130,7 @@ struct flow_mod { size_t n_actions; }; -int ofputil_decode_flow_mod(struct flow_mod *, const struct ofp_header *, - enum nx_flow_format); +int ofputil_decode_flow_mod(struct flow_mod *, const struct ofp_header *); struct ofpbuf *ofputil_encode_flow_mod(const struct flow_mod *, enum nx_flow_format); @@ -150,8 +143,7 @@ struct flow_stats_request { }; int ofputil_decode_flow_stats_request(struct flow_stats_request *, - const struct ofp_header *, - enum nx_flow_format); + const struct ofp_header *); struct ofpbuf *ofputil_encode_flow_stats_request( const struct flow_stats_request *, enum nx_flow_format); @@ -171,8 +163,7 @@ struct ofputil_flow_stats { }; int ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *, - struct ofpbuf *msg, - enum nx_flow_format); + struct ofpbuf *msg); /* Flow removed message, independent of flow format. */ struct ofputil_flow_removed { @@ -187,8 +178,7 @@ struct ofputil_flow_removed { }; int ofputil_decode_flow_removed(struct ofputil_flow_removed *, - const struct ofp_header *, - enum nx_flow_format); + const struct ofp_header *); struct ofpbuf *ofputil_encode_flow_removed(const struct ofputil_flow_removed *, enum nx_flow_format); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index fb63606a..7eef93a1 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2746,8 +2746,7 @@ handle_table_stats_request(struct ofconn *ofconn, ots = append_ofp_stats_reply(sizeof *ots, ofconn, &msg); memset(ots, 0, sizeof *ots); strcpy(ots->name, "classifier"); - ots->wildcards = (ofconn_get_flow_format(ofconn) == NXFF_OPENFLOW10 - ? htonl(OFPFW_ALL) : htonl(OVSFW_ALL)); + ots->wildcards = htonl(OFPFW_ALL); ots->max_entries = htonl(1024 * 1024); /* An arbitrary big number. */ ots->active_count = htonl(classifier_count(&p->cls)); put_32aligned_be64(&ots->lookup_count, htonll(0)); /* XXX */ @@ -2835,7 +2834,6 @@ put_ofp_flow_stats(struct ofconn *ofconn, struct rule *rule, { struct ofp_flow_stats *ofs; uint64_t packet_count, byte_count; - ovs_be64 cookie; size_t act_len, len; if (rule_is_hidden(rule) || !rule_has_out_port(rule, out_port)) { @@ -2851,9 +2849,8 @@ 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_get_flow_format(ofconn), - &ofs->match, rule->flow_cookie, &cookie); - put_32aligned_be64(&ofs->cookie, cookie); + ofputil_cls_rule_to_match(&rule->cr, &ofs->match); + put_32aligned_be64(&ofs->cookie, rule->flow_cookie); calc_flow_duration(rule->created, &ofs->duration_sec, &ofs->duration_nsec); ofs->priority = htons(rule->cr.priority); ofs->idle_timeout = htons(rule->idle_timeout); @@ -2895,8 +2892,7 @@ handle_flow_stats_request(struct ofconn *ofconn, const struct ofp_header *oh) struct cls_rule target; struct rule *rule; - ofputil_cls_rule_from_match(&fsr->match, 0, NXFF_OPENFLOW10, 0, - &target); + ofputil_cls_rule_from_match(&fsr->match, 0, &target); cls_cursor_init(&cursor, &ofproto->cls, &target); CLS_CURSOR_FOR_EACH (rule, cr, &cursor) { put_ofp_flow_stats(ofconn, rule, fsr->out_port, &reply); @@ -3068,8 +3064,7 @@ handle_aggregate_stats_request(struct ofconn *ofconn, struct cls_rule target; struct ofpbuf *msg; - ofputil_cls_rule_from_match(&request->match, 0, NXFF_OPENFLOW10, 0, - &target); + ofputil_cls_rule_from_match(&request->match, 0, &target); msg = start_ofp_stats_reply(oh, sizeof *reply); reply = append_ofp_stats_reply(sizeof *reply, ofconn, &msg); @@ -3516,7 +3511,7 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh) return error; } - error = ofputil_decode_flow_mod(&fm, oh, ofconn_get_flow_format(ofconn)); + error = ofputil_decode_flow_mod(&fm, oh); if (error) { return error; } @@ -3558,19 +3553,6 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh) } } -static int -handle_tun_id_from_cookie(struct ofconn *ofconn, const struct ofp_header *oh) -{ - const struct nxt_tun_id_cookie *msg - = (const struct nxt_tun_id_cookie *) oh; - enum nx_flow_format flow_format; - - flow_format = msg->set ? NXFF_TUN_ID_FROM_COOKIE : NXFF_OPENFLOW10; - ofconn_set_flow_format(ofconn, flow_format); - - return 0; -} - static int handle_role_request(struct ofconn *ofconn, const struct ofp_header *oh) { @@ -3611,7 +3593,6 @@ handle_nxt_set_flow_format(struct ofconn *ofconn, const struct ofp_header *oh) format = ntohl(msg->format); if (format == NXFF_OPENFLOW10 - || format == NXFF_TUN_ID_FROM_COOKIE || format == NXFF_NXM) { ofconn_set_flow_format(ofconn, format); return 0; @@ -3676,9 +3657,6 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg) return 0; /* Nicira extension requests. */ - case OFPUTIL_NXT_TUN_ID_FROM_COOKIE: - return handle_tun_id_from_cookie(ofconn, oh); - case OFPUTIL_NXT_ROLE_REQUEST: return handle_role_request(ofconn, oh); diff --git a/tests/ofp-print.at b/tests/ofp-print.at index 6e1e3bd4..923e00e9 100644 --- a/tests/ofp-print.at +++ b/tests/ofp-print.at @@ -601,16 +601,6 @@ OFPT_BARRIER_REPLY (xid=0x1): ]) AT_CLEANUP -AT_SETUP([NXT_TUN_ID_FROM_COOKIE]) -AT_KEYWORDS([ofp-print]) -AT_CHECK([ovs-ofctl ofp-print "\ -01 04 00 18 00 00 00 02 00 00 23 20 00 00 00 09 \ -01 00 00 00 00 00 00 00 \ -"], [0], [dnl -NXT_TUN_ID_FROM_COOKIE (xid=0x2): set=1 -]) -AT_CLEANUP - AT_SETUP([NXT_ROLE_REQUEST]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl ofp-print "\ diff --git a/tests/ofproto.at b/tests/ofproto.at index fc7ff57e..10080e67 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -48,13 +48,9 @@ AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS], [0], [NXST_FLOW reply: ]) AT_CHECK([echo 'in_port=1,actions=0' | ovs-ofctl add-flows br0 -]) AT_CHECK([ovs-ofctl add-flow br0 in_port=0,actions=1]) -dnl Tests for a bug in which ofproto ignored tun_id in tun_id_from_cookie -dnl flow_mod commands. -AT_CHECK([ovs-ofctl add-flow -F tun_id_from_cookie br0 tun_id=1,actions=mod_vlan_vid:4]) AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl cookie=0x0, duration=?s, table_id=0, n_packets=0, n_bytes=0, in_port=1 actions=output:0 cookie=0x0, duration=?s, table_id=0, n_packets=0, n_bytes=0, in_port=65534 actions=output:1 - cookie=0x100000000, duration=?s, table_id=0, n_packets=0, n_bytes=0, tun_id=0x1 actions=mod_vlan_vid:4 NXST_FLOW reply: ]) AT_CHECK([ovs-ofctl del-flows br0]) diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 466ade6b..10147108 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -28,12 +28,11 @@ OFPT_FLOW_MOD: ADD tcp,nw_src=192.168.0.3,tp_dst=80 actions=set_queue:37,output: OFPT_FLOW_MOD: ADD udp,nw_src=192.168.0.3,tp_dst=53 actions=pop_queue,output:1 OFPT_FLOW_MOD: ADD priority=60000 cookie:0x123456789abcdef hard:10 actions=CONTROLLER:65535 OFPT_FLOW_MOD: ADD actions=note:41.42.43.00.00.00,note:00.01.02.03.04.05.06.07.00.00.00.00.00.00,note:00.00.00.00.00.00 -NXT_TUN_ID_FROM_COOKIE: set=1 -OFPT_FLOW_MOD: ADD cookie:0x123400005678 actions=FLOOD -OFPT_FLOW_MOD: ADD actions=set_tunnel:0x1234,set_tunnel64:0x9876,set_tunnel64:0x123456789 -OFPT_FLOW_MOD: ADD actions=multipath(eth_src,50,hrw,12,0,NXM_NX_REG0[0..3]),multipath(symmetric_l4,1024,iter_hash,5000,5050,NXM_NX_REG0[0..12]) -OFPT_FLOW_MOD: ADD actions=drop NXT_SET_FLOW_FORMAT: format=nxm +NXT_FLOW_MOD: ADD tun_id=0x1234 cookie:0x5678 actions=FLOOD +NXT_FLOW_MOD: ADD actions=set_tunnel:0x1234,set_tunnel64:0x9876,set_tunnel64:0x123456789 +NXT_FLOW_MOD: ADD actions=multipath(eth_src,50,hrw,12,0,NXM_NX_REG0[0..3]),multipath(symmetric_l4,1024,iter_hash,5000,5050,NXM_NX_REG0[0..12]) +NXT_FLOW_MOD: ADD actions=drop NXT_FLOW_MOD: ADD tun_id=0x1234000056780000/0xffff0000ffff0000 actions=drop ]]) AT_CHECK([sed 's/.*|//' stderr], [0], [dnl @@ -46,12 +45,6 @@ post: wildcards= 0x3fffff in_port=65534 dl_src=00:00:00:00:00:00 dl_dst=00:0 normalization changed ofp_match, details: pre: wildcards= 0x3820ff in_port=65534 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 post: wildcards= 0x3fffff in_port=65534 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 -normalization changed ofp_match, details: - pre: wildcards= 0x3820ff in_port=65534 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 -post: wildcards= 0x3fffff in_port=65534 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 -normalization changed ofp_match, details: - pre: wildcards= 0x23820ff in_port=65534 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 -post: wildcards= 0x23fffff in_port=65534 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 ]) AT_CLEANUP @@ -469,7 +462,7 @@ dnl Check that "-F openflow10" rejects a flow_mod with a tun_id, since dnl OpenFlow 1.0 doesn't support tunnels. AT_SETUP([ovs-ofctl -F option and tun_id]) AT_CHECK([ovs-ofctl -F openflow10 add-flow dummy tun_id=123,actions=drop], - [1], [], [ovs-ofctl: flow cannot be expressed in flow format openflow10 (flow format tun_id_from_cookie or better is required) + [1], [], [ovs-ofctl: flow cannot be expressed in flow format openflow10 (flow format nxm or better is required) ]) AT_CLEANUP diff --git a/tests/test-flows.c b/tests/test-flows.c index 2edfa35f..559af3a3 100644 --- a/tests/test-flows.c +++ b/tests/test-flows.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2010 Nicira Networks. + * Copyright (c) 2009, 2010, 2011 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -70,8 +70,7 @@ 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, - 0, NULL); + ofputil_cls_rule_to_match(&rule, &extracted_match); if (memcmp(&expected_match, &extracted_match, sizeof expected_match)) { char *exp_s = ofp_match_to_string(&expected_match, 2); diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index d9742c8e..c44de463 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -764,11 +764,6 @@ increasing capability: This is the standard OpenFlow 1.0 flow format. It should be supported by all OpenFlow switches. . -.IP "\fBtun_id_from_cookie\fR" -This Nicira extension to OpenFlow adds minimal and limited support for -\fBtun_id\fR, but it does not support any other Nicira flow -extensions. (This flow format is deprecated.) -. .IP "\fBnxm\fR (Nicira Extended Match)" This Nicira extension to OpenFlow is flexible and extensible. It supports all of the Nicira flow extensions, such as \fBtun_id\fR and diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index bc8cc6bd..c0ff4dc1 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -515,8 +515,6 @@ negotiate_highest_flow_format(struct vconn *vconn, if (try_set_flow_format(vconn, NXFF_NXM)) { flow_format = NXFF_NXM; - } else if (try_set_flow_format(vconn, NXFF_TUN_ID_FROM_COOKIE)) { - flow_format = NXFF_TUN_ID_FROM_COOKIE; } else { flow_format = NXFF_OPENFLOW10; } @@ -544,7 +542,7 @@ do_dump_flows__(int argc, char *argv[], bool aggregate) parse_ofp_flow_stats_request_str(&fsr, aggregate, argc > 2 ? argv[2] : ""); open_vconn(argv[1], &vconn); - min_flow_format = ofputil_min_flow_format(&fsr.match, false, 0); + min_flow_format = ofputil_min_flow_format(&fsr.match); flow_format = negotiate_highest_flow_format(vconn, min_flow_format); request = ofputil_encode_flow_stats_request(&fsr, flow_format); dump_stats_transaction(argv[1], request); @@ -1056,7 +1054,7 @@ read_flows_from_file(const char *filename, struct classifier *cls, int index) version->n_actions = actions.size / sizeof *version->actions; version->actions = ofpbuf_steal_data(&actions); - min_ff = ofputil_min_flow_format(&fm.cr, true, fm.cookie); + min_ff = ofputil_min_flow_format(&fm.cr); min_flow_format = MAX(min_flow_format, min_ff); check_final_format_for_flow_mod(min_flow_format); @@ -1122,8 +1120,7 @@ read_flows_from_switch(struct vconn *vconn, enum nx_flow_format flow_format, struct ofputil_flow_stats fs; int retval; - retval = ofputil_decode_flow_stats_reply(&fs, reply, - flow_format); + retval = ofputil_decode_flow_stats_reply(&fs, reply); if (retval) { if (retval != EOF) { ovs_fatal(0, "parse error in reply"); -- 2.30.2