On Mon, 2019-04-08 at 21:58 +0200, Alexander Wetzel wrote:
> Here how I understand the situation:
>
> The intent of db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on crypto
> controlled devices") was, to allow devices setting SW_CRYPTO_CONTROL to
> still support AP_VLAN group keys.
Yes, I think so.
> It unintentionally allowed more than it should for that...
I guess you're going to explain that in the text below :-)
> Prior to the commit db3bdcb9c3ff installing a group key when in AP_VLAN
> mode could not work. Mac80211 was initializing ret to -EOPNOTSUPP and
> then jumped to "out_unsupported" when the interface was in AP_VLAN mode
> without trying to install the key and giving the driver a chance to
> allow SW crypto. Software fallback was then blocked by ret != 1 with
> SW_CRYPTO_CONTROL set by the driver.
Right.
> But any driver setting NL80211_IFTYPE_AP_VLAN already has confirmed to
> be fine with SW crypto for group keys, since these CAN only be handled
> with SW crypto currently.
Well, NL80211_IFTYPE_AP_VLAN was set by mac80211 at the time still. Said
commit also changed that to only set it if SW_CRYPTO_CONTROL isn't set.
But yes, with the commit I agree - the driver is now setting
NL80211_IFTYPE_AP_VLAN so it's its own problem if something's wrong.
> The net effect was that ath10k - the only
> driver in tree setting SW_CRYPTO_CONTROL and also NL80211_IFTYPE_AP -
> was not able to send out frames encrypted with the group key at all.
I don't think I follow here. Before the commit in question, ath10k
would've rejected the key and not been able to. After the commit, and of
course also after 4920ce3bf7e0d ("ath10k: add dynamic vlan support")
(which you linked to below) ath10k again sets the NL80211_IFTYPE_AP_VLAN
bit under certain conditions, and thus *can* do software-crypto of such
frames.
> Now the fix for that was commit db3bdcb9c3ff. Unfortunately this went
> too far:
> We now not only allows fallback to SW crypto for group keys (which the
> driver allowed by setting NL80211_IFTYPE_AP_VLAN) but for ANY keys as
> long as the interface is in AP_VLAN mode.
Aha, now I follow.
> So if for some reasons ath10k is not able to install a pairwise key
> mac80211 now incorrectly allows the fallback to SW crypto...
Indeed, I agree.
> This patch here tries to fix that and excludes pairwise keys from the
> special AP_VLAN handling again (but keeps it for group keys).
Right, ok, makes sense now. I'll probably edit the commit message, but
yeah, I can follow your patch now.
Thanks,
johannes
