ofproto: Use original in_port for executing NXAST_RESUBMIT actions.

If NXAST_RESUBMIT adopts the replacement in_port for executing actions,
then OFPP_NORMAL will believe that traffic originated from whatever port
that is.  This seems unlikely to ever be useful and in fact breaks
applications that use NXAST_RESUBMIT for two-stage ACLs.

Bug #2644.

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 535cfc3..17d86a8 100644 (file)
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -61,14 +61,16 @@ enum nx_action_subtype {
     /* Searches the flow table again, using a flow that is slightly modified
      * from the original lookup:
      *
-     *    - The flow's in_port is changed to that specified in the 'in_port'
-     *      member of struct nx_action_resubmit.
+     *    - The 'in_port' member of struct nx_action_resubmit is used as the
+     *      flow's in_port.
      *
      *    - 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.
      *
-     * If the modified flow matches in the flow table, then the corresponding
+     * Following the lookup, the original in_port is restored.
+     *
+     * If the modified flow matched in the flow table, then the corresponding
      * actions are executed, except that NXAST_RESUBMIT actions found in the
      * secondary set of actions are ignored.  Afterward, actions following
      * NXAST_RESUBMIT in the original set of actions, if any, are executed; any

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 83dc99d..3b5cf48 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2062,11 +2062,17 @@ static void
 xlate_table_action(struct action_xlate_ctx *ctx, uint16_t in_port)
 {
     if (!ctx->recurse) {
-        uint16_t old_in_port = ctx->flow.in_port;
+        uint16_t old_in_port;
         struct rule *rule;
 
+        /* 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 = lookup_valid_rule(ctx->ofproto, &ctx->flow);
+        ctx->flow.in_port = old_in_port;
+
         if (rule) {
             if (rule->super) {
                 rule = rule->super;
@@ -2076,7 +2082,6 @@ xlate_table_action(struct action_xlate_ctx *ctx, uint16_t in_port)
             do_xlate_actions(rule->actions, rule->n_actions, ctx);
             ctx->recurse--;
         }
-        ctx->flow.in_port = old_in_port;
     }
 }