On some Armada 370-based NAS, I have experimented kernel bugs and
crashes when the mvneta Ethernet driver fails to refill Rx DMA buffers
due to memory shortage.

With the actual code, if the memory allocation fails while refilling a
Rx DMA buffer, then the corresponding descriptor is let with the address
of an unmapped DMA buffer already passed to the network stack. Since the
driver still increments the non-occupied counter for Rx descriptor (if a
next packet is handled successfully), then the Ethernet controller is
allowed to reuse the unfilled Rx descriptor...

As a fix, this patch first refills a Rx descriptor before handling the
stored data and unmapping the associated Rx DMA buffer. Additionally the
occupied and non-occupied counters for the Rx descriptors queues are now
both updated with the rx_done value: the number of descriptors ready to
be returned to the networking controller. Moreover note that there is no
point in using different values for this counters because both the
mvneta driver and the Ethernet controller are unable to handle holes in
the Rx descriptors queues.

Signed-off-by: Simon Guinot <simon.gui...@sequanux.org>
Fixes: c5aff18204da ("net: mvneta: driver for Marvell Armada 370/XP network 
unit")
Cc: <sta...@vger.kernel.org> # v3.8+
Tested-by: Yoann Sculo <yo...@sculo.fr>
---
 drivers/net/ethernet/marvell/mvneta.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c 
b/drivers/net/ethernet/marvell/mvneta.c
index ce5f7f9cff06..ac3da11e63a0 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -1455,7 +1455,7 @@ static int mvneta_rx(struct mvneta_port *pp, int rx_todo,
                     struct mvneta_rx_queue *rxq)
 {
        struct net_device *dev = pp->dev;
-       int rx_done, rx_filled;
+       int rx_done;
        u32 rcvd_pkts = 0;
        u32 rcvd_bytes = 0;
 
@@ -1466,7 +1466,6 @@ static int mvneta_rx(struct mvneta_port *pp, int rx_todo,
                rx_todo = rx_done;
 
        rx_done = 0;
-       rx_filled = 0;
 
        /* Fairness NAPI loop */
        while (rx_done < rx_todo) {
@@ -1477,7 +1476,6 @@ static int mvneta_rx(struct mvneta_port *pp, int rx_todo,
                int rx_bytes, err;
 
                rx_done++;
-               rx_filled++;
                rx_status = rx_desc->status;
                rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
                data = (unsigned char *)rx_desc->buf_cookie;
@@ -1517,6 +1515,14 @@ static int mvneta_rx(struct mvneta_port *pp, int rx_todo,
                        continue;
                }
 
+               /* Refill processing */
+               err = mvneta_rx_refill(pp, rx_desc);
+               if (err) {
+                       netdev_err(dev, "Linux processing - Can't refill\n");
+                       rxq->missed++;
+                       goto err_drop_frame;
+               }
+
                skb = build_skb(data, pp->frag_size > PAGE_SIZE ? 0 : 
pp->frag_size);
                if (!skb)
                        goto err_drop_frame;
@@ -1536,14 +1542,6 @@ static int mvneta_rx(struct mvneta_port *pp, int rx_todo,
                mvneta_rx_csum(pp, rx_status, skb);
 
                napi_gro_receive(&pp->napi, skb);
-
-               /* Refill processing */
-               err = mvneta_rx_refill(pp, rx_desc);
-               if (err) {
-                       netdev_err(dev, "Linux processing - Can't refill\n");
-                       rxq->missed++;
-                       rx_filled--;
-               }
        }
 
        if (rcvd_pkts) {
@@ -1556,7 +1554,7 @@ static int mvneta_rx(struct mvneta_port *pp, int rx_todo,
        }
 
        /* Update rxq management counters */
-       mvneta_rxq_desc_num_update(pp, rxq, rx_done, rx_filled);
+       mvneta_rxq_desc_num_update(pp, rxq, rx_done, rx_done);
 
        return rx_done;
 }
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to