[PATCH net 0/3] net: dsa: b53: minor fdb related fixes

[PATCH net 0/3] net: dsa: b53: minor fdb related fixes

[Jonas Gorski]

While investigating and fixing/implenting proper ARL support for
bcm63xx, I encountered multiple minor issues in the current ARL
implementation:

* The ARL multicast support was not properly enabled for older chips,
  and instead a potentially reserved bit was toggled.
* While traversing the ARL table, "Search done" triggered one final
  entry which will be invalid for 4 ARL bin chips, and failed to stop
  the search on chips with only one result register.
* For chips where we have only one result register, we only traversed at
  most half the maximum entries.

I also had a fix for IVL_SVL_SELECT which is only valid for some chips,
but since this would only have an effect for !vlan_enabled, and we
always have that enabled, it isn't really worth fixing (and rather drop
the !vlan_enabled paths).

Jonas Gorski (3):
  net: dsa: b53: fix enabling ip multicast
  net: dsa: b53: stop reading ARL entries if search is done
  net: dsa: b53: properly bound ARL searches for < 4 ARL bin chips

 drivers/net/dsa/b53/b53_common.c | 15 +++++++++------
 drivers/net/dsa/b53/b53_regs.h   |  3 +--
 2 files changed, 10 insertions(+), 8 deletions(-)


base-commit: d7d2fcf7ae31471b4e08b7e448b8fd0ec2e06a1b
-- 
2.43.0

[PATCH net 1/3] net: dsa: b53: fix enabling ip multicast

[Jonas Gorski]

In the New Control register bit 1 is either reserved, or has a different
function:

    Out of Range Error Discard

    When enabled, the ingress port discards any frames
    if the Length field is between 1500 and 1536
    (excluding 1500 and 1536) and with good CRC.

The actual bit for enabling IP multicast is bit 0, which was only
explicitly enabled for BCM5325 so far.

For older switch chips, this bit defaults to 0, so we want to enable it
as well, while newer switch chips default to 1, and their documentation
says "It is illegal to set this bit to zero."

So drop the wrong B53_IPMC_FWD_EN define, enable the IP multicast bit
also for other switch chips. While at it, rename it to (B53_)IP_MC as
that is how it is called in Broadcom code.

Fixes: 63cc54a6f073 ("net: dsa: b53: Fix egress flooding settings")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 4 ++--
 drivers/net/dsa/b53/b53_regs.h   | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 2f846381d5a7..77571a46311e 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -371,11 +371,11 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
 		 * frames should be flooded or not.
 		 */
 		b53_read8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, &mgmt);
-		mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IPMC_FWD_EN;
+		mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN | B53_IP_MC;
 		b53_write8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, mgmt);
 	} else {
 		b53_read8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, &mgmt);
-		mgmt |= B53_IP_MCAST_25;
+		mgmt |= B53_IP_MC;
 		b53_write8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, mgmt);
 	}
 }
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index 309fe0e46dad..8ce1ce72e938 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -111,8 +111,7 @@
 
 /* IP Multicast control (8 bit) */
 #define B53_IP_MULTICAST_CTRL		0x21
-#define  B53_IP_MCAST_25		BIT(0)
-#define  B53_IPMC_FWD_EN		BIT(1)
+#define  B53_IP_MC			BIT(0)
 #define  B53_UC_FWD_EN			BIT(6)
 #define  B53_MC_FWD_EN			BIT(7)
 
-- 
2.43.0

[PATCH net 2/3] net: dsa: b53: stop reading ARL entries if search is done

[Jonas Gorski]

The switch clears the ARL_SRCH_STDN bit when the search is done, i.e. it
finished traversing the ARL table.

This means that there will be no valid result, so we should not attempt
to read and process any further entries.

We only ever check the validity of the entries for 4 ARL bin chips, and
only after having passed the first entry to the b53_fdb_copy().

This means that we always pass an invalid entry at the end to the
b53_fdb_copy(). b53_fdb_copy() does check the validity though before
passing on the entry, so it never gets passed on.

On < 4 ARL bin chips, we will even continue reading invalid entries
until we reach the result limit.

