On Thu, Mar 03, 2016 at 05:40:14PM +0000, Xie, Huawei wrote:
> On 2/18/2016 9:48 PM, Yuanhan Liu wrote:
> > The current rte_vhost_dequeue_burst() implementation is a bit messy
> [...]
> > +
> >  uint16_t
> >  rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
> >     struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count)
> >  {
> > -   struct rte_mbuf *m, *prev;
> >     struct vhost_virtqueue *vq;
> > -   struct vring_desc *desc;
> > -   uint64_t vb_addr = 0;
> > -   uint64_t vb_net_hdr_addr = 0;
> > -   uint32_t head[MAX_PKT_BURST];
> > +   uint32_t desc_indexes[MAX_PKT_BURST];
> 
> indices

http://dictionary.reference.com/browse/index

    index
    noun, plural indexes, indices 

> 
> 
> >     uint32_t used_idx;
> >     uint32_t i;
> > -   uint16_t free_entries, entry_success = 0;
> > +   uint16_t free_entries;
> >     uint16_t avail_idx;
> > -   struct virtio_net_hdr *hdr = NULL;
> > +   struct rte_mbuf *m;
> >  
> >     if (unlikely(!is_valid_virt_queue_idx(queue_id, 1, dev->virt_qp_nb))) {
> >             RTE_LOG(ERR, VHOST_DATA,
> > @@ -730,197 +813,49 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, 
> > uint16_t queue_id,
> >             return 0;
> >  
> > -           if (entry_success < (free_entries - 1)) {
> > -                   /* Prefetch descriptor index. */
> > -                   rte_prefetch0(&vq->desc[head[entry_success+1]]);
> > -                   rte_prefetch0(&vq->used->ring[(used_idx + 1) & 
> > (vq->size - 1)]);
> > -           }
> 
> Why is this prefetch silently dropped in the patch?

Oops, good catching. Will fix it. Thanks.


> >                     break;
> > +           pkts[i] = m;
> >  
> > -           m->nb_segs = seg_num;
> > -           if ((hdr->flags != 0) || (hdr->gso_type != 
> > VIRTIO_NET_HDR_GSO_NONE))
> > -                   vhost_dequeue_offload(hdr, m);
> > -
> > -           pkts[entry_success] = m;
> > -           vq->last_used_idx++;
> > -           entry_success++;
> > +           used_idx = vq->last_used_idx++ & (vq->size - 1);
> > +           vq->used->ring[used_idx].id  = desc_indexes[i];
> > +           vq->used->ring[used_idx].len = 0;
> 
> What is the correct value for ring[used_idx].len,  the packet length or 0?

Good question. I didn't notice that before. Sounds buggy to me. However,
that's from the old code. Will check it.

        --yliu

Reply via email to