Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH net 0/2] net: enetc: restore RX ring congestion mode after ring reconfiguration
@ 2026-07-28  2:32 wei.fang
  2026-07-28  2:32 ` [PATCH net 1/2] " wei.fang
  2026-07-28  2:32 ` [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4 wei.fang
  0 siblings, 2 replies; 7+ messages in thread
From: wei.fang @ 2026-07-28  2:32 UTC (permalink / raw)
  To: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
	davem, edumazet, kuba, pabeni, Frank.Li
  Cc: wei.fang, imx, netdev, linux-kernel

From: Wei Fang <wei.fang@nxp.com>

The RX BD ring congestion mode (CM) enables the ENETC MAC to generate
PAUSE frames when ingress congestion occurs. It is configured only in
the phylink .mac_link_up() callback, which is invoked when the link
status changes.

However, enetc_reconfigure() tears down and re-creates the RX BD rings
at runtime without any link status change, for example when enabling or
disabling PTP RX hardware timestamping. enetc_setup_rxbdr() rebuilds the
RBMR register from zero, which clears the CM bit, and since the link
status does not change, .mac_link_up() is not called again to restore
it. As a result, flow control silently stops working after such a
reconfiguration.

To solve this issue, track the desired CM state in a software flag
ENETC_RXBDR_CM, which is maintained by the .mac_link_up() /
.mac_link_down() callbacks and consulted by enetc_setup_rxbdr() when the
RX BD rings are (re)configured. Both ENETC v1 and ENETC v4 are affected
and are fixed in the same way.

Wei Fang (2):
  net: enetc: restore RX ring congestion mode after ring reconfiguration
  net: enetc: restore RX ring congestion mode for ENETC v4

 drivers/net/ethernet/freescale/enetc/enetc.c     | 4 ++++
 drivers/net/ethernet/freescale/enetc/enetc.h     | 1 +
 drivers/net/ethernet/freescale/enetc/enetc4_pf.c | 8 ++++++++
 drivers/net/ethernet/freescale/enetc/enetc_pf.c  | 5 +++++
 4 files changed, 18 insertions(+)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net 1/2] net: enetc: restore RX ring congestion mode after ring reconfiguration
  2026-07-28  2:32 [PATCH net 0/2] net: enetc: restore RX ring congestion mode after ring reconfiguration wei.fang
