From: Ben Pfaff Date: Fri, 23 Aug 2013 18:11:03 +0000 (-0700) Subject: meta-flow: Use correct OF1.2+ errors for invalid fields in actions. X-Git-Tag: v2.0~125 X-Git-Url: http://git.cascardo.eti.br/?a=commitdiff_plain;h=9e404d1e1c7d36f0746de29ccc6fa57bc4696fda;p=cascardo%2Fovs.git meta-flow: Use correct OF1.2+ errors for invalid fields in actions. OFPERR_OFPBAC_BAD_ARGUMENT is not as specific as the errors provided by OpenFlow 1.2 and later. Some of these errors needed Nicira extension numbers for use with OpenFlow 1.0 and 1.1, so this commit also adds those. Some of these errors had poor explanations likely to confuse users, so this commits improves them. Some of the errors had the wrong names, so this commit fixes them. Reported-by: Jean Tourrilhes Signed-off-by: Ben Pfaff Acked-by: Jean Tourrilhes Acked-by: Jarno Rajahalme --- diff --git a/lib/meta-flow.c b/lib/meta-flow.c index 86a7a1d29..902a21d05 100644 --- a/lib/meta-flow.c +++ b/lib/meta-flow.c @@ -1957,23 +1957,26 @@ mf_check__(const struct mf_subfield *sf, const struct flow *flow, { if (!sf->field) { VLOG_WARN_RL(&rl, "unknown %s field", type); + return OFPERR_OFPBAC_BAD_SET_TYPE; } else if (!sf->n_bits) { VLOG_WARN_RL(&rl, "zero bit %s field %s", type, sf->field->name); + return OFPERR_OFPBAC_BAD_SET_LEN; } else if (sf->ofs >= sf->field->n_bits) { VLOG_WARN_RL(&rl, "bit offset %d exceeds %d-bit width of %s field %s", sf->ofs, sf->field->n_bits, type, sf->field->name); + return OFPERR_OFPBAC_BAD_SET_LEN; } else if (sf->ofs + sf->n_bits > sf->field->n_bits) { VLOG_WARN_RL(&rl, "bit offset %d and width %d exceeds %d-bit width " "of %s field %s", sf->ofs, sf->n_bits, sf->field->n_bits, type, sf->field->name); + return OFPERR_OFPBAC_BAD_SET_LEN; } else if (flow && !mf_are_prereqs_ok(sf->field, flow)) { VLOG_WARN_RL(&rl, "%s field %s lacks correct prerequisites", type, sf->field->name); + return OFPERR_OFPBAC_MATCH_INCONSISTENT; } else { return 0; } - - return OFPERR_OFPBAC_BAD_ARGUMENT; } /* Checks whether 'sf' is valid for reading a subfield out of 'flow'. Returns @@ -1995,7 +1998,7 @@ mf_check_dst(const struct mf_subfield *sf, const struct flow *flow) if (!error && !sf->field->writable) { VLOG_WARN_RL(&rl, "destination field %s is not writable", sf->field->name); - return OFPERR_OFPBAC_BAD_ARGUMENT; + return OFPERR_OFPBAC_BAD_SET_ARGUMENT; } return error; } diff --git a/lib/nx-match.c b/lib/nx-match.c index e9b1375ef..3bb71e2e4 100644 --- a/lib/nx-match.c +++ b/lib/nx-match.c @@ -1173,19 +1173,19 @@ nxm_reg_load_from_openflow12_set_field( /* ofp12_action_set_field is padded to 64 bits by zero */ if (oasf_len != ROUND_UP(sizeof(*oasf) + oxm_length, 8)) { - return OFPERR_OFPBAC_BAD_ARGUMENT; + return OFPERR_OFPBAC_BAD_SET_LEN; } if (!is_all_zeros((const uint8_t *)(oasf) + sizeof *oasf + oxm_length, oasf_len - oxm_length - sizeof *oasf)) { - return OFPERR_OFPBAC_BAD_ARGUMENT; + return OFPERR_OFPBAC_BAD_SET_ARGUMENT; } if (NXM_HASMASK(oxm_header)) { - return OFPERR_OFPBAC_BAD_ARGUMENT; + return OFPERR_OFPBAC_BAD_SET_TYPE; } mf = mf_from_nxm_header(oxm_header); if (!mf) { - return OFPERR_OFPBAC_BAD_ARGUMENT; + return OFPERR_OFPBAC_BAD_SET_TYPE; } load = ofpact_put_REG_LOAD(ofpacts); ofpact_set_field_init(load, mf, oasf + 1); diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h index 5bf5826df..79acd303d 100644 --- a/lib/ofp-errors.h +++ b/lib/ofp-errors.h @@ -214,7 +214,8 @@ enum ofperr { /* OF1.1+(2,9). Invalid group id in forward action. */ OFPERR_OFPBAC_BAD_OUT_GROUP, - /* OF1.1+(2,10). Action can't apply for this match. */ + /* NX1.0(1,522), OF1.1+(2,10). Action can't apply for this match or a + * prerequisite for use of this field is unmet. */ OFPERR_OFPBAC_MATCH_INCONSISTENT, /* OF1.1+(2,11). Action order is unsupported for the action list in an @@ -224,14 +225,17 @@ enum ofperr { /* OF1.1+(2,12). Actions uses an unsupported tag/encap. */ OFPERR_OFPBAC_BAD_TAG, - /* OF1.2+(2,13). Unsupported type in SET_FIELD action. */ - OFPERR_OFPBAC_SET_TYPE, + /* NX1.0-1.1(1,523), OF1.2+(2,13). Action uses unknown or unsupported OXM + * or NXM field. */ + OFPERR_OFPBAC_BAD_SET_TYPE, - /* OF1.2+(2,14). Length problem in SET_FIELD action. */ - OFPERR_OFPBAC_SET_LEN, + /* NX1.0-1.1(1,524), OF1.2+(2,14). Action references past the end of an + * OXM or NXM field, or uses a length of zero. */ + OFPERR_OFPBAC_BAD_SET_LEN, - /* OF1.2+(2,15). Bad argument in SET_FIELD action. */ - OFPERR_OFPBAC_ARGUMENT, + /* NX1.0-1.1(1,525), OF1.2+(2,15). Action sets a field to an invalid or + * unsupported value, or modifies a read-only field. */ + OFPERR_OFPBAC_BAD_SET_ARGUMENT, /* NX1.0-1.1(2,256), NX1.2+(11). Must-be-zero action argument had nonzero * value. */ diff --git a/tests/learn.at b/tests/learn.at index fc8d07199..63356b4ac 100644 --- a/tests/learn.at +++ b/tests/learn.at @@ -75,13 +75,13 @@ AT_CHECK([[ovs-ofctl parse-flow 'actions=learn(load:5->NXM_OF_IP_DST[])']], [1], [], [stderr]) AT_CHECK([sed -e 's/.*|meta_flow|WARN|//' < stderr], [0], [[destination field ip_dst lacks correct prerequisites -ovs-ofctl: actions are invalid with specified match (OFPBAC_BAD_ARGUMENT) +ovs-ofctl: actions are invalid with specified match (OFPBAC_MATCH_INCONSISTENT) ]], [[]]) AT_CHECK([[ovs-ofctl parse-flow 'actions=learn(load:NXM_OF_IP_DST[]->NXM_NX_REG1[])']], [1], [], [stderr]) AT_CHECK([sed -e 's/.*|meta_flow|WARN|//' < stderr], [0], [[source field ip_dst lacks correct prerequisites -ovs-ofctl: actions are invalid with specified match (OFPBAC_BAD_ARGUMENT) +ovs-ofctl: actions are invalid with specified match (OFPBAC_MATCH_INCONSISTENT) ]]) AT_CLEANUP