[PATCH net 0/2] net: xilinx: axienet: Fix kernel crash in dmaengine transmit path

[PATCH net 0/2] net: xilinx: axienet: Fix kernel crash in dmaengine transmit path

[Suraj Gupta]

This series fixes kernel crash in dmaengine transmit path. To fix it,
enqueue Tx packets in dql before starting dmaengine and check if queue is
not stopped.

Suraj Gupta (2):
  net: xilinx: axienet: Enqueue Tx packets in dql before dmaengine
    starts
  net: xilinx: axienet: Check if Tx queue enabled

 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH net 1/2] net: xilinx: axienet: Enqueue Tx packets in dql before dmaengine starts

[Suraj Gupta]

Enqueue packets in dql after dma engine starts causes race condition.
Tx transfer starts once dma engine is started and may execute dql dequeue
in completion before it gets queued. It results in following kernel crash
while running iperf stress test:

kernel BUG at lib/dynamic_queue_limits.c:99!
<snip>
Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
pc : dql_completed+0x238/0x248
lr : dql_completed+0x3c/0x248

Call trace:
  dql_completed+0x238/0x248
  axienet_dma_tx_cb+0xa0/0x170
  xilinx_dma_do_tasklet+0xdc/0x290
  tasklet_action_common+0xf8/0x11c
  tasklet_action+0x30/0x3c
  handle_softirqs+0xf8/0x230
<snip>

Start dmaengine after enqueue in dql fixes the crash.

Fixes: 6a91b846af85 ("net: axienet: Introduce dmaengine support")
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 273ec5f70005..0f4b02fe6f85 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -924,13 +924,13 @@ axienet_start_xmit_dmaengine(struct sk_buff *skb, struct net_device *ndev)
 	skbuf_dma->sg_len = sg_len;
 	dma_tx_desc->callback_param = lp;
 	dma_tx_desc->callback_result = axienet_dma_tx_cb;
-	dmaengine_submit(dma_tx_desc);
-	dma_async_issue_pending(lp->tx_chan);
 	txq = skb_get_tx_queue(lp->ndev, skb);
 	netdev_tx_sent_queue(txq, skb->len);
 	netif_txq_maybe_stop(txq, CIRC_SPACE(lp->tx_ring_head, lp->tx_ring_tail, TX_BD_NUM_MAX),
 			     MAX_SKB_FRAGS + 1, 2 * MAX_SKB_FRAGS);
 
+	dmaengine_submit(dma_tx_desc);
+	dma_async_issue_pending(lp->tx_chan);
 	return NETDEV_TX_OK;
 
 xmit_error_unmap_sg:
-- 
2.25.1

[PATCH net 2/2] net: xilinx: axienet: Check if Tx queue enabled

[Suraj Gupta]

Check return value of netif_txq_maybe_stop() in transmit
direction and start dma engine only if queue is enabled.

Fixes: 6a91b846af85 ("net: axienet: Introduce dmaengine support")
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 0f4b02fe6f85..620c19edeeee 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -926,8 +926,14 @@ axienet_start_xmit_dmaengine(struct sk_buff *skb, struct net_device *ndev)
 	dma_tx_desc->callback_result = axienet_dma_tx_cb;
 	txq = skb_get_tx_queue(lp->ndev, skb);
 	netdev_tx_sent_queue(txq, skb->len);
-	netif_txq_maybe_stop(txq, CIRC_SPACE(lp->tx_ring_head, lp->tx_ring_tail, TX_BD_NUM_MAX),
-			     MAX_SKB_FRAGS + 1, 2 * MAX_SKB_FRAGS);
+
+	/* Check if queue stopped */
+	if (!netif_txq_maybe_stop(txq, CIRC_SPACE(lp->tx_ring_head, lp->tx_ring_tail,
+						  TX_BD_NUM_MAX),
+						  MAX_SKB_FRAGS + 1, 2 * MAX_SKB_FRAGS)) {
+		dma_unmap_sg(lp->dev, skbuf_dma->sgl, sg_len, DMA_TO_DEVICE);
+		return NETDEV_TX_BUSY;
+	}
 
 	dmaengine_submit(dma_tx_desc);
 	dma_async_issue_pending(lp->tx_chan);
-- 
2.25.1

Re: [PATCH net 2/2] net: xilinx: axienet: Check if Tx queue enabled

[Jakub Kicinski]

On Wed, 30 Oct 2024 11:55:33 +0530 Suraj Gupta wrote:
> Check return value of netif_txq_maybe_stop() in transmit
> direction and start dma engine only if queue is enabled.

The first patch makes sense, let me apply that one.
But this one I don't understand - what is the problem you're trying 
to fix? netif_txq_maybe_stop() tries to stop the queue if the *next*
packet may not fit in the queue. The currently processed packet is
assumed to have already been queued.

Re: [PATCH net 0/2] net: xilinx: axienet: Fix kernel crash in dmaengine transmit path

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 30 Oct 2024 11:55:31 +0530 you wrote:
> This series fixes kernel crash in dmaengine transmit path. To fix it,
> enqueue Tx packets in dql before starting dmaengine and check if queue is
> not stopped.
> 
> Suraj Gupta (2):
>   net: xilinx: axienet: Enqueue Tx packets in dql before dmaengine
>     starts
>   net: xilinx: axienet: Check if Tx queue enabled
> 
> [...]

Here is the summary with links:
  - [net,1/2] net: xilinx: axienet: Enqueue Tx packets in dql before dmaengine starts
    https://git.kernel.org/netdev/net/c/5ccdcdf186ae
  - [net,2/2] net: xilinx: axienet: Check if Tx queue enabled
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

RE: [PATCH net 2/2] net: xilinx: axienet: Check if Tx queue enabled

[Gupta, Suraj]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Monday, November 4, 2024 4:07 AM
> To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> Cc: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> pabeni@redhat.com; Simek, Michal <michal.simek@amd.com>;
> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>
> Subject: Re: [PATCH net 2/2] net: xilinx: axienet: Check if Tx queue enabled
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> On Wed, 30 Oct 2024 11:55:33 +0530 Suraj Gupta wrote:
> > Check return value of netif_txq_maybe_stop() in transmit direction and
> > start dma engine only if queue is enabled.
> 
> The first patch makes sense, let me apply that one.
> But this one I don't understand - what is the problem you're trying to fix?
> netif_txq_maybe_stop() tries to stop the queue if the *next* packet may not fit in the
> queue. The currently processed packet is assumed to have already been queued.

I was under impression that it tries to stop if "current" packet may not fit in the queue. This check won't be required then, thanks for applying first patch.

Regards,
Suraj