Re: Per MLO link TX stats

[Johannes Berg <johannes@sipsolutions.net>]

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.)

> > > > > In the case where there is a single active link, then I can hack something together
> > > > > that should be at least mostly right, but that won't fix any future radio that can
> > > > > do 2+ active links.
> > > > > 
> > > > > Any suggestions for best path forward on this?
> > > > 
> > > > I really think we also need to do some work on the API/cfg80211 level,
> > > > and have link station statistics in cfg80211 instead of full station,
> > > > and then combine them to (older) userspace in cfg80211, i.e. if
> > > > userspace doesn't request broken out per-link statistics. There's
> > > > probably a bunch of work here, and I only have a vague idea of how it
> > > > should be done...
> > > 
> > > I think first step is to get the driver(s) able to report the link-id in
> > > the tx-status.  After that, mac80211 can gather the stats.
> > 
> > Yeah, that makes sense, at least partially that's needed. I suspect that
> > also we need to extend the API down to the sta_statistics call though to
> > return per-link statistics, e.g. the TX bitrate would seem should be
> > reported per link, and that's done through that call now I believe.
> 
> Yes.  At least at cfg80211 level, I think we should be able to query all
> link stats in a single call into mac80211.  Down in mac80211, then per-link
> stats structs appears to be how things are done now and seems like a good
> solution.

Not sure I have a strong opinion on this being a single call or not,
either way seems reasonable really.

> > 
> > > I hacked
> > > tx/rx link stats into mac80211 ethtool (for first 3 links), but it is still not reliable since
> > > mac80211 doesn't know the actual tx link id.
> > 
> > Right.
> > 
> > > After that, then certainly I'd be happy to have per-link stats available,
> > > and combining them in cfg80211 seems like a fine idea as well.  Some things
> > > that don't combine well (rssi, link rates, etc) would take a bit of kludging
> > > if trying to provide a single 'sta' view of stats.
> > 
> > True, some can't just be added up and we'd have to find a sane different
> > "best" view, perhaps for rates it'd be the better of the two or the sum
> > if only reporting the bitrate, or better of the RSSI, etc. Case by case,
> > I guess.
> 
> I'd suggest using highest active band when we have no better option for
> summing/combining.

But is that the most useful thing? What if the RSSI is quite bad there
(but still good) and then NetworkManager shows a really weak signal when
in reality you have a good connection on another link? Anyway, dunno,
we're not near that bridge yet I think ;)

johannes