Re: [PATCH v9 4/5] net/enetfec: add Rx/Tx support

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 11/10/2021 7:48 AM, Apeksha Gupta wrote:
> This patch adds burst enqueue and dequeue operations to the enetfec
> PMD. Loopback mode is also added, compile time flag 'ENETFEC_LOOPBACK' is
> used to enable this feature. By default loopback mode is disabled.
> Basic features added like promiscuous enable, basic stats.
> 

Commit log needs to be updated since 'ENETFEC_LOOPBACK' is not more exists.

> Signed-off-by: Sachin Saxena <sachin.saxena@nxp.com>
> Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>

<...>

> +static int
> +enetfec_eth_link_update(struct rte_eth_dev *dev,
> +			int wait_to_complete __rte_unused)
> +{
> +	struct rte_eth_link link;
> +	unsigned int lstatus = 1;
> +
> +	memset(&link, 0, sizeof(struct rte_eth_link));
> +
> +	link.link_status = lstatus;
> +	link.link_speed = ETH_SPEED_NUM_1G;

Can you please use updated macro: RTE_ETH_SPEED_NUM_1G

<...>

> +/* This function does enetfec_rx_queue processing. Dequeue packet from Rx queue
> + * When update through the ring, just set the empty indicator.
> + */
> +uint16_t
> +enetfec_recv_pkts(void *rxq1, __rte_unused struct rte_mbuf **rx_pkts,

I am sure 'rx_pkts' is used, can drop '__rte_unused'.

> +		uint16_t nb_pkts)
> +{
> +	struct rte_mempool *pool;
> +	struct bufdesc *bdp;
> +	struct rte_mbuf *mbuf, *new_mbuf = NULL;
> +	unsigned short status;
> +	unsigned short pkt_len;
> +	int pkt_received = 0, index = 0;
> +	void *data;
> +	struct enetfec_priv_rx_q *rxq  = (struct enetfec_priv_rx_q *)rxq1;
> +	struct rte_eth_stats *stats = &rxq->fep->stats;
> +	pool = rxq->pool;
> +	bdp = rxq->bd.cur;
> +
> +	/* Process the incoming packet */
> +	status = rte_le_to_cpu_16(rte_read16(&bdp->bd_sc));
> +	while ((status & RX_BD_EMPTY) == 0) {
> +		if (pkt_received >= nb_pkts)
> +			break;
> +
> +		new_mbuf = rte_pktmbuf_alloc(pool);
> +		if (unlikely(new_mbuf == NULL)) {
> +			stats->ierrors++;

'rx_mbuf_alloc_failed' is used to store mbuf alloc failures, not 'ierrors'.

<...>

> +
> +		if (mbuf->nb_segs > 1) {
> +			ENETFEC_PMD_DEBUG("SG not supported");

It is not good idea to use dynamic debug macros in the datapath.
'ENETFEC_DP_LOG()' is the one to use.