Re: [PATCH net-next v6 7/9] net: dsa: lan9645x: add mac table integration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] L2 multicast FDB entries are incorrectly mapped to a single port instead of a PGID, overwriting previous entries.
- [Medium] Static FDB entries are excluded from `lan9645x_mact_dsa_dump`, making them invisible on standalone ports.
--

commit 27e48dc68296c269fcc025aa1f9ae3a7d03b1da7
Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>

net: dsa: lan9645x: add mac table integration

This commit adds MAC table support and DSA FDB callback integration for the
lan9645x switch. It introduces functions for MAC table manipulation, aging,
and dumping entries.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c
> index 0000000000000..1079e49f62718
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c
[ ... ]
> +int lan9645x_mact_dsa_dump(struct lan9645x *lan9645x, int port,
> +			   dsa_fdb_dump_cb_t *cb, void *data)
> +{
[ ... ]
> +		/* Only dynamic entries are surfaced through the user port dump.
> +		 * ENTRYTYPE_LOCKED entries are already reported by the bridge
> +		 * master's ndo_fdb_dump as NTF_MASTER, so we avoid duplicating
> +		 * them as NTF_SELF.
> +		 * Entries toward the host (NTF_SELF) have DEST_IDX == CPU_PORT
> +		 * and are filtered out by the DEST_IDX check.
> +		 */
> +		if (ANA_MACACCESS_DEST_IDX_GET(maca) == port &&
> +		    type == ENTRYTYPE_NORMAL) {

[Severity: Medium]
Will this filter out static FDB entries on standalone ports?

While the comment notes that locked entries are reported by the bridge
master, standalone switch ports not enslaved to a bridge won't have a software
FDB to report them. If a user adds a static FDB entry to a standalone port,
it appears it would be programmed as ENTRYTYPE_LOCKED but completely filtered
out here, making it invisible to user-space tools like `bridge fdb show`.

> +			addr = (u64)ANA_MACHDATA_MACHDATA_GET(mach) << 32 |
> +			       macl;
[ ... ]
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index eb13dfadbd3cf..a76e61fb37096 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> +static int lan9645x_fdb_add(struct dsa_switch *ds, int port,
> +			    const unsigned char *addr, u16 vid,
> +			    struct dsa_db db)
> +{
> +	struct net_device *br = lan9645x_db2bridge(db);
> +	struct dsa_port *dp = dsa_to_port(ds, port);
> +	struct lan9645x *lan9645x = ds->priv;
> +	int dest;
> +
> +	if (IS_ERR(br))
> +		return PTR_ERR(br);
> +
> +	if (dsa_port_is_cpu(dp) && !br &&
> +	    dsa_fdb_present_in_other_db(ds, port, addr, vid, db))
> +		return 0;
> +
> +	if (!vid)
> +		vid = lan9645x_vlan_unaware_pvid(!!br);
> +
> +	dest = dsa_port_is_cpu(dp) ? PGID_CPU : port;
> +
> +	return lan9645x_mact_learn(lan9645x, dest, addr, vid, ENTRYTYPE_LOCKED);

[Severity: High]
Does this code handle L2 multicast FDB entries correctly?

Without a check for is_multicast_ether_addr(addr), L2 multicast entries are
mapped to a single port instead of a PGID. Because the hardware MAC table
is keyed by (MAC, VID), adding the multicast address to a second port via
the software bridge would overwrite the destination index of the first entry.
This would cause the hardware to only forward multicast traffic to the last
configured port, creating a mismatch with the software bridge FDB.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-dsa_lan9645x_switch_driver_base-v6-0-4d409ae64f3c@microchip.com?part=7