Re: [PATCH net-next v11 4/9] net: dsa: lan9645x: add basic dsa driver for LAN9645X

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Standalone ports drop all normal data plane traffic destined for the CPU because the CPU port module is missing from the source port forwarding masks (PGID_SRC) and the flooding masks (PGID_UC, PGID_MC).
- [Medium] Pause frame start watermark (PAUSE_START) exceeds the port's maximum allowed memory (ATOP) for jumbo frames, breaking flow control and causing silent tail drops.
--

--- Patch [4]: [PATCH net-next v11 4/9] net: dsa: lan9645x: add basic dsa driver for LAN9645X ---
commit dd8bddd9c4ac1f2dbff1feda2560552c682fa913
Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
Date:   Wed Aug 5 14:34:52 2026 +0000

    net: dsa: lan9645x: add basic dsa driver for LAN9645X
    
    Add the LAN9645X basic DSA driver with initialization, parent regmap
    requests, port module initialization for NPI, CPU ports and front ports,
    and phylink integration for MAC side configuration.

> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> +static int lan9645x_setup(struct dsa_switch *ds)
> +{
[ ... ]
> +	/* Disable bridging by default */
> +	for (int p = 0; p < lan9645x->num_phys_ports; p++) {
> +		lan_wr(0, lan9645x, ANA_PGID(PGID_SRC + p));
> +
> +		/* Do not forward BPDU frames to the front ports and copy them
> +		 * to CPU
> +		 */
> +		lan_wr(ANA_CPU_FWD_BPDU_CFG_BPDU_REDIR_ENA,
> +		       lan9645x, ANA_CPU_FWD_BPDU_CFG(p));
> +	}
[ ... ]
> +	/* Multicast to all front ports */
> +	lan_wr(front_ports, lan9645x, ANA_PGID(PGID_MC));
> +
> +	/* IP multicast to all front ports */
> +	lan_wr(front_ports, lan9645x, ANA_PGID(PGID_MCIPV4));
> +	lan_wr(front_ports, lan9645x, ANA_PGID(PGID_MCIPV6));
> +
> +	/* Unicast to all front ports */
> +	lan_wr(front_ports, lan9645x, ANA_PGID(PGID_UC));

[Severity: High]
Does this configuration drop normal data plane traffic intended for the CPU on
standalone ports? Setting the source forwarding mask (PGID_SRC) to 0 for all
user ports seems to block them from forwarding frames to any destination,
including the NPI or CPU port module.

Furthermore, deriving front_ports from dsa_user_ports(ds) excludes the CPU
port from the flooding masks. Should the CPU port be included in the source
forwarding and flooding masks to ensure host traffic is not blackholed?

> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
[ ... ]
> +void lan9645x_port_set_tail_drop_wm(struct lan9645x *lan9645x)
> +{
> +	int shared_per_port;
> +	struct dsa_port *dp;
> +
> +	/* Configure tail dropping watermark */
> +	shared_per_port =
> +		lan9645x->shared_queue_sz / (lan9645x->num_phys_ports + 1);
> +
> +	/* The total memory size is divided by number of front ports plus CPU
> +	 * port.
> +	 */
> +	dsa_switch_for_each_available_port(dp, lan9645x->ds)
> +		lan_wr(lan9645x_wm_enc(shared_per_port), lan9645x,
> +		       SYS_ATOP(dp->index));
[ ... ]
> +int lan9645x_port_set_maxlen(struct lan9645x *lan9645x, int port, size_t sdu)
> +{
> +	struct lan9645x_port *p = lan9645x_to_port(lan9645x, port);
> +	int maxlen = sdu + ETH_HLEN + ETH_FCS_LEN;
[ ... ]
> +	/* Set Pause WM hysteresis */
> +	lan_rmw(SYS_PAUSE_CFG_PAUSE_STOP_SET(lan9645x_wm_enc(4 * maxlen)) |
> +		SYS_PAUSE_CFG_PAUSE_START_SET(lan9645x_wm_enc(6 * maxlen)),
> +		SYS_PAUSE_CFG_PAUSE_START |
> +		SYS_PAUSE_CFG_PAUSE_STOP,
> +		lan9645x,
> +		SYS_PAUSE_CFG(p->chip_port));
> +
> +	return 0;
> +}

[Severity: Medium]
Will jumbo frames hit the tail drop watermark before triggering flow control?

The tail drop watermark (SYS_ATOP) in lan9645x_port_set_tail_drop_wm() appears
to be statically set to around 16KB. However, PAUSE_START dynamically scales
with maxlen (6 * maxlen) in lan9645x_port_set_maxlen().

For MTUs larger than roughly 2.7KB, PAUSE_START will exceed SYS_ATOP. Could
this cause the port to silently drop frames because it hits the tail drop
limit before ever reaching the threshold to emit pause frames?

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