Skip to content

Commit ff58fda

Browse files
vladimirolteankuba-moo
authored andcommitted
net: enetc: prioritize ability to go down over packet processing
napi_synchronize() from enetc_stop() waits until the softirq has finished execution and no longer wants to be rescheduled. However under high traffic load, this will never happen, and the interface can never be closed. The problem is the fact that the NAPI poll routine is written to update the consumer index which makes the device want to put more buffers in the RX ring, which restarts the madness again. Browsing around, it seems that some drivers like i40e keep a bit (__I40E_VSI_DOWN) which they use as communication between the control path and the data path. But that isn't my first choice, because complications ensue - since the enetc hardirq may trigger while we are in a theoretical ENETC_DOWN state, it may happen that enetc_msix() masks it, but enetc_poll() never unmasks it. To prevent a stall in that case, one would need to schedule all NAPI instances when ENETC_DOWN gets cleared, to process what's pending. I find it more desirable for the control path - enetc_stop() - to just quiesce the RX ring and let the softirq finish what remains there, without any explicit communication, just by making hardware not provide any more packets. This seems possible with the Enable bit of the RX BD ring (RBaMR[EN]). I can't seem to find an exact definition of what this bit does, but when the RX ring is disabled, the port seems to no longer update the producer index, and not react to software updates of the consumer index. In fact, the RBaMR[EN] bit is already toggled by the driver, but too late for what we want: enetc_close() -> enetc_stop() -> napi_synchronize() -> enetc_clear_bdrs() -> enetc_clear_rxbdr() The enetc_clear_bdrs() function contains not only logic to disable the RX and TX rings, but also logic to wait for the TX ring stop being busy. We split enetc_clear_bdrs() into enetc_disable_bdrs() and enetc_wait_bdrs(). One needs to run before napi_synchronize() and the other after (NAPI also processes TX completions, so we maximize our chances of not waiting for the ENETC_TBSR_BUSY bit - unless a packet is stuck for some reason, ofc). We also split off enetc_enable_bdrs() from enetc_setup_bdrs(), and call this from the mirror position in enetc_start() compared to enetc_stop(), i.e. right before netif_tx_start_all_queues(). Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent c33bfaf commit ff58fda

File tree

1 file changed

+63
-18
lines changed
  • drivers/net/ethernet/freescale/enetc

1 file changed

+63
-18
lines changed

drivers/net/ethernet/freescale/enetc/enetc.c

+63-18
Original file line numberDiff line numberDiff line change
@@ -2088,7 +2088,7 @@ static void enetc_setup_txbdr(struct enetc_hw *hw, struct enetc_bdr *tx_ring)
20882088
/* enable Tx ints by setting pkt thr to 1 */
20892089
enetc_txbdr_wr(hw, idx, ENETC_TBICR0, ENETC_TBICR0_ICEN | 0x1);
20902090

2091-
tbmr = ENETC_TBMR_EN | ENETC_TBMR_SET_PRIO(tx_ring->prio);
2091+
tbmr = ENETC_TBMR_SET_PRIO(tx_ring->prio);
20922092
if (tx_ring->ndev->features & NETIF_F_HW_VLAN_CTAG_TX)
20932093
tbmr |= ENETC_TBMR_VIH;
20942094

@@ -2104,7 +2104,7 @@ static void enetc_setup_rxbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring,
21042104
bool extended)
21052105
{
21062106
int idx = rx_ring->index;
2107-
u32 rbmr;
2107+
u32 rbmr = 0;
21082108

21092109
enetc_rxbdr_wr(hw, idx, ENETC_RBBAR0,
21102110
lower_32_bits(rx_ring->bd_dma_base));
@@ -2131,8 +2131,6 @@ static void enetc_setup_rxbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring,
21312131
/* enable Rx ints by setting pkt thr to 1 */
21322132
enetc_rxbdr_wr(hw, idx, ENETC_RBICR0, ENETC_RBICR0_ICEN | 0x1);
21332133

2134-
rbmr = ENETC_RBMR_EN;
2135-
21362134
rx_ring->ext_en = extended;
21372135
if (rx_ring->ext_en)
21382136
rbmr |= ENETC_RBMR_BDS;
@@ -2151,7 +2149,6 @@ static void enetc_setup_rxbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring,
21512149
enetc_refill_rx_ring(rx_ring, enetc_bd_unused(rx_ring));
21522150
enetc_unlock_mdio();
21532151

2154-
/* enable ring */
21552152
enetc_rxbdr_wr(hw, idx, ENETC_RBMR, rbmr);
21562153
}
21572154

@@ -2167,21 +2164,70 @@ static void enetc_setup_bdrs(struct enetc_ndev_priv *priv, bool extended)
21672164
enetc_setup_rxbdr(hw, priv->rx_ring[i], extended);
21682165
}
21692166

