possible bug on ppp_async

possible bug on ppp_async

[Gustavo Fernando Padovan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 2325 bytes --]

Hello,

I found out a problem on drivers/net/ppp_async.c: I'm testing a modem
(Motorola SM56 Data Fax Modem [1057:3052] that uses the slmodem
driver) and I noted that it stopped to work correctly with kernel
2.6.24 or greater. The modem connects once and at the second try it
becomes busy and I have to restart its driver to put it to work again.
I tested this only with this hardware, but I think this is a hardware
independent issue.

Theses changes make it stop to work:

http://lkml.org/lkml/2007/11/2/100
http://lkml.org/lkml/2007/11/2/131

The problem occurs when the execution enters on switch's default case
and call tty_mode_ioctl() and the cmd is not TCGETS or cmd is not
TCGETA. I don't know why this happens, but happens. On kernel 2.6.23
the switch's default case returns immediately with an error. But now
on kernel 2.6.24 and 2.6.25 switch's default case call
tty_mode_ioctl(). This is the unique real change between the two
versions, the others are only code restructuring. Then I made this
patch to revert part of changes and make modem works again. I don't
know if this is correct. I just put the modem to work.

--- a/drivers/net/ppp_async.c.     2008-06-09 13:12:25.000000000 -0300
+++ b/drivers/net/ppp_async.c      2008-06-09 13:15:57.000000000 -0300
@@ -309,6 +309,11 @@
                err = 0;
                break;

+       case TCGETS:
+       case TCGETA:
+               err = tty_mode_ioctl(tty, file, cmd, arg);
+               break;
+
        case TCFLSH:
                /* flush our buffers and the serial port's buffer */
                if (arg = TCIOFLUSH || arg = TCOFLUSH)
@@ -324,8 +329,7 @@
                break;

        default:
-               /* Try the various mode ioctls */
-               err = tty_mode_ioctl(tty, file, cmd, arg);
+               err = -ENOIOCTLCMD;
        }

        ap_put(ap);



-------------------------------------------
Gustavo Fernando Padovan
Engenharia de Computação 2006
LAS - Laboratório de Administração e Segurança de Sistemas
Instituto de Computação - UNICAMP

gfpadovan@gmail.com
ra061316@students.ic.unicamp.br
pao@jabber.org
-------------------------------------------

Seja Livre, use Software Livre
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þšiþ)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢jÿ¾\a«þG«éÿ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿþø\x1e¯ù\x1e®w¥þŠàþf£¢·hšâúÿ†Ù¥

Re: possible bug on ppp_async

[Charlie Brady]

On Mon, 9 Jun 2008, Gustavo Fernando Padovan wrote:

> I found out a problem on drivers/net/ppp_async.c: I'm testing a modem
> (Motorola SM56 Data Fax Modem [1057:3052] that uses the slmodem
> driver) and I noted that it stopped to work correctly with kernel
> 2.6.24 or greater. The modem connects once and at the second try it
> becomes busy and I have to restart its driver to put it to work again.

Why do you not suspect the slmodem driver?

Re: possible bug on ppp_async

[Gustavo Fernando Padovan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1190 bytes --]

On Mon, Jun 9, 2008 at 3:52 PM, Charlie Brady
<charlieb@budge.apana.org.au> wrote:
>
> On Mon, 9 Jun 2008, Gustavo Fernando Padovan wrote:
>
>> I found out a problem on drivers/net/ppp_async.c: I'm testing a modem
>> (Motorola SM56 Data Fax Modem [1057:3052] that uses the slmodem
>> driver) and I noted that it stopped to work correctly with kernel
>> 2.6.24 or greater. The modem connects once and at the second try it
>> becomes busy and I have to restart its driver to put it to work again.
>
> Why do you not suspect the slmodem driver?
Because the slmodem driver is the same. It didn't change. And I think
that ppp don't need to know what is the modem driver, it only knows it
have a modem. I think.

>



-- 
----------------------------------------------
Gustavo Fernando Padovan
Engenharia de Computação 2006
LAS - Laboratório de Administração e Segurança de Sistemas
Instituto de Computação - UNICAMP

gfpadovan@gmail.com
ra061316@students.ic.unicamp.br
pao@jabber.org
-------------------------------------------

Seja Livre, use Software Livre
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þšiþ)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢jÿ¾\a«þG«éÿ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿþø\x1e¯ù\x1e®w¥þŠàþf£¢·hšâúÿ†Ù¥

Re: possible bug on ppp_async

[Alan Cox]

On Mon, Jun 09, 2008 at 03:23:33PM -0300, Gustavo Fernando Padovan wrote:
> The problem occurs when the execution enters on switch's default case
> and call tty_mode_ioctl() and the cmd is not TCGETS or cmd is not
> TCGETA. I don't know why this happens, but happens. On kernel 2.6.23

