Re: [PATCH net v2] net: hsr: clean up synced addresses and allmulti in hsr_del_port()

[Hangbin Liu <hangbin.liu@linux.dev>]

On Thu, Nov 12, 2026 at 02:47:16PM +0800, Wenbo Sun wrote:
> hsr_set_rx_mode() syncs unicast and multicast addresses to slave A and
> slave B devices via dev_uc_sync_multiple() and dev_mc_sync_multiple(),
> and hsr_change_rx_flags() propagates IFF_ALLMULTI via
> dev_set_allmulti().
> 
> When an UP HSR device is deleted via hsr_dellink(), hsr_del_ports()
> removes all ports from hsr->ports before unregister_netdevice_queue()
> leads to hsr_dev_close(). Consequently, hsr_dev_close() iterates over an
> already-empty port list and cannot execute dev_uc_unsync() or
> dev_mc_unsync() on the detached slaves. Likewise, when a slave device is
> unregistered while the HSR master is UP, hsr_netdev_notify() calls
> hsr_del_port() directly without running hsr_dev_close(). In addition,
> hsr_dev_close() does not release IFF_ALLMULTI on slave devices either.
> As a result, detached slave devices can retain synced unicast/multicast
> addresses and an elevated allmulti reference count.
> 
> Clean up these states in hsr_del_port() when detaching slave A or slave B
> by calling dev_uc_unsync(), dev_mc_unsync(), and, when IFF_ALLMULTI is
> set on the master device, dev_set_allmulti(port->dev, -1).
> 
> This issue was identified with the assistance of an LLM-based code
> review tool, confirmed by reproducing the retained allmulti reference
> count and synced multicast address in a User-Mode Linux (UML)
> environment upon deleting an active HSR interface, and verified in UML
> that both states are properly cleared after HSR port deletion.
> 
> Fixes: 36b20fcdd966 ("net: hsr: Add support for MC filtering at the slave device")
> Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
> Signed-off-by: Wenbo Sun <sunwenbo@cmss.chinamobile.com>
> ---
> Changes in v2:
> - Clarify the commit message to explain explicitly why hsr_dev_close()
>   iterates over an already-empty hsr->ports list on hsr_dellink() and
>   does not release IFF_ALLMULTI, answering Hangbin Liu's question.
> - Add Reviewed-by from Hangbin Liu.
> - Link to v1: https://lore.kernel.org/r/20261111060143.183670-1-sunwenbo@cmss.chinamobile.com
> 
>  net/hsr/hsr_slave.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
> index a546f70f9cc8..bab0c27dd06a 100644
> --- a/net/hsr/hsr_slave.c
> +++ b/net/hsr/hsr_slave.c
> @@ -245,8 +245,14 @@ void hsr_del_port(struct hsr_port *port)
>  		netdev_rx_handler_unregister(port->dev);
>  		if (!port->hsr->fwd_offloaded || port->type == HSR_PT_INTERLINK)
>  			dev_set_promiscuity(port->dev, -1);
> -		if (port->type == HSR_PT_SLAVE_A || port->type == HSR_PT_SLAVE_B)
> +		if (port->type == HSR_PT_SLAVE_A ||
> +		    port->type == HSR_PT_SLAVE_B) {
>  			vlan_vids_del_by_dev(port->dev, master->dev);
> +			dev_uc_unsync(port->dev, master->dev);
> +			dev_mc_unsync(port->dev, master->dev);
> +			if (master->dev->flags & IFF_ALLMULTI)
> +				dev_set_allmulti(port->dev, -1);
> +		}
>  		netdev_upper_dev_unlink(port->dev, master->dev);
>  		if (hsr->prot_version == PRP_V1 &&
>  		    port->type == HSR_PT_SLAVE_B) {
> -- 
> 2.33.0
> 
> 
> 

Sorry, you post v2 version to fast. Please check if
dev_uc_unsync()/dev_mc_unsync() could be called from hsr_dev_close(). If not,
should we remove them from hsr_dev_close()?

Thanks
Hangbin