From: Ben Pfaff Date: Fri, 16 Nov 2012 05:01:13 +0000 (-0800) Subject: ofp-util: Rename OFPUTIL_P_* constants and update comments for clarity. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=85813857630f6a09786f3ab2b2129a8513491e64;p=openvswitch ofp-util: Rename OFPUTIL_P_* constants and update comments for clarity. It wasn't clear to me, at least, whether an OFPUTIL_P_* constant indicated an OpenFlow version and a flow format or just a flow format. After some reflection, I think it's more useful if it indicates both, because otherwise it might be necessary to pass both an OpenFlow version and an OFPUTIL_P_* constant in some contexts, but this way only the latter is needed. Signed-off-by: Ben Pfaff Reviewed-by: Simon Horman --- diff --git a/lib/ofp-util.c b/lib/ofp-util.c index dea4aeaf..c2e97384 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -581,15 +581,15 @@ struct proto_abbrev { /* Most users really don't care about some of the differences between * protocols. These abbreviations help with that. */ static const struct proto_abbrev proto_abbrevs[] = { - { OFPUTIL_P_ANY, "any" }, - { OFPUTIL_P_OF10_ANY, "OpenFlow10" }, - { OFPUTIL_P_NXM_ANY, "NXM" }, + { OFPUTIL_P_ANY, "any" }, + { OFPUTIL_P_OF10_STD_ANY, "OpenFlow10" }, + { OFPUTIL_P_OF10_NXM_ANY, "NXM" }, }; #define N_PROTO_ABBREVS ARRAY_SIZE(proto_abbrevs) enum ofputil_protocol ofputil_flow_dump_protocols[] = { - OFPUTIL_P_NXM, - OFPUTIL_P_OF10, + OFPUTIL_P_OF10_NXM, + OFPUTIL_P_OF10_STD, }; size_t ofputil_n_flow_dump_protocols = ARRAY_SIZE(ofputil_flow_dump_protocols); @@ -603,9 +603,9 @@ ofputil_protocol_from_ofp_version(enum ofp_version version) { switch (version) { case OFP10_VERSION: - return OFPUTIL_P_OF10; + return OFPUTIL_P_OF10_STD; case OFP12_VERSION: - return OFPUTIL_P_OF12; + return OFPUTIL_P_OF12_OXM; case OFP11_VERSION: default: return 0; @@ -618,12 +618,12 @@ enum ofp_version ofputil_protocol_to_ofp_version(enum ofputil_protocol protocol) { switch (protocol) { - case OFPUTIL_P_OF10: - case OFPUTIL_P_OF10_TID: - case OFPUTIL_P_NXM: - case OFPUTIL_P_NXM_TID: + case OFPUTIL_P_OF10_STD: + case OFPUTIL_P_OF10_STD_TID: + case OFPUTIL_P_OF10_NXM: + case OFPUTIL_P_OF10_NXM_TID: return OFP10_VERSION; - case OFPUTIL_P_OF12: + case OFPUTIL_P_OF12_OXM: return OFP12_VERSION; } @@ -652,16 +652,16 @@ enum ofputil_protocol ofputil_protocol_set_tid(enum ofputil_protocol protocol, bool enable) { switch (protocol) { - case OFPUTIL_P_OF10: - case OFPUTIL_P_OF10_TID: - return enable ? OFPUTIL_P_OF10_TID : OFPUTIL_P_OF10; + case OFPUTIL_P_OF10_STD: + case OFPUTIL_P_OF10_STD_TID: + return enable ? OFPUTIL_P_OF10_STD_TID : OFPUTIL_P_OF10_STD; - case OFPUTIL_P_NXM: - case OFPUTIL_P_NXM_TID: - return enable ? OFPUTIL_P_NXM_TID : OFPUTIL_P_NXM; + case OFPUTIL_P_OF10_NXM: + case OFPUTIL_P_OF10_NXM_TID: + return enable ? OFPUTIL_P_OF10_NXM_TID : OFPUTIL_P_OF10_NXM; - case OFPUTIL_P_OF12: - return OFPUTIL_P_OF12; + case OFPUTIL_P_OF12_OXM: + return OFPUTIL_P_OF12_OXM; default: NOT_REACHED(); @@ -686,16 +686,16 @@ ofputil_protocol_set_base(enum ofputil_protocol cur, bool tid = (cur & OFPUTIL_P_TID) != 0; switch (new_base) { - case OFPUTIL_P_OF10: - case OFPUTIL_P_OF10_TID: - return ofputil_protocol_set_tid(OFPUTIL_P_OF10, tid); + case OFPUTIL_P_OF10_STD: + case OFPUTIL_P_OF10_STD_TID: + return ofputil_protocol_set_tid(OFPUTIL_P_OF10_STD, tid); - case OFPUTIL_P_NXM: - case OFPUTIL_P_NXM_TID: - return ofputil_protocol_set_tid(OFPUTIL_P_NXM, tid); + case OFPUTIL_P_OF10_NXM: + case OFPUTIL_P_OF10_NXM_TID: + return ofputil_protocol_set_tid(OFPUTIL_P_OF10_NXM, tid); - case OFPUTIL_P_OF12: - return ofputil_protocol_set_tid(OFPUTIL_P_OF12, tid); + case OFPUTIL_P_OF12_OXM: + return ofputil_protocol_set_tid(OFPUTIL_P_OF12_OXM, tid); default: NOT_REACHED(); @@ -713,19 +713,19 @@ ofputil_protocol_to_string(enum ofputil_protocol protocol) /* Use a "switch" statement for single-bit names so that we get a compiler * warning if we forget any. */ switch (protocol) { - case OFPUTIL_P_NXM: + case OFPUTIL_P_OF10_NXM: return "NXM-table_id"; - case OFPUTIL_P_NXM_TID: + case OFPUTIL_P_OF10_NXM_TID: return "NXM+table_id"; - case OFPUTIL_P_OF10: + case OFPUTIL_P_OF10_STD: return "OpenFlow10-table_id"; - case OFPUTIL_P_OF10_TID: + case OFPUTIL_P_OF10_STD_TID: return "OpenFlow10+table_id"; - case OFPUTIL_P_OF12: + case OFPUTIL_P_OF12_OXM: return NULL; } @@ -972,68 +972,68 @@ ofputil_usable_protocols(const struct match *match) /* NXM and OF1.1+ supports bitwise matching on ethernet addresses. */ if (!eth_mask_is_exact(wc->masks.dl_src) && !eth_addr_is_zero(wc->masks.dl_src)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } if (!eth_mask_is_exact(wc->masks.dl_dst) && !eth_addr_is_zero(wc->masks.dl_dst)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* NXM and OF1.1+ support matching metadata. */ if (wc->masks.metadata != htonll(0)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports matching ARP hardware addresses. */ if (!eth_addr_is_zero(wc->masks.arp_sha) || !eth_addr_is_zero(wc->masks.arp_tha)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports matching IPv6 traffic. */ if (match->flow.dl_type == htons(ETH_TYPE_IPV6)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports matching registers. */ if (!regs_fully_wildcarded(wc)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports matching tun_id. */ if (wc->masks.tunnel.tun_id != htonll(0)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports matching fragments. */ if (wc->masks.nw_frag) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports matching IPv6 flow label. */ if (wc->masks.ipv6_label) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports matching IP ECN bits. */ if (wc->masks.nw_tos & IP_ECN_MASK) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports matching IP TTL/hop limit. */ if (wc->masks.nw_ttl) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports non-CIDR IPv4 address masks. */ if (!ip_is_cidr(wc->masks.nw_src) || !ip_is_cidr(wc->masks.nw_dst)) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Only NXM supports bitwise matching on transport port. */ if ((wc->masks.tp_src && wc->masks.tp_src != htons(UINT16_MAX)) || (wc->masks.tp_dst && wc->masks.tp_dst != htons(UINT16_MAX))) { - return OFPUTIL_P_NXM_ANY; + return OFPUTIL_P_OF10_NXM_ANY; } /* Other formats can express this rule. */ @@ -1216,17 +1216,17 @@ ofputil_encode_set_protocol(enum ofputil_protocol current, *next = ofputil_protocol_set_base(current, want_base); switch (want_base) { - case OFPUTIL_P_NXM: + case OFPUTIL_P_OF10_NXM: return ofputil_encode_nx_set_flow_format(NXFF_NXM); - case OFPUTIL_P_OF10: + case OFPUTIL_P_OF10_STD: return ofputil_encode_nx_set_flow_format(NXFF_OPENFLOW10); - case OFPUTIL_P_OF12: + case OFPUTIL_P_OF12_OXM: return ofputil_encode_nx_set_flow_format(NXFF_OPENFLOW12); - case OFPUTIL_P_OF10_TID: - case OFPUTIL_P_NXM_TID: + case OFPUTIL_P_OF10_STD_TID: + case OFPUTIL_P_OF10_NXM_TID: NOT_REACHED(); } } @@ -1268,13 +1268,13 @@ ofputil_nx_flow_format_to_protocol(enum nx_flow_format flow_format) { switch (flow_format) { case NXFF_OPENFLOW10: - return OFPUTIL_P_OF10; + return OFPUTIL_P_OF10_STD; case NXFF_NXM: - return OFPUTIL_P_NXM; + return OFPUTIL_P_OF10_NXM; case NXFF_OPENFLOW12: - return OFPUTIL_P_OF12; + return OFPUTIL_P_OF12_OXM; default: return 0; @@ -1499,7 +1499,7 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm, struct ofpbuf *msg; switch (protocol) { - case OFPUTIL_P_OF12: { + case OFPUTIL_P_OF12_OXM: { struct ofp11_flow_mod *ofm; msg = ofpraw_alloc(OFPRAW_OFPT11_FLOW_MOD, OFP12_VERSION, @@ -1525,8 +1525,8 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm, break; } - case OFPUTIL_P_OF10: - case OFPUTIL_P_OF10_TID: { + case OFPUTIL_P_OF10_STD: + case OFPUTIL_P_OF10_STD_TID: { struct ofp10_flow_mod *ofm; msg = ofpraw_alloc(OFPRAW_OFPT10_FLOW_MOD, OFP10_VERSION, @@ -1545,8 +1545,8 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm, break; } - case OFPUTIL_P_NXM: - case OFPUTIL_P_NXM_TID: { + case OFPUTIL_P_OF10_NXM: + case OFPUTIL_P_OF10_NXM_TID: { struct nx_flow_mod *nfm; int match_len; @@ -1599,7 +1599,7 @@ ofputil_flow_mod_usable_protocols(const struct ofputil_flow_mod *fms, /* 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_OF10_NXM_ANY; } } assert(usable_protocols); @@ -1720,7 +1720,7 @@ ofputil_encode_flow_stats_request(const struct ofputil_flow_stats_request *fsr, enum ofpraw raw; switch (protocol) { - case OFPUTIL_P_OF12: { + case OFPUTIL_P_OF12_OXM: { struct ofp11_flow_stats_request *ofsr; raw = (fsr->aggregate @@ -1737,8 +1737,8 @@ ofputil_encode_flow_stats_request(const struct ofputil_flow_stats_request *fsr, break; } - case OFPUTIL_P_OF10: - case OFPUTIL_P_OF10_TID: { + case OFPUTIL_P_OF10_STD: + case OFPUTIL_P_OF10_STD_TID: { struct ofp10_flow_stats_request *ofsr; raw = (fsr->aggregate @@ -1752,8 +1752,8 @@ ofputil_encode_flow_stats_request(const struct ofputil_flow_stats_request *fsr, break; } - case OFPUTIL_P_NXM: - case OFPUTIL_P_NXM_TID: { + case OFPUTIL_P_OF10_NXM: + case OFPUTIL_P_OF10_NXM_TID: { struct nx_flow_stats_request *nfsr; int match_len; @@ -1791,7 +1791,7 @@ ofputil_flow_stats_request_usable_protocols( usable_protocols = ofputil_usable_protocols(&fsr->match); if (fsr->cookie_mask != htonll(0)) { - usable_protocols &= OFPUTIL_P_NXM_ANY; + usable_protocols &= OFPUTIL_P_OF10_NXM_ANY; } return usable_protocols; } @@ -2203,7 +2203,7 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr, struct ofpbuf *msg; switch (protocol) { - case OFPUTIL_P_OF12: { + case OFPUTIL_P_OF12_OXM: { struct ofp12_flow_removed *ofr; msg = ofpraw_alloc_xid(OFPRAW_OFPT11_FLOW_REMOVED, @@ -2224,8 +2224,8 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr, break; } - case OFPUTIL_P_OF10: - case OFPUTIL_P_OF10_TID: { + case OFPUTIL_P_OF10_STD: + case OFPUTIL_P_OF10_STD_TID: { struct ofp_flow_removed *ofr; msg = ofpraw_alloc_xid(OFPRAW_OFPT10_FLOW_REMOVED, OFP10_VERSION, @@ -2243,8 +2243,8 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr, break; } - case OFPUTIL_P_NXM: - case OFPUTIL_P_NXM_TID: { + case OFPUTIL_P_OF10_NXM: + case OFPUTIL_P_OF10_NXM_TID: { struct nx_flow_removed *nfr; int match_len; @@ -2396,7 +2396,7 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin, struct ofpbuf *packet; /* Add OFPT_PACKET_IN. */ - if (protocol == OFPUTIL_P_OF12) { + if (protocol == OFPUTIL_P_OF12_OXM) { struct ofp12_packet_in *opi; struct match match; diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 4bd5a001..a081cbf0 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -45,12 +45,15 @@ ovs_be32 ofputil_wcbits_to_netmask(int wcbits); int ofputil_netmask_to_wcbits(ovs_be32 netmask); /* Protocols. + * + * A "protocol" is an OpenFlow version plus, for some OpenFlow versions, + * a bit extra about the flow match format in use. * * These are arranged from most portable to least portable, or alternatively - * from least powerful to most powerful. Formats earlier on the list are more - * likely to be understood for the purpose of making requests, but formats - * later on the list are more likely to accurately describe a flow within a - * switch. + * from least powerful to most powerful. Protocols earlier on the list are + * more likely to be understood for the purpose of making requests, but + * protocol later on the list are more likely to accurately describe a flow + * within a switch. * * On any given OpenFlow connection, a single protocol is in effect at any * given time. These values use separate bits only because that makes it easy @@ -58,24 +61,35 @@ int ofputil_netmask_to_wcbits(ovs_be32 netmask); * to implement set union and intersection. */ enum ofputil_protocol { - /* OpenFlow 1.0-based protocols. */ - OFPUTIL_P_OF10 = 1 << 0, /* OpenFlow 1.0 flow format. */ - OFPUTIL_P_OF10_TID = 1 << 1, /* OF1.0 + flow_mod_table_id extension. */ -#define OFPUTIL_P_OF10_ANY (OFPUTIL_P_OF10 | OFPUTIL_P_OF10_TID) - - /* OpenFlow 1.0 with NXM-based flow formats. */ - OFPUTIL_P_NXM = 1 << 2, /* Nicira extended match. */ - OFPUTIL_P_NXM_TID = 1 << 3, /* NXM + flow_mod_table_id extension. */ -#define OFPUTIL_P_NXM_ANY (OFPUTIL_P_NXM | OFPUTIL_P_NXM_TID) - - /* OpenFlow 1.2 */ - OFPUTIL_P_OF12 = 1 << 4, /* OpenFlow 1.2 flow format. */ + /* OpenFlow 1.0 protocols. + * + * The "STD" protocols use the standard OpenFlow 1.0 flow format. + * The "NXM" protocols use the Nicira Extensible Match (NXM) flow format. + * + * The protocols with "TID" mean that the nx_flow_mod_table_id Nicira + * extension has been enabled. The other protocols have it disabled. + */ + OFPUTIL_P_OF10_STD = 1 << 0, + OFPUTIL_P_OF10_STD_TID = 1 << 1, + OFPUTIL_P_OF10_NXM = 1 << 2, + OFPUTIL_P_OF10_NXM_TID = 1 << 3, +#define OFPUTIL_P_OF10_STD_ANY (OFPUTIL_P_OF10_STD | OFPUTIL_P_OF10_STD_TID) +#define OFPUTIL_P_OF10_NXM_ANY (OFPUTIL_P_OF10_NXM | OFPUTIL_P_OF10_NXM_TID) + + /* OpenFlow 1.2 protocol (only one variant). + * + * This uses the standard OpenFlow Extensible Match (OXM) flow format. + * + * OpenFlow 1.2 always operates with an equivalent of the + * nx_flow_mod_table_id Nicira extension enabled, so there is no "TID" + * variant. */ + OFPUTIL_P_OF12_OXM = 1 << 4, /* All protocols. */ -#define OFPUTIL_P_ANY (OFPUTIL_P_OF10_ANY | OFPUTIL_P_NXM_ANY) +#define OFPUTIL_P_ANY (OFPUTIL_P_OF10_STD_ANY | OFPUTIL_P_OF10_NXM_ANY) /* Protocols in which a specific table may be specified in flow_mods. */ -#define OFPUTIL_P_TID (OFPUTIL_P_OF10_TID | OFPUTIL_P_NXM_TID) +#define OFPUTIL_P_TID (OFPUTIL_P_OF10_STD_TID | OFPUTIL_P_OF10_NXM_TID) }; /* Protocols to use for flow dumps, from most to least preferred. */ diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index ba93a1d5..28858046 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -840,7 +840,7 @@ ofconn_get_invalid_ttl_to_controller(struct ofconn *ofconn) /* Returns the currently configured protocol for 'ofconn', one of OFPUTIL_P_*. * - * The default, if no other format has been set, is OFPUTIL_P_OPENFLOW10. */ + * The default, if no other format has been set, is OFPUTIL_P_OF10_STD. */ enum ofputil_protocol ofconn_get_protocol(struct ofconn *ofconn) { @@ -1034,7 +1034,7 @@ ofconn_flush(struct ofconn *ofconn) int i; ofconn->role = NX_ROLE_OTHER; - ofconn->protocol = OFPUTIL_P_OF10; + ofconn->protocol = OFPUTIL_P_OF10_STD; ofconn->packet_in_format = NXPIF_OPENFLOW10; /* Disassociate 'ofconn' from all of the ofopgroups that it initiated that