Re: [PATCH] vhost-net: set packet weight of tx polling to 2 * vq size

[Michael S. Tsirkin]

On Fri, Apr 06, 2018 at 08:22:37AM +0000, haibinzhang(åææ) wrote:
> handle_tx will delay rx for tens or even hundreds of milliseconds when tx busy
> polling udp packets with small length(e.g. 1byte udp payload), because setting
> VHOST_NET_WEIGHT takes into account only sent-bytes but no single packet length.
> 
> Ping-Latencies shown below were tested between two Virtual Machines using
> netperf (UDP_STREAM, len=1), and then another machine pinged the client:
> 
> Packet-Weight      Ping-Latencies(millisecond)
>                    min      avg       max
> Origin           3.319   18.489    57.303
> 64               1.643    2.021     2.552
> 128              1.825    2.600     3.224
> 256              1.997    2.710     4.295
> 512              1.860    3.171     4.631
> 1024             2.002    4.173     9.056
> 2048             2.257    5.650     9.688
> 4096             2.093    8.508    15.943

And this is with Q size 256 right?

> Ring size is a hint from device about a burst size it can tolerate. Based on
> benchmarks, set the weight to 2 * vq size.
> 
> To evaluate this change, another tests were done using netperf(RR, TX) between
> two machines with Intel(R) Xeon(R) Gold 6133 CPU @ 2.50GHz, and vq size was
> tweaked through qemu. Results shown below does not show obvious changes.

What I asked for is ping-latency with different VQ sizes,
streaming below does not show anything.

> vq size=256 TCP_RR                vq size=512 TCP_RR
> size/sessions/+thu%/+normalize%   size/sessions/+thu%/+normalize%
>    1/       1/  -7%/        -2%      1/       1/   0%/        -2%
>    1/       4/  +1%/         0%      1/       4/  +1%/         0%
>    1/       8/  +1%/        -2%      1/       8/   0%/        +1%
>   64/       1/  -6%/         0%     64/       1/  +7%/        +3%
>   64/       4/   0%/        +2%     64/       4/  -1%/        +1%
>   64/       8/   0%/         0%     64/       8/  -1%/        -2%
>  256/       1/  -3%/        -4%    256/       1/  -4%/        -2%
>  256/       4/  +3%/        +4%    256/       4/  +1%/        +2%
>  256/       8/  +2%/         0%    256/       8/  +1%/        -1%
> 
> vq size=256 UDP_RR                vq size=512 UDP_RR
> size/sessions/+thu%/+normalize%   size/sessions/+thu%/+normalize%
>    1/       1/  -5%/        +1%      1/       1/  -3%/        -2%
>    1/       4/  +4%/        +1%      1/       4/  -2%/        +2%
>    1/       8/  -1%/        -1%      1/       8/  -1%/         0%
>   64/       1/  -2%/        -3%     64/       1/  +1%/        +1%
>   64/       4/  -5%/        -1%     64/       4/  +2%/         0%
>   64/       8/   0%/        -1%     64/       8/  -2%/        +1%
>  256/       1/  +7%/        +1%    256/       1/  -7%/         0%
>  256/       4/  +1%/        +1%    256/       4/  -3%/        -4%
>  256/       8/  +2%/        +2%    256/       8/  +1%/        +1%
> 
> vq size=256 TCP_STREAM            vq size=512 TCP_STREAM
> size/sessions/+thu%/+normalize%   size/sessions/+thu%/+normalize%
>   64/       1/   0%/        -3%     64/       1/   0%/         0%
>   64/       4/  +3%/        -1%     64/       4/  -2%/        +4%
>   64/       8/  +9%/        -4%     64/       8/  -1%/        +2%
>  256/       1/  +1%/        -4%    256/       1/  +1%/        +1%
>  256/       4/  -1%/        -1%    256/       4/  -3%/         0%
>  256/       8/  +7%/        +5%    256/       8/  -3%/         0%
>  512/       1/  +1%/         0%    512/       1/  -1%/        -1%
>  512/       4/  +1%/        -1%    512/       4/   0%/         0%
>  512/       8/  +7%/        -5%    512/       8/  +6%/        -1%
> 1024/       1/   0%/        -1%   1024/       1/   0%/        +1%
> 1024/       4/  +3%/         0%   1024/       4/  +1%/         0%
> 1024/       8/  +8%/        +5%   1024/       8/  -1%/         0%
> 2048/       1/  +2%/        +2%   2048/       1/  -1%/         0%
> 2048/       4/  +1%/         0%   2048/       4/   0%/        -1%
> 2048/       8/  -2%/         0%   2048/       8/   5%/        -1%
> 4096/       1/  -2%/         0%   4096/       1/  -2%/         0%
> 4096/       4/  +2%/         0%   4096/       4/   0%/         0%
> 4096/       8/  +9%/        -2%   4096/       8/  -5%/        -1%
> 
> Signed-off-by: Haibin Zhang <haibinzhang@xxxxxxxxxxx>
> Signed-off-by: Yunfang Tai <yunfangtai@xxxxxxxxxxx>
> Signed-off-by: Lidong Chen <lidongchen@xxxxxxxxxxx>

Code is fine but I'd like to see validation of the heuristic
2*vq->num with another vq size.



> ---
>  drivers/vhost/net.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 8139bc70ad7d..3563a305cc0a 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -44,6 +44,10 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
>   * Using this limit prevents one virtqueue from starving others. */
>  #define VHOST_NET_WEIGHT 0x80000
>  
> +/* Max number of packets transferred before requeueing the job.
> + * Using this limit prevents one virtqueue from starving rx. */
> +#define VHOST_NET_PKT_WEIGHT(vq) ((vq)->num * 2)
> +
>  /* MAX number of TX used buffers for outstanding zerocopy */
>  #define VHOST_MAX_PEND 128
>  #define VHOST_GOODCOPY_LEN 256
> @@ -473,6 +477,7 @@ static void handle_tx(struct vhost_net *net)
>  	struct socket *sock;
>  	struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
>  	bool zcopy, zcopy_used;
> +	int sent_pkts = 0;
>  
>  	mutex_lock(&vq->mutex);
>  	sock = vq->private_data;
> @@ -580,7 +585,8 @@ static void handle_tx(struct vhost_net *net)
>  		else
>  			vhost_zerocopy_signal_used(net, vq);
>  		vhost_net_tx_packet(net);
> -		if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
> +		if (unlikely(total_len >= VHOST_NET_WEIGHT) ||
> +		    unlikely(++sent_pkts >= VHOST_NET_PKT_WEIGHT(vq))) {
>  			vhost_poll_queue(&vq->poll);
>  			break;
>  		}
> -- 
> 2.12.3
>