public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Zefir Kurtisi <zefir.kurtisi@gmail.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: claudiu.manoil@nxp.com, wei.fang@nxp.com, xiaoning.wang@nxp.com,
	davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Zefir Kurtisi <zefir.kurtisi@westermo.com>
Subject: Re: [PATCH] net: enetc: fix sirq-storm by clearing IDR registers
Date: Fri, 27 Feb 2026 12:28:17 +0100	[thread overview]
Message-ID: <786338e0-0a6b-4b7b-9e22-8253bb655aec@gmail.com> (raw)
In-Reply-To: <20260226195158.sagjq3hgs6l64w2u@skbuf>



On 2/26/26 20:51, Vladimir Oltean wrote:
> [...]
> If I understand correctly, this patch should resolve your issue?
> 
>  From 887cf74648fb10b7dee3c60199349d184c5a851e Mon Sep 17 00:00:00 2001
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Date: Thu, 26 Feb 2026 21:50:13 +0200
> Subject: [PATCH] net: enetc: avoid sending too short Ethernet frames
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>   drivers/net/ethernet/freescale/enetc/enetc.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index b0b47b0f7723..4f5e593b348a 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -982,6 +982,9 @@ static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
>   	struct enetc_bdr *tx_ring;
>   	int count;
>   
> +	if (eth_skb_pad(skb))
> +		return NETDEV_TX_OK;
> +
>   	/* Queue one-step Sync packet if already locked */
>   	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
>   		if (test_and_set_bit_lock(ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS,

Hi Vladimir,

yes, that fixes it.

I expected identifying invalid frames with all corner-cases to be 
difficult, not realizing that making them valid is way easier. Thank you 
for that insight.

Tested-by: Zefir Kurtisi <zefir.kurtisi@westermo.com>


While I was debugging the issue I extended enetc with features that 
still might be useful; the first change was to selectively process only 
the pending BDs instead of looping over all available, saving a 
significant number of unneeded calls to enetc_clean_rx/tx_ring(); the 
second adds a fail-safe mechanism for potentially other issues taking 
the same code path based on the fact that now a BD is only processed 
when its IDR is active, which means there must be BDs available to process.

Please find the patches below, if you think they are of some value, take 
as you see fit or let me know to prepare a v2.

Thank you for looking into it and resolving it so swiftly.

Cheers,
Zefir

---
 From 54791dad5555d6dea0c0276268058d491b25548a Mon Sep 17 00:00:00 2001
From: Zefir Kurtisi <zefir.kurtisi@westermo.com>
Date: Fri, 27 Feb 2026 11:14:08 +0100
Subject: [PATCH 1/2] net: enetc: process only those BDs that have IDR set

The enetc_poll() function currently tries to process all
BDs without considering which one caused the interrupt.

When a frame is received, it then still loops over all
TX BDs to in enetc_clean_tx_ring() find out there are no
pending BDs to clean.

SITXIDR/SIRXIDR provide us with the list of BDs that were
triggering the interrupt, so we can limit processing to
those affected.

This changes enetc_poll processing accordingly.

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@westermo.com>
---
  drivers/net/ethernet/freescale/enetc/enetc.c | 24 +++++++++++++++-----
  1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c 
b/drivers/net/ethernet/freescale/enetc/enetc.c
index e380a4f39855..e9d600506d83 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -2114,19 +2114,31 @@ static int enetc_poll(struct napi_struct *napi, 
int budget)
  	bool complete = true;
  	int work_done;
  	int i;
+	u32 si_idr;

  	enetc_lock_mdio();

-	for (i = 0; i < v->count_tx_rings; i++)
+	si_idr = enetc_rd_reg_hot(v->tx_ring[0].idr);
+	for (i = 0; i < v->count_tx_rings; i++) {
+		/* only process TX BDs that have IDR bit set */
+		if (!(si_idr & BIT(v->tx_ring[i].index)))
+			continue;
  		if (!enetc_clean_tx_ring(&v->tx_ring[i], budget))
  			complete = false;
+	}
+
+	si_idr = enetc_rd_reg_hot(rx_ring->idr);
  	enetc_unlock_mdio();

-	prog = rx_ring->xdp.prog;
-	if (prog)
-		work_done = enetc_clean_rx_ring_xdp(rx_ring, napi, budget, prog);
-	else
-		work_done = enetc_clean_rx_ring(rx_ring, napi, budget);
+	if (si_idr & BIT(rx_ring->index)) {
+		prog = rx_ring->xdp.prog;
+		if (prog)
+			work_done = enetc_clean_rx_ring_xdp(rx_ring, napi, budget, prog);
+		else
+			work_done = enetc_clean_rx_ring(rx_ring, napi, budget);
+	} else {
+		work_done = 0;
+	}
  	if (work_done == budget)
  		complete = false;
  	if (work_done)
-- 
2.43.0
---
 From a144fc991784ea7ee51c4a46d03b46ee4a17956a Mon Sep 17 00:00:00 2001
From: Zefir Kurtisi <zefir.kurtisi@westermo.com>
Date: Fri, 27 Feb 2026 11:21:49 +0100
Subject: [PATCH 2/2] net: enetc: clear sticky IDR bit causing IRQ-storms

We observed enetc entering a state with a sticky IDR bit
set causing IRQ-storms. It could be tracked down to
a zero-sized IP frame being transmitted, which causes the
HW to issue IDR but not advance the TX BD.

Vladimir Oltean fixed this by ensuring TX frames are padded
to the minimum size.

This adds a fail-safe mechanism to recover from such issues
just in case there are other stimuli causing taking the
same code path.

The mechanism bases on the fact that the current TX BD is
being processed because it issued a IDR. If contrary to
that no descriptors are pending, a warning is issued and
the TBaIDR is cleared.

Signed-off-by: Zefir Kurtisi <zefir.kurtisi@westermo.com>
---
  drivers/net/ethernet/freescale/enetc/enetc.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c 
b/drivers/net/ethernet/freescale/enetc/enetc.c
index e9d600506d83..d8680a5768b5 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1229,6 +1229,14 @@ static bool enetc_clean_tx_ring(struct enetc_bdr 
*tx_ring, int napi_budget)

  	bds_to_clean = enetc_bd_ready_count(tx_ring, i);

+	if (!bds_to_clean) {
+		/* this shall not happen, since IDR indicated data available */
+		netdev_warn(ndev, "TX[%d]: IDR set, but no BDs to clean => "
+			    "clearing IDR to recover\n", tx_ring->index);
+		enetc_wr_reg_hot(tx_ring->idr, BIT(tx_ring->index) |
+				 BIT(16 + tx_ring->index));
+	}
+
  	do_twostep_tstamp = false;

  	while (bds_to_clean && tx_frm_cnt < ENETC_DEFAULT_TX_WORK) {
-- 
2.43.0


  reply	other threads:[~2026-02-27 11:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-20 13:29 [PATCH] net: enetc: fix sirq-storm by clearing IDR registers Zefir Kurtisi
2026-02-23 16:32 ` Vladimir Oltean
2026-02-26 12:00   ` Vladimir Oltean
2026-02-26 19:28   ` Zefir Kurtisi
2026-02-26 19:51     ` Vladimir Oltean
2026-02-27 11:28       ` Zefir Kurtisi [this message]
2026-02-27 11:57         ` Vladimir Oltean
2026-02-27 13:03           ` Zefir Kurtisi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=786338e0-0a6b-4b7b-9e22-8253bb655aec@gmail.com \
    --to=zefir.kurtisi@gmail.com \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=vladimir.oltean@nxp.com \
    --cc=wei.fang@nxp.com \
    --cc=xiaoning.wang@nxp.com \
    --cc=zefir.kurtisi@westermo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox