From: David Woodhouse <dwmw2@infradead.org>
To: chas williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Cc: netdev@vger.kernel.org, David Miller <davem@davemloft.net>,
paulus@samba.org, Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH] pppoatm: Fix excessive queue bloat
Date: Tue, 10 Apr 2012 21:28:34 +0100 [thread overview]
Message-ID: <1334089714.31812.177.camel@shinybook.infradead.org> (raw)
In-Reply-To: <20120410102612.368b1a4f@thirdoffive.cmf.nrl.navy.mil>
[-- Attachment #1: Type: text/plain, Size: 2662 bytes --]
On Tue, 2012-04-10 at 10:26 -0400, chas williams - CONTRACTOR wrote:
> On Sun, 08 Apr 2012 21:53:57 +0200
> David Woodhouse <dwmw2@infradead.org> wrote:
>
> > Seriously, this gets *much* easier if we just ditch the checks against
> > sk_sndbuf. We just wake up whenever decrementing ->inflight from zero.
> > Can I?
>
> i dont know. on a 'low' speed connection like queuing 2 packets might
> be enough to keep something busy but imagine an interace like oc3 or
> oc12. i dont know anything running pppoatm on such an interface but it
> seems like just dropping sk_sndbuf isnt right either.
That looks like a response to my patch, not to the question that you
cited. My patch reduces the buffering to MAX(vcc->sk_sndbuf, 2 packets)
so if there are issues with keeping faster devices busy, they'll happen
anyway with my patch. (My question was just whether we can ditch the
sk_sndbuf bit altogether, and just make it a hard-coded two packets.)
The limit of two packets was chosen on the basis that the PPP core is
designed to feed us new packets when we need them, with *low* latency.
So when the hardware finishes sending packet #1 and starts on packet #2,
we *should* be able to get a packet #3 into its queue by the time it
needs it.
But if that approach could really cause an issue with keeping faster
devices busy, perhaps the limit should be "x ms worth of packets" based
on the upload speed of the device, rather than a hard-coded 2?
Not that we *know* the upload speed of the device, for asymmetric
links... do we?
> sk_sndbuf is per vcc and that isnt right either since the transmit
> limit is actually the atm device's transmit queue depth.
Hm, I don't think that's a limit that I care about. You're talking about
the maximum number of packets that can be queued to the hardware at a
time. What I care about is the *minimum* number of packets that *need*
to be queued to the hardware, to ensure that it doesn't stall waiting
for us to replenish its queue.
Which is largely a factor of how well the PPP core does the job it was
*designed* to do, as I see it.
> what is the "queue depth" of your atm device's transmit queue?
We're still using MMIO for the Solos ADSL2+ devices at the moment, so
there's no descriptor ring. It used to have internal buffering which was
only limited by the device's internal memory — it would continue to
accept packets from the host until it had nowhere else to put them. I
got them to fix that in its firmware, so now it only has two or three
packets queued internally. But as far as the host is concerned, those
packets are *gone* already.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5818 bytes --]
next prev parent reply other threads:[~2012-04-10 20:28 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-22 21:03 [STRAW MAN PATCH] sch_teql doesn't load-balance ppp(oatm) slaves David Woodhouse
2012-03-23 3:03 ` David Miller
2012-03-25 10:43 ` David Woodhouse
2012-03-25 21:36 ` David Miller
2012-03-26 8:32 ` David Woodhouse
2012-03-26 9:45 ` David Woodhouse
2012-03-26 10:03 ` [PATCH] ppp: Don't stop and restart queue on every TX packet David Woodhouse
2012-04-03 21:29 ` David Miller
2012-04-08 19:58 ` David Woodhouse
2012-04-08 20:01 ` ppp: Fix race condition with queue start/stop David Woodhouse
2012-04-13 17:07 ` David Miller
2012-03-27 19:10 ` [STRAW MAN PATCH] sch_teql doesn't load-balance ppp(oatm) slaves David Woodhouse
2012-03-27 19:55 ` Eric Dumazet
2012-03-27 20:35 ` David Woodhouse
2012-04-08 19:53 ` [PATCH] pppoatm: Fix excessive queue bloat David Woodhouse
2012-04-10 14:26 ` chas williams - CONTRACTOR
2012-04-10 20:28 ` David Woodhouse [this message]
2012-04-13 17:04 ` David Miller
2012-04-13 17:27 ` David Miller
2012-04-13 17:05 ` David Miller
2012-04-08 19:55 ` David Woodhouse
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=1334089714.31812.177.camel@shinybook.infradead.org \
--to=dwmw2@infradead.org \
--cc=chas@cmf.nrl.navy.mil \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=paulus@samba.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).