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 <[email protected]> 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 <[email protected]> > > --- > > ?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 > > [email protected] > > http://openvswitch.org/mailman/listinfo/dev > > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
