netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Woodhouse <dwmw2@infradead.org>
To: Nathan Williams <nathan@traverse.com.au>
Cc: Karl Hiramoto <karl@hiramoto.org>,
	"David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org
Subject: Re: PPPoE performance regression
Date: Sun, 10 Jun 2012 09:32:16 +0100	[thread overview]
Message-ID: <1339317136.2851.54.camel@shinybook.infradead.org> (raw)
In-Reply-To: <1339289425.2661.27.camel@laptop>

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

 On Sun, 2012-06-10 at 10:50 +1000, Nathan Williams wrote:
> > When using iperf with UDP, we can get 20Mbps downstream, but only about
> > 15Mbps throughput when using TCP on a short ADSL line (line sync at
> > 25Mbps).  Using iperf to send UDP traffic upstream at the same time
> > doesn't affect the downstream rate.
>
> ...
>
> I found the change responsible for the performance problem and rebuilt
> OpenWrt with the patch reversed on kernel 3.3.8 to confirm everything
> still works.  So the TX buffer is getting full, which causes the netif
> queue to be stopped and restarted after some skbs have been freed?

The *Ethernet* netif queue, yes. But not the PPP netif queue, I believe.
I think the PPP code keeps just blindly calling dev_queue_xmit() and
throwing away packets when they're not accepted.

> commit 137742cf9738f1b4784058ff79aec7ca85e769d4
> Author: Karl Hiramoto <karl@hiramoto.org>
> Date:   Wed Sep 2 23:26:39 2009 -0700
> 
>     atm/br2684: netif_stop_queue() when atm device busy and
> netif_wake_queue() when we can send packets again.

Nice work; well done finding that. I've added Karl and DaveM, and the
netdev@ list to Cc.

(Btw, I assume the performance problem also goes away if you use PPPoA?
I've made changes in the PPPoA code recently to *eliminate* excessive
calls to netif_wake_queue(), and also to stop it from filling the ATM
device queue. That was commit 9d02daf7 in 3.5-rc1, which is already in
OpenWRT.)

I was already looking vaguely at how we could limit the PPP queue depth
for PPPoE and implement byte queue limits. Currently the PPP code just
throws the packets at the Ethernet device and considers them 'gone',
which is why it's hitting the ATM limits all the time. The patch you
highlight is changing the behaviour in a case that should never *happen*
with PPP. It's suffering massive queue bloat if it's filling the ATM
queue, and we should fix *that*.

I was looking to see if we could (ab)use the skb->destructor somehow so
that we get *notified* when the packet is actually sent (or dropped),
and then that would allow us to manage the queue 'downstream' of PPP
more sanely. But I haven't really got very far with that yet.

I was planning to find some time to look into it a bit better, and then
send mail to netdev@ asking for more clue. But since you're now falling
over it and it isn't just a theoretical problem, this mail will have to
suffice for now...

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

       reply	other threads:[~2012-06-10  8:32 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1339143949.24571.72.camel@dualcore.traverse>
     [not found] ` <1339144110.13998.1.camel@i7.infradead.org>
     [not found]   ` <1339144954.24571.80.camel@dualcore.traverse>
     [not found]     ` <1339147045.13998.3.camel@i7.infradead.org>
     [not found]       ` <1339289425.2661.27.camel@laptop>
2012-06-10  8:32         ` David Woodhouse [this message]
2012-06-13  9:57           ` PPPoE performance regression David Woodhouse
2012-06-13 13:50             ` David Woodhouse
2012-06-13 15:55               ` Benjamin LaHaise
2012-06-13 16:11                 ` David Woodhouse
2012-06-13 16:31                   ` Benjamin LaHaise
2012-06-13 16:32                     ` David Laight
2012-06-13 16:59                       ` David Woodhouse
2012-06-13 16:53                     ` David Woodhouse
2012-06-13 17:21                       ` Benjamin LaHaise
2012-06-13 17:43                         ` David Woodhouse
2012-06-14  6:18               ` Paul Mackerras
2012-06-14  6:49                 ` David Woodhouse
2012-06-14 10:35                 ` David Woodhouse
2012-06-13 20:17           ` Karl Hiramoto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1339317136.2851.54.camel@shinybook.infradead.org \
    --to=dwmw2@infradead.org \
    --cc=davem@davemloft.net \
    --cc=karl@hiramoto.org \
    --cc=nathan@traverse.com.au \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).