[PATCH] [2.5] adding ppp xon/xoff support

[PATCH] [2.5] adding ppp xon/xoff support

[Samuel Thibault]

Hi,

This patch wasn't applied, probably because the misplaced else
statement, here is a corrected version. This would at last make linux
support xon/xoff for ppp connections (it hasn't been available since 2.0
at least).

Please apply,

Regards,
Samuel Thibault

diff -ur linux-2.5.71-orig/drivers/net/ppp_async.c linux-2.5.71-perso/drivers/net/ppp_async.c
--- linux-2.5.71-orig/drivers/net/ppp_async.c	2003-06-14 17:13:03.000000000 -0400
+++ linux-2.5.71-perso/drivers/net/ppp_async.c	2003-07-06 13:51:23.000000000 -0400
@@ -893,6 +893,24 @@
 			process_input_packet(ap);
 		} else if (c == PPP_ESCAPE) {
 			ap->state |= SC_ESCAPE;
+		} else if (I_IXON(ap->tty)) {
+			if (c == STOP_CHAR(ap->tty)) {
+				if (!ap->tty->stopped) {
+					ap->tty->stopped = 1;
+					if (ap->tty->driver->stop)
+						(ap->tty->driver->stop)(ap->tty);
+				}
+			} else if (c == START_CHAR(ap->tty)) {
+				if (ap->tty->stopped) {
+					ap->tty->stopped = 0;
+					if (ap->tty->driver->start)
+						(ap->tty->driver->start)(ap->tty);
+					if ((test_bit(TTY_DO_WRITE_WAKEUP, &ap->tty->flags)) &&
+					    ap->tty->ldisc.write_wakeup)
+						(ap->tty->ldisc.write_wakeup)(ap->tty);
+					wake_up_interruptible(&ap->tty->write_wait);
+				}
+			}
 		}
 		/* otherwise it's a char in the recv ACCM */
 		++n;

Re: [PATCH] [2.5] adding ppp xon/xoff support

[Paul Mackerras]

Samuel Thibault writes:

> This patch wasn't applied, probably because the misplaced else
> statement, here is a corrected version. This would at last make linux
> support xon/xoff for ppp connections (it hasn't been available since 2.0
> at least).
> 
> Please apply,

Have you tested it?  If so, how?

Thanks,
Paul.

Re: [PATCH] [2.5] adding ppp xon/xoff support

[Samuel Thibault]

Le sam 12 jui 2003 11:30:48 GMT, Paul Mackerras a tapoté sur son clavier :
> Samuel Thibault writes:
> 
> > This patch wasn't applied, probably because the misplaced else
> > statement, here is a corrected version. This would at last make linux
> > support xon/xoff for ppp connections (it hasn't been available since 2.0
> > at least).
> > 
> > Please apply,
> 
> Have you tested it?  If so, how?

I've been testing it for 3 years now :) I have my own ppp implementation
on my calculator, and I really needed xon/xoff.

I've just tested it again with 2.5.75: I run a program on the calc which
shows the current buffer length and sends a XOFF as soon as 100 bytes are
available.

I then launch /usr/sbin/pppd /dev/ttyS0 9600 local xonxoff passive on
the linux side.

Without the patch, the buffer length increases until the buffer is full.
With the patch, it stops at 106 bytes as expected (2 configure
requests, the calculator being too slow to be able to send XOFF as soon
as the 100th bytes arrives). If you launch pppd without any argument
(hence having /dev/tty1 used by pppd), you can even see the keyboard
scroll lock light lit when pressing ^S (the tty->stop() stuff seems to do
that)

Well, I had a look at tty_io.c, and it changed a little bit, so although
it works in my case, the previous patch might not be sufficient. Here is a
maybe much better patch which has tty_io.c export start_tty() and
stop_tty(), so that ppp_async can call them even when module-loaded.
(I tested it again)

Regards,
Samuel Thibault

diff -ur linux-2.5.75-orig/drivers/char/tty_io.c linux-2.5.75-perso/drivers/char/tty_io.c
--- linux-2.5.75-orig/drivers/char/tty_io.c	2003-07-11 22:37:57.000000000 -0400
+++ linux-2.5.75-perso/drivers/char/tty_io.c	2003-07-11 22:37:31.000000000 -0400
@@ -611,6 +611,8 @@
 		(tty->driver->stop)(tty);
 }
 
+EXPORT_SYMBOL(stop_tty);
+
 void start_tty(struct tty_struct *tty)
 {
 	if (!tty->stopped || tty->flow_stopped)
@@ -629,6 +631,8 @@
 	wake_up_interruptible(&tty->write_wait);
 }
 
