[PATCH net 0/8] net: bcmgenet: Collection of bug fixes

[PATCH net 0/8] net: bcmgenet: Collection of bug fixes

[Florian Fainelli]

This patch series contains a collection of bug fixes found during a
LLM-assisted coding session.

Florian Fainelli (8):
  net: bcmgenet: fix 64-bit RTNL stats reading in ethtool on 32-bit
    systems
  net: bcmgenet: initialize u64 stats seq counter for all queues
  net: bcmgenet: do not skip WoL power up on GENET V1
  net: bcmgenet: clean up RX NAPI on bcmgenet_init_rx_queues failure
  net: bcmgenet: acquire ring lock with BH disabled in
    bcmgenet_dump_tx_queue
  net: bcmgenet: validate Ethernet address in bcmgenet_set_mac_addr
  net: bcmgenet: zero-initialize Transmit Status Block in
    bcmgenet_add_tsb
  net: bcmgenet: mask DMA_TIMEOUT_MASK when reading DMA_RING0_TIMEOUT

 .../net/ethernet/broadcom/genet/bcmgenet.c    | 34 ++++++++++++-------
 1 file changed, 21 insertions(+), 13 deletions(-)

-- 
2.34.1

[PATCH net 1/8] net: bcmgenet: fix 64-bit RTNL stats reading in ethtool on 32-bit systems

[Florian Fainelli]

When bcmgenet was converted to 64-bit statistics, STAT_RTNL members were
switched to point into struct rtnl_link_stats64, whose fields are 64-bit
(__u64) regardless of architecture.

However, bcmgenet_get_ethtool_stats() retained a legacy check:
  if (sizeof(unsigned long) != sizeof(u32) &&
      s->stat_sizeof == sizeof(unsigned long))

On 32-bit systems, sizeof(unsigned long) == sizeof(u32), causing this
condition to evaluate to false. As a result, 64-bit RTNL stats fields were
read via *(u32 *)p. On 32-bit Big-Endian systems (such as MIPS BE), this
reads the high 32 bits and returns 0 until the counter exceeds 4GB; on
32-bit Little-Endian systems (such as 32-bit ARM), the value is truncated
to 32 bits.

Fix this by checking if s->stat_sizeof == sizeof(u64) so 64-bit fields are
always read as 64-bit values.

Fixes: 59aa6e3072aa ("net: bcmgenet: switch to use 64bit statistics")
Assisted-by: LLM
Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: I3e9f2f8d4fd136392148d6c0cd4eb40f7ccbe4ef
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index b916080f4ff1..7b089de9484e 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1346,9 +1346,8 @@ static void bcmgenet_get_ethtool_stats(struct net_device *dev,
 				p = (char *)&stats64;
 
 			p += s->stat_offset;
-			if (sizeof(unsigned long) != sizeof(u32) &&
-				s->stat_sizeof == sizeof(unsigned long))
-				data[i] = *(unsigned long *)p;
+			if (s->stat_sizeof == sizeof(u64))
+				data[i] = *(u64 *)p;
 			else
 				data[i] = *(u32 *)p;
 		}
-- 
2.34.1

Re: [PATCH net 1/8] net: bcmgenet: fix 64-bit RTNL stats reading in ethtool on 32-bit systems

[Nicolai Buchwitz]

