diff mbox series

[1/2] net: stmmac: don't set own bit too early for jumbo frames

Message ID 20190314194320.25566-1-aaro.koskinen@iki.fi
State Accepted
Delegated to: David Miller
Headers show
Series [1/2] net: stmmac: don't set own bit too early for jumbo frames | expand

Commit Message

Aaro Koskinen March 14, 2019, 7:43 p.m. UTC
From: Aaro Koskinen <aaro.koskinen@nokia.com>

Commit 0e80bdc9a72d ("stmmac: first frame prep at the end of xmit
routine") overlooked jumbo frames when re-ordering the code, and as a
result the own bit was not getting set anymore for the first jumbo frame
descriptor. Commit 487e2e22ab79 ("net: stmmac: Set OWN bit for jumbo
frames") tried to fix this, but now the bit is getting set too early and
the DMA may start while we are still setting up the remaing descriptors.
And with the chain mode the own bit remains still unset.

Fix by setting the own bit at the end of xmit also with jumbo frames.

Fixes: 0e80bdc9a72d ("stmmac: first frame prep at the end of xmit routine")
Fixes: 487e2e22ab79 ("net: stmmac: Set OWN bit for jumbo frames")
Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c   |  4 ++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++------
 2 files changed, 10 insertions(+), 8 deletions(-)

Comments

Sergei Shtylyov March 15, 2019, 8:45 a.m. UTC | #1
Hello!

On 14.03.2019 22:43, Aaro Koskinen wrote:

> From: Aaro Koskinen <aaro.koskinen@nokia.com>
> 
> Commit 0e80bdc9a72d ("stmmac: first frame prep at the end of xmit
> routine") overlooked jumbo frames when re-ordering the code, and as a
> result the own bit was not getting set anymore for the first jumbo frame
> descriptor. Commit 487e2e22ab79 ("net: stmmac: Set OWN bit for jumbo
> frames") tried to fix this, but now the bit is getting set too early and
> the DMA may start while we are still setting up the remaing descriptors.

    Remaining?

> And with the chain mode the own bit remains still unset.
> 
> Fix by setting the own bit at the end of xmit also with jumbo frames.
> 
> Fixes: 0e80bdc9a72d ("stmmac: first frame prep at the end of xmit routine")
> Fixes: 487e2e22ab79 ("net: stmmac: Set OWN bit for jumbo frames")
> Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
[...]

MBR, Sergei
Jose Abreu March 15, 2019, 9:47 a.m. UTC | #2
On 3/14/2019 7:43 PM, Aaro Koskinen wrote:
> From: Aaro Koskinen <aaro.koskinen@nokia.com>
> 
> Commit 0e80bdc9a72d ("stmmac: first frame prep at the end of xmit
> routine") overlooked jumbo frames when re-ordering the code, and as a
> result the own bit was not getting set anymore for the first jumbo frame
> descriptor. Commit 487e2e22ab79 ("net: stmmac: Set OWN bit for jumbo
> frames") tried to fix this, but now the bit is getting set too early and
> the DMA may start while we are still setting up the remaing descriptors.
> And with the chain mode the own bit remains still unset.
> 
> Fix by setting the own bit at the end of xmit also with jumbo frames.
> 
> Fixes: 0e80bdc9a72d ("stmmac: first frame prep at the end of xmit routine")
> Fixes: 487e2e22ab79 ("net: stmmac: Set OWN bit for jumbo frames")
> Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>

With Sergei comment fix:

Acked-by: Jose Abreu <joabreu@synopsys.com>

Thanks,
Jose Miguel Abreu
David Miller March 15, 2019, 6:39 p.m. UTC | #3
From: Aaro Koskinen <aaro.koskinen@iki.fi>
Date: Thu, 14 Mar 2019 21:43:19 +0200

> From: Aaro Koskinen <aaro.koskinen@nokia.com>
> 
> Commit 0e80bdc9a72d ("stmmac: first frame prep at the end of xmit
> routine") overlooked jumbo frames when re-ordering the code, and as a
> result the own bit was not getting set anymore for the first jumbo frame
> descriptor. Commit 487e2e22ab79 ("net: stmmac: Set OWN bit for jumbo
> frames") tried to fix this, but now the bit is getting set too early and
> the DMA may start while we are still setting up the remaing descriptors.
> And with the chain mode the own bit remains still unset.
> 
> Fix by setting the own bit at the end of xmit also with jumbo frames.
> 
> Fixes: 0e80bdc9a72d ("stmmac: first frame prep at the end of xmit routine")
> Fixes: 487e2e22ab79 ("net: stmmac: Set OWN bit for jumbo frames")
> Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>

Applied with the "remaining" typo fixed.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
index d8c5bc412219..bc83ced94e1b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
@@ -59,7 +59,7 @@  static int jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
 		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
 		stmmac_prepare_tx_desc(priv, desc, 1, bmax, csum,
-				STMMAC_RING_MODE, 1, false, skb->len);
+				STMMAC_RING_MODE, 0, false, skb->len);
 		tx_q->tx_skbuff[entry] = NULL;
 		entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);
 
@@ -91,7 +91,7 @@  static int jumbo_frm(void *p, struct sk_buff *skb, int csum)
 		tx_q->tx_skbuff_dma[entry].is_jumbo = true;
 		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
 		stmmac_prepare_tx_desc(priv, desc, 1, nopaged_len, csum,
-				STMMAC_RING_MODE, 1, true, skb->len);
+				STMMAC_RING_MODE, 0, true, skb->len);
 	}
 
 	tx_q->cur_tx = entry;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 97c5e1aad88f..6a2e1031a62a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3216,14 +3216,16 @@  static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 		stmmac_prepare_tx_desc(priv, first, 1, nopaged_len,
 				csum_insertion, priv->mode, 1, last_segment,
 				skb->len);
-
-		/* The own bit must be the latest setting done when prepare the
-		 * descriptor and then barrier is needed to make sure that
-		 * all is coherent before granting the DMA engine.
-		 */
-		wmb();
+	} else {
+		stmmac_set_tx_owner(priv, first);
 	}
 
+	/* The own bit must be the latest setting done when prepare the
+	 * descriptor and then barrier is needed to make sure that
+	 * all is coherent before granting the DMA engine.
+	 */
+	wmb();
+
 	netdev_tx_sent_queue(netdev_get_tx_queue(dev, queue), skb->len);
 
 	stmmac_enable_dma_transmission(priv, priv->ioaddr);