cvs commit: src/sys/dev/an if_an.c src/sys/dev/arl if_arl_isa.c src/sys/dev/awi if_awi_pccard.c src/sys/dev/cm if_cm_isa.c src/sys/dev/cnw if_cnw.c src/sys/dev/cp if_cp.c src/sys/dev/cs if_cs.c src/sys/dev/ed if_ed.c src/sys/dev/em if_em.c ...

[M. Warner Losh]

In message: <20050921193410.GC24516 at ip.net.ua>
            Ruslan Ermilov <ru at freebsd.org> writes:
: On Wed, Sep 21, 2005 at 01:23:22PM -0600, M. Warner Losh wrote:
: > In message: <20050921191906.GA24516 at ip.net.ua>
: >             Ruslan Ermilov <ru at FreeBSD.org> writes:
: > : On Wed, Sep 21, 2005 at 01:02:41PM -0600, M. Warner Losh wrote:
: > : > In message: <200509211455.59154.jhb at FreeBSD.org>
: > : >             John Baldwin <jhb at FreeBSD.org> writes:
: > : > : > 5.  Fix all drivers to set some flag in foo_detach() and foo_shutdown()
: > : > : >     and refuse to work in foo_ioctl() if it's set.  This should fix
: > : > : >     panics when BPF listener is attached while interface goes away or
: > : > : >     module is unloaded.
: > : > 
: > : > : I'd rather 5) be simpler in 
: > : > : that it only check in the flags case to not force the driver lock to be 
: > : > : acquired for all the ioctls that the driver doesn't actually handle.  
: > : > : Actually, I think I'd really prefer that we think about how to fix the BPF 
: > : > : issue in BPF itself if possible.  It may be that we don't need to set the 
: > : > : flags (i.e. skip the actual ioctl) if the interface is in the process of 
: > : > : detaching and we can make that change centrally without having to scatter 
: > : > : gone flags in all the drivers.
: > : > 
: > : > This would solve the race at hand.  However, it wouldn't solve the
: > : > problems with driver shutdown racing with other things in the system
: > : > (like ifconfig during detach).
: > : > 
: > : > I've knocked around the idea of creating a if_dead() function that one
: > : > would call before foo_stop().  if_dead would just return (possibly an
: > : > error) for all the entry points before the device is actually
: > : > detached.  This would mean that we don't need to add flags to all the
: > : > drivers, but we do need to change all the detach routine.  This would
: > : > avoid the LOCK operations that you are worried about...
: > : > 
: > : I'm afraid you cannot safely change the ifnet method pointers, if
: > : you mean that foo_detach() should set ifp->if_start etc. to point
: > : to if_dead().
: > 
: > Why not?  What's the problem?
: > 
: The time you change say ifp->if_start in foo_detach(), stack may call
: it and get a half-changed pointer.  Unless we invent locking shared
: between stack and the drivers.

pointers can be changed atomically on all our platforms.  The only
race is that there's a number of pointers that you need to change and
you can't change all of them at the same time.  ifp->if_start will
either point to the old driver start routine, or it will point to the
new routine.  It won't be 1/2 and 1/2.

Warner