Removed error category types (OFPERR_OFPET_*) from enum ofperr to make it
harder for contributors to use
unencodable error codes. Removed corresponding functions
ofperr_is_category() and ofperr_decode_type().
Added OFPERR_NX__UNKNOWN_ERROR (NX1.0+(0xfffe,256)), which is now encoded
if the error code given to
ofperr_encode_msg__() cannot be encoded with the given ofp_version. The
rationale is that the corresponding
OF transaction has failed, and the controller must know this, so sending no
error message is not an option.
Changed ofperr_encode_msg__() logging from WARN to ERR, as using
unencodable error codes for the underlying
openflow version is an error.
Added OFPERR_NXBRC_FM_BAD_EVENT to avoid using an unencodable error code in
ofputil_decode_flow_update().
Replaced unencodable error codes in ofp-actions.c with encodable ones.
Signed-off-by: Jarno Rajahalme <jarno.rajaha...@nsn.com>
---
build-aux/extract-ofp-errors | 22 +---------------------
lib/ofp-actions.c | 7 ++++---
lib/ofp-errors.c | 77
++++++++++++++++++++++++++++++++-------------------------------------------
lib/ofp-errors.h | 60
+++++++++++++---------------------------------------------
lib/ofp-util.c | 2 +-
tests/ofp-actions.at | 4 ++--
6 files changed, 54 insertions(+), 118 deletions(-)
diff --git a/build-aux/extract-ofp-errors b/build-aux/extract-ofp-errors
index c327304..fd53001 100755
--- a/build-aux/extract-ofp-errors
+++ b/build-aux/extract-ofp-errors
@@ -294,7 +294,6 @@ struct ofperr_domain {
const char *name;
uint8_t version;
enum ofperr (*decode)(uint16_t type, uint16_t code);
- enum ofperr (*decode_type)(uint16_t type);
struct pair errors[OFPERR_N_ERRORS];
};
@@ -333,24 +332,6 @@ static enum ofperr
}
return 0;
-}
-
-static enum ofperr
-%s_decode_type(uint16_t type)
-{
- switch (type) {""" % name
- for enum in names:
- if enum not in map:
- continue
- type_, code = map[enum]
- if code is not None:
- continue
- print " case %d:" % type_
- print " return OFPERR_%s;" % enum
- print """\
- }
-
- return 0;
}"""
print """
@@ -358,8 +339,7 @@ static const struct ofperr_domain %s = {
"%s",
%d,
%s_decode,
- %s_decode_type,
- {""" % (name, description, version, name, name)
+ {""" % (name, description, version, name)
for enum in names:
if enum in map:
type_, code = map[enum]
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 170e796..c20a521 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -718,7 +718,7 @@ ofpact_from_openflow11(const union ofp_action *a, struct
ofpbuf *out)
if (((const struct ofp11_action_push *)a)->ethertype !=
htons(ETH_TYPE_VLAN_8021Q)) {
/* TODO:XXX 802.1AD(QinQ) isn't supported at the moment */
- return OFPERR_OFPET_BAD_ACTION;
+ return OFPERR_OFPBAC_BAD_ARGUMENT;
}
ofpact_put_PUSH_VLAN(out);
break;
@@ -912,7 +912,9 @@ decode_openflow11_instructions(const struct
ofp11_instruction insts[],
}
if (out[type]) {
- return OFPERR_OFPIT_BAD_INSTRUCTION;
+ return OFPERR_OFPBAC_UNSUPPORTED_ORDER; /* No specific code for
+ * a duplicate instruction
+ * exist */
}
out[type] = inst;
}
@@ -1152,7 +1154,6 @@ ofpacts_verify(const struct ofpact ofpacts[], size_t
ofpacts_len)
if (om) {
if (a->type == OFPACT_WRITE_METADATA) {
VLOG_WARN("duplicate write_metadata instruction specified");
- /* should be OFPERR_OFPET_BAD_ACTION? */
return OFPERR_OFPBAC_UNSUPPORTED_ORDER;
} else {
VLOG_WARN("write_metadata instruction must be specified after "
diff --git a/lib/ofp-errors.c b/lib/ofp-errors.c
index 9fd48fe..c09aa4e 100644
--- a/lib/ofp-errors.c
+++ b/lib/ofp-errors.c
@@ -53,17 +53,6 @@ ofperr_is_valid(enum ofperr error)
return error >= OFPERR_OFS && error < OFPERR_OFS + OFPERR_N_ERRORS;
}
-/* Returns true if 'error' is a valid OFPERR_* value that designates a whole
- * category of errors instead of a particular error, e.g. if it is an
- * OFPERR_OFPET_* value, and false otherwise. */
-bool
-ofperr_is_category(enum ofperr error)
-{
- return (ofperr_is_valid(error)
- && ofperr_of10.errors[error - OFPERR_OFS].code == -1
- && ofperr_of11.errors[error - OFPERR_OFS].code == -1);
-}
-
/* Returns true if 'error' is a valid OFPERR_* value that is a Nicira
* extension, e.g. if it is an OFPERR_NX* value, and false otherwise. */
bool
@@ -71,7 +60,9 @@ ofperr_is_nx_extension(enum ofperr error)
{
return (ofperr_is_valid(error)
&& (ofperr_of10.errors[error - OFPERR_OFS].code >= 0x100 ||
- ofperr_of11.errors[error - OFPERR_OFS].code >= 0x100));
+ ofperr_of11.errors[error - OFPERR_OFS].code >= 0x100 ||
+ ofperr_of12.errors[error - OFPERR_OFS].code >= 0x100 ||
+ ofperr_of13.errors[error - OFPERR_OFS].code >= 0x100));
}
/* Returns true if 'error' can be encoded as an OpenFlow error message in
@@ -97,16 +88,6 @@ ofperr_decode(enum ofp_version version, uint16_t type,
uint16_t code)
return domain ? domain->decode(type, code) : 0;
}
-/* Returns the OFPERR_* value that corresponds to the category 'type' within
- * 'version', or 0 if either no such OFPERR_* value exists or 'version' is
- * unknown. */
-enum ofperr
-ofperr_decode_type(enum ofp_version version, uint16_t type)
-{
- const struct ofperr_domain *domain = ofperr_domain_from_version(version);
- return domain ? domain->decode_type(type) : 0;
-}
-
/* Returns the name of 'error', e.g. "OFPBRC_BAD_TYPE" if 'error' is
* OFPBRC_BAD_TYPE, or "<invalid>" if 'error' is not a valid OFPERR_* value.
*
@@ -167,33 +148,45 @@ ofperr_encode_msg__(enum ofperr error, enum ofp_version
ofp_version,
struct ofpbuf *buf;
const struct ofperr_domain *domain;
+ /*
+ * This corresponds to OFPERR_NX__UNKNOWN_ERROR, and is here
+ * to make sure we always have this, even if we have failed
+ * to implement an error domain for the ofp_version in use.
+ */
+ static const struct pair unknown_error = { 0xfffe, 256 };
+
domain = ofperr_domain_from_version(ofp_version);
- if (!domain) {
- return NULL;
- }
if (!ofperr_is_encodable(error, ofp_version)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
- if (!ofperr_is_valid(error)) {
+ if (!domain) {
+ VLOG_ERR_RL(&rl, "Error domain for %s does not exist, "
+ "Cannot encode error (%s)",
+ ofputil_version_to_string(ofp_version),
+ ofperr_get_name(error));
+ } else if (!ofperr_is_valid(error)) {
/* 'error' seems likely to be a system errno value. */
- VLOG_WARN_RL(&rl, "invalid OpenFlow error code %d (%s)",
- error, strerror(error));
+ VLOG_ERR_RL(&rl, "invalid OpenFlow error code %d (%s)",
+ error, strerror(error));
} else {
- const char *s = ofperr_get_name(error);
- if (ofperr_is_category(error)) {
- VLOG_WARN_RL(&rl, "cannot encode error category (%s)", s);
- } else {
- VLOG_WARN_RL(&rl, "cannot encode %s for %s", s, domain->name);
- }
+ VLOG_ERR_RL(&rl, "cannot encode %s for %s",
+ ofperr_get_name(error), domain->name);
}
- return NULL;
+ /* The caller expects the controller to get an error message,
+ * must send something! */
+ error = OFPERR_NX__UNKNOWN_ERROR;
+ VLOG_WARN_RL(&rl, "Substituting with generic error %s",
+ ofperr_get_name(error));
+ pair = &unknown_error;
+ }
+ else { /* error is encodable => domain exists */
+ pair = ofperr_get_pair__(error, domain);
}
- pair = ofperr_get_pair__(error, domain);
if (!ofperr_is_nx_extension(error)) {
- buf = ofpraw_alloc_xid(OFPRAW_OFPT_ERROR, domain->version, xid,
+ buf = ofpraw_alloc_xid(OFPRAW_OFPT_ERROR, ofp_version, xid,
sizeof *oem + data_len);
oem = ofpbuf_put_uninit(buf, sizeof *oem);
@@ -202,7 +195,7 @@ ofperr_encode_msg__(enum ofperr error, enum ofp_version
ofp_version,
} else {
struct nx_vendor_error *nve;
- buf = ofpraw_alloc_xid(OFPRAW_OFPT_ERROR, domain->version, xid,
+ buf = ofpraw_alloc_xid(OFPRAW_OFPT_ERROR, ofp_version, xid,
sizeof *oem + sizeof *nve + data_len);
oem = ofpbuf_put_uninit(buf, sizeof *oem);
@@ -295,7 +288,7 @@ ofperr_get_code(enum ofperr error, enum ofp_version version)
return domain ? ofperr_get_pair__(error, domain)->code : -1;
}
-/* Tries to decodes 'oh', which should be an OpenFlow OFPT_ERROR message.
+/* Tries to decode 'oh', which should be an OpenFlow OFPT_ERROR message.
* Returns an OFPERR_* constant on success, 0 on failure.
*
* If 'payload' is nonnull, on success '*payload' is initialized to the
@@ -341,12 +334,8 @@ ofperr_decode_msg(const struct ofp_header *oh, struct
ofpbuf *payload)
code = ntohs(nve->code);
}
- /* Translate the error type and code into an ofperr.
- * If we don't know the error type and code, at least try for the type. */
+ /* Translate the error type and code into an ofperr. */
error = ofperr_decode(oh->version, type, code);
- if (!error) {
- error = ofperr_decode_type(oh->version, type);
- }
if (error && payload) {
ofpbuf_use_const(payload, b.data, b.size);
}
diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h
index e9fedb9..49e2d58 100644
--- a/lib/ofp-errors.h
+++ b/lib/ofp-errors.h
@@ -68,11 +68,8 @@ enum ofperr {
/* ## OFPET_HELLO_FAILED ## */
/* ## ------------------ ## */
- /* OF1.0+(0). Hello protocol failed. */
- OFPERR_OFPET_HELLO_FAILED = OFPERR_OFS,
-
/* OF1.0+(0,0). No compatible version. */
- OFPERR_OFPHFC_INCOMPATIBLE,
+ OFPERR_OFPHFC_INCOMPATIBLE = OFPERR_OFS,
/* OF1.0+(0,1). Permissions error. */
OFPERR_OFPHFC_EPERM,
@@ -81,9 +78,6 @@ enum ofperr {
/* ## OFPET_BAD_REQUEST ## */
/* ## ----------------- ## */
- /* OF1.0+(1). Request was not understood. */
- OFPERR_OFPET_BAD_REQUEST,
-
/* OF1.0+(1,0). ofp_header.version not supported. */
OFPERR_OFPBRC_BAD_VERSION,
@@ -161,13 +155,15 @@ enum ofperr {
* 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
+ * NXFME_MODIFY. */
+ OFPERR_NXBRC_FM_BAD_EVENT,
+
/* ## ---------------- ## */
/* ## OFPET_BAD_ACTION ## */
/* ## ---------------- ## */
- /* OF1.0+(2). Error in action description. */
- OFPERR_OFPET_BAD_ACTION,
-
/* OF1.0+(2,0). Unknown action type. */
OFPERR_OFPBAC_BAD_TYPE,
@@ -224,9 +220,6 @@ enum ofperr {
/* ## OFPET_BAD_INSTRUCTION ## */
/* ## --------------------- ## */
- /* OF1.1+(3). Error in instruction list. */
- OFPERR_OFPIT_BAD_INSTRUCTION,
-
/* OF1.1+(3,0). Unknown instruction. */
OFPERR_OFPBIC_UNKNOWN_INST,
@@ -258,9 +251,6 @@ enum ofperr {
/* ## OFPET_BAD_MATCH ## */
/* ## --------------- ## */
- /* OF1.1+(4). Error in match. */
- OFPERR_OFPET_BAD_MATCH,
-
/* OF1.1+(4,0). Unsupported match type specified by the match */
OFPERR_OFPBMC_BAD_TYPE,
@@ -306,9 +296,6 @@ enum ofperr {
/* ## OFPET_FLOW_MOD_FAILED ## */
/* ## --------------------- ## */
- /* OF1.0(3), OF1.1+(5). Problem modifying flow entry. */
- OFPERR_OFPET_FLOW_MOD_FAILED,
-
/* OF1.1+(5,0). Unspecified error. */
OFPERR_OFPFMFC_UNKNOWN,
@@ -354,9 +341,6 @@ enum ofperr {
/* ## OFPET_GROUP_MOD_FAILED ## */
/* ## ---------------------- ## */
- /* OF1.1+(6). Problem modifying group entry. */
- OFPERR_OFPET_GROUP_MOD_FAILED,
-
/* OF1.1+(6,0). Group not added because a group ADD attempted to replace
* an already-present group. */
OFPERR_OFPGMFC_GROUP_EXISTS,
@@ -412,9 +396,6 @@ enum ofperr {
/* ## OFPET_PORT_MOD_FAILED ## */
/* ## --------------------- ## */
- /* OF1.0(4), OF1.1+(7). OFPT_PORT_MOD failed. */
- OFPERR_OFPET_PORT_MOD_FAILED,
-
/* OF1.0(4,0), OF1.1+(7,0). Specified port does not exist. */
OFPERR_OFPPMFC_BAD_PORT,
@@ -435,9 +416,6 @@ enum ofperr {
/* ## OFPET_TABLE_MOD_FAILED ## */
/* ## ---------------------- ## */
- /* OF1.1+(8). Table mod request failed. */
- OFPERR_OFPET_TABLE_MOD_FAILED,
-
/* OF1.1+(8,0). Specified table does not exist. */
OFPERR_OFPTMFC_BAD_TABLE,
@@ -451,9 +429,6 @@ enum ofperr {
/* ## OFPET_QUEUE_OP_FAILED ## */
/* ## --------------------- ## */
- /* OF1.0(5), OF1.1+(9). Queue operation failed. */
- OFPERR_OFPET_QUEUE_OP_FAILED,
-
/* OF1.0(5,0), OF1.1+(9,0). Invalid port (or port does not exist). */
OFPERR_OFPQOFC_BAD_PORT,
@@ -467,9 +442,6 @@ enum ofperr {
/* ## OFPET_SWITCH_CONFIG_FAILED ## */
/* ## -------------------------- ## */
- /* OF1.1+(10). Switch config request failed. */
- OFPERR_OFPET_SWITCH_CONFIG_FAILED,
-
/* OF1.1+(10,0). Specified flags is invalid. */
OFPERR_OFPSCFC_BAD_FLAGS,
@@ -483,9 +455,6 @@ enum ofperr {
/* ## OFPET_ROLE_REQUEST_FAILED ## */
/* ## ------------------------- ## */
- /* OF1.2+(11). Controller Role request failed. */
- OFPERR_OFPET_ROLE_REQUEST_FAILED,
-
/* OF1.2+(11,0). Stale Message: old generation_id. */
OFPERR_OFPRRFC_STALE,
@@ -499,9 +468,6 @@ enum ofperr {
/* ## OFPET_METER_MOD_FAILED ## */
/* ## ---------------------- ## */
- /* OF1.3+(12). Error in meter. */
- OFPERR_OFPET_METER_MOD_FAILED,
-
/* OF1.3+(12,0). Unspecified error. */
OFPERR_OFPMMFC_UNKNOWN,
@@ -545,9 +511,6 @@ enum ofperr {
/* ## OFPET_TABLE_FEATURES_FAILED ## */
/* ## --------------------------- ## */
- /* OF1.3+(13). Setting table features failed. */
- OFPERR_OFPET_TABLE_FEATURES_FAILED,
-
/* OF1.3+(13,0). Specified table does not exist. */
OFPERR_OFPTFFC_BAD_TABLE,
@@ -566,23 +529,26 @@ enum ofperr {
/* OF1.3+(13,5). Permissions error. */
OFPERR_OFPTFFC_EPERM,
+/* ## ----------------- ## */
+/* ## NX__UNKNOWN_ERROR ## */
+/* ## ----------------- ## */
+
+ /* NX1.0+(0xfffe,256). The switch failed to encode a specific error code.
+ * The operation referred to has failed due to an unknown reason. */
+ OFPERR_NX__UNKNOWN_ERROR,
+
/* ## ------------------ ## */
/* ## OFPET_EXPERIMENTER ## */
/* ## ------------------ ## */
-
- /* OF1.2+(0xffff). Experimenter error messages. */
- OFPERR_OFPET_EXPERIMENTER,
};
const char *ofperr_domain_get_name(enum ofp_version);
bool ofperr_is_valid(enum ofperr);
-bool ofperr_is_category(enum ofperr);
bool ofperr_is_nx_extension(enum ofperr);
bool ofperr_is_encodable(enum ofperr, enum ofp_version);
enum ofperr ofperr_decode(enum ofp_version, uint16_t type, uint16_t code);
-enum ofperr ofperr_decode_type(enum ofp_version, uint16_t type);
enum ofperr ofperr_from_name(const char *);
enum ofperr ofperr_decode_msg(const struct ofp_header *,
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 49cbe2d..348c6fc 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3681,7 +3681,7 @@ ofputil_decode_flow_update(struct ofputil_flow_update
*update,
VLOG_WARN_RL(&bad_ofmsg_rl,
"NXST_FLOW_MONITOR reply has bad event %"PRIu16,
ntohs(nfuh->event));
- return OFPERR_OFPET_BAD_REQUEST;
+ return OFPERR_NXBRC_FM_BAD_EVENT;
}
bad_len:
diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
index e232435..503900f 100644
--- a/tests/ofp-actions.at
+++ b/tests/ofp-actions.at
@@ -341,7 +341,7 @@ dnl Check that an empty Apply-Actions instruction gets
dropped.
0004 0008 00000000
dnl Duplicate instruction type:
-# bad OF1.1 instructions: OFPIT_BAD_INSTRUCTION
+# bad OF1.1 instructions: OFPBAC_UNSUPPORTED_ORDER
0004 0008 00000000 0004 0008 00000000
dnl Instructions not multiple of 8 in length.
@@ -379,7 +379,7 @@ dnl Write-Metadata too long.
0002 0020 00000000 fedcba9876543210 ffffffffffffffff 0000000000000000
dnl Write-Metadata duplicated.
-# bad OF1.1 instructions: OFPIT_BAD_INSTRUCTION
+# bad OF1.1 instructions: OFPBAC_UNSUPPORTED_ORDER
0002 0018 00000000 fedcba9876543210 ff00ff00ff00ff00 0002 0018 00000000
fedcba9876543210 ff00ff00ff00ff00
dnl Write-Metadata in wrong position.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
