RE: [PATCH v2] vhost: fix avail idx update error when desc copy failed

["Xia, Chenbo" <chenbo.xia@intel.com>]

> -----Original Message-----
> From: Gaoxiang Liu <gaoxiangliu0@163.com>
> Sent: Wednesday, June 22, 2022 9:20 AM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; liugaoxiang@huawei.com; Gaoxiang Liu
> <gaoxiangliu0@163.com>; stable@dpdk.org
> Subject: [PATCH v2] vhost: fix avail idx update error when desc copy
> failed
> 
> When copy_desc_to_mbuf function failed, i added 1.

Function name now is desc_to_mbuf

> And last_avail_idx added i, other than i - 1.
> It may cause that the first mbuf in mbuf-list is dropped,
> the second mbuf in mbuf-list is received in the following
> rx procedure.
> And The pkt_len of the second mbuf is zero, resulting in
> segment fault when parsing the mbuf.

Could you help elaborate more? Do you mean first mbuf len is zero
as it's dropped? And where does the segfault happen? APP? Please
describe more to help understand the issue.

But I do notice one problem here is if vhost APP does not handle
the mbuf array correctly, some packets will be missed in the case
of pkts got dropped in the middle of a burst.

Thanks,
Chenbo

> 
> Fixes: 0fd5608ef97f ("vhost: handle mbuf allocation failure")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Gaoxiang Liu <liugaoxiang@huawei.com>
> 
> ---
> v2:
> * Fixed other idx update errors.
> ---
>  lib/vhost/virtio_net.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> index 68a26eb17d..eb254e1024 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -2850,11 +2850,11 @@ virtio_dev_tx_split(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  	if (dropped)
>  		rte_pktmbuf_free_bulk(&pkts[i - 1], count - i + 1);
> 
> -	vq->last_avail_idx += i;
> +	vq->last_avail_idx += i - dropped;
> 
>  	do_data_copy_dequeue(vq);
> -	if (unlikely(i < count))
> -		vq->shadow_used_idx = i;
> +	if (unlikely((i - dropped) < count))
> +		vq->shadow_used_idx = i - dropped;
>  	if (likely(vq->shadow_used_idx)) {
>  		flush_shadow_used_ring_split(dev, vq);
>  		vhost_vring_call_split(dev, vq);
> --
> 2.32.0