Re: [PATCH] patches: workaround for br_del_if race

[Ryan Harper <ryanh@us.ibm.com>]

* Keir Fraser <Keir.Fraser@cl.cam.ac.uk> [2005-08-19 07:37]:
> 
> On 18 Aug 2005, at 22:01, Ryan Harper wrote:
> 
> >This patch provides a workaround for bugzilla #90 which shows up far 
> >too
> >often when creating and then destroying lots of domUs and dom0 is SMP.
> >Details are in the [1]bug.  With this patch, I now can create/destroy
> >domains in a tight loop for hours where previously every 3 to 10 cycles
> >would blow up.
> 
> Please also submit this to the Ethernet bridge maintainer. Details are 
> in the Linux MAINTAINERS file.

I've had a little discussion on [1]netdev about this but since this
isn't the proper fix I'm doing some more digging.

The real race is between when a call_rcu() callback runs and when the
netif_destroy() calls unregister_netdev().  

When we get an oops, the brctl delif IOCTL has run, and br_del_if() has
been called setting the port state to DISABLED, and then queues up an
rcu callback, destroy_npb(),  which will set dev->br_port = NULL.  

add_del_if()                       // IOCTL handler from brctl delif 
                                   // xen-br0 $VIF
  br_del_if()                      
    del_nbp()
      br_stp_disable_port()        // Set port->state = BR_STATE_DISABLED
      call_rcu(destroy_nbp_rcu)    // setup rcu callback to run 
                                   // destory_nbp() which will set
                                   // dev->br_port = NULL
      
After the xen scripts have called the brctl command, xend sends the
disconnect and destroy control messages, which trigger:

netif_destroy()
   unregister_netdev()
      unregister_netdevice()
         notify_call_chain(NETDEV_UNREGISTER)
            br_device_event()    // The first thing done here is to check 
                                 // the device's br_port to see if it is
                                 // NULL

If the destory_nbp_rcu() callback isn't fired before br_device_event()
checks dev->br_port, then the NULL check fails and a second call to
br_del_if() is invoked and we blow up in sysfs/kobject BUG_ON() for ref
counts of dentrys. [2]

Before I go back to netdev, I wanted to check if there is anything we
should be doing to be more defensive or does this seem to be something
the bridge code should handle (error out, whatever)?

1. http://oss.sgi.com/archives/netdev/2005-08/msg00097.html
2. Routines are in linux/net/bridge/{br_if.c, br_ioctl.c, br_notify.c}

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253   T/L: 678-9253
ryanh@us.ibm.com