Which ioctl is making it go silly ?

> tty_mode_ioctl(). This is the unique real change between the two
> versions, the others are only code restructuring. Then I made this
> patch to revert part of changes and make modem works again. I don't
> know if this is correct. I just put the modem to work.

It's very useful as we now know what change has had the effect, but not
what your pppd is doing which is breaking it (and thus where the real bug
may lie)

> --- a/drivers/net/ppp_async.c.     2008-06-09 13:12:25.000000000 -0300
> +++ b/drivers/net/ppp_async.c      2008-06-09 13:15:57.000000000 -0300
> @@ -309,6 +309,11 @@
>                 err = 0;
>                 break;
> 
> +       case TCGETS:
> +       case TCGETA:
> +               err = tty_mode_ioctl(tty, file, cmd, arg);
> +               break;
> +
>         case TCFLSH:
>                 /* flush our buffers and the serial port's buffer */
>                 if (arg = TCIOFLUSH || arg = TCOFLUSH)
> @@ -324,8 +329,7 @@
>                 break;
> 
>         default:
> -               /* Try the various mode ioctls */
> -               err = tty_mode_ioctl(tty, file, cmd, arg);
> +               err = -ENOIOCTLCMD;

Can you add a printk to log which ioctl calls end up here and what they
return (both with/without the patch). That should give the needed info to
identify what is being done and returned which then breaks it.

From that I can work out what is going on and what the right fix may be.

Alan

Re: possible bug on ppp_async

[Gustavo Fernando Padovan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 3595 bytes --]

On Mon, Jun 9, 2008 at 6:03 PM, Alan Cox <alan@redhat.com> wrote:
> On Mon, Jun 09, 2008 at 03:23:33PM -0300, Gustavo Fernando Padovan wrote:
>> The problem occurs when the execution enters on switch's default case
>> and call tty_mode_ioctl() and the cmd is not TCGETS or cmd is not
>> TCGETA. I don't know why this happens, but happens. On kernel 2.6.23
>
> Which ioctl is making it go silly ?
>
>> tty_mode_ioctl(). This is the unique real change between the two
>> versions, the others are only code restructuring. Then I made this
>> patch to revert part of changes and make modem works again. I don't
>> know if this is correct. I just put the modem to work.
>
> It's very useful as we now know what change has had the effect, but not
> what your pppd is doing which is breaking it (and thus where the real bug
> may lie)
>
>> --- a/drivers/net/ppp_async.c.     2008-06-09 13:12:25.000000000 -0300
>> +++ b/drivers/net/ppp_async.c      2008-06-09 13:15:57.000000000 -0300
>> @@ -309,6 +309,11 @@
>>                 err = 0;
>>                 break;
>>
>> +       case TCGETS:
>> +       case TCGETA:
>> +               err = tty_mode_ioctl(tty, file, cmd, arg);
>> +               break;
>> +
>>         case TCFLSH:
>>                 /* flush our buffers and the serial port's buffer */
>>                 if (arg = TCIOFLUSH || arg = TCOFLUSH)
>> @@ -324,8 +329,7 @@
>>                 break;
>>
>>         default:
>> -               /* Try the various mode ioctls */
>> -               err = tty_mode_ioctl(tty, file, cmd, arg);
>> +               err = -ENOIOCTLCMD;
>
> Can you add a printk to log which ioctl calls end up here and what they
> return (both with/without the patch). That should give the needed info to
> identify what is being done and returned which then breaks it.

I made two tries to connect with and without the patch. The tries are
separated by a blank line.

Debug without the patch:
 [ 5955.465357] cmd = 21506 (TCSETS)
 [ 5955.465369] err = 0
 [ 5955.465372] ----------

 [ 5987.058871] TCGETS
 [ 5987.059094] err = 0
 [ 5987.059276] ----------
 [ 5987.059645] TCGETS
 [ 5987.059786] err = 0
 [ 5987.059863] ----------
 [ 5987.060008] cmd = 21534 (TIOCGSERIAL)
 [ 5987.060117] err = -515     (-ENOIOCTLCML)
 [ 5987.060194] ----------
 [ 5987.060283] cmd = 21506
 [ 5987.060371] err = 0
 [ 5987.060444] ----------
 [ 5987.110619] cmd = 21506
 [ 5987.110760] err = 0
 [ 5987.110836] ----------
 [ 5987.110949] cmd = 21506
 [ 5987.111037] err = 0
 [ 5987.111109] ----------
 [ 5987.111192] TCGETS
 [ 5987.111272] err = 0
 [ 5987.111345] ----------
 [ 6000.334483] cmd = 21506
 [ 6000.334494] err = 0
 [ 6000.334496] ----------

Debug with the patch:
[ 8207.865204] cmd = 21506 (TCSETS)
[ 8207.865213] err = -515
[ 8207.865215] ----------

