Hi Sunil,

> -----Original Message-----
> From: Pai G, Sunil <sunil.pa...@intel.com>
> Sent: Monday, March 28, 2022 2:08 PM
> To: Wang, YuanX <yuanx.w...@intel.com>; maxime.coque...@redhat.com;
> Xia, Chenbo <chenbo....@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu...@intel.com>; Ding, Xuan
> <xuan.d...@intel.com>; Yang, YvonneX <yvonnex.y...@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...@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

Reply via email to