netdev - Re: [RFC PATCH] Regression in linux 2.6.32 virtio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20091213122512.GA17255@gondor.apana.org.au>
Date:	Sun, 13 Dec 2009 20:25:12 +0800
From:	Herbert Xu <herbert@...dor.apana.org.au>
To:	Sridhar Samudrala <sri@...ibm.com>
Cc:	rusty@...tcorp.com.au, mst@...hat.com, netdev@...r.kernel.org
Subject: Re: [RFC PATCH] Regression in linux 2.6.32 virtio_net seen with
	vhost-net

Sridhar Samudrala <sri@...ibm.com> wrote:
> When testing vhost-net patches with a guest running linux 2.6.32, i am
> running into "Unexpected full queue" warning messages from start_xmit() in
> virtio_net.c causing a lot of requeues and a drastic reduction in throughput.
> 
> With a guest running 2.6.31, i get guest to host throughput around 7000Mb/s,
> but it drops to around 3200Mb/s with 2.6.32.
> 
> I don't see this problem with usermode virtio in qemu, but i get a thruput of
> only 2700Mb/s with both 2.6.31 and 2.6.32.
> 
> The following patch fixes this problem by dropping the skb and not requeuing
> to qdisc when it cannot be added to ring buffer. With this patch, i see
> similar performance as 2.6.31 with vhost-net.
> 
> Signed-off-by: Sridhar Samudrala <sri@...ibm.com>
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index b9e002f..307cfd6 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -528,7 +528,11 @@ again:
>                        netif_start_queue(dev);
>                        goto again;
>                }
> -               return NETDEV_TX_BUSY;
> +
> +               /* drop the skb under stress. */ 
> +               vi->dev->stats.tx_dropped++;
> +               kfree_skb(skb);
> +               return NETDEV_TX_OK;
>        }

This patch only hides the real problem.  The issue is that we
should never start the queue unless we can accomodate a full
64KB packet.

In your case, whenever we have the space for a single slot we'd
start the queue.  However, if the head of the queue required more
than one slot it would immediately return BUSY and stop the queue
again.

Your patch simply drops the packet (likely to be TSO) which would
end up upsetting the TCP transmitter.

Please see what a real Ethernet driver (e.g., tg3 or ixgbe) does
for queue management.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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