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

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2006-11-15 at 17:25 +0100, Johannes Berg wrote:

> Instead of putting all this into the stack, however, I think we could
> make those drivers that require it do the bookkeeping and export
> ieee80211_tkip_gen_phase1key and ieee80211_tkip_gen_rc4key for their
> use. Maybe that's too much against Jiri's doctrine of "be easy on
> drivers" though :)

Thought about this a bit more.

I think we should simplify the stack code by making it have *only* the
distinction between between hardware and software crypto (on a per-key
basis).

This would entail getting rid of the flags

  IEEE80211_HW_TKIP_INCLUDE_MMIC
  IEEE80211_HW_TKIP_REQ_PHASE1_KEY
  IEEE80211_HW_TKIP_REQ_PHASE2_KEY

and possibly

  IEEE80211_HW_NO_TKIP_WMM_HWACCEL


Instead, I would like to provide "library" functions to do these tasks
and let drivers use them when they are necessary.

This should simplify and streamline code in the stack by getting rid of
all the tests for these flags and associated code.

My reasoning for this is that
 1) it doesn't break Jiri's doctrine of making things easy for drivers
    because, well, simple drivers just use sw crypto
 2) since drivers will always hard-code these flags it actually makes
    the code more efficient -- no testing of flags required, just
    function calls we'd be making elsewhere after testing flags
 3) instead of drivers hard-coding the flag setting they hardcode
    (yes, in a few more lines) the crypto they need

I think putting the code into the driver vs. the stack is a trade-off we
should make because it makes things more efficient.

Could you live with a patch to do this and adjust ipw3945 accordingly?

For ipw3945 it would be just an addition of a call to
ieee80211_tkip_gen_rc4key() as it lives in the stack after this patch,
and for bcm43xx it would be similar to the code you added within 
"else if (flags & IEEE80211_HW_TKIP_REQ_PHASE1_KEY) {"

As for IEEE80211_HW_NO_TKIP_WMM_HWACCEL, we would have to tell the
hw->set_key routine if WME is enabled for the STA associated with the
key. I'm not yet sure how to do this best, plus the flag isn't actually
checked in the TX path but only the key setting path so...

Basically, I think we have too much knowledge of hardware quirks in the
stack. I'll be submitting a patch for this item later today.

johannes