Re: [PATCH 2/2] rt2x00: enable experimental MFP with HW crypt

[Stanislaw Gruszka <sgruszka@redhat.com>]

On Tue, Mar 26, 2019 at 11:41:37AM +0100, Felix Fietkau wrote:
> On 2019-03-13 15:41, Tom Psyborg wrote:
> > On 13/03/2019, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> >> On Wed, Mar 13, 2019 at 02:48:01PM +0100, Tom Psyborg wrote:
> >>> On 13/03/2019, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> >>> > On Wed, Mar 13, 2019 at 02:02:32PM +0100, Tom Psyborg wrote:
> >>> >> On 13/03/2019, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> >>> >> > On Tue, Mar 12, 2019 at 08:11:42PM +0100, Tomislav Požega wrote:
> >>> >> >> MFP can work with enabled HW crypt engine, but in this case
> >>> >> >> available bandwidth is reduced at least when connecting to
> >>> >> >> Archer C7 (QCA9558). Enable the feature for known to work chipsets-
> >>> >> >> MT7620, RT3070 and RT5390. Userspace setting for ieee80211w should
> >>> >> >> default to 0 in order to prevent unintentional bandwidth drop.
> >>> >> >>
> >>> >> >> Signed-off-by: Tomislav Po?ega <pozega.tomislav@gmail.com>
> >>> >> >> ---
> >>> >> >>  drivers/net/wireless/ralink/rt2x00/rt2800lib.c |   11 +++++++----
> >>> >> >>  1 files changed, 7 insertions(+), 4 deletions(-)
> >>> >> >>
> >>> >> >> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >>> >> >> b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >>> >> >> index a03b528..bb8204d 100644
> >>> >> >> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >>> >> >> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >>> >> >> @@ -9326,6 +9326,13 @@ static int rt2800_probe_hw_mode(struct
> >>> >> >> rt2x00_dev
> >>> >> >> *rt2x00dev)
> >>> >> >>  	ieee80211_hw_set(rt2x00dev->hw, SIGNAL_DBM);
> >>> >> >>  	ieee80211_hw_set(rt2x00dev->hw, SUPPORTS_PS);
> >>> >> >>
> >>> >> >> +	/* Experimental: Set MFP with HW crypto enabled. */
> >>> >> >> +	if (rt2x00_rt(rt2x00dev, RT3070) || rt2x00_rt(rt2x00dev, RT5390)
> >>> >> >> ||
> >>> >> >> +	    rt2x00_rt(rt2x00dev, RT6352))
> >>> >> >> +		ieee80211_hw_set(rt2x00dev->hw, MFP_CAPABLE);
> >>> >> >
> >>> >> > Is not that we support MFP in hardware. We just return -EOPNOTSUPP
> >>> >> > in rt2x00mac_set_key() when mac80211 will try to set MFP ciphers
> >>> >> > (since rt2x00crypto_key_to_cipher() will return CIPHER_NONE) and
> >>> >> > we fallback to software encryption.
> >>> >> >
> >>> >> > Please repost patch that enable MFP unconditionally with
> >>> >> > 'Cc: stable@vger.kernel.org' tag.
> >>> >> >
> >>> >> > Stanislaw
> >>> >> >
> >>> >>
> >>> >> No, I have not test any other chipsets besides the ones I enabled it
> >>> >> for. It is possible this would cause problems on other devices, so
> >>> >> just enable it for the known to work ones.
> >>> >
> >>> > It's just matter of sending already encrypted frames. All chipsets
> >>> > handle that.
> >>> >
> >>> > Stanislaw
> >>> >
> >>>
> >>> The question is how well all chipsets handle that. I've seen some lags
> >>> too with MFP enabled connection. While being about 40-50% lower,
> >>> throughput would still occasionally drop to very low values, like
> >>> 800Kbps.
> >>
> >> The only reason I can image that might have impact on throughput
> >> in this case is limited CPU power since encryption is done in
> >> software. Would be good to compare with PA2 with nohwcrypte, if
> >> there are similar lags. However MFP can require more CPU power.
> >>
> >> Stanislaw
> >>
> > 
> > nohwcrypt=y mfp=require: there was no throughput drop, no lag,
> > measured about 80-85Mbps but the connection frozen after a minute or
> > two. might be related to rekeying.
> > 
> > CPU power cannot be the problem since I run it on laptop (2x2.20GHz)
> The lag is probably caused by failed A-MPDU setup exchanges due to
> broken management frames.
> 
> I think it's likely that rt28xx chipsets are behaving like mt7612e in
> that area (since the MAC design is very similar).
> I've spent quite a bit of time making MFP work properly in mt76 on that
> chipset, and here are my findings:
> Hardware crypto for management frames is broken.
> When using management frame protection, the same pairwise encryption key
> needs to be used for both data frames and management frames.
> There is a flag in mac80211 that allows software-encryption for
> management frames only.
> That leads us to the next issue: CCMP PN is usually generated in
> hardware and stored in on-chip memory, which means the mac80211 key
> tx_pn counter will be out of sync and software-encrypted management
> packets will trigger replay protection on the other side.
> To fix this, you need to enable software generated IV if management
> frames are to be sent with the same key.
> In that case you need to set TXINFO_W0_WIV and fill TXWI_W2_IV+TXWI_W3_EIV.
> Also, for software encrypted frames you need to mask out WCID, since
> otherwise the hardware will try to encrypt the frame again.
> You can use mt76 as reference for how to do all of this

I see, I got it completely wrong. I thought with MFP we will use non
supported rt2x00 cipher and we will fallback to software encryption
anyway.

So, until properly implemented we should stay with nohwcrypt for MFP.

Stanislaw