Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <943d0ea35c97f2e8076f127dbb43f5bf@mail.infomaniak.com>
Date:   Tue, 28 Mar 2023 11:23:54 +0200
Subject: Re: wifi: mt76: mt7915e: mt7916 monitor ampdu id is invalid
From:   Florian Schmidt <florian@fls.name>
Reply-To: Florian Schmidt <florian@fls.name>
To:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
References: <c7fc198f2a4ed9c1345a05c0397a9fc5@mail.infomaniak.com>
 <2f3ea768bc27a26918a4e8a4363b8c97@mail.infomaniak.com>
In-Reply-To: <2f3ea768bc27a26918a4e8a4363b8c97@mail.infomaniak.com>
Precedence: bulk

>  Hi all,
>=20
> Using a mt7916 in monitor mode, I see that the AMPDU reference number is =
incremented for each part of a single AMPDU. Instead it should be constant,=
 and ideally, the last subframe field should be set on the last part of the=
 AMPDU. Capturing the same traffic with an intel AX200 show each parts of t=
he AMPDU with the same id as expected.
>=20
> This issue is still present using kernel from tag wireless-testing-2023-0=
3-16.
>=20
> How can I be of any assist investigating and fixing this issue?
>=20
> Thanks,
> Florian

Replying to myself, it look like a firmware issue.=20

I dived a bit in the code. Toying with the code snippet below (from mt7915/=
mac.c), show that the firmware actually does something incorrect. It never =
increment the ampu_ref. This would be okay because the code below increment=
 the ampdu_ref when the timestamp are the same for all parts of the ampdu, =
unfortunately, the firmware put a different timestamp to each part of the A=
MPDU. Because of those two conditions, the ampdu_ref is simply incremented =
by the driver for every single packet even when parts of the same AMPDU.

/* all subframes of an A-MPDU have the same timestamp */
if (phy->rx_ampdu_ts !=3D status->timestamp) {
=09dev_warn(dev->mt76.dev, "AMPDU Timestamp diff: ref:%d t1:%d t2:%d\n",
=09=09=09 phy->ampdu_ref, phy->rx_ampdu_ts, status->timestamp);
=09if (!++phy->ampdu_ref)
=09=09phy->ampdu_ref++;
}

I guess I could maybe do some hack around this myself finding some relation=
 between timestamp of the ampdu, computing the end time of the previous par=
t of the AMPDU, but this would be really hacky and complicated for a workar=
ound to what look like a firmware issue.

Is there any mediatek dev around with access to firmware sources to help wi=
th this?

Thanks,
Florian