[ovs-dev] [flood-vlans 3/3] ofproto-dpif: Restore former NORMAL action behavior when revalidating.

Ethan Jackson ethan at nicira.com
Tue Jul 26 22:28:37 UTC 2011


Looks like a code cleanup anyways.

Ethan

On Fri, Jul 22, 2011 at 15:32, Ben Pfaff <blp at nicira.com> wrote:
> Before commit fa066f015 "bridge: Move packet processing functionality into
> ofproto," the code that called into what became xlate_normal() prevented
> a flow from being installed if it was called at revalidation time and
> the MAC learning table lacked an entry for the flow's destination MAC.
> That commit instead started dropping packets in that case (because it
> incorrectly ignored xlate_normal()'s return value).
>
> This restores the former behavior.  It's not clear that the former behavior
> is the best possible, but it is strictly better than starting to drop
> packets at revalidation time.
>
> Along with the previously fixed problem where flood_vlans were interpreted
> incorrectly, this bug broke controller connectivity when flood_vlans was
> set to any nonempty value that did not include the VLAN used for the
> controller connection (that is, when flood_vlans was interpreted as
> flooding the controller VLAN).
>
> Reported-by: David Tsai <dtsai at nicira.com>
> ---
>  ofproto/ofproto-dpif.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 2b1d5e4..9541750 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2784,7 +2784,7 @@ send_packet(struct ofproto_dpif *ofproto, uint32_t odp_port,
>
>  static void do_xlate_actions(const union ofp_action *in, size_t n_in,
>                              struct action_xlate_ctx *ctx);
> -static bool xlate_normal(struct action_xlate_ctx *);
> +static void xlate_normal(struct action_xlate_ctx *);
>
>  static void
>  commit_odp_actions(struct action_xlate_ctx *ctx)
> @@ -3766,10 +3766,7 @@ is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
>     return true;
>  }
>
> -/* If the composed actions may be applied to any packet in the given 'flow',
> - * returns true.  Otherwise, the actions should only be applied to 'packet', or
> - * not at all, if 'packet' was NULL. */
> -static bool
> +static void
>  xlate_normal(struct action_xlate_ctx *ctx)
>  {
>     struct ofbundle *in_bundle;
> @@ -3800,7 +3797,8 @@ xlate_normal(struct action_xlate_ctx *ctx)
>          * of time where we could learn from a packet reflected on a bond and
>          * blackhole packets before the learning table is updated to reflect
>          * the correct port. */
> -        return false;
> +        ctx->may_set_up_flow = false;
> +        return;
>     } else {
>         out_bundle = OFBUNDLE_FLOOD;
>     }
> @@ -3814,8 +3812,6 @@ done:
>     if (in_bundle) {
>         compose_actions(ctx, vlan, in_bundle, out_bundle);
>     }
> -
> -    return true;
>  }
>
>  static bool
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list