Re: [PATCH net-next v11 6/9] net: dsa: lan9645x: add vlan support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Code Fix needed in lan9645x_port_vlan_filtering to explicitly ignore the NPI port, preventing REW_TAG_CFG corruption.

Pre-existing issues:
- [High] The MAC table dump function `lan9645x_mact_dsa_dump` acquires a sleepable mutex and performs sleep-polling within an atomic RCU read-side critical section.
--

--- Patch [6]: [PATCH net-next v11 6/9] net: dsa: lan9645x: add vlan support ---
commit 7eb00c9a8e8dbdc6e13b0734d6fc7a8d54af0360
Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
Subject: net: dsa: lan9645x: add vlan support

This commit adds VLAN support for VLAN-aware and VLAN-unaware bridges.
VID 4095 is reserved for standalone ports and VID 0 is used for
VLAN-unaware bridge ports to isolate their FDB domains.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> @@ -576,11 +580,52 @@ static void lan9645x_port_bridge_leave(struct dsa_switch *ds, int port,
[ ... ]
> +static int lan9645x_port_vlan_filtering(struct dsa_switch *ds, int port,
> +					bool enabled,
> +					struct netlink_ext_ack *extack)
> +{
> +	struct lan9645x *lan9645x = ds->priv;
> +	struct lan9645x_port *p;
> +
> +	p = lan9645x_to_port(lan9645x, port);
> +	p->vlan_aware = enabled;
> +	lan9645x_vlan_port_apply(p);

[Severity: High]
Does this code inadvertently apply standard VLAN configurations to the NPI
(CPU) port?

When the bridge's VLAN filtering state changes, the DSA core propagates the
change to all ports in the bridge tree, including the CPU port. Here,
lan9645x_port_vlan_filtering() unconditionally executes
lan9645x_vlan_port_apply() which proceeds to modify the egress tagging.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
[ ... ]
> +static void
> +lan9645x_vlan_port_apply_egress(struct lan9645x_port *p,
> +				struct lan9645x_vlan_port_info *info)
> +{
[ ... ]
> +	lan_rmw(REW_TAG_CFG_TAG_TPID_CFG_SET(3) |
> +		REW_TAG_CFG_TAG_CFG_SET(tag_cfg),
> +		REW_TAG_CFG_TAG_TPID_CFG |
> +		REW_TAG_CFG_TAG_CFG,
> +		lan9645x, REW_TAG_CFG(p->chip_port));

[Severity: High]
Is it safe to rewrite the egress tag configuration for the NPI port here?

Writing the dynamically evaluated egress tag configuration to the
REW_TAG_CFG register erases the mandatory LAN9645X_TAG_DISABLED
configuration that is normally set during CPU port initialization.

If IFH extraction and packet rewriting conflict, a pushed VLAN tag could
overwrite the end of the extraction header with the top 4 bytes of the DMAC,
breaking CPU traffic. Should lan9645x_port_vlan_filtering() explicitly
ignore the NPI port?

[Severity: High]
This is a pre-existing issue, but does the MAC table dump function sleep
while holding rcu_read_lock()?

A user-space request to dump the Forwarding Database (e.g., via
bridge fdb show) invokes the netlink rtnl_fdb_dump() handler. This handler
takes rcu_read_lock() and iterates over devices, calling ndo_fdb_dump().
DSA passes this down to the driver's lan9645x_mact_dsa_dump().

Inside the RCU critical section, the driver acquires mact_lock (a sleepable
mutex) and uses lan9645x_mac_wait_for_completion() to poll the hardware,
which uses a regmap poll with a non-zero sleep interval:

drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c:lan9645x_mact_dsa_dump() {
	...
	mutex_lock(&lan9645x->mact_lock);
	...
	err = lan9645x_mac_wait_for_completion(lan9645x, &maca);
	...
}

Could this trigger a "scheduling while atomic" kernel panic or cause a
deadlock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-dsa_lan9645x_switch_driver_base-v11-0-007ebc983a0a@microchip.com?part=6