RE: [RFC v2 1/2] vhost: support clear in-flight packets for async dequeue

["Wang, YuanX" <yuanx.wang@intel.com>]

Hi Sunil,

> -----Original Message-----
> From: Pai G, Sunil <sunil.pai.g@intel.com>
> Sent: Monday, March 28, 2022 2:08 PM
> To: Wang, YuanX <yuanx.wang@intel.com>; maxime.coquelin@redhat.com;
> Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>
> Subject: RE: [RFC v2 1/2] vhost: support clear in-flight packets for async
> dequeue
> 
> Hi Yuan,
> 
> Thanks for the patch, comment inline.
> 
> > +uint16_t
> > +rte_vhost_clear_queue(int vid, uint16_t queue_id, struct rte_mbuf
> **pkts,
> > +		uint16_t count, int16_t dma_id, uint16_t vchan_id) {
> > +	struct virtio_net *dev = get_device(vid);
> > +	struct vhost_virtqueue *vq;
> > +	uint16_t n_pkts_cpl = 0;
> > +
> > +	if (!dev)
> > +		return 0;
> > +
> > +	VHOST_LOG_DATA(DEBUG, "(%s) %s\n", dev->ifname, __func__);
> > +	if (unlikely(queue_id >= dev->nr_vring)) {
> > +		VHOST_LOG_DATA(ERR, "(%s) %s: invalid virtqueue
> idx %d.\n",
> > +			dev->ifname, __func__, queue_id);
> > +		return 0;
> > +	}
> > +
> > +	vq = dev->virtqueue[queue_id];
> > +
> 
> I think the following checks must be protected by spinlock.
> Similar to :
> https://patches.dpdk.org/project/dpdk/patch/20220328020754.1155063-1-
> jiayu.hu@intel.com/

Thanks for the comment.
Will fix it in next version.

Thanks,
Yuan

> 
> > +	if (unlikely(!vq->async)) {
> > +		VHOST_LOG_DATA(ERR, "(%s) %s: async not registered for
> queue
> > id %d.\n",
> > +			dev->ifname, __func__, queue_id);
> > +		return 0;
> > +	}
> > +
> > +	if (unlikely(!dma_copy_track[dma_id].vchans ||
> > +
> > 	!dma_copy_track[dma_id].vchans[vchan_id].pkts_cmpl_flag_addr))
> {
> > +		VHOST_LOG_DATA(ERR, "(%s) %s: invalid channel %d:%u.\n",
> dev-
> > >ifname, __func__,
> > +				dma_id, vchan_id);
> > +		return 0;
> > +	}
> > +
> > +	if (!rte_spinlock_trylock(&vq->access_lock)) {
> > +		VHOST_LOG_DATA(ERR,
> > +			"(%d) %s: failed to clear async queue id %d,
> virtqueue
> > busy.\n",
> > +			dev->vid, __func__, queue_id);
> > +		return 0;
> > +	}
> > +
> 
> <snipped>
> 
> Thanks and regards,
> Sunil