Messages in this thread | | | Date | Fri, 28 Jul 2023 17:24:19 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH v12 06/10] net: ti: icssg-prueth: Add ICSSG ethernet driver |
| |
On Thu, 27 Jul 2023 16:58:23 +0530 MD Danish Anwar wrote: > +static int emac_tx_complete_packets(struct prueth_emac *emac, int chn, > + int budget) > +{ > + struct net_device *ndev = emac->ndev; > + struct cppi5_host_desc_t *desc_tx; > + struct netdev_queue *netif_txq; > + struct prueth_tx_chn *tx_chn; > + unsigned int total_bytes = 0; > + struct sk_buff *skb; > + dma_addr_t desc_dma; > + int res, num_tx = 0; > + void **swdata; > + > + tx_chn = &emac->tx_chns[chn]; > + > + while (budget) { > + res = k3_udma_glue_pop_tx_chn(tx_chn->tx_chn, &desc_dma); > + if (res == -ENODATA) > + break;
You shouldn't limit the number of serviced packets to budget for Tx NAPI.
https://docs.kernel.org/next/networking/napi.html#driver-api
> + skb->dev = ndev; > + if (!netif_running(skb->dev)) { > + dev_kfree_skb_any(skb); > + return 0; > + }
why do you check if the interface is running? If a packet arrives, it means the interface is running..
> +drop_free_descs: > + prueth_xmit_free(tx_chn, first_desc); > +drop_stop_q: > + netif_tx_stop_queue(netif_txq);
Do not stop the queue on DMA errors. If the queue is empty nothing will wake it up. Queue should only be stopped based on occupancy.
> + dev_kfree_skb_any(skb); > + > + /* error */ > + ndev->stats.tx_dropped++; > + netdev_err(ndev, "tx: error: %d\n", ret); > + > + return ret; -- pw-bot: cr
| |