From a368bb53d9769ae42042e122775672ac9546e3f9 Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Wed, 20 Jul 2011 15:07:46 -0700 Subject: [PATCH] bundle: New action "bundle_load". The bundle_load action behaves the same as the bundle action, except instead of outputting, it writes its result to a register. --- include/openflow/nicira-ext.h | 24 +++++-- lib/bundle.c | 121 ++++++++++++++++++++++++++++------ lib/bundle.h | 7 +- lib/ofp-parse.c | 2 + lib/ofp-print.c | 1 + lib/ofp-util.c | 4 +- lib/ofp-util.h | 3 +- ofproto/ofproto-dpif.c | 7 ++ tests/bundle.at | 8 +-- tests/ovs-ofctl.at | 8 +++ tests/test-bundle.c | 13 ++-- utilities/ovs-ofctl.8.in | 13 ++++ 12 files changed, 175 insertions(+), 36 deletions(-) diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index 09f4ee39..b97cd4fc 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -278,7 +278,8 @@ enum nx_action_subtype { NXAST_SET_TUNNEL64, /* struct nx_action_set_tunnel64 */ NXAST_MULTIPATH, /* struct nx_action_multipath */ NXAST_AUTOPATH, /* struct nx_action_autopath */ - NXAST_BUNDLE /* struct nx_action_bundle */ + NXAST_BUNDLE, /* struct nx_action_bundle */ + NXAST_BUNDLE_LOAD /* struct nx_action_bundle */ }; /* Header for Nicira-defined actions. */ @@ -664,10 +665,11 @@ struct nx_action_autopath { }; OFP_ASSERT(sizeof(struct nx_action_autopath) == 24); -/* Action structure for NXAST_BUNDLE. +/* Action structure for NXAST_BUNDLE and NXAST_BUNDLE_LOAD. * - * NXAST_BUNDLE chooses a slave from a supplied list of options, and outputs to - * its selection. + * The bundle actions choose a slave from a supplied list of options. + * NXAST_BUNDLE outputs to its selection. NXAST_BUNDLE_LOAD writes its + * selection to a register. * * 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' @@ -693,7 +695,14 @@ 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. */ + * bytes in the 'zero' field. + * + * NXAST_BUNDLE actions should have 'ofs_nbits' and 'dst' zeroed. Switches + * should reject actions which have nonzero bytes in either of these fields. + * + * NXAST_BUNDLE_LOAD stores the OpenFlow port number of the selected slave 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. */ struct nx_action_bundle { ovs_be16 type; /* OFPAT_VENDOR. */ ovs_be16 len; /* Length including slaves. */ @@ -710,7 +719,10 @@ struct nx_action_bundle { ovs_be32 slave_type; /* NXM_OF_IN_PORT. */ ovs_be16 n_slaves; /* Number of slaves. */ - uint8_t zero[10]; /* Reserved. Must be zero. */ + ovs_be16 ofs_nbits; /* (ofs << 6) | (n_bits - 1). */ + ovs_be32 dst; /* Destination. */ + + uint8_t zero[4]; /* Reserved. Must be zero. */ }; OFP_ASSERT(sizeof(struct nx_action_bundle) == 32); diff --git a/lib/bundle.c b/lib/bundle.c index 593bd87d..227d3597 100644 --- a/lib/bundle.c +++ b/lib/bundle.c @@ -88,12 +88,22 @@ bundle_execute(const struct nx_action_bundle *nab, const struct flow *flow, } } +void +bundle_execute_load(const struct nx_action_bundle *nab, struct flow *flow, + bool (*slave_enabled)(uint16_t ofp_port, void *aux), + void *aux) +{ + nxm_reg_load(nab->dst, nab->ofs_nbits, + bundle_execute(nab, flow, slave_enabled, aux), flow); +} + /* Checks that 'nab' specifies a bundle action which is supported by this * bundle module. Uses the 'max_ports' parameter to validate each port using * ofputil_check_output_port(). Returns 0 if 'nab' is supported, otherwise an * OpenFlow error code (as returned by ofp_mkerr()). */ int -bundle_check(const struct nx_action_bundle *nab, int max_ports) +bundle_check(const struct nx_action_bundle *nab, int max_ports, + const struct flow *flow) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); uint16_t n_slaves, fields, algorithm, subtype; @@ -129,6 +139,15 @@ bundle_check(const struct nx_action_bundle *nab, int max_ports) } } + if (subtype == NXAST_BUNDLE && (nab->ofs_nbits || nab->dst)) { + VLOG_WARN_RL(&rl, "bundle action has nonzero reserved fields"); + error = ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT); + } + + if (subtype == NXAST_BUNDLE_LOAD) { + error = nxm_dst_check(nab->dst, nab->ofs_nbits, 16, flow) || error; + } + 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 " @@ -158,24 +177,16 @@ bundle_check(const struct nx_action_bundle *nab, int max_ports) return error; } -/* Converts a bundle action string contained in 's' to an nx_action_bundle and - * stores it in 'b'. Sets 'b''s l2 pointer to NULL. */ -void -bundle_parse(struct ofpbuf *b, const char *s) +/* Helper for bundle_parse and bundle_parse_load. */ +static void +bundle_parse__(struct ofpbuf *b, const char *s, char **save_ptr, + const char *fields, const char *basis, const char *algorithm, + const char *slave_type, const char *dst, + const char *slave_delim) { - char *fields, *basis, *algorithm, *slave_type, *slave_delim; struct nx_action_bundle *nab; - char *tokstr, *save_ptr; uint16_t n_slaves; - save_ptr = NULL; - tokstr = xstrdup(s); - fields = strtok_r(tokstr, ", ", &save_ptr); - basis = strtok_r(NULL, ", ", &save_ptr); - algorithm = strtok_r(NULL, ", ", &save_ptr); - slave_type = strtok_r(NULL, ", ", &save_ptr); - slave_delim = strtok_r(NULL, ": ", &save_ptr); - if (!slave_delim) { ovs_fatal(0, "%s: not enough arguments to bundle action", s); } @@ -192,7 +203,7 @@ bundle_parse(struct ofpbuf *b, const char *s) ovs_be16 slave_be; char *slave; - slave = strtok_r(NULL, ", ", &save_ptr); + slave = strtok_r(NULL, ", ", save_ptr); if (!slave || n_slaves >= BUNDLE_MAX_SLAVES) { break; } @@ -212,7 +223,7 @@ bundle_parse(struct ofpbuf *b, const char *s) nab->type = htons(OFPAT_VENDOR); nab->len = htons(b->size - ((char *) b->l2 - (char *) b->data)); nab->vendor = htonl(NX_VENDOR_ID); - nab->subtype = htons(NXAST_BUNDLE); + nab->subtype = htons(dst ? NXAST_BUNDLE_LOAD: NXAST_BUNDLE); nab->n_slaves = htons(n_slaves); nab->basis = htons(atoi(basis)); @@ -238,7 +249,60 @@ bundle_parse(struct ofpbuf *b, const char *s) ovs_fatal(0, "%s: unknown slave_type `%s'", s, slave_type); } + if (dst) { + uint32_t reg; + int ofs, n_bits; + + nxm_parse_field_bits(dst, ®, &ofs, &n_bits); + + nab->dst = htonl(reg); + nab->ofs_nbits = nxm_encode_ofs_nbits(ofs, n_bits); + } + b->l2 = NULL; +} + +/* Converts a bundle action string contained in 's' to an nx_action_bundle and + * stores it in 'b'. Sets 'b''s l2 pointer to NULL. */ +void +bundle_parse(struct ofpbuf *b, const char *s) +{ + char *fields, *basis, *algorithm, *slave_type, *slave_delim; + char *tokstr, *save_ptr; + + save_ptr = NULL; + tokstr = xstrdup(s); + fields = strtok_r(tokstr, ", ", &save_ptr); + basis = strtok_r(NULL, ", ", &save_ptr); + algorithm = strtok_r(NULL, ", ", &save_ptr); + slave_type = strtok_r(NULL, ", ", &save_ptr); + slave_delim = strtok_r(NULL, ": ", &save_ptr); + + bundle_parse__(b, s, &save_ptr, fields, basis, algorithm, slave_type, NULL, + slave_delim); + free(tokstr); +} + +/* Converts a bundle_load action string contained in 's' to an nx_action_bundle + * and stores it in 'b'. Sets 'b''s l2 pointer to NULL. */ +void +bundle_parse_load(struct ofpbuf *b, const char *s) +{ + char *fields, *basis, *algorithm, *slave_type, *dst, *slave_delim; + char *tokstr, *save_ptr; + + save_ptr = NULL; + tokstr = xstrdup(s); + fields = strtok_r(tokstr, ", ", &save_ptr); + basis = strtok_r(NULL, ", ", &save_ptr); + algorithm = strtok_r(NULL, ", ", &save_ptr); + slave_type = strtok_r(NULL, ", ", &save_ptr); + dst = strtok_r(NULL, ", ", &save_ptr); + slave_delim = strtok_r(NULL, ": ", &save_ptr); + + bundle_parse__(b, s, &save_ptr, fields, basis, algorithm, slave_type, dst, + slave_delim); + free(tokstr); } @@ -246,7 +310,7 @@ bundle_parse(struct ofpbuf *b, const char *s) void bundle_format(const struct nx_action_bundle *nab, struct ds *s) { - const char *fields, *algorithm, *slave_type; + const char *action, *fields, *algorithm, *slave_type; size_t i; fields = flow_hash_fields_to_str(ntohs(nab->fields)); @@ -270,9 +334,28 @@ bundle_format(const struct nx_action_bundle *nab, struct ds *s) slave_type = ""; } - ds_put_format(s, "bundle(%s,%"PRIu16",%s,%s,slaves:", fields, + switch (ntohs(nab->subtype)) { + case NXAST_BUNDLE: + action = "bundle"; + break; + case NXAST_BUNDLE_LOAD: + action = "bundle_load"; + break; + default: + NOT_REACHED(); + } + + ds_put_format(s, "%s(%s,%"PRIu16",%s,%s,", action, fields, ntohs(nab->basis), algorithm, slave_type); + if (nab->subtype == htons(NXAST_BUNDLE_LOAD)) { + nxm_format_field_bits(s, ntohl(nab->dst), + nxm_decode_ofs(nab->ofs_nbits), + nxm_decode_n_bits(nab->ofs_nbits)); + ds_put_cstr(s, ","); + } + + ds_put_cstr(s, "slaves:"); for (i = 0; i < ntohs(nab->n_slaves); i++) { if (i) { ds_put_cstr(s, ","); diff --git a/lib/bundle.h b/lib/bundle.h index a202ade4..12497f78 100644 --- a/lib/bundle.h +++ b/lib/bundle.h @@ -35,8 +35,13 @@ struct ofpbuf; uint16_t bundle_execute(const struct nx_action_bundle *, const struct flow *, bool (*slave_enabled)(uint16_t ofp_port, void *aux), void *aux); -int bundle_check(const struct nx_action_bundle *, int max_ports); +void bundle_execute_load(const struct nx_action_bundle *, struct flow *, + bool (*slave_enabled)(uint16_t ofp_port, void *aux), + void *aux); +int bundle_check(const struct nx_action_bundle *, int max_ports, + const struct flow *); void bundle_parse(struct ofpbuf *, const char *); +void bundle_parse_load(struct ofpbuf *b, const char *); void bundle_format(const struct nx_action_bundle *, struct ds *); /* Returns the 'i'th slave in 'nab'. */ diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 25056e7f..58b0da1e 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -507,6 +507,8 @@ str_to_action(char *str, struct ofpbuf *b) autopath_parse(naa, arg); } else if (!strcasecmp(act, "bundle")) { bundle_parse(b, arg); + } else if (!strcasecmp(act, "bundle_load")) { + bundle_parse_load(b, arg); } else if (!strcasecmp(act, "output")) { put_output_action(b, str_to_u32(arg)); } else if (!strcasecmp(act, "enqueue")) { diff --git a/lib/ofp-print.c b/lib/ofp-print.c index aa3ff542..cb361a63 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -344,6 +344,7 @@ ofp_print_action(struct ds *s, const union ofp_action *a, break; case OFPUTIL_NXAST_BUNDLE: + case OFPUTIL_NXAST_BUNDLE_LOAD: bundle_format((const struct nx_action_bundle *) a, s); break; diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 2e62b918..df3377ae 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -2039,8 +2039,9 @@ validate_actions(const union ofp_action *actions, size_t n_actions, break; case OFPUTIL_NXAST_BUNDLE: + case OFPUTIL_NXAST_BUNDLE_LOAD: error = bundle_check((const struct nx_action_bundle *) a, - max_ports); + max_ports, flow); break; case OFPUTIL_OFPAT_STRIP_VLAN: @@ -2116,6 +2117,7 @@ static const struct ofputil_nxast_action nxast_actions[] = { { OFPUTIL_NXAST_MULTIPATH, 32, 32 }, { OFPUTIL_NXAST_AUTOPATH, 24, 24 }, { OFPUTIL_NXAST_BUNDLE, 32, UINT_MAX }, + { OFPUTIL_NXAST_BUNDLE_LOAD, 32, UINT_MAX }, }; static int diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 7938aa0c..7ee3e69f 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -302,7 +302,8 @@ enum ofputil_action_code { OFPUTIL_NXAST_SET_TUNNEL64, OFPUTIL_NXAST_MULTIPATH, OFPUTIL_NXAST_AUTOPATH, - OFPUTIL_NXAST_BUNDLE + OFPUTIL_NXAST_BUNDLE, + OFPUTIL_NXAST_BUNDLE_LOAD, }; int ofputil_decode_action(const union ofp_action *); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index c88569cf..61685b72 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3228,6 +3228,13 @@ do_xlate_actions(const union ofp_action *in, size_t n_in, slave_enabled_cb, ctx->ofproto), 0); break; + + case OFPUTIL_NXAST_BUNDLE_LOAD: + ctx->ofproto->has_bundle_action = true; + nab = (const struct nx_action_bundle *) ia; + bundle_execute_load(nab, &ctx->flow, slave_enabled_cb, + ctx->ofproto); + break; } } } diff --git a/tests/bundle.at b/tests/bundle.at index 8a8cb4f2..8a3c2919 100644 --- a/tests/bundle.at +++ b/tests/bundle.at @@ -8,7 +8,7 @@ AT_BANNER([bundle link selection]) # if the test does fail. AT_SETUP([hrw bundle link selection]) -AT_CHECK([[test-bundle 'symmetric_l4,60,hrw,ofport,slaves:1,2,3,4,5,6']], +AT_CHECK([[test-bundle 'symmetric_l4,60,hrw,ofport,NXM_NX_REG0[],slaves:1,2,3,4,5']], [0], [ignore]) # 100000: disruption=1.00 (perfect=1.00) 1.00 0.00 0.00 0.00 0.00 0.00 # 110000: disruption=0.50 (perfect=0.50) 0.50 0.50 0.00 0.00 0.00 0.00 @@ -78,7 +78,7 @@ AT_CHECK([[test-bundle 'symmetric_l4,60,hrw,ofport,slaves:1,2,3,4,5,6']], AT_CLEANUP AT_SETUP([active_backup bundle link selection]) -AT_CHECK([[test-bundle 'symmetric_l4,60,active_backup,ofport,slaves:1,2,3,4,5,6']], +AT_CHECK([[test-bundle 'symmetric_l4,60,active_backup,ofport,NXM_NX_REG0[],slaves:1,2,3,4,5,6']], [0], [100000: disruption=1.00 (perfect=1.00) 1.00 0.00 0.00 0.00 0.00 0.00 110000: disruption=0.00 (perfect=0.00) 1.00 0.00 0.00 0.00 0.00 0.00 @@ -149,7 +149,7 @@ AT_CHECK([[test-bundle 'symmetric_l4,60,active_backup,ofport,slaves:1,2,3,4,5,6' AT_CLEANUP AT_SETUP([hrw bundle single link selection]) -AT_CHECK([[test-bundle 'symmetric_l4,60,hrw,ofport,slaves:1']], +AT_CHECK([[test-bundle 'symmetric_l4,60,hrw,ofport,NXM_NX_REG0[],slaves:1']], [0], [ignore]) # 1: disruption=1.00 (perfect=1.00) 1.00 # 0: disruption=1.00 (perfect=1.00) 0.00 @@ -157,7 +157,7 @@ AT_CHECK([[test-bundle 'symmetric_l4,60,hrw,ofport,slaves:1']], AT_CLEANUP AT_SETUP([hrw bundle no link selection]) -AT_CHECK([[test-bundle 'symmetric_l4,60,hrw,ofport,slaves:']], +AT_CHECK([[test-bundle 'symmetric_l4,60,hrw,ofport,NXM_NX_REG0[],slaves:']], [0], [ignore]) AT_CLEANUP #: disruption=0.00 (perfect=0.00) diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 63292dbb..1b4cfd90 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -19,6 +19,10 @@ 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 +actions=bundle_load(eth_src,50,active_backup,ofport,NXM_NX_REG0[],slaves:1) +actions=bundle_load(symmetric_l4,60,hrw,ofport,NXM_NX_REG0[0..15],slaves:2,3) +actions=bundle_load(symmetric_l4,60,hrw,ofport,NXM_NX_REG0[0..30],slaves:) +actions=output:1,bundle_load(eth_src,0,hrw,ofport,NXM_NX_REG0[16..31],slaves:1),output:2 ]]) AT_CHECK([ovs-ofctl parse-flows flows.txt ], [0], [stdout]) @@ -41,6 +45,10 @@ NXT_FLOW_MOD: ADD table:255 actions=bundle(eth_src,50,active_backup,ofport,slave 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 +NXT_FLOW_MOD: ADD table:255 actions=bundle_load(eth_src,50,active_backup,ofport,NXM_NX_REG0[],slaves:1) +NXT_FLOW_MOD: ADD table:255 actions=bundle_load(symmetric_l4,60,hrw,ofport,NXM_NX_REG0[0..15],slaves:2,3) +NXT_FLOW_MOD: ADD table:255 actions=bundle_load(symmetric_l4,60,hrw,ofport,NXM_NX_REG0[0..30],slaves:) +NXT_FLOW_MOD: ADD table:255 actions=output:1,bundle_load(eth_src,0,hrw,ofport,NXM_NX_REG0[16..31],slaves:1),output:2 ]]) AT_CLEANUP diff --git a/tests/test-bundle.c b/tests/test-bundle.c index fb15faf9..16e264d2 100644 --- a/tests/test-bundle.c +++ b/tests/test-bundle.c @@ -71,7 +71,7 @@ parse_bundle_actions(char *actions) struct ofpbuf b; ofpbuf_init(&b, 0); - bundle_parse(&b, actions); + bundle_parse_load(&b, actions); nab = ofpbuf_steal_data(&b); ofpbuf_uninit(&b); @@ -136,7 +136,7 @@ main(int argc, char *argv[]) flows[i].regs[0] = OFPP_NONE; } - if (bundle_check(nab, 1024)) { + if (bundle_check(nab, 1024, flows)) { ovs_fatal(0, "Bundle action fails to check."); } @@ -185,10 +185,15 @@ main(int argc, char *argv[]) changed = 0; for (j = 0; j < N_FLOWS; j++) { struct flow *flow = &flows[j]; - uint16_t old_slave_id; + uint16_t old_slave_id, ofp_port; old_slave_id = flow->regs[0]; - flow->regs[0] = bundle_execute(nab, flow, slave_enabled_cb, &sg); + ofp_port = bundle_execute(nab, flow, slave_enabled_cb, &sg); + bundle_execute_load(nab, flow, slave_enabled_cb, &sg); + if (flow->regs[0] != ofp_port) { + ovs_fatal(0, "bundle_execute_load() and bundle_execute() " + "disagree"); + } if (flow->regs[0] != OFPP_NONE) { slave_lookup(&sg, flow->regs[0])->flow_count++; diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index b45f01c6..bb52be22 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -743,6 +743,19 @@ 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. +. +.IP "\fBbundle_load(\fIfields\fB, \fIbasis\fB, \fIalgorithm\fB, \fIslave_type\fB, \fIdst\fB[\fIstart\fB..\fIend\fB], slaves:[\fIs1\fB, \fIs2\fB, ...])\fR" +Has the same behavior as the \fBbundle\fR action, with one exception. Instead +of outputting to the selected slave, it writes its selection to +\fIdst\fB[\fIstart\fB..\fIend\fB]\fR, which must be an NXM field as described +above. +.IP +Example: \fBbundle_load(eth_src,0,hrw,ofport,NXM_NX_REG0[],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, and writes the selection to +\fBNXM_NX_REG0[]\fR. +.IP +Refer to \fBnicira\-ext.h\fR for more details. .RE . .IP -- 2.30.2