On 04/06/2017 08:31 PM, Benjamin Herrenschmidt wrote:
> Add NETIF_F_SG and create multiple TX ring entries for skb fragments.
>
> On reclaim, the skb is only freed on the segment marked as "last".
>
> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
>
[snip]
>
> - dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
> + if (skb_shinfo(skb)->nr_frags == 0 && len < ETH_ZLEN)
> + len = ETH_ZLEN;
This is where skb_put_padto() would help you eliminate this test since
you'd be dealing skb->len >= ETH_ZLEN.
> + dma_unmap_single(priv->dev, map, len, DMA_TO_DEVICE);
> + } else {
> + dma_unmap_page(priv->dev, map,
> + ftgmac100_txdes_get_buffer_size(txdes),
> + DMA_TO_DEVICE);
> + }
>
> - dev_kfree_skb(skb);
> + if (ftgmac100_txdes_get_last_segment(txdes))
> + dev_kfree_skb(skb);
This makes you do an uncached access to the descriptor, right? is there
a way you could use bookeeping information to free the last fragment?
> priv->tx_skbs[pointer] = NULL;
>
> /* Clear txdes0 except end of ring bit, clear txdes1 as we
> @@ -623,10 +642,9 @@ static void ftgmac100_tx_complete(struct ftgmac100 *priv)
> static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
> struct net_device *netdev)
> {
> - unsigned int len = (skb->len < ETH_ZLEN) ? ETH_ZLEN : skb->len;
> struct ftgmac100 *priv = netdev_priv(netdev);
> - struct ftgmac100_txdes *txdes;
> - unsigned int pointer;
> + struct ftgmac100_txdes *txdes, *first;
> + unsigned int pointer, nfrags, len, i, j;
> dma_addr_t map;
>
> /* The HW doesn't pad small frames */
> @@ -642,26 +660,35 @@ static int ftgmac100_hard_start_xmit(struct sk_buff
> *skb,
> goto drop;
> }
>
> - map = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
> DMA_TO_DEVICE);
> - if (unlikely(dma_mapping_error(priv->dev, map))) {
> - /* drop packet */
> + /* Do we have a limit on #fragments ? I yet have to get a reply
> + * from Aspeed. If there's one I haven't hit it.
> + */
> + nfrags = skb_shinfo(skb)->nr_frags;
> +
> + /* Get header len and pad for non-fragmented packets */
> + len = skb_headlen(skb);
> + if (nfrags == 0 && len < ETH_ZLEN)
> + len = ETH_ZLEN;
Same here skb_put_padto() would eliminate the test.
[snip]
>
> + dma_err:
> + if (net_ratelimit())
> + netdev_err(netdev, "map tx fragment failed\n");
You may consider adding a software counter that tracks mapping failures
(few drivers do that) in a subsequent set of changes.
--
Florian