Fixes: 1da6df85c6fb ("net: dsa: b53: Implement ARL add/del/dump operations")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 77571a46311e..82cce7b82da2 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2018,7 +2018,7 @@ static int b53_arl_search_wait(struct b53_device *dev)
 	do {
 		b53_read8(dev, B53_ARLIO_PAGE, offset, &reg);
 		if (!(reg & ARL_SRCH_STDN))
-			return 0;
+			return -ENOENT;
 
 		if (reg & ARL_SRCH_VLID)
 			return 0;
-- 
2.43.0

[PATCH net 3/3] net: dsa: b53: properly bound ARL searches for < 4 ARL bin chips

[Jonas Gorski]

When iterating over the ARL table we stop at max ARL entries / 2, but
this is only valid if the chip actually returns 2 results at once. For
chips with only one result register we will stop before reaching the end
of the table if it is more than half full.

Fix this by only dividing the maximum results by two if we have a chip
with more than one result register (i.e. those with 4 ARL bins).

Fixes: cd169d799bee ("net: dsa: b53: Bound check ARL searches")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 82cce7b82da2..6232eb5336a3 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2068,13 +2068,16 @@ static int b53_fdb_copy(int port, const struct b53_arl_entry *ent,
 int b53_fdb_dump(struct dsa_switch *ds, int port,
 		 dsa_fdb_dump_cb_t *cb, void *data)
 {
+	unsigned int count = 0, results_per_hit = 1;
 	struct b53_device *priv = ds->priv;
 	struct b53_arl_entry results[2];
-	unsigned int count = 0;
 	u8 offset;
 	int ret;
 	u8 reg;
 
+	if (priv->num_arl_bins > 2)
+		results_per_hit = 2;
+
 	mutex_lock(&priv->arl_mutex);
 
 	if (is5325(priv) || is5365(priv))
@@ -2096,7 +2099,7 @@ int b53_fdb_dump(struct dsa_switch *ds, int port,
 		if (ret)
 			break;
 
-		if (priv->num_arl_bins > 2) {
+		if (results_per_hit == 2) {
 			b53_arl_search_rd(priv, 1, &results[1]);
 			ret = b53_fdb_copy(port, &results[1], cb, data);
 			if (ret)
@@ -2106,7 +2109,7 @@ int b53_fdb_dump(struct dsa_switch *ds, int port,
 				break;
 		}
 
-	} while (count++ < b53_max_arl_entries(priv) / 2);
+	} while (count++ < b53_max_arl_entries(priv) / results_per_hit);
 
 	mutex_unlock(&priv->arl_mutex);
 
-- 
2.43.0

Re: [PATCH net 2/3] net: dsa: b53: stop reading ARL entries if search is done

[Florian Fainelli]

On 11/2/2025 2:07 AM, Jonas Gorski wrote:
> The switch clears the ARL_SRCH_STDN bit when the search is done, i.e. it
> finished traversing the ARL table.
> 
> This means that there will be no valid result, so we should not attempt
> to read and process any further entries.
> 
> We only ever check the validity of the entries for 4 ARL bin chips, and
> only after having passed the first entry to the b53_fdb_copy().
> 
> This means that we always pass an invalid entry at the end to the
> b53_fdb_copy(). b53_fdb_copy() does check the validity though before
> passing on the entry, so it never gets passed on.
> 
> On < 4 ARL bin chips, we will even continue reading invalid entries
> until we reach the result limit.
> 
> Fixes: 1da6df85c6fb ("net: dsa: b53: Implement ARL add/del/dump operations")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net 3/3] net: dsa: b53: properly bound ARL searches for < 4 ARL bin chips

[Florian Fainelli]

On 11/2/2025 2:07 AM, Jonas Gorski wrote:
> When iterating over the ARL table we stop at max ARL entries / 2, but
> this is only valid if the chip actually returns 2 results at once. For
> chips with only one result register we will stop before reaching the end
> of the table if it is more than half full.
> 
> Fix this by only dividing the maximum results by two if we have a chip
> with more than one result register (i.e. those with 4 ARL bins).
> 
> Fixes: cd169d799bee ("net: dsa: b53: Bound check ARL searches")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net 1/3] net: dsa: b53: fix enabling ip multicast

[Florian Fainelli]

On 11/2/2025 2:07 AM, Jonas Gorski wrote:
> In the New Control register bit 1 is either reserved, or has a different
> function:
> 
>      Out of Range Error Discard
> 
>      When enabled, the ingress port discards any frames
>      if the Length field is between 1500 and 1536
>      (excluding 1500 and 1536) and with good CRC.
> 
> The actual bit for enabling IP multicast is bit 0, which was only
> explicitly enabled for BCM5325 so far.
> 
> For older switch chips, this bit defaults to 0, so we want to enable it
> as well, while newer switch chips default to 1, and their documentation
> says "It is illegal to set this bit to zero."

The IP_MC bit is definitively a better name and matches what exists in 
the newer switching IP, it does default to 1 there as well, and it has 
the mention that it is illegal to set to zero (makes you wonder why it 
is exposed then).

> 
> So drop the wrong B53_IPMC_FWD_EN define, enable the IP multicast bit
> also for other switch chips. While at it, rename it to (B53_)IP_MC as
> that is how it is called in Broadcom code.
> 
> Fixes: 63cc54a6f073 ("net: dsa: b53: Fix egress flooding settings")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net 0/3] net: dsa: b53: minor fdb related fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sun,  2 Nov 2025 11:07:55 +0100 you wrote:
> While investigating and fixing/implenting proper ARL support for
> bcm63xx, I encountered multiple minor issues in the current ARL
> implementation:
> 
> * The ARL multicast support was not properly enabled for older chips,
>   and instead a potentially reserved bit was toggled.
> * While traversing the ARL table, "Search done" triggered one final
>   entry which will be invalid for 4 ARL bin chips, and failed to stop
>   the search on chips with only one result register.
> * For chips where we have only one result register, we only traversed at
>   most half the maximum entries.
> 
> [...]

Here is the summary with links:
  - [net,1/3] net: dsa: b53: fix enabling ip multicast
    https://git.kernel.org/netdev/net/c/c264294624e9
  - [net,2/3] net: dsa: b53: stop reading ARL entries if search is done
    https://git.kernel.org/netdev/net/c/0be04b5fa62a
  - [net,3/3] net: dsa: b53: properly bound ARL searches for < 4 ARL bin chips
    https://git.kernel.org/netdev/net/c/e57723fe536f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html