[PATCH] net: dsa: lan9303: allow vid != 0 in port_fdb_{add|del} methods

[PATCH] net: dsa: lan9303: allow vid != 0 in port_fdb_{add|del} methods

[A. Sverdlin]

From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

LAN9303 doesn't associate FDB (ALR) entries with VLANs, it has just one
global Address Logic Resolution table [1].

Ignore VID in port_fdb_{add|del} methods, go on with the global table. This
is the same semantics as hellcreek or RZ/N1 implement.

Visible symptoms:
LAN9303_MDIO 5b050000.ethernet-1:00: port 2 failed to delete 00:xx:xx:xx:xx:cf vid 1 from fdb: -2
LAN9303_MDIO 5b050000.ethernet-1:00: port 2 failed to add 00:xx:xx:xx:xx:cf vid 1 to fdb: -95

[1] https://ww1.microchip.com/downloads/en/DeviceDoc/00002308A.pdf

Fixes: 0620427ea0d6 ("net: dsa: lan9303: Add fdb/mdb manipulation")
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 drivers/net/dsa/lan9303-core.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index cbe831875347..c0215a8770f4 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1188,8 +1188,6 @@ static int lan9303_port_fdb_add(struct dsa_switch *ds, int port,
 	struct lan9303 *chip = ds->priv;
 
 	dev_dbg(chip->dev, "%s(%d, %pM, %d)\n", __func__, port, addr, vid);
-	if (vid)
-		return -EOPNOTSUPP;
 
 	return lan9303_alr_add_port(chip, addr, port, false);
 }
@@ -1201,8 +1199,6 @@ static int lan9303_port_fdb_del(struct dsa_switch *ds, int port,
 	struct lan9303 *chip = ds->priv;
 
 	dev_dbg(chip->dev, "%s(%d, %pM, %d)\n", __func__, port, addr, vid);
-	if (vid)
-		return -EOPNOTSUPP;
 	lan9303_alr_del_port(chip, addr, port);
 
 	return 0;
-- 
2.40.1

Re: [PATCH] net: dsa: lan9303: allow vid != 0 in port_fdb_{add|del} methods

[Vladimir Oltean]

On Wed, May 31, 2023 at 04:38:26PM +0200, A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> LAN9303 doesn't associate FDB (ALR) entries with VLANs, it has just one
> global Address Logic Resolution table [1].
> 
> Ignore VID in port_fdb_{add|del} methods, go on with the global table. This
> is the same semantics as hellcreek or RZ/N1 implement.
> 
> Visible symptoms:
> LAN9303_MDIO 5b050000.ethernet-1:00: port 2 failed to delete 00:xx:xx:xx:xx:cf vid 1 from fdb: -2
> LAN9303_MDIO 5b050000.ethernet-1:00: port 2 failed to add 00:xx:xx:xx:xx:cf vid 1 to fdb: -95
> 
> [1] https://ww1.microchip.com/downloads/en/DeviceDoc/00002308A.pdf
> 
> Fixes: 0620427ea0d6 ("net: dsa: lan9303: Add fdb/mdb manipulation")
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> ---

Thanks for taking a look. Although it would probably be safer to add:

Fixes: 2fd186501b1c ("net: dsa: be louder when a non-legacy FDB operation fails")

since I'm not sure it has a reason to be backported beyond that. Anyway:

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Yuck.

Re: [PATCH] net: dsa: lan9303: allow vid != 0 in port_fdb_{add|del} methods

[Sverdlin, Alexander]

Hi Vladimir,

thank you for quick review!

On Wed, 2023-05-31 at 18:16 +0300, Vladimir Oltean wrote:
> > LAN9303 doesn't associate FDB (ALR) entries with VLANs, it has just
> > one
> > global Address Logic Resolution table [1].
> > 
> > Ignore VID in port_fdb_{add|del} methods, go on with the global
> > table. This
> > is the same semantics as hellcreek or RZ/N1 implement.
> > 
> > Visible symptoms:
> > LAN9303_MDIO 5b050000.ethernet-1:00: port 2 failed to delete
> > 00:xx:xx:xx:xx:cf vid 1 from fdb: -2
> > LAN9303_MDIO 5b050000.ethernet-1:00: port 2 failed to add
> > 00:xx:xx:xx:xx:cf vid 1 to fdb: -95
> > 
> > [1] https://ww1.microchip.com/downloads/en/DeviceDoc/00002308A.pdf
> > 
> > Fixes: 0620427ea0d6 ("net: dsa: lan9303: Add fdb/mdb manipulation")
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> > ---
> 
> Thanks for taking a look. Although it would probably be safer to add:
> 
> Fixes: 2fd186501b1c ("net: dsa: be louder when a non-legacy FDB
> operation fails")
> 
> since I'm not sure it has a reason to be backported beyond that.

Well, it's not only about visible errors, but the driver also refused
to install the FDB entries, so it's change in behaviour, not only
cosmetics.

> Anyway:
> 
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Re: [PATCH] net: dsa: lan9303: allow vid != 0 in port_fdb_{add|del} methods

[Vladimir Oltean]

On Wed, May 31, 2023 at 03:20:19PM +0000, Sverdlin, Alexander wrote:
> Well, it's not only about visible errors, but the driver also refused
> to install the FDB entries, so it's change in behaviour, not only
> cosmetics.

Ok, makes sense. Let's see what will happen with the backport - to be
honest I'm not completely sure. If you want to be completely sure I
didn't just throw a wrench into your plans, feel free to resend a v2
with just my review tag (dropping my Fixes tag), and you could also add
a comment stating that the ALR is VLAN-unaware while you're at it.

Re: [PATCH] net: dsa: lan9303: allow vid != 0 in port_fdb_{add|del} methods

[Jakub Kicinski]

On Wed, 31 May 2023 18:35:19 +0300 Vladimir Oltean wrote:
> If you want to be completely sure I didn't just throw a wrench into
> your plans, feel free to resend a v2 with just my review tag
> (dropping my Fixes tag)

FWIW if you worry that the Fixes tag will get added automatically - 
for whatever reason that still doesn't work. We add them manually
when someone provides a tag in response.

Re: [PATCH] net: dsa: lan9303: allow vid != 0 in port_fdb_{add|del} methods

[Vladimir Oltean]

On Wed, May 31, 2023 at 09:41:50PM -0700, Jakub Kicinski wrote:
> On Wed, 31 May 2023 18:35:19 +0300 Vladimir Oltean wrote:
> > If you want to be completely sure I didn't just throw a wrench into
> > your plans, feel free to resend a v2 with just my review tag
> > (dropping my Fixes tag)
> 
> FWIW if you worry that the Fixes tag will get added automatically - 
> for whatever reason that still doesn't work. We add them manually
> when someone provides a tag in response.

Aha, ok. So as long as the maintainer who applies the patch does not
append the second Fixes: tag that I had proposed, all is well and this
change can be applied as is.

Re: [PATCH] net: dsa: lan9303: allow vid != 0 in port_fdb_{add|del} methods

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 31 May 2023 16:38:26 +0200 you wrote:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> LAN9303 doesn't associate FDB (ALR) entries with VLANs, it has just one
> global Address Logic Resolution table [1].
> 
> Ignore VID in port_fdb_{add|del} methods, go on with the global table. This
> is the same semantics as hellcreek or RZ/N1 implement.
> 
> [...]

Here is the summary with links:
  - net: dsa: lan9303: allow vid != 0 in port_fdb_{add|del} methods
    https://git.kernel.org/netdev/net/c/5a59a58ec25d

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