On Tue, Jan 03, 2012 at 01:45:42PM -0800, Justin Pettit wrote:
> Thanks for tracking this down so quickly. Looks good to me. Does
> the problem exist in later branches?
No. On later branches, port configuration is "atomic": we don't do
part of it (leaving the port in an invalid state), then go off and do
some other configuration, and then come back and finish it up.
> Did you want to add Dominic to AUTHORS? I suspect you, like me,
> assumed he was already there.
That's a good idea but I think I'll do it on master instead. Not much
point on an essentially dead branch.
I pushed this to vlan-maint.
> On Jan 3, 2012, at 1:23 PM, Ben Pfaff wrote:
>
> > If a bridge is in standalone mode and no controllers are configured, and
> > then reconfiguration (e.g. ovs-vsctl) adds an SLB or TCP-balanced bond,
> > and at the moment of reconfiguration active kernel flows exist that output
> > to the newly added bond, then ovs-vswitchd would segfault because the
> > bond's 'bond_hash' member is not yet initialized when flows are
> > retranslated as part of adding the flow that ensures normal standalone
> > switch behavior.
> >
> > This commit fixes the problem by adding the flow later, after bonds have
> > been fully initialized.
> >
> > NIC-443.
> > Reported-by: Dominic Curran <dominic.cur...@citrix.com>
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > Not yet tested.
> >
> > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> > index f1adf4e..fb2ad83 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -1,4 +1,4 @@
> > -/* Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks
> > +/* Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -268,6 +268,7 @@ static void bridge_reconfigure_one(struct bridge *);
> > static void bridge_reconfigure_remotes(struct bridge *,
> > const struct sockaddr_in *managers,
> > size_t n_managers);
> > +static void bridge_reconfigure_remotes_late(struct bridge *);
> > static void bridge_get_all_ifaces(const struct bridge *, struct shash
> > *ifaces);
> > static void bridge_fetch_dp_ifaces(struct bridge *);
> > static void bridge_flush(struct bridge *);
> > @@ -930,6 +931,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
> > *ovs_cfg)
> > HMAP_FOR_EACH (iface, dp_ifidx_node, &br->ifaces) {
> > iface_update_cfm(iface);
> > }
> > + bridge_reconfigure_remotes_late(br);
> > }
> >
> > free(managers);
> > @@ -2109,13 +2111,23 @@ bridge_reconfigure_remotes(struct bridge *br,
> > if (had_primary != ofproto_has_primary_controller(br->ofproto)) {
> > ofproto_flush_flows(br->ofproto);
> > }
> > +}
> >
> > +/* Does configuration of remotes that must happen after all of the ports
> > and
> > + * interfaces are fully configured, that is, when flow translation can be
> > + * expected to succeed. (This is because ofproto_add_flow() immediately
> > + * re-translates any existing facets for the rule that it replaces, if
> > any.)
> > + * In particular, it must be called after port_update_bonding(), to ensure
> > that
> > + * 'bond_hash' is non-NULL for bonded ports. */
> > +static void
> > +bridge_reconfigure_remotes_late(struct bridge *br)
> > +{
> > /* If there are no controllers and the bridge is in standalone
> > * mode, set up a flow that matches every packet and directs
> > * them to OFPP_NORMAL (which goes to us). Otherwise, the
> > * switch is in secure mode and we won't pass any traffic until
> > * a controller has been defined and it tells us to do so. */
> > - if (!n_controllers
> > + if (!bridge_get_controllers(br, NULL)
> > && ofproto_get_fail_mode(br->ofproto) == OFPROTO_FAIL_STANDALONE) {
> > union ofp_action action;
> > struct cls_rule rule;
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
