nx-match: Update register check functions. review
authorEthan Jackson <ethan@nicira.com>
Fri, 12 Aug 2011 18:52:59 +0000 (11:52 -0700)
committerBen Pfaff <blp@nicira.com>
Fri, 12 Aug 2011 19:02:34 +0000 (12:02 -0700)
This patch simplifies the API of nxm_dst_check() and adds a new
function nxm_src_check() for checking source fields.

lib/autopath.c
lib/bundle.c
lib/multipath.c
lib/nx-match.c
lib/nx-match.h

index 22c7c11..b42826c 100644 (file)
@@ -82,5 +82,14 @@ autopath_parse(struct nx_action_autopath *ap, const char *s_)
 int
 autopath_check(const struct nx_action_autopath *ap, const struct flow *flow)
 {
-    return nxm_dst_check(ap->dst, ap->ofs_nbits, 16, flow);
+    int n_bits = nxm_decode_n_bits(ap->ofs_nbits);
+    int ofs = nxm_decode_ofs(ap->ofs_nbits);
+
+    if (n_bits < 16) {
+        VLOG_WARN("at least 16 bit destination is required for autopath "
+                  "action.");
+        return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT);
+    }
+
+    return nxm_dst_check(ap->dst, ofs, n_bits, flow);
 }
index 227d359..86762f9 100644 (file)
@@ -145,7 +145,16 @@ bundle_check(const struct nx_action_bundle *nab, int max_ports,
     }
 
     if (subtype == NXAST_BUNDLE_LOAD) {
-        error = nxm_dst_check(nab->dst, nab->ofs_nbits, 16, flow) || error;
+        int ofs = nxm_decode_ofs(nab->ofs_nbits);
+        int n_bits = nxm_decode_n_bits(nab->ofs_nbits);
+
+        if (n_bits < 16) {
+            VLOG_WARN_RL(&rl, "bundle_load action requires at least 16 bit "
+                         "destination.");
+            error = ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT);
+        } else {
+            error = nxm_dst_check(nab->dst, ofs, n_bits, flow) || error;
+        }
     }
 
     if (slaves_size < n_slaves * sizeof(ovs_be16)) {
index 9684556..e85829a 100644 (file)
@@ -38,9 +38,11 @@ multipath_check(const struct nx_action_multipath *mp, const struct flow *flow)
 {
     uint32_t n_links = ntohs(mp->max_link) + 1;
     size_t min_n_bits = log_2_floor(n_links) + 1;
+    int ofs = nxm_decode_ofs(mp->ofs_nbits);
+    int n_bits = nxm_decode_n_bits(mp->ofs_nbits);
     int error;
 
-    error = nxm_dst_check(mp->dst, mp->ofs_nbits, min_n_bits, flow);
+    error = nxm_dst_check(mp->dst, ofs, n_bits, flow);
     if (error) {
         return error;
     }
@@ -53,6 +55,9 @@ multipath_check(const struct nx_action_multipath *mp, const struct flow *flow)
                && mp->algorithm != htons(NX_MP_ALG_ITER_HASH)) {
         VLOG_WARN_RL(&rl, "unsupported algorithm %"PRIu16,
                      ntohs(mp->algorithm));
+    } else if (n_bits < min_n_bits) {
+        VLOG_WARN_RL(&rl, "multipath action requires at least %zu bits for "
+                     "%"PRIu32" links", min_n_bits, n_links);
     } else {
         return 0;
     }
index cccf6fe..23cf89c 100644 (file)
@@ -1191,49 +1191,51 @@ int
 nxm_check_reg_move(const struct nx_action_reg_move *action,
                    const struct flow *flow)
 {
-    const struct nxm_field *src;
-    const struct nxm_field *dst;
+    int src_ofs, dst_ofs, n_bits;
+    int error;
 
-    if (action->n_bits == htons(0)) {
-        return BAD_ARGUMENT;
-    }
+    n_bits = ntohs(action->n_bits);
+    src_ofs = ntohs(action->src_ofs);
+    dst_ofs = ntohs(action->dst_ofs);
 
-    src = nxm_field_lookup(ntohl(action->src));
-    if (!field_ok(src, flow, ntohs(action->src_ofs) + ntohs(action->n_bits))) {
-        return BAD_ARGUMENT;
-    }
+    error = nxm_src_check(action->src, src_ofs, n_bits, flow);
+    error = error ? error : nxm_dst_check(action->dst, dst_ofs, n_bits, flow);
+    return error;
+}
 
-    dst = nxm_field_lookup(ntohl(action->dst));
-    if (!field_ok(dst, flow, ntohs(action->dst_ofs) + ntohs(action->n_bits))) {
-        return BAD_ARGUMENT;
-    }
+/* Given a flow, checks that the source field represented by 'src_header'
+ * in the range ['ofs', 'ofs' + 'n_bits') is valid. */
+int
+nxm_src_check(ovs_be32 src_header, unsigned int ofs, unsigned int n_bits,
+              const struct flow *flow)
+{
+    const struct nxm_field *src = nxm_field_lookup(ntohl(src_header));
 
-    if (!dst->writable) {
-        return BAD_ARGUMENT;
+    if (!n_bits) {
+        VLOG_WARN_RL(&rl, "zero bit source field");
+    } else if (!field_ok(src, flow, ofs + n_bits)) {
+        VLOG_WARN_RL(&rl, "invalid source field");
+    } else {
+        return 0;
     }
 
-    return 0;
+    return BAD_ARGUMENT;
 }
 
 /* Given a flow, checks that the destination field represented by 'dst_header'
- * and 'ofs_nbits' is valid and large enough for 'min_n_bits' bits of data. */
+ * in the range ['ofs', 'ofs' + 'n_bits') is valid. */
 int
-nxm_dst_check(ovs_be32 dst_header, ovs_be16 ofs_nbits, size_t min_n_bits,
+nxm_dst_check(ovs_be32 dst_header, unsigned int ofs, unsigned int n_bits,
               const struct flow *flow)
 {
-    const struct nxm_field *dst;
-    int ofs, n_bits;
-
-    ofs = nxm_decode_ofs(ofs_nbits);
-    n_bits = nxm_decode_n_bits(ofs_nbits);
-    dst = nxm_field_lookup(ntohl(dst_header));
+    const struct nxm_field *dst = nxm_field_lookup(ntohl(dst_header));
 
-    if (!field_ok(dst, flow, ofs + n_bits)) {
+    if (!n_bits) {
+        VLOG_WARN_RL(&rl, "zero bit destination field");
+    } else if (!field_ok(dst, flow, ofs + n_bits)) {
         VLOG_WARN_RL(&rl, "invalid destination field");
     } else if (!dst->writable) {
         VLOG_WARN_RL(&rl, "destination field is not writable");
-    } else if (n_bits < min_n_bits) {
-        VLOG_WARN_RL(&rl, "insufficient bits in destination");
     } else {
         return 0;
     }
@@ -1245,17 +1247,17 @@ int
 nxm_check_reg_load(const struct nx_action_reg_load *action,
                    const struct flow *flow)
 {
-    int n_bits;
+    unsigned int ofs = nxm_decode_ofs(action->ofs_nbits);
+    unsigned int n_bits = nxm_decode_n_bits(action->ofs_nbits);
     int error;
 
-    error = nxm_dst_check(action->dst, action->ofs_nbits, 0, flow);
+    error = nxm_dst_check(action->dst, ofs, n_bits, flow);
     if (error) {
         return error;
     }
 
     /* Reject 'action' if a bit numbered 'n_bits' or higher is set to 1 in
      * action->value. */
-    n_bits = nxm_decode_n_bits(action->ofs_nbits);
     if (n_bits < 64 && ntohll(action->value) >> n_bits) {
         return BAD_ARGUMENT;
     }
index 5365cca..a7441d0 100644 (file)
@@ -49,7 +49,9 @@ void nxm_format_reg_load(const struct nx_action_reg_load *, struct ds *);
 
 int nxm_check_reg_move(const struct nx_action_reg_move *, const struct flow *);
 int nxm_check_reg_load(const struct nx_action_reg_load *, const struct flow *);
-int nxm_dst_check(ovs_be32 dst, ovs_be16 ofs_nbits, size_t min_n_bits,
+int nxm_src_check(ovs_be32 src, unsigned int ofs, unsigned int n_bits,
+                  const struct flow *);
+int nxm_dst_check(ovs_be32 dst, unsigned int ofs, unsigned int n_bits,
                   const struct flow *);
 
 void nxm_execute_reg_move(const struct nx_action_reg_move *, struct flow *);