[PATCH] info to atm layer about broken connection

[Stanislaw Gruszka]

Hi 
> > simply after disconnect modem, ppp demon still works 
> > and ifconfig command show me connection is ok.
> > Using vcc->push(vcc, NULL) kill pppd with
> > Modem hangup message in logs.
> 
> that sounds like a bug in the pppoatm driver.  I will look into this.
I don't know, however it seems pushing NULL is ok.
I found in net/atm/pppoatm.c :

/* Called when an AAL5 PDU comes in */
static void pppoatm_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
{
	struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
	DPRINTK("pppoatm push\n");
	if (skb == NULL) {			/* VCC was closed */
		DPRINTK("removing ATMPPP VCC %p\n", pvcc);
		pppoatm_unassign_vcc(atmvcc);
		atmvcc->push(atmvcc, NULL);	/* Pass along bad news */
		return;
	}


and in net/atm/raw.c :

/*
 * SKB == NULL indicates that the link is being closed
 */

static void atm_push_raw(struct atm_vcc *vcc,struct sk_buff *skb)
{
	if (skb) {
		struct sock *sk = sk_atm(vcc);

		skb_queue_tail(&sk->sk_receive_queue, skb);
		sk->sk_data_ready(sk, skb->len);
	}
}

>
> > > 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
- you must reload atm module before you can establish new connection, 
even when old one was disable. 	
     
> 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.

> For example, the proc 
> filesystem entries /proc/net/atm/speed* can also continue to exist after 
> the speedtch module is unloaded, or the device is removed, because it
> would be much harder to do it differently.

Ok, but if we call read on unloaded procfs entries we get error 
(propably -ENOENT),  with internet connection there is no error,
only sleeping forever.

Summary:
 - Info to atm layer that link is broken when device is removed or module unloaded is necessary. 
 - Prevent subdriver module unloading when there is working connection is nice (lets root 
disable connection (kill pppd or atmarpd) before unload module).

__
Thanks
Staszek Gruszka