On 18.9.2026 02:07, Florian Fainelli wrote:
> When bcmgenet was converted to 64-bit statistics, STAT_RTNL members 
> were
> switched to point into struct rtnl_link_stats64, whose fields are 
> 64-bit
> (__u64) regardless of architecture.
> 
> However, bcmgenet_get_ethtool_stats() retained a legacy check:
>   if (sizeof(unsigned long) != sizeof(u32) &&
>       s->stat_sizeof == sizeof(unsigned long))
> 
> On 32-bit systems, sizeof(unsigned long) == sizeof(u32), causing this
> condition to evaluate to false. As a result, 64-bit RTNL stats fields 
> were
> read via *(u32 *)p. On 32-bit Big-Endian systems (such as MIPS BE), 
> this
> reads the high 32 bits and returns 0 until the counter exceeds 4GB; on
> 32-bit Little-Endian systems (such as 32-bit ARM), the value is 
> truncated
> to 32 bits.
> 
> Fix this by checking if s->stat_sizeof == sizeof(u64) so 64-bit fields 
> are
> always read as 64-bit values.
> 
> Fixes: 59aa6e3072aa ("net: bcmgenet: switch to use 64bit statistics")
> Assisted-by: LLM
> Co-authored-by: Cursor <cursoragent@cursor.com>
> Change-Id: I3e9f2f8d4fd136392148d6c0cd4eb40f7ccbe4ef
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index b916080f4ff1..7b089de9484e 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1346,9 +1346,8 @@ static void bcmgenet_get_ethtool_stats(struct 
> net_device *dev,
>  				p = (char *)&stats64;
> 
>  			p += s->stat_offset;
> -			if (sizeof(unsigned long) != sizeof(u32) &&
> -				s->stat_sizeof == sizeof(unsigned long))
> -				data[i] = *(unsigned long *)p;
> +			if (s->stat_sizeof == sizeof(u64))
> +				data[i] = *(u64 *)p;
>  			else
>  				data[i] = *(u32 *)p;
>  		}

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

[PATCH net 2/8] net: bcmgenet: initialize u64 stats seq counter for all queues

[Florian Fainelli]

bcmgenet_gstrings_stats statically defines ethtool statistics for queues
0 through GENET_MAX_MQ_CNT (4). However, bcmgenet_probe() only initialized
the u64_stats_sync seq counter up to priv->hw_params->rx_queues and
priv->hw_params->tx_queues.

Since priv->hw_params->rx_queues is 0 across all hardware versions (and
priv->hw_params->tx_queues is 0 on GENET V1), rings 1..4 have uninitialized
u64_stats_sync structures. When ethtool -S is run on 32-bit kernels,
bcmgenet_get_ethtool_stats() reads stats from rx_rings[1..4], causing
lockdep warnings due to the uninitialized sequence counters.

Initialize the sequence counters for all GENET_MAX_MQ_CNT + 1 queues.

Fixes: ffc2c8c4a714 ("net: bcmgenet: Initialize u64 stats seq counter")
Assisted-by: LLM
Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: I6c3debbd9fa5e7a151789fccf3d9a38db184f434
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 7b089de9484e..055e1362173b 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -4134,10 +4134,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
 		priv->rx_rings[i].rx_max_coalesced_frames = 1;
 
 	/* Initialize u64 stats seq counter for 32bit machines */
-	for (i = 0; i <= priv->hw_params->rx_queues; i++)
+	for (i = 0; i <= GENET_MAX_MQ_CNT; i++) {
 		u64_stats_init(&priv->rx_rings[i].stats64.syncp);
-	for (i = 0; i <= priv->hw_params->tx_queues; i++)
 		u64_stats_init(&priv->tx_rings[i].stats64.syncp);
+	}
 
 	/* libphy will determine the link state */
 	netif_carrier_off(dev);
-- 
2.34.1

Re: [PATCH net 2/8] net: bcmgenet: initialize u64 stats seq counter for all queues

[Nicolai Buchwitz]