+EXPORT_SYMBOL(start_tty);
+
 static ssize_t tty_read(struct file * file, char * buf, size_t count, 
 			loff_t *ppos)
 {
diff -ur linux-2.5.75-orig/drivers/net/ppp_async.c linux-2.5.75-perso/drivers/net/ppp_async.c
--- linux-2.5.75-orig/drivers/net/ppp_async.c	2003-07-10 16:12:49.000000000 -0400
+++ linux-2.5.75-perso/drivers/net/ppp_async.c	2003-07-11 22:24:37.000000000 -0400
@@ -891,6 +891,11 @@
 			process_input_packet(ap);
 		} else if (c == PPP_ESCAPE) {
 			ap->state |= SC_ESCAPE;
+		} else if (I_IXON(ap->tty)) {
+			if (c == STOP_CHAR(ap->tty))
+				stop_tty(ap->tty);
+			else if (c == START_CHAR(ap->tty))
+				start_tty(ap->tty);
 		}
 		/* otherwise it's a char in the recv ACCM */
 		++n;

Bug in open() function (?)

[J.C. Wren]

	I was playing around today and found that if an existing file is opened with 
O_TRUNC | O_RDONLY, the existing file is truncated.  This is contrary to the 
documentation for "man 2 open".  Which behavior is correct, the man page, or 
what actually happens?  Or wold this be considered a glibc/libc problem?  
This is on a stock 2.5.74 kernel.

	'man 2 open', on O_TRUNC: If  the  file already exists and is a regular file 
and the open mode allows writing (i.e., is O_RDWR or O_WRONLY) it will be 
truncated to length 0.

	--John

#include <stdio.h>
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>

int main (int argc, char **argv)
{
   int fd;

   if ((fd = open ("test", O_TRUNC | O_RDONLY)) == -1)
   {
      printf ("%d:%s\n", errno, strerror (errno));
      exit (1);
   }

   close (fd);

   exit (0);
}

[bash] cc test.c
[bash] ls -l >test
[bash] ls -l test
-rw-r--r--    1 jcw      users         195 Jul 11 23:06 test
[bash] ./a.out
[bash] ls -l test
-rw-r--r--    1 jcw      users           0 Jul 11 23:06 test

Re: Bug in open() function (?)

[Andrew Morton]

"J.C. Wren" <jcwren@jcwren.com> wrote:
>
> I was playing around today and found that if an existing file is opened with 
>  O_TRUNC | O_RDONLY, the existing file is truncated.

Well that's fairly idiotic, isn't it?

The Open Group go on to say "The result of using O_TRUNC with O_RDONLY is
undefined" which is also rather silly.

I'd be inclined to leave it as-is, really.

Re: Bug in open() function (?)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

On Fri, 11 Jul 2003 20:38:09 PDT, Andrew Morton said:
> "J.C. Wren" <jcwren@jcwren.com> wrote:
> >
> > I was playing around today and found that if an existing file is opened wit
h 
> >  O_TRUNC | O_RDONLY, the existing file is truncated.
> 
> Well that's fairly idiotic, isn't it?

Not idiotic at all, and even if it was, it's still contrary to specific
language in the manpage.

I could *easily* see some program having a line of code:

	if (do_ro_testing) openflags |= O_RDONLY;

I'd not be surprised if J.C. was playing around because a file unexpectedly
shrank to zero size because of code like this.  There's a LOT of programs that
implement some sort of "don't really do it" option, from "/bin/bash -n" to
"cdrecord -dummy".  So you do something like the above to make your
file R/O - and O_TRUNC *STILL* zaps the file, in *direct violation* of
the language in the manpage.

Whoops.  Ouch. Where's the backup tapes?

> The Open Group go on to say "The result of using O_TRUNC with O_RDONLY is
> undefined" which is also rather silly.
> 
> I'd be inclined to leave it as-is, really.

I hate to think how many programmers are relying on the *documented* behavior to
prevent data loss during debugging/test runs....

/Valdis

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: Bug in open() function (?)

[Andrew Morton]

Valdis.Kletnieks@vt.edu wrote:
>
> On Fri, 11 Jul 2003 20:38:09 PDT, Andrew Morton said:
> > "J.C. Wren" <jcwren@jcwren.com> wrote:
> > >
> > > I was playing around today and found that if an existing file is opened wit
> h 
> > >  O_TRUNC | O_RDONLY, the existing file is truncated.
> > 
> > Well that's fairly idiotic, isn't it?
> 
> Not idiotic at all

Sigh.  I meant the kernel behaviour is idiotic.  Returning -EINVAL would
have been much better behaviour.

> 
> > The Open Group go on to say "The result of using O_TRUNC with O_RDONLY is
> > undefined" which is also rather silly.
> > 
> > I'd be inclined to leave it as-is, really.
> 
> I hate to think how many programmers are relying on the *documented* behavior to
> prevent data loss during debugging/test runs....

