[PATCH 3/3] wifi: mac80211: update link RX NSS by ieee80211_sta_set_rx_nss() in ieee80211_assoc_config_link()

[Wen Gong]

On 9/26/2023 5:42 PM, Johannes Berg wrote:
> On Fri, 2023-09-15 at 15:53 +0800, Wen Gong wrote:
>> On 9/13/2023 5:04 PM, Johannes Berg wrote:
>>> On Wed, 2023-09-06 at 06:34 -0400, Wen Gong wrote:
>>>> Currently for MLO connection, only deflink's rx_nss is set to correct
>>>> value. The others links' rx_nss of struct ieee80211_link_sta is
>>>> value 0 in ieee80211_set_associated(), because they are not pass into
>>>> ieee80211_sta_set_rx_nss() in mac80211 except the deflink in
>>>> rate_control_rate_init(). This leads driver get NSS = 0 for other links.
>>>> Add the ieee80211_sta_set_rx_nss() in ieee80211_assoc_config_link(),
>>>> then the other links' rx_nss will be set to the correct value.
>>> This is pretty much true, but I also think it's problematic the way you
>>> phrase it. Software rate control is pretty much, at least currently,
>>> _not_ supported for MLO (and I don't really see how to support it, if
>>> firmware picks the link to transmit on, as it probably should).
>>>
>>> Thus, I'm not even sure we should be calling rate_control_rate_init().
>>> Clearly we do today, but it's also obviously wrong for everything except
>>> the call to ieee80211_sta_set_rx_nss().
>>>
>>> So while I agree that there's a problem with the RX NSS, I disagree that
>>> this patch is the right way to fix it. Yes, it also fairly obviously
>>> fixes the problem, but it just makes an existing design problem worse.
>>>
>>> Please change change the overall design here so that
>>> ieee80211_sta_set_rx_nss() isn't related to rate control at all.
>>>
>>> johannes
>> So should I delete ieee80211_sta_set_rx_nss() in rate_control_rate_init(),
>> and add it into ieee80211_assoc_config_link() as you said before here?
>> https://lore.kernel.org/linux-wireless/ca0f6ea2d78538ffb6640f2e56d65c89c86f5221.camel@sipsolutions.net/
> I think that would make sense. After all, rate_control_rate_init() is
> related to the software rate control which isn't really supported with
> MLD, and the NSS init is unrelated, it's just updating a piece of per
> (link) station data.
>
>> I checked the git log, ieee80211_sta_set_rx_nss() is added into
>> rate_control_rate_init() here for VHT, so is it correct to delete
>> ieee80211_sta_set_rx_nss() in rate_control_rate_init()?
>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?id=8921d04e8df7475d733d853564bdb001e83bf33f
> Well we'll have to call it appropriately when rate_control_rate_init()
> is called today, and then the new places in your patch, I guess.
>
> But I per the above that makes more sense semantically, since we don't
> support software rate control on link stations.
>
> johannes

OK. So I will send new version patch to remove the

ieee80211_sta_set_rx_nss() in rate_control_rate_init() as you said.