[PATCH net v2 2/3] net: macb: propagate RX ring refill errors

[Théo Lebrun]

gem_rx_refill() is responsible for Rx SKB allocation, including at open,
but its prototype indicates a void return value.

Therefore we change the code to propagate allocation and DMA mapping
errors back up the stack, making sure the open fails if no descriptors
were allocated. Change all those to return errno-style ints:
 - gem_rx_refill()
 - its parent gem_init_rx_ring()
 - its grand-parent gem_init_rings()
 - the macbgem_ops.mog_init_rings function pointer
 - its grand-uncle macb_init_rings()

We tolerate some allocation failures: we accept running with the rx ring
only partially filled with successful descriptors. It is important we
refuse the zero-valid-descriptor case: nothing would ever trigger a
refill, which only happens once a frame has been received.

Theoretical bugfix, never encountered in practice. To reproduce,
introduce memory pressure (less than 512 SKBs of free memory) and open
the interface. I expect the last queue to be unusable because it has
zero usable rx buffers.

Note that other callers of refill (resume, HRESP error task, NAPI)
cannot do anything useful with that error and keep their best-effort
refill, hoping it will improve.

Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
---
 drivers/net/ethernet/cadence/macb.h      |  2 +-
 drivers/net/ethernet/cadence/macb_main.c | 33 +++++++++++++++++++++++++-------
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d6931c41f39d..cfaa0ca49f1a 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1197,7 +1197,7 @@ struct macb_queue;
 struct macb_or_gem_ops {
 	int	(*mog_alloc_rx_buffers)(struct macb *bp);
 	void	(*mog_free_rx_buffers)(struct macb *bp);
-	void	(*mog_init_rings)(struct macb *bp);
+	int	(*mog_init_rings)(struct macb *bp);
 	int	(*mog_rx)(struct macb_queue *queue, struct napi_struct *napi,
 			  int budget);
 };
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 7f25574928d7..18a1b5f7ad91 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1486,7 +1486,7 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 	return packets;
 }
 
-static void gem_rx_refill(struct macb_queue *queue)
+static int gem_rx_refill(struct macb_queue *queue)
 {
 	struct macb *bp = queue->bp;
 	struct macb_dma_desc *desc;
@@ -1558,6 +1558,14 @@ static void gem_rx_refill(struct macb_queue *queue)
 
 	netdev_vdbg(bp->netdev, "rx ring: queue: %p, prepared head %d, tail %d\n",
 		    queue, queue->rx_prepared_head, queue->rx_tail);
+
+	/* Fail if queue has zero prepared descriptors. This is critical because
+	 * nothing will ever trigger a refill again.
+	 */
+	if (queue->rx_prepared_head == queue->rx_tail)
+		return -ENOMEM;
+
+	return 0;
 }
 
 /* Mark DMA descriptors from begin up to and not including end as unused */
@@ -2804,7 +2812,7 @@ static int macb_alloc(struct macb *bp)
 	return -ENOMEM;
 }
 
-static void gem_init_rx_ring(struct macb_queue *queue)
+static int gem_init_rx_ring(struct macb_queue *queue)
 {
 	unsigned int i;
 
@@ -2814,14 +2822,16 @@ static void gem_init_rx_ring(struct macb_queue *queue)
 	for (i = 0; i < queue->bp->rx_ring_size; i++)
 		macb_rx_desc(queue, i)->addr |= MACB_BIT(RX_USED);
 
-	gem_rx_refill(queue);
+	return gem_rx_refill(queue);
 }
 
-static void gem_init_rings(struct macb *bp)
+static int gem_init_rings(struct macb *bp)
 {
 	struct macb_queue *queue;
 	struct macb_dma_desc *desc = NULL;
+	int last_err = 0;
 	unsigned int q;
+	int err;
 	int i;
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
@@ -2834,11 +2844,15 @@ static void gem_init_rings(struct macb *bp)
 		queue->tx_head = 0;
 		queue->tx_tail = 0;
 
-		gem_init_rx_ring(queue);
+		err = gem_init_rx_ring(queue);
+		if (err)
+			last_err = err;
 	}
+
+	return last_err;
 }
 
-static void macb_init_rings(struct macb *bp)
+static int macb_init_rings(struct macb *bp)
 {
 	int i;
 	struct macb_dma_desc *desc = NULL;
@@ -2853,6 +2867,8 @@ static void macb_init_rings(struct macb *bp)
 	bp->queues[0].tx_head = 0;
 	bp->queues[0].tx_tail = 0;
 	desc->ctrl |= MACB_BIT(TX_WRAP);
+
+	return 0;
 }
 
 static void macb_reset_hw(struct macb *bp)
@@ -3183,7 +3199,9 @@ static int macb_open(struct net_device *netdev)
 		goto pm_exit;
 	}
 
-	bp->macbgem_ops.mog_init_rings(bp);
+	err = bp->macbgem_ops.mog_init_rings(bp);
+	if (err)
+		goto free_rings;
 	macb_init_buffers(bp);
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
@@ -3221,6 +3239,7 @@ static int macb_open(struct net_device *netdev)
 		napi_disable(&queue->napi_rx);
 		napi_disable(&queue->napi_tx);
 	}
+free_rings:
 	macb_free(bp);
 pm_exit:
 	pm_runtime_put_sync(&bp->pdev->dev);

-- 
2.55.0