Re: [PATCH 05/12] wifi: mt76: mt7915: report tx-retries

From: Ryder Lee <ryder.lee@mediatek.com>
To: Ben Greear <greearb@candelatech.com>, <linux-wireless@vger.kernel.org>
Cc: <nbd@nbd.name>
Subject: Re: [PATCH 05/12] wifi: mt76: mt7915: report tx-retries
Date: Thu, 28 Jul 2022 12:22:10 +0800	[thread overview]
Message-ID: <da8d837a5c39ed9fdef227db3ab624c4968f709a.camel@mediatek.com> (raw)
In-Reply-To: <309601ec-2b09-fc99-c003-882a895469d0@candelatech.com>

On Wed, 2022-07-27 at 21:11 -0700, Ben Greear wrote:
> On 7/27/22 8:20 PM, Ryder Lee wrote:
> > On Wed, 2022-07-27 at 16:01 -0700, greearb@candelatech.com wrote:
> > > From: Ben Greear <greearb@candelatech.com>
> > > 
> > > mac80211 stack will only report tx-status for skb claiming to be
> > > ampdu heads, so lie a bit in mt7915 and set the flag so that
> > > mac80211
> > > will record status for each skb.
> > > 
> > > mt7915 appears to report retry status on an individual per-skb
> > > manner,
> > > so that method above seems to work.
> > > 
> > > Re-constitute the txinfo status rate info so that the rix and
> > > flags
> > > is also at least close to correct.  No direct way to report HE
> > > rates that way, so mac80211 might could use some tweaking in
> > > the ieee80211_tx_status_ext to take both info and status->rate
> > > into account.
> > > 
> > > Signed-off-by: Ben Greear <greearb@candelatech.com>
> > > ---
> > >   drivers/net/wireless/mediatek/mt76/mac80211.c |  4 +
> > >   drivers/net/wireless/mediatek/mt76/mt76.h     |  5 +
> > >   .../net/wireless/mediatek/mt76/mt7915/init.c  |  1 +
> > >   .../net/wireless/mediatek/mt76/mt7915/mac.c   | 95
> > > ++++++++++++++++++-
> > >   .../net/wireless/mediatek/mt76/mt7915/mac.h   |  6 +-
> > >   .../net/wireless/mediatek/mt76/mt7915/main.c  |  4 +
> > >   .../net/wireless/mediatek/mt76/mt7921/main.c  |  4 +
> > >   drivers/net/wireless/mediatek/mt76/tx.c       | 18 +++-
> > >   8 files changed, 129 insertions(+), 8 deletions(-)
> > 
> > Hi Ben,
> > 
> > Looks like we have a similar goal but I don't use txs_for_no_skb.
> > 
> > What about directly switching to PPDU bases reporting (it should
> > include MPDUs total/failed/retries counts/bytes) and using
> > .sta_statistics? I think this might simplify the codes and can use
> > mac80211 ethtool as is.
> 
> I like per-packet tx-status because then I can do histograms up in
> mac80211
> (and the driver) to show distributions of nss, mcs, encoding types,
> etc.
> 
> I have not actually coded against the histograms I put in mac80211,
> but instead I'm using ethtool
> to get the histograms directly out of mt76 driver.
> 
> At least for 7915, I think you have to enable txs-for-no-skb logic to
> get
> details on mcs, nss and similar rate info?
> 
> Thanks,
> Ben
> 

Per PPDU txs provides mcs, nss and rate info as well, then we can use
.sta_statistics to report all sinfo we need. This is the way we use
for .net_fill_forward_path (WED) since it bypasses entire mac80211 data
path. We can also use iw station dump or ethtool to get histograms.

Ryder
> > 
> > I made these changes for .net_fill_forward_path (WED) cases. We're
> > doing some throughput tests.
> > 
https://urldefense.com/v3/__https://pastebin.com/raw/qCzNNDzw__;!!CTRNKA9wMg0ARbw!2EM8sO1SfUIboAp28ohdNT0OLwvCmyr-EuYoE-W1-IGsWn2RN8LI5LFNRjLBk3JM$
> >  
> > 
https://urldefense.com/v3/__https://pastebin.com/raw/25Vsk2xs__;!!CTRNKA9wMg0ARbw!2EM8sO1SfUIboAp28ohdNT0OLwvCmyr-EuYoE-W1-IGsWn2RN8LI5LFNRrLTxq5e$
> >  
> > 
> > 
> > Ryder
> > 
> > 
> 
> 