lkml.org 
[lkml]   [2017]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 012/294] net: bcmgenet: rewrite bcmgenet_rx_refill()
    3.16.50-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Petri Gynther <pgynther@google.com>

    commit d6707bec598649450ee0887bf11896e525777874 upstream.

    Currently, bcmgenet_desc_rx() calls bcmgenet_rx_refill() at the end of
    Rx packet processing loop, after the current Rx packet has already been
    passed to napi_gro_receive(). However, bcmgenet_rx_refill() might fail
    to allocate a new Rx skb, thus leaving a hole on the Rx queue where no
    valid Rx buffer exists.

    To eliminate this situation:
    1. Rewrite bcmgenet_rx_refill() to retain the current Rx skb on the Rx
    queue if a new replacement Rx skb can't be allocated and DMA-mapped.
    In this case, the data on the current Rx skb is effectively dropped.
    2. Modify bcmgenet_desc_rx() to call bcmgenet_rx_refill() at the top of
    Rx packet processing loop, so that the new replacement Rx skb is
    already in place before the current Rx skb is processed.

    Signed-off-by: Petri Gynther <pgynther@google.com>
    Tested-by: Jaedon Shin <jaedon.shin@gmail.com>--
    Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
    Tested-by: Florian Fainelli <f.fainelli@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    [bwh: Backported to 3.16:
    - There's no alloc_rx_buff_failed statistic
    - Adjust context]
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
    +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
    @@ -1275,33 +1275,41 @@ out:
    return ret;
    }

    -
    -static int bcmgenet_rx_refill(struct bcmgenet_priv *priv,
    - struct enet_cb *cb)
    +static struct sk_buff *bcmgenet_rx_refill(struct bcmgenet_priv *priv,
    + struct enet_cb *cb)
    {
    struct device *kdev = &priv->pdev->dev;
    struct sk_buff *skb;
    + struct sk_buff *rx_skb;
    dma_addr_t mapping;
    - int ret;

    + /* Allocate a new Rx skb */
    skb = netdev_alloc_skb(priv->dev,
    priv->rx_buf_len + SKB_ALIGNMENT);
    - if (!skb)
    - return -ENOMEM;
    + if (!skb) {
    + netif_err(priv, rx_err, priv->dev,
    + "%s: Rx skb allocation failed\n", __func__);
    + return NULL;
    + }

    - /* a caller did not release this control block */
    - WARN_ON(cb->skb != NULL);
    - cb->skb = skb;
    - mapping = dma_map_single(kdev, skb->data,
    - priv->rx_buf_len, DMA_FROM_DEVICE);
    - ret = dma_mapping_error(kdev, mapping);
    - if (ret) {
    - bcmgenet_free_cb(cb);
    + /* DMA-map the new Rx skb */
    + mapping = dma_map_single(kdev, skb->data, priv->rx_buf_len,
    + DMA_FROM_DEVICE);
    + if (dma_mapping_error(kdev, mapping)) {
    + dev_kfree_skb_any(skb);
    netif_err(priv, rx_err, priv->dev,
    - "%s DMA map failed\n", __func__);
    - return ret;
    + "%s: Rx skb DMA mapping failed\n", __func__);
    + return NULL;
    }

    + /* Grab the current Rx skb from the ring and DMA-unmap it */
    + rx_skb = cb->skb;
    + if (likely(rx_skb))
    + dma_unmap_single(kdev, dma_unmap_addr(cb, dma_addr),
    + priv->rx_buf_len, DMA_FROM_DEVICE);
    +
    + /* Put the new Rx skb on the ring */
    + cb->skb = skb;
    dma_unmap_addr_set(cb, dma_addr, mapping);
    /* assign packet, prepare descriptor, and advance pointer */

    @@ -1314,7 +1322,8 @@ static int bcmgenet_rx_refill(struct bcm
    priv->rx_bd_assign_ptr = priv->rx_bds +
    (priv->rx_bd_assign_index * DMA_DESC_SIZE);

    - return 0;
    + /* Return the current Rx skb to caller */
    + return rx_skb;
    }

    /* bcmgenet_desc_rx - descriptor based rx process.
    @@ -1329,7 +1338,7 @@ static unsigned int bcmgenet_desc_rx(str
    struct sk_buff *skb;
    u32 dma_length_status;
    unsigned long dma_flag;
    - int len, err;
    + int len;
    unsigned int rxpktprocessed = 0, rxpkttoprocess;
    unsigned int p_index;
    unsigned int chksum_ok = 0;
    @@ -1351,26 +1360,14 @@ static unsigned int bcmgenet_desc_rx(str
    (rxpktprocessed < budget)) {

    cb = &priv->rx_cbs[priv->rx_read_ptr];
    - skb = cb->skb;
    + skb = bcmgenet_rx_refill(priv, cb);

    - /* We do not have a backing SKB, so we do not have a
    - * corresponding DMA mapping for this incoming packet since
    - * bcmgenet_rx_refill always either has both skb and mapping or
    - * none.
    - */
    if (unlikely(!skb)) {
    dev->stats.rx_dropped++;
    dev->stats.rx_errors++;
    - goto refill;
    + goto next;
    }

    - /* Unmap the packet contents such that we can use the
    - * RSV from the 64 bytes descriptor when enabled and save
    - * a 32-bits register read
    - */
    - dma_unmap_single(kdev, dma_unmap_addr(cb, dma_addr),
    - priv->rx_buf_len, DMA_FROM_DEVICE);
    -
    if (!priv->desc_64b_en) {
    dma_length_status = dmadesc_get_length_status(priv,
    priv->rx_bds +
    @@ -1398,10 +1395,10 @@ static unsigned int bcmgenet_desc_rx(str
    "Droping fragmented packet!\n");
    dev->stats.rx_dropped++;
    dev->stats.rx_errors++;
    - dev_kfree_skb_any(cb->skb);
    - cb->skb = NULL;
    - goto refill;
    + dev_kfree_skb_any(skb);
    + goto next;
    }
    +
    /* report errors */
    if (unlikely(dma_flag & (DMA_RX_CRC_ERROR |
    DMA_RX_OV |
    @@ -1420,11 +1417,8 @@ static unsigned int bcmgenet_desc_rx(str
    dev->stats.rx_length_errors++;
    dev->stats.rx_dropped++;
    dev->stats.rx_errors++;
    -
    - /* discard the packet and advance consumer index.*/
    - dev_kfree_skb_any(cb->skb);
    - cb->skb = NULL;
    - goto refill;
    + dev_kfree_skb_any(skb);
    + goto next;
    } /* error packet */

    chksum_ok = (dma_flag & priv->dma_rx_chk_bit) &&
    @@ -1457,15 +1451,9 @@ static unsigned int bcmgenet_desc_rx(str

    /* Notify kernel */
    napi_gro_receive(&priv->napi, skb);
    - cb->skb = NULL;
    netif_dbg(priv, rx_status, dev, "pushed up to kernel\n");

    - /* refill RX path on the current control block */
    -refill:
    - err = bcmgenet_rx_refill(priv, cb);
    - if (err)
    - netif_err(priv, rx_err, dev, "Rx refill failed\n");
    -
    +next:
    rxpktprocessed++;
    priv->rx_read_ptr++;
    priv->rx_read_ptr &= (priv->num_rx_bds - 1);
    @@ -1478,7 +1466,7 @@ refill:
    static int bcmgenet_alloc_rx_buffers(struct bcmgenet_priv *priv)
    {
    struct enet_cb *cb;
    - int ret = 0;
    + struct sk_buff *skb;
    int i;

    netif_dbg(priv, hw, priv->dev, "%s:\n", __func__);
    @@ -1486,16 +1474,14 @@ static int bcmgenet_alloc_rx_buffers(str
    /* loop here for each buffer needing assign */
    for (i = 0; i < priv->num_rx_bds; i++) {
    cb = &priv->rx_cbs[priv->rx_bd_assign_index];
    - if (cb->skb)
    - continue;
    -
    - ret = bcmgenet_rx_refill(priv, cb);
    - if (ret)
    - break;
    -
    + skb = bcmgenet_rx_refill(priv, cb);
    + if (skb)
    + dev_kfree_skb_any(skb);
    + if (!cb->skb)
    + return -ENOMEM;
    }

    - return ret;
    + return 0;
    }

    static void bcmgenet_free_rx_buffers(struct bcmgenet_priv *priv)
    \
     
     \ /
      Last update: 2017-11-08 20:59    [W:4.197 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site