From: Ben Pfaff Date: Mon, 24 Jun 2013 20:49:40 +0000 (-0700) Subject: ofp-errors: Implement OpenFlow 1.2+ experimenter error codes. X-Git-Tag: sliver-openvswitch-1.10.90-3~6^2~57 X-Git-Url: http://git.onelab.eu/?p=sliver-openvswitch.git;a=commitdiff_plain;h=514887ee4666894c4d0b93960ce3d33515611fa9 ofp-errors: Implement OpenFlow 1.2+ experimenter error codes. OpenFlow 1.2 standardized experimenter error codes in a way different from the Nicira extension. This commit implements the OpenFlow 1.2+ version. This commit also makes it easy to add error codes for new experimenter IDs by adding new *_VENDOR_ID definitions to openflow-common.h. Signed-off-by: Ben Pfaff --- diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+ index c4bcc3400..fb398730f 100644 --- a/OPENFLOW-1.1+ +++ b/OPENFLOW-1.1+ @@ -92,9 +92,6 @@ OpenFlow 1.2 support requires OpenFlow 1.1 as a prerequisite, plus the following additional work. (This is based on the change log at the end of the OF1.2 spec. I didn't compare the specs carefully yet.) - * Use new OpenFlow extensible error infrastructure, on OF1.2+ - only, instead of the OVS-specific extension used until now. - * OFPT_FLOW_MOD: * MODIFY and MODIFY_STRICT commands now never insert new flows diff --git a/build-aux/extract-ofp-errors b/build-aux/extract-ofp-errors index 32d0913c4..6e86252ad 100755 --- a/build-aux/extract-ofp-errors +++ b/build-aux/extract-ofp-errors @@ -6,6 +6,13 @@ import re macros = {} +# Map from OpenFlow version number to version ID used in ofp_header. +version_map = {"1.0": 0x01, + "1.1": 0x02, + "1.2": 0x03, + "1.3": 0x04} +version_reverse_map = dict((v, k) for (k, v) in version_map.iteritems()) + token = None line = "" idRe = "[a-zA-Z_][a-zA-Z_0-9]*" @@ -13,12 +20,24 @@ tokenRe = "#?" + idRe + "|[0-9]+|." inComment = False inDirective = False -def getLine(): +def open_file(fn): + global fileName + global inputFile + global lineNumber + fileName = fn + inputFile = open(fileName) + lineNumber = 0 + +def tryGetLine(): + global inputFile global line global lineNumber line = inputFile.readline() lineNumber += 1 - if line == "": + return line != "" + +def getLine(): + if not tryGetLine(): fatal("unexpected end of input") def getToken(): @@ -133,146 +152,185 @@ def usage(): argv0 = os.path.basename(sys.argv[0]) print ('''\ %(argv0)s, for extracting OpenFlow error codes from header files -usage: %(argv0)s FILE [FILE...] +usage: %(argv0)s ERROR_HEADER VENDOR_HEADER -This program reads the header files specified on the command line and -outputs a C source file for translating OpenFlow error codes into -strings, for use as lib/ofp-errors.c in the Open vSwitch source tree. +This program reads VENDOR_HEADER to obtain OpenFlow vendor (aka +experimenter IDs), then ERROR_HEADER to obtain OpenFlow error number. +It outputs a C source file for translating OpenFlow error codes into +strings. -This program is specialized for reading lib/ofp-errors.h. It will not -work on arbitrary header files without extensions.\ +ERROR_HEADER should point to lib/ofp-errors.h. +VENDOR_HEADER should point to include/openflow/openflow-common.h. +The output is suitable for use as lib/ofp-errors.inc.\ ''' % {"argv0": argv0}) sys.exit(0) -def extract_ofp_errors(filenames): +def extract_vendor_ids(fn): + global vendor_map + vendor_map = {} + vendor_loc = {} + + open_file(fn) + while tryGetLine(): + m = re.match(r'#define\s+([A-Z0-9_]+)_VENDOR_ID\s+(0x[0-9a-fA-F]+|[0-9]+)', line) + if not m: + continue + + name = m.group(1) + id_ = int(m.group(2), 0) + + if name in vendor_map: + error("%s: duplicate definition of vendor" % name) + sys.stderr.write("%s: Here is the location of the previous " + "definition.\n" % vendor_loc[name]) + sys.exit(1) + + vendor_map[name] = id_ + vendor_loc[name] = "%s:%d" % (fileName, lineNumber) + + if not vendor_map: + fatal("%s: no vendor definitions found" % fn) + + inputFile.close() + + vendor_reverse_map = {} + for name, id_ in vendor_map.items(): + if id_ in vendor_reverse_map: + fatal("%s: duplicate vendor id for vendors %s and %s" + % (id_, vendor_reverse_map[id_], name)) + vendor_reverse_map[id_] = name + +def extract_ofp_errors(fn): error_types = {} comments = [] names = [] domain = {} reverse = {} - for domain_name in ("OF1.0", "OF1.1", "OF1.2", "OF1.3", - "NX1.0", "NX1.1", "NX1.2", "NX1.3"): + for domain_name in version_map.values(): domain[domain_name] = {} reverse[domain_name] = {} n_errors = 0 expected_errors = {} - global fileName - for fileName in filenames: - global inputFile - global lineNumber - inputFile = open(fileName) - lineNumber = 0 + open_file(fn) - while True: - getLine() - if re.match('enum ofperr', line): - break + while True: + getLine() + if re.match('enum ofperr', line): + break + + while True: + getLine() + if line.startswith('/*') or not line or line.isspace(): + continue + elif re.match('}', line): + break + + if not line.lstrip().startswith('/*'): + fatal("unexpected syntax between errors") - while True: + comment = line.lstrip()[2:].strip() + while not comment.endswith('*/'): getLine() if line.startswith('/*') or not line or line.isspace(): - continue - elif re.match('}', line): - break - - if not line.lstrip().startswith('/*'): - fatal("unexpected syntax between errors") - - comment = line.lstrip()[2:].strip() - while not comment.endswith('*/'): - getLine() - if line.startswith('/*') or not line or line.isspace(): - fatal("unexpected syntax within error") - comment += ' %s' % line.lstrip('* \t').rstrip(' \t\r\n') - comment = comment[:-2].rstrip() - - m = re.match('Expected: (.*)\.$', comment) - if m: - expected_errors[m.group(1)] = (fileName, lineNumber) - continue + fatal("unexpected syntax within error") + comment += ' %s' % line.lstrip('* \t').rstrip(' \t\r\n') + comment = comment[:-2].rstrip() - m = re.match('((?:.(?!\. ))+.)\. (.*)$', comment) - if not m: - fatal("unexpected syntax between errors") + m = re.match('Expected: (.*)\.$', comment) + if m: + expected_errors[m.group(1)] = (fileName, lineNumber) + continue - dsts, comment = m.groups() + m = re.match('((?:.(?!\. ))+.)\. (.*)$', comment) + if not m: + fatal("unexpected syntax between errors") - getLine() - m = re.match('\s+(?:OFPERR_((?:OFP|NX)[A-Z0-9_]+))(\s*=\s*OFPERR_OFS)?,', - line) - if not m: - fatal("syntax error expecting enum value") + dsts, comment = m.groups() - enum = m.group(1) + getLine() + m = re.match('\s+(?:OFPERR_([A-Z0-9_]+))(\s*=\s*OFPERR_OFS)?,', + line) + if not m: + fatal("syntax error expecting enum value") - comments.append(re.sub('\[[^]]*\]', '', comment)) - names.append(enum) + enum = m.group(1) + if enum in names: + fatal("%s specified twice" % enum) - for dst in dsts.split(', '): - m = re.match(r'([A-Z0-9.+]+)\((\d+)(?:,(\d+))?\)$', dst) - if not m: - fatal("%s: syntax error in destination" % dst) - targets = m.group(1) - type_ = int(m.group(2)) - if m.group(3): - code = int(m.group(3)) - else: - code = None - - target_map = {"OF1.0+": ("OF1.0", "OF1.1", "OF1.2", "OF1.3"), - "OF1.1+": ("OF1.1", "OF1.2", "OF1.3"), - "OF1.2+": ("OF1.2", "OF1.3"), - "OF1.3+": ("OF1.3",), - "OF1.0": ("OF1.0",), - "OF1.1": ("OF1.1",), - "OF1.2": ("OF1.2",), - "OF1.3": ("OF1.3",), - "NX1.0+": ("OF1.0", "OF1.1", "OF1.2", "OF1.3"), - "NX1.1+": ("OF1.1", "OF1.2", "OF1.3"), - "NX1.2+": ("OF1.2", "OF1.3"), - "NX1.3+": ("OF1.3",), - "NX1.0": ("OF1.0",), - "NX1.1": ("OF1.1",), - "NX1.2": ("OF1.2",), - "NX1.3": ("OF1.3",)} - if targets not in target_map: - fatal("%s: unknown error domain" % targets) - if targets.startswith('NX') and code < 0x100: - fatal("%s: NX domain code cannot be less than 0x100" % dst) - if targets.startswith('OF') and code >= 0x100: - fatal("%s: OF domain code cannot be greater than 0x100" - % dst) - for target in target_map[targets]: - domain[target].setdefault(type_, {}) - if code in domain[target][type_]: - msg = "%d,%d in %s means both %s and %s" % ( - type_, code, target, - domain[target][type_][code][0], enum) - if msg in expected_errors: - del expected_errors[msg] - else: - error("%s: %s." % (dst, msg)) - sys.stderr.write("%s:%d: %s: Here is the location " - "of the previous definition.\n" - % (domain[target][type_][code][1], - domain[target][type_][code][2], - dst)) + comments.append(re.sub('\[[^]]*\]', '', comment)) + names.append(enum) + + for dst in dsts.split(', '): + m = re.match(r'([A-Z]+)([0-9.]+)(\+|-[0-9.]+)?\((\d+)(?:,(\d+))?\)$', dst) + if not m: + fatal("%r: syntax error in destination" % dst) + vendor_name = m.group(1) + version1_name = m.group(2) + version2_name = m.group(3) + type_ = int(m.group(4)) + if m.group(5): + code = int(m.group(5)) + else: + code = None + + if vendor_name not in vendor_map: + fatal("%s: unknown vendor" % vendor_name) + vendor = vendor_map[vendor_name] + + if version1_name not in version_map: + fatal("%s: unknown OpenFlow version" % version1_name) + v1 = version_map[version1_name] + + if version2_name is None: + v2 = v1 + elif version2_name == "+": + v2 = max(version_map.values()) + elif version2_name[1:] not in version_map: + fatal("%s: unknown OpenFlow version" % version2_name[1:]) + else: + v2 = version_map[version2_name[1:]] + + if v2 < v1: + fatal("%s%s: %s precedes %s" + % (version1_name, version2_name, + version2_name, version1_name)) + + if vendor == vendor_map['NX']: + if v1 >= version_map['1.2'] or v2 >= version_map['1.2']: + if code is not None: + fatal("%s: NX1.2+ domains do not have codes" % dst) + code = 0 + elif vendor != vendor_map['OF']: + if code is not None: + fatal("%s: %s domains do not have codes" % vendor_name) + + for version in range(v1, v2 + 1): + domain[version].setdefault(vendor, {}) + domain[version][vendor].setdefault(type_, {}) + if code in domain[version][vendor][type_]: + msg = "%#x,%d,%d in OF%s means both %s and %s" % ( + vendor, type_, code, version_reverse_map[version], + domain[version][vendor][type_][code][0], enum) + if msg in expected_errors: + del expected_errors[msg] else: - domain[target][type_][code] = (enum, fileName, - lineNumber) + error("%s: %s." % (dst, msg)) + sys.stderr.write("%s:%d: %s: Here is the location " + "of the previous definition.\n" + % (domain[version][vendor][type_][code][1], + domain[version][vendor][type_][code][2], + dst)) + else: + domain[version][vendor][type_][code] = (enum, fileName, + lineNumber) - if enum in reverse[target]: - error("%s: %s in %s means both %d,%d and %d,%d." % - (dst, enum, target, - reverse[target][enum][0], - reverse[target][enum][1], - type_, code)) - reverse[target][enum] = (type_, code) + assert enum not in reverse[version] + reverse[version][enum] = (vendor, type_, code) - inputFile.close() + inputFile.close() for fn, ln in expected_errors.values(): sys.stderr.write("%s:%d: expected duplicate not used.\n" % (fn, ln)) @@ -289,8 +347,8 @@ def extract_ofp_errors(filenames): struct ofperr_domain { const char *name; uint8_t version; - enum ofperr (*decode)(uint16_t type, uint16_t code); - struct pair errors[OFPERR_N_ERRORS]; + enum ofperr (*decode)(uint32_t vendor, uint16_t type, uint16_t code); + struct triplet errors[OFPERR_N_ERRORS]; }; static const char *error_names[OFPERR_N_ERRORS] = { @@ -308,21 +366,25 @@ static const char *error_comments[OFPERR_N_ERRORS] = { def output_domain(map, name, description, version): print (""" static enum ofperr -%s_decode(uint16_t type, uint16_t code) +%s_decode(uint32_t vendor, uint16_t type, uint16_t code) { - switch ((type << 16) | code) {""" % name) + switch (((uint64_t) vendor << 32) | (type << 16) | code) {""" % name) found = set() for enum in names: if enum not in map: continue - type_, code = map[enum] + vendor, type_, code = map[enum] if code is None: continue - value = (type_ << 16) | code + value = (vendor << 32) | (type_ << 16) | code if value in found: continue found.add(value) - print (" case (%d << 16) | %d:" % (type_, code)) + if vendor: + vendor_s = "(%#xULL << 32) | " % vendor + else: + vendor_s = "" + print (" case %s(%d << 16) | %d:" % (vendor_s, type_, code)) print (" return OFPERR_%s;" % enum) print ("""\ } @@ -338,27 +400,28 @@ static const struct ofperr_domain %s = { {""" % (name, description, version, name)) for enum in names: if enum in map: - type_, code = map[enum] + vendor, type_, code = map[enum] if code == None: code = -1 + print " { %#8x, %2d, %3d }, /* %s */" % (vendor, type_, code, enum) else: - type_ = code = -1 - print (" { %2d, %3d }, /* %s */" % (type_, code, enum)) + print (" { -1, -1, -1 }, /* %s */" % enum) print ("""\ }, };""") - output_domain(reverse["OF1.0"], "ofperr_of10", "OpenFlow 1.0", 0x01) - output_domain(reverse["OF1.1"], "ofperr_of11", "OpenFlow 1.1", 0x02) - output_domain(reverse["OF1.2"], "ofperr_of12", "OpenFlow 1.2", 0x03) - output_domain(reverse["OF1.3"], "ofperr_of13", "OpenFlow 1.3", 0x04) + for version_name, id_ in version_map.items(): + var = 'ofperr_of' + re.sub('[^A-Za-z0-9_]', '', version_name) + description = "OpenFlow %s" % version_name + output_domain(reverse[id_], var, description, id_) if __name__ == '__main__': if '--help' in sys.argv: usage() - elif len(sys.argv) < 2: - sys.stderr.write("at least one non-option argument required; " + elif len(sys.argv) != 3: + sys.stderr.write("exactly two non-options arguments required; " "use --help for help\n") sys.exit(1) else: - extract_ofp_errors(sys.argv[1:]) + extract_vendor_ids(sys.argv[2]) + extract_ofp_errors(sys.argv[1]) diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index 5ca343e1a..6319f4e77 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -24,7 +24,6 @@ * standardized, so they are not included in openflow.h. Some of them may be * suitable for standardization; others we never expect to standardize. */ -#define NX_VENDOR_ID 0x00002320 /* Nicira vendor-specific error messages extension. * diff --git a/include/openflow/openflow-1.2.h b/include/openflow/openflow-1.2.h index 554631340..29770472e 100644 --- a/include/openflow/openflow-1.2.h +++ b/include/openflow/openflow-1.2.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, 2011, 2012 The Board of Trustees of The Leland Stanford +/* Copyright (c) 2008, 2011, 2012, 2013 The Board of Trustees of The Leland Stanford * Junior University * * We are making the OpenFlow specification and associated documentation @@ -55,6 +55,9 @@ #include "openflow/openflow-1.1.h" +/* Error type for experimenter error messages. */ +#define OFPET12_EXPERIMENTER 0xffff + /* * OXM Class IDs. * The high order bit differentiate reserved classes from member classes. diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h index ee7913650..d3b4e6194 100644 --- a/include/openflow/openflow-common.h +++ b/include/openflow/openflow-common.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, 2011, 2012 The Board of Trustees of The Leland Stanford +/* Copyright (c) 2008, 2011, 2012, 2013 The Board of Trustees of The Leland Stanford * Junior University * * We are making the OpenFlow specification and associated documentation @@ -78,6 +78,29 @@ enum ofp_version { OFP13_VERSION = 0x04 }; +/* Vendor (aka experimenter) IDs. + * + * These are used in various places in OpenFlow to identify an extension + * defined by some vendor, as opposed to a standardized part of the core + * OpenFlow protocol. + * + * Vendor IDs whose top 8 bits are 0 hold an Ethernet OUI in their low 24 bits. + * The Open Networking Foundation assigns vendor IDs whose top 8 bits are + * nonzero. + * + * A few vendor IDs are special: + * + * - OF_VENDOR_ID is not a real vendor ID and does not appear in the + * OpenFlow protocol itself. It can occasionally be useful within Open + * vSwitch to identify a standardized part of OpenFlow. + * + * - ONF_VENDOR_ID is being used within the ONF "extensibility" working + * group to identify extensions being proposed for standardization. + */ +#define OF_VENDOR_ID 0 +#define NX_VENDOR_ID 0x00002320 /* Nicira. */ +#define ONF_VENDOR_ID 0x4f4e4600 /* Open Networking Foundation. */ + #define OFP_MAX_TABLE_NAME_LEN 32 #define OFP_MAX_PORT_NAME_LEN 16 diff --git a/lib/automake.mk b/lib/automake.mk index eec71dd6d..ed4b9929c 100644 --- a/lib/automake.mk +++ b/lib/automake.mk @@ -336,9 +336,12 @@ lib/dirs.c: lib/dirs.c.in Makefile mv lib/dirs.c.tmp lib/dirs.c $(srcdir)/lib/ofp-errors.inc: \ - lib/ofp-errors.h $(srcdir)/build-aux/extract-ofp-errors + lib/ofp-errors.h include/openflow/openflow-common.h \ + $(srcdir)/build-aux/extract-ofp-errors $(run_python) $(srcdir)/build-aux/extract-ofp-errors \ - $(srcdir)/lib/ofp-errors.h > $@.tmp && mv $@.tmp $@ + $(srcdir)/lib/ofp-errors.h \ + $(srcdir)/include/openflow/openflow-common.h > $@.tmp + mv $@.tmp $@ $(srcdir)/lib/ofp-errors.c: $(srcdir)/lib/ofp-errors.inc EXTRA_DIST += build-aux/extract-ofp-errors lib/ofp-errors.inc diff --git a/lib/ofp-errors.c b/lib/ofp-errors.c index a141f3f51..52856458c 100644 --- a/lib/ofp-errors.c +++ b/lib/ofp-errors.c @@ -27,7 +27,8 @@ VLOG_DEFINE_THIS_MODULE(ofp_errors); -struct pair { +struct triplet { + uint32_t vendor; int type, code; }; @@ -73,10 +74,11 @@ ofperr_is_valid(enum ofperr error) * 'version', or 0 if either no such OFPERR_* value exists or 'version' is * unknown. */ static enum ofperr -ofperr_decode(enum ofp_version version, uint16_t type, uint16_t code) +ofperr_decode(enum ofp_version version, + uint32_t vendor, uint16_t type, uint16_t code) { const struct ofperr_domain *domain = ofperr_domain_from_version(version); - return domain ? domain->decode(type, code) : 0; + return domain ? domain->decode(vendor, type, code) : 0; } /* Returns the name of 'error', e.g. "OFPBRC_BAD_TYPE" if 'error' is @@ -121,8 +123,8 @@ ofperr_get_description(enum ofperr error) : ""); } -static const struct pair * -ofperr_get_pair__(enum ofperr error, const struct ofperr_domain *domain) +static const struct triplet * +ofperr_get_triplet__(enum ofperr error, const struct ofperr_domain *domain) { size_t ofs = error - OFPERR_OFS; @@ -136,8 +138,8 @@ ofperr_encode_msg__(enum ofperr error, enum ofp_version ofp_version, { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); const struct ofperr_domain *domain; + const struct triplet *triplet; struct ofp_error_msg *oem; - const struct pair *pair; struct ofpbuf *buf; /* Get the error domain for 'ofp_version', or fall back to OF1.0. */ @@ -160,15 +162,15 @@ ofperr_encode_msg__(enum ofperr error, enum ofp_version ofp_version, error = OFPERR_NXBRC_UNENCODABLE_ERROR; } - pair = ofperr_get_pair__(error, domain); - if (pair->code < 0x100) { + triplet = ofperr_get_triplet__(error, domain); + if (!triplet->vendor) { buf = ofpraw_alloc_xid(OFPRAW_OFPT_ERROR, domain->version, xid, sizeof *oem + data_len); oem = ofpbuf_put_uninit(buf, sizeof *oem); - oem->type = htons(pair->type); - oem->code = htons(pair->code); - } else { + oem->type = htons(triplet->type); + oem->code = htons(triplet->code); + } else if (ofp_version <= OFP11_VERSION) { struct nx_vendor_error *nve; buf = ofpraw_alloc_xid(OFPRAW_OFPT_ERROR, domain->version, xid, @@ -179,9 +181,19 @@ ofperr_encode_msg__(enum ofperr error, enum ofp_version ofp_version, oem->code = htons(NXVC_VENDOR_ERROR); nve = ofpbuf_put_uninit(buf, sizeof *nve); - nve->vendor = htonl(NX_VENDOR_ID); - nve->type = htons(pair->type); - nve->code = htons(pair->code); + nve->vendor = htonl(triplet->vendor); + nve->type = htons(triplet->type); + nve->code = htons(triplet->code); + } else { + ovs_be32 vendor = htonl(triplet->vendor); + + buf = ofpraw_alloc_xid(OFPRAW_OFPT_ERROR, domain->version, xid, + sizeof *oem + sizeof(uint32_t) + data_len); + + oem = ofpbuf_put_uninit(buf, sizeof *oem); + oem->type = htons(OFPET12_EXPERIMENTER); + oem->code = htons(triplet->type); + ofpbuf_put(buf, &vendor, sizeof vendor); } ofpbuf_put(buf, data, data_len); @@ -222,6 +234,13 @@ ofperr_encode_hello(enum ofperr error, enum ofp_version ofp_version, return ofperr_encode_msg__(error, ofp_version, htonl(0), s, strlen(s)); } +int +ofperr_get_vendor(enum ofperr error, enum ofp_version version) +{ + const struct ofperr_domain *domain = ofperr_domain_from_version(version); + return domain ? ofperr_get_triplet__(error, domain)->vendor : -1; +} + /* Returns the value that would go into an OFPT_ERROR message's 'type' for * encoding 'error' in 'domain'. Returns -1 if 'error' is not encodable in * 'version' or 'version' is unknown. @@ -231,7 +250,7 @@ int ofperr_get_type(enum ofperr error, enum ofp_version version) { const struct ofperr_domain *domain = ofperr_domain_from_version(version); - return domain ? ofperr_get_pair__(error, domain)->type : -1; + return domain ? ofperr_get_triplet__(error, domain)->type : -1; } /* Returns the value that would go into an OFPT_ERROR message's 'code' for @@ -245,7 +264,7 @@ int ofperr_get_code(enum ofperr error, enum ofp_version version) { const struct ofperr_domain *domain = ofperr_domain_from_version(version); - return domain ? ofperr_get_pair__(error, domain)->code : -1; + return domain ? ofperr_get_triplet__(error, domain)->code : -1; } /* Tries to decode 'oh', which should be an OpenFlow OFPT_ERROR message. @@ -256,12 +275,11 @@ ofperr_get_code(enum ofperr error, enum ofp_version version) enum ofperr ofperr_decode_msg(const struct ofp_header *oh, struct ofpbuf *payload) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); - const struct ofp_error_msg *oem; enum ofpraw raw; uint16_t type, code; enum ofperr error; + uint32_t vendor; struct ofpbuf b; if (payload) { @@ -277,6 +295,7 @@ ofperr_decode_msg(const struct ofp_header *oh, struct ofpbuf *payload) oem = ofpbuf_pull(&b, sizeof *oem); /* Get the error type and code. */ + vendor = 0; type = ntohs(oem->type); code = ntohs(oem->code); if (type == NXET_VENDOR && code == NXVC_VENDOR_ERROR) { @@ -285,17 +304,22 @@ ofperr_decode_msg(const struct ofp_header *oh, struct ofpbuf *payload) return 0; } - if (nve->vendor != htonl(NX_VENDOR_ID)) { - VLOG_WARN_RL(&rl, "error contains unknown vendor ID %#"PRIx32, - ntohl(nve->vendor)); - return 0; - } + vendor = ntohl(nve->vendor); type = ntohs(nve->type); code = ntohs(nve->code); + } else if (type == OFPET12_EXPERIMENTER) { + const ovs_be32 *vendorp = ofpbuf_try_pull(&b, sizeof *vendorp); + if (!vendorp) { + return 0; + } + + vendor = ntohl(*vendorp); + type = code; + code = 0; } /* Translate the error type and code into an ofperr. */ - error = ofperr_decode(oh->version, type, code); + error = ofperr_decode(oh->version, vendor, type, code); if (error && payload) { ofpbuf_use_const(payload, b.data, b.size); } diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h index 1f7ea69ba..d14479364 100644 --- a/lib/ofp-errors.h +++ b/lib/ofp-errors.h @@ -51,8 +51,22 @@ struct ofpbuf; * time and used to determine the mapping between "enum ofperr" constants and * error type/code values used in the OpenFlow protocol: * - * - The first part of each comment specifies OpenFlow type/code for each - * protocol that supports the error. + * - The first part of each comment specifies the vendor, OpenFlow versions, + * type, and sometimes a code for each protocol that supports the error: + * + * # The vendor is OF for standard OpenFlow error codes. Otherwise it + * is one of the *_VENDOR_ID codes defined in openflow-common.h. + * + * # The version can specify a specific OpenFlow version, a version + * range delimited by "-", or an open-ended range with "+". + * + * # Standard OpenFlow errors have both a type and a code. Extension + * errors generally have only a type, no code. There is one + * exception: Nicira extension (NX) errors for OpenFlow 1.0 and 1.1 + * have both a type and a code. (This means that the version + * specification for NX errors may not include version 1.0 or 1.1 (or + * both) along with version 1.2 or later, because the requirements + * for those versions are different.) * * - Additional text is a human-readable description of the meaning of each * error, used to explain the error to the user. Any text enclosed in @@ -61,7 +75,7 @@ struct ofpbuf; enum ofperr { /* Expected duplications. */ - /* Expected: 3,5 in OF1.1 means both OFPBIC_BAD_EXPERIMENTER and + /* Expected: 0x0,3,5 in OF1.1 means both OFPBIC_BAD_EXPERIMENTER and * OFPBIC_BAD_EXP_TYPE. */ /* ## ------------------ ## */ @@ -115,10 +129,10 @@ enum ofperr { /* OF1.2+(1,10). Denied because controller is slave. */ OFPERR_OFPBRC_IS_SLAVE, - /* NX1.0(1,514), NX1.1(1,514), OF1.2+(1,11). Invalid port. - * [ A non-standard error (1,514), formerly - * OFPERR_NXBRC_BAD_IN_PORT is used for OpenFlow 1.0 and 1.1 as there - * seems to be no appropriate error code defined the specifications. ] */ + /* NX1.0-1.1(1,514), OF1.2+(1,11). Invalid port. [ A non-standard error + * (1,514), formerly OFPERR_NXBRC_BAD_IN_PORT is used for OpenFlow 1.0 and + * 1.1 as there seems to be no appropriate error code defined the + * specifications. ] */ OFPERR_OFPBRC_BAD_PORT, /* OF1.2+(1,12). Invalid packet in packet-out. */ @@ -127,41 +141,43 @@ enum ofperr { /* OF1.3+(1,13). Multipart request overflowed the assigned buffer. */ OFPERR_OFPBRC_MULTIPART_BUFFER_OVERFLOW, - /* NX1.0+(1,256). Invalid NXM flow match. */ + /* NX1.0-1.1(1,256), NX1.2+(2). Invalid NXM flow match. */ OFPERR_NXBRC_NXM_INVALID, - /* NX1.0+(1,257). The nxm_type, or nxm_type taken in combination with - * nxm_hasmask or nxm_length or both, is invalid or not implemented. */ + /* NX1.0-1.1(1,257), NX1.2+(3). The nxm_type, or nxm_type taken in + * combination with nxm_hasmask or nxm_length or both, is invalid or not + * implemented. */ OFPERR_NXBRC_NXM_BAD_TYPE, - /* NX1.0+(1,515). Must-be-zero field had nonzero value. */ + /* NX1.0-1.1(1,515), NX1.2+(4). Must-be-zero field had nonzero value. */ OFPERR_NXBRC_MUST_BE_ZERO, - /* NX1.0+(1,516). The reason in an ofp_port_status message is not - * valid. */ + /* NX1.0-1.1(1,516), NX1.2+(5). The reason in an ofp_port_status message + * is not valid. */ OFPERR_NXBRC_BAD_REASON, - /* NX1.0+(1,517). The 'id' in an NXST_FLOW_MONITOR request is the same as - * an existing monitor id (or two monitors in the same NXST_FLOW_MONITOR - * request have the same 'id'). */ + /* NX1.0-1.1(1,517), NX1.2+(6). The 'id' in an NXST_FLOW_MONITOR request + * is the same as an existing monitor id (or two monitors in the same + * NXST_FLOW_MONITOR request have the same 'id'). */ OFPERR_NXBRC_FM_DUPLICATE_ID, - /* NX1.0+(1,518). The 'flags' in an NXST_FLOW_MONITOR request either does - * not specify at least one of the NXFMF_ADD, NXFMF_DELETE, or NXFMF_MODIFY - * flags, or specifies a flag bit that is not defined. */ + /* NX1.0-1.1(1,518), NX1.2+(7). The 'flags' in an NXST_FLOW_MONITOR + * request either does not specify at least one of the NXFMF_ADD, + * NXFMF_DELETE, or NXFMF_MODIFY flags, or specifies a flag bit that is not + * defined. */ OFPERR_NXBRC_FM_BAD_FLAGS, - /* NX1.0+(1,519). The 'id' in an NXT_FLOW_MONITOR_CANCEL request is not - * the id of any existing monitor. */ + /* NX1.0-1.1(1,519), NX1.2+(8). The 'id' in an NXT_FLOW_MONITOR_CANCEL + * request is not the id of any existing monitor. */ OFPERR_NXBRC_FM_BAD_ID, - /* NX1.0+(1,520). The 'event' in an NXST_FLOW_MONITOR reply does not - * specify one of the NXFME_ABBREV, NXFME_ADD, NXFME_DELETE, or + /* NX1.0-1.1(1,520), NX1.2+(9). The 'event' in an NXST_FLOW_MONITOR reply + * does not specify one of the NXFME_ABBREV, NXFME_ADD, NXFME_DELETE, or * NXFME_MODIFY. */ OFPERR_NXBRC_FM_BAD_EVENT, - /* NX1.0+(1,521). The error that occurred cannot be represented in this - * OpenFlow version. */ + /* NX1.0-1.1(1,521), NX1.2+(10). The error that occurred cannot be + * represented in this OpenFlow version. */ OFPERR_NXBRC_UNENCODABLE_ERROR, /* ## ---------------- ## */ @@ -217,7 +233,8 @@ enum ofperr { /* OF1.2+(2,15). Bad argument in SET_FIELD action. */ OFPERR_OFPBAC_ARGUMENT, - /* NX1.0+(2,256). Must-be-zero action argument had nonzero value. */ + /* NX1.0-1.1(2,256), NX1.2+(11). Must-be-zero action argument had nonzero + * value. */ OFPERR_NXBAC_MUST_BE_ZERO, /* ## --------------------- ## */ @@ -282,15 +299,14 @@ enum ofperr { * field. */ OFPERR_OFPBMC_BAD_VALUE, - /* NX1.0(1,259), NX1.1(1,259), OF1.2+(4,8). Unsupported mask specified in - * the match, field is not dl-address or nw-address. */ + /* NX1.0-1.1(1,259), OF1.2+(4,8). Unsupported mask specified in the match, + * field is not dl-address or nw-address. */ OFPERR_OFPBMC_BAD_MASK, - /* NX1.0(1,260), NX1.1(1,260), OF1.2+(4,9). A prerequisite was not met. */ + /* NX1.0-1.1(1,260), OF1.2+(4,9). A prerequisite was not met. */ OFPERR_OFPBMC_BAD_PREREQ, - /* NX1.0(1,261), NX1.1(1,261), OF1.2+(4,10). A field type was - * duplicated. */ + /* NX1.0-1.1(1,261), OF1.2+(4,10). A field type was duplicated. */ OFPERR_OFPBMC_DUP_FIELD, /* OF1.2+(4,11). Permissions error. */ @@ -333,12 +349,12 @@ enum ofperr { * specified. */ OFPERR_OFPFMFC_UNSUPPORTED, - /* NX1.0(3,256), NX1.1(5,256). Generic hardware error. */ + /* NX1.0-1.1(5,256), NX1.2+(12). Generic hardware error. */ OFPERR_NXFMFC_HARDWARE, - /* NX1.0(3,257), NX1.1(5,257). A nonexistent table ID was specified in the - * "command" field of struct ofp_flow_mod, when the nxt_flow_mod_table_id - * extension is enabled. */ + /* NX1.0-1.1(5,257), NX1.2+(13). A nonexistent table ID was specified in + * the "command" field of struct ofp_flow_mod, when the + * nxt_flow_mod_table_id extension is enabled. */ OFPERR_NXFMFC_BAD_TABLE_ID, /* ## ---------------------- ## */ @@ -465,7 +481,7 @@ enum ofperr { /* OF1.2+(11,1). Controller role change unsupported. */ OFPERR_OFPRRFC_UNSUP, - /* NX1.0(1,513), NX1.1(1,513), OF1.2+(11,2). Invalid role. */ + /* NX1.0-1.1(1,513), OF1.2+(11,2). Invalid role. */ OFPERR_OFPRRFC_BAD_ROLE, /* ## ---------------------- ## */ @@ -549,6 +565,7 @@ enum ofperr ofperr_decode_msg(const struct ofp_header *, struct ofpbuf *ofperr_encode_reply(enum ofperr, const struct ofp_header *); struct ofpbuf *ofperr_encode_hello(enum ofperr, enum ofp_version ofp_version, const char *); +int ofperr_get_vendor(enum ofperr, enum ofp_version); int ofperr_get_type(enum ofperr, enum ofp_version); int ofperr_get_code(enum ofperr, enum ofp_version); diff --git a/tests/ofp-errors.at b/tests/ofp-errors.at index e99aca9af..92f2a98f3 100644 --- a/tests/ofp-errors.at +++ b/tests/ofp-errors.at @@ -87,16 +87,14 @@ dnl Thus, for OF1.1 we translate both of the latter error codes into 3,5. AT_SETUP([encoding OFPBIC_* experimenter errors]) AT_KEYWORDS([ofp-print ofp-errors]) AT_CHECK([ovs-ofctl print-error OFPBIC_BAD_EXPERIMENTER], [0], [dnl -OpenFlow 1.0: -1,-1 -OpenFlow 1.1: 3,5 -OpenFlow 1.2: 3,5 -OpenFlow 1.3: 3,5 +OpenFlow 1.1: vendor 0, type 3, code 5 +OpenFlow 1.2: vendor 0, type 3, code 5 +OpenFlow 1.3: vendor 0, type 3, code 5 ]) AT_CHECK([ovs-ofctl print-error OFPBIC_BAD_EXP_TYPE], [0], [dnl -OpenFlow 1.0: -1,-1 -OpenFlow 1.1: 3,5 -OpenFlow 1.2: 3,6 -OpenFlow 1.3: 3,6 +OpenFlow 1.1: vendor 0, type 3, code 5 +OpenFlow 1.2: vendor 0, type 3, code 6 +OpenFlow 1.3: vendor 0, type 3, code 6 ]) AT_CLEANUP @@ -127,14 +125,47 @@ AT_CHECK([ovs-ofctl ofp-print '0201001455555555 00030005 0102000811111111'], [0] OFPT_ERROR (OF1.1) (xid=0x55555555): OFPBIC_BAD_EXPERIMENTER OFPT_ECHO_REQUEST (xid=0x11111111): 0 bytes of payload ]) -AT_KEYWORDS([ofp-print ofp-errors]) AT_CHECK([ovs-ofctl ofp-print '0301001455555555 00030005 0102000811111111'], [0], [dnl OFPT_ERROR (OF1.2) (xid=0x55555555): OFPBIC_BAD_EXPERIMENTER OFPT_ECHO_REQUEST (xid=0x11111111): 0 bytes of payload ]) -AT_KEYWORDS([ofp-print ofp-errors]) AT_CHECK([ovs-ofctl ofp-print '0301001455555555 00030006 0102000811111111'], [0], [dnl OFPT_ERROR (OF1.2) (xid=0x55555555): OFPBIC_BAD_EXP_TYPE OFPT_ECHO_REQUEST (xid=0x11111111): 0 bytes of payload ]) AT_CLEANUP + +AT_SETUP([decoding experimenter errors]) +AT_KEYWORDS([ofp-print ofp-errors]) +AT_CHECK([ovs-ofctl ofp-print '0101001c55555555 b0c20000 0000232000010203 0102000811111111'], [0], [dnl +OFPT_ERROR (xid=0x55555555): NXBRC_MUST_BE_ZERO +OFPT_ECHO_REQUEST (xid=0x11111111): 0 bytes of payload +]) +AT_CHECK([ovs-ofctl ofp-print '0201001c55555555 b0c20000 0000232000010203 0102000811111111'], [0], [dnl +OFPT_ERROR (OF1.1) (xid=0x55555555): NXBRC_MUST_BE_ZERO +OFPT_ECHO_REQUEST (xid=0x11111111): 0 bytes of payload +]) +AT_CHECK([ovs-ofctl ofp-print '0301001855555555 ffff0004 00002320 0102000811111111'], [0], [dnl +OFPT_ERROR (OF1.2) (xid=0x55555555): NXBRC_MUST_BE_ZERO +OFPT_ECHO_REQUEST (xid=0x11111111): 0 bytes of payload +]) +AT_CLEANUP + +AT_SETUP([encoding experimenter errors]) +AT_KEYWORDS([ofp-print ofp-errors]) +AT_CHECK( + [ovs-ofctl encode-error-reply NXBRC_MUST_BE_ZERO 0100000812345678], [0], [dnl +00000000 01 01 00 1c 12 34 56 78-b0 c2 00 00 00 00 23 20 @&t@ +00000010 00 01 02 03 01 00 00 08-12 34 56 78 @&t@ +]) +AT_CHECK( + [ovs-ofctl encode-error-reply NXBRC_MUST_BE_ZERO 0200000812345678], [0], [dnl +00000000 02 01 00 1c 12 34 56 78-b0 c2 00 00 00 00 23 20 @&t@ +00000010 00 01 02 03 02 00 00 08-12 34 56 78 @&t@ +]) +AT_CHECK( + [ovs-ofctl encode-error-reply NXBRC_MUST_BE_ZERO 0300000812345678], [0], [dnl +00000000 03 01 00 18 12 34 56 78-ff ff 00 04 00 00 23 20 @&t@ +00000010 03 00 00 08 12 34 56 78- +]) +AT_CLEANUP diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 28e3863b2..b2470e61e 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -2786,13 +2786,16 @@ ofctl_print_error(int argc OVS_UNUSED, char *argv[]) for (version = 0; version <= UINT8_MAX; version++) { const char *name = ofperr_domain_get_name(version); - if (!name) { - continue; + if (name) { + int vendor = ofperr_get_vendor(error, version); + int type = ofperr_get_type(error, version); + int code = ofperr_get_code(error, version); + + if (vendor != -1 || type != -1 || code != -1) { + printf("%s: vendor %#x, type %d, code %d\n", + name, vendor, type, code); + } } - printf("%s: %d,%d\n", - ofperr_domain_get_name(version), - ofperr_get_type(error, version), - ofperr_get_code(error, version)); } }