tty: closing n_gsm line discipline always times out

tty: closing n_gsm line discipline always times out

[Sascha Hauer]

Hi All,

When the n_gsm line discipline is closed it wants to shutdown the line
discipline properly and asks the link partner to end the mux protocol. This is
done in gsm_cleanup_mux() around line 2050 in n_gsm.c:

	if (dlci) {
		gc = gsm_control_send(gsm, CMD_CLD, NULL, 0);
		if (gc)
			gsm_control_wait(gsm, gc);
	}

The call stack is like this:

(struct tty_ldisc_ops).close
  -> gsmld_close
    -> gsmld_detach_gsm
      -> gsm_cleanup_mux

(struct tty_ldisc_ops).close is called at tty_release time or when the line
discipline is changed.  At tty_release time the tty is already shutdown, so we
cannot send anything anymore. In this case our link partner never closes the
mux protocol, which means we can never re-open it again.  When instead the line
discipline is changed back to N_TTY to close the protocol, gsm_control_send
works fine, but the tty_ldisc is locked and thus the chars received by the UART
never make it to the line discipline and gsm_control_wait times out.

There's obviously something wrong here. Any ideas how to fix that? When we are
not allowed to send/receive at tty_release time, do we need a new n_gsm specific
ioctl to shutdown the mux?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: tty: closing n_gsm line discipline always times out

[Alan Cox]

On Wed, 17 May 2017 15:44:56 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi All,
> 
> When the n_gsm line discipline is closed it wants to shutdown the line
> discipline properly and asks the link partner to end the mux protocol. This is
> done in gsm_cleanup_mux() around line 2050 in n_gsm.c:
> 
> 	if (dlci) {
> 		gc = gsm_control_send(gsm, CMD_CLD, NULL, 0);
> 		if (gc)
> 			gsm_control_wait(gsm, gc);
> 	}
> 
> The call stack is like this:
> 
> (struct tty_ldisc_ops).close
>   -> gsmld_close
>     -> gsmld_detach_gsm
>       -> gsm_cleanup_mux  
> 
> (struct tty_ldisc_ops).close is called at tty_release time or when the line
> discipline is changed.  At tty_release time the tty is already shutdown, so we
> cannot send anything anymore. In this case our link partner never closes the
> mux protocol, which means we can never re-open it again.  When instead the line
> discipline is changed back to N_TTY to close the protocol, gsm_control_send
> works fine, but the tty_ldisc is locked and thus the chars received by the UART
> never make it to the line discipline and gsm_control_wait times out.
> 
> There's obviously something wrong here. Any ideas how to fix that? When we are
> not allowed to send/receive at tty_release time, do we need a new n_gsm specific
> ioctl to shutdown the mux?

Probably - it used to work fine but other changes in the core tty code
broke it, and I think because 99% of the users of that code are Android
they've not yet caught up with this decade 8)

The other option would be to just remove the CLD sending and have the
caller do whatever cleanup handling it wants in N_TTY ldisc. You can just
write() the frame rather than having an ioctl.

That said there are power management cases where being able to turn it
on/off without ldisc changing might be useful. However you've then got to
deal with all the locking of active sessions against being in down state.

So I think just having the caller use a write() in N_TTY would be simpler
since we'd have to change user space anyway.

Alan

Re: tty: closing n_gsm line discipline always times out

[Sascha Hauer]

On Wed, May 17, 2017 at 04:23:58PM +0100, Alan Cox wrote:
> On Wed, 17 May 2017 15:44:56 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Hi All,
> > 
> > When the n_gsm line discipline is closed it wants to shutdown the line
> > discipline properly and asks the link partner to end the mux protocol. This is
> > done in gsm_cleanup_mux() around line 2050 in n_gsm.c:
> > 
> > 	if (dlci) {
> > 		gc = gsm_control_send(gsm, CMD_CLD, NULL, 0);
> > 		if (gc)
> > 			gsm_control_wait(gsm, gc);
> > 	}
> > 
> > The call stack is like this:
> > 
> > (struct tty_ldisc_ops).close
> >   -> gsmld_close
> >     -> gsmld_detach_gsm
> >       -> gsm_cleanup_mux  
> > 
> > (struct tty_ldisc_ops).close is called at tty_release time or when the line
> > discipline is changed.  At tty_release time the tty is already shutdown, so we
> > cannot send anything anymore. In this case our link partner never closes the
> > mux protocol, which means we can never re-open it again.  When instead the line
> > discipline is changed back to N_TTY to close the protocol, gsm_control_send
> > works fine, but the tty_ldisc is locked and thus the chars received by the UART
> > never make it to the line discipline and gsm_control_wait times out.
> > 
> > There's obviously something wrong here. Any ideas how to fix that? When we are
> > not allowed to send/receive at tty_release time, do we need a new n_gsm specific
> > ioctl to shutdown the mux?
> 
> Probably - it used to work fine but other changes in the core tty code
> broke it, and I think because 99% of the users of that code are Android
> they've not yet caught up with this decade 8)
> 
> The other option would be to just remove the CLD sending and have the
> caller do whatever cleanup handling it wants in N_TTY ldisc. You can just
> write() the frame rather than having an ioctl.
> 
> That said there are power management cases where being able to turn it
> on/off without ldisc changing might be useful. However you've then got to
> deal with all the locking of active sessions against being in down state.
> 
> So I think just having the caller use a write() in N_TTY would be simpler
> since we'd have to change user space anyway.

This option doesn't sound very nice because then userspace needs to know
details of the protocol that it just wants to use. Also the cleanup
won't be done if the userspace process is killed for some reason.

I think I'll try and see where I end up with an ioctl. If the result is
not acceptable then we can still opt for your suggestion to write() the
close frame in userspace.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: tty: closing n_gsm line discipline always times out

[Alan Cox]

> This option doesn't sound very nice because then userspace needs to know
> details of the protocol that it just wants to use. Also the cleanup
> won't be done if the userspace process is killed for some reason.

The same is basically true of an ioctl unfortunately.

Alan