[PATCH] info to atm layer about broken connection

[Stanislaw Gruszka]

Hi Duncan
> Here's what Chas Williams (ATM maintainer told me):
> 
> "... I believe that all
> you should need to do is go through the list of vcc's that belong to your
> card and call vcc_release_async().  the next time the user space process
> that has control of the socket belonging to the vcc will get an error
> and close the socket.  the socket release() is responsible for sending
> the NULL skb to the next protocol layer."
> 
> So indeed a NULL skb should be sent, but not by the driver.  That's why
> I say it looks like a bug in the ATM layer.
> 
> > > > > Also, as far as I can see the use of try_module_get is pointless.  Why do
> > > > > you think it is needed?
> > > > It depends on approach, someone would like to finish connection when 
> > > > net driver module is unloaded, other one would like to prevent unloading 
> > > > module. I preffer second approach as is in (for example) pppoatm .
> > > >
> > > > Adding try_module_get and module_put pair in subdriver atm_start() and atm_stop() 
> > > > propably block unload module anyway.
> > > 
> > > It would, which is why I don't want it.  Can you please explain why it is
> > > useful to have the connection be gone before the module unload completes?
> > Remain connection when device is gone give some problems (at least on pppoatm): 
> > - you can read/write to not working interface and there is no indication 
> > for user the link is broken
> 
> This has nothing to do with module unloading.  You are explaining why you want
> the connection to be shut down when the device is removed.  I agree that the
> connection should be shutdown.  I guess I wasn't clear: my question was: why
> is it important that the connection be completely shutdown before the rmmod
> completes, rather than (maybe) soon afterwards.  This is the difference between
> doing a synchronous shutdown (in which you tell the ATM layer to shutdown, and
> you wait for the shutdown to complete before allowing the module unload to finish),
> and an asynchronous shutdown (in which you tell the ATM layer to shutdown, but
> don't wait for it to complete).
> > - you must reload atm module before you can establish new connection, 
> > even when old one was disable. 	
> 
> Same remark as above.
> 
> > > I don't really have anything against it, except that this kind of
> > > synchronous unloading can easily cause rmmod to hang indefinitely (for
> > > example: the output of rmmod is piped to a socket that goes through the
> > > ATM device; I don't know if this is really possible, just giving you
> > > the idea), so better to avoid it if possible.  
> > I hope higher layers properly handle this race condition possibility.
> 
> It's not a race condition, and in a sense there's nothing that can be done
> about it.  If A is waiting for B, but B is waiting for A, you get a deadlock,
> that's all.  The trick is to make sure this never happens, by making sure that
> A or B doesn't wait.  For example, if, on module unload, the driver code doesn't
> wait for anything, there is no problem.  That is the case now, and I'd rather
> not change it.  But it seems to me you don't really want synchronous shutdown
> anyway: you just want the connection to be killed.  
Yes, vcc should be closed asynchronously but vcc_release_async() don't work now, 
does it ? 

AFAIK try_module_get() in usbatm_atm_open() and module_put() in usbatm_atm_close() 
prevent unload module when there is atm connection, so there shouldn't be 
any deadlock with rmmod. 

My patch send NULL skb only when modem is physically disconnect form usb port. 
I think it is good to synchronously shutdown connection in such situation. 
I review push function in pppoatm, clip and br2684 drivers, all of them expect 
to get NULL skb and it is interpreted in the same manner - link is broken.

__
Regards
Staszek Gruszka