Re: [PATCH net-next 9/9] net: dsa: netc: implement dynamic FDB entry aging

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Setting aging time to 0 incorrectly results in aggressive 1-second aging instead of disabling aging.
- [Medium] Changes to the aging time do not take effect immediately due to failure to reschedule the pending delayed work.
--

commit d6713501ccd21f4b93ade1b9678b912b0922c0ca
Author: Wei Fang <wei.fang@nxp.com>

net: dsa: netc: implement dynamic FDB entry aging

The NETC switch does not age out dynamic FDB entries automatically. This
commit adds a delayed work to periodically remove unrefreshed entries,
along with DSA operations to configure aging time and flush per-port
entries on demand.

> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index 1fe3b43e04598..cffa479e19b3f 100644
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c

[ ... ]

> @@ -1977,6 +2026,34 @@ static void netc_port_bridge_leave(struct dsa_switch *ds, int port,
>  	netc_port_del_vlan_entry(np, vlan_unaware_pvid);
>  }
>  
> +static int netc_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
> +{
> +	struct netc_switch *priv = ds->priv;
> +	u32 secs = msecs / 1000;
> +	u32 act_cnt, interval;
> +
> +	if (!secs)
> +		secs = 1;

[Severity: High]
In the bridge subsystem, an aging time of 0 explicitly means that dynamic
FDB entries should never age out. Because the driver does not configure
ds->ageing_time_min, the DSA layer allows an msecs value of 0 to reach
netc_set_ageing_time().

Does forcing secs to 1 here break the ability to disable aging? 

Instead of disabling the software aging work, it configures the work to run
every 1 second. Can this cause immediate MAC address eviction when the
expected behavior is for aging to be disabled?

> +
> +	for (interval = 1; interval <= secs; interval++) {
> +		act_cnt = secs / interval;
> +		if (act_cnt <= FDBT_ACT_CNT)
> +			break;
> +	}
> +
> +	WRITE_ONCE(priv->fdbt_acteu_interval, (unsigned long)interval * HZ);
> +	WRITE_ONCE(priv->fdbt_ageing_act_cnt, act_cnt);

[Severity: Medium]
When netc_set_ageing_time() updates priv->fdbt_acteu_interval, it only uses
WRITE_ONCE() without updating the currently scheduled fdbt_clean work.

If the previous aging interval was set to a large value and is then reduced,
will the system wait for the remainder of the old duration before waking up
and applying the new aging parameters? Should this call mod_delayed_work()
to apply the changes immediately?

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527100217.794987-1-wei.fang@oss.nxp.com?part=9