Re: [PATCH net-next v5 8/9] net: dsa: lan9645x: add mdb management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Deleting a port from an L2 MDB entry can fail with -ENOSPC, leaving the port permanently active in the hardware multicast group.
- [High] IP multicast frames may be duplicated to the CPU port because the CPU port is specified in both the MAC inline portmask and the `MAC_CPU_COPY` flag.
--

commit 12dd40a6c7c29e212ec6866149d379306e63d403
Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
net: dsa: lan9645x: add mdb management

Add support for dsa mdb callbacks to handle L2 multicast and IP multicast, 
using PGID indices for L2 and inline port masks for IP multicast.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_mdb.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_mdb.c
> new file mode 100644
> index 0000000000000..0ba3e0974dc08
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_mdb.c
[ ... ]
> +static int lan9645x_mdb_update_dest(struct lan9645x *lan9645x,
> +				    struct lan9645x_mdb_entry *mdb_entry,
> +				    enum macaccess_entry_type type,
> +				    struct lan9645x_pgid_entry *new_pgid,
> +				    u16 new_ports)
> +{
> +	struct lan9645x_pgid_entry *old_pgid;
> +	unsigned char mac[ETH_ALEN];
> +	int err, pgid_index;
> +	bool cpu_copy;
> +
> +	old_pgid = mdb_entry->pgid;
> +	lan9645x_mdb_encode_mac(mac, mdb_entry->mac, new_ports, type);
> +	cpu_copy = !!(new_ports & BIT(CPU_PORT));

Can IP multicast frames be duplicated to the CPU port when both the
destination mask and the explicit CPU extraction queue are configured?

When new_ports includes the CPU port, it appears to be both encoded into
the MAC address bytes via lan9645x_mdb_encode_mac() and explicitly passed
as the cpu_copy flag to lan9645x_mact_learn_cpu_copy().

Should the CPU_PORT bit be explicitly stripped from the portmask before
encoding it into the MAC address, similar to how it is stripped for L2
multicast PGIDs?

[ ... ]
> +static int __lan9645x_mdb_del(struct lan9645x *lan9645x, int chip_port,
> +			      const unsigned char addr[ETH_ALEN], u16 vid,
> +			      enum macaccess_entry_type type)
> +{
> +	struct lan9645x_pgid_entry *new_pgid;
> +	struct lan9645x_mdb_entry *mdb_entry;
> +	u16 new_ports;
> +	int err;
> +
> +	mdb_entry = lan9645x_mdb_entry_lookup(lan9645x, addr, vid);
> +	if (!mdb_entry)
> +		return -ENOENT;
> +
> +	if (!(mdb_entry->ports & BIT(chip_port)))
> +		return 0;
> +
> +	new_ports = mdb_entry->ports & ~BIT(chip_port);
> +
> +	if (!new_ports) {
[ ... ]
> +		lan9645x_mdb_entry_dealloc(lan9645x, mdb_entry);
> +		return 0;
> +	}
> +
> +	/* Update PGID ptr for non-IP entries (L2 multicast) */
> +	new_pgid = lan9645x_mdb_pgid_entry_get(lan9645x, new_ports, type);
> +	if (IS_ERR(new_pgid))
> +		/* Continue forwarding to old port group. */
> +		return PTR_ERR(new_pgid);

Will an out-of-resources condition here cause the port to remain permanently
active in the hardware multicast group?

If lan9645x_mdb_pgid_entry_get() returns -ENOSPC because all hardware PGIDs
are in use, the deletion aborts and the hardware is not updated. Because the
bridge network stack ignores errors from port_mdb_del, the software state
would diverge from the hardware state.

Would it be possible to reuse and update the existing PGID in-place if its
reference count is exactly 1?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-dsa_lan9645x_switch_driver_base-v5-0-968fbf34ffa3@microchip.com?part=8