Hi Sunil,
> -----Original Message-----
> From: Pai G, Sunil <[email protected]>
> Sent: Monday, March 28, 2022 2:08 PM
> To: Wang, YuanX <[email protected]>; [email protected];
> Xia, Chenbo <[email protected]>
> Cc: [email protected]; Hu, Jiayu <[email protected]>; Ding, Xuan
> <[email protected]>; Yang, YvonneX <[email protected]>
> 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-
> [email protected]/
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
