[ovs-dev] [PATCH] meta-flow: Use correct OF1.2+ errors for invalid fields in actions.

Ben Pfaff blp at nicira.com
Thu Aug 22 18:17:12 UTC 2013


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 <jt at hpl.hp.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/meta-flow.c  |    9 ++++++---
 lib/nx-match.c   |    8 ++++----
 lib/ofp-errors.h |   18 +++++++++++-------
 tests/learn.at   |    4 ++--
 4 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 86a7a1d..902a21d 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 e9b1375..3bb71e2 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 5bf5826..79acd30 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 fc8d071..63356b4 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
 
-- 
1.7.10.4




More information about the dev mailing list