On Mon, 2008-01-28 at 09:32 -0600, Anthony Liguori wrote:
> Hi Dor,
>
> How are you measuring performance? The numbers I've gotten with netperf
> before and after your patch are:
>
> tx - 647.27mbit
> rx - 89.22
>
> tx - 27.82
> rx - 79.93
>
I've been testing with iperf (patched with Ingo's fix).
I also tested tcp/udp (udp tx is only 550Mbps with the patch, w/o it's
only 220Mbps)
> So this patch is pretty much killing performance for netperf.
>
Did you have hrtimer configured on the host?
> Dor Laor wrote:
> > There was a problem with the location of the notify call in
> > add_buff function:
> > When VRING_USED_F_NO_NOTIFY is set, the host does not kick the
> > guest when packets were transmitted, as a result the guest runs
> > out of tx buffers sometimes.
>
> But even if F_NO_NOTIFY is set, if the tx buffer is full, we notify the
> guest, so this prevents that from happening.
>
> > This is fine but the problem lies
> > when add_buf fails, it called notify and the host sends all the
> > pending tx pkts. When enable_cb was called, more_used(vq) returned
> > false so eventually the skb was dropped.
> >
>
> I'm having a tough time following this part. If add_buf fails, we
> notify unconditionally (which is, I think what we want). I'm not sure
> how that relates to a packet getting dropped though.
>
Here is start_xmit function:
again:
/* Free up any pending old buffers before queueing new ones. */
free_old_xmit_skbs(vi);
err = vi->svq->vq_ops->add_buf(vi->svq, sg, num, 0, skb);
<<< here add_buf might fail since the ring is full.
<<< once it fails, the original code called notify which triggered the
<<< host to send all the pending tx so all descriptors are used.
if (err) {
vi->stats.sendq_full++;
pr_debug("%s: virtio not prepared to send\n",dev->name);
netif_stop_queue(dev);
/* Activate callback for using skbs: if this fails it
* means some were used in the meantime. */
vi->stats.sendq_enabled++;
if (unlikely(!vi->svq->vq_ops->enable_cb(vi->svq))) {
<<< Since before the patch enable_cb returned true, thus the goto below
<<< was not called.
<<< The patch moves the notify to enable_cb above.
printk("Unlikely: restart svq failed\n");
vi->stats.sendq_enable_failed++;
netif_start_queue(dev);
goto again;
}
__skb_unlink(skb, &vi->send);
return NETDEV_TX_BUSY;
}
> Regards,
>
> Anthony Liguori
>
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization
