Subject: Re: [RFC PATCH v3 07/12] iwlwifi: Extended Key ID support (NATIVE)
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     linux-wireless@vger.kernel.org
References: <20190210210620.31181-1-alexander@wetzel-home.de>
 <20190210210620.31181-8-alexander@wetzel-home.de>
 <1a3b6e515c73a2c185e8dad84ab2ebfd8982a6ce.camel@sipsolutions.net>
 <e84c30de-28d6-7d1b-0fa3-d1ca466e257b@wetzel-home.de>
 <69e6577f90d99289acaa9853fe236e6f15f9e774.camel@sipsolutions.net>
 <14c9d8f7-7cf6-d7e1-a1c0-9f1a10920d4e@wetzel-home.de>
 <ed6e95dad39ef0ac98c62e3acd3f78fd7b3a6d2b.camel@sipsolutions.net>
 <185ea9a2-f3c6-04a5-000b-44191da5a0ee@wetzel-home.de>
 <0de9d60ffef574b34e9a76ad2cea68fab49aac0f.camel@sipsolutions.net>
 <45ae97d6-3357-64ac-0a40-9ae3ea4a8ed2@wetzel-home.de>
 <84694a97bf884985afd49d93e28b309a92801916.camel@sipsolutions.net>
 <577d4307-27ca-c5f5-8814-bbef515559e3@wetzel-home.de>
 <91d60fa7ba614c96fe2814375a28802e9165218b.camel@sipsolutions.net>
From:   Alexander Wetzel <alexander@wetzel-home.de>
Message-ID: <7338263c-9e01-1559-888f-adcb4b7c8ca1@wetzel-home.de>
Date:   Tue, 16 Apr 2019 20:28:28 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <91d60fa7ba614c96fe2814375a28802e9165218b.camel@sipsolutions.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

>> Could we not simply ask (at least) the drivers supporting Extended Key
>> IDs to implement a special for the remote STA invisible A-MPDU "stop" mode?
>> In this new mode each A-MPDU would simply be build out of a single MPDU.
>> We then could keep Block-Ack active and we don't have to tell the remote
>> STA anything about that decision. After all a A-MPDU with only one MPDU
>> is allowed...
>> We then could tell the driver to switch to this mode when installing the
>> new key and out of it again when we have activated the new key for Tx.
>> That should be perfectly fine to run only in the control path and
>> comparable simple to set up, too.
> 
> Sounds doable, but I'm still debating if we even should give them a
> signal - they have all the information in a sense, and do we have a good
> idea of when we can go out of this mode again?

Handling that on the Tx path would be tricky, but I you are right: 
Drivers should be able to handle that as it is on the control path, too.

They can enable the mode when a key with IEEE80211_KEY_FLAG_NO_AUTO_TX 
set and quiet it again as soon as they get a MPDU using the new KeyID.
Since switching back to normal doesn't have to be done immediately a 
asyc call from Tx path or even a worker should do the job just fine.

Btw:
This also means we'll have to update the merged mac80211 Extended Key ID 
support: We can only enable it for cards without HW crypto when they do 
not set AMPDU_AGGREGATION. With the updated userspace these cards will 
start using Extended Key ID with the already merged patches.

Drivers which should be able to use Extended Key ID with the two merged 
patches seem to be:
admtek/adm8211.c
ath/ar5523/ar5523.c
broadcom/b43legacy/main.c
broadcom/brcm80211/brcmsmac/mac80211_if.c
mac80211_hwsim.c
marvell/libertas_tf/main.c
ralink/rt2x00/rt2400pci.c
ralink/rt2x00/rt2500pci.c
realtek/rtl818x/rtl8180/dev.c
realtek/rtl818x/rtl8187/dev.c
zydas/zd1211rw/zd_mac.c

Of those only hwsim and brcmsmac seems to support AMPDU and only 
brcmsmac relly needs the fix to not lose some packets when rekeying.


>> That also sounds like something all drivers supporting A-MPDU should be
>> able to pull off somehow. (But then I've never even looked at more than
>> ath9k and iwlwifi so far for A-MPDU, and at those not close, yet.)
>> Do you see any problems with that solution and/or a better idea?
> 
> It ought to be possible, or if not then the device just can't support
> extended key ID?

Agree. Or drivers can decide to deny A-MPDU setup when the key has been 
installed with IEEE80211_KEY_FLAG_NO_AUTO_TX when they want.

>> Also would you prefer we first sort out the A-MPDU issue prior I adding
>> test cases to the hostapd/wpa_supplicant or the other way round?
> 
> I think adding the code to hostapd/wpa_s is fine - right now we're
> obviously OK since we have no drivers using extended key ID that use
> hardware crypto, and if we have software crypto there's no problem
> either way, even if A-MPDUs are built (which is probably not the case
> for any such drivers anyway.)
> 
> In a sense I'd prefer getting the necessary bits and pieces elsewhere in
> the stack upstream first since that's a prerequisite for anyone
> else being able to easily work on this, and that's something we need for
> drivers to pick it up.

Ok:-)

I assume we still have to wait till the API is in mainline (probably 
5.2) to ask hostapd/wpa_supplicant to merge the patches?

At the moment I'm planning to get the patches merge ready and posted as 
RFC till 5.2 is out. I'm also planing to keep Extended Key ID for Mesh 
networks till later, so we can focus on AP/STA.


Alexander