Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   Vincent Whitchurch <vincent.whitchurch@axis.com>
To:     Giuseppe Cavallaro <peppe.cavallaro@st.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>
CC:     <kernel@axis.com>, Lars Persson <larper@axis.com>,
        Vincent Whitchurch <vincent.whitchurch@axis.com>,
        Srinivas Kandagatla <srinivas.kandagatla@st.com>,
        <netdev@vger.kernel.org>,
        <linux-stm32@st-md-mailman.stormreply.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>
Subject: [PATCH] net: stmmac: Enable NAPI before interrupts go live
Date:   Thu, 17 Feb 2022 15:55:26 +0100
Message-ID: <20220217145527.2696444-1-vincent.whitchurch@axis.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk

From: Lars Persson <larper@axis.com>

The stmmac_open function has a race window between enabling the RX
path and its interrupt to the point where napi_enabled is called.

A chatty network with plenty of broadcast/multicast traffic has the
potential to completely fill the RX ring before the interrupt handler
is installed. In this scenario the single interrupt taken will find
napi disabled and the RX ring will not be processed. No further RX
interrupt will be delivered because the ring is full.

The RX stall could eventually clear because the TX path will trigger a
DMA interrupt once the tx_coal_frames threshold is reached and then
NAPI becomes scheduled.

Fixes: 523f11b5d4fd72efb ("net: stmmac: move hardware setup for stmmac_open to new function")
Signed-off-by: Lars Persson <larper@axis.com>
[vincent.whitchurch@axis.com: Forward-port to mainline, change xdp_open too]
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6708ca2aa4f7..8bd4123515b0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3753,11 +3753,12 @@ static int stmmac_open(struct net_device *dev)
 	/* We may have called phylink_speed_down before */
 	phylink_speed_up(priv->phylink);
 
+	stmmac_enable_all_queues(priv);
+
 	ret = stmmac_request_irq(dev);
 	if (ret)
 		goto irq_error;
 
-	stmmac_enable_all_queues(priv);
 	netif_tx_start_all_queues(priv->dev);
 
 	return 0;
@@ -3768,6 +3769,7 @@ static int stmmac_open(struct net_device *dev)
 	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
 		hrtimer_cancel(&priv->tx_queue[chan].txtimer);
 
+	stmmac_disable_all_queues(priv);
 	stmmac_hw_teardown(dev);
 init_error:
 	free_dma_desc_resources(priv);
@@ -6562,12 +6564,13 @@ int stmmac_xdp_open(struct net_device *dev)
 	/* Start Rx & Tx DMA Channels */
 	stmmac_start_all_dma(priv);
 
+	/* Enable NAPI process*/
+	stmmac_enable_all_queues(priv);
+
 	ret = stmmac_request_irq(dev);
 	if (ret)
 		goto irq_error;
 
-	/* Enable NAPI process*/
-	stmmac_enable_all_queues(priv);
 	netif_carrier_on(dev);
 	netif_tx_start_all_queues(dev);
 
@@ -6577,6 +6580,7 @@ int stmmac_xdp_open(struct net_device *dev)
 	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
 		hrtimer_cancel(&priv->tx_queue[chan].txtimer);
 
+	stmmac_disable_all_queues(priv);
 	stmmac_hw_teardown(dev);
 init_error:
 	free_dma_desc_resources(priv);
-- 
2.34.1