On 18.9.2026 02:07, Florian Fainelli wrote:
> bcmgenet_gstrings_stats statically defines ethtool statistics for 
> queues
> 0 through GENET_MAX_MQ_CNT (4). However, bcmgenet_probe() only 
> initialized
> the u64_stats_sync seq counter up to priv->hw_params->rx_queues and
> priv->hw_params->tx_queues.
> 
> Since priv->hw_params->rx_queues is 0 across all hardware versions (and
> priv->hw_params->tx_queues is 0 on GENET V1), rings 1..4 have 
> uninitialized
> u64_stats_sync structures. When ethtool -S is run on 32-bit kernels,
> bcmgenet_get_ethtool_stats() reads stats from rx_rings[1..4], causing
> lockdep warnings due to the uninitialized sequence counters.
> 
> Initialize the sequence counters for all GENET_MAX_MQ_CNT + 1 queues.
> 
> Fixes: ffc2c8c4a714 ("net: bcmgenet: Initialize u64 stats seq counter")
> Assisted-by: LLM
> Co-authored-by: Cursor <cursoragent@cursor.com>
> Change-Id: I6c3debbd9fa5e7a151789fccf3d9a38db184f434
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 7b089de9484e..055e1362173b 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -4134,10 +4134,10 @@ static int bcmgenet_probe(struct 
> platform_device *pdev)
>  		priv->rx_rings[i].rx_max_coalesced_frames = 1;
> 
>  	/* Initialize u64 stats seq counter for 32bit machines */
> -	for (i = 0; i <= priv->hw_params->rx_queues; i++)
> +	for (i = 0; i <= GENET_MAX_MQ_CNT; i++) {
>  		u64_stats_init(&priv->rx_rings[i].stats64.syncp);
> -	for (i = 0; i <= priv->hw_params->tx_queues; i++)
>  		u64_stats_init(&priv->tx_rings[i].stats64.syncp);
> +	}
> 
>  	/* libphy will determine the link state */
>  	netif_carrier_off(dev);

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

[PATCH net 3/8] net: bcmgenet: do not skip WoL power up on GENET V1

[Florian Fainelli]

bcmgenet_power_up() had an early check for bcmgenet_has_ext(priv) before
dispatching by power mode. GENET V1 does not have the EXT block (unlike
GENET V2+), which causes bcmgenet_power_up() to immediately return 0.

As a consequence, when waking up from GENET_POWER_WOL_MAGIC on GENET V1,
bcmgenet_wol_power_up_cfg() is never invoked to disable the WoL clock,
clear wake event masks, and restore normal PHY and MAC operations.

