On Wed, May 04, 2016 at 08:50:27AM +0800, Huawei Xie wrote:
> -int virtio_dev_queue_setup(struct rte_eth_dev *dev,
> - int queue_type,
> - uint16_t queue_idx,
> +static int
> +virtio_dev_cq_queue_setup(struct rte_eth_dev *dev,
While it's good to split Rx/Tx specific stuff, but why are you trying to
remove a common queue_setup function that does common setups, such as vring
memory allocation.
This results to much duplicated code: following diff summary also shows
it clearly:
7 files changed, 655 insertions(+), 422 deletions(-)
which makes it harder for maintaining.
> -}
> + rxvq = (struct virtnet_rx *)RTE_PTR_ADD(vq,
> + sizeof(*vq) + vq_size * sizeof(struct vq_desc_extra));
> + rxvq->vq = vq;
> + vq->sw_ring = sw_ring;
sw_ring is needed for rx queue only, why not moving it to rx queue struct?
> static void
> -virtio_update_packet_stats(struct virtqueue *vq, struct rte_mbuf *mbuf)
> +virtio_update_rxq_stats(struct virtnet_rx *rxvq, struct rte_mbuf *mbuf)
> {
> uint32_t s = mbuf->pkt_len;
> struct ether_addr *ea;
>
> if (s == 64) {
> - vq->size_bins[1]++;
> + rxvq->stats.size_bins[1]++;
> } else if (s > 64 && s < 1024) {
> uint32_t bin;
>
> /* count zeros, and offset into correct bin */
> bin = (sizeof(s) * 8) - __builtin_clz(s) - 5;
> - vq->size_bins[bin]++;
> + rxvq->stats.size_bins[bin]++;
> } else {
> if (s < 64)
> - vq->size_bins[0]++;
> + rxvq->stats.size_bins[0]++;
> else if (s < 1519)
> - vq->size_bins[6]++;
> + rxvq->stats.size_bins[6]++;
> else if (s >= 1519)
> - vq->size_bins[7]++;
> + rxvq->stats.size_bins[7]++;
> }
>
> ea = rte_pktmbuf_mtod(mbuf, struct ether_addr *);
> if (is_multicast_ether_addr(ea)) {
> if (is_broadcast_ether_addr(ea))
> - vq->broadcast++;
> + rxvq->stats.broadcast++;
> else
> - vq->multicast++;
> + rxvq->stats.multicast++;
> + }
> +}
> +
> +static void
> +virtio_update_txq_stats(struct virtnet_tx *txvq, struct rte_mbuf *mbuf)
Why not taking "struct virtnet_stats *stats" as the arg, so that we
don't have to implment two exactly same functions.
> diff --git a/drivers/net/virtio/virtio_rxtx.h
> b/drivers/net/virtio/virtio_rxtx.h
> index a76c3e5..ced55a3 100644
> --- a/drivers/net/virtio/virtio_rxtx.h
> +++ b/drivers/net/virtio/virtio_rxtx.h
> @@ -34,7 +34,59 @@
> #define RTE_PMD_VIRTIO_RX_MAX_BURST 64
>
> #ifdef RTE_MACHINE_CPUFLAG_SSSE3
> -int virtio_rxq_vec_setup(struct virtqueue *rxq);
> +
> +struct virtnet_stats {
Another remind again: you should put following codes before the
"#ifdef".
--yliu
