Re: [PATCH RFC wireless-next] wifi: mac80211_hwsim: dont perform rate validation for S1G

[Johannes Berg <johannes@sipsolutions.net>]

On Tue, 2025-10-21 at 14:36 +1100, Lachlan Hodges wrote:
> > > If an S1G driver is submitted it
> > > would be considered a "modern" driver and would probably contain its own
> > > rate control. There has been some S1G hostap work recently submitted [1]
> > > though it's only STA side though this will need to be fixed in order for
> > > proper tests to be run.
> > 
> > I'm not sure really to be considered "modern" you need driver/firmware
> > rate control, but I'd assume a new driver were to do that anyway :)
> > 
> > So overall it seems reasonable to mostly ignore the rate control issue,
> > but maybe it'd make more sense to be "cleaner" about it, maybe
> > separating out something like HAS_RATE_CONTROL to be per station, i.e.
> > letting drivers somehow (not) set WLAN_STA_RATE_CONTROL?
> 
> I do agree - I have some initial rough thoughts -
> 
> Separating out HAS_RATE_CONTROL to be per station rather then per wiphy
> seems to be a major change for just this single case? 

Yeah, that's true.

> Now Im not sure what
> Wi-Fi8 entails (heh) but I also understand it's best to do this properly rather
> then some S1G specific hack.

WiFi8/UHR isn't going to change anything in this area, it's not really
any different from WiFi7/EHT wrt. this.

> It seems that introducing a flags parameter into the ieee80211_link_sta where
> drivers can apply the inverse i.e LINK_STA_NO_RATE_CONTROL or something similar
> that before drv_link_sta_rc_update() is called the flag can be tested, something
> like, for example within rate_control_rate_update():
> 
> [...]
> 	if (sta->uploaded && !(link_sta->pub->flags & LINK_STA_NO_RC))
> 		drv_link_sta_rc_update(local, sta->sdata, link_sta->pub,
> 				       changed);
> [...]
> 
> Then w.r.t setting the flag, we could introduce a new function that gets
> called within mac80211_hwsim_sta_add() called mac80211_hwsim_link_sta_set_flags()
> or something where it could enumerate the links of the ieee80211_sta and
> apply the flag to the link with an S1G band channel context?
> 
> The reason for it being driver-facing is that any future upstream S1G drivers
> may make use of the .sta_rc_update() callback.

But the .sta_rc_update() call is desired/needed either way (if
implemented) - the issue is about the _other_ side of this, "if (ref &&
ref->ops->rate_update)", no?

So I'd rather think we need something like

static int sta_prepare_rate_control(struct ieee80211_local *local,
                                    struct sta_info *sta, gfp_t gfp)
{
        if (ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL))
                return 0;

+	if (something new here)
+		return 0;
+
        sta->rate_ctrl = local->rate_ctrl;
        sta->rate_ctrl_priv = rate_control_alloc_sta(sta->rate_ctrl,
                                                     sta, gfp);
        if (!sta->rate_ctrl_priv)
                return -ENOMEM;

        return 0;
}


However, an issue is that the "something new here" can't check a per-STA
flag because this happens before the driver is actually told about the
station.

I suppose the easiest way, if a bit special to hwsim, would be to add a
specific flag for S1G, so that "something new here" becomes just

	on_s1g && ieee80211_hw_check(&local->hw, HAS_S1G_RATE_CONTROL)

(where on_s1g needs the sdata passed to the function and the chanctx
checked)

Then hwsim can sort it out or even just call rate_control_send_low() for
that all the time (which has a TODO for S1G buried, but that probably
needs addressing either way eventually.)

johannes