Most recent kernel where this bug did not occur: Distribution: gentoo 2005.1 Hardware Environment: i386 (Intel(R) Celeron(R) M processor 1.40GHz), RAM 256M Software Environment: Problem Description: Hello. I tried to setup pptp (pptpd) service on my linux box, but it don't work correctly. Problem: traffic freezes when sending large amount of data. I find lack in ppp_async.c. /* try to push more stuff out */ if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap)) ppp_output_wakeup(&ap->chan); ---->> I added this lines and pptpd starts work. else { if (test_bit(XMIT_FULL, &ap->xmit_flags)) ppp_asynctty_wakeup(ap->tty); }
bugme-daemon@bugzilla.kernel.org wrote: > > http://bugzilla.kernel.org/show_bug.cgi?id=6530 > > Summary: MAINLINE > Kernel Version: 2.6.16 > Status: NEW > Severity: normal > Owner: jgarzik@pobox.com > Submitter: xeb@mail.ru > > > Most recent kernel where this bug did not occur: > Distribution: gentoo 2005.1 > Hardware Environment: i386 (Intel(R) Celeron(R) M processor 1.40GHz), RAM 256M > Software Environment: > Problem Description: > Hello. > I tried to setup pptp (pptpd) service on my linux box, but it don't work correctly. > Problem: traffic freezes when sending large amount of data. > > I find lack in ppp_async.c. > > /* try to push more stuff out */ > if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap)) > ppp_output_wakeup(&ap->chan); > ---->> I added this lines and pptpd starts work. > else > { > if (test_bit(XMIT_FULL, &ap->xmit_flags)) > ppp_asynctty_wakeup(ap->tty); > } > hm, a PPP fix. We seem to need some of those lately. Paul, does this look sane? From: <xeb@mail.ru> Adapted from http://bugzilla.kernel.org/show_bug.cgi?id=6530 Reschedule the async Tx tasklet if the transmit queue was full. Cc: Paul Mackerras <paulus@samba.org> Signed-off-by: Andrew Morton <akpm@osdl.org> --- drivers/net/ppp_async.c | 2 ++ 1 files changed, 2 insertions(+) diff -puN drivers/net/ppp_async.c~ppp_async-hang-fix drivers/net/ppp_async.c --- devel/drivers/net/ppp_async.c~ppp_async-hang-fix 2006-05-10 02:28:15.000000000 -0700 +++ devel-akpm/drivers/net/ppp_async.c 2006-05-10 02:28:53.000000000 -0700 @@ -516,6 +516,8 @@ static void ppp_async_process(unsigned l /* try to push more stuff out */ if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap)) ppp_output_wakeup(&ap->chan); + else if (test_bit(XMIT_FULL, &ap->xmit_flags)) + ppp_asynctty_wakeup(ap->tty); } /* _
Andrew Morton writes: > hm, a PPP fix. We seem to need some of those lately. > > Paul, does this look sane? /me pages in 7 year old code... > @@ -516,6 +516,8 @@ static void ppp_async_process(unsigned l > /* try to push more stuff out */ > if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap)) > ppp_output_wakeup(&ap->chan); > + else if (test_bit(XMIT_FULL, &ap->xmit_flags)) > + ppp_asynctty_wakeup(ap->tty); ppp_asynctty_wakeup is supposed to be called by the serial driver when it can take more output. It's slightly bogus having ppp_async call it itself whether or not the serial driver can take more output at the moment, but I suppose it won't hurt. I would really like to know the precise circumstances where we need this fake wakeup though. Is the serial driver failing to give us a wakeup call where it should, or is ppp_async ignoring a wakeup for some reason? I think the same effect could be achieved without an extra trip through tasklet_schedule et al. by making those lines look like this (untested): if ((test_bit(XMIT_WAKEUP, &ap->xmit_flags) || test_bit(XMIT_FULL, &ap->xmit_flags)) && ppp_async_push(ap)) ppp_output_wakeup(&ap->chan); so that ppp_async_push gets called if either XMIT_WAKEUP or XMIT_FULL is set. This is all relying on getting some input to kick off more output when the wakeup gets missed, though. That's a reasonable workaround in most situations, I guess, but I'd really like to know why the wakeup is getting missed. Paul.
Boys, in this construction: if ((test_bit(XMIT_WAKEUP, &ap->xmit_flags) || test_bit(XMIT_FULL, &ap->xmit_flags)) && ppp_async_push(ap)) ppp_output_wakeup(&ap->chan); if ppp_async_push() doesn't send any data i.e. XMIT_FULL is set then all (transfer) hangs up while somebody push again, for instance lcp-echo.
Paul Mackerras <paulus@samba.org> wrote: > > Andrew Morton writes: > > > hm, a PPP fix. We seem to need some of those lately. > > > > Paul, does this look sane? > > /me pages in 7 year old code... > > > @@ -516,6 +516,8 @@ static void ppp_async_process(unsigned l > > /* try to push more stuff out */ > > if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap)) > > ppp_output_wakeup(&ap->chan); > > + else if (test_bit(XMIT_FULL, &ap->xmit_flags)) > > + ppp_asynctty_wakeup(ap->tty); > > ppp_asynctty_wakeup is supposed to be called by the serial driver when > it can take more output. It's slightly bogus having ppp_async call it > itself whether or not the serial driver can take more output at the > moment, but I suppose it won't hurt. I would really like to know the > precise circumstances where we need this fake wakeup though. Is the > serial driver failing to give us a wakeup call where it should, or is > ppp_async ignoring a wakeup for some reason? > > I think the same effect could be achieved without an extra trip > through tasklet_schedule et al. by making those lines look like this > (untested): > > if ((test_bit(XMIT_WAKEUP, &ap->xmit_flags) || > test_bit(XMIT_FULL, &ap->xmit_flags)) && ppp_async_push(ap)) > ppp_output_wakeup(&ap->chan); > > so that ppp_async_push gets called if either XMIT_WAKEUP or XMIT_FULL > is set. > > This is all relying on getting some input to kick off more output when > the wakeup gets missed, though. That's a reasonable workaround in most > situations, I guess, but I'd really like to know why the wakeup is > getting missed. > (xeb, on this bug please respond via email using reply-to-all rather than via the bugzilla web form). xeb has said: in this construction: if ((test_bit(XMIT_WAKEUP, &ap->xmit_flags) || test_bit(XMIT_FULL, &ap->xmit_flags)) && ppp_async_push(ap)) ppp_output_wakeup(&ap->chan); if ppp_async_push() doesn't send any data i.e. XMIT_FULL is set then all (transfer) hangs up while somebody push again, for instance lcp-echo.
Andrew Morton writes: > xeb has said: > > in this construction: > > if ((test_bit(XMIT_WAKEUP, &ap->xmit_flags) || > test_bit(XMIT_FULL, &ap->xmit_flags)) && ppp_async_push(ap)) > ppp_output_wakeup(&ap->chan); > > if ppp_async_push() doesn't send any data i.e. XMIT_FULL is set then all > (transfer) hangs up while somebody push again, for instance lcp-echo. If XMIT_FULL and ppp_async_push doesn't send any data, that means the serial driver's output buffer was full. If that's the case, *and* we don't see a call to ppp_output_wakeup, then the finger points squarely at the serial driver as the source of the bug. Paul.
Paul Mackerras <paulus@samba.org> wrote: > > Andrew Morton writes: > > > xeb has said: > > > > in this construction: > > > > if ((test_bit(XMIT_WAKEUP, &ap->xmit_flags) || > > test_bit(XMIT_FULL, &ap->xmit_flags)) && ppp_async_push(ap)) > > ppp_output_wakeup(&ap->chan); > > > > if ppp_async_push() doesn't send any data i.e. XMIT_FULL is set then all > > (transfer) hangs up while somebody push again, for instance lcp-echo. > > If XMIT_FULL and ppp_async_push doesn't send any data, that means the > serial driver's output buffer was full. If that's the case, *and* we > don't see a call to ppp_output_wakeup, then the finger points squarely > at the serial driver as the source of the bug. > OK, thanks. So the next question is: which driver is being used?
Reply-To: andy@andynet.net On Wed, 2006-05-10 at 20:27 +1000, Paul Mackerras wrote: > Andrew Morton writes: > > > hm, a PPP fix. We seem to need some of those lately. > > > > Paul, does this look sane? > > /me pages in 7 year old code... > > > @@ -516,6 +516,8 @@ static void ppp_async_process(unsigned l > > /* try to push more stuff out */ > > if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap)) > > ppp_output_wakeup(&ap->chan); > > + else if (test_bit(XMIT_FULL, &ap->xmit_flags)) > > + ppp_asynctty_wakeup(ap->tty); > > ppp_asynctty_wakeup is supposed to be called by the serial driver when > it can take more output. How does the serial driver know it has to call ppp_asynctty_wakeup()? I'd have thought it wouldn't know or care if it's being used for ppp or any other async traffic. There were a bunch of changes to the serial drivers between 2.6.15 and 2.6.16, maybe that's where this problem was introduced. Do we know which serial driver is involved in the original report? (I'm interested in this because I need to convert an out of tree serial driver for 2.6.16+, I'm wondering if I'll need to do anything special to support ppp). > It's slightly bogus having ppp_async call it > itself whether or not the serial driver can take more output at the > moment, but I suppose it won't hurt. I would really like to know the > precise circumstances where we need this fake wakeup though. Is the > serial driver failing to give us a wakeup call where it should, or is > ppp_async ignoring a wakeup for some reason? > > I think the same effect could be achieved without an extra trip > through tasklet_schedule et al. by making those lines look like this > (untested): > > if ((test_bit(XMIT_WAKEUP, &ap->xmit_flags) || > test_bit(XMIT_FULL, &ap->xmit_flags)) && ppp_async_push(ap)) > ppp_output_wakeup(&ap->chan); > > so that ppp_async_push gets called if either XMIT_WAKEUP or XMIT_FULL > is set. > > This is all relying on getting some input to kick off more output when > the wakeup gets missed, though. That's a reasonable workaround in most > situations, I guess, but I'd really like to know why the wakeup is > getting missed. > > Paul. > - > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Andy Gay <andy@andynet.net> wrote: > > On Wed, 2006-05-10 at 20:27 +1000, Paul Mackerras wrote: > > Andrew Morton writes: > > > > > hm, a PPP fix. We seem to need some of those lately. > > > > > > Paul, does this look sane? > > > > /me pages in 7 year old code... > > > > > @@ -516,6 +516,8 @@ static void ppp_async_process(unsigned l > > > /* try to push more stuff out */ > > > if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap)) > > > ppp_output_wakeup(&ap->chan); > > > + else if (test_bit(XMIT_FULL, &ap->xmit_flags)) > > > + ppp_asynctty_wakeup(ap->tty); > > > > ppp_asynctty_wakeup is supposed to be called by the serial driver when > > it can take more output. > > How does the serial driver know it has to call ppp_asynctty_wakeup()? > I'd have thought it wouldn't know or care if it's being used for ppp or > any other async traffic. xeb (who forgot to do reply-to-all) tells me that pptpd uses ptys.
Andy Gay writes: > How does the serial driver know it has to call ppp_asynctty_wakeup()? The serial driver is supposed to call the line discipline's wakeup function when it has room in the output buffer and the TTY_DO_WRITE_WAKEUP bit is set in tty->flags. When the serial port is set to the ppp line discipline, then it uses the functions defined in the ppp_ldisc structure in drivers/net/ppp_async.c, and the write_wakeup field in that structure points to ppp_asynctty_wakeup. > There were a bunch of changes to the serial drivers between 2.6.15 and > 2.6.16, maybe that's where this problem was introduced. Do we know which > serial driver is involved in the original report? Apparently it's the pty driver. Paul.
Reply-To: andy@andynet.net On Fri, 2006-05-12 at 11:59 +1000, Paul Mackerras wrote: > Andy Gay writes: > > > How does the serial driver know it has to call ppp_asynctty_wakeup()? > > The serial driver is supposed to call the line discipline's wakeup > function when it has room in the output buffer and the > TTY_DO_WRITE_WAKEUP bit is set in tty->flags. When the serial port is > set to the ppp line discipline, then it uses the functions defined in > the ppp_ldisc structure in drivers/net/ppp_async.c, and the > write_wakeup field in that structure points to ppp_asynctty_wakeup. > OK, thanks for the explanation. I'll pay special attention to that stuff in my driver! > > There were a bunch of changes to the serial drivers between 2.6.15 and > > 2.6.16, maybe that's where this problem was introduced. Do we know which > > serial driver is involved in the original report? > > Apparently it's the pty driver. > So I heard. Hopefully the maintainer of that driver will see this.... > Paul.
Andrew Morton writes: > xeb (who forgot to do reply-to-all) tells me that pptpd uses ptys. I tried to replicate this using pppd running on a pty, with a "charshunt" process on the master side of the pty transferring characters between it and a socket. I didn't see any freezeups in either direction. So - precise details on how to replicate this would be appreciated. Paul.
You need two computers on 100Mbit ethernet. Setup pptpd (server) on one . Setup pptp (client) on other. Then setup ppp-connection and try to send random file (min 100M) from one comp. to other. Software: http://poptop.sourceforge.net, http://pptpclient.sourceforge.net/
Turns out xeb's mail server is rejecting my emails because they come from ozlabs.org. *shrug* *plonk*
> Turns out xeb's mail server is rejecting my emails because they come from > ozlabs.org. > > *shrug* *plonk* > > ------- You are receiving this mail because: ------- > You reported the bug, or are watching the reporter. > Try this xxebxebx@gmail.com
OK. I may get time to set up pptp and pptpd on the weekend...
I've been able to reproduce this on my setup at home. Using git bisect I found the commit that first triggers this problem is: [PATCH] TTY layer buffering revamp 33f0f88f1c51ae5c2d593d26960c760ea154c2e2 I used the following setup. Server Debian testing/unstable i386 kernel 2.6.17-rc4 (from git) ppp 2.4.4b1-1 pptpd 1.2.3-1.1 pptp-linux 1.7.0-2 :/etc/pptpd.conf option /etc/ppp/pptpd-options logwtmp localip 192.168.250.1 remoteip 192.168.251.64-128 :/etc/ppp/pptpd-options name pptpvpn proxyarp nodefaultroute lock nobsdcomp :/etc/ppp/options asyncmap 0 auth crtscts lock hide-password modem proxyamp lcp-echo-interval 30 lcp-echo-failure 4 noipx :/etc/ppp/chap-secrets pptptest pptpvpn pptptest * Client i386 Debian testing/unstable kernel various (from git) ppp 2.4.4b1-1 pptp-linux 1.7.0-2 :/etc/ppp/peers/pptpvpn pty "pptp <address of server> --nolauchppd" name pptptest remotename pptpvpn ipparam pptpvpn lock noauth nobsdcomp nodefalte :/etc/ppp/chap-secrets pptptest pptpvpn pptptest * The from the client: # pon pptpvpn # scp random.data 192.168.250.1: If the file is large the upload will stall a few times and the upload seems to be slower. Hope that helps.
This looks like bug #6402. xeb, can you confirm this is fixed in recent kernels?
Sorry, I can't do it now, but this bug was fixed and pptp worked fine on 2.6.17, 2.6.18