From: Alexander Sverdlin <[email protected]>
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 <[email protected]>
---
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
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 <[email protected]>
> > ---
>
> 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 <[email protected]>
--
Alexander Sverdlin
Siemens AG
http://www.siemens.com
On Wed, May 31, 2023 at 04:38:26PM +0200, A. Sverdlin wrote:
> From: Alexander Sverdlin <[email protected]>
>
> 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 <[email protected]>
> ---
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 <[email protected]>
Yuck.
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.
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.
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.
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
On Wed, 31 May 2023 16:38:26 +0200 you wrote:
> From: Alexander Sverdlin <[email protected]>
>
> 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