Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <e6ea3009c489fae910adbf2e1c766f2d827f287f.camel@sipsolutions.net>
Subject: Re: [PATCH 3/3] wifi: mac80211: update link RX NSS by
 ieee80211_sta_set_rx_nss() in ieee80211_assoc_config_link()
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Wen Gong <quic_wgong@quicinc.com>, ath12k@lists.infradead.org
Cc:     linux-wireless@vger.kernel.org
Date:   Wed, 13 Sep 2023 11:04:26 +0200
In-Reply-To: <20230906103458.24092-4-quic_wgong@quicinc.com>
References: <20230906103458.24092-1-quic_wgong@quicinc.com>
         <20230906103458.24092-4-quic_wgong@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

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 =3D 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