@ 2026-07-28  2:32 ` wei.fang
  2026-07-29  2:29   ` sashiko-bot
  2026-07-28  2:32 ` [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4 wei.fang
  1 sibling, 1 reply; 7+ messages in thread
From: wei.fang @ 2026-07-28  2:32 UTC (permalink / raw)
  To: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
	davem, edumazet, kuba, pabeni, Frank.Li
  Cc: wei.fang, imx, netdev, linux-kernel

From: Wei Fang <wei.fang@nxp.com>

The congestion mode (CM) of the RX BD rings is only configured in the
phylink .mac_link_up() callback enetc_pl_mac_link_up(), where the
ENETC_RBMR_CM bit is set when tx_pause is enabled. This callback is
only invoked when the link status changes.

However, enetc_reconfigure() tears down and re-creates the RX BD rings
at runtime without any link status change, for example when attaching
or detaching an XDP program, or when enabling/disabling PTP RX hardware
timestamping. During ring reconfiguration, enetc_setup_rxbdr() rebuilds
the RBMR register starting from zero, which clears the ENETC_RBMR_CM
bit. Because the link status remains unchanged, enetc_pl_mac_link_up()
is not called again, so the CM bit is never restored.

As a result, the ENETC MAC can no longer generate PAUSE frames when
congestion occurs at the ingress direction, and flow control stops
working after such a reconfiguration.

Track the desired CM state in a software flag ENETC_RXBDR_CM. Set or
clear the flag in enetc_pl_mac_link_up() according to tx_pause, and
clear it in enetc_pl_mac_link_down(). When the RX BD rings are
(re)configured, enetc_setup_rxbdr() consults this flag and restores
the ENETC_RBMR_CM bit accordingly, so flow control survives ring
reconfiguration even when the link status does not change.

Fixes: 5093406c784f ("net: enetc: implement ring reconfiguration procedure for PTP RX timestamping")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c    | 4 ++++
 drivers/net/ethernet/freescale/enetc/enetc.h    | 1 +
 drivers/net/ethernet/freescale/enetc/enetc_pf.c | 5 +++++
 3 files changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 8e3f345dd9aa..9282e13ccfb2 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -2631,6 +2631,7 @@ static void enetc_setup_txbdr(struct enetc_hw *hw, struct enetc_bdr *tx_ring)
 static void enetc_setup_rxbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring,
 			      bool extended)
 {
+	struct enetc_ndev_priv *priv = netdev_priv(rx_ring->ndev);
 	int idx = rx_ring->index;
 	u32 rbmr = 0;
 
@@ -2666,6 +2667,9 @@ static void enetc_setup_rxbdr(struct enetc_hw *hw, struct enetc_bdr *rx_ring,
 	if (rx_ring->ndev->features & NETIF_F_HW_VLAN_CTAG_RX)
 		rbmr |= ENETC_RBMR_VTE;
 
+	if (test_bit(ENETC_RXBDR_CM, &priv->flags))
+		rbmr |= ENETC_RBMR_CM;
+
 	rx_ring->rcir = hw->reg + ENETC_BDR(RX, idx, ENETC_RBCIR);
 	rx_ring->idr = hw->reg + ENETC_SIRXIDR;
 
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index 04a5dd5ea6c7..341f54856deb 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -419,6 +419,7 @@ enum enetc_active_offloads {
 enum enetc_flags_bit {
 	ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS = 0,
 	ENETC_TX_DOWN,
+	ENETC_RXBDR_CM,
 };
 
 /* interrupt coalescing modes */
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 2d687bb8c3a0..3fb689d27a02 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -633,6 +633,8 @@ static void enetc_pl_mac_link_up(struct phylink_config *config,
 	}
 
 	if (tx_pause) {
+		set_bit(ENETC_RXBDR_CM, &priv->flags);
+
 		/* When the port first enters congestion, send a PAUSE request
 		 * with the maximum number of quanta. When the port exits
 		 * congestion, it will automatically send a PAUSE frame with
@@ -652,6 +654,8 @@ static void enetc_pl_mac_link_up(struct phylink_config *config,
 		 */
 		pause_on_thresh = 3 * ENETC_MAC_MAXFRM_SIZE;
 		pause_off_thresh = 1 * ENETC_MAC_MAXFRM_SIZE;
+	} else {
+		clear_bit(ENETC_RXBDR_CM, &priv->flags);
 	}
 
 	enetc_port_mac_wr(si, ENETC_PM0_PAUSE_QUANTA, init_quanta);
@@ -683,6 +687,7 @@ static void enetc_pl_mac_link_down(struct phylink_config *config,
 	struct enetc_ndev_priv *priv;
 
 	priv = netdev_priv(si->ndev);
+	clear_bit(ENETC_RXBDR_CM, &priv->flags);
 
 	if (si->hw_features & ENETC_SI_F_QBU)
 		enetc_mm_link_state_update(priv, false);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4
  2026-07-28  2:32 [PATCH net 0/2] net: enetc: restore RX ring congestion mode after ring reconfiguration wei.fang
  2026-07-28  2:32 ` [PATCH net 1/2] " wei.fang
@ 2026-07-28  2:32 ` wei.fang
  2026-07-29  2:29   ` sashiko-bot
  1 sibling, 1 reply; 7+ messages in thread
From: wei.fang @ 2026-07-28  2:32 UTC (permalink / raw)
  To: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
	davem, edumazet, kuba, pabeni, Frank.Li
  Cc: wei.fang, imx, netdev, linux-kernel

From: Wei Fang <wei.fang@nxp.com>

ENETC v4 has the same problem as ENETC v1: the RX BD ring congestion
mode (CM) is only configured in the phylink .mac_link_up() callback,
so it is cleared when enetc_reconfigure() rebuilds the RX BD rings at
runtime (for example when enabling or disabling PTP RX hardware
timestamping) without a link status change, and it is never restored.
As a result, the MAC can no longer generate PAUSE frames on ingress
congestion and flow control stops working.

Fix it in the same way as ENETC v1, by tracking the desired CM state in
the software flag ENETC_RXBDR_CM. Set or clear the flag in
enetc4_set_tx_pause() according to tx_pause, and clear it in
enetc4_pl_mac_link_down(). enetc_setup_rxbdr() already restores the CM
bit from this flag when the RX BD rings are (re)configured.

