This patch simplifies the API of nxm_dst_check() and adds a new
function nxm_src_check() for checking source fields.
---
Come to think of it, I don't really like the interface of either
nxm_dst_check() or nxm_src_check(). What do you think of something like this?
---
lib/autopath.c | 11 ++++++++-
lib/bundle.c | 11 ++++++++-
lib/multipath.c | 7 +++++-
lib/nx-match.c | 62 ++++++++++++++++++++++++++++--------------------------
lib/nx-match.h | 4 ++-
5 files changed, 61 insertions(+), 34 deletions(-)
diff --git a/lib/autopath.c b/lib/autopath.c
index 22c7c11..cbd5c65 100644
--- a/lib/autopath.c
+++ b/lib/autopath.c
@@ -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);
+ unsigned int n_bits = nxm_decode_n_bits(ap->ofs_nbits);
+ unsigned 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);
}
diff --git a/lib/bundle.c b/lib/bundle.c
index 227d359..8882506 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -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;
+ unsigned int ofs = nxm_decode_ofs(nab->ofs_nbits);
+ unsigned 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)) {
diff --git a/lib/multipath.c b/lib/multipath.c
index 9684556..9e44e58 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -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;
+ unsigned int ofs = nxm_decode_ofs(mp->ofs_nbits);
+ unsigned 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;
}
diff --git a/lib/nx-match.c b/lib/nx-match.c
index cc29c6c..61a979c 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1192,49 +1192,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;
+ unsigned 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;
}
@@ -1246,17 +1248,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;
}
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 28692c7..42092d1 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -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 *);
--
1.7.6
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
