Re: [PATCH] virtio-net: lower min ring num_free for efficiency

From: Jason Wang
Date: Wed Aug 14 2019 - 23:17:37 EST


On 2019/8/15 äå11:11, å jiang wrote:
On 2019/8/15 11:01, Jason Wang wrote:
On 2019/8/14 äå10:06, ? jiang wrote:
This change lowers ring buffer reclaim threshold from 1/2*queue to
budget
for better performance. According to our test with qemu + dpdk, packet
dropping happens when the guest is not able to provide free buffer in
avail ring timely with default 1/2*queue. The value in the patch has
been
tested and does show better performance.

Please add your tests setup and result here.

Thanks


Signed-off-by: jiangkidd <jiangkidd@xxxxxxxxxxx>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0d4115c9e20b..bc08be7925eb 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1331,7 +1331,7 @@ static int virtnet_receive(struct receive_queue
*rq, int budget,
ÂÂÂÂÂÂÂÂÂ }
ÂÂÂÂÂ }
 - if (rq->vq->num_free > virtqueue_get_vring_size(rq->vq) / 2) {
+ÂÂÂ if (rq->vq->num_free > min((unsigned int)budget,
virtqueue_get_vring_size(rq->vq)) / 2) {
ÂÂÂÂÂÂÂÂÂ if (!try_fill_recv(vi, rq, GFP_ATOMIC))
ÂÂÂÂÂÂÂÂÂÂÂÂÂ schedule_delayed_work(&vi->refill, 0);
ÂÂÂÂÂ }
Sure, here are the details:


Thanks for the details, but I meant it's better if you could summarize you test result in the commit log in a compact way.

Btw, some comments, see below:




Test setup & result:

----------------------------------------------------

Below is the snippet from our test result. Test1 was done with default
driver with the value of 1/2 * queue, while test2 is with my patch. We
can see average
drop packets do decrease a lot in test2.

test1TimeÂÂÂ avgDropPacketsÂÂÂ test2TimeÂÂÂ avgDropPackets pps

16:21.0ÂÂÂ 12.295ÂÂÂ 56:50.4ÂÂÂ 0 300k
17:19.1ÂÂÂ 15.244ÂÂÂ 56:50.4ÂÂÂ 0ÂÂÂ 300k
18:17.5ÂÂÂ 18.789ÂÂÂ 56:50.4ÂÂÂ 0ÂÂÂ 300k
19:15.1ÂÂÂ 14.208ÂÂÂ 56:50.4ÂÂÂ 0ÂÂÂ 300k
20:13.2ÂÂÂ 20.818ÂÂÂ 56:50.4ÂÂÂ 0.267ÂÂÂ 300k
21:11.2ÂÂÂ 12.397ÂÂÂ 56:50.4ÂÂÂ 0ÂÂÂ 300k
22:09.3ÂÂÂ 12.599ÂÂÂ 56:50.4ÂÂÂ 0ÂÂÂ 300k
23:07.3ÂÂÂ 15.531ÂÂÂ 57:48.4ÂÂÂ 0ÂÂÂ 300k
24:05.5ÂÂÂ 13.664ÂÂÂ 58:46.5ÂÂÂ 0ÂÂÂ 300k
25:03.7ÂÂÂ 13.158ÂÂÂ 59:44.5ÂÂÂ 4.73ÂÂÂ 300k
26:01.1ÂÂÂ 2.486ÂÂÂ 00:42.6ÂÂÂ 0ÂÂÂ 300k
26:59.1ÂÂÂ 11.241ÂÂÂ 01:40.6ÂÂÂ 0ÂÂÂ 300k
27:57.2ÂÂÂ 20.521ÂÂÂ 02:38.6ÂÂÂ 0ÂÂÂ 300k
28:55.2ÂÂÂ 30.094ÂÂÂ 03:36.7ÂÂÂ 0ÂÂÂ 300k
29:53.3ÂÂÂ 16.828ÂÂÂ 04:34.7ÂÂÂ 0.963ÂÂÂ 300k
30:51.3ÂÂÂ 46.916ÂÂÂ 05:32.8ÂÂÂ 0ÂÂÂ 400k
31:49.3ÂÂÂ 56.214ÂÂÂ 05:32.8ÂÂÂ 0ÂÂÂ 400k
32:47.3ÂÂÂ 58.69ÂÂÂ 05:32.8ÂÂÂ 0ÂÂÂ 400k
33:45.3ÂÂÂ 61.486ÂÂÂ 05:32.8ÂÂÂ 0ÂÂÂ 400k
34:43.3ÂÂÂ 72.175ÂÂÂ 05:32.8ÂÂÂ 0.598ÂÂÂ 400k
35:41.3ÂÂÂ 56.699ÂÂÂ 05:32.8ÂÂÂ 0ÂÂÂ 400k
36:39.3ÂÂÂ 61.071ÂÂÂ 05:32.8ÂÂÂ 0ÂÂÂ 400k
37:37.3ÂÂÂ 43.355ÂÂÂ 06:30.8ÂÂÂ 0ÂÂÂ 400k
38:35.4ÂÂÂ 44.644ÂÂÂ 06:30.8ÂÂÂ 0ÂÂÂ 400k
39:33.4ÂÂÂ 72.336ÂÂÂ 06:30.8ÂÂÂ 0ÂÂÂ 400k
40:31.4ÂÂÂ 70.676ÂÂÂ 06:30.8ÂÂÂ 0ÂÂÂ 400k
41:29.4ÂÂÂ 108.009ÂÂÂ 06:30.8ÂÂÂ 0ÂÂÂ 400k
42:27.4ÂÂÂ 65.216ÂÂÂ 06:30.8ÂÂÂ 0ÂÂÂ 400k


Why there're difference in test time? Could you summarize them like:

Test setup: e.g testpmd or pktgen to generate packets to guest

avg packets drop before: XXX

avg packets drop after: YYY(-ZZZ%)

Thanks




Data to prove why the patch helps:

----------------------------------------------------

We did have completed several rounds of test with setting the value to
budget (64 as the default value). It does improve a lot with pps is
below 400pps for a single stream. We are confident that it runs out of free
buffer in avail ring when packet dropping happens with below systemtap:

Just a snippet:

probe module("virtio_ring").function("virtqueue_get_buf")
{
ÂÂÂÂ x = (@cast($_vq, "vring_virtqueue")->vring->used->idx)-
(@cast($_vq, "vring_virtqueue")->last_used_idx) ---> we use this one
to verify if the queue is full, which means guest is not able to take
buffer from the queue timely

ÂÂÂÂ if (x<0 && (x+65535)<4096)
ÂÂÂÂ ÂÂÂ x = x+65535

ÂÂÂÂ if((x==1024) && @cast($_vq, "vring_virtqueue")->vq->callback ==
callback_addr)
ÂÂÂÂ ÂÂÂ netrxcount[x] <<< gettimeofday_s()
}


probe module("virtio_ring").function("virtqueue_add_inbuf")
{
ÂÂÂÂ y = (@cast($vq, "vring_virtqueue")->vring->avail->idx)-
(@cast($vq, "vring_virtqueue")->vring->used->idx) ---> we use this one
to verify if we run out of free buffer in avail ring
ÂÂÂÂ if (y<0 && (y+65535)<4096)
ÂÂÂÂ ÂÂÂ y = y+65535

ÂÂÂÂ if(@2=="debugon")
ÂÂÂÂ {
ÂÂÂÂ ÂÂÂ if(y==0 && @cast($vq, "vring_virtqueue")->vq->callback ==
callback_addr)
ÂÂÂÂ ÂÂÂ {
ÂÂÂÂ ÂÂÂ ÂÂÂ netrxfreecount[y] <<< gettimeofday_s()

ÂÂÂÂ ÂÂÂ ÂÂÂ printf("no avail ring left seen, printing most recent 5
num free, vq: %lx, current index: %d\n", $vq, recentfreecount)
ÂÂÂÂ ÂÂÂ ÂÂÂ for(i=recentfreecount; i!=((recentfreecount+4) % 5);
i=((i+1) % 5))
ÂÂÂÂ ÂÂÂ ÂÂÂ {
ÂÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ printf("index: %d, num free: %d\n", i, recentfree[$vq,
i])
ÂÂÂÂ ÂÂÂ ÂÂÂ }

ÂÂÂÂ ÂÂÂ ÂÂÂ printf("index: %d, num free: %d\n", i, recentfree[$vq, i])
ÂÂÂÂ ÂÂÂ ÂÂÂ //exit()
ÂÂÂÂ ÂÂÂ }
ÂÂÂÂ }
}


probe
module("virtio_net").statement("virtnet_receive@drivers/net/virtio_net.c:732")

{
ÂÂÂÂ recentfreecount++
ÂÂÂÂ recentfreecount = recentfreecount % 5
ÂÂÂÂ recentfree[$rq->vq, recentfreecount] = $rq->vq->num_free --->
record the num_free for the last 5 calls to virtnet_receive, so we can
see if lowering the bar helps.
}


Here is the result:

no avail ring left seen, printing most recent 5 num free, vq:
ffff9c13c1200000, current index: 1
index: 1, num free: 561
index: 2, num free: 305
index: 3, num free: 369
index: 4, num free: 433
index: 0, num free: 497
no avail ring left seen, printing most recent 5 num free, vq:
ffff9c13c1200000, current index: 1
index: 1, num free: 543
index: 2, num free: 463
index: 3, num free: 469
index: 4, num free: 476
index: 0, num free: 479
no avail ring left seen, printing most recent 5 num free, vq:
ffff9c13c1200000, current index: 2
index: 2, num free: 555
index: 3, num free: 414
index: 4, num free: 420
index: 0, num free: 427
index: 1, num free: 491

We can see in the last 4 calls to virtnet_receive before we run out
of free buffer and start to relaim, num_free is quite high. So if we
can do the reclaim earlier, it will certainly help.

Jiang


Right, but I think there's no need to put those thing in the commit log.

Thanks