Re: Per MLO link TX stats

[Ben Greear <greearb@candelatech.com>]

On 8/27/24 09:41, Johannes Berg wrote:
> On Tue, 2024-08-27 at 09:28 -0700, Ben Greear wrote:
>> On 8/27/24 09:20, Johannes Berg wrote:
>>> On Tue, 2024-08-27 at 09:12 -0700, Ben Greear wrote:
>>>>
>>>> When be200 goes into eMLSR mode, both 5 and 6Ghz links are shown as active, so at least
>>>> you cannot use 'active link' to reliably update stats.
>>>
>>> Sure, not active link - but there's an LMAC bit somewhere ... Ah, it's
>>> not documented, it's actually documented *differently*, but it should be
>>> bit 31 in len_n_flags in struct iwl_rx_packet.
>>>
>>> Given the LMAC ID on the TX response notification we should know which
>>> LMAC transmitted it, and then I think it's a simple mapping to the
>>> active link. But I haven't actually really tried it.
>>
>> If you can share a patch that documents this bit (like 0 means 5Ghz and 1 means 6Ghz??)
>> then we can try it out.
> 
> I think yes, 5 GHz should be on LMAC 0 and 6 GHz on LMAC 1, and that's
> the only case where we can have two active links simultaneously.
> 
> There's still a race though, when we change the active links while
> transmitting, not sure how to handle that.
> 
> Oh wait, it's simpler than that - we have the STA pointer in there
> already (see iwl_mvm_rx_tx_cmd_single and iwl_mvm_rx_tx_cmd_agg), but
> since we get that from the FW STA ID, we obviously also know the *link*
> STA since the FW STA IDs are per link, so we can just go from there to
> the link ID directly.
> 
> link_sta = rcu_dereference(mvm->fw_id_to_link_sta[notif->sta_id]);
> 
> link_sta->link_id
> 
>> And maybe your idea for how to report it in tx-status too since that will touch
>> mac80211?
> 
> I hadn't really thought about that ... I guess we could use the
> IEEE80211_TX_CTRL_MLO_LINK space also for status? It's already filled to
> the link ID by mac80211 for TX if the frame must go out on a specific
> link (or 0xF otherwise which is an invalid link ID anyway.)

So, something like this (compile tested only at this point)?

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
index e7cb6dcde182..92f7d0d34421 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
@@ -1963,6 +1963,9 @@ static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,
         u8 lq_color;
         u16 next_reclaimed, seq_ctl;
         bool is_ndp = false;
+       struct ieee80211_link_sta *link_sta;
+
+       link_sta = rcu_dereference(mvm->fw_id_to_link_sta[sta_id]);

         __skb_queue_head_init(&skbs);

@@ -1989,6 +1992,10 @@ static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,

                 memset(&info->status, 0, sizeof(info->status));
                 info->flags &= ~(IEEE80211_TX_STAT_ACK | IEEE80211_TX_STAT_TX_FILTERED);
+               if (link_sta) {
+                       info->control.flags &= ~(u32_encode_bits(0xF, IEEE80211_TX_CTRL_MLO_LINK));
+                       info->control.flags |= u32_encode_bits(link_sta->link_id, IEEE80211_TX_CTRL_MLO_LINK);
+               }

                 /* inform mac80211 about what happened with the frame */
                 switch (status & TX_STATUS_MSK) {

And something similar for the agg path....

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com