netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Greear <greearb@candelatech.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Subject: Re: [net-next 1/2] qdisc: Allow qdiscs to provide backpressure up the stack.
Date: Fri, 27 Aug 2010 08:26:49 -0700	[thread overview]
Message-ID: <4C77D939.2050807@candelatech.com> (raw)
In-Reply-To: <20100826.231110.260096663.davem@davemloft.net>

On 08/26/2010 11:11 PM, David Miller wrote:
> From: Ben Greear<greearb@candelatech.com>
> Date: Thu, 26 Aug 2010 22:58:41 -0700
>
>> And if it does that, then something must know how to restart the
>> transmit logic.
>
> When SKBs get freed up, space opens up in the socket send buffer,
> waking up the process or signalling it from poll() so it can write
> more.

Seems there is still room for problems:

*  There may be zero frames already in the qdisc for this UDP connection,
    so aside from the one that failed to enqueue, none other will be freed.

*  If deleting that single SKB that was to be enqueued opens space, then it's
    not really throttling, just busy-spinning on deleting skbs, but I think
    that the wakeup heuristics won't wake the queue until it's about 1/2 full
    anyway, so that single skb isn't going to wake any queues in most cases.

I'll look through the code later today and see how much churn it would take
to support the BUSY return code from dev_queue_xmit, without adding any new
qdisc API methods.

For the trivial case, I can just kfree_skb when BUSY is returned, for the
same overall behaviour as today.  For something like UDP, I might be able
to poke the SKB back into the queue instead of freeing it.

> pktgen eliminates this whole layer of queueing and signalling, which
> is why it continually succeptible this behavior you dislike.

Proper backoff on error codes from hard_start_xmit and some wake logic
when the unerlying netdevice wakes up again fixes this.  You can make pktgen
run w/out busy spinning.  It's a big messy patch (as I coded it up)
though.

I think we could concentrate on allowing qdisc to return some sort of
congestion notification w/out always freeing the skb first, and once
that seems workable, go back to worrying about how to properly propagate
that through macvlans.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

  reply	other threads:[~2010-08-27 15:26 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-25 19:00 [net-next 1/2] qdisc: Allow qdiscs to provide backpressure up the stack Ben Greear
2010-08-25 19:00 ` [net-next 2/2] macvlan: Enable qdisc backoff logic Ben Greear
2010-08-25 19:24   ` Arnd Bergmann
2010-08-25 19:27     ` Ben Greear
2010-08-25 19:38       ` Hagen Paul Pfeifer
2010-08-25 19:49         ` Ben Greear
2010-08-25 19:59       ` Arnd Bergmann
2010-08-25 20:49         ` Ben Greear
2010-08-26 13:55           ` Arnd Bergmann
2010-08-26 15:33             ` Ben Greear
2010-08-26 17:45             ` Ben Greear
2010-08-27 13:16               ` Arnd Bergmann
2010-08-25 20:44 ` [net-next 1/2] qdisc: Allow qdiscs to provide backpressure up the stack Stephen Hemminger
2010-08-25 20:56   ` Ben Greear
2010-08-26 22:59 ` David Miller
2010-08-27  4:14   ` Ben Greear
2010-08-27  4:34     ` David Miller
2010-08-27  5:22       ` Ben Greear
2010-08-27  5:36         ` David Miller
2010-08-27  5:58           ` Ben Greear
2010-08-27  6:11             ` David Miller
2010-08-27 15:26               ` Ben Greear [this message]
2010-08-27 15:59                 ` Eric Dumazet
2010-08-27 17:00                   ` Ben Greear

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=4C77D939.2050807@candelatech.com \
    --to=greearb@candelatech.com \
    --cc=davem@davemloft.net \
    --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).