[dpdk-dev] [PATCH v2 03/14] net/avf: enable queue and device

[Ferruh Yigit]

On 11/23/2017 10:33 PM, Jingjing Wu wrote:
> enable device and queue setup ops like:
> 
>  - dev_configure
>  - dev_start
>  - dev_stop
>  - dev_close
>  - dev_infos_get
>  - rx_queue_start
>  - rx_queue_stop
>  - tx_queue_start
>  - tx_queue_stop
>  - rx_queue_setup
>  - rx_queue_release
>  - tx_queue_setup
>  - tx_queue_release
> 
> Signed-off-by: Jingjing Wu <jingjing.wu at intel.com>

<...>

> +/* HW desc structure, both 16-byte and 32-byte types are supported */
> +#ifdef RTE_LIBRTE_AVF_16BYTE_RX_DESC

Do you want to add this config option in this patch?

<...>

> +/* Structure associated with each Rx queue. */
> +struct avf_rx_queue {
> +	struct rte_mempool *mp;       /* mbuf pool to populate Rx ring */
> +	const struct rte_memzone *mz; /* memzone for Rx ring */
> +	volatile union avf_rx_desc *rx_ring; /* Rx ring virtual address */
> +	uint64_t rx_ring_phys_addr;   /* Rx ring DMA address */
> +	struct rte_mbuf **sw_ring;     /* address of SW ring */
> +	uint16_t nb_rx_desc;          /* ring length */
> +	uint16_t rx_tail;             /* current value of tail */
> +	volatile uint8_t *qrx_tail;   /* register address of tail */
> +	uint16_t rx_free_thresh;      /* max free RX desc to hold */
> +	uint16_t nb_rx_hold;          /* number of held free RX desc */
> +	struct rte_mbuf *pkt_first_seg; /* first segment of current packet */
> +	struct rte_mbuf *pkt_last_seg;  /* last segment of current packet */
> +	struct rte_mbuf fake_mbuf;      /* dummy mbuf */
> +
> +	uint8_t port_id;        /* device port ID */

If this is ethdev port_id, this needs to be 16bits now.

<...>

> +/* Structure associated with each TX queue. */
> +struct avf_tx_queue {
> +	const struct rte_memzone *mz;  /* memzone for Tx ring */
> +	volatile struct avf_tx_desc *tx_ring; /* Tx ring virtual address */
> +	uint64_t tx_ring_phys_addr;    /* Tx ring DMA address */
> +	struct avf_tx_entry *sw_ring;  /* address array of SW ring */
> +	uint16_t nb_tx_desc;           /* ring length */
> +	uint16_t tx_tail;              /* current value of tail */
> +	volatile uint8_t *qtx_tail;    /* register address of tail */
> +	uint16_t nb_used;              /* number of used desc since RS bit set */
> +	uint16_t nb_free;
> +	uint16_t last_desc_cleaned;    /* last desc have been cleaned*/
> +	uint16_t free_thresh;
> +	uint16_t rs_thresh;
> +
> +	uint8_t port_id;

Same here.

<...>

> +
> +#ifdef RTE_LIBRTE_AVF_RX_DUMP
> +#define AVF_DUMP_RX_DESC(rxq, desc, rx_id) \
> +	avf_dump_rx_descriptor(rxq, desc, rx_id);
> +#else
> +#define AVF_DUMP_RX_DESC(rxq, desc, rx_id) do { } while (0)
> +#endif
> +
> +#ifdef RTE_LIBRTE_AVF_TX_DUMP
> +#define AVF_DUMP_TX_DESC(txq, desc, tx_id) \
> +	avf_dump_tx_descriptor(txq, desc, tx_id);
> +#else
> +#define AVF_DUMP_TX_DESC(txq, desc, tx_id) do { } while (0)
> +#endif

These are not defined anywhere and will be replaced in next patch, so why not
completely removed in this patch, and add correct one in next patch?

<...>