[ovs-dev] [bundle 5/5] vswitch: Implement bundle action.
Ethan Jackson
ethan at nicira.com
Tue Jul 19 02:10:24 UTC 2011
Here is an incremental
---
include/openflow/nicira-ext.h | 17 ++++++-------
lib/bundle.c | 52 ++++++++++++++++------------------------
lib/bundle.h | 11 ++++----
tests/bundle.at | 2 +-
tests/ovs-ofctl.at | 2 +
tests/test-bundle.c | 16 +++++-------
utilities/ovs-ofctl.8.in | 9 +++++-
7 files changed, 52 insertions(+), 57 deletions(-)
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 131a76e..960b53f 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -671,15 +671,15 @@ OFP_ASSERT(sizeof(struct nx_action_autopath) == 24);
* NXAST_BUNDLE chooses a slave from a supplied list of options, and outputs to
* its selection.
*
- * The list of possible slaves is appended to the end of the nx_action_bundle
- * structure. The size of each slave is governed by its type as indicated by
- * the 'slave_type' parameter. The list of slaves should be padded at its end
- * with zeros to make the total length of the action a multiple of 8.
+ * The list of possible slaves follows the nx_action_bundle structure. The size
+ * of each slave is governed by its type as indicated by the 'slave_type'
+ * parameter. The list of slaves should be padded at its end with zeros to make
+ * the total length of the action a multiple of 8.
*
* Switches infer from the 'slave_type' parameter the size of each slave. All
* implementations must support the NXM_OF_IN_PORT 'slave_type' which indicates
* that the slaves are OpenFlow port numbers with NXM_LENGTH(NXM_OF_IN_PORT) ==
- * 16 bit width. Switches should reject actions which indicate unknown or
+ * 2 byte width. Switches should reject actions which indicate unknown or
* unsupported slave types.
*
* Switches use a strategy dictated by the 'algorithm' parameter to choose a
@@ -696,7 +696,6 @@ OFP_ASSERT(sizeof(struct nx_action_autopath) == 24);
* The 'zero' parameter at the end of the action structure is reserved for
* future use. Switches are required to reject actions which have nonzero
* bytes in the 'zero' field. */
-
struct nx_action_bundle {
ovs_be16 type; /* OFPAT_VENDOR. */
ovs_be16 len; /* Length including slaves. */
@@ -710,10 +709,10 @@ struct nx_action_bundle {
ovs_be16 fields; /* One of NX_BD_FIELDS_*. */
ovs_be16 basis; /* Universal hash parameter. */
- ovs_be16 slave_type; /* NXM_OF_IN_PORT. */
+ ovs_be32 slave_type; /* NXM_OF_IN_PORT. */
ovs_be16 n_slaves; /* Number of slaves. */
- uint8_t zero[12]; /* Reserved. Must be zero. */
+ uint8_t zero[10]; /* Reserved. Must be zero. */
};
OFP_ASSERT(sizeof(struct nx_action_bundle) == 32);
@@ -735,7 +734,7 @@ enum nx_bd_algorithm {
* O(n_slaves) performance.
*
* Uses the 'fields' and 'basis' parameters. */
- NX_BD_ALG_HRW = NX_MP_ALG_HRW /* Highest Random Weight. */
+ NX_BD_ALG_HRW /* Highest Random Weight. */
};
/* Flexible flow specifications (aka NXM = Nicira Extended Match).
diff --git a/lib/bundle.c b/lib/bundle.c
index 2b7b1a0..323e1b3 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -33,24 +33,22 @@
VLOG_DEFINE_THIS_MODULE(bundle);
/* Executes 'nab' on 'flow'. Uses 'slave_enabled' to determine if the slave
- * designated by 'ofp_port' is up. Returns the choosen slave, or OFPP_NONE if
+ * designated by 'ofp_port' is up. Returns the chosen slave, or OFPP_NONE if
* none of the slaves are acceptable. */
uint16_t
bundle_execute(const struct nx_action_bundle *nab, const struct flow *flow,
- bool slave_enabled(uint16_t ofp_port, void *aux), void *aux)
+ bool (*slave_enabled)(uint16_t ofp_port, void *aux), void *aux)
{
uint32_t flow_hash, best_hash;
- const ovs_be16 *slaves;
int best, i;
assert(nab->algorithm == htons(NX_BD_ALG_HRW));
flow_hash = flow_hash_fields(flow, ntohs(nab->fields), ntohs(nab->basis));
- slaves = bundle_slaves(nab);
best = -1;
for (i = 0; i < ntohs(nab->n_slaves); i++) {
- if (slave_enabled(ntohs(slaves[i]), aux)) {
+ if (slave_enabled(bundle_get_slave(nab, i), aux)) {
uint32_t hash = hash_2words(i, flow_hash);
if (best < 0 || hash > best_hash) {
@@ -60,7 +58,7 @@ bundle_execute(const struct nx_action_bundle *nab, const struct flow *flow,
}
}
- return best >= 0 ? ntohs(slaves[best]) : OFPP_NONE;
+ return best >= 0 ? bundle_get_slave(nab, best) : OFPP_NONE;
}
/* Checks that 'nab' specifies a bundle action which is supported by this
@@ -83,8 +81,7 @@ bundle_check(const struct nx_action_bundle *nab, int max_ports)
slaves_size = ntohs(nab->len) - sizeof *nab;
error = ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT);
- if (fields != NX_HASH_FIELDS_ETH_SRC
- && fields != NX_HASH_FIELDS_SYMMETRIC_L4) {
+ if (!flow_hash_fields_valid(fields)) {
VLOG_WARN_RL(&rl, "unsupported fields %"PRIu16, fields);
} else if (n_slaves > BUNDLE_MAX_SLAVES) {
VLOG_WARN_RL(&rl, "too may slaves");
@@ -104,15 +101,15 @@ bundle_check(const struct nx_action_bundle *nab, int max_ports)
}
if (slaves_size < n_slaves * sizeof(ovs_be16)) {
- VLOG_WARN_RL(&rl, "Nicira action %"PRIu16" only has %zu bytes"
- " allocated for slaves. %zu bytes are required for"
- " %"PRIu16" slaves.\n", subtype, slaves_size,
+ VLOG_WARN_RL(&rl, "Nicira action %"PRIu16" only has %zu bytes "
+ "allocated for slaves. %zu bytes are required for "
+ "%"PRIu16" slaves.", subtype, slaves_size,
n_slaves * sizeof(ovs_be16), n_slaves);
error = ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
}
for (i = 0; i < n_slaves; i++) {
- uint16_t ofp_port = ntohs(bundle_slaves(nab)[i]);
+ uint16_t ofp_port = bundle_get_slave(nab, i);
int ofputil_error = ofputil_check_output_port(ofp_port, max_ports);
if (ofputil_error) {
@@ -133,7 +130,7 @@ bundle_check(const struct nx_action_bundle *nab, int max_ports)
}
/* Converts a bundle action string contained in 's' to an nx_action_bundle and
- * stores it in 'b'. 'b' will be cleared before populated. */
+ * stores it in 'b'. Sets 'b''s l2 pointer to NULL. */
void
bundle_parse(struct ofpbuf *b, const char *s)
{
@@ -159,8 +156,7 @@ bundle_parse(struct ofpbuf *b, const char *s)
s, slave_delim);
}
- ofpbuf_clear(b);
- ofpbuf_prealloc_headroom(b, sizeof *nab);
+ b->l2 = ofpbuf_put_zeros(b, sizeof *nab);
n_slaves = 0;
for (;;) {
@@ -179,11 +175,13 @@ bundle_parse(struct ofpbuf *b, const char *s)
}
/* Slaves array must be multiple of 8 bytes long. */
- ofpbuf_put_zeros(b, 8 - b->size % 8);
+ if (b->size % 8) {
+ ofpbuf_put_zeros(b, 8 - (b->size % 8));
+ }
- nab = ofpbuf_push_zeros(b, sizeof *nab);
+ nab = b->l2;
nab->type = htons(OFPAT_VENDOR);
- nab->len = htons(b->size);
+ nab->len = htons(b->size - ((char *) b->l2 - (char *) b->data));
nab->vendor = htonl(NX_VENDOR_ID);
nab->subtype = htons(NXAST_BUNDLE);
nab->n_slaves = htons(n_slaves);
@@ -211,26 +209,18 @@ bundle_parse(struct ofpbuf *b, const char *s)
ovs_fatal(0, "%s: unknown slave_type `%s'", s, slave_type);
}
+ b->l2 = NULL;
free(tokstr);
}
-/* Appends a human-readbale representation of 'nab' to 's'. */
+/* Appends a human-readable representation of 'nab' to 's'. */
void
bundle_format(const struct nx_action_bundle *nab, struct ds *s)
{
- char *fields, *algorithm, *slave_type;
+ const char *fields, *algorithm, *slave_type;
size_t i;
- switch (ntohs(nab->fields)) {
- case NX_HASH_FIELDS_ETH_SRC:
- fields = "eth_src";
- break;
- case NX_HASH_FIELDS_SYMMETRIC_L4:
- fields = "symmetric_l4";
- break;
- default:
- fields = "<unknown>";
- }
+ fields = flow_hash_fields_to_str(ntohs(nab->fields));
switch (ntohs(nab->algorithm)) {
case NX_BD_ALG_HRW:
@@ -259,7 +249,7 @@ bundle_format(const struct nx_action_bundle *nab, struct ds *s)
ds_put_cstr(s, ",");
}
- ds_put_format(s, "%"PRIu16, ntohs(bundle_slaves(nab)[i]));
+ ds_put_format(s, "%"PRIu16, bundle_get_slave(nab, i));
}
ds_put_cstr(s, ")");
diff --git a/lib/bundle.h b/lib/bundle.h
index 2802244..a202ade 100644
--- a/lib/bundle.h
+++ b/lib/bundle.h
@@ -16,6 +16,7 @@
#ifndef BUNDLE_H
#define BUNDLE_H 1
+#include <arpa/inet.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
@@ -32,17 +33,17 @@ struct ofpbuf;
* See include/openflow/nicira-ext.h for NXAST_BUNDLE specification. */
uint16_t bundle_execute(const struct nx_action_bundle *, const struct flow *,
- bool slave_enabled(uint16_t ofp_port, void *aux),
+ bool (*slave_enabled)(uint16_t ofp_port, void *aux),
void *aux);
int bundle_check(const struct nx_action_bundle *, int max_ports);
void bundle_parse(struct ofpbuf *, const char *);
void bundle_format(const struct nx_action_bundle *, struct ds *);
-/* Returns a pointer to the array of slaves contained in 'nab' */
-static inline const ovs_be16 *
-bundle_slaves(const struct nx_action_bundle *nab)
+/* Returns the 'i'th slave in 'nab'. */
+static inline uint16_t
+bundle_get_slave(const struct nx_action_bundle *nab, size_t i)
{
- return (ovs_be16 *)((char *)nab + sizeof *nab);
+ return ntohs(((ovs_be16 *)(nab + 1))[i]);
}
#endif /* bundle.h */
diff --git a/tests/bundle.at b/tests/bundle.at
index fb2842d..063cdd3 100644
--- a/tests/bundle.at
+++ b/tests/bundle.at
@@ -116,7 +116,7 @@ AT_CHECK([ovs-ofctl parse-flow 'actions=bundle(symmetric_l4,60,hrw,robot,slaves:
])
AT_CLEANUP
-AT_SETUP([bundle action bad slave delimeter])
+AT_SETUP([bundle action bad slave delimiter])
AT_CHECK([ovs-ofctl parse-flow 'actions=bundle(symmetric_l4,60,hrw,ofport,robot:1,2))'], [1], [],
[ovs-ofctl: symmetric_l4,60,hrw,ofport,robot:1,2: missing slave delimiter, expected `slaves' got `robot'
])
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 7d6212d..63292db 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -18,6 +18,7 @@ tun_id=0x1234000056780000/0xffff0000ffff0000,actions=drop
actions=bundle(eth_src,50,active_backup,ofport,slaves:1)
actions=bundle(symmetric_l4,60,hrw,ofport,slaves:2,3)
actions=bundle(symmetric_l4,60,hrw,ofport,slaves:)
+actions=output:1,bundle(eth_src,0,hrw,ofport,slaves:1),output:2
]])
AT_CHECK([ovs-ofctl parse-flows flows.txt
], [0], [stdout])
@@ -39,6 +40,7 @@ NXT_FLOW_MOD: ADD table:255 tun_id=0x1234000056780000/0xffff0000ffff0000 actions
NXT_FLOW_MOD: ADD table:255 actions=bundle(eth_src,50,active_backup,ofport,slaves:1)
NXT_FLOW_MOD: ADD table:255 actions=bundle(symmetric_l4,60,hrw,ofport,slaves:2,3)
NXT_FLOW_MOD: ADD table:255 actions=bundle(symmetric_l4,60,hrw,ofport,slaves:)
+NXT_FLOW_MOD: ADD table:255 actions=output:1,bundle(eth_src,0,hrw,ofport,slaves:1),output:2
]])
AT_CLEANUP
diff --git a/tests/test-bundle.c b/tests/test-bundle.c
index 2108135..8a89292 100644
--- a/tests/test-bundle.c
+++ b/tests/test-bundle.c
@@ -26,8 +26,8 @@
#include "util.h"
-#define N_FLOWS 30000
-#define BD_MAX_SLAVES 8
+#define N_FLOWS 50000
+#define MAX_SLAVES 8 /* Maximum supported by this test framework. */
struct slave {
uint16_t slave_id;
@@ -38,7 +38,7 @@ struct slave {
struct slave_group {
size_t n_slaves;
- struct slave slaves[BD_MAX_SLAVES];
+ struct slave slaves[MAX_SLAVES];
};
static struct slave *
@@ -75,8 +75,8 @@ parse_bundle_actions(char *actions)
nab = ofpbuf_steal_data(&b);
ofpbuf_uninit(&b);
- if (ntohs(nab->n_slaves) > BD_MAX_SLAVES) {
- ovs_fatal(0, "At most %u slaves are supported", BD_MAX_SLAVES);
+ if (ntohs(nab->n_slaves) > MAX_SLAVES) {
+ ovs_fatal(0, "At most %u slaves are supported", MAX_SLAVES);
}
return nab;
@@ -118,9 +118,7 @@ main(int argc, char *argv[])
/* Generate 'slaves' array. */
sg.n_slaves = 0;
for (i = 0; i < ntohs(nab->n_slaves); i++) {
- uint16_t slave_id;
-
- slave_id = ntohs(bundle_slaves(nab)[i]);
+ uint16_t slave_id = bundle_get_slave(nab, i);
if (slave_lookup(&sg, slave_id)) {
ovs_fatal(0, "Redundant slaves are not supported. ");
@@ -162,7 +160,7 @@ main(int argc, char *argv[])
for (j = 0; j < sg.n_slaves; j++) {
slave = &sg.slaves[j];
slave->flow_count = 0;
- slave->enabled = (1 << j) & mask;
+ slave->enabled = ((1 << j) & mask) != 0;
if (slave->enabled) {
n_enabled++;
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 9695f85..8572302 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -727,16 +727,21 @@ the normal bond selection logic will be used to choose the destination port.
Otherwise, the register will be populated with \fIid\fR itself.
.IP
Refer to \fBnicira\-ext.h\fR for more details.
-.RE
.
.IP "\fBbundle(\fIfields\fB, \fIbasis\fB, \fIalgorithm\fB, \fIslave_type\fB, slaves:[\fIs1\fB, \fIs2\fB, ...])\fR"
Hashes \fIfields\fR using \fIbasis\fR as a universal hash parameter, then
applies the bundle link selection \fIalgorithm\fR to choose one of the listed
-slaves represented as \fIslave_type\fR. Outputs to the selected slave.
+slaves represented as \fIslave_type\fR. Currently the only supported
+\fIslave_type\fR is \fBofport\fR. Thus, each \fIs1\fR through \fIsN\fR should
+be an OpenFlow port number. Outputs to the selected slave.
.IP
Currently, \fIfields\fR must be either \fBeth_src\fR or \fBsymmetric_l4\fR and
\fIalgorithm\fR must be one of \fBhrw\fR and \fBactive_backup\fR.
.IP
+Example: \fBbundle(eth_src,0,hrw,ofport,slaves:4,8)\fR uses an Ethernet source
+hash with basis 0, to select between OpenFlow ports 4 and 8 using the Highest
+Random Weight algorithm.
+.IP
Refer to \fBnicira\-ext.h\fR for more details.
.RE
.
--
1.7.6
More information about the dev
mailing list