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

[Ben Pfaff]

Thanks for the reviews of this series.  I will push this soon.

On Tue, Jul 26, 2011 at 03:28:37PM -0700, Ethan Jackson wrote:
> 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
> >