All of
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <>
To: Johannes Berg <>,
	Jens Axboe <>,
	Emmanuel Grumbach <>,
	Luca Coelho <>
Cc: "linux-wireless\"
	Networking <>
Subject: Re: iwlwifi warnings in 5.5-rc1
Date: Wed, 11 Dec 2019 15:02:26 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Johannes Berg <> writes:

> Hi Toke,
>> > OK, I talked with Emmanuel and I think it's the GSO path - it'll end up
>> > with skb_clone() and then report both of them back.
>> Right, figured it was something like that; just couldn't find the place
>> in the driver where it did that from my cursory browsing.
> Yeah, deeply hidden in the guts :)
>> > Regardless, I think I'll probably have to disable AQL and make it more
>> > opt-in for the driver - I found a bunch of other issues ...
>> Issues like what? Making it opt-in was going to be my backup plan; I was
>> kinda hoping we could work out any issues so it would be a "no harm"
>> kind of thing that could be left as always-on. Maybe that was a bit too
>> optimistic; but it's also a pain having to keep track of which drivers
>> have which features/fixes...
> Sorry to keep you in suspense, had to run when I sent that email and
> didn't have time to elaborate.
> 1) Hardware building A-MPDU will probably make the airtime estimate
>    quite a bit wrong. Maybe this doesn't matter? But I wasn't sure how
>    this works now with ath10k where (most of?) the testing was.

Yeah, not too worried about this...

> 2) GSO/TSO like what we have - it's not really clear how to handle it.
>    The airtime estimate will shoot *way* up (64kB frame) once that frame
>    enters, and then ... should it "trickle back down" as the generated 
>    parts are transmitted? But then the driver needs to split up the
>    airtime estimate? Or should it just go back down entirely? On the
>    first frame? That might overshoot. On the last frame? Opposite
>    problem ...

Well, ideally it would be divided out over the component packets; but
yeah, who is going to do that? I think reporting it on the first packet
would be the safest if we had to choose. Also, ideally we would want the
GSO/TSO mechanism to lower the size of the superpackets at lower rates
(does it?). At higher rates this matters less...

> 3) I'm not quite convinced that all drivers report the TX rate
>    completely correctly in the status, some don't even use this path
>    but the ieee80211_tx_status_ext() which doesn't update the rate.
> 4) Probably most importantly, this is completely broken with HE because
>    there's currently no way to report HE rates in the TX status at all!
>    I just worked around that in our driver for 'iw' reporting purposes
>    by implementing the rate reporting in the sta_statistics callback,
>    but that data won't be used by the airtime estimates.

Hmm, yeah, both of those are good points. I guess I just kinda assumed
that the drivers were already doing the right thing there... :)

> Now, (1) probably doesn't matter, the estimates don't need to be that
> accurate. (2) I'm not sure how to solve; (3) and (4) could both be
> solved by having some mechanism of the rate scaling to tell us what the
> current rate is whenever it updates, rather than relying on the
> last_rate. Really we should do that much more, and even phase out
> last_rate entirely, it's a stupid concept.

Yes, that last bit would be good!

> There's an additional wrinkle here - what about HE scheduled mode, where
> the AP decided when and at what rate you're allowed to send? We don't
> report that at all, not even as part of rate scaling, since rate scaling
> only affects *our* decision, not when we send as a response to a trigger
> frame. This is _still_ relevant for AQL, but there we can only see what
> the AP used last (**), but we don't track that now nor do we track the
> proportion of packets that we sent triggered vs. normal medium
> access...

Huh, wasn't aware that was a thing in HE; that's cool! And yeah, could
have interesting interactions with AQL...


  parent reply	other threads:[~2019-12-11 14:02 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-10 20:46 iwlwifi warnings in 5.5-rc1 Jens Axboe
2019-12-11  8:36 ` Johannes Berg
2019-12-11  8:53   ` Toke Høiland-Jørgensen
2019-12-11 10:11     ` Johannes Berg
2019-12-11 10:23       ` Toke Høiland-Jørgensen
2019-12-11 11:51         ` Johannes Berg
2019-12-11 13:42           ` Johannes Berg
2019-12-11 14:04             ` Toke Høiland-Jørgensen
2019-12-11 14:12               ` Johannes Berg
2019-12-11 14:47                 ` Toke Høiland-Jørgensen
2019-12-11 21:18                   ` Johannes Berg
2019-12-12 10:45                     ` Toke Høiland-Jørgensen
2019-12-11 14:02           ` Toke Høiland-Jørgensen [this message]
2019-12-11 21:17             ` Johannes Berg
2019-12-12 10:55               ` Toke Høiland-Jørgensen
2019-12-12 11:00                 ` Johannes Berg
2019-12-21  0:55   ` Jens Axboe
2019-12-21  9:17     ` Johannes Berg
2019-12-21 13:45       ` Jens Axboe
2019-12-11 13:45 ` Johannes Berg
2019-12-11 14:09   ` Toke Høiland-Jørgensen
2019-12-11 14:13     ` Johannes Berg
2019-12-11 14:55       ` Toke Høiland-Jørgensen
2019-12-11 21:19         ` Johannes Berg
2019-12-12 10:55           ` Toke Høiland-Jørgensen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.