Re: [net-next PATCH v3 2/8] net: dsa: realtek: rtl8365mb: use dsa helpers for port iteration

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: realtek: rtl8365mb: use dsa helpers for port iteration

This commit modifies the port initialization to use DSA iterator helpers
instead of manually iterating over ports. It introduces an initial blocking
phase for all ports and updates the isolation rules for CPU and user ports.

> diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
> --- a/drivers/net/dsa/realtek/rtl8365mb.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb.c
[ ... ]
> @@ -1974,44 +1977,58 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
[ ... ]
> +	/* Start with all ports blocked, including unused ports */
> +	dsa_switch_for_each_port(dp, ds) {
[ ... ]
> +		/* Start with all port completely isolated */
> +		ret = rtl8365mb_port_set_isolation(priv, dp->index, 0);
[ ... ]
> +	userports_mask = dsa_user_ports(ds);
> +
>  	/* Configure CPU tagging */
>  	dsa_switch_for_each_cpu_port(cpu_dp, ds) {
[ ... ]
> +		/* Forward to all user ports */
> +		ret = rtl8365mb_port_set_isolation(priv, cpu_dp->index,
> +						   userports_mask);

Will CPU ports be prevented from forwarding traffic to downstream switches
in a multi-switch configuration?

Since dsa_user_ports(ds) only includes user ports, the userports_mask will
exclude DSA cascade ports.

> +		if (ret)
> +			goto out_teardown_irq;
>  	}
> +
>  	cpu->mask = dsa_cpu_ports(ds);
>  	cpu->enable = cpu->mask > 0;
>  	ret = rtl8365mb_cpu_config(priv);
>  	if (ret)
>  		goto out_teardown_irq;
>  
> -	/* Configure ports */
> -	for (i = 0; i < priv->num_ports; i++) {
> -		struct rtl8365mb_port *p = &mb->ports[i];
> -
> -		if (dsa_is_unused_port(ds, i))
> -			continue;
> -
> +	/* Configure user ports */
> +	dsa_switch_for_each_user_port(dp, ds) {
>  		/* Forward only to the CPU */
> -		ret = rtl8365mb_port_set_isolation(priv, i, cpu->mask);
> +		ret = rtl8365mb_port_set_isolation(priv, dp->index, cpu->mask);
>  		if (ret)
>  			goto out_teardown_irq;
>  	}

Does this leave DSA cascade ports permanently isolated?

In the previous code, the loop skipped unused ports and applied the CPU mask
to all active ports, which implicitly included DSA cascade ports.

Because the new setup explicitly targets only user ports and CPU ports, it
appears DSA cascade ports (DSA_PORT_TYPE_DSA) are never unblocked after
being isolated in the initial blocking phase.