Re: [patch 1/2]d80211: hardware TKIP support for ipw3945

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2006-10-25 at 16:28 +0800, Hong Liu wrote:

> I'd prefer to let the stack tell the driver when there is new phase1 key
> generated.

Fine too, I guess.

> +	u8 tkip_keylen;

What do you need that for? The driver should know whether it requested a
phase 1 or phase 2 key.

> +	u8 tkip_key[16];/* generated RC4/phase1 key for hw TKIP */

Do we really have to stick this into this structure? But I'll let Jiri
figure out how to remove the structure bloat :)
 
> +	/* calculate Michael MIC for an MSDU when doing hwcrypto */
> +#define IEEE80211_HW_TKIP_INCLUDE_MMIC (1<<12)
> +	/* Do TKIP phase1 key mixing in stack to support cards only do
> +	 * phase2 key mixing when doing hwcrypto */
> +#define IEEE80211_HW_TKIP_REQ_PHASE1_KEY (1<<13)
> +	/* Do TKIP phase1 and phase2 key mixing in stack and send the generated
> +	 * per-packet RC4 key with each TX frame when doing hwcrypto */
> +#define IEEE80211_HW_TKIP_REQ_PHASE2_KEY (1<<14)

Maybe a comment indicating that you must not set both of these flags
would be good. Or (see below)

Should there be some flag indicating if the hw/firmware checked the MIC
on reception? The current code has bad assumptions there:

(from the pre-flags version)

        /* Some devices handle Michael MIC internally and do not include MIC in
         * the received packets passed up. device_strips_mic must be set
         * for such devices. The 'encryption' frame control bit is expected to
         * be still set in the IEEE 802.11 header with this option unlike with
         * the device_hides_wep configuration option.
         */
        unsigned int device_strips_mic:1;

What if the devices leaves the MIC there but indicates if it was checked?
 
> +		if (flags & IEEE80211_HW_TKIP_REQ_PHASE1_KEY) {
...
> +		} else if (flags & IEEE80211_HW_TKIP_REQ_PHASE2_KEY) {
...

if you change the order of these tests then setting both flags will be
fine.

johannes