[dpdk-dev] [PATCH v2 1/6] vhost: rewrite enqueue
On Thu, Aug 18, 2016 at 02:33:06AM -0400, Zhihong Wang wrote: > This patch implements the vhost logic from scratch into a single function > designed for high performance and better maintainability. > > Signed-off-by: Zhihong Wang > --- > lib/librte_vhost/vhost_rxtx.c | 212 > -- > 1 file changed, 205 insertions(+), 7 deletions(-) > > diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c > index 08a73fd..8e6d782 100644 > --- a/lib/librte_vhost/vhost_rxtx.c > +++ b/lib/librte_vhost/vhost_rxtx.c > @@ -91,7 +91,7 @@ is_valid_virt_queue_idx(uint32_t idx, int is_tx, uint32_t > qp_nb) > return (is_tx ^ (idx & 1)) == 0 && idx < qp_nb * VIRTIO_QNUM; > } > > -static void > +static inline void __attribute__((always_inline)) > virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr > *net_hdr) > { > if (m_buf->ol_flags & PKT_TX_L4_MASK) { > @@ -533,19 +533,217 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t > queue_id, > return pkt_idx; > } > > +static inline uint32_t __attribute__((always_inline)) > +loop_check(struct vhost_virtqueue *vq, uint16_t avail_idx, uint32_t pkt_left) > +{ > + if (pkt_left == 0 || avail_idx == vq->last_used_idx) > + return 1; > + > + return 0; > +} Hmmm, I don't see any benifit from making such simple check into a function. > +static inline uint32_t __attribute__((always_inline)) > +enqueue_packet(struct virtio_net *dev, struct vhost_virtqueue *vq, > + uint16_t avail_idx, struct rte_mbuf *mbuf, > + uint32_t is_mrg_rxbuf) > +{ > + struct virtio_net_hdr_mrg_rxbuf *virtio_hdr; > + struct vring_desc *desc; > + uint64_t desc_host_write_addr = 0; > + uint32_t desc_chain_head = 0; > + uint32_t desc_chain_len = 0; > + uint32_t desc_current = 0; > + uint32_t desc_write_offset = 0; > + uint32_t mbuf_len = 0; > + uint32_t mbuf_len_left = 0; > + uint32_t copy_len = 0; The dequeue function uses var like desc_addr, desc_avail, desc_offset, mbuf_avail, ..., I see no reason to use something different here. This breaks the code consistency. Besides that, var name like desc_host_write_addr looks redundant; desc_addr is much cleaner. --yliu
[dpdk-dev] [PATCH v2 1/6] vhost: rewrite enqueue
> -Original Message- > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com] > Sent: Friday, August 19, 2016 10:39 AM > To: Wang, Zhihong > Cc: dev at dpdk.org; maxime.coquelin at redhat.com > Subject: Re: [PATCH v2 1/6] vhost: rewrite enqueue > > On Thu, Aug 18, 2016 at 02:33:06AM -0400, Zhihong Wang wrote: > > This patch implements the vhost logic from scratch into a single function > > designed for high performance and better maintainability. > > > > Signed-off-by: Zhihong Wang > > --- > > lib/librte_vhost/vhost_rxtx.c | 212 > -- > > 1 file changed, 205 insertions(+), 7 deletions(-) > > > > diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c > > index 08a73fd..8e6d782 100644 > > --- a/lib/librte_vhost/vhost_rxtx.c > > +++ b/lib/librte_vhost/vhost_rxtx.c > > @@ -91,7 +91,7 @@ is_valid_virt_queue_idx(uint32_t idx, int is_tx, uint32_t > qp_nb) > > return (is_tx ^ (idx & 1)) == 0 && idx < qp_nb * VIRTIO_QNUM; > > } > > > > -static void > > +static inline void __attribute__((always_inline)) > > virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr > *net_hdr) > > { > > if (m_buf->ol_flags & PKT_TX_L4_MASK) { > > @@ -533,19 +533,217 @@ virtio_dev_merge_rx(struct virtio_net *dev, > uint16_t queue_id, > > return pkt_idx; > > } > > > > +static inline uint32_t __attribute__((always_inline)) > > +loop_check(struct vhost_virtqueue *vq, uint16_t avail_idx, uint32_t > > pkt_left) > > +{ > > + if (pkt_left == 0 || avail_idx == vq->last_used_idx) > > + return 1; > > + > > + return 0; > > +} > > Hmmm, I don't see any benifit from making such simple check into a > function. It's for prefetch code later to be merged into this function. > > > +static inline uint32_t __attribute__((always_inline)) > > +enqueue_packet(struct virtio_net *dev, struct vhost_virtqueue *vq, > > + uint16_t avail_idx, struct rte_mbuf *mbuf, > > + uint32_t is_mrg_rxbuf) > > +{ > > + struct virtio_net_hdr_mrg_rxbuf *virtio_hdr; > > + struct vring_desc *desc; > > + uint64_t desc_host_write_addr = 0; > > + uint32_t desc_chain_head = 0; > > + uint32_t desc_chain_len = 0; > > + uint32_t desc_current = 0; > > + uint32_t desc_write_offset = 0; > > + uint32_t mbuf_len = 0; > > + uint32_t mbuf_len_left = 0; > > + uint32_t copy_len = 0; > > The dequeue function uses var like desc_addr, desc_avail, desc_offset, > mbuf_avail, ..., I see no reason to use something different here. This > breaks the code consistency. Besides that, var name like desc_host_write_addr > looks redundant; desc_addr is much cleaner. Okay. > > --yliu
[dpdk-dev] [PATCH v2 1/6] vhost: rewrite enqueue
This patch implements the vhost logic from scratch into a single function designed for high performance and better maintainability. Signed-off-by: Zhihong Wang --- lib/librte_vhost/vhost_rxtx.c | 212 -- 1 file changed, 205 insertions(+), 7 deletions(-) diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c index 08a73fd..8e6d782 100644 --- a/lib/librte_vhost/vhost_rxtx.c +++ b/lib/librte_vhost/vhost_rxtx.c @@ -91,7 +91,7 @@ is_valid_virt_queue_idx(uint32_t idx, int is_tx, uint32_t qp_nb) return (is_tx ^ (idx & 1)) == 0 && idx < qp_nb * VIRTIO_QNUM; } -static void +static inline void __attribute__((always_inline)) virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr *net_hdr) { if (m_buf->ol_flags & PKT_TX_L4_MASK) { @@ -533,19 +533,217 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, return pkt_idx; } +static inline uint32_t __attribute__((always_inline)) +loop_check(struct vhost_virtqueue *vq, uint16_t avail_idx, uint32_t pkt_left) +{ + if (pkt_left == 0 || avail_idx == vq->last_used_idx) + return 1; + + return 0; +} + +static inline uint32_t __attribute__((always_inline)) +enqueue_packet(struct virtio_net *dev, struct vhost_virtqueue *vq, + uint16_t avail_idx, struct rte_mbuf *mbuf, + uint32_t is_mrg_rxbuf) +{ + struct virtio_net_hdr_mrg_rxbuf *virtio_hdr; + struct vring_desc *desc; + uint64_t desc_host_write_addr = 0; + uint32_t desc_chain_head = 0; + uint32_t desc_chain_len = 0; + uint32_t desc_current = 0; + uint32_t desc_write_offset = 0; + uint32_t mbuf_len = 0; + uint32_t mbuf_len_left = 0; + uint32_t copy_len = 0; + uint32_t extra_buffers = 0; + uint32_t used_idx_round = 0; + + /* start with the first mbuf of the packet */ + mbuf_len = rte_pktmbuf_data_len(mbuf); + mbuf_len_left = mbuf_len; + + /* get the current desc */ + desc_current = vq->avail->ring[(vq->last_used_idx) & (vq->size - 1)]; + desc_chain_head = desc_current; + desc = >desc[desc_current]; + desc_host_write_addr = gpa_to_vva(dev, desc->addr); + if (unlikely(!desc_host_write_addr)) + goto error; + + /* handle virtio header */ + virtio_hdr = (struct virtio_net_hdr_mrg_rxbuf *) + (uintptr_t)desc_host_write_addr; + memset((void *)(uintptr_t)&(virtio_hdr->hdr), + 0, dev->vhost_hlen); + virtio_enqueue_offload(mbuf, &(virtio_hdr->hdr)); + vhost_log_write(dev, desc->addr, dev->vhost_hlen); + desc_write_offset = dev->vhost_hlen; + desc_chain_len = desc_write_offset; + desc_host_write_addr += desc_write_offset; + if (is_mrg_rxbuf) + virtio_hdr->num_buffers = 1; + + /* start copy from mbuf to desc */ + while (1) { + /* get the next mbuf if the current done */ + if (!mbuf_len_left) { + if (mbuf->next) { + mbuf = mbuf->next; + mbuf_len = rte_pktmbuf_data_len(mbuf); + mbuf_len_left = mbuf_len; + } else + break; + } + + /* get the next desc if the current done */ + if (desc->len <= desc_write_offset) { + if (desc->flags & VRING_DESC_F_NEXT) { + /* go on with the current desc chain */ + desc_write_offset = 0; + desc_current = desc->next; + desc = >desc[desc_current]; + desc_host_write_addr = + gpa_to_vva(dev, desc->addr); + if (unlikely(!desc_host_write_addr)) + goto rollback; + } else if (is_mrg_rxbuf) { + /* start with the next desc chain */ + used_idx_round = vq->last_used_idx + & (vq->size - 1); + vq->used->ring[used_idx_round].id = + desc_chain_head; + vq->used->ring[used_idx_round].len = + desc_chain_len; + vhost_log_used_vring(dev, vq, + offsetof(struct vring_used, + ring[used_idx_round]), + sizeof(vq->used->ring[ + used_idx_round])); + vq->last_used_idx++; + extra_buffers++; +