Move the bcmgenet_has_ext() checks to the GENET_POWER_PASSIVE and
GENET_POWER_CABLE_SENSE cases where the EXT registers are actually
accessed, allowing GENET_POWER_WOL_MAGIC cleanup to execute on all
hardware versions.

Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Assisted-by: LLM
Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: Id6c85b2790191786aa535d71fea357d1cd49a2ec
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 055e1362173b..bfb74dca55d6 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1762,13 +1762,12 @@ static int bcmgenet_power_up(struct bcmgenet_priv *priv,
 	int ret = 0;
 	u32 reg;
 
-	if (!bcmgenet_has_ext(priv))
-		return ret;
-
-	reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
-
 	switch (mode) {
 	case GENET_POWER_PASSIVE:
+		if (!bcmgenet_has_ext(priv))
+			break;
+
+		reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
 		reg &= ~(EXT_PWR_DOWN_DLL | EXT_PWR_DOWN_BIAS |
 			 EXT_ENERGY_DET_MASK);
 		if (GENET_IS_V5(priv) && !bcmgenet_has_ephy_16nm(priv)) {
@@ -1792,8 +1791,12 @@ static int bcmgenet_power_up(struct bcmgenet_priv *priv,
 		break;
 
 	case GENET_POWER_CABLE_SENSE:
+		if (!bcmgenet_has_ext(priv))
+			break;
+
 		/* enable APD */
 		if (!GENET_IS_V5(priv)) {
+			reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
 			reg |= EXT_PWR_DN_EN_LD;
 			bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
 		}
-- 
2.34.1

Re: [PATCH net 3/8] net: bcmgenet: do not skip WoL power up on GENET V1

[Nicolai Buchwitz]

On 18.9.2026 02:07, Florian Fainelli wrote:
> bcmgenet_power_up() had an early check for bcmgenet_has_ext(priv) 
> before
> dispatching by power mode. GENET V1 does not have the EXT block (unlike
> GENET V2+), which causes bcmgenet_power_up() to immediately return 0.
> 
> As a consequence, when waking up from GENET_POWER_WOL_MAGIC on GENET 
> V1,
> bcmgenet_wol_power_up_cfg() is never invoked to disable the WoL clock,
> clear wake event masks, and restore normal PHY and MAC operations.
> 
> Move the bcmgenet_has_ext() checks to the GENET_POWER_PASSIVE and
> GENET_POWER_CABLE_SENSE cases where the EXT registers are actually
> accessed, allowing GENET_POWER_WOL_MAGIC cleanup to execute on all
> hardware versions.
> 
> Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")

c3ae64ae0c08 ("net: bcmgenet: handle GENET_POWER_WOL_MAGIC") ?

> [...]

With the correct Fixes tag:

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

[PATCH net 4/8] net: bcmgenet: clean up RX NAPI on bcmgenet_init_rx_queues failure

[Florian Fainelli]

bcmgenet_init_rx_ring() registers a NAPI instance for each RX ring via
netif_napi_add(). If bcmgenet_init_rx_queues() fails while initializing
subsequent rings, bcmgenet_init_dma() frees the RX buffers and page pools
but fails to clean up already registered NAPI instances with
bcmgenet_fini_rx_napi(). This leaves stale NAPI structs on the net_device
napi_list.

Call bcmgenet_fini_rx_napi() in the error handling path of
bcmgenet_init_dma() when bcmgenet_init_rx_queues() fails.

Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Assisted-by: LLM
Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: I45982e0e198a44f3f56b1300f462870462b81108
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index bfb74dca55d6..ef155a170fa6 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3167,6 +3167,7 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv, bool flush_rx)
 	ret = bcmgenet_init_rx_queues(priv->dev);
 	if (ret) {
 		netdev_err(priv->dev, "failed to initialize Rx queues\n");
+		bcmgenet_fini_rx_napi(priv);
 		bcmgenet_free_rx_buffers(priv);
 		bcmgenet_destroy_rx_page_pools(priv);
 		kfree(priv->rx_cbs);
-- 
2.34.1

Re: [PATCH net 4/8] net: bcmgenet: clean up RX NAPI on bcmgenet_init_rx_queues failure

[Nicolai Buchwitz]

Hi Florian

On 18.9.2026 02:07, Florian Fainelli wrote:
> bcmgenet_init_rx_ring() registers a NAPI instance for each RX ring via
> netif_napi_add(). If bcmgenet_init_rx_queues() fails while initializing
> subsequent rings, bcmgenet_init_dma() frees the RX buffers and page 
> pools
> but fails to clean up already registered NAPI instances with
> bcmgenet_fini_rx_napi(). This leaves stale NAPI structs on the 
> net_device
> napi_list.
> 
> Call bcmgenet_fini_rx_napi() in the error handling path of
> bcmgenet_init_dma() when bcmgenet_init_rx_queues() fails.
> 
> Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
> Assisted-by: LLM
> Co-authored-by: Cursor <cursoragent@cursor.com>
> Change-Id: I45982e0e198a44f3f56b1300f462870462b81108
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index bfb74dca55d6..ef155a170fa6 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3167,6 +3167,7 @@ static int bcmgenet_init_dma(struct bcmgenet_priv 
> *priv, bool flush_rx)
>  	ret = bcmgenet_init_rx_queues(priv->dev);
>  	if (ret) {
>  		netdev_err(priv->dev, "failed to initialize Rx queues\n");
> +		bcmgenet_fini_rx_napi(priv);


napi->dev is still NULL here, bcmgenet_init_rx_ring() only fails before
netif_napi_add().  netif_napi_del() does netdev_lock(napi->dev), so this
oopses on -ENOMEM at first open.

rx_queues is 0 in all hw_params anyway, so there is nothing to clean up.
Drop this one?

>  		bcmgenet_free_rx_buffers(priv);
>  		bcmgenet_destroy_rx_page_pools(priv);
>  		kfree(priv->rx_cbs);

Thanks,
Nicolai

Re: [PATCH net 4/8] net: bcmgenet: clean up RX NAPI on bcmgenet_init_rx_queues failure

[Florian Fainelli]

On 9/18/26 03:43, Nicolai Buchwitz wrote:
> Hi Florian
> 
> On 18.9.2026 02:07, Florian Fainelli wrote:
>> bcmgenet_init_rx_ring() registers a NAPI instance for each RX ring via
>> netif_napi_add(). If bcmgenet_init_rx_queues() fails while initializing
>> subsequent rings, bcmgenet_init_dma() frees the RX buffers and page pools
>> but fails to clean up already registered NAPI instances with
>> bcmgenet_fini_rx_napi(). This leaves stale NAPI structs on the net_device
>> napi_list.
>>
>> Call bcmgenet_fini_rx_napi() in the error handling path of
>> bcmgenet_init_dma() when bcmgenet_init_rx_queues() fails.
>>
>> Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
>> Assisted-by: LLM
>> Co-authored-by: Cursor <cursoragent@cursor.com>
>> Change-Id: I45982e0e198a44f3f56b1300f462870462b81108
>> ---
>>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/ 
>> net/ethernet/broadcom/genet/bcmgenet.c
>> index bfb74dca55d6..ef155a170fa6 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> @@ -3167,6 +3167,7 @@ static int bcmgenet_init_dma(struct 
>> bcmgenet_priv *priv, bool flush_rx)
>>      ret = bcmgenet_init_rx_queues(priv->dev);
>>      if (ret) {
>>          netdev_err(priv->dev, "failed to initialize Rx queues\n");
>> +        bcmgenet_fini_rx_napi(priv);
> 
> 
> napi->dev is still NULL here, bcmgenet_init_rx_ring() only fails before
> netif_napi_add().  netif_napi_del() does netdev_lock(napi->dev), so this
> oopses on -ENOMEM at first open.
> 
> rx_queues is 0 in all hw_params anyway, so there is nothing to clean up.
> Drop this one?

Yes, that's a good point, thanks!
-- 
Florian

[PATCH net 5/8] net: bcmgenet: acquire ring lock with BH disabled in bcmgenet_dump_tx_queue

[Florian Fainelli]

bcmgenet_dump_tx_queue() is called from bcmgenet_timeout() in process or
timer context and acquires ring->lock using spin_lock(). If a softirq
such as TX NAPI (bcmgenet_tx_poll()) fires on the same CPU while the lock
is held, it will deadlock trying to acquire ring->lock.

Use spin_lock_bh() and spin_unlock_bh() in bcmgenet_dump_tx_queue(),
matching bcmgenet_tx_reclaim().

Fixes: 13ea657806cf ("net: bcmgenet: improve TX timeout")
Assisted-by: LLM
Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: I0c4c6392590b2d16abd44f75d3f6974f3e1b874b
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index ef155a170fa6..799da63e3a4e 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3508,14 +3508,14 @@ static void bcmgenet_dump_tx_queue(struct bcmgenet_tx_ring *ring)
 
 	txq = netdev_get_tx_queue(priv->dev, ring->index);
 
-	spin_lock(&ring->lock);
+	spin_lock_bh(&ring->lock);
 	intsts = ~bcmgenet_intrl2_1_readl(priv, INTRL2_CPU_MASK_STATUS);
 	intmsk = 1 << ring->index;
 	c_index = bcmgenet_tdma_ring_readl(priv, ring->index, TDMA_CONS_INDEX);
 	p_index = bcmgenet_tdma_ring_readl(priv, ring->index, TDMA_PROD_INDEX);
 	txq_stopped = netif_tx_queue_stopped(txq);
 	free_bds = ring->free_bds;
-	spin_unlock(&ring->lock);
+	spin_unlock_bh(&ring->lock);
 
 	netif_err(priv, tx_err, priv->dev, "Ring %d queue %d status summary\n"
 		  "TX queue status: %s, interrupts: %s\n"
-- 
2.34.1

Re: [PATCH net 5/8] net: bcmgenet: acquire ring lock with BH disabled in bcmgenet_dump_tx_queue

[Nicolai Buchwitz]

Hi Florian

On 18.9.2026 02:07, Florian Fainelli wrote:
> bcmgenet_dump_tx_queue() is called from bcmgenet_timeout() in process 
> or
> timer context and acquires ring->lock using spin_lock(). If a softirq
> such as TX NAPI (bcmgenet_tx_poll()) fires on the same CPU while the 
> lock
> is held, it will deadlock trying to acquire ring->lock.

AFAIU bcmgenet_timeout() only runs from dev_watchdog(), so BH is already
disabled and no deadlock?

> 
> Use spin_lock_bh() and spin_unlock_bh() in bcmgenet_dump_tx_queue(),
> matching bcmgenet_tx_reclaim().
> 
> Fixes: 13ea657806cf ("net: bcmgenet: improve TX timeout")
> Assisted-by: LLM
> Co-authored-by: Cursor <cursoragent@cursor.com>
> Change-Id: I0c4c6392590b2d16abd44f75d3f6974f3e1b874b
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index ef155a170fa6..799da63e3a4e 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3508,14 +3508,14 @@ static void bcmgenet_dump_tx_queue(struct 
> bcmgenet_tx_ring *ring)
> 
>  	txq = netdev_get_tx_queue(priv->dev, ring->index);
> 
> -	spin_lock(&ring->lock);
> +	spin_lock_bh(&ring->lock);
>  	intsts = ~bcmgenet_intrl2_1_readl(priv, INTRL2_CPU_MASK_STATUS);
>  	intmsk = 1 << ring->index;
>  	c_index = bcmgenet_tdma_ring_readl(priv, ring->index, 
> TDMA_CONS_INDEX);
>  	p_index = bcmgenet_tdma_ring_readl(priv, ring->index, 
> TDMA_PROD_INDEX);
>  	txq_stopped = netif_tx_queue_stopped(txq);
>  	free_bds = ring->free_bds;
> -	spin_unlock(&ring->lock);
> +	spin_unlock_bh(&ring->lock);
> 
>  	netif_err(priv, tx_err, priv->dev, "Ring %d queue %d status 
> summary\n"
>  		  "TX queue status: %s, interrupts: %s\n"

Regards,
Nicolai

Re: [PATCH net 5/8] net: bcmgenet: acquire ring lock with BH disabled in bcmgenet_dump_tx_queue

[Florian Fainelli]

On 9/18/26 03:57, Nicolai Buchwitz wrote:
> Hi Florian
> 
> On 18.9.2026 02:07, Florian Fainelli wrote:
>> bcmgenet_dump_tx_queue() is called from bcmgenet_timeout() in process or
>> timer context and acquires ring->lock using spin_lock(). If a softirq
>> such as TX NAPI (bcmgenet_tx_poll()) fires on the same CPU while the lock
>> is held, it will deadlock trying to acquire ring->lock.
> 
> AFAIU bcmgenet_timeout() only runs from dev_watchdog(), so BH is already
> disabled and no deadlock?

Yes indeed, not sure what I was on with that.
-- 
Florian

[PATCH net 6/8] net: bcmgenet: validate Ethernet address in bcmgenet_set_mac_addr

[Florian Fainelli]

bcmgenet_set_mac_addr() did not check whether the provided MAC address is a
valid Ethernet address before applying it. Userspace could configure an
invalid address (such as all zeroes or a multicast address) while the
interface is down.

Add a call to is_valid_ether_addr() and return -EADDRNOTAVAIL if the MAC
address is not valid.

Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Assisted-by: LLM
Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: I1cae9178ff2c9243510556f42a194058c10aea7e
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 799da63e3a4e..90ca4f220cc7 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3635,6 +3635,9 @@ static int bcmgenet_set_mac_addr(struct net_device *dev, void *p)
 	if (netif_running(dev))
 		return -EBUSY;
 
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+
 	eth_hw_addr_set(dev, addr->sa_data);
 
 	return 0;
-- 
2.34.1

Re: [PATCH net 6/8] net: bcmgenet: validate Ethernet address in bcmgenet_set_mac_addr

[Nicolai Buchwitz]

On 18.9.2026 02:07, Florian Fainelli wrote:
> bcmgenet_set_mac_addr() did not check whether the provided MAC address 
> is a
> valid Ethernet address before applying it. Userspace could configure an
> invalid address (such as all zeroes or a multicast address) while the
> interface is down.
> 
> Add a call to is_valid_ether_addr() and return -EADDRNOTAVAIL if the 
> MAC
> address is not valid.
> 
> Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
> Assisted-by: LLM
> Co-authored-by: Cursor <cursoragent@cursor.com>
> Change-Id: I1cae9178ff2c9243510556f42a194058c10aea7e
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 799da63e3a4e..90ca4f220cc7 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3635,6 +3635,9 @@ static int bcmgenet_set_mac_addr(struct 
> net_device *dev, void *p)
>  	if (netif_running(dev))
>  		return -EBUSY;
> 
> +	if (!is_valid_ether_addr(addr->sa_data))
> +		return -EADDRNOTAVAIL;
> +
>  	eth_hw_addr_set(dev, addr->sa_data);
> 
>  	return 0;

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

[PATCH net 7/8] net: bcmgenet: zero-initialize Transmit Status Block in bcmgenet_add_tsb

[Florian Fainelli]

bcmgenet_add_tsb() pushes 64 bytes of headroom onto the skb to form the
Transmit Status Block (TSB) but only assigns status->tx_csum_info when
skb->ip_summed == CHECKSUM_PARTIAL. For packets without checksum offload,
or for other fields within struct status_64, the status block contains
uninitialized stack/heap memory from previous skb operations.

Zero-initialize the status block with memset() after pushing headroom.

Fixes: 9a9ba2a4aaaa ("net: bcmgenet: always enable status blocks")
Assisted-by: LLM
Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: I6935e24408ea10f68eb61dce4cced9b0ead5c005
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 90ca4f220cc7..3a6a2f075959 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2084,6 +2084,7 @@ static struct sk_buff *bcmgenet_add_tsb(struct net_device *dev,
 
 	skb_push(skb, sizeof(*status));
 	status = (struct status_64 *)skb->data;
+	memset(status, 0, sizeof(*status));
 
 	if (skb->ip_summed  == CHECKSUM_PARTIAL) {
 		ip_ver = skb->protocol;
-- 
2.34.1

Re: [PATCH net 7/8] net: bcmgenet: zero-initialize Transmit Status Block in bcmgenet_add_tsb

[Nicolai Buchwitz]

Hi Florian

On 18.9.2026 02:07, Florian Fainelli wrote:
> bcmgenet_add_tsb() pushes 64 bytes of headroom onto the skb to form the
> Transmit Status Block (TSB) but only assigns status->tx_csum_info when
> skb->ip_summed == CHECKSUM_PARTIAL. For packets without checksum 
> offload,
> or for other fields within struct status_64, the status block contains
> uninitialized stack/heap memory from previous skb operations.

The MAC only reads tx_csum_info when the descriptor has DMA_TX_DO_CSUM,
and bcmgenet_xmit() sets that only for CHECKSUM_PARTIAL.
The stale value seems to be never used?

Did you hit an actual failure?  Otherwise this would be a 64 byte memset
on every transmitted packet.

> [...]

Thanks,
Nicolai

Re: [PATCH net 7/8] net: bcmgenet: zero-initialize Transmit Status Block in bcmgenet_add_tsb

[Florian Fainelli]

On 9/18/26 04:03, Nicolai Buchwitz wrote:
> Hi Florian
> 
> On 18.9.2026 02:07, Florian Fainelli wrote:
>> bcmgenet_add_tsb() pushes 64 bytes of headroom onto the skb to form the
>> Transmit Status Block (TSB) but only assigns status->tx_csum_info when
>> skb->ip_summed == CHECKSUM_PARTIAL. For packets without checksum offload,
>> or for other fields within struct status_64, the status block contains
>> uninitialized stack/heap memory from previous skb operations.
> 
> The MAC only reads tx_csum_info when the descriptor has DMA_TX_DO_CSUM,
> and bcmgenet_xmit() sets that only for CHECKSUM_PARTIAL.
> The stale value seems to be never used?
> 
> Did you hit an actual failure?  Otherwise this would be a 64 byte memset
> on every transmitted packet.

Nope this is entirely theoretical, and one could argue that this could 
create a slight drop in performance, happy to drop that one as well 
since we never really experienced any actual issue.
-- 
Florian

[PATCH net 8/8] net: bcmgenet: mask DMA_TIMEOUT_MASK when reading DMA_RING0_TIMEOUT

[Florian Fainelli]

bcmgenet_get_coalesce() reads DMA_RING0_TIMEOUT to calculate
rx_coalesce_usecs without masking out bits outside DMA_TIMEOUT_MASK
(16 bits). If upper bits are non-zero or contain status/flags, the
computed value of rx_coalesce_usecs returned to userspace via ethtool
becomes corrupted.

Mask the register read with DMA_TIMEOUT_MASK before computing the
timeout in microseconds.

Fixes: 4a29645bfe6c ("net: bcmgenet: Implement RX coalescing control knobs")
Assisted-by: LLM
Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: I4c5b5019e764dbd0c7e51bd8c7365939a4736e44
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 3a6a2f075959..b15e64a484b4 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -852,7 +852,8 @@ static int bcmgenet_get_coalesce(struct net_device *dev,
 	ec->rx_max_coalesced_frames =
 		bcmgenet_rdma_ring_readl(priv, 0, DMA_MBUF_DONE_THRESH);
 	ec->rx_coalesce_usecs =
-		bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT) * 8192 / 1000;
+		(bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT) &
+		 DMA_TIMEOUT_MASK) * 8192 / 1000;
 
 	for (i = 0; i <= priv->hw_params->rx_queues; i++) {
 		ring = &priv->rx_rings[i];
-- 
2.34.1

Re: [PATCH net 8/8] net: bcmgenet: mask DMA_TIMEOUT_MASK when reading DMA_RING0_TIMEOUT

[Nicolai Buchwitz]

On 18.9.2026 02:07, Florian Fainelli wrote:
> bcmgenet_get_coalesce() reads DMA_RING0_TIMEOUT to calculate
> rx_coalesce_usecs without masking out bits outside DMA_TIMEOUT_MASK
> (16 bits). If upper bits are non-zero or contain status/flags, the
> computed value of rx_coalesce_usecs returned to userspace via ethtool
> becomes corrupted.
> 
> Mask the register read with DMA_TIMEOUT_MASK before computing the
> timeout in microseconds.
> 
> Fixes: 4a29645bfe6c ("net: bcmgenet: Implement RX coalescing control 
> knobs")
> Assisted-by: LLM
> Co-authored-by: Cursor <cursoragent@cursor.com>
> Change-Id: I4c5b5019e764dbd0c7e51bd8c7365939a4736e44
> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
> b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 3a6a2f075959..b15e64a484b4 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -852,7 +852,8 @@ static int bcmgenet_get_coalesce(struct net_device 
> *dev,
>  	ec->rx_max_coalesced_frames =
>  		bcmgenet_rdma_ring_readl(priv, 0, DMA_MBUF_DONE_THRESH);
>  	ec->rx_coalesce_usecs =
> -		bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT) * 8192 / 1000;
> +		(bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT) &
> +		 DMA_TIMEOUT_MASK) * 8192 / 1000;
> 
>  	for (i = 0; i <= priv->hw_params->rx_queues; i++) {
>  		ring = &priv->rx_rings[i];

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

Re: [PATCH net 0/8] net: bcmgenet: Collection of bug fixes

[Florian Fainelli]

On 9/17/2026 5:07 PM, Florian Fainelli wrote:
> This patch series contains a collection of bug fixes found during a
> LLM-assisted coding session.

My bad, I forgot to remove some tags while the patches were sent out, 
will wait for feedback and repost.

> 
> Florian Fainelli (8):
>    net: bcmgenet: fix 64-bit RTNL stats reading in ethtool on 32-bit
>      systems
>    net: bcmgenet: initialize u64 stats seq counter for all queues
>    net: bcmgenet: do not skip WoL power up on GENET V1
>    net: bcmgenet: clean up RX NAPI on bcmgenet_init_rx_queues failure
>    net: bcmgenet: acquire ring lock with BH disabled in
>      bcmgenet_dump_tx_queue
>    net: bcmgenet: validate Ethernet address in bcmgenet_set_mac_addr
>    net: bcmgenet: zero-initialize Transmit Status Block in
>      bcmgenet_add_tsb
>    net: bcmgenet: mask DMA_TIMEOUT_MASK when reading DMA_RING0_TIMEOUT
> 
>   .../net/ethernet/broadcom/genet/bcmgenet.c    | 34 ++++++++++++-------
>   1 file changed, 21 insertions(+), 13 deletions(-)
> 

-- 
Florian