2170-
static void enetc_clear_rxbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring)
2167+
static void enetc_enable_txbdr(struct enetc_hw *hw, struct enetc_bdr *tx_ring)
2168+
{
2169+
int idx = tx_ring->index;
2170+
u32 tbmr;
2171+
2172+
tbmr = enetc_txbdr_rd(hw, idx, ENETC_TBMR);
2173+
tbmr |= ENETC_TBMR_EN;
2174+
enetc_txbdr_wr(hw, idx, ENETC_TBMR, tbmr);
2175+
}
2176+
2177+
static void enetc_enable_rxbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring)
2178+
{
2179+
int idx = rx_ring->index;
2180+
u32 rbmr;
2181+
2182+
rbmr = enetc_rxbdr_rd(hw, idx, ENETC_RBMR);
2183+
rbmr |= ENETC_RBMR_EN;
2184+
enetc_rxbdr_wr(hw, idx, ENETC_RBMR, rbmr);
2185+
}
2186+
2187+
static void enetc_enable_bdrs(struct enetc_ndev_priv *priv)
2188+
{
2189+
struct enetc_hw *hw = &priv->si->hw;
2190+
int i;
2191+
2192+
for (i = 0; i < priv->num_tx_rings; i++)
2193+
enetc_enable_txbdr(hw, priv->tx_ring[i]);
2194+
2195+
for (i = 0; i < priv->num_rx_rings; i++)
2196+
enetc_enable_rxbdr(hw, priv->rx_ring[i]);
2197+
}
2198+
2199+
static void enetc_disable_rxbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring)
21712200
{
21722201
int idx = rx_ring->index;
21732202

21742203
/* disable EN bit on ring */
21752204
enetc_rxbdr_wr(hw, idx, ENETC_RBMR, 0);
21762205
}
21772206

2178-
static void enetc_clear_txbdr(struct enetc_hw *hw, struct enetc_bdr *tx_ring)
2207+
static void enetc_disable_txbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring)
21792208
{
2180-
int delay = 8, timeout = 100;
2181-
int idx = tx_ring->index;
2209+
int idx = rx_ring->index;
21822210

21832211
/* disable EN bit on ring */
21842212
enetc_txbdr_wr(hw, idx, ENETC_TBMR, 0);
2213+
}
2214+
2215+
static void enetc_disable_bdrs(struct enetc_ndev_priv *priv)
2216+
{
2217+
struct enetc_hw *hw = &priv->si->hw;
2218+
int i;
2219+
2220+
for (i = 0; i < priv->num_tx_rings; i++)
2221+
enetc_disable_txbdr(hw, priv->tx_ring[i]);
2222+
2223+
for (i = 0; i < priv->num_rx_rings; i++)
2224+
enetc_disable_rxbdr(hw, priv->rx_ring[i]);
2225+
}
2226+
2227+
static void enetc_wait_txbdr(struct enetc_hw *hw, struct enetc_bdr *tx_ring)
2228+
{
2229+
int delay = 8, timeout = 100;
2230+
int idx = tx_ring->index;
21852231

21862232
/* wait for busy to clear */
21872233
while (delay < timeout &&
@@ -2195,18 +2241,13 @@ static void enetc_clear_txbdr(struct enetc_hw *hw, struct enetc_bdr *tx_ring)
21952241
idx);
21962242
}
21972243

2198-
static void enetc_clear_bdrs(struct enetc_ndev_priv *priv)
2244+
static void enetc_wait_bdrs(struct enetc_ndev_priv *priv)
21992245
{
22002246
struct enetc_hw *hw = &priv->si->hw;
22012247
int i;
22022248

22032249
for (i = 0; i < priv->num_tx_rings; i++)
2204-
enetc_clear_txbdr(hw, priv->tx_ring[i]);
2205-
2206-
for (i = 0; i < priv->num_rx_rings; i++)
2207-
enetc_clear_rxbdr(hw, priv->rx_ring[i]);
2208-
2209-
udelay(1);
2250+
enetc_wait_txbdr(hw, priv->tx_ring[i]);
22102251
}
22112252

22122253
static int enetc_setup_irqs(struct enetc_ndev_priv *priv)
@@ -2381,6 +2422,8 @@ void enetc_start(struct net_device *ndev)
23812422
enable_irq(irq);
23822423
}
23832424

2425+
enetc_enable_bdrs(priv);
2426+
23842427
netif_tx_start_all_queues(ndev);
23852428
}
23862429

@@ -2452,6 +2495,8 @@ void enetc_stop(struct net_device *ndev)
24522495

24532496
netif_tx_stop_all_queues(ndev);
24542497

2498+
enetc_disable_bdrs(priv);
2499+
24552500
for (i = 0; i < priv->bdr_int_num; i++) {
24562501
int irq = pci_irq_vector(priv->si->pdev,
24572502
ENETC_BDR_INT_BASE_IDX + i);
@@ -2461,6 +2506,8 @@ void enetc_stop(struct net_device *ndev)
24612506
napi_disable(&priv->int_vector[i]->napi);
24622507
}
24632508

2509+
enetc_wait_bdrs(priv);
2510+
24642511
enetc_clear_interrupts(priv);
24652512
}
24662513

@@ -2469,7 +2516,6 @@ int enetc_close(struct net_device *ndev)
24692516
struct enetc_ndev_priv *priv = netdev_priv(ndev);
24702517

24712518
enetc_stop(ndev);
2472-
enetc_clear_bdrs(priv);
24732519

24742520
if (priv->phylink) {
24752521
phylink_stop(priv->phylink);
@@ -2521,7 +2567,6 @@ static int enetc_reconfigure(struct enetc_ndev_priv *priv, bool extended,
25212567
}
25222568

25232569
enetc_stop(priv->ndev);
2524-
enetc_clear_bdrs(priv);
25252570
enetc_free_rxtx_rings(priv);
25262571

25272572
/* Interface is down, run optional callback now */

0 commit comments

Comments
 (0)