Message-ID: <b963b4257bba8b5423b93dd58c5828604fc07df4.camel@sipsolutions.net>
Subject: Re: [RFC PATCH v3 04/12] mac80211: Compatibility Extended Key ID
 support
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Alexander Wetzel <alexander@wetzel-home.de>
Cc:     linux-wireless@vger.kernel.org
Date:   Fri, 22 Feb 2019 09:53:08 +0100
In-Reply-To: <bad49ffb-5334-03f5-62e0-4f06d7c36242@wetzel-home.de>
References: <20190210210620.31181-1-alexander@wetzel-home.de>
         <20190210210620.31181-5-alexander@wetzel-home.de>
         <0519f25cf73d8a91202e40b08e94de8d2520411e.camel@sipsolutions.net>
         <bad49ffb-5334-03f5-62e0-4f06d7c36242@wetzel-home.de>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Thu, 2019-02-21 at 21:07 +0100, Alexander Wetzel wrote:
> > 
> > > +	if (!ext_native && key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
> > > +		key->flags |= KEY_FLAG_RX_SW_CRYPTO;
> > > +		/* Activate Rx crypto offload after max 10s when idle */
> > > +		ieee80211_queue_delayed_work(&local->hw, &sta->ext_key_compat_wk,
> > > +					     round_jiffies_relative(HZ * 10));
> > > +	}
> > 
> > Is there much point in this?
> > 
> > > +		if (unlikely(rx->key->flags & KEY_FLAG_RX_SW_CRYPTO)) {
> > > +			rx->key->flags &= ~KEY_FLAG_RX_SW_CRYPTO;
> > > +			cancel_delayed_work(&rx->sta->ext_key_compat_wk);
> > > +			ieee80211_queue_delayed_work(&rx->local->hw,
> > > +						     &rx->sta->ext_key_compat_wk, 0);
> > > +		}
> > 
> > We'll almost certainly do it from here, so never exercise the other
> > path?
> 
> This is mostly to have a definite time we know the new key is used also 
> for RX. In probably 99.9% of all cases it will be triggered from the Rx 
> path.
> Some special purpose devices may not send any packets for a long time 
> and trigger the fallback, as (wrong) firewall rules. (I've e.g. tested 
> it by dropping all outgoing packets on the remote sta.)
> 
> The idea was to be sure that a rekey intervall >10s prevents activating 
> Rx crypt when rekeying the next key. Which now sounds kind of thin...

Not sure I even understand this?

You meant "Rx crypto offload"? I'm not really sure we _care_ that much?

Then again, an issue may be that some firmware may want (need) the keys
for RX so it can look at certain frames (action frames?) itself. So if
we never install the RX key and then only get an action frame that the
firmware should handle, we lose. Such firmware could not support COMPAT
mode then I guess, which may mean a bunch of iwlwifi devices shouldn't
use COMPAT mode.

johannes