[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20100414.045506.52212061.davem@davemloft.net>
Date: Wed, 14 Apr 2010 04:55:06 -0700 (PDT)
From: David Miller <davem@...emloft.net>
To: herbert@...dor.apana.org.au
Cc: eric.dumazet@...il.com, mst@...hat.com, jan.kiszka@...mens.com,
paul.moore@...com, David.Woodhouse@...el.com,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
qemu-devel@...gnu.org
Subject: Re: [PATCH] tun: orphan an skb on tx
From: Herbert Xu <herbert@...dor.apana.org.au>
Date: Wed, 14 Apr 2010 08:58:22 +0800
> On Tue, Apr 13, 2010 at 08:31:03PM +0200, Eric Dumazet wrote:
>>
>> Herbert Acked your patch, so I guess its OK, but I think it can be
>> dangerous.
>
> The tun socket accounting was never designed to stop it from
> flooding another tun interface. It's there to stop it from
> transmitting above a destination interface TX bandwidth and
> cause unnecessary packet drops. It also limits the total amount
> of kernel memory that can be pinned down by a single tun interface.
>
> In this case, all we're doing is shifting the accounting from the
> "hardware" queue to the qdisc queue.
>
> So your ability to flood a tun interface is essentially unchanged.
>
> BTW we do the same thing in a number of hardware drivers, as well
> as virtio-net.
Right. Although this reminds me about the whole SKB
orphaning on xmit issue that keeps coming back to haunt
us.
If there weren't odd references to the SKB's socket in
the packet scheduler et al. we could just orphan these
things right upon entry to the qdisc and not have to
add hacks like this to every driver.
In fact... maybe we can just do it in dev_hard_queue_xmit()
since we are out of the qdisc at that point.... but I guess
there might be weird drivers that want the SKB socket in
their ->xmit routine... Ho hum.
In any event that's net-next-2.6 exploratory material, and I've
applied this patch to net-2.6, thanks!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists