On Wed, 2016-08-24 at 12:36 +0200, Christophe Leroy wrote:
> Initially, a NAPI TX routine has been implemented separately from
> NAPI RX, as done on the freescale/gianfar driver.
>
> By merging NAPI RX and NAPI TX, we reduce the amount of TX completion
> interrupts.
>
> Handling of the budget in association with TX interrupts is based on
> indications provided at https://wiki.linuxfoundation.org/networking/napi
>
> At the same time, we fix an issue in the handling of fep->tx_free:
>
> It is only when fep->tx_free goes up to MAX_SKB_FRAGS that
> we need to wake up the queue. There is no need to call
> netif_wake_queue() at every packet successfully transmitted.
>
> Signed-off-by: Christophe Leroy <christophe.le...@c-s.fr>
> ---
> .../net/ethernet/freescale/fs_enet/fs_enet-main.c | 259
> +++++++++------------
> drivers/net/ethernet/freescale/fs_enet/fs_enet.h | 16 +-
> drivers/net/ethernet/freescale/fs_enet/mac-fcc.c | 57 ++---
> drivers/net/ethernet/freescale/fs_enet/mac-fec.c | 57 ++---
> drivers/net/ethernet/freescale/fs_enet/mac-scc.c | 57 ++---
> 5 files changed, 153 insertions(+), 293 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> index 61fd486..7cd3ef9 100644
> --- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> +++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> @@ -79,8 +79,8 @@ static void skb_align(struct sk_buff *skb, int align)
> skb_reserve(skb, align - off);
> }
>
> -/* NAPI receive function */
> -static int fs_enet_rx_napi(struct napi_struct *napi, int budget)
> +/* NAPI function */
> +static int fs_enet_napi(struct napi_struct *napi, int budget)
> {
> struct fs_enet_private *fep = container_of(napi, struct
> fs_enet_private, napi);
> struct net_device *dev = fep->ndev;
> @@ -90,9 +90,100 @@ static int fs_enet_rx_napi(struct napi_struct *napi, int
> budget)
> int received = 0;
> u16 pkt_len, sc;
> int curidx;
> + int dirtyidx, do_wake, do_restart;
>
> - if (budget <= 0)
> - return received;
> + spin_lock(&fep->tx_lock);
> + bdp = fep->dirty_tx;
> +
> + /* clear status bits for napi*/
> + (*fep->ops->napi_clear_event)(dev);
> +
> + do_wake = do_restart = 0;
> + while (((sc = CBDR_SC(bdp)) & BD_ENET_TX_READY) == 0) {
I am afraid you could live lock here on SMP.
You should make sure you do not loop forever, not assuming cpu is faster
than NIC.
> + dirtyidx = bdp - fep->tx_bd_base;
> +
> + if (fep->tx_free == fep->tx_ring)
> + break;
> +
> + skb = fep->tx_skbuff[dirtyidx];
> +
> + /*
> + * Check for errors.
> + */
> + if (sc & (BD_ENET_TX_HB | BD_ENET_TX_LC |
> + BD_ENET_TX_RL | BD_ENET_TX_UN | BD_ENET_TX_CSL)) {
> +
> + if (sc & BD_ENET_TX_HB) /* No heartbeat */
> + fep->stats.tx_heartbeat_errors++;
> + if (sc & BD_ENET_TX_LC) /* Late collision */
> + fep->stats.tx_window_errors++;
> + if (sc & BD_ENET_TX_RL) /* Retrans limit */
> + fep->stats.tx_aborted_errors++;
> + if (sc & BD_ENET_TX_UN) /* Underrun */
> + fep->stats.tx_fifo_errors++;
> + if (sc & BD_ENET_TX_CSL) /* Carrier lost */
> + fep->stats.tx_carrier_errors++;
> +
> + if (sc & (BD_ENET_TX_LC | BD_ENET_TX_RL |
> BD_ENET_TX_UN)) {
> + fep->stats.tx_errors++;
> + do_restart = 1;
> + }
> + } else
> + fep->stats.tx_packets++;
> +
> + if (sc & BD_ENET_TX_READY) {
> + dev_warn(fep->dev,
> + "HEY! Enet xmit interrupt and TX_READY.\n");
> + }
> +
> + /*
> + * Deferred means some collisions occurred during transmit,
> + * but we eventually sent the packet OK.
> + */
> + if (sc & BD_ENET_TX_DEF)
> + fep->stats.collisions++;
> +
> + /* unmap */
> + if (fep->mapped_as_page[dirtyidx])
> + dma_unmap_page(fep->dev, CBDR_BUFADDR(bdp),
> + CBDR_DATLEN(bdp), DMA_TO_DEVICE);
> + else
> + dma_unmap_single(fep->dev, CBDR_BUFADDR(bdp),
> + CBDR_DATLEN(bdp), DMA_TO_DEVICE);
> +
> + /*
> + * Free the sk buffer associated with this last transmit.
> + */
> + if (skb) {
> + dev_kfree_skb(skb);
> + fep->tx_skbuff[dirtyidx] = NULL;
> + }
> +
> + /*
> + * Update pointer to next buffer descriptor to be transmitted.
> + */
> + if ((sc & BD_ENET_TX_WRAP) == 0)
> + bdp++;
> + else
> + bdp = fep->tx_bd_base;
> +
> + /*
> + * Since we have freed up a buffer, the ring is no longer
> + * full.
> + */
> + if (++fep->tx_free == MAX_SKB_FRAGS)
> + do_wake = 1;
> + }
> +
> + fep->dirty_tx = bdp;
> +
> + if (do_restart)
> + (*fep->ops->tx_restart)(dev);
> +
> + spin_unlock(&fep->tx_lock);
> +
> + if (do_wake)
> + netif_wake_queue(dev);
>