[ 8866.939075] cmd = 21506
[ 8866.939084] err = -515
[ 8866.939086] ----------

The problem seems the ioctl TCSETS, with the patch (and on kernel
2.6.23) it returns immediately, and on kernel 2.6.24 (without the
patch) it goes to tty_mode_ioctl().

>
> From that I can work out what is going on and what the right fix may be.
>
> Alan
>
>



-- 
----------------------------------------------
Gustavo Fernando Padovan
Engenharia de Computação 2006
LAS - Laboratório de Administração e Segurança de Sistemas
Instituto de Computação - UNICAMP

gfpadovan@gmail.com
ra061316@students.ic.unicamp.br
pao@jabber.org
-------------------------------------------

Seja Livre, use Software Livre
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þšiþ)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢jÿ¾\a«þG«éÿ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿþø\x1e¯ù\x1e®w¥þŠàþf£¢·hšâúÿ†Ù¥

Re: possible bug on ppp_async

[Gustavo Fernando Padovan]

On Wed, Jun 11, 2008 at 5:55 PM, Gustavo Fernando Padovan
<gfpadovan@gmail.com> wrote:
> On Mon, Jun 9, 2008 at 6:03 PM, Alan Cox <alan@redhat.com> wrote:
>> On Mon, Jun 09, 2008 at 03:23:33PM -0300, Gustavo Fernando Padovan wrote:
>>> The problem occurs when the execution enters on switch's default case
>>> and call tty_mode_ioctl() and the cmd is not TCGETS or cmd is not
>>> TCGETA. I don't know why this happens, but happens. On kernel 2.6.23
>>
>> Which ioctl is making it go silly ?
>>
>>> tty_mode_ioctl(). This is the unique real change between the two
>>> versions, the others are only code restructuring. Then I made this
>>> patch to revert part of changes and make modem works again. I don't
>>> know if this is correct. I just put the modem to work.
>>
>> It's very useful as we now know what change has had the effect, but not
>> what your pppd is doing which is breaking it (and thus where the real bug
>> may lie)
>>
>>> --- a/drivers/net/ppp_async.c.     2008-06-09 13:12:25.000000000 -0300
>>> +++ b/drivers/net/ppp_async.c      2008-06-09 13:15:57.000000000 -0300
>>> @@ -309,6 +309,11 @@
>>>                 err = 0;
>>>                 break;
>>>
>>> +       case TCGETS:
>>> +       case TCGETA:
>>> +               err = tty_mode_ioctl(tty, file, cmd, arg);
>>> +               break;
>>> +
>>>         case TCFLSH:
>>>                 /* flush our buffers and the serial port's buffer */
>>>                 if (arg = TCIOFLUSH || arg = TCOFLUSH)
>>> @@ -324,8 +329,7 @@
>>>                 break;
>>>
>>>         default:
>>> -               /* Try the various mode ioctls */
>>> -               err = tty_mode_ioctl(tty, file, cmd, arg);
>>> +               err = -ENOIOCTLCMD;
>>
>> Can you add a printk to log which ioctl calls end up here and what they
>> return (both with/without the patch). That should give the needed info to
>> identify what is being done and returned which then breaks it.
>
> I made two tries to connect with and without the patch. The tries are
> separated by a blank line.
>
> Debug without the patch:
>  [ 5955.465357] cmd = 21506 (TCSETS)
>  [ 5955.465369] err = 0
>  [ 5955.465372] ----------
>
>  [ 5987.058871] TCGETS
>  [ 5987.059094] err = 0
>  [ 5987.059276] ----------
>  [ 5987.059645] TCGETS
>  [ 5987.059786] err = 0
>  [ 5987.059863] ----------
>  [ 5987.060008] cmd = 21534 (TIOCGSERIAL)
>  [ 5987.060117] err = -515     (-ENOIOCTLCML)
>  [ 5987.060194] ----------
>  [ 5987.060283] cmd = 21506
>  [ 5987.060371] err = 0
>  [ 5987.060444] ----------
>  [ 5987.110619] cmd = 21506
>  [ 5987.110760] err = 0
>  [ 5987.110836] ----------
>  [ 5987.110949] cmd = 21506
>  [ 5987.111037] err = 0
>  [ 5987.111109] ----------
>  [ 5987.111192] TCGETS
>  [ 5987.111272] err = 0
>  [ 5987.111345] ----------
>  [ 6000.334483] cmd = 21506
>  [ 6000.334494] err = 0
>  [ 6000.334496] ----------
>
> Debug with the patch:
> [ 8207.865204] cmd = 21506 (TCSETS)
> [ 8207.865213] err = -515
> [ 8207.865215] ----------
>
> [ 8866.939075] cmd = 21506
> [ 8866.939084] err = -515
> [ 8866.939086] ----------
>
> The problem seems the ioctl TCSETS, with the patch (and on kernel
> 2.6.23) it returns immediately, and on kernel 2.6.24 (without the
> patch) it goes to tty_mode_ioctl().

