netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Greear <greearb@candelatech.com>
To: Hagen Paul Pfeifer <hagen@jauu.net>
Cc: Arnd Bergmann <arnd@arndb.de>, netdev@vger.kernel.org
Subject: Re: [net-next 2/2] macvlan:  Enable qdisc backoff logic.
Date: Wed, 25 Aug 2010 12:49:37 -0700	[thread overview]
Message-ID: <4C7573D1.9070400@candelatech.com> (raw)
In-Reply-To: <20100825193800.GA9118@nuttenaction>

On 08/25/2010 12:38 PM, Hagen Paul Pfeifer wrote:
> * Ben Greear | 2010-08-25 12:27:43 [-0700]:
>
>>> I suppose we need to do something in macvtap to handle this as
>>> well, right? A guest trying to send a frame through qemu
>>> or vhost net into macvtap needs to be prevented from sending
>>> more when we get into this path. Right now, we just ignore
>>> the return value of macvlan_start_xmit.
>>
>> I have a similar, though slightly more complex, patch for 802.1q
>> vlans, but I haven't looked at macvtap at all.
>>
>> If these two patches are accepted, I'll post the .1q patch as well.
>
> I do not completely understand the benefit for macvlan. I think this BUSY logic
> shifts functionality and make upper level code more complicated (e.g. handle
> NET_XMIT_SUCCESS and skb bookkeeping). At the end it boils down to two
> scenarios:

Code that is calling hard_start_xmit already has to know how to deal
with the NETDEV_TX_BUSY return code, this just allows mac-vlans to return that
code instead of always dropping in overload scenarios.

This *should* allow backpressure up to user-space socket buffers to fill up
and provide indication that they should slow down transmitting (and
perhaps sleep a bit) instead of continually doing work to send packets
that are being dropped.

> a) the congestion is temporary
> b) the congestion is for a longer period
>
> For a), a increased link queue length can bridge a longer period too. There is
> no need to shift the logic in the upper layer. For b): at the end the upper
> layer must also drop skb's - there is no alternative. Or require qemu other,
> special handling? (e.g. sleep until the queue is free again).

For b, the thing generating packets can back off.  I'm not 100% sure the
back-pressure logic goes all the way up the stack properly, but there
is no fundamental reason it couldn't, and this macvlan patch just
makes it work a small bit better.

If something was trying to take a pkt out of a queue for xmit,
and it got the NETDEV_TX_BUSY when it tried to send, it could simply
poke that skb back into the queue and return EBUSY or whatever to
the calling code.

> For case a) the shift in the upper layers _can_ be superior because it can
> dynamically increase the skb buffer, whatever. But why not implementing a more
> clever, dynamic fifo. E.g. pfifo_dynamic (not really serious)? Is this
> functionality qemu centric or are there any other use cases?

I don't use it with qemu.  I primarily wrote this to make pktgen able to
back off when sending pkts on mac-vlans.  High-speed user-space senders
should also benefit.

Thanks,
Ben

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


  reply	other threads:[~2010-08-25 19:49 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 [this message]
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
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=4C7573D1.9070400@candelatech.com \
    --to=greearb@candelatech.com \
    --cc=arnd@arndb.de \
    --cc=hagen@jauu.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).