drivers: eth_nxp_enet_qos: Fix deadlock in system workqueue

There was a deadlock occurring, exposed by http server sample because of
situations like this caused by tx done work being blocked in deadlock:
1) The TX would be started by some thread and the driver TX sem would be
   taken.
2) The http server socket would get scheduled on the system workqueue to
   send something, claim the network TX interface mutex,
   and be blocked taking the semaphore.
3) The RX traffic class handler would get blocked trying to claim the
   network interface TX mutex, while trying to send an ACK in the TCP
   callback. This means the RX packets would not be processed.
4) Lots of RX unable to allocate packets errors would happen, and all RX
   would be dropped. This was the main symptom of the deadlock, which
   made it look like a memory leak but actually had nothing to do with
   the RX code nor any memory leak.
5) The TX DMA would finish and schedule the TX DMA done work onto the
   system work queue, behind the http server socket which is blocked on
   the waiting for the driver TX semaphore.
6) If the TX DMA done work would have ran, that's what gives the TX
   driver semaphore. So this is the reason for the deadlock of all these
   different threads and work items, the misqueue in the system
   workqueue.

Fix by just calling the TX DMA done code directly from the ISR, it
should be ISR safe, and really not a lot of code to execute, just
freeing some net buffers and the packet and updating the stats.
An optimization can be made later if needed, but for now,
solving the deadlock is a more urgent priority.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Author: decsny

drivers/ethernet/eth_nxp_enet_qos/eth_nxp_enet_qos_mac.c

@@ -92,6 +92,7 @@ static int eth_nxp_enet_qos_tx(const struct device *dev, struct net_pkt *pkt)
 
 	/* One TX at a time in the current implementation */
 	k_sem_take(&data->tx.tx_sem, K_FOREVER);
+	LOG_DBG("Took driver TX sem %p by thread %p", &data->tx.tx_sem, k_current_get());
 
 	net_pkt_ref(pkt);
 
@@ -141,12 +142,10 @@ static int eth_nxp_enet_qos_tx(const struct device *dev, struct net_pkt *pkt)
 	return 0;
 }
 
-static void tx_dma_done(struct k_work *work)
+static void tx_dma_done(const struct device *dev)
 {
-	struct nxp_enet_qos_tx_data *tx_data =
-		CONTAINER_OF(work, struct nxp_enet_qos_tx_data, tx_done_work);
-	struct nxp_enet_qos_mac_data *data =
-		CONTAINER_OF(tx_data, struct nxp_enet_qos_mac_data, tx);
+	struct nxp_enet_qos_mac_data *data = dev->data;
+	struct nxp_enet_qos_tx_data *tx_data = &data->tx;
 	struct net_pkt *pkt = tx_data->pkt;
 	struct net_buf *fragment = pkt->frags;
 
@@ -165,6 +164,7 @@ static void tx_dma_done(struct k_work *work)
 
 	/* Allows another send */
 	k_sem_give(&data->tx.tx_sem);
+	LOG_DBG("Gave driver TX sem %p by thread %p", &data->tx.tx_sem, k_current_get());
 }
 
 static enum ethernet_hw_caps eth_nxp_enet_qos_get_capabilities(const struct device *dev)
@@ -474,7 +474,7 @@ static void eth_nxp_enet_qos_mac_isr(const struct device *dev)
 	if (ENET_QOS_REG_GET(DMA_INTERRUPT_STATUS, DC0IS, dma_interrupts)) {
 		if (ENET_QOS_REG_GET(DMA_CH_DMA_CHX_STAT, TI, dma_ch0_interrupts)) {
 			/* add pending tx to queue */
-			k_work_submit(&data->tx.tx_done_work);
+			tx_dma_done(dev);
 		}
 
 		if (ENET_QOS_REG_GET(DMA_CH_DMA_CHX_STAT, RI, dma_ch0_interrupts)) {
@@ -842,9 +842,6 @@ static int eth_nxp_enet_qos_mac_init(const struct device *dev)
 	/* Work upon a reception of a packet to a buffer */
 	k_work_init(&data->rx.rx_work, eth_nxp_enet_qos_rx);
 
-	/* Work upon a complete transmission by a channel's TX DMA */
-	k_work_init(&data->tx.tx_done_work, tx_dma_done);
-
 	return ret;
 }
 

drivers/ethernet/eth_nxp_enet_qos/nxp_enet_qos_priv.h

@@ -106,7 +106,6 @@ struct nxp_enet_qos_mac_config {
 struct nxp_enet_qos_tx_data {
 	struct k_sem tx_sem;
 	struct net_pkt *pkt;
-	struct k_work tx_done_work;
 	struct net_buf *tx_header;
 	volatile union nxp_enet_qos_tx_desc descriptors[NUM_TX_BUFDESC];
 };