That's a good thought, thank you.

On Tue, Aug 09, 2011 at 09:37:05AM -0700, Justin Pettit wrote:
> What about "DESIGN"?
> 
> --Justin
> 
> 
> On Aug 9, 2011, at 9:24 AM, Ben Pfaff wrote:
> 
> > I decided to further add:
> > 
> >     Tables 128 and above are reserved for use by the switch
> >        itself; please use only tables 0 through 127.
> > 
> > I'd like to document this somewhere else in the tree but it's not
> > clear where.
> > 
> > On Tue, Aug 09, 2011 at 09:22:29AM -0700, Ben Pfaff wrote:
> >> Good idea.  I added:
> >> 
> >>    - ovs-vswitchd:
> >>      - The software switch now supports 255 OpenFlow tables, instead
> >>        of just one.  By default, only table 0 is consulted, but the
> >>        new NXAST_RESUBMIT_TABLE action can look up in additional
> >>        tables.
> >> 
> >> I'll push this soon.
> >> 
> >> On Mon, Aug 08, 2011 at 05:25:59PM -0700, Ethan Jackson wrote:
> >>> Looks good,
> >>> 
> >>> May want to update NEWS for this series.
> >>> Ethan
> >>> 
> >>> On Thu, Aug 4, 2011 at 16:29, Ben Pfaff <b...@nicira.com> wrote:
> >>>> This makes multiple table support in ofproto-dpif useful, by allowing
> >>>> resubmits into tables other than 0.
> >>>> ---
> >>>> ?include/openflow/nicira-ext.h | ? 52 ++++++++++++++++++++++---------
> >>>> ?lib/ofp-parse.c ? ? ? ? ? ? ? | ? 39 +++++++++++++++++++++--
> >>>> ?lib/ofp-print.c ? ? ? ? ? ? ? | ? 13 ++++++++
> >>>> ?lib/ofp-util.c ? ? ? ? ? ? ? ?| ? 15 +++++++++
> >>>> ?lib/ofp-util.h ? ? ? ? ? ? ? ?| ? ?1 +
> >>>> ?ofproto/ofproto-dpif.c ? ? ? ?| ? 68 
> >>>> +++++++++++++++++++++++++++++-----------
> >>>> ?tests/automake.mk ? ? ? ? ? ? | ? ?1 +
> >>>> ?tests/ofproto-dpif.at ? ? ? ? | ? 19 +++++++++++
> >>>> ?tests/ovs-ofctl.at ? ? ? ? ? ?| ? ?3 ++
> >>>> ?tests/testsuite.at ? ? ? ? ? ?| ? ?1 +
> >>>> ?utilities/ovs-ofctl.8.in ? ? ?| ? 14 ++++++--
> >>>> ?11 files changed, 184 insertions(+), 42 deletions(-)
> >>>> ?create mode 100644 tests/ofproto-dpif.at
> >>>> 
> >>>> diff --git a/include/openflow/nicira-ext.h 
> >>>> b/include/openflow/nicira-ext.h
> >>>> index 5cf02e7..776ec39 100644
> >>>> --- a/include/openflow/nicira-ext.h
> >>>> +++ b/include/openflow/nicira-ext.h
> >>>> @@ -279,7 +279,8 @@ enum nx_action_subtype {
> >>>> ? ? NXAST_MULTIPATH, ? ? ? ? ? ?/* struct nx_action_multipath */
> >>>> ? ? NXAST_AUTOPATH, ? ? ? ? ? ? /* struct nx_action_autopath */
> >>>> ? ? NXAST_BUNDLE, ? ? ? ? ? ? ? /* struct nx_action_bundle */
> >>>> - ? ?NXAST_BUNDLE_LOAD ? ? ? ? ? /* struct nx_action_bundle */
> >>>> + ? ?NXAST_BUNDLE_LOAD, ? ? ? ? ?/* struct nx_action_bundle */
> >>>> + ? ?NXAST_RESUBMIT_TABLE ? ? ? ?/* struct nx_action_resubmit */
> >>>> ?};
> >>>> 
> >>>> ?/* Header for Nicira-defined actions. */
> >>>> @@ -292,31 +293,51 @@ struct nx_action_header {
> >>>> ?};
> >>>> ?OFP_ASSERT(sizeof(struct nx_action_header) == 16);
> >>>> 
> >>>> -/* Action structure for NXAST_RESUBMIT.
> >>>> +/* Action structures for NXAST_RESUBMIT and NXAST_RESUBMIT_TABLE.
> >>>> ?*
> >>>> - * NXAST_RESUBMIT searches the flow table again, using a flow that is 
> >>>> slightly
> >>>> - * modified from the original lookup:
> >>>> + * These actions search one of the switch's flow tables:
> >>>> ?*
> >>>> - * ? ?- The 'in_port' member of struct nx_action_resubmit is used as 
> >>>> the flow's
> >>>> - * ? ? ?in_port.
> >>>> + * ? ?- For NXAST_RESUBMIT_TABLE only, if the 'table' member is not 
> >>>> 255, then
> >>>> + * ? ? ?it specifies the table to search.
> >>>> ?*
> >>>> - * ? ?- If NXAST_RESUBMIT is preceded by actions that affect the flow
> >>>> - * ? ? ?(e.g. OFPAT_SET_VLAN_VID), then the flow is updated with the new
> >>>> - * ? ? ?values.
> >>>> + * ? ?- Otherwise (for NXAST_RESUBMIT_TABLE with a 'table' of 255, or 
> >>>> for
> >>>> + * ? ? ?NXAST_RESUBMIT regardless of 'table'), it searches the current 
> >>>> flow
> >>>> + * ? ? ?table, that is, the OpenFlow flow table that contains the flow 
> >>>> from
> >>>> + * ? ? ?which this action was obtained. ?If this action did not come 
> >>>> from a
> >>>> + * ? ? ?flow table (e.g. it came from an OFPT_PACKET_OUT message), then 
> >>>> table 0
> >>>> + * ? ? ?is the current table.
> >>>> + *
> >>>> + * The flow table lookup uses a flow that may be slightly modified from 
> >>>> the
> >>>> + * original lookup:
> >>>> + *
> >>>> + * ? ?- For NXAST_RESUBMIT, the 'in_port' member of struct 
> >>>> nx_action_resubmit
> >>>> + * ? ? ?is used as the flow's in_port.
> >>>> + *
> >>>> + * ? ?- For NXAST_RESUBMIT_TABLE, if the 'in_port' member is not 
> >>>> OFPP_IN_PORT,
> >>>> + * ? ? ?then its value is used as the flow's in_port. ?Otherwise, the 
> >>>> original
> >>>> + * ? ? ?in_port is used.
> >>>> + *
> >>>> + * ? ?- If actions that modify the flow (e.g. OFPAT_SET_VLAN_VID) 
> >>>> precede the
> >>>> + * ? ? ?resubmit action, then the flow is updated with the new values.
> >>>> ?*
> >>>> ?* Following the lookup, the original in_port is restored.
> >>>> ?*
> >>>> ?* If the modified flow matched in the flow table, then the corresponding
> >>>> - * actions are executed. ?Afterward, actions following NXAST_RESUBMIT 
> >>>> in the
> >>>> + * actions are executed. ?Afterward, actions following the resubmit in 
> >>>> the
> >>>> ?* original set of actions, if any, are executed; any changes made to the
> >>>> ?* packet (e.g. changes to VLAN) by secondary actions persist when those
> >>>> ?* actions are executed, although the original in_port is restored.
> >>>> ?*
> >>>> - * NXAST_RESUBMIT may be used any number of times within a set of 
> >>>> actions.
> >>>> + * Resubmit actions may be used any number of times within a set of 
> >>>> actions.
> >>>> + *
> >>>> + * Resubmit actions may nest to an implementation-defined depth. 
> >>>> ?Beyond this
> >>>> + * implementation-defined depth, further resubmit actions are simply 
> >>>> ignored.
> >>>> + *
> >>>> + * NXAST_RESUBMIT ignores 'table' and 'pad'. ?NXAST_RESUBMIT_TABLE 
> >>>> requires
> >>>> + * 'pad' to be all-bits-zero.
> >>>> ?*
> >>>> - * NXAST_RESUBMIT may nest to an implementation-defined depth. ?Beyond 
> >>>> this
> >>>> - * implementation-defined depth, further NXAST_RESUBMIT actions are 
> >>>> simply
> >>>> - * ignored. ?(Open vSwitch 1.0.1 and earlier did not support recursion.)
> >>>> + * Open vSwitch 1.0.1 and earlier did not support recursion. ?Open 
> >>>> vSwitch
> >>>> + * before 1.2.90 did not support NXAST_RESUBMIT_TABLE.
> >>>> ?*/
> >>>> ?struct nx_action_resubmit {
> >>>> ? ? ovs_be16 type; ? ? ? ? ? ? ? ? ?/* OFPAT_VENDOR. */
> >>>> @@ -324,7 +345,8 @@ struct nx_action_resubmit {
> >>>> ? ? ovs_be32 vendor; ? ? ? ? ? ? ? ?/* NX_VENDOR_ID. */
> >>>> ? ? ovs_be16 subtype; ? ? ? ? ? ? ? /* NXAST_RESUBMIT. */
> >>>> ? ? ovs_be16 in_port; ? ? ? ? ? ? ? /* New in_port for checking flow 
> >>>> table. */
> >>>> - ? ?uint8_t pad[4];
> >>>> + ? ?uint8_t table; ? ? ? ? ? ? ? ? ?/* NXAST_RESUBMIT_TABLE: table to 
> >>>> use. */
> >>>> + ? ?uint8_t pad[3];
> >>>> ?};
> >>>> ?OFP_ASSERT(sizeof(struct nx_action_resubmit) == 16);
> >>>> 
> >>>> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> >>>> index 58b0da1..c171293 100644
> >>>> --- a/lib/ofp-parse.c
> >>>> +++ b/lib/ofp-parse.c
> >>>> @@ -285,7 +285,6 @@ put_dl_addr_action(struct ofpbuf *b, uint16_t type, 
> >>>> const char *addr)
> >>>> ? ? str_to_mac(addr, oada->dl_addr);
> >>>> ?}
> >>>> 
> >>>> -
> >>>> ?static bool
> >>>> ?parse_port_name(const char *name, uint16_t *port)
> >>>> ?{
> >>>> @@ -318,6 +317,40 @@ parse_port_name(const char *name, uint16_t *port)
> >>>> ?}
> >>>> 
> >>>> ?static void
> >>>> +parse_resubmit(struct nx_action_resubmit *nar, char *arg)
> >>>> +{
> >>>> + ? ?char *in_port_s, *table_s;
> >>>> + ? ?uint16_t in_port;
> >>>> + ? ?uint8_t table;
> >>>> +
> >>>> + ? ?in_port_s = strsep(&arg, ",");
> >>>> + ? ?if (in_port_s && in_port_s[0]) {
> >>>> + ? ? ? ?if (!parse_port_name(in_port_s, &in_port)) {
> >>>> + ? ? ? ? ? ?in_port = str_to_u32(in_port_s);
> >>>> + ? ? ? ?}
> >>>> + ? ?} else {
> >>>> + ? ? ? ?in_port = OFPP_IN_PORT;
> >>>> + ? ?}
> >>>> +
> >>>> + ? ?table_s = strsep(&arg, ",");
> >>>> + ? ?table = table_s && table_s[0] ? str_to_u32(table_s) : 255;
> >>>> +
> >>>> + ? ?if (in_port == OFPP_IN_PORT && table == 255) {
> >>>> + ? ? ? ?ovs_fatal(0, "at least one \"in_port\" or \"table\" must be 
> >>>> specified "
> >>>> + ? ? ? ? ? ? ? ? ?" on resubmit");
> >>>> + ? ?}
> >>>> +
> >>>> + ? ?nar->vendor = htonl(NX_VENDOR_ID);
> >>>> + ? ?nar->in_port = htons(in_port);
> >>>> + ? ?if (in_port != OFPP_IN_PORT && table == 255) {
> >>>> + ? ? ? ?nar->subtype = htons(NXAST_RESUBMIT);
> >>>> + ? ?} else {
> >>>> + ? ? ? ?nar->subtype = htons(NXAST_RESUBMIT_TABLE);
> >>>> + ? ? ? ?nar->table = table;
> >>>> + ? ?}
> >>>> +}
> >>>> +
> >>>> +static void
> >>>> ?str_to_action(char *str, struct ofpbuf *b)
> >>>> ?{
> >>>> ? ? bool drop = false;
> >>>> @@ -421,9 +454,7 @@ str_to_action(char *str, struct ofpbuf *b)
> >>>> ? ? ? ? } else if (!strcasecmp(act, "resubmit")) {
> >>>> ? ? ? ? ? ? struct nx_action_resubmit *nar;
> >>>> ? ? ? ? ? ? nar = put_action(b, sizeof *nar, OFPAT_VENDOR);
> >>>> - ? ? ? ? ? ?nar->vendor = htonl(NX_VENDOR_ID);
> >>>> - ? ? ? ? ? ?nar->subtype = htons(NXAST_RESUBMIT);
> >>>> - ? ? ? ? ? ?nar->in_port = htons(str_to_u32(arg));
> >>>> + ? ? ? ? ? ?parse_resubmit(nar, arg);
> >>>> ? ? ? ? } else if (!strcasecmp(act, "set_tunnel")
> >>>> ? ? ? ? ? ? ? ? ? ?|| !strcasecmp(act, "set_tunnel64")) {
> >>>> ? ? ? ? ? ? uint64_t tun_id = str_to_u64(arg);
> >>>> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> >>>> index 0265f30..9311c14 100644
> >>>> --- a/lib/ofp-print.c
> >>>> +++ b/lib/ofp-print.c
> >>>> @@ -295,6 +295,19 @@ ofp_print_action(struct ds *s, const union 
> >>>> ofp_action *a,
> >>>> ? ? ? ? ofp_print_port_name(s, ntohs(nar->in_port));
> >>>> ? ? ? ? break;
> >>>> 
> >>>> + ? ?case OFPUTIL_NXAST_RESUBMIT_TABLE:
> >>>> + ? ? ? ?nar = (struct nx_action_resubmit *)a;
> >>>> + ? ? ? ?ds_put_format(s, "resubmit(");
> >>>> + ? ? ? ?if (nar->in_port != htons(OFPP_IN_PORT)) {
> >>>> + ? ? ? ? ? ?ofp_print_port_name(s, ntohs(nar->in_port));
> >>>> + ? ? ? ?}
> >>>> + ? ? ? ?ds_put_char(s, ',');
> >>>> + ? ? ? ?if (nar->table != 255) {
> >>>> + ? ? ? ? ? ?ds_put_format(s, "%"PRIu8, nar->table);
> >>>> + ? ? ? ?}
> >>>> + ? ? ? ?ds_put_char(s, ')');
> >>>> + ? ? ? ?break;
> >>>> +
> >>>> ? ? case OFPUTIL_NXAST_SET_TUNNEL:
> >>>> ? ? ? ? nast = (struct nx_action_set_tunnel *)a;
> >>>> ? ? ? ? ds_put_format(s, "set_tunnel:%#"PRIx32, ntohl(nast->tun_id));
> >>>> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> >>>> index df3377a..fdd61fb 100644
> >>>> --- a/lib/ofp-util.c
> >>>> +++ b/lib/ofp-util.c
> >>>> @@ -1966,6 +1966,15 @@ ofputil_check_output_port(uint16_t port, int 
> >>>> max_ports)
> >>>> ? ? }
> >>>> ?}
> >>>> 
> >>>> +static int
> >>>> +check_resubmit_table(const struct nx_action_resubmit *nar)
> >>>> +{
> >>>> + ? ?if (nar->pad[0] || nar->pad[1] || nar->pad[2]) {
> >>>> + ? ? ? ?return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT);
> >>>> + ? ?}
> >>>> + ? ?return 0;
> >>>> +}
> >>>> +
> >>>> ?int
> >>>> ?validate_actions(const union ofp_action *actions, size_t n_actions,
> >>>> ? ? ? ? ? ? ? ? ?const struct flow *flow, int max_ports)
> >>>> @@ -2044,6 +2053,11 @@ validate_actions(const union ofp_action *actions, 
> >>>> size_t n_actions,
> >>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?max_ports, flow);
> >>>> ? ? ? ? ? ? break;
> >>>> 
> >>>> + ? ? ? ?case OFPUTIL_NXAST_RESUBMIT_TABLE:
> >>>> + ? ? ? ? ? ?error = check_resubmit_table(
> >>>> + ? ? ? ? ? ? ? ?(const struct nx_action_resubmit *) a);
> >>>> + ? ? ? ? ? ?break;
> >>>> +
> >>>> ? ? ? ? case OFPUTIL_OFPAT_STRIP_VLAN:
> >>>> ? ? ? ? case OFPUTIL_OFPAT_SET_NW_SRC:
> >>>> ? ? ? ? case OFPUTIL_OFPAT_SET_NW_DST:
> >>>> @@ -2118,6 +2132,7 @@ static const struct ofputil_nxast_action 
> >>>> nxast_actions[] = {
> >>>> ? ? { OFPUTIL_NXAST_AUTOPATH, ? ? 24, 24 },
> >>>> ? ? { OFPUTIL_NXAST_BUNDLE, ? ? ? 32, UINT_MAX },
> >>>> ? ? { OFPUTIL_NXAST_BUNDLE_LOAD, ?32, UINT_MAX },
> >>>> + ? ?{ OFPUTIL_NXAST_RESUBMIT_TABLE, 16, 16 },
> >>>> ?};
> >>>> 
> >>>> ?static int
> >>>> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> >>>> index 7ee3e69..a473364 100644
> >>>> --- a/lib/ofp-util.h
> >>>> +++ b/lib/ofp-util.h
> >>>> @@ -304,6 +304,7 @@ enum ofputil_action_code {
> >>>> ? ? OFPUTIL_NXAST_AUTOPATH,
> >>>> ? ? OFPUTIL_NXAST_BUNDLE,
> >>>> ? ? OFPUTIL_NXAST_BUNDLE_LOAD,
> >>>> + ? ?OFPUTIL_NXAST_RESUBMIT_TABLE
> >>>> ?};
> >>>> 
> >>>> ?int ofputil_decode_action(const union ofp_action *);
> >>>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> >>>> index 1ec7dc2..1b5854e 100644
> >>>> --- a/ofproto/ofproto-dpif.c
> >>>> +++ b/ofproto/ofproto-dpif.c
> >>>> @@ -60,7 +60,7 @@ COVERAGE_DEFINE(facet_invalidated);
> >>>> ?COVERAGE_DEFINE(facet_revalidate);
> >>>> ?COVERAGE_DEFINE(facet_unexpected);
> >>>> 
> >>>> -/* Maximum depth of flow table recursion (due to NXAST_RESUBMIT 
> >>>> actions) in a
> >>>> +/* Maximum depth of flow table recursion (due to resubmit actions) in a
> >>>> ?* flow translation. */
> >>>> ?#define MAX_RESUBMIT_RECURSION 16
> >>>> 
> >>>> @@ -96,8 +96,8 @@ static struct rule_dpif *rule_dpif_cast(const struct 
> >>>> rule *rule)
> >>>> ? ? return rule ? CONTAINER_OF(rule, struct rule_dpif, up) : NULL;
> >>>> ?}
> >>>> 
> >>>> -static struct rule_dpif *rule_dpif_lookup(struct ofproto_dpif *ofproto,
> >>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const struct flow *flow);
> >>>> +static struct rule_dpif *rule_dpif_lookup(struct ofproto_dpif *,
> >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const struct flow *, uint8_t 
> >>>> table);
> >>>> 
> >>>> ?#define MAX_MIRRORS 32
> >>>> ?typedef uint32_t mirror_mask_t;
> >>>> @@ -188,6 +188,7 @@ struct action_xlate_ctx {
> >>>> ? ? uint32_t priority; ? ? ? ? ?/* Current flow priority. 0 if none. */
> >>>> ? ? struct flow base_flow; ? ? ?/* Flow at the last commit. */
> >>>> ? ? uint32_t base_priority; ? ? /* Priority at the last commit. */
> >>>> + ? ?uint8_t table_id; ? ? ? ? ? /* OpenFlow table ID where flow was 
> >>>> found. */
> >>>> ?};
> >>>> 
> >>>> ?static void action_xlate_ctx_init(struct action_xlate_ctx *,
> >>>> @@ -1654,7 +1655,7 @@ handle_miss_upcall(struct ofproto_dpif *ofproto, 
> >>>> struct dpif_upcall *upcall)
> >>>> 
> >>>> ? ? facet = facet_lookup_valid(ofproto, &flow);
> >>>> ? ? if (!facet) {
> >>>> - ? ? ? ?struct rule_dpif *rule = rule_dpif_lookup(ofproto, &flow);
> >>>> + ? ? ? ?struct rule_dpif *rule = rule_dpif_lookup(ofproto, &flow, 0);
> >>>> ? ? ? ? if (!rule) {
> >>>> ? ? ? ? ? ? /* Don't send a packet-in if OFPPC_NO_PACKET_IN asserted. */
> >>>> ? ? ? ? ? ? struct ofport_dpif *port = get_ofp_port(ofproto, 
> >>>> flow.in_port);
> >>>> @@ -2434,7 +2435,7 @@ facet_revalidate(struct ofproto_dpif *ofproto, 
> >>>> struct facet *facet)
> >>>> ? ? COVERAGE_INC(facet_revalidate);
> >>>> 
> >>>> ? ? /* Determine the new rule. */
> >>>> - ? ?new_rule = rule_dpif_lookup(ofproto, &facet->flow);
> >>>> + ? ?new_rule = rule_dpif_lookup(ofproto, &facet->flow, 0);
> >>>> ? ? if (!new_rule) {
> >>>> ? ? ? ? /* No new rule, so delete the facet. */
> >>>> ? ? ? ? facet_remove(ofproto, facet);
> >>>> @@ -2592,10 +2593,11 @@ flow_push_stats(const struct rule_dpif *rule,
> >>>> ?/* Rules. */
> >>>> 
> >>>> ?static struct rule_dpif *
> >>>> -rule_dpif_lookup(struct ofproto_dpif *ofproto, const struct flow *flow)
> >>>> +rule_dpif_lookup(struct ofproto_dpif *ofproto, const struct flow *flow,
> >>>> + ? ? ? ? ? ? ? ? uint8_t table_id)
> >>>> ?{
> >>>> ? ? return rule_dpif_cast(rule_from_cls_rule(
> >>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?classifier_lookup(&ofproto->up.tables[0],
> >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? 
> >>>> ?classifier_lookup(&ofproto->up.tables[table_id],
> >>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? flow)));
> >>>> ?}
> >>>> 
> >>>> @@ -2717,7 +2719,7 @@ rule_execute(struct rule *rule_, struct flow 
> >>>> *flow, struct ofpbuf *packet)
> >>>> 
> >>>> ? ? /* Otherwise, if 'rule' is in fact the correct rule for 'packet', 
> >>>> then
> >>>> ? ? ?* create a new facet for it and use that. */
> >>>> - ? ?if (rule_dpif_lookup(ofproto, flow) == rule) {
> >>>> + ? ?if (rule_dpif_lookup(ofproto, flow, 0) == rule) {
> >>>> ? ? ? ? facet = facet_create(rule, flow, packet);
> >>>> ? ? ? ? facet_execute(ofproto, facet, packet);
> >>>> ? ? ? ? facet_install(ofproto, facet, true);
> >>>> @@ -2889,18 +2891,23 @@ add_output_action(struct action_xlate_ctx *ctx, 
> >>>> uint16_t ofp_port)
> >>>> ?}
> >>>> 
> >>>> ?static void
> >>>> -xlate_table_action(struct action_xlate_ctx *ctx, uint16_t in_port)
> >>>> +xlate_table_action(struct action_xlate_ctx *ctx,
> >>>> + ? ? ? ? ? ? ? ? ? uint16_t in_port, uint8_t table_id)
> >>>> ?{
> >>>> ? ? if (ctx->recurse < MAX_RESUBMIT_RECURSION) {
> >>>> ? ? ? ? struct rule_dpif *rule;
> >>>> ? ? ? ? uint16_t old_in_port;
> >>>> + ? ? ? ?uint8_t old_table_id;
> >>>> +
> >>>> + ? ? ? ?old_table_id = ctx->table_id;
> >>>> + ? ? ? ?ctx->table_id = table_id;
> >>>> 
> >>>> ? ? ? ? /* Look up a flow with 'in_port' as the input port. ?Then 
> >>>> restore the
> >>>> ? ? ? ? ?* original input port (otherwise OFPP_NORMAL and OFPP_IN_PORT 
> >>>> will
> >>>> ? ? ? ? ?* have surprising behavior). */
> >>>> ? ? ? ? old_in_port = ctx->flow.in_port;
> >>>> ? ? ? ? ctx->flow.in_port = in_port;
> >>>> - ? ? ? ?rule = rule_dpif_lookup(ctx->ofproto, &ctx->flow);
> >>>> + ? ? ? ?rule = rule_dpif_lookup(ctx->ofproto, &ctx->flow, table_id);
> >>>> ? ? ? ? ctx->flow.in_port = old_in_port;
> >>>> 
> >>>> ? ? ? ? if (ctx->resubmit_hook) {
> >>>> @@ -2912,15 +2919,32 @@ xlate_table_action(struct action_xlate_ctx *ctx, 
> >>>> uint16_t in_port)
> >>>> ? ? ? ? ? ? do_xlate_actions(rule->up.actions, rule->up.n_actions, ctx);
> >>>> ? ? ? ? ? ? ctx->recurse--;
> >>>> ? ? ? ? }
> >>>> +
> >>>> + ? ? ? ?ctx->table_id = old_table_id;
> >>>> ? ? } else {
> >>>> ? ? ? ? static struct vlog_rate_limit recurse_rl = 
> >>>> VLOG_RATE_LIMIT_INIT(1, 1);
> >>>> 
> >>>> - ? ? ? ?VLOG_ERR_RL(&recurse_rl, "NXAST_RESUBMIT recursed over %d 
> >>>> times",
> >>>> + ? ? ? ?VLOG_ERR_RL(&recurse_rl, "resubmit actions recursed over %d 
> >>>> times",
> >>>> ? ? ? ? ? ? ? ? ? ? MAX_RESUBMIT_RECURSION);
> >>>> ? ? }
> >>>> ?}
> >>>> 
> >>>> ?static void
> >>>> +xlate_resubmit_table(struct action_xlate_ctx *ctx,
> >>>> + ? ? ? ? ? ? ? ? ? ? const struct nx_action_resubmit *nar)
> >>>> +{
> >>>> + ? ?uint16_t in_port;
> >>>> + ? ?uint8_t table_id;
> >>>> +
> >>>> + ? ?in_port = (nar->in_port == htons(OFPP_IN_PORT)
> >>>> + ? ? ? ? ? ? ? ? ctx->flow.in_port
> >>>> + ? ? ? ? ? ? ? : ntohs(nar->in_port));
> >>>> + ? ?table_id = nar->table == 255 ? ctx->table_id : nar->table;
> >>>> +
> >>>> + ? ?xlate_table_action(ctx, in_port, table_id);
> >>>> +}
> >>>> +
> >>>> +static void
> >>>> ?flood_packets(struct action_xlate_ctx *ctx, ovs_be32 mask)
> >>>> ?{
> >>>> ? ? struct ofport_dpif *ofport;
> >>>> @@ -2950,7 +2974,7 @@ xlate_output_action__(struct action_xlate_ctx *ctx,
> >>>> ? ? ? ? add_output_action(ctx, ctx->flow.in_port);
> >>>> ? ? ? ? break;
> >>>> ? ? case OFPP_TABLE:
> >>>> - ? ? ? ?xlate_table_action(ctx, ctx->flow.in_port);
> >>>> + ? ? ? ?xlate_table_action(ctx, ctx->flow.in_port, ctx->table_id);
> >>>> ? ? ? ? break;
> >>>> ? ? case OFPP_NORMAL:
> >>>> ? ? ? ? xlate_normal(ctx);
> >>>> @@ -3182,7 +3206,11 @@ do_xlate_actions(const union ofp_action *in, 
> >>>> size_t n_in,
> >>>> 
> >>>> ? ? ? ? case OFPUTIL_NXAST_RESUBMIT:
> >>>> ? ? ? ? ? ? nar = (const struct nx_action_resubmit *) ia;
> >>>> - ? ? ? ? ? ?xlate_table_action(ctx, ntohs(nar->in_port));
> >>>> + ? ? ? ? ? ?xlate_table_action(ctx, ntohs(nar->in_port), ctx->table_id);
> >>>> + ? ? ? ? ? ?break;
> >>>> +
> >>>> + ? ? ? ?case OFPUTIL_NXAST_RESUBMIT_TABLE:
> >>>> + ? ? ? ? ? ?xlate_resubmit_table(ctx, (const struct nx_action_resubmit 
> >>>> *) ia);
> >>>> ? ? ? ? ? ? break;
> >>>> 
> >>>> ? ? ? ? case OFPUTIL_NXAST_SET_TUNNEL:
> >>>> @@ -3272,6 +3300,7 @@ xlate_actions(struct action_xlate_ctx *ctx,
> >>>> ? ? ctx->priority = 0;
> >>>> ? ? ctx->base_priority = 0;
> >>>> ? ? ctx->base_flow = ctx->flow;
> >>>> + ? ?ctx->table_id = 0;
> >>>> 
> >>>> ? ? if (process_special(ctx->ofproto, &ctx->flow, ctx->packet)) {
> >>>> ? ? ? ? ctx->may_set_up_flow = false;
> >>>> @@ -3925,7 +3954,8 @@ struct ofproto_trace {
> >>>> ?};
> >>>> 
> >>>> ?static void
> >>>> -trace_format_rule(struct ds *result, int level, const struct rule *rule)
> >>>> +trace_format_rule(struct ds *result, uint8_t table_id, int level,
> >>>> + ? ? ? ? ? ? ? ? ?const struct rule *rule)
> >>>> ?{
> >>>> ? ? ds_put_char_multiple(result, '\t', level);
> >>>> ? ? if (!rule) {
> >>>> @@ -3933,8 +3963,8 @@ trace_format_rule(struct ds *result, int level, 
> >>>> const struct rule *rule)
> >>>> ? ? ? ? return;
> >>>> ? ? }
> >>>> 
> >>>> - ? ?ds_put_format(result, "Rule: cookie=%#"PRIx64" ",
> >>>> - ? ? ? ? ? ? ? ? ?ntohll(rule->flow_cookie));
> >>>> + ? ?ds_put_format(result, "Rule: table=%"PRIu8" cookie=%#"PRIx64" ",
> >>>> + ? ? ? ? ? ? ? ? ?table_id, ntohll(rule->flow_cookie));
> >>>> ? ? cls_rule_format(&rule->cr, result);
> >>>> ? ? ds_put_char(result, '\n');
> >>>> 
> >>>> @@ -3967,7 +3997,7 @@ trace_resubmit(struct action_xlate_ctx *ctx, 
> >>>> struct rule_dpif *rule)
> >>>> 
> >>>> ? ? ds_put_char(result, '\n');
> >>>> ? ? trace_format_flow(result, ctx->recurse + 1, "Resubmitted flow", 
> >>>> trace);
> >>>> - ? ?trace_format_rule(result, ctx->recurse + 1, &rule->up);
> >>>> + ? ?trace_format_rule(result, ctx->table_id, ctx->recurse + 1, 
> >>>> &rule->up);
> >>>> ?}
> >>>> 
> >>>> ?static void
> >>>> @@ -4054,8 +4084,8 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, 
> >>>> const char *args_,
> >>>> ? ? flow_format(&result, &flow);
> >>>> ? ? ds_put_char(&result, '\n');
> >>>> 
> >>>> - ? ?rule = rule_dpif_lookup(ofproto, &flow);
> >>>> - ? ?trace_format_rule(&result, 0, &rule->up);
> >>>> + ? ?rule = rule_dpif_lookup(ofproto, &flow, 0);
> >>>> + ? ?trace_format_rule(&result, 0, 0, &rule->up);
> >>>> ? ? if (rule) {
> >>>> ? ? ? ? struct ofproto_trace trace;
> >>>> ? ? ? ? struct ofpbuf *odp_actions;
> >>>> diff --git a/tests/automake.mk b/tests/automake.mk
> >>>> index eb6351f..28b4e1d 100644
> >>>> --- a/tests/automake.mk
> >>>> +++ b/tests/automake.mk
> >>>> @@ -28,6 +28,7 @@ TESTSUITE_AT = \
> >>>> ? ? ? ?tests/timeval.at \
> >>>> ? ? ? ?tests/lockfile.at \
> >>>> ? ? ? ?tests/reconnect.at \
> >>>> + ? ? ? tests/ofproto-dpif.at \
> >>>> ? ? ? ?tests/ofproto-macros.at \
> >>>> ? ? ? ?tests/ofproto.at \
> >>>> ? ? ? ?tests/ovsdb.at \
> >>>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> >>>> new file mode 100644
> >>>> index 0000000..846eccb
> >>>> --- /dev/null
> >>>> +++ b/tests/ofproto-dpif.at
> >>>> @@ -0,0 +1,19 @@
> >>>> +AT_BANNER([ofproto-dpif])
> >>>> +
> >>>> +AT_SETUP([ofproto-dpif - resubmit])
> >>>> +OFPROTO_START
> >>>> +AT_DATA([flows.txt], [dnl
> >>>> +table=0 in_port=1 priority=1000 icmp 
> >>>> actions=output(10),resubmit(2),output(19),resubmit(3),output(21)
> >>>> +table=0 in_port=2 priority=1500 icmp 
> >>>> actions=output(11),resubmit(,1),output(16),resubmit(2,1),output(18)
> >>>> +table=0 in_port=3 priority=2000 icmp actions=output(20)
> >>>> +table=1 in_port=1 priority=1000 icmp 
> >>>> actions=output(12),resubmit(4,1),output(13),resubmit(3),output(15)
> >>>> +table=1 in_port=2 priority=1500 icmp actions=output(17),resubmit(,2)
> >>>> +table=1 in_port=3 priority=1500 icmp actions=output(14),resubmit(,2)
> >>>> +])
> >>>> +AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >>>> +AT_CHECK([ovs-appctl -t test-openflowd ofproto/trace br0 
> >>>> 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0),icmp(type=8,code=0)'],
> >>>>  [0], [stdout])
> >>>> +AT_CHECK([tail -1 stdout], [0],
> >>>> + ?[Datapath actions: 10,11,12,13,14,15,16,17,18,19,20,21
> >>>> +])
> >>>> +OFPROTO_STOP
> >>>> +AT_CLEANUP
> >>>> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> >>>> index 20dcc90..1edfb62 100644
> >>>> --- a/tests/ovs-ofctl.at
> >>>> +++ b/tests/ovs-ofctl.at
> >>>> @@ -23,7 +23,9 @@ 
> >>>> 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
> >>>> +actions=resubmit:1,resubmit(2),resubmit(,3),resubmit(2,3)
> >>>> ?]])
> >>>> +
> >>>> ?AT_CHECK([ovs-ofctl parse-flows flows.txt
> >>>> ?], [0], [stdout])
> >>>> ?AT_CHECK([[sed 's/ (xid=0x[0-9a-fA-F]*)//' stdout]], [0],
> >>>> @@ -49,6 +51,7 @@ NXT_FLOW_MOD: ADD table:255 
> >>>> actions=bundle_load(eth_src,50,active_backup,ofport,
> >>>> ?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
> >>>> +NXT_FLOW_MOD: ADD table:255 
> >>>> actions=resubmit:1,resubmit:2,resubmit(,3),resubmit(2,3)
> >>>> ?]])
> >>>> ?AT_CLEANUP
> >>>> 
> >>>> diff --git a/tests/testsuite.at b/tests/testsuite.at
> >>>> index c22ece7..b394cb6 100644
> >>>> --- a/tests/testsuite.at
> >>>> +++ b/tests/testsuite.at
> >>>> @@ -60,6 +60,7 @@ m4_include([tests/timeval.at])
> >>>> ?m4_include([tests/lockfile.at])
> >>>> ?m4_include([tests/reconnect.at])
> >>>> ?m4_include([tests/ofproto.at])
> >>>> +m4_include([tests/ofproto-dpif.at])
> >>>> ?m4_include([tests/ovsdb.at])
> >>>> ?m4_include([tests/ovs-vsctl.at])
> >>>> ?m4_include([tests/interface-reconfigure.at])
> >>>> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> >>>> index c59bca9..aa37969 100644
> >>>> --- a/utilities/ovs-ofctl.8.in
> >>>> +++ b/utilities/ovs-ofctl.8.in
> >>>> @@ -655,10 +655,16 @@ only known to be implemented by Open vSwitch:
> >>>> ?.RS
> >>>> ?.
> >>>> ?.IP \fBresubmit\fB:\fIport\fR
> >>>> -Re-searches the OpenFlow flow table with the \fBin_port\fR field
> >>>> -replaced by \fIport\fR and executes the actions found, if any, in
> >>>> -addition to any other actions in this flow entry. ?Recursive
> >>>> -\fBresubmit\fR actions are ignored.
> >>>> +.IQ \fBresubmit\fB(\fR[\fIport\fR]\fB,\fR[\fItable\fR]\fB)
> >>>> +Re-searches this OpenFlow flow table (or the table whose number is
> >>>> +specified by \fItable\fR) with the \fBin_port\fR field replaced by
> >>>> +\fIport\fR (if \fIport\fR is specified) and executes the actions
> >>>> +found, if any, in addition to any other actions in this flow entry.
> >>>> +.IP
> >>>> +Recursive \fBresubmit\fR actions are obeyed up to an
> >>>> +implementation-defined maximum depth. ?Open vSwitch 1.0.1 and earlier
> >>>> +did not support recursion; Open vSwitch before 1.2.90 did not support
> >>>> +\fItable\fR.
> >>>> ?.
> >>>> ?.IP \fBset_tunnel\fB:\fIid\fR
> >>>> ?.IQ \fBset_tunnel64\fB:\fIid\fR
> >>>> --
> >>>> 1.7.4.4
> >>>> 
> >>>> _______________________________________________
> >>>> dev mailing list
> >>>> dev@openvswitch.org
> >>>> http://openvswitch.org/mailman/listinfo/dev
> >>>> 
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to