This patch also resolves the 'bug'. It intercepts TCSETS on
switch(cmd) and return -ENOIOCTLCMD;


--- a/drivers/net/ppp_async.c        2008-06-13 11:59:10.000000000 -0300
+++ b/drivers/net/ppp_async.c     2008-06-13 11:58:34.000000000 -0300
@@ -323,6 +323,10 @@
                err = 0;
                break;

+    case TCSETS:
+        err = -ENOIOCTLCMD;
+        break;
+
        default:
                /* Try the various mode ioctls */
                err = tty_mode_ioctl(tty, file, cmd, arg);


>
>>
>> From that I can work out what is going on and what the right fix may be.
>>
>> Alan
>>
>>
>
>
>
> --
> ----------------------------------------------
> Gustavo Fernando Padovan
> Engenharia de Computação 2006
> LAS - Laboratório de Administração e Segurança de Sistemas
> Instituto de Computação - UNICAMP
>
> gfpadovan@gmail.com
> ra061316@students.ic.unicamp.br
> pao@jabber.org
> -------------------------------------------
>
> Seja Livre, use Software Livre
>



-- 
----------------------------------------------
Gustavo Fernando Padovan
Engenharia de Computação 2006
LAS - Laboratório de Administração e Segurança de Sistemas
Instituto de Computação - UNICAMP

gfpadovan@gmail.com
ra061316@students.ic.unicamp.br
pao@jabber.org
-------------------------------------------

Seja Livre, use Software Livre

Re: possible bug on ppp_async

[Charlie Brady]

On Fri, 13 Jun 2008, Gustavo Fernando Padovan wrote:

>> The problem seems the ioctl TCSETS, with the patch (and on kernel
>> 2.6.23) it returns immediately, and on kernel 2.6.24 (without the
>> patch) it goes to tty_mode_ioctl().
>
> This patch also resolves the 'bug'. It intercepts TCSETS on
> switch(cmd) and return -ENOIOCTLCMD;
>
>
> --- a/drivers/net/ppp_async.c        2008-06-13 11:59:10.000000000 -0300
> +++ b/drivers/net/ppp_async.c     2008-06-13 11:58:34.000000000 -0300

No, that patch doesn't resolve the bug. That just avoids the bug. The bug 
is that the driver "goes silly" when passed a TCSETS cmd, and you need to 
reload the driver to make it sane again.

Aren't you curious as to why TCSETS calls are made, and why things go 
wrong when they are?

Re: possible bug on ppp_async

[Alan Cox]

On Fri, Jun 13, 2008 at 03:01:52PM -0300, Gustavo Fernando Padovan wrote:
> > The problem seems the ioctl TCSETS, with the patch (and on kernel
> > 2.6.23) it returns immediately, and on kernel 2.6.24 (without the
> > patch) it goes to tty_mode_ioctl().

That is a fix for a long standing bug. It makes TCSETS/TCGETS work correctly
on a ppp session.  So the bug is in your user space code somewhere it appears
and just happens to be triggered by fixing other bugs.

Re: possible bug on ppp_async

[Gustavo Fernando Padovan]

On Fri, Jun 13, 2008 at 4:38 PM, Alan Cox <alan@redhat.com> wrote:
> On Fri, Jun 13, 2008 at 03:01:52PM -0300, Gustavo Fernando Padovan wrote:
>> > The problem seems the ioctl TCSETS, with the patch (and on kernel
>> > 2.6.23) it returns immediately, and on kernel 2.6.24 (without the
>> > patch) it goes to tty_mode_ioctl().
>
> That is a fix for a long standing bug. It makes TCSETS/TCGETS work correctly
> on a ppp session.  So the bug is in your user space code somewhere it appears
> and just happens to be triggered by fixing other bugs.
I tested with other dialer and all works fine now. The bug was in the
dialer wvdial.
Sorry for inconvenience.
>
>



-- 
----------------------------------------------
Gustavo Fernando Padovan
Engenharia de Computação 2006
LAS - Laboratório de Administração e Segurança de Sistemas
Instituto de Computação - UNICAMP

gfpadovan@gmail.com
ra061316@students.ic.unicamp.br
pao@jabber.org
-------------------------------------------

Seja Livre, use Software Livre

Re: possible bug on ppp_async

[Alan Cox]

On Mon, Jun 16, 2008 at 09:54:01AM -0300, Gustavo Fernando Padovan wrote:
> I tested with other dialer and all works fine now. The bug was in the
> dialer wvdial.
> Sorry for inconvenience.

No problem - better to test and find things out. Thanks for letting me know
you found the cause