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 <blp@nicira.com>

10 files changed, 376 insertions, 213 deletions

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)
             : "<invalid>");
 }
 
-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));
     }
 }