This patch creates two new helper functions, nxm_reg_load() and
nxm_dst_check(). The new nxm_dst_check() function may be used to
check the validity of destination fields used by actions. The new
nxm_reg_load() function may be used by actions which need to write
to NXM fields.
This patch also allows multipath and autopath to write their result
to non-register NXM fields.
---
include/openflow/nicira-ext.h | 6 +--
lib/autopath.c | 31 ++++--------------
lib/autopath.h | 2 +-
lib/multipath.c | 30 +++++++-----------
lib/multipath.h | 2 +-
lib/nx-match.c | 67 +++++++++++++++++++++++++----------------
lib/nx-match.h | 4 ++
lib/ofp-util.c | 6 ++-
tests/autopath.at | 6 ----
tests/multipath.at | 6 ----
utilities/ovs-ofctl.8.in | 4 +-
11 files changed, 73 insertions(+), 91 deletions(-)
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 960b53f..9427450 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -540,8 +540,7 @@ OFP_ASSERT(sizeof(struct nx_action_note) == 16);
*
* 3. Stores 'link' in dst[ofs:ofs+n_bits]. The format and semantics of
* 'dst' and 'ofs_nbits' are similar to those for the NXAST_REG_LOAD
- * action, except that 'dst' must be NXM_NX_REG(idx) for 'idx' in the
- * switch's supported range.
+ * action.
*
* The switch will reject actions that have an unknown 'fields', or an unknown
* 'algorithm', or in which ofs+n_bits is greater than the width of 'dst', or
@@ -645,8 +644,7 @@ enum nx_mp_algorithm {
* 3. Stores 'port' in dst[ofs:ofs+n_bits].
*
* The format and semantics of 'dst' and 'ofs_nbits' are similar to those
- * for the NXAST_REG_LOAD action, except that 'dst' must be
- * NXM_NX_REG(idx) for 'idx' in the switch's supported range.
+ * for the NXAST_REG_LOAD action.
*
* The switch will reject actions in which ofs+n_bits is greater than the width
* of 'dst', with error type OFPET_BAD_ACTION, code OFPBAC_BAD_ARGUMENT.
diff --git a/lib/autopath.c b/lib/autopath.c
index 889b037..133162e 100644
--- a/lib/autopath.c
+++ b/lib/autopath.c
@@ -37,11 +37,7 @@ void
autopath_execute(const struct nx_action_autopath *ap, struct flow *flow,
uint16_t ofp_port)
{
- uint32_t *reg = &flow->regs[NXM_NX_REG_IDX(ntohl(ap->dst))];
- int ofs = nxm_decode_ofs(ap->ofs_nbits);
- int n_bits = nxm_decode_n_bits(ap->ofs_nbits);
- uint32_t mask = n_bits == 32 ? UINT32_MAX : (UINT32_C(1) << n_bits) - 1;
- *reg = (*reg & ~(mask << ofs)) | ((ofp_port & mask) << ofs);
+ nxm_reg_load(ap->dst, ap->ofs_nbits, ofp_port, flow);
}
void
@@ -68,10 +64,6 @@ autopath_parse(struct nx_action_autopath *ap, const char *s_)
}
nxm_parse_field_bits(dst, ®, &ofs, &n_bits);
- if (!NXM_IS_NX_REG(reg) || NXM_NX_REG_IDX(reg) >= FLOW_N_REGS) {
- ovs_fatal(0, "%s: destination field must be a register", s_);
- }
-
if (n_bits < 16) {
ovs_fatal(0, "%s: %d-bit destination field has %u possible values, "
"less than required 65536", s_, n_bits, 1u << n_bits);
@@ -90,21 +82,12 @@ autopath_parse(struct nx_action_autopath *ap, const char
*s_)
}
int
-autopath_check(const struct nx_action_autopath *ap)
+autopath_check(const struct nx_action_autopath *ap, const struct flow *flow)
{
- uint32_t dst = ntohl(ap->dst);
- int ofs = nxm_decode_ofs(ap->ofs_nbits);
- int n_bits = nxm_decode_n_bits(ap->ofs_nbits);
-
- if (!NXM_IS_NX_REG(dst) || NXM_NX_REG_IDX(dst) >= FLOW_N_REGS) {
- VLOG_WARN_RL(&rl, "unsupported destination field %#"PRIx32, dst);
- } else if (ofs + n_bits > nxm_field_bits(dst)) {
- VLOG_WARN_RL(&rl, "destination overflows output field");
- } else if (n_bits < 16) {
- VLOG_WARN_RL(&rl, "minimum of 16 bits required in output field");
- } else {
- return 0;
- }
+ int error = nxm_dst_check(ap->dst, ap->ofs_nbits, 16, flow);
- return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT);
+ if (error) {
+ VLOG_WARN_RL(&rl, "invalid destination");
+ }
+ return error;
}
diff --git a/lib/autopath.h b/lib/autopath.h
index ba55f90..98b02b4 100644
--- a/lib/autopath.h
+++ b/lib/autopath.h
@@ -29,6 +29,6 @@ struct nx_action_autopath;
void autopath_execute(const struct nx_action_autopath *, struct flow *,
uint16_t ofp_port);
void autopath_parse(struct nx_action_autopath *, const char *);
-int autopath_check(const struct nx_action_autopath *);
+int autopath_check(const struct nx_action_autopath *, const struct flow *);
#endif /* autopath.h */
diff --git a/lib/multipath.c b/lib/multipath.c
index 4ee6fd0..ff7a6b6 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -34,11 +34,17 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1,
5);
/* multipath_check(). */
int
-multipath_check(const struct nx_action_multipath *mp)
+multipath_check(const struct nx_action_multipath *mp, const struct flow *flow)
{
- uint32_t dst = ntohl(mp->dst);
- int ofs = nxm_decode_ofs(mp->ofs_nbits);
- int n_bits = nxm_decode_n_bits(mp->ofs_nbits);
+ uint32_t n_links = ntohs(mp->max_link) + 1;
+ size_t min_n_bits = log_2_floor(n_links) + 1;
+ int error;
+
+ error = nxm_dst_check(mp->dst, mp->ofs_nbits, min_n_bits, flow);
+ if (error) {
+ VLOG_WARN_RL(&rl, "invalid destination");
+ return error;
+ }
if (!flow_hash_fields_valid(ntohs(mp->fields))) {
VLOG_WARN_RL(&rl, "unsupported fields %"PRIu16, ntohs(mp->fields));
@@ -48,12 +54,6 @@ multipath_check(const struct nx_action_multipath *mp)
&& mp->algorithm != htons(NX_MP_ALG_ITER_HASH)) {
VLOG_WARN_RL(&rl, "unsupported algorithm %"PRIu16,
ntohs(mp->algorithm));
- } else if (!NXM_IS_NX_REG(dst) || NXM_NX_REG_IDX(dst) >= FLOW_N_REGS) {
- VLOG_WARN_RL(&rl, "unsupported destination field %#"PRIx32, dst);
- } else if (ofs + n_bits > nxm_field_bits(dst)) {
- VLOG_WARN_RL(&rl, "destination overflows output field");
- } else if (n_bits < 16 && ntohs(mp->max_link) > (1u << n_bits)) {
- VLOG_WARN_RL(&rl, "max_link overflows output field");
} else {
return 0;
}
@@ -76,12 +76,7 @@ multipath_execute(const struct nx_action_multipath *mp,
struct flow *flow)
ntohs(mp->max_link) + 1,
ntohl(mp->arg));
- /* Store it. */
- uint32_t *reg = &flow->regs[NXM_NX_REG_IDX(ntohl(mp->dst))];
- int ofs = nxm_decode_ofs(mp->ofs_nbits);
- int n_bits = nxm_decode_n_bits(mp->ofs_nbits);
- uint32_t mask = n_bits == 32 ? UINT32_MAX : (UINT32_C(1) << n_bits) - 1;
- *reg = (*reg & ~(mask << ofs)) | (link << ofs);
+ nxm_reg_load(mp->dst, mp->ofs_nbits, link, flow);
}
static uint16_t
@@ -214,9 +209,6 @@ multipath_parse(struct nx_action_multipath *mp, const char
*s_)
mp->arg = htonl(atoi(arg));
nxm_parse_field_bits(dst, &header, &ofs, &n_bits);
- if (!NXM_IS_NX_REG(header) || NXM_NX_REG_IDX(header) >= FLOW_N_REGS) {
- ovs_fatal(0, "%s: destination field must be register", s_);
- }
if (n_bits < 16 && n_links > (1u << n_bits)) {
ovs_fatal(0, "%s: %d-bit destination field has %u possible values, "
"less than specified n_links %d",
diff --git a/lib/multipath.h b/lib/multipath.h
index 962602b..8ac4bfd 100644
--- a/lib/multipath.h
+++ b/lib/multipath.h
@@ -29,7 +29,7 @@ struct nx_action_reg_move;
* See include/openflow/nicira-ext.h for NXAST_MULTIPATH specification.
*/
-int multipath_check(const struct nx_action_multipath *);
+int multipath_check(const struct nx_action_multipath *, const struct flow *);
void multipath_execute(const struct nx_action_multipath *, struct flow *);
void multipath_parse(struct nx_action_multipath *, const char *);
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 3712781..b382b26 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1211,30 +1211,46 @@ nxm_check_reg_move(const struct nx_action_reg_move
*action,
return 0;
}
+/* 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. */
int
-nxm_check_reg_load(const struct nx_action_reg_load *action,
- const struct flow *flow)
+nxm_dst_check(ovs_be32 dst_header, ovs_be16 ofs_nbits, size_t min_n_bits,
+ const struct flow *flow)
{
const struct nxm_field *dst;
int ofs, n_bits;
- ofs = nxm_decode_ofs(action->ofs_nbits);
- n_bits = nxm_decode_n_bits(action->ofs_nbits);
- dst = nxm_field_lookup(ntohl(action->dst));
- if (!field_ok(dst, flow, ofs + n_bits)) {
+ ofs = nxm_decode_ofs(ofs_nbits);
+ n_bits = nxm_decode_n_bits(ofs_nbits);
+ dst = nxm_field_lookup(ntohl(dst_header));
+
+ if (!field_ok(dst, flow, ofs + n_bits) || !dst->writable
+ || n_bits < min_n_bits) {
return BAD_ARGUMENT;
}
+ return 0;
+}
+
+int
+nxm_check_reg_load(const struct nx_action_reg_load *action,
+ const struct flow *flow)
+{
+ int n_bits;
+ int error;
+
+ error = nxm_dst_check(action->dst, action->ofs_nbits, 0, 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;
}
- if (!dst->writable) {
- return BAD_ARGUMENT;
- }
-
return 0;
}
@@ -1427,31 +1443,30 @@ nxm_execute_reg_move(const struct nx_action_reg_move
*action,
int src_ofs = ntohs(action->src_ofs);
uint64_t src_data = nxm_read_field(src, flow) & (mask << src_ofs);
- /* Get the remaining bits of the destination field. */
- const struct nxm_field *dst = nxm_field_lookup(ntohl(action->dst));
- int dst_ofs = ntohs(action->dst_ofs);
- uint64_t dst_data = nxm_read_field(dst, flow) & ~(mask << dst_ofs);
-
- /* Get the final value. */
- uint64_t new_data = dst_data | ((src_data >> src_ofs) << dst_ofs);
-
- nxm_write_field(dst, flow, new_data);
+ nxm_reg_load(action->dst,
+ nxm_encode_ofs_nbits(ntohs(action->dst_ofs), n_bits),
+ src_data, flow);
}
void
nxm_execute_reg_load(const struct nx_action_reg_load *action,
struct flow *flow)
{
- /* Preparation. */
- int n_bits = nxm_decode_n_bits(action->ofs_nbits);
- uint64_t mask = n_bits == 64 ? UINT64_MAX : (UINT64_C(1) << n_bits) - 1;
+ nxm_reg_load(action->dst, action->ofs_nbits, ntohll(action->value), flow);
+}
- /* Get source data. */
- uint64_t src_data = ntohll(action->value);
+/* Calculates ofs and n_bits from the given 'ofs_nbits' parameter, and copies
+ * 'src_data'[0:n_bits] to 'dst_header'[ofs:ofs+n_bits] in the given 'flow'. */
+void
+nxm_reg_load(ovs_be32 dst_header, ovs_be16 ofs_nbits, uint64_t src_data,
+ struct flow *flow)
+{
+ int n_bits = nxm_decode_n_bits(ofs_nbits);
+ int dst_ofs = nxm_decode_ofs(ofs_nbits);
+ uint64_t mask = n_bits == 64 ? UINT64_MAX : (UINT64_C(1) << n_bits) - 1;
/* Get remaining bits of the destination field. */
- const struct nxm_field *dst = nxm_field_lookup(ntohl(action->dst));
- int dst_ofs = nxm_decode_ofs(action->ofs_nbits);
+ const struct nxm_field *dst = nxm_field_lookup(ntohl(dst_header));
uint64_t dst_data = nxm_read_field(dst, flow) & ~(mask << dst_ofs);
/* Get the final value. */
diff --git a/lib/nx-match.h b/lib/nx-match.h
index b2260b1..5365cca 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -49,9 +49,13 @@ 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,
+ const struct flow *);
void nxm_execute_reg_move(const struct nx_action_reg_move *, struct flow *);
void nxm_execute_reg_load(const struct nx_action_reg_load *, struct flow *);
+void nxm_reg_load(ovs_be32 dst, ovs_be16 ofs_nbits, uint64_t src_data,
+ struct flow *);
int nxm_field_bytes(uint32_t header);
int nxm_field_bits(uint32_t header);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 803b033..2e62b91 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2029,11 +2029,13 @@ validate_actions(const union ofp_action *actions,
size_t n_actions,
break;
case OFPUTIL_NXAST_MULTIPATH:
- error = multipath_check((const struct nx_action_multipath *) a);
+ error = multipath_check((const struct nx_action_multipath *) a,
+ flow);
break;
case OFPUTIL_NXAST_AUTOPATH:
- error = autopath_check((const struct nx_action_autopath *) a);
+ error = autopath_check((const struct nx_action_autopath *) a,
+ flow);
break;
case OFPUTIL_NXAST_BUNDLE:
diff --git a/tests/autopath.at b/tests/autopath.at
index 79ecb02..6b837f8 100644
--- a/tests/autopath.at
+++ b/tests/autopath.at
@@ -21,12 +21,6 @@ AT_CHECK([ovs-ofctl parse-flow 'actions=autopath(bad,
NXM_NX_REG0[[]])'], [1], [
])
AT_CLEANUP
-AT_SETUP([autopath action bad destination])
-AT_CHECK([ovs-ofctl parse-flow 'actions=autopath(1, NXM_OF_VLAN_TCI[[]])'],
[1], [],
- [ovs-ofctl: 1, NXM_OF_VLAN_TCI[[]]: destination field must be a register
-])
-AT_CLEANUP
-
AT_SETUP([autopath action destination too narrow])
AT_CHECK([ovs-ofctl parse-flow 'actions=autopath(1,NXM_NX_REG0[[0..7]])'],
[1], [],
[ovs-ofctl: 1,NXM_NX_REG0[[0..7]]: 8-bit destination field has 256 possible
values, less than required 65536
diff --git a/tests/multipath.at b/tests/multipath.at
index 6eafa9a..361c2c4 100644
--- a/tests/multipath.at
+++ b/tests/multipath.at
@@ -303,12 +303,6 @@ AT_CHECK([ovs-ofctl parse-flow
'actions=multipath(eth_src,50,modulo_n,0,0,NXM_NX
])
AT_CLEANUP
-AT_SETUP([multipath action bad destination])
-AT_CHECK([ovs-ofctl parse-flow
'actions=multipath(eth_src,50,modulo_n,1,0,NXM_OF_VLAN_TCI[[]])'], [1], [],
- [ovs-ofctl: eth_src,50,modulo_n,1,0,NXM_OF_VLAN_TCI[[]]: destination field
must be register
-])
-AT_CLEANUP
-
AT_SETUP([multipath action destination too narrow])
AT_CHECK([ovs-ofctl parse-flow
'actions=multipath(eth_src,50,modulo_n,1024,0,NXM_NX_REG0[[0..7]])'], [1], [],
[ovs-ofctl: eth_src,50,modulo_n,1024,0,NXM_NX_REG0[[0..7]]: 8-bit
destination field has 256 possible values, less than specified n_links 1024
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 8572302..b45f01c 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -705,7 +705,7 @@ Hashes \fIfields\fR using \fIbasis\fR as a universal hash
parameter,
then the applies multipath link selection \fIalgorithm\fR (with
parameter \fIarg\fR) to choose one of \fIn_links\fR output links
numbered 0 through \fIn_links\fR minus 1, and stores the link into
-\fIdst\fB[\fIstart\fB..\fIend\fB]\fR, which must be an NXM register as
+\fIdst\fB[\fIstart\fB..\fIend\fB]\fR, which must be an NXM field as
described above.
.IP
Currently, \fIfields\fR must be either \fBeth_src\fR or
@@ -717,7 +717,7 @@ Refer to \fBnicira\-ext.h\fR for more details.
.
.IP "\fBautopath(\fIid\fB, \fIdst\fB[\fIstart\fB..\fIend\fB])\fR"
Given \fIid\fR, chooses an OpenFlow port and populates it in
-\fIdst\fB[\fIstart\fB..\fIend\fB]\fR, which must be an NXM register as
+\fIdst\fB[\fIstart\fB..\fIend\fB]\fR, which must be an NXM field as
described above.
.IP
Currently, \fIid\fR should be the OpenFlow port number of an interface on the
--
1.7.6
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
