actions: Make "next" action able to jump from egress to ingress pipel…

…ine. This feature is useful for centralized gateways. Signed-off-by: Ben Pfaff <[email protected]> Acked-by: Mickey Spiegel <[email protected]>
dkarhi · Jan 21, 2017 · 4c99cb1 · 4c99cb1
1 parent c571f48
commit 4c99cb1
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 48 deletions.
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
@@ -151,13 +151,15 @@ struct ovnact_next {
     struct ovnact ovnact;
 
     /* Arguments. */
-    uint8_t ltable;             /* Logical table ID of next table. */
+    uint8_t ltable;                /* Logical table ID of next table. */
+    enum ovnact_pipeline pipeline; /* Pipeline of next table. */
 
     /* Information about the flow that the action is in.  This does not affect
      * behavior, since the implementation of "next" doesn't depend on the
      * source table or pipeline.  It does affect how ovnacts_format() prints
      * the action. */
-    uint8_t src_ltable;            /* Logical table ID of source table. */
+    uint8_t src_ltable;                /* Logical table ID of source table. */
+    enum ovnact_pipeline src_pipeline; /* Pipeline of source table. */
 };
 
 /* OVNACT_LOAD. */
@@ -402,22 +404,26 @@ struct ovnact_parse_params {
     /* hmap of 'struct dhcp_opts_map'  to support 'put_dhcpv6_opts' action */
     const struct hmap *dhcpv6_opts;
 
-    /* OVN maps each logical flow table (ltable), one-to-one, onto a physical
-     * OpenFlow flow table (ptable).  A number of parameters describe this
-     * mapping and data related to flow tables:
+    /* Each OVN flow exists in a logical table within a logical pipeline.
+     * These parameters express this context for a set of OVN actions being
+     * parsed:
      *
-     *     - 'first_ptable' and 'n_tables' define the range of OpenFlow tables
-     *        to which the logical "next" action should be able to jump.
-     *        Logical table 0 maps to OpenFlow table 'first_ptable', logical
-     *        table 1 to 'first_ptable + 1', and so on.  If 'n_tables' is 0
-     *        then "next" is disallowed entirely.
+     *     - 'n_tables' is the number of tables in the logical ingress and
+     *        egress pipelines, that is, "next" may specify a table less than
+     *        or equal to 'n_tables'.  If 'n_tables' is 0 then "next" is
+     *        disallowed entirely.
      *
-     *     - 'cur_ltable' is an offset from 'first_ptable' (e.g. 0 <=
-     *       cur_ltable < n_tables) of the logical flow that contains the
-     *       actions.  If cur_ltable + 1 < n_tables, then this defines the
-     *       default table that "next" will jump to. */
-    uint8_t n_tables;           /* Number of flow tables. */
-    uint8_t cur_ltable;         /* 0 <= cur_ltable < n_tables. */
+     *     - 'cur_ltable' is the logical table of the current flow, within
+     *       'pipeline'.  If cur_ltable + 1 < n_tables, then this defines the
+     *       default table that "next" will jump to.
+     *
+     *     - 'pipeline' is the logical pipeline.  It is the default pipeline to
+     *       which 'next' will jump.  If 'pipeline' is OVNACT_P_EGRESS, then
+     *       'next' will also be able to jump into the ingress pipeline, but
+     *       the reverse is not true. */
+    enum ovnact_pipeline pipeline; /* Logical pipeline. */
+    uint8_t n_tables;              /* Number of logical flow tables. */
+    uint8_t cur_ltable;            /* 0 <= cur_ltable < n_tables. */
 };
 
 bool ovnacts_parse(struct lexer *, const struct ovnact_parse_params *,
@@ -448,20 +454,23 @@ struct ovnact_encode_params {
      * OpenFlow flow table (ptable).  A number of parameters describe this
      * mapping and data related to flow tables:
      *
-     *     - 'first_ptable' and 'n_tables' define the range of OpenFlow tables
-     *        to which the logical "next" action should be able to jump.
-     *        Logical table 0 maps to OpenFlow table 'first_ptable', logical
-     *        table 1 to 'first_ptable + 1', and so on.  If 'n_tables' is 0
-     *        then "next" is disallowed entirely.
+     *     - 'pipeline' is the logical pipeline in which the actions are
+     *       executing.
      *
-     *     - 'cur_ltable' is an offset from 'first_ptable' (e.g. 0 <=
-     *       cur_ltable < n_ptables) of the logical flow that contains the
-     *       actions.  If cur_ltable + 1 < n_tables, then this defines the
-     *       default table that "next" will jump to.
+     *     - 'ingress_ptable' is the OpenFlow table that corresponds to OVN
+     *       ingress table 0.
+     *
+     *     - 'egress_ptable' is the OpenFlow table that corresponds to OVN
+     *       egress table 0.
      *
      *     - 'output_ptable' should be the OpenFlow table to which the logical
-     *       "output" action will resubmit. */
-    uint8_t first_ptable;       /* First OpenFlow table. */
+     *       "output" action will resubmit.
+     *
+     *     - 'mac_bind_ptable' should be the OpenFlow table used to track MAC
+     *       bindings. */
+    enum ovnact_pipeline pipeline; /* Logical pipeline. */
+    uint8_t ingress_ptable;     /* First OpenFlow ingress table. */
+    uint8_t egress_ptable;      /* First OpenFlow egress table. */
     uint8_t output_ptable;      /* OpenFlow table for 'output' to resubmit. */
     uint8_t mac_bind_ptable;    /* OpenFlow table for 'get_arp'/'get_nd' to
                                    resubmit. */

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2015, 2016 Nicira, Inc.
+/* Copyright (c) 2015, 2016, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -191,6 +191,7 @@ consider_logical_flow(const struct lport_index *lports,
         .dhcp_opts = dhcp_opts,
         .dhcpv6_opts = dhcpv6_opts,
 
+        .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
         .n_tables = LOG_PIPELINE_LEN,
         .cur_ltable = lflow->table_id,
     };
@@ -223,7 +224,9 @@ consider_logical_flow(const struct lport_index *lports,
         .ct_zones = ct_zones,
         .group_table = group_table,
 
-        .first_ptable = first_ptable,
+        .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
+        .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE,
+        .egress_ptable = OFTABLE_LOG_EGRESS_PIPELINE,
         .output_ptable = output_ptable,
         .mac_bind_ptable = OFTABLE_MAC_BINDING,
     };

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
@@ -170,6 +170,15 @@ put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
     bitwise_copy(&n_value, 8, 0, sf->value, sf->field->n_bytes, ofs, n_bits);
     bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
 }
+
+static uint8_t
+first_ptable(const struct ovnact_encode_params *ep,
+             enum ovnact_pipeline pipeline)
+{
+    return (pipeline == OVNACT_P_INGRESS
+            ? ep->ingress_ptable
+            : ep->egress_ptable);
+}
 
 /* Context maintained during ovnacts_parse(). */
 struct action_context {
@@ -262,29 +271,70 @@ parse_NEXT(struct action_context *ctx)
         return;
     }
 
+    int pipeline = ctx->pp->pipeline;
     int table = ctx->pp->cur_ltable + 1;
-    if (lexer_match(ctx->lexer, LEX_T_LPAREN)
-        && (!lexer_force_int(ctx->lexer, &table) ||
-            !lexer_force_match(ctx->lexer, LEX_T_RPAREN))) {
-        return;
+    if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
+        if (lexer_is_int(ctx->lexer)) {
+            lexer_get_int(ctx->lexer, &table);
+        } else {
+            do {
+                if (lexer_match_id(ctx->lexer, "pipeline")) {
+                    if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
+                        return;
+                    }
+                    if (lexer_match_id(ctx->lexer, "ingress")) {
+                        pipeline = OVNACT_P_INGRESS;
+                    } else if (lexer_match_id(ctx->lexer, "egress")) {
+                        pipeline = OVNACT_P_EGRESS;
+                    } else {
+                        lexer_syntax_error(
+                            ctx->lexer, "expecting \"ingress\" or \"egress\"");
+                        return;
+                    }
+                } else if (lexer_match_id(ctx->lexer, "table")) {
+                    if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS) ||
+                        !lexer_force_int(ctx->lexer, &table)) {
+                        return;
+                    }
+                } else {
+                    lexer_syntax_error(ctx->lexer,
+                                       "expecting \"pipeline\" or \"table\"");
+                    return;
+                }
+            } while (lexer_match(ctx->lexer, LEX_T_COMMA));
+        }
+        if (!lexer_force_match(ctx->lexer, LEX_T_RPAREN)) {
+            return;
+        }
     }
 
