* Stephen Hemminger <shemminger@xxxxxxxx> [2005-08-18 17:36]:
> On Thu, 18 Aug 2005 17:23:23 -0500
> Ryan Harper <ryanh@xxxxxxxxxx> wrote:
>
> > * Stephen Hemminger <shemminger@xxxxxxxx> [2005-08-18 17:11]:
> > > On Thu, 18 Aug 2005 16:40:36 -0500
> > > Ryan Harper <ryanh@xxxxxxxxxx> wrote:
> > >
> > > > Hello,
> > > >
> > > > I've encountered several oops when adding and removing interfaces from
> > > > bridges while using Xen. Most of the details are available [1]here.
> > > > The short of it is the following sequence:
> > >
> > > Doesn't the mutex in RTNL work right? or are you calling
> > > routines with out asserting it?
> >
> > unregister_netdevice asserts RTNL, add_del_if() in br_ioctl.c doesn't
> > seem to do so. I don't see it down dev_get_by_index() path either. It
> > looks like any caller of add_del_if() isn't asserting RTNL. The two
> > callers I see are:
> >
> > br_dev_ioctl() in br_ioctl.c
> > old_dev_ioctl() in br_ioctl.c
>
> But the pat to br_dev_ioctl() is via the socket ioctl and that
> should already have gotten RTNL.
>
>
> dev_ioctl
> rtnl_lock()
> dev_ifsioc()
> dev->do_ioctl --> br_dev_ioctl
Hrm. OK. It sounds like both paths are doing the right thing w.r.t
asserting RTNL, but br_device_event() still gets called with:
1) dev->br_port != NULL
2) dev->br_port->state = BR_STATE_DISABLED
3) event = NETDEV_UNREGISTER
which results in br_del_if() being called a second time on the same
port.
Some of the other cases (NETDEV_FEAT_CHANGE, NETDEV_CHANGE) do a state
check before calling a subsequent function. Does it make sense for
br_del_if() to be called on a port whose state is BR_STATE_DISABLED?
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253 T/L: 678-9253
ryanh@xxxxxxxxxx
|