The first N entries in priv->vlans are reserved for managing ports which
are not part of a bridge. Use priv->hw_info->max_ports to consistently
access per-bridge entries at index 7. Starting at
priv->hw_info->cpu_port (6) is harmless in this case because
priv->vlan[6].bridge is always NULL so the comparison result is always
false (which results in this entry being skipped).
Fixes: 58c59ef9e930c4 ("net: dsa: lantiq: Add Forwarding Database access")
Acked-by: Hauke Mehrtens <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/net/dsa/lantiq_gswip.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 12c15da55664..0c313db23451 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1360,7 +1360,7 @@ static int gswip_port_fdb(struct dsa_switch *ds, int port,
struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
struct gswip_priv *priv = ds->priv;
struct gswip_pce_table_entry mac_bridge = {0,};
- unsigned int cpu_port = priv->hw_info->cpu_port;
+ unsigned int max_ports = priv->hw_info->max_ports;
int fid = -1;
int i;
int err;
@@ -1368,7 +1368,7 @@ static int gswip_port_fdb(struct dsa_switch *ds, int port,
if (!bridge)
return -EINVAL;
- for (i = cpu_port; i < ARRAY_SIZE(priv->vlans); i++) {
+ for (i = max_ports; i < ARRAY_SIZE(priv->vlans); i++) {
if (priv->vlans[i].bridge == bridge) {
fid = priv->vlans[i].fid;
break;
--
2.36.1
Hi Martin,
On Tue, May 17, 2022 at 09:40:14PM +0200, Martin Blumenstingl wrote:
> The first N entries in priv->vlans are reserved for managing ports which
> are not part of a bridge. Use priv->hw_info->max_ports to consistently
> access per-bridge entries at index 7. Starting at
> priv->hw_info->cpu_port (6) is harmless in this case because
> priv->vlan[6].bridge is always NULL so the comparison result is always
> false (which results in this entry being skipped).
>
> Fixes: 58c59ef9e930c4 ("net: dsa: lantiq: Add Forwarding Database access")
> Acked-by: Hauke Mehrtens <[email protected]>
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
The patch, as well as other improvements you might want to bring to the gswip driver
(we have more streamlined support in DSA now for FDB isolation, see ds->fdb_isolation)
is appreciated.
But I don't think that a code cleanup patch that makes no functional
difference, and isn't otherwise needed by other backported patches,
should be sent to the "net" tree, and be backported to "stable" later?
Hi Vladimir,
On Wed, May 18, 2022 at 1:45 PM Vladimir Oltean <[email protected]> wrote:
[...]
> The patch, as well as other improvements you might want to bring to the gswip driver
> (we have more streamlined support in DSA now for FDB isolation, see ds->fdb_isolation)
> is appreciated.
Thank you very much for this hint! I was not aware of the
ds->fdb_isolation flag and additionally I have some other questions
regarding FDB - I'll send these in a separate email though.
Also thank you for being quick to review my patches and on top of that
providing extra hints!
> But I don't think that a code cleanup patch that makes no functional
> difference, and isn't otherwise needed by other backported patches,
> should be sent to the "net" tree, and be backported to "stable" later?
Sure, I will actually re-send the whole series based on net-next.
When I initially wrote this patch I thought that it would fix a more
severe issue. Only later on I found that the bug is harmless (as
mentioned in the patch description). When I then finally sent the
patch I just stuck with my original plan to send it for the net.git
tree - instead of re-thinking whether that's still needed after my
latest findings.
Best regards,
Martin
On Wed, May 18, 2022 at 07:58:58PM +0200, Martin Blumenstingl wrote:
> Hi Vladimir,
>
> On Wed, May 18, 2022 at 1:45 PM Vladimir Oltean <[email protected]> wrote:
> [...]
> > The patch, as well as other improvements you might want to bring to the gswip driver
> > (we have more streamlined support in DSA now for FDB isolation, see ds->fdb_isolation)
> > is appreciated.
> Thank you very much for this hint! I was not aware of the
> ds->fdb_isolation flag and additionally I have some other questions
> regarding FDB - I'll send these in a separate email though.
> Also thank you for being quick to review my patches and on top of that
> providing extra hints!
Ok, feel free to ask.
Please note that there's also this discussion with Alvin about FDB
isolation and host filtering which hopefully helped to clear some more
concepts.
https://lore.kernel.org/all/[email protected]/