ofp-errors: Fix bugs in treatment of OpenFlow experimenter errors.

OpenFlow 1.2 and later have "experimenter errors".  The OVS implementation
was buggy in a few ways.  First, a bug in extract-ofp-errors prevented
OF1.2+ experimenter errors from being properly decoded.  Second,
OF1.2+ experimenter errors have only a type, not a code, whereas all other
types of errors (standard errors, OF1.0/1.1 Nicira extension errors) have
both, but extract-ofp-errors didn't properly enforce that.

This commit fixes both problems and improves existing tests to verify that
encoding and decoding of experimenter errors now works properly.

This commit also fixes the definition of OFPBIC_DUP_INST.  It claimed to
have an OF1.1 experimenter error value although OF1.1 didn't have
experimenter errors.  This commit changes it to use a Nicira extension
error in OF1.1 instead.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

1 files changed, 24 insertions, 6 deletions

diff --git a/build-aux/extract-ofp-errors b/build-aux/extract-ofp-errors
index 89dd31a3d..16bfbc721 100755
--- a/build-aux/extract-ofp-errors
+++ b/build-aux/extract-ofp-errors
@@ -300,14 +300,34 @@ def extract_ofp_errors(fn):
                       % (version1_name, version2_name,
                          version2_name, version1_name))
 
-            if vendor == vendor_map['NX']:
+            if vendor == vendor_map['OF']:
+                # All standard OpenFlow errors have a type and a code.
+                if code is None:
+                    fatal("%s: %s domain requires code" % (dst, vendor_name))
+            elif vendor == vendor_map['NX']:
+                # Before OpenFlow 1.2, OVS used a Nicira extension to
+                # define errors that included a type and a code.
+                #
+                # In OpenFlow 1.2 and later, Nicira extension errors
+                # are defined using the OpenFlow experimenter error
+                # mechanism that includes a type but not a code.
+                if v1 < version_map['1.2'] or v2 < version_map['1.2']:
+                    if code is None:
+                        fatal("%s: NX1.0 and NX1.1 domains require code"
+                              % (dst, vendor_name))
                 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']:
+            else:
+                # Experimenter extension error for OF1.2+ only.
+                if v1 < version_map['1.2']:
+                    fatal("%s: %s domain not supported before OF1.2"
+                          % (dst, vendor_name))
                 if code is not None:
-                    fatal("%s: %s domains do not have codes" % vendor_name)
+                    fatal("%s: %s domains do not have codes"
+                          % (dst, vendor_name))
+            if code is None:
+                code = 0
 
             for version in range(v1, v2 + 1):
                 domain[version].setdefault(vendor, {})
@@ -376,8 +396,6 @@ static enum ofperr
             if enum not in map:
                 continue
             vendor, type_, code = map[enum]
-            if code is None:
-                continue
             value = (vendor << 32) | (type_ << 16) | code
             if value in found:
                 continue