lkml.org 
[lkml]   [2020]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 06/11] net: ethernet: mtk-eth-mac: new driver
    On Wed, 6 May 2020 09:09:52 +0200 Bartosz Golaszewski wrote:
    > > > +}
    > >
    > > Why do you clean the TX ring from a work rather than from the NAPI
    > > context?
    >
    > So this was unclear to me, that's why I went with a workqueue. The
    > budget argument in napi poll is for RX. Should I put some cap on the
    > number of TX descriptors processed in napi context?

    The prevailing wisdom is to not count the TX cleanup as work at all.
    I think the best practice is to first clean up all the TX you can,
    and then do at must @budget of RX.

    Perhaps one day we will come up with a good way of capping TX, but
    today not counting it towards budget is the safe choice.

    > > > +static int mtk_mac_receive_packet(struct mtk_mac_priv *priv)
    > > > +{
    > > > + struct net_device *ndev = mtk_mac_get_netdev(priv);
    > > > + struct mtk_mac_ring *ring = &priv->rx_ring;
    > > > + struct device *dev = mtk_mac_get_dev(priv);
    > > > + struct mtk_mac_ring_desc_data desc_data;
    > > > + struct sk_buff *new_skb;
    > > > + int ret;
    > > > +
    > > > + mtk_mac_lock(priv);
    > > > + ret = mtk_mac_ring_pop_tail(ring, &desc_data);
    > > > + mtk_mac_unlock(priv);
    > > > + if (ret)
    > > > + return -1;
    > > > +
    > > > + mtk_mac_dma_unmap_rx(priv, &desc_data);
    > > > +
    > > > + if ((desc_data.flags & MTK_MAC_DESC_BIT_RX_CRCE) ||
    > > > + (desc_data.flags & MTK_MAC_DESC_BIT_RX_OSIZE)) {
    > > > + /* Error packet -> drop and reuse skb. */
    > > > + new_skb = desc_data.skb;
    > > > + goto map_skb;
    > > > + }
    > > > +
    > > > + new_skb = mtk_mac_alloc_skb(ndev);
    > > > + if (!new_skb) {
    > > > + netdev_err(ndev, "out of memory for skb\n");
    > >
    > > No need for printing, kernel will complain loudly about oom.
    > >
    > > > + ndev->stats.rx_dropped++;
    > > > + new_skb = desc_data.skb;
    > > > + goto map_skb;
    > > > + }
    > > > +
    > > > + skb_put(desc_data.skb, desc_data.len);
    > > > + desc_data.skb->ip_summed = CHECKSUM_NONE;
    > > > + desc_data.skb->protocol = eth_type_trans(desc_data.skb, ndev);
    > > > + desc_data.skb->dev = ndev;
    > > > + netif_receive_skb(desc_data.skb);
    > > > +
    > > > +map_skb:
    > > > + desc_data.dma_addr = mtk_mac_dma_map_rx(priv, new_skb);
    > > > + if (dma_mapping_error(dev, desc_data.dma_addr)) {
    > > > + dev_kfree_skb(new_skb);
    > > > + netdev_err(ndev, "DMA mapping error of RX descriptor\n");
    > > > + return -ENOMEM;
    > >
    > > In this case nothing will ever replenish the RX ring right? If we hit
    > > this condition 128 times the ring will be empty?
    >
    > Indeed. What should I do if this fails though?

    I think if you move things around it should work:

    skb = pop_tail();
    if (!skb)
    return;

    new_skb = alloc();
    if (!new_skb)
    goto reuse;

    dma_map(new_skb);
    if (error)
    goto reuse;

    dma_unmap(skb);

    if (do_packet_processing())
    free(skb);
    else
    receive(skb);

    put_on_ring(new_skb);

    \
     
     \ /
      Last update: 2020-05-06 19:20    [W:3.534 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site