Re: [dpdk-dev] [PATCH v3 07/21] net/virtio: implement transmit path for packed queues

[Tiwei Bie <tiwei.bie@intel.com>]

On Thu, Apr 05, 2018 at 12:10:17PM +0200, Jens Freimann wrote:
[...]
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index a8aa87b32..9f9b5a8f8 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -38,6 +38,101 @@
>  #define  VIRTIO_DUMP_PACKET(m, len) do { } while (0)
>  #endif
>  
> +#define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
> +	ETH_TXQ_FLAGS_NOOFFLOADS)

Why add this?


> +
> +/* Cleanup from completed transmits. */
> +static void
> +virtio_xmit_cleanup_packed(struct virtqueue *vq)
> +{
> +	uint16_t idx;
> +	uint16_t size = vq->vq_nentries;
> +	struct vring_desc_packed *desc = vq->vq_ring.desc_packed;
> +
> +	idx = vq->vq_used_cons_idx & (size - 1);
> +	while (desc_is_used(&desc[idx]) &&
> +	       vq->vq_free_cnt < size) {
> +		vq->vq_free_cnt++;
> +		idx = ++vq->vq_used_cons_idx & (size - 1);

Driver needs to keep track of the number of descriptors
to be skipped for each used descriptor.


> +	}
> +}
> +
> +uint16_t
> +virtio_xmit_pkts_packed(void *tx_queue, struct rte_mbuf **tx_pkts,
> +		     uint16_t nb_pkts)
> +{
[...]
> +}
>  int
>  virtio_dev_rx_queue_done(void *rxq, uint16_t offset)
>  {

Please add an empty line between the functions.


> @@ -547,6 +642,10 @@ virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev,
>  
>  	PMD_INIT_FUNC_TRACE();
>  
> +	if (vtpci_packed_queue(hw)) {
> +		vq->vq_ring.avail_wrap_counter = 1;

virtio_dev_tx_queue_setup_finish() will be called during a
dev_stop()/dev_start(). The problem is that, the dev_stop()
doesn't really stop the device. So we can't reset the wrap
counter to 1 in dev_start().


> +	}
> +
>  	if (hw->use_simple_tx) {
>  		for (desc_idx = 0; desc_idx < mid_idx; desc_idx++) {
>  			vq->vq_ring.avail->ring[desc_idx] =
> @@ -567,7 +666,8 @@ virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev,
>  			vq->vq_ring.avail->ring[desc_idx] = desc_idx;
>  	}
>  
> -	VIRTQUEUE_DUMP(vq);
> +	if (!vtpci_packed_queue(hw))

The check isn't needed.


> +		VIRTQUEUE_DUMP(vq);
>  
>  	return 0;
>  }
> -- 
> 2.14.3
>