There are some false positives reported by Sashiko, see the comments
from Sashiko and my response inline. Other comments are useful and
I will add improvements in v6.
> static int netc_setup(struct dsa_switch *ds)
> {
> struct netc_switch *priv = ds->priv;
> @@ -393,6 +627,8 @@ static int netc_setup(struct dsa_switch *ds)
> if (err)
> return err;
>
> + netc_get_switch_capabilities(priv);
> +
> err = netc_init_all_ports(priv);
Sashiko says:
Since the DSA subsystem can call setup() and teardown() multiple times
(for example, if the CPU master interface is unbound and rebound), does
calling netc_init_all_ports() here leak memory?
Looking at netc_init_all_ports(), it uses devm_kcalloc() and devm_kzalloc()
for the priv->ports array and port structures. Because devm_ allocations
are tied to the PCI device lifetime, they won't be freed during teardown(),
which could lead to a memory leak on each rebind.
Response: Analysis of the DSA core driver reveals the following call stack
for setup():
netc_switch_probe()
└──dsa_register_switch()
└── dsa_switch_probe()
└── dsa_tree_setup()
└── dsa_tree_setup_switches()
└── dsa_switch_setup()
netc_setup() is only called when the switch driver probes, so it will not
be called multiple times during the lifetime of the DSA switch device.
[...]
> +static int netc_port_fdb_add(struct dsa_switch *ds, int port,
> + const unsigned char *addr, u16 vid,
> + struct dsa_db db)
> +{
> + struct netc_port *np = NETC_PORT(ds, port);
> +
> + /* Currently, only support standalone port mode, so only
> + * NETC_STANDALONE_PVID (= 0) is supported here.
> + */
> + if (vid != NETC_STANDALONE_PVID)
> + return -EOPNOTSUPP;
> +
> + return netc_port_set_fdb_entry(np, addr, vid);
> +}
Sashiko says:
Does this implementation break network isolation between standalone ports?
The driver sets ds->fdb_isolation = true in netc_switch_probe(), which
means it must isolate FDB entries per standalone port or bridge. However,
the isolation context (struct dsa_db db) is ignored here and all entries
are placed in NETC_STANDALONE_PVID (0).
If the same MAC address is added to multiple standalone ports, such as an
IPv6 multicast group, the single FDB entry in VID 0 is updated to include
all these ports in its port_bitmap. A frame matching this MAC arriving on
one standalone port will then be switched in hardware to the other
standalone ports, instead of keeping the traffic properly isolated.
Response: Currently, the NETC switch only supports single port mode, so
all the FDB entries are only added to the CPU port, they will not be added
to the user ports. So this is a false positive.
