Re: [PATCH net 1/5] net: dsa: mt7530: fix FDB entries not aging out with short timeout

[Paolo Abeni]

On 5/5/26 4:16 PM, Daniel Golle wrote:
> When setting a low ageing time such as 10 seconds, the algorithm in
> mt7530_set_ageing_time() finds AGE_CNT=0 and AGE_UNIT=9 as the first
> exact match (starting the search from tmp_age_count=0).
> 
> On the MT7530/MT7531 hardware, the per-entry aging counter is
> initialized to AGE_CNT when a MAC address is learned. With AGE_CNT=0,
> new entries start with a counter value of 0, which the hardware treats
> as "already aged" and never removes, effectively disabling aging.
> 
> Fix this by starting the search from tmp_age_count=1 to ensure entries
> always have a non-zero initial aging counter. For a 10-second ageing
> time this yields AGE_CNT=1 and AGE_UNIT=4 instead: the timer ticks
> every 5 seconds and entries are removed after 2 ticks.
> 
> Fixes: ea6d5c924e39 ("net: dsa: mt7530: support setting ageing time")
> Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
> ---
>  drivers/net/dsa/mt7530.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 44d670904ad8..b1903da7d500 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1027,8 +1027,12 @@ mt7530_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
>  	if (secs < 1 || secs > (AGE_CNT_MAX + 1) * (AGE_UNIT_MAX + 1))
>  		return -ERANGE;
>  
> -	/* iterate through all possible age_count to find the closest pair */
> -	for (tmp_age_count = 0; tmp_age_count <= AGE_CNT_MAX; ++tmp_age_count) {
> +	/* Iterate through all possible age_count values to find the closest
> +	 * pair. Start from 1 because the per-entry aging counter is
> +	 * initialized to AGE_CNT and a value of 0 means the entry will
> +	 * never be aged out.
> +	 */
> +	for (tmp_age_count = 1; tmp_age_count <= AGE_CNT_MAX; ++tmp_age_count) {
>  		unsigned int tmp_age_unit = secs / (tmp_age_count + 1) - 1;
>  
>  		if (tmp_age_unit <= AGE_UNIT_MAX) {

Sashiko noted that the above will have problem with secs == 1:

What happens here if secs is 1?
Since the bounds check at the start of the function allows secs == 1,
tmp_age_unit would be calculated as 1 / (1 + 1) - 1, which evaluates to
0 - 1, resulting in an unsigned underflow to UINT_MAX.
>  		if (tmp_age_unit <= AGE_UNIT_MAX) {
Because UINT_MAX is greater than AGE_UNIT_MAX, this condition will fail for
all iterations of the loop.
[ ... ]
>  	mt7530_write(priv, MT7530_AAC, AGE_CNT(age_count) | AGE_UNIT(age_unit));
If the loop exits without ever finding a match and entering the if block,
age_count and age_unit will remain uninitialized. Could this result in
uninitialized stack variables being written to the MT7530_AAC hardware
register?

/P