-    if (table >= ctx->pp->n_tables) {
+    if (pipeline == OVNACT_P_EGRESS && ctx->pp->pipeline == OVNACT_P_INGRESS) {
+        lexer_error(ctx->lexer,
+                    "\"next\" action cannot advance from ingress to egress "
+                    "pipeline (use \"output\" action instead)");
+    } else if (table >= ctx->pp->n_tables) {
         lexer_error(ctx->lexer,
                     "\"next\" action cannot advance beyond table %d.",
                     ctx->pp->n_tables - 1);
         return;
     }
 
     struct ovnact_next *next = ovnact_put_NEXT(ctx->ovnacts);
+    next->pipeline = pipeline;
     next->ltable = table;
+    next->src_pipeline = ctx->pp->pipeline;
     next->src_ltable = ctx->pp->cur_ltable;
 }
 
 static void
 format_NEXT(const struct ovnact_next *next, struct ds *s)
 {
-    if (next->ltable != next->src_ltable + 1) {
+    if (next->pipeline != next->src_pipeline) {
+        ds_put_format(s, "next(pipeline=%s, table=%d);",
+                      (next->pipeline == OVNACT_P_INGRESS
+                       ? "ingress" : "egress"),
+                      next->ltable);
+    } else if (next->ltable != next->src_ltable + 1) {
         ds_put_format(s, "next(%d);", next->ltable);
     } else {
         ds_put_cstr(s, "next;");
@@ -296,7 +346,7 @@ encode_NEXT(const struct ovnact_next *next,
             const struct ovnact_encode_params *ep,
             struct ofpbuf *ofpacts)
 {
-    emit_resubmit(ofpacts, ep->first_ptable + next->ltable);
+    emit_resubmit(ofpacts, first_ptable(ep, next->pipeline) + next->ltable);
 }
 
 static void
@@ -541,7 +591,7 @@ encode_CT_NEXT(const struct ovnact_ct_next *ct_next,
                 struct ofpbuf *ofpacts)
 {
     struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
-    ct->recirc_table = ep->first_ptable + ct_next->ltable;
+    ct->recirc_table = first_ptable(ep, ep->pipeline) + ct_next->ltable;
     ct->zone_src.field = ep->is_switch ? mf_from_id(MFF_LOG_CT_ZONE)
                             : mf_from_id(MFF_LOG_DNAT_ZONE);
     ct->zone_src.ofs = 0;
@@ -745,7 +795,7 @@ encode_ct_nat(const struct ovnact_ct_nat *cn,
     ofpbuf_pull(ofpacts, ct_offset);
 
     struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
-    ct->recirc_table = cn->ltable + ep->first_ptable;
+    ct->recirc_table = cn->ltable + first_ptable(ep, ep->pipeline);
     if (snat) {
         ct->zone_src.field = mf_from_id(MFF_LOG_SNAT_ZONE);
     } else {
@@ -889,7 +939,7 @@ encode_CT_LB(const struct ovnact_ct_lb *cl,
              const struct ovnact_encode_params *ep,
              struct ofpbuf *ofpacts)
 {
-    uint8_t recirc_table = cl->ltable + ep->first_ptable;
+    uint8_t recirc_table = cl->ltable + first_ptable(ep, ep->pipeline);
     if (!cl->n_dsts) {
         /* ct_lb without any destinations means that this is an established
          * connection and we just need to do a NAT. */

diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
@@ -956,10 +956,16 @@
 
         <dt><code>next;</code></dt>
         <dt><code>next(<var>table</var>);</code></dt>
+        <dt><code>next(pipeline=<var>pipeline</var>, table=<var>table</var>);</code></dt>
         <dd>
-          Executes another logical datapath table as a subroutine.  By default,
-          the table after the current one is executed.  Specify
-          <var>table</var> to jump to a specific table in the same pipeline.
+          Executes the given logical datapath <var>table</var> in
+          <var>pipeline</var> as a subroutine.  The default <var>table</var> is
+          just after the current one.  If <var>pipeline</var> is specified, it
+          may be <code>ingress</code> or <code>egress</code>; the default
+          <var>pipeline</var> is the one currently executing.  Actions in the
+          ingress pipeline may not use <code>next</code> to jump into the
+          egress pipeline (use the <code>output</code> instead), but
+          transitions in the opposite direction are allowed.
         </dd>
 
         <dt><code><var>field</var> = <var>constant</var>;</code></dt>

diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
@@ -409,6 +409,16 @@ ovntrace_port_find_by_key(const struct ovntrace_datapath *dp,
     return NULL;
 }
 
+static const char *
+ovntrace_port_key_to_name(const struct ovntrace_datapath *dp,
+                          uint16_t key)
+{
+    const struct ovntrace_port *port = ovntrace_port_find_by_key(dp, key);
+    return (port ? port->name
+            : !key ? ""
+            : "(unnamed)");
+}
+
 static const struct ovntrace_mcgroup *
 ovntrace_mcgroup_find_by_key(const struct ovntrace_datapath *dp,
                              uint16_t tunnel_key)
@@ -673,6 +683,9 @@ read_flows(void)
             .symtab = &symtab,
             .dhcp_opts = &dhcp_opts,
             .dhcpv6_opts = &dhcpv6_opts,
+            .pipeline = (!strcmp(sblf->pipeline, "ingress")
+                         ? OVNACT_P_INGRESS
+                         : OVNACT_P_EGRESS),
             .n_tables = 16,
             .cur_ltable = sblf->table_id,
         };
@@ -1162,8 +1175,7 @@ execute_output(const struct ovntrace_datapath *dp, struct flow *uflow,
     }
 
     uint16_t in_key = uflow->regs[MFF_LOG_INPORT - MFF_REG0];
-    const struct ovntrace_port *inport = ovntrace_port_find_by_key(dp, in_key);
-    const char *inport_name = !in_key ? "" : inport ? inport->name : "(unnamed)";
+    const char *inport_name = ovntrace_port_key_to_name(dp, in_key);
     uint32_t flags = uflow->regs[MFF_LOG_FLAGS - MFF_REG0];
     bool allow_loopback = (flags & MLF_ALLOW_LOOPBACK) != 0;
 
@@ -1364,6 +1376,23 @@ execute_put_dhcp_opts(const struct ovnact_put_dhcp_opts *pdo,
     mf_write_subfield_flow(&sf, &sv, uflow);
 }
 
+static void
+execute_next(const struct ovnact_next *next,
+             const struct ovntrace_datapath *dp, struct flow *uflow,
+             enum ovnact_pipeline pipeline, struct ovs_list *super)
+{
+    if (pipeline != next->pipeline) {
+        ovs_assert(next->pipeline == OVNACT_P_INGRESS);
+
+        uint16_t in_key = uflow->regs[MFF_LOG_INPORT - MFF_REG0];
+        struct ovntrace_node *node = ovntrace_node_append(
+            super, OVNTRACE_NODE_PIPELINE, "ingress(dp=\"%s\", inport=\"%s\")",
+            dp->name, ovntrace_port_key_to_name(dp, in_key));
+        super = &node->subs;
+    }
+    trace__(dp, uflow, next->ltable, next->pipeline, super);
+}
+
 static void
 trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
               const struct ovntrace_datapath *dp, struct flow *uflow,
@@ -1388,7 +1417,7 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
             break;
 
         case OVNACT_NEXT:
-            trace__(dp, uflow, ovnact_get_NEXT(a)->ltable, pipeline, super);
+            execute_next(ovnact_get_NEXT(a), dp, uflow, pipeline, super);
             break;
 
         case OVNACT_LOAD:

diff --git a/tests/ovn.at b/tests/ovn.at
@@ -653,12 +653,32 @@ next(15);
     encodes as resubmit(,31)
 
 next();
-    Syntax error at `)' expecting small integer.
+    Syntax error at `)' expecting "pipeline" or "table".
 next(10;
     Syntax error at `;' expecting `)'.
 next(16);
     "next" action cannot advance beyond table 15.
 
+next(table=11);
+    formats as next;
+    encodes as resubmit(,27)
+next(pipeline=ingress);
+    formats as next;
+    encodes as resubmit(,27)
+next(table=11, pipeline=ingress);
+    formats as next;
+    encodes as resubmit(,27)
+next(pipeline=ingress, table=11);
+    formats as next;
+    encodes as resubmit(,27)
+
+next(pipeline=egress);
+    "next" action cannot advance from ingress to egress pipeline (use "output" action instead)
+
+next(table=10);
+    formats as next(10);
+    encodes as resubmit(,26)
+
 # Loading a constant value.
 tcp.dst=80;
     formats as tcp.dst = 80;

diff --git a/tests/test-ovn.c b/tests/test-ovn.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2015, 2016, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1246,7 +1246,9 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED)
                 .ct_zones = &ct_zones,
                 .group_table = &group_table,
 
-                .first_ptable = 16,
+                .pipeline = OVNACT_P_INGRESS,
+                .ingress_ptable = 16,
+                .egress_ptable = 48,
                 .output_ptable = 64,
                 .mac_bind_ptable = 65,
             };