We've lived with it for this long.

The behaviour is "undefined".  Any application which uses O_RDONLY|O_TRUNC
is buggy.

If we were to alter the behaviour now, any buggy-but-happens-to-work app
which is accidentally using O_RDONLY|O_TRUNC may break.  And now is not the
time to break things.

Given that the behaviour is undefined, the behaviour which we should
implement is clearly "whatever 2.4 is doing".  So let's leave it alone.

Re: Bug in open() function (?)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 362 bytes --]

On Fri, 11 Jul 2003 22:23:00 PDT, Andrew Morton said:

> We've lived with it for this long.

Well... you have a point there..

> Given that the behaviour is undefined, the behaviour which we should
> implement is clearly "whatever 2.4 is doing".  So let's leave it alone.

I suppose I could live with that *IF* somebody fixes 'man 2 open' to
reflect reality.




[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: Bug in open() function (?)

[Andries Brouwer]

On Sat, Jul 12, 2003 at 02:14:43AM -0400, Valdis.Kletnieks@vt.edu wrote:

> On Fri, 11 Jul 2003 22:23:00 PDT, Andrew Morton said:
> 
> > We've lived with it for this long.
> 
> Well... you have a point there..
> 
> > Given that the behaviour is undefined, the behaviour which we should
> > implement is clearly "whatever 2.4 is doing".  So let's leave it alone.
> 
> I suppose I could live with that *IF* somebody fixes 'man 2 open' to
> reflect reality.

Corrections and additions to manpages are always welcome.
Mail to aeb@cwi.nl .


(Concerning the topic under discussion, the man page says

       O_TRUNC
              If  the  file  already exists and is a regular file
              and the open mode allows writing (i.e.,  is  O_RDWR
              or  O_WRONLY) it will be truncated to length 0.  If
              the file is a FIFO or  terminal  device  file,  the
              O_TRUNC  flag  is  ignored. Otherwise the effect of
              O_TRUNC is unspecified.

which is precisely right. It continues

                                       (On many Linux versions it
              will  be  ignored; on other versions it will return
              an error.)

where someone may read this as if this is an exhaustive list of
possibilities. So adding ", or actually do the truncate" will
clarify.)


Concerning the desired behaviour: if I recall things correctly
doing the truncate was old SunOS behaviour, not doing it,
that is, honouring the O_RDONLY, is new Solaris behaviour.
Maybe someone with access to such machines can check.

Software exists that does O_RDONLY | O_TRUNC.

Andries

Re: Bug in open() function (?)

[jw schultz]

On Sat, Jul 12, 2003 at 11:37:08AM +0200, Andries Brouwer wrote:
> On Sat, Jul 12, 2003 at 02:14:43AM -0400, Valdis.Kletnieks@vt.edu wrote:
> 
> > On Fri, 11 Jul 2003 22:23:00 PDT, Andrew Morton said:
> > 
> > > We've lived with it for this long.
> > 
> > Well... you have a point there..
> > 
> > > Given that the behaviour is undefined, the behaviour which we should
> > > implement is clearly "whatever 2.4 is doing".  So let's leave it alone.
> > 
> > I suppose I could live with that *IF* somebody fixes 'man 2 open' to
> > reflect reality.
> 
> Corrections and additions to manpages are always welcome.
> Mail to aeb@cwi.nl .
> 
> 
> (Concerning the topic under discussion, the man page says
> 
>        O_TRUNC
>               If  the  file  already exists and is a regular file
>               and the open mode allows writing (i.e.,  is  O_RDWR
>               or  O_WRONLY) it will be truncated to length 0.  If
>               the file is a FIFO or  terminal  device  file,  the
>               O_TRUNC  flag  is  ignored. Otherwise the effect of
>               O_TRUNC is unspecified.
> 
> which is precisely right. It continues
> 
>                                        (On many Linux versions it
>               will  be  ignored; on other versions it will return
>               an error.)
> 
> where someone may read this as if this is an exhaustive list of
> possibilities. So adding ", or actually do the truncate" will
> clarify.)

I'd be inclined to at least drop the parenthetic, it only
confuses things.  The alternative would be to tie the
parenthetic to the FIFO and device files.

I'll grant that O_RDONLY would cause one to expect that the
file would not be modified in any way so truncating it on a
read-only seems wrong but that does fall under the
definition of undefined so is not contrary to the
documentation.

Anyone depending on undefined behaviour is asking for
trouble.  Given that there is code floating around expecting
O_TRUNC|O_RDONLY to truncate, caution should be applied in
changing this.

I'd suggest replacing this text to match that of SUSv3 which
is much clearer.  Perhaps with the addition of a clause
stating "The use of O_TRUNC in combination with O_RDONLY to
truncate files is deprecated" or something to that effect.

SUSv3:
|	O_TRUNC
|		If the file exists and is a regular file,
|		and the file is successfully opened O_RDWR
|		or O_WRONLY, its length shall be truncated
|		to 0, and the mode and owner shall be
|		unchanged. It shall have no effect on FIFO
|		special files or terminal device files. Its
|		effect on other file types is
|		implementation-defined. The result of using
|		O_TRUNC with O_RDONLY is undefined.

-- 
________________________________________________________________
	J.W. Schultz            Pegasystems Technologies
	email address:		jw@pegasys.ws

		Remember Cernan and Schmitt

Re: Bug in open() function (?)

[J.C. Wren]

	While I'm sure I don't have a grasp of "the big picture", I would have 
imagines these would have acted like a mask against the file system 
attributes.  In effect setting O_RDONLY would clear the write permission bits 
read by stat(), and O_WRONLY would clear the read permission bits.  O_RDONLY 
+ O_WRONLY (O_RDWR) would leave the permissions alone.  These would be 
applied somewhere around the may_open() call in fs/open_namei.c

	A poor mans umaks(), I guess.

	--John

On Saturday 12 July 2003 01:11 am, Valdis.Kletnieks@vt.edu wrote:
> On Fri, 11 Jul 2003 20:38:09 PDT, Andrew Morton said:
> > "J.C. Wren" <jcwren@jcwren.com> wrote:
> > > I was playing around today and found that if an existing file is opened
> > > wit
>
> h
>
> > >  O_TRUNC | O_RDONLY, the existing file is truncated.
> >
> > Well that's fairly idiotic, isn't it?
>
> Not idiotic at all, and even if it was, it's still contrary to specific
> language in the manpage.
>
> I could *easily* see some program having a line of code:
>
> 	if (do_ro_testing) openflags |= O_RDONLY;
>
> I'd not be surprised if J.C. was playing around because a file unexpectedly
> shrank to zero size because of code like this.  There's a LOT of programs
> that implement some sort of "don't really do it" option, from "/bin/bash
> -n" to "cdrecord -dummy".  So you do something like the above to make your
> file R/O - and O_TRUNC *STILL* zaps the file, in *direct violation* of the
> language in the manpage.
>
> Whoops.  Ouch. Where's the backup tapes?
>
> > The Open Group go on to say "The result of using O_TRUNC with O_RDONLY is
> > undefined" which is also rather silly.
> >
> > I'd be inclined to leave it as-is, really.
>
> I hate to think how many programmers are relying on the *documented*
> behavior to prevent data loss during debugging/test runs....
>
> /Valdis

[PATCH] [2.6] adding xon/xoff support to ppp

[Samuel Thibault]

Hi,

It seems to have been dropped again, so I resend it.

Linux' ppp has not been implementing xon/xoff since 2.0 at least (a
thread on linux-ppp clearly stated that some years ago: just type
"pppd xonxoff": without the patch you can't stop the flow), here is a
patch to correct this on 2.6.0-test1 kernel. It has been well tested and
updated over the 2.2, 2.4 and 2.6 kernels.

Regards,
Samuel thibault



Add xon/xoff support to the ppp line discipline for async ports.

--- linux-2.6.0-test1-orig/drivers/char/tty_io.c	2003-07-26 17:25:28.000000000 -0400
+++ linux-2.6.0-test1-perso/drivers/char/tty_io.c	2003-07-14 01:11:25.000000000 -0400
@@ -611,6 +611,8 @@
 		(tty->driver->stop)(tty);
 }
 
+EXPORT_SYMBOL(stop_tty);
+
 void start_tty(struct tty_struct *tty)
 {
 	if (!tty->stopped || tty->flow_stopped)
@@ -629,6 +631,8 @@
 	wake_up_interruptible(&tty->write_wait);
 }
 
+EXPORT_SYMBOL(start_tty);
+
 static ssize_t tty_read(struct file * file, char * buf, size_t count, 
 			loff_t *ppos)
 {
--- linux-2.6.0-test1-orig/drivers/net/ppp_async.c	2003-07-26 17:23:56.000000000 -0400
+++ linux-2.6.0-test1-perso/drivers/net/ppp_async.c	2003-07-14 01:12:45.000000000 -0400
@@ -891,6 +891,11 @@
 			process_input_packet(ap);
 		} else if (c == PPP_ESCAPE) {
 			ap->state |= SC_ESCAPE;
+		} else if (I_IXON(ap->tty)) {
+			if (c == STOP_CHAR(ap->tty))
+				stop_tty(ap->tty);
+			else if (c == START_CHAR(ap->tty))
+				start_tty(ap->tty);
 		}
 		/* otherwise it's a char in the recv ACCM */
 		++n;