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@xxxxxxxxxxxxxxxx/
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
