Re: [PATCH net-next v8 5/9] net: dsa: lan9645x: add bridge support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing ds->ageing_time_min allows invalid ageing times to be silently rounded up to 2 seconds.
--

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

net: dsa: lan9645x: add bridge support

This commit adds support for hardware offloading of the bridge on a single
device. It configures basic bridge operations, flooding, and ageing time.

> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index f95ce9e48a5c2..4dd449d9014bd 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> @@ -268,15 +271,35 @@ static int lan9645x_setup(struct dsa_switch *ds)
[ ... ]
>  	ds->mtu_enforcement_ingress = true;
>  	ds->assisted_learning_on_cpu_port = true;
>  	ds->fdb_isolation = true;
>  
> +	/* ANA_AUTOAGE_AGE_PERIOD is a seconds-based field and entries are
> +	 * aged after 2 * AGE_PERIOD, giving (2 * FIELD_MAX) seconds of
> +	 * maximum aging.
> +	 */
> +	ds->ageing_time_max = 2U * MSEC_PER_SEC *
> +			      FIELD_MAX(ANA_AUTOAGE_AGE_PERIOD);
> +

[Severity: Medium]
Should ds->ageing_time_min be configured here as well?

Without a minimum, the DSA core's -ERANGE check is bypassed when
userspace configures a bridge ageing time less than 2 seconds, which
allows unsupported small values to reach the driver.

>  	dev_info(lan9645x->dev,
>  		 "SKU features: max_ports=%d\n",
>  		 lan9645x->num_phys_ports - lan9645x->num_port_dis);
>  
>  	return 0;
[ ... ]
> @@ -285,6 +308,282 @@ static void lan9645x_port_phylink_get_caps(struct dsa_switch *ds, int port,
[ ... ]
> +static int lan9645x_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
> +{
> +	u32 age_secs = max(1, msecs / MSEC_PER_SEC / 2);

[Severity: Medium]
Could this silently round up unsupported small ageing times?

If msecs is non-zero but less than 2000, max(1, msecs / 2000) evaluates
to 1 hardware unit (2 seconds). Because the driver doesn't return an
error for unsupported values, the hardware is configured with an
ageing time larger than requested by the user, which violates the
switchdev API contract.

> +	struct lan9645x *lan9645x = ds->priv;
> +
> +	/* Entry must suffer two aging scans before it is removed, so it is
> +	 * aged after 2*AGE_PERIOD, and the unit is in seconds.
> +	 * An age period of 0 disables automatic aging.
> +	 */
> +	lan_rmw(ANA_AUTOAGE_AGE_PERIOD_SET(msecs ? age_secs : 0),
> +		ANA_AUTOAGE_AGE_PERIOD,
> +		lan9645x, ANA_AUTOAGE);
> +	return 0;
> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-dsa_lan9645x_switch_driver_base-v8-0-90228d8bba58@microchip.com?part=5