[PATCH] info to atm layer about broken connection
stf_xl at wp.pl
Mon Oct 31 07:33:20 EST 2005
> 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.
More information about the Usbatm