Thanks.

I'm going to spend a little more time tomorrow trying to reproduce this
and figure out the root cause.  If I can't do that, I'll go with what's
here.

I was debugging this on branch-1.11 (that's where my bug report is) but
Justin pointed out that it's not just there, it also affects other
branches.  I'll apply it to all affected branches.

On Wed, Aug 14, 2013 at 09:00:01AM +0800, Ethan Jackson wrote:
> Acked-by: Ethan Jackson <et...@nicira.com>
> 
> 
> On Wed, Aug 14, 2013 at 8:44 AM, Ben Pfaff <b...@nicira.com> wrote:
> > When the ofp_port argument to bundle_add_port() refers to an ofport_dpif
> > that already belongs to some other bundle, bundle_add_port() removed
> > the port from the other bundle, correctly, with bundle_del_port().
> > If the other bundle now contained no ports, however, this violated the
> > invariant that a bundle always contains at least one port.
> >
> > Normally, this would get fixed up when the other bundle was processed
> > later during reconfiguration.  I haven't quite zeroed in on the exact
> > case where this is not true, but segfaults have happened here in
> > production, in particular when port adds and deletes happen simultaneously
> > and the new port reuses the OpenFlow port number of one of the deleted
> > ports.  It seems that the duplicate port number allows some port to rip
> > away the new port from its bundle without destroying that bundle.  I
> > suspect, therefore, that there is still a more subtle bug here, but I
> > hope that this will fix the segfault.
> >
> > Bug #18967.
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> >  ofproto/ofproto-dpif.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index f3f87f0..20a180e 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -2525,7 +2525,7 @@ bundle_add_port(struct ofbundle *bundle, uint32_t 
> > ofp_port,
> >      if (port->bundle != bundle) {
> >          bundle->ofproto->backer->need_revalidate = REV_RECONFIGURE;
> >          if (port->bundle) {
> > -            bundle_del_port(port);
> > +            bundle_remove(&port->up);
> >          }
> >
> >          port->bundle = bundle;
> > --
> > 1.7.10.4
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to