Fixes: f5b9a1cde0a2 ("net: enetc: add PTP synchronization support for ENETC v4")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc4_pf.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
index 437a15bbb47b..8db5a47fdbae 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
@@ -748,6 +748,7 @@ static void enetc4_set_rx_pause(struct enetc_pf *pf, bool rx_pause)
 
 static void enetc4_set_tx_pause(struct enetc_pf *pf, int num_rxbdr, bool tx_pause)
 {
+	struct enetc_ndev_priv *priv = netdev_priv(pf->si->ndev);
 	u32 pause_off_thresh = 0, pause_on_thresh = 0;
 	u32 init_quanta = 0, refresh_quanta = 0;
 	struct enetc_hw *hw = &pf->si->hw;
@@ -764,6 +765,8 @@ static void enetc4_set_tx_pause(struct enetc_pf *pf, int num_rxbdr, bool tx_paus
 	}
 
 	if (tx_pause) {
+		set_bit(ENETC_RXBDR_CM, &priv->flags);
+
 		/* When the port first enters congestion, send a PAUSE request
 		 * with the maximum number of quanta. When the port exits
 		 * congestion, it will automatically send a PAUSE frame with
@@ -783,6 +786,8 @@ static void enetc4_set_tx_pause(struct enetc_pf *pf, int num_rxbdr, bool tx_paus
 		 */
 		pause_on_thresh = 3 * ENETC_MAC_MAXFRM_SIZE;
 		pause_off_thresh = 1 * ENETC_MAC_MAXFRM_SIZE;
+	} else {
+		clear_bit(ENETC_RXBDR_CM, &priv->flags);
 	}
 
 	enetc_port_mac_wr(pf->si, ENETC4_PM_PAUSE_QUANTA(0), init_quanta);
@@ -949,7 +954,10 @@ static void enetc4_pl_mac_link_down(struct phylink_config *config,
 				    phy_interface_t interface)
 {
 	struct enetc_pf *pf = phylink_to_enetc_pf(config);
+	struct enetc_ndev_priv *priv;
 
+	priv = netdev_priv(pf->si->ndev);
+	clear_bit(ENETC_RXBDR_CM, &priv->flags);
 	enetc4_mac_rx_graceful_stop(pf);
 	enetc4_mac_tx_graceful_stop(pf);
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4
  2026-07-28  2:32 ` [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4 wei.fang
@ 2026-07-29  2:29   ` sashiko-bot
  2026-07-29  6:16     ` Wei Fang (OSS)
  0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-07-29  2:29 UTC (permalink / raw)
  To: wei.fang; +Cc: Frank.Li, imx

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] State desynchronization due to TOCTOU race condition updating the newly added software flag.

Pre-existing issues:
- [Critical] Unprotected read-modify-write of hardware registers leads to DMA memory corruption (Use-After-Free).
--

--- Patch [2]: [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4 ---
commit c984fc5e7d3c90c2dfd0f53ae9ad32adf715c689
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: restore RX ring congestion mode for ENETC v4

This commit fixes an issue where the RX BD ring congestion mode is cleared
when RX BD rings are rebuilt at runtime without a link status change. It
tracks the desired CM state in the software flag ENETC_RXBDR_CM so it can
be restored during reconfiguration.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index 437a15bbb47ba..8db5a47fdbae4 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -748,6 +748,7 @@ static void enetc4_set_rx_pause(struct enetc_pf *pf, bool rx_pause)
>  
>  static void enetc4_set_tx_pause(struct enetc_pf *pf, int num_rxbdr, bool tx_pause)
>  {
> +	struct enetc_ndev_priv *priv = netdev_priv(pf->si->ndev);
>  	u32 pause_off_thresh = 0, pause_on_thresh = 0;
>  	u32 init_quanta = 0, refresh_quanta = 0;
>  	struct enetc_hw *hw = &pf->si->hw;
> @@ -764,6 +765,8 @@ static void enetc4_set_tx_pause(struct enetc_pf *pf, int num_rxbdr, bool tx_paus
>  	int i;
>  
>  	for (i = 0; i < num_rxbdr; i++) {
>  		old_rbmr = enetc_rxbdr_rd(hw, i, ENETC_RBMR);
>  		rbmr = u32_replace_bits(old_rbmr, tx_pause ? 1 : 0, ENETC_RBMR_CM);
>  		if (rbmr == old_rbmr)
>  			continue;
>  
>  		enetc_rxbdr_wr(hw, i, ENETC_RBMR, rbmr);
>  	}

[Severity: Critical]
This is a pre-existing issue, but could this unprotected read-modify-write
lead to memory corruption if it races with enetc_disable_rxbdr()?

If phylink_resolve() executes enetc4_set_tx_pause() concurrently with
enetc_reconfigure(), this loop can read ENETC_RBMR before the ring is
torn down. 

If it writes the modified value back after enetc_disable_rxbdr()
clears the enable bit, it would inadvertently re-enable the RX ring in
hardware:

drivers/net/ethernet/freescale/enetc/enetc.c:enetc_disable_rxbdr() {
    ...
    /* disable EN bit on ring */
    enetc_rxbdr_wr(hw, idx, ENETC_RBMR, 0);
    ...
}

Because the software frees the associated DMA buffers during teardown, the
hardware would then DMA incoming packets directly into freed memory.

>  
>  	if (tx_pause) {
> +		set_bit(ENETC_RXBDR_CM, &priv->flags);
> +

[Severity: High]
Can this result in the software state and hardware register becoming
desynchronized?

Since ENETC_RXBDR_CM is set after the hardware registers are updated in the
loop above, there appears to be a race window. If enetc_setup_rxbdr() is
called by a concurrent enetc_reconfigure() during this window, it would
see the unset flag:

drivers/net/ethernet/freescale/enetc/enetc.c:enetc_setup_rxbdr() {
    ...
    if (test_bit(ENETC_RXBDR_CM, &priv->flags))
        rbmr |= ENETC_RBMR_CM;
    ...
}

This would overwrite the hardware register without the CM bit. Then this
thread sets the software flag, leaving flow control disabled in hardware
when the software state expects it to be enabled.

>  		/* When the port first enters congestion, send a PAUSE request
>  		 * with the maximum number of quanta. When the port exits

[ ... ]

> @@ -949,7 +954,10 @@ static void enetc4_pl_mac_link_down(struct phylink_config *config,
>  				    phy_interface_t interface)
>  {
>  	struct enetc_pf *pf = phylink_to_enetc_pf(config);
> +	struct enetc_ndev_priv *priv;
>  
> +	priv = netdev_priv(pf->si->ndev);
> +	clear_bit(ENETC_RXBDR_CM, &priv->flags);
>  	enetc4_mac_rx_graceful_stop(pf);
>  	enetc4_mac_tx_graceful_stop(pf);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728023217.2499420-1-wei.fang@oss.nxp.com?part=2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 1/2] net: enetc: restore RX ring congestion mode after ring reconfiguration
  2026-07-28  2:32 ` [PATCH net 1/2] " wei.fang
@ 2026-07-29  2:29   ` sashiko-bot
  2026-07-29  7:00     ` Wei Fang (OSS)
  0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-07-29  2:29 UTC (permalink / raw)
  To: wei.fang; +Cc: Frank.Li, imx

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Lockless race between phylink's mac_link_up and driver reconfiguration corrupts hardware registers and newly introduced software state, leading to DMA use-after-free and loss of flow control.
--

--- Patch [1]: [PATCH net 1/2] net: enetc: restore RX ring congestion mode after ring reconfiguration ---
commit 299f421076cc1f539a88e83a486105279c6c0395
Author: Wei Fang <wei.fang@nxp.com>
Subject: net: enetc: restore RX ring congestion mode after ring reconfiguration

This commit addresses an issue where the RX ring congestion mode is lost
during runtime ring reconfiguration. It introduces a software flag to
track the congestion mode state and restores it when the RX ring is rebuilt,
ensuring flow control continues working.

> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -633,6 +633,8 @@ static void enetc_pl_mac_link_up(struct phylink_config *config,
>  	}
>  
>  	if (tx_pause) {
> +		set_bit(ENETC_RXBDR_CM, &priv->flags);
> +

[Severity: Critical]
This is a pre-existing issue, but can this lockless read-modify-write on
the ENETC_RBMR register race with driver reconfiguration?

Looking at enetc_pl_mac_link_up() in enetc_pf.c, there is an unsynchronized
read-modify-write loop on ENETC_RBMR just before this flag update:

	for (idx = 0; idx < priv->num_rx_rings; idx++) {
		rbmr = enetc_rxbdr_rd(hw, idx, ENETC_RBMR);
		...
		enetc_rxbdr_wr(hw, idx, ENETC_RBMR, rbmr);
	}

If phylink's mac_link_up executes asynchronously in the phylink_resolve
workqueue, could it run concurrently with a user triggering a driver
reconfiguration under rtnl_lock (like attaching an XDP program)?

If so, enetc_reconfigure() might call enetc_disable_rxbdr(), which clears
the ENETC_RBMR register to disable the ring:

	/* disable EN bit on ring */
	enetc_rxbdr_wr(hw, idx, ENETC_RBMR, 0);

If mac_link_up() reads the ENETC_RBMR register before it is cleared by
reconfiguration, and writes it back after it is cleared, it will write
back the stale ENETC_RBMR_EN (enable) bit.

Since enetc_reconfigure() proceeds to call enetc_free_rxtx_rings() to free
the DMA memory, re-enabling the hardware RX ring at this moment would cause
the hardware to DMA incoming network packets into freed kernel memory.

Additionally, because the new ENETC_RXBDR_CM software flag is set here after
the hardware read-modify-write loop, concurrent readers like
enetc_setup_rxbdr() could observe a stale flag state and permanently lose
flow control.

>  		/* When the port first enters congestion, send a PAUSE request
>  		 * with the maximum number of quanta. When the port exits
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728023217.2499420-1-wei.fang@oss.nxp.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4
  2026-07-29  2:29   ` sashiko-bot
@ 2026-07-29  6:16     ` Wei Fang (OSS)
  0 siblings, 0 replies; 7+ messages in thread
From: Wei Fang (OSS) @ 2026-07-29  6:16 UTC (permalink / raw)
  To: sashiko-reviews@lists.linux.dev
  Cc: Frank.Li@kernel.org, imx@lists.linux.dev, network dev

> >  static void enetc4_set_tx_pause(struct enetc_pf *pf, int num_rxbdr, bool
> tx_pause)
> >  {
> > +	struct enetc_ndev_priv *priv = netdev_priv(pf->si->ndev);
> >  	u32 pause_off_thresh = 0, pause_on_thresh = 0;
> >  	u32 init_quanta = 0, refresh_quanta = 0;
> >  	struct enetc_hw *hw = &pf->si->hw;
> > @@ -764,6 +765,8 @@ static void enetc4_set_tx_pause(struct enetc_pf *pf,
> int num_rxbdr, bool tx_paus
> >  	int i;
> >
> >  	for (i = 0; i < num_rxbdr; i++) {
> >  		old_rbmr = enetc_rxbdr_rd(hw, i, ENETC_RBMR);
> >  		rbmr = u32_replace_bits(old_rbmr, tx_pause ? 1 : 0,
> ENETC_RBMR_CM);
> >  		if (rbmr == old_rbmr)
> >  			continue;
> >
> >  		enetc_rxbdr_wr(hw, i, ENETC_RBMR, rbmr);
> >  	}
> 
> [Severity: Critical]
> This is a pre-existing issue, but could this unprotected read-modify-write
> lead to memory corruption if it races with enetc_disable_rxbdr()?

Yes, this is a potential issue, l can add a spinlock to fix this corner issue,
but I'm not sure if it is worth doing this, because the benefits are minimal.

> 
> If phylink_resolve() executes enetc4_set_tx_pause() concurrently with
> enetc_reconfigure(), this loop can read ENETC_RBMR before the ring is
> torn down.
> 
> If it writes the modified value back after enetc_disable_rxbdr()
> clears the enable bit, it would inadvertently re-enable the RX ring in
> hardware:
> 
> drivers/net/ethernet/freescale/enetc/enetc.c:enetc_disable_rxbdr() {
>     ...
>     /* disable EN bit on ring */
>     enetc_rxbdr_wr(hw, idx, ENETC_RBMR, 0);
>     ...
> }
> 
> Because the software frees the associated DMA buffers during teardown, the
> hardware would then DMA incoming packets directly into freed memory.
> 
> >
> >  	if (tx_pause) {
> > +		set_bit(ENETC_RXBDR_CM, &priv->flags);
> > +
> 
> [Severity: High]
> Can this result in the software state and hardware register becoming
> desynchronized?
> 
> Since ENETC_RXBDR_CM is set after the hardware registers are updated in the
> loop above, there appears to be a race window. If enetc_setup_rxbdr() is
> called by a concurrent enetc_reconfigure() during this window, it would
> see the unset flag:
> 
> drivers/net/ethernet/freescale/enetc/enetc.c:enetc_setup_rxbdr() {
>     ...
>     if (test_bit(ENETC_RXBDR_CM, &priv->flags))
>         rbmr |= ENETC_RBMR_CM;
>     ...
> }
> 
> This would overwrite the hardware register without the CM bit. Then this
> thread sets the software flag, leaving flow control disabled in hardware
> when the software state expects it to be enabled.
> 
> >  		/* When the port first enters congestion, send a PAUSE request
> >  		 * with the maximum number of quanta. When the port exits
> 
> [ ... ]
> 
> > @@ -949,7 +954,10 @@ static void enetc4_pl_mac_link_down(struct
> phylink_config *config,
> >  				    phy_interface_t interface)
> >  {
> >  	struct enetc_pf *pf = phylink_to_enetc_pf(config);
> > +	struct enetc_ndev_priv *priv;
> >
> > +	priv = netdev_priv(pf->si->ndev);
> > +	clear_bit(ENETC_RXBDR_CM, &priv->flags);
> >  	enetc4_mac_rx_graceful_stop(pf);
> >  	enetc4_mac_tx_graceful_stop(pf);
> >  }
> 
> --
> Sashiko AI
> review · https://sashiko.dev/#/patchset/20260728023217.2499420-1-wei.fang
> @oss.nxp.com?part=2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH net 1/2] net: enetc: restore RX ring congestion mode after ring reconfiguration
  2026-07-29  2:29   ` sashiko-bot
@ 2026-07-29  7:00     ` Wei Fang (OSS)
  0 siblings, 0 replies; 7+ messages in thread
From: Wei Fang (OSS) @ 2026-07-29  7:00 UTC (permalink / raw)
  To: sashiko-reviews@lists.linux.dev
  Cc: Frank.Li@kernel.org, imx@lists.linux.dev, network dev

> > --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> > @@ -633,6 +633,8 @@ static void enetc_pl_mac_link_up(struct
> phylink_config *config,
> >  	}
> >
> >  	if (tx_pause) {
> > +		set_bit(ENETC_RXBDR_CM, &priv->flags);
> > +
> 
> [Severity: Critical]
> This is a pre-existing issue, but can this lockless read-modify-write on
> the ENETC_RBMR register race with driver reconfiguration?
> 
> Looking at enetc_pl_mac_link_up() in enetc_pf.c, there is an unsynchronized
> read-modify-write loop on ENETC_RBMR just before this flag update:
> 
> 	for (idx = 0; idx < priv->num_rx_rings; idx++) {
> 		rbmr = enetc_rxbdr_rd(hw, idx, ENETC_RBMR);
> 		...
> 		enetc_rxbdr_wr(hw, idx, ENETC_RBMR, rbmr);
> 	}
> 
> If phylink's mac_link_up executes asynchronously in the phylink_resolve
> workqueue, could it run concurrently with a user triggering a driver
> reconfiguration under rtnl_lock (like attaching an XDP program)?
> 
> If so, enetc_reconfigure() might call enetc_disable_rxbdr(), which clears
> the ENETC_RBMR register to disable the ring:
> 
> 	/* disable EN bit on ring */
> 	enetc_rxbdr_wr(hw, idx, ENETC_RBMR, 0);
> 
> If mac_link_up() reads the ENETC_RBMR register before it is cleared by
> reconfiguration, and writes it back after it is cleared, it will write
> back the stale ENETC_RBMR_EN (enable) bit.
> 
> Since enetc_reconfigure() proceeds to call enetc_free_rxtx_rings() to free
> the DMA memory, re-enabling the hardware RX ring at this moment would
> cause
> the hardware to DMA incoming network packets into freed kernel memory.
> 
> Additionally, because the new ENETC_RXBDR_CM software flag is set here after
> the hardware read-modify-write loop, concurrent readers like
> enetc_setup_rxbdr() could observe a stale flag state and permanently lose
> flow control.
> 
> >  		/* When the port first enters congestion, send a PAUSE request
> >  		 * with the maximum number of quanta. When the port exits
> [ ... ]

I will improve this patch set to solve the pre-existing issue.

--
pw-bot: cr


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-29  7:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-28  2:32 [PATCH net 0/2] net: enetc: restore RX ring congestion mode after ring reconfiguration wei.fang
2026-07-28  2:32 ` [PATCH net 1/2] " wei.fang
2026-07-29  2:29   ` sashiko-bot
2026-07-29  7:00     ` Wei Fang (OSS)
2026-07-28  2:32 ` [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4 wei.fang
2026-07-29  2:29   ` sashiko-bot
2026-07-29  6:16     ` Wei Fang (OSS)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox