[ovs-dev] [PATCH] ofproto: Use original in_port for executing NXAST_RESUBMIT actions.

Justin Pettit jpettit at nicira.com
Thu Apr 15 00:00:22 UTC 2010


Looks good.

--Justin


On Apr 14, 2010, at 10:50 AM, Ben Pfaff wrote:

> 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.
> ---
> include/openflow/nicira-ext.h |    8 +++++---
> ofproto/ofproto.c             |    9 +++++++--
> 2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index 535cfc3..17d86a8 100644
> --- 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
> --- 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;
>     }
> }
> 
> -- 
> 1.6.6.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list