[PATCH net] net: microchip: fdma: fix DCB corruption under sustained load

[PATCH net] net: microchip: fdma: fix DCB corruption under sustained load

[Daniel Machon]

__fdma_dcb_add() prepares a DCB and links it into a channel's chain by
writing its address to the NEXT_PTR of the current last DCB. The FDMA
may be processing the chain while this happens.

Before this patch, the link is written before the new DCB's own nextptr
and info fields, so the FDMA can follow it to a DCB that hasn't been fully
prepared yet. Under sustained load this stalls the FDMA.

Prepare the new DCB fully before linking it, and update the previous
DCB's NEXT_PTR using the two-step sequence from the datasheet (poison
the LSB and flush, then clear it) so the FDMA never observes a
partially-updated pointer.

Reported by a customer running iperf3 --bidir against a lan969x board.

Reproduced on lan969x with iperf3, throughput collapsing to zero:

  [  5]   1.00-2.00   sec  73.9 MBytes   620 Mbits/sec    0    305 KBytes
  [  5]   2.00-3.00   sec  73.9 MBytes   620 Mbits/sec    0    305 KBytes
  [  5]   3.00-4.00   sec  13.9 MBytes   116 Mbits/sec    2   1.41 KBytes
  [  5]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec    1   1.41 KBytes
  [  5]   5.00-6.00   sec  0.00 Bytes  0.00 bits/sec    0   1.41 KBytes
  [  5]   6.00-7.00   sec  0.00 Bytes  0.00 bits/sec    1   1.41 KBytes

The race is timing-dependent: across three reproductions the stall
appeared anywhere from ~400s to ~3000s into the run.

Fixes: 30e48a75df9c ("net: microchip: add FDMA library")
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 drivers/net/ethernet/microchip/fdma/fdma_api.c | 30 ++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microchip/fdma/fdma_api.c b/drivers/net/ethernet/microchip/fdma/fdma_api.c
index f7a42348e932c..fed41da550c59 100644
--- a/drivers/net/ethernet/microchip/fdma/fdma_api.c
+++ b/drivers/net/ethernet/microchip/fdma/fdma_api.c
@@ -43,9 +43,19 @@ int __fdma_dcb_add(struct fdma *fdma, int dcb_idx, u64 info, u64 status,
 				u64 *dataptr))
 {
 	struct fdma_dcb *dcb = fdma_dcb_get(fdma, dcb_idx);
+	struct fdma_dcb *prev = fdma->last_dcb;
 	u64 nextptr;
 	int i, err;
 
+	/* Per the FDMA datasheet, adding a DCB to an active channel takes
+	 * three steps:
+	 * 1) Write NEXT_PTR of the current last DCB with the LSB forced to
+	 *    FDMA_DCB_INVALID_DATA, and flush that write.
+	 * 2) Clear the LSB to complete the link.
+	 * 3) Reload the channel (FDMA_CH_RELOAD); this is done by the caller
+	 *    once the chain has been extended.
+	 */
+
 	for (i = 0; i < fdma->n_dbs; i++) {
 		err = __fdma_db_add(fdma, dcb_idx, i, status, db_cb);
 		if (unlikely(err))
@@ -56,13 +66,25 @@ int __fdma_dcb_add(struct fdma *fdma, int dcb_idx, u64 info, u64 status,
 	if (unlikely(err))
 		return err;
 
-	fdma->last_dcb->nextptr = cpu_to_le64(nextptr);
-
-	fdma->last_dcb = dcb;
-
 	dcb->nextptr = cpu_to_le64(FDMA_DCB_INVALID_DATA);
 	dcb->info = cpu_to_le64(info);
 
+	/* On the first DCB of fdma_dcbs_init() prev is the DCB itself;
+	 * don't link it to itself.
+	 */
+	if (prev != dcb) {
+		/* Write NEXT_PTR of the current last DCB with the LSB forced
+		 * to FDMA_DCB_INVALID_DATA, and flush that write.
+		 */
+		prev->nextptr = cpu_to_le64(nextptr | FDMA_DCB_INVALID_DATA);
+		dma_wmb();
+
+		/* Clear the LSB to complete the link. */
+		prev->nextptr = cpu_to_le64(nextptr);
+	}
+
+	fdma->last_dcb = dcb;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__fdma_dcb_add);

---
base-commit: 6430fb7f1cc4e329e386521b7b208df443de92f2
change-id: 20261026-microchip-fdma-dcb-corruption-6bf402b99b12

Best regards,
-- 
Daniel Machon <daniel.machon@microchip.com>