Re: [PATCH wireless-next] wifi: mac80211: Remove deleted sta links in ieee80211_ml_reconf_work()

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1665 bytes --]

On Mar 09, Johannes Berg wrote:
> On Sun, 2026-03-08 at 14:28 +0100, Lorenzo Bianconi wrote:
> > > > +	rcu_read_lock();
> > > > +	sta = sta_info_get(sdata, sdata->vif.cfg.ap_addr);
> > > > +	if (sta) {
> > > > +		unsigned long removed_links = sdata->u.mgd.removed_links;
> > > > +		unsigned int link_id;
> > > > +
> > > > +		for_each_set_bit(link_id, &removed_links,
> > > > +				 IEEE80211_MLD_MAX_NUM_LINKS)
> > > > +			ieee80211_sta_free_link(sta, link_id);
> > > > +	}
> > > > +	rcu_read_unlock();
> > > > +
> > > 
> > > Could use scoped_guard(rcu) instead?
> > 
> > I do not have a strong opinion here.
> > @Johannes: Which one do you prefer?
> > 
> 
> To answer the literal question: No strong preference I guess, given that
> there's no error path here this seems fine, and the scoped version would
> just add another indentation level.
> 
> But you really should just remove the rcu_read_lock/unlock anyway, it's
> not needed since this holds wiphy mutex, which is sufficient to access
> the STA table etc.

ack, I will fix it in v2.

> 
> Also, I think you need ieee80211_sta_remove_link() to tell the driver?

I think we already inform the driver running ieee80211_set_active_links() so
ieee80211_sta_remove_link() seems redundant, right?

ieee80211_set_active_links() -> _ieee80211_set_active_links() -> drv_change_sta_links()

> 
> And I realized another thing - this needs to destroy TDLS stations that
> were on the link being removed, but maybe that's a separate commit.

Do you mean ieee80211_teardown_tdls_peers()? Is it done in
_ieee80211_set_active_links() too?

Regards,
Lorenzo

> 
> johannes

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]