Python seems good, nice use of int(X, 0)
One question on
> + print " { -1, -1, -1 }, /* %s */" % enum
If you moved the print outside this and used the same print (below) for both
> + print " { %#8x, %2d, %3d }, /* %s */" % (vendor,
> type_, code, enum)
this would've looked like -0x1. If the else just did
vendor = type_ = code = -1
in the else, seems easy to read/maintain
On Wed, Feb 13, 2013 at 2:32 PM, Ben Pfaff <[email protected]> wrote:
> OpenFlow 1.2 standardized experimenter error codes in a way different from
> the Nicira extension. This commit implements the OpenFlow 1.2+ version.
>
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
> build-aux/extract-ofp-errors | 149
> +++++++++++++++++++++++----------------
> include/openflow/openflow-1.2.h | 5 +-
> lib/ofp-errors.c | 72 +++++++++++++------
> lib/ofp-errors.h | 75 +++++++++++---------
> tests/ofp-errors.at | 78 ++++++++++++++++++---
> utilities/ovs-ofctl.c | 15 +++--
> 6 files changed, 258 insertions(+), 136 deletions(-)
>
> diff --git a/build-aux/extract-ofp-errors b/build-aux/extract-ofp-errors
> index aa1f4b7..375876b 100755
> --- a/build-aux/extract-ofp-errors
> +++ b/build-aux/extract-ofp-errors
> @@ -151,10 +151,10 @@ def extract_ofp_errors(filenames):
> 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 ("1.0", "1.1", "1.2", "1.3"):
> domain[domain_name] = {}
> reverse[domain_name] = {}
> + vendor_map = {"OF": 0}
>
> n_errors = 0
> expected_errors = {}
> @@ -194,6 +194,11 @@ def extract_ofp_errors(filenames):
> expected_errors[m.group(1)] = (fileName, lineNumber)
> continue
>
> + m =
> re.match('Experimenter\s+([A-Z]+)\s+(?:\([^)]*\)\s+)?=\s+([x0-9a-fA-F]+)\.$',
> comment)
> + if m:
> + vendor_map[m.group(1)] = int(m.group(2), 0)
> + continue
> +
> m = re.match('((?:.(?!\. ))+.)\. (.*)$', comment)
> if not m:
> fatal("unexpected syntax between errors")
> @@ -201,7 +206,7 @@ def extract_ofp_errors(filenames):
> dsts, comment = m.groups()
>
> getLine()
> - m =
> re.match('\s+(?:OFPERR_((?:OFP|NX)[A-Z0-9_]+))(\s*=\s*OFPERR_OFS)?,',
> + m = re.match('\s+(?:OFPERR_([A-Z0-9_]+))(\s*=\s*OFPERR_OFS)?,',
> line)
> if not m:
> fatal("syntax error expecting enum value")
> @@ -212,65 +217,82 @@ def extract_ofp_errors(filenames):
> names.append(enum)
>
> for dst in dsts.split(', '):
> - m = re.match(r'([A-Z0-9.+]+)\((\d+)(?:,(\d+))?\)$', dst)
> + m = re.match(r'([A-Z]+)([-+.0-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))
> + vendor_s = m.group(1)
> + version_s = m.group(2)
> + type_ = int(m.group(3))
> + if m.group(4):
> + code = int(m.group(4))
> 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)
> + version_map = {"": ("1.0", "1.1", "1.2", "1.3"),
> + "1.0+": ("1.0", "1.1", "1.2", "1.3"),
> + "1.1+": ("1.1", "1.2", "1.3"),
> + "1.2+": ("1.2", "1.3"),
> + "1.3+": ("1.3",),
> + "1.0": ("1.0",),
> + "1.1": ("1.1",),
> + "1.2": ("1.2",),
> + "1.3": ("1.3",),
> + "1.0-1.1": ("1.0", "1.1")}
> + if version_s not in version_map:
> + fatal("%s: unknown version(s) %s" % (dst, version_s))
> + versions = version_map[version_s]
> +
> + if vendor_s not in vendor_map:
> + fatal("%s: unknown vendor %s" % (dst, vendor_s))
> + vendor = vendor_map[vendor_s]
> + if vendor == 0:
> + if code is None:
> + fatal("%s: both type and code are required" % dst)
> + else:
> + pre12 = '1.0' in versions or '1.1' in versions
> + post12 = set(versions) - set(['1.0', '1.1']) != set()
> + if post12 and code is not None:
> + fatal("%s: OF1.2+ experimenter errors do not "
> + "have codes" % dst)
> +
> + for version in versions:
> + if code is None:
> + if version in ('1.0', '1.1'):
> + t = 65535
> + c = type_
> + else:
> + t = type_
> + c = 0
> + else:
> + t = type_
> + c = code
> + domain[version].setdefault(vendor, {})
> + domain[version][vendor].setdefault(t, {})
> + if c in domain[version][vendor][t]:
> + msg = "%#x,%d,%d in OF%s means both %s and %s" % (
> + vendor, t, c, version,
> + domain[version][vendor][t][c][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],
> + %
> (domain[version][vendor][t][c][1],
> +
> domain[version][vendor][t][c][2],
> dst))
> else:
> - domain[target][type_][code] = (enum, fileName,
> + domain[version][vendor][t][c] = (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)
> + if enum in reverse[version]:
> + error("%s: %s in OF%s means both %#x,%d,%d and
> %#x,%d,%d." %
> + (dst, enum, version,
> + reverse[version][enum][0],
> + reverse[version][enum][1],
> + reverse[version][enum][2],
> + vendor, t, c))
> + reverse[version][enum] = (vendor, t, c)
>
> inputFile.close()
>
> @@ -289,8 +311,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 +330,26 @@ 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,20 +365,20 @@ 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)
> + output_domain(reverse["1.0"], "ofperr_of10", "OpenFlow 1.0", 0x01)
> + output_domain(reverse["1.1"], "ofperr_of11", "OpenFlow 1.1", 0x02)
> + output_domain(reverse["1.2"], "ofperr_of12", "OpenFlow 1.2", 0x03)
> + output_domain(reverse["1.3"], "ofperr_of13", "OpenFlow 1.3", 0x04)
>
> if __name__ == '__main__':
> if '--help' in sys.argv:
> diff --git a/include/openflow/openflow-1.2.h b/include/openflow/openflow-1.2.h
> index 5546313..2977047 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/lib/ofp-errors.c b/lib/ofp-errors.c
> index e2449b3..1cfa62d 100644
> --- a/lib/ofp-errors.c
> +++ b/lib/ofp-errors.c
> @@ -11,7 +11,8 @@
>
> VLOG_DEFINE_THIS_MODULE(ofp_errors);
>
> -struct pair {
> +struct triplet {
> + uint32_t vendor;
> int type, code;
> };
>
> @@ -57,10 +58,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
> @@ -105,8 +107,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;
>
> @@ -120,8 +122,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. */
> @@ -144,15 +146,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,
> @@ -163,9 +165,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);
> @@ -206,6 +218,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.
> @@ -215,7 +234,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
> @@ -229,7 +248,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.
> @@ -240,12 +259,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) {
> @@ -261,6 +279,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) {
> @@ -269,17 +288,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 1f7ea69..7da6f6d 100644
> --- a/lib/ofp-errors.h
> +++ b/lib/ofp-errors.h
> @@ -59,9 +59,13 @@ struct ofpbuf;
> * square brackets is omitted; this can be used to explain rationale for
> * choice of error codes in the case where this is desirable. */
> enum ofperr {
> +/* Experimenter codes. */
> +
> + /* Experimenter NX (Nicira Networks) = 0x00002320. */
> +
> /* 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 +119,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 +131,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 +223,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 +289,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 +339,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 +471,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 +555,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 e99aca9..f39a213 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,74 @@ 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_CHECK([ovs-ofctl ofp-print '0101001c55555555 b0c20000 00002320ffff0001
> 0102000811111111'], [0], [dnl
> +OFPT_ERROR (xid=0x55555555): NX_DUP_INSTRUCTION
> +OFPT_ECHO_REQUEST (xid=0x11111111): 0 bytes of payload
> +])
> +AT_CHECK([ovs-ofctl ofp-print '0201001c55555555 b0c20000 00002320ffff0001
> 0102000811111111'], [0], [dnl
> +OFPT_ERROR (OF1.1) (xid=0x55555555): NX_DUP_INSTRUCTION
> +OFPT_ECHO_REQUEST (xid=0x11111111): 0 bytes of payload
> +])
> +AT_CHECK([ovs-ofctl ofp-print '0301001855555555 ffff0001 00002320
> 0102000811111111'], [0], [dnl
> +OFPT_ERROR (OF1.2) (xid=0x55555555): NX_DUP_INSTRUCTION
> +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_CHECK(
> + [ovs-ofctl encode-error-reply NX_DUP_INSTRUCTION 0100000812345678], [0],
> +[00000000 01 01 00 1c 12 34 56 78-b0 c2 00 00 00 00 23 20 @&t@
> +00000010 ff ff 00 01 01 00 00 08-12 34 56 78 @&t@
> +])
> +AT_CHECK(
> + [ovs-ofctl encode-error-reply NX_DUP_INSTRUCTION 0200000812345678], [0],
> +[00000000 02 01 00 1c 12 34 56 78-b0 c2 00 00 00 00 23 20 @&t@
> +00000010 ff ff 00 01 02 00 00 08-12 34 56 78 @&t@
> +])
> +AT_CHECK(
> + [ovs-ofctl encode-error-reply NX_DUP_INSTRUCTION 0300000812345678], [0],
> +[00000000 03 01 00 18 12 34 56 78-ff ff 00 01 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 ec775c7..cbdfba6 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -2747,13 +2747,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));
> }
> }
>
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
