netdev - Re: iwlwifi warnings in 5.5-rc1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 12 Dec 2019 12:00:31 +0100
From:   Johannes Berg <johannes@...solutions.net>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Jens Axboe <axboe@...nel.dk>,
        Emmanuel Grumbach <emmanuel.grumbach@...el.com>,
        Luca Coelho <luciano.coelho@...el.com>
Cc:     "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>
Subject: Re: iwlwifi warnings in 5.5-rc1

On Thu, 2019-12-12 at 11:55 +0100, Toke Høiland-Jørgensen wrote:
> 
> > I'm not even sure we *can* do this easily - do we know up-front how many
> > packets this will expand to? We should know, but it might not be so easy
> > given the abstraction layers. We could guess and if it's wrong just set
> > it to 0 on any remaining ones.
> 
> I was thinking about a scheme where we re-defined the value in the cb to
> be a "time per byte" value, that we could just multiply by the packet
> length; that would make it trivial to do partial reporting. Not sure
> it's quite workable in practice, though; it would be hard to avoid
> rounding errors, and there's also the additional headers when splitting
> a packet, so the lengths don't necessarily add up.

Yeah, that won't really work. We could only estimate the # of pieces and
split up the value across them.

> > It's really just an artifact of our software implementation that we
> > report the SKBs back as used with partial content. Maybe we shouldn't
> > even do that, since they weren't generated by mac80211 in the first
> > place, and only report the original skb or something.
> 
> Hmm, yeah, was wondering how that works, actually. I assumed you send
> the whole thing to the hardware as one superpacket? But if so how do you
> get the completion events back? Or are you splitting it in the driver
> just before you send it to the hardware?

If we get say a 64k superpacket, we'll split it first into SKBs of max
A-MSDU size, and then rejigger the pieces inside each A-MSDU thing using
the DMA engine so we get A-MSDUs of MTU-sized packets out at the end.
The hardware is dumb here, it only takes care of TCP checksum.

> > I'm not really sure I want to rely on this - this was never really
> > needed *functionally*, just from a *statistics* point of view (e.g. "iw
> > link" or such).
> 
> Right, I see. Well I guess now that we're turning this on one driver at
> a time, we can ensure that the driver provides sufficiently accurate
> rate information as part of that.

Right.

> BTW, since we're discussing this in the context of iwlwifi: do you have
> any data as to how much benefit AQL would be for that? I.e., do the
> Intel devices tend to buffer a lot of data in hardware/firmware?

Hardware we have queues up to ~240 frames or so, otherwise no real
buffering. Per station/TID.

> > Ideally, it'd be a function call from the rate scaling to mac80211 so we
> > don't have to call a function every time we need the value, but the rate
> > scaling just calls us whenever it updates. This would even work with
> > iwlwifi's offloaded algorithm - it notifies the host on all changes.
> 
> Yup, this makes sense, and would be easy to integrate with Minstrel as
> well, I think. We already have ieee80211_sta_set_expected_throughput(),
> so maybe expanding that? It just provides a single number now, but we
> could change it to set the full rate info instead?

Right, was thinking around that area too. Not sure about the details
really though.

johannes