On 03/25/2018 12:45 PM, Alexander Wetzel wrote:
What will happen to drivers like ath10k that cannot do software
encrypt/decrypt?
ath10k can support multiple key-ids as far as I can tell,
so maybe it would just never hit this code?
Still learning how that all fits together, but I'm sure any card using
mac80211 will also use ieee80211_key_replace, including ath10k.
We are in a race with the remote station there is no chance that we can
switch over exactly at the same time. If we can't fall pack to software
encryption we'll just have to drop some more packets.
I'm pretty sure mac80211 will just encrypt a frame in software and
send it to ath10 for processing once we have removed the key from the hw
in the same way as for any other card.
I don't think ath10k can handle sending already-encrypted data packets,
but possibly it works with newer upstream firmware/driver.
Either way, as long as it does not fundamentally break something (like
a non-recoverable data stall), then maybe your patch is fine anyway
and ath10k may just drop a few extra frames.
My expectation here would be, that the driver detects and drops the
pre-encrypted frames it no longer has a hw key for.
Unfortunately this is just an assumption, since I haven't found the code
handling this case in ath10k. And even if true this could well cause
some undesired warning messages.
I guess we should therefore make sure we do not send out any packets in
the critical time window.
Now stopping and flushing the queues seems to be bad idea which could
cause a real performance impact for on a busy AP with many stations and
rekeys enabled...
Luckily it looks like we can instead just set KEY_FLAG_TAINTED for the
old key to make sure we stop sending packets till the rekey is done.
That should cause ieee80211_tx_h_select_key to drop all packets without
a new per-packet check and also should cover potential undesired side
effects, isn't it?
I get lost in the weeds when trying to understand all of this, and some
previous attempts of mine to fix some of this evidently wasn't correct
enough to accept upstream:
https://www.spinics.net/lists/hostap/msg03677.html
So I really don't know enough to properly review
your patch. Just be aware that ath10k is weird about sw-crypt, maybe make
sure your patch is tested on it to make sure it doesn't out-right break something.
Thanks,
Ben
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
