Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Thu, 2006-11-16 at 09:40 -0800, Jouni Malinen wrote: > It's been awhile since I working on this part, but if I remember > correctly, this can be done easily by the driver behaving different > based on the frame being fragmented or not (it is fragmented if MoreFrag > flag is set or seq# != 0). That's the way it was done with hardware > designs that allow full TKIP hwaccel for MSDUs that are not fragmented, > but require software implementation of Michael MIC if MSDU is > fragmented. Oh right, I forgot that the driver can just look into the 802.11 header, was checking tx_control only :) johannes signature.asc Description: This is a digitally signed message part
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Thu, Nov 16, 2006 at 06:38:20PM +0100, Johannes Berg wrote: > Oh, good point. Will need to think about this more. But right now, if > that MMIC flag is set (i.e. hw does MMIC), we still do MMIC for > fragmented frames. And there's actually a bug here I think: if hw does > MMIC, how can the driver tell not to do it on those frames it's getting > as fragments? It's been awhile since I working on this part, but if I remember correctly, this can be done easily by the driver behaving different based on the frame being fragmented or not (it is fragmented if MoreFrag flag is set or seq# != 0). That's the way it was done with hardware designs that allow full TKIP hwaccel for MSDUs that are not fragmented, but require software implementation of Michael MIC if MSDU is fragmented. -- Jouni MalinenPGP id EFC895FA - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Thu, 2006-11-16 at 09:21 -0800, Jouni Malinen wrote: > Hmm.. I think I need to see the patch for doing this before being able > to decide whether I'd like this or not.. Michael MIC is a bit > unfortunate design since it happens at MSDU, not MPDU level. Because of > this, most hardware designs have to do something special when > fragmentation is used. If the fragmentation is done in the stack, it may > be difficult to move the Michael MIC operations into the driver since > they need to happen before fragmentation (which would be done in the > stack in this case).. Oh, good point. Will need to think about this more. But right now, if that MMIC flag is set (i.e. hw does MMIC), we still do MMIC for fragmented frames. And there's actually a bug here I think: if hw does MMIC, how can the driver tell not to do it on those frames it's getting as fragments? > Other than this special case, it sounds reasonable to keep most of the > small hardware design changes hidden in the driver code as long as the > functions that are exported from the stack are the same ones that the > stack would be using anyway (i.e., not having to maintain two versions > of the same code). Of course, for this TKIP stuff I'd only export those functions the drivers will need and remove the flags, for software crypto (in stack) they'd still be used as-is. johannes signature.asc Description: This is a digitally signed message part
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Thu, Nov 16, 2006 at 10:52:42AM +0100, Johannes Berg wrote: > 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). Hmm.. I think I need to see the patch for doing this before being able to decide whether I'd like this or not.. Michael MIC is a bit unfortunate design since it happens at MSDU, not MPDU level. Because of this, most hardware designs have to do something special when fragmentation is used. If the fragmentation is done in the stack, it may be difficult to move the Michael MIC operations into the driver since they need to happen before fragmentation (which would be done in the stack in this case).. Other than this special case, it sounds reasonable to keep most of the small hardware design changes hidden in the driver code as long as the functions that are exported from the stack are the same ones that the stack would be using anyway (i.e., not having to maintain two versions of the same code). -- Jouni MalinenPGP id EFC895FA - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
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
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Tue, 2006-11-14 at 10:22 +0800, Hong Liu wrote: > Resend the patch according to Johannes's comments. Thanks :) > Still put he tkip_key in tx_control structure. Having the tkip_key in there is actually pretty bad for when/if we want to push the tx_control into skb->cb (possibly combined with some header or whatever). I don't think that has been decided on yet, will be a large change anyway and this helps a lot so :) 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 :) This might actually also go some way towards unification with ipw2x00's ieee80211 because those drivers require such functions as well as far as I know. Haven't looked too deeply though. johannes - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
Resend the patch according to Johannes's comments.
Still put he tkip_key in tx_control structure.
Signed-off-by: Hong Liu <[EMAIL PROTECTED]>
diff --git a/include/net/d80211.h b/include/net/d80211.h
index 812f2d1..cf87adc 100644
--- a/include/net/d80211.h
+++ b/include/net/d80211.h
@@ -159,6 +159,7 @@ #define IEEE80211_TXCTL_CLEAR_DST_MASK (
#define IEEE80211_TXCTL_REQUEUE(1<<7)
#define IEEE80211_TXCTL_FIRST_FRAGMENT (1<<8) /* this is a first fragment of
* the frame */
+#define IEEE80211_TXCTL_TKIP_NEW_PHASE1_KEY (1<<9)
u32 flags; /* tx control flags defined
* above */
u16 rts_cts_duration; /* duration field for RTS/CTS frame */
@@ -169,6 +170,7 @@ #define IEEE80211_TXCTL_FIRST_FRAGMENT (
* hw->set_key() */
u8 icv_len; /* length of the ICV/MIC field in octets */
u8 iv_len; /* length of the IV field in octets */
+ u8 tkip_key[16];/* generated phase2/phase1 key for hw TKIP */
u8 queue; /* hardware queue to use for this frame;
* 0 = highest, hw->queues-1 = lowest */
u8 sw_retry_attempt;/* number of times hw has tried to
@@ -487,6 +489,15 @@ #define IEEE80211_HW_MONITOR_DURING_OPER
* i.e. more than one skb per frame */
#define IEEE80211_HW_FRAGLIST (1<<11)
+ /* 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)
+
u32 flags; /* hardware flags defined above */
/* This is the time in us to change channels
diff --git a/net/d80211/tkip.c b/net/d80211/tkip.c
index 7e3665a..fd02449 100644
--- a/net/d80211/tkip.c
+++ b/net/d80211/tkip.c
@@ -190,17 +190,16 @@ u8 * ieee80211_tkip_add_iv(u8 *pos, stru
return pos;
}
-
-/* Encrypt packet payload with TKIP using @key. @pos is a pointer to the
- * beginning of the buffer containing payload. This payload must include
- * headroom of eight octets for IV and Ext. IV and taildroom of four octets
- * for ICV. @payload_len is the length of payload (_not_ including extra
- * headroom and tailroom). @ta is the transmitter addresses. */
-void ieee80211_tkip_encrypt_data(struct crypto_tfm *tfm, struct ieee80211_key
*key,
-u8 *pos, size_t payload_len, u8 *ta)
+void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
+ u16 *phase1key)
{
- u8 rc4key[16];
+ tkip_mixing_phase1(ta, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
+ key->u.tkip.iv32, phase1key);
+}
+void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key, u8 *ta,
+ u8 *rc4key)
+{
/* Calculate per-packet key */
if (key->u.tkip.iv16 == 0 || !key->u.tkip.tx_initialized) {
/* IV16 wrapped around - perform TKIP phase 1 */
@@ -211,7 +210,19 @@ void ieee80211_tkip_encrypt_data(struct
tkip_mixing_phase2(key->u.tkip.p1k, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
key->u.tkip.iv16, rc4key);
+}
+
+/* Encrypt packet payload with TKIP using @key. @pos is a pointer to the
+ * beginning of the buffer containing payload. This payload must include
+ * headroom of eight octets for IV and Ext. IV and taildroom of four octets
+ * for ICV. @payload_len is the length of payload (_not_ including extra
+ * headroom and tailroom). @ta is the transmitter addresses. */
+void ieee80211_tkip_encrypt_data(struct crypto_tfm *tfm, struct ieee80211_key
*key,
+u8 *pos, size_t payload_len, u8 *ta)
+{
+ u8 rc4key[16];
+ ieee80211_tkip_gen_rc4key(key, ta, rc4key);
pos = ieee80211_tkip_add_iv(pos, key, rc4key[0], rc4key[1], rc4key[2]);
ieee80211_wep_encrypt_data(tfm, rc4key, 16, pos, payload_len);
}
diff --git a/net/d80211/tkip.h b/net/d80211/tkip.h
index e36b85c..9b22717 100644
--- a/net/d80211/tkip.h
+++ b/net/d80211/tkip.h
@@ -15,6 +15,10 @@ #include "ieee80211_key.h"
u8 * ieee80211_tkip_add_iv(u8 *pos, struct ieee80211_key *key,
u8 iv0, u8 iv1, u8 iv2);
+void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
+ u16 *phase1key);
+void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key, u8 *ta,
+ u8 *rc4key);
void ieee80211_tkip_encrypt_data(struct crypto_tfm *tfm, struct ieee80211_key
*key,
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
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
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Tue, 2006-10-24 at 17:10, Johannes Berg wrote:
> On Tue, 2006-10-24 at 16:38 +0800, Hong Liu wrote:
>
> > The first time when we set the TKIP key, we can set the phase1 key if
> > the driver requires.
>
> Right.
>
> > The problem is when IV16 wraps, who will generate the new phase1 key?
>
> Ok, I was confused for a moment, but yes, when the 16-bit part of the IV
> wraps then the upper part changes and hence you need to regenerate the
> phase1 key.
>
> > If the stack need to do this, then we will need to call set_key in the
> > packet TX path which I think may not be appropriate.
>
> Well, I checked again and found that bcm34xx requires the IV (48) to be
> maintained by software, iow with each packet you tell the hardware what
> the IV16 should be. Hence you know when it has wrapped and thus (before
> submitting the DMA for the packet) you can regenerate the phase1 key in
> software and upload it to the hardware key memory.
>
> This is getting quite complicated. How about having the stack maintain
> the IV *only*, and adding helper functions for
> * generating the phase 1 key
> * generating the phase 2 key based on the phase 1 key
> that the drivers can use as required?
>
> ipw3945 would call both key generation functions as required, and
> bcm43xx would just call the phase 1 function when a key is uploaded and
> when the iv16 wraps, not using the phase 2 function at all.
>
> johannes
>
>
I'd prefer to let the stack tell the driver when there is new phase1 key
generated.
Rebase the patch on David Kimdon's bitfield removing patches.
Signed-off-by: Hong Liu <[EMAIL PROTECTED]>
diff --git a/include/net/d80211.h b/include/net/d80211.h
index 812f2d1..2570072 100644
--- a/include/net/d80211.h
+++ b/include/net/d80211.h
@@ -159,6 +159,7 @@ #define IEEE80211_TXCTL_CLEAR_DST_MASK (
#define IEEE80211_TXCTL_REQUEUE(1<<7)
#define IEEE80211_TXCTL_FIRST_FRAGMENT (1<<8) /* this is a first fragment of
* the frame */
+#define IEEE80211_TXCTL_TKIP_NEW_PHASE1_KEY (1<<9)
u16 flags; /* tx control flags defined
* above */
u16 rts_cts_duration; /* duration field for RTS/CTS frame */
@@ -169,6 +170,8 @@ #define IEEE80211_TXCTL_FIRST_FRAGMENT (
* hw->set_key() */
u8 icv_len; /* length of the ICV/MIC field in octets */
u8 iv_len; /* length of the IV field in octets */
+ u8 tkip_keylen;
+ u8 tkip_key[16];/* generated RC4/phase1 key for hw TKIP */
u8 queue; /* hardware queue to use for this frame;
* 0 = highest, hw->queues-1 = lowest */
u8 sw_retry_attempt;/* number of times hw has tried to
@@ -487,6 +490,15 @@ #define IEEE80211_HW_MONITOR_DURING_OPER
* i.e. more than one skb per frame */
#define IEEE80211_HW_FRAGLIST (1<<11)
+ /* 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)
+
u32 flags; /* hardware flags defined above */
/* This is the time in us to change channels
diff --git a/net/d80211/tkip.c b/net/d80211/tkip.c
index 7e3665a..fd02449 100644
--- a/net/d80211/tkip.c
+++ b/net/d80211/tkip.c
@@ -190,17 +190,16 @@ u8 * ieee80211_tkip_add_iv(u8 *pos, stru
return pos;
}
-
-/* Encrypt packet payload with TKIP using @key. @pos is a pointer to the
- * beginning of the buffer containing payload. This payload must include
- * headroom of eight octets for IV and Ext. IV and taildroom of four octets
- * for ICV. @payload_len is the length of payload (_not_ including extra
- * headroom and tailroom). @ta is the transmitter addresses. */
-void ieee80211_tkip_encrypt_data(struct crypto_tfm *tfm, struct ieee80211_key
*key,
-u8 *pos, size_t payload_len, u8 *ta)
+void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
+ u16 *phase1key)
{
- u8 rc4key[16];
+ tkip_mixing_phase1(ta, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
+ key->u.tkip.iv32, phase1key);
+}
+void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key, u8 *ta,
+ u8 *rc4key)
+{
/* Calculate per-packet key */
if (key->u.tkip.iv16 == 0 || !key->u.tkip.tx_initialized) {
/* IV16 wrapped around - perform TKIP phase 1 */
@@ -211,7 +210,19 @@ void ieee80211_tkip_encrypt_data(struct
tkip_
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Tue, 2006-10-24 at 16:38 +0800, Hong Liu wrote: > The first time when we set the TKIP key, we can set the phase1 key if > the driver requires. Right. > The problem is when IV16 wraps, who will generate the new phase1 key? Ok, I was confused for a moment, but yes, when the 16-bit part of the IV wraps then the upper part changes and hence you need to regenerate the phase1 key. > If the stack need to do this, then we will need to call set_key in the > packet TX path which I think may not be appropriate. Well, I checked again and found that bcm34xx requires the IV (48) to be maintained by software, iow with each packet you tell the hardware what the IV16 should be. Hence you know when it has wrapped and thus (before submitting the DMA for the packet) you can regenerate the phase1 key in software and upload it to the hardware key memory. This is getting quite complicated. How about having the stack maintain the IV *only*, and adding helper functions for * generating the phase 1 key * generating the phase 2 key based on the phase 1 key that the drivers can use as required? ipw3945 would call both key generation functions as required, and bcm43xx would just call the phase 1 function when a key is uploaded and when the iv16 wraps, not using the phase 2 function at all. johannes - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Tue, 2006-10-24 at 11:10 +0200, Johannes Berg wrote: > How about having the stack maintain > the IV *only*, The idea here is that even if not required by hw that does both, it's cheap to maintain this counter. Though for all I care the driver could do it too. johannes - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Tue, 2006-10-24 at 16:35, Johannes Berg wrote: > On Tue, 2006-10-24 at 16:20 +0800, Hong Liu wrote: > > > It's really strange bcm43xx only does phase2 mixing in hw. With all the > > code that does phase2 mixing, doing phase1 mixing is very cheap. > > Yeah, well, they have the tkip sbox in hw. > > > With Jiri's ieee80211_tx_ctrl_common patch, I think we can put the > > rc4key or phase1 key in ieee80211_tx_control with each TX packet. > > No, the phase1 key is not dependent on the packet so it should not be > there. It should be set via the regular key setting mechanism, instead > of the TKIP key the driver would be given the phase1 key if required. > > johannes The first time when we set the TKIP key, we can set the phase1 key if the driver requires. The problem is when IV16 wraps, who will generate the new phase1 key? If the stack need to do this, then we will need to call set_key in the packet TX path which I think may not be appropriate. Thanks, Hong - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Tue, 2006-10-24 at 16:20 +0800, Hong Liu wrote: > It's really strange bcm43xx only does phase2 mixing in hw. With all the > code that does phase2 mixing, doing phase1 mixing is very cheap. Yeah, well, they have the tkip sbox in hw. > With Jiri's ieee80211_tx_ctrl_common patch, I think we can put the > rc4key or phase1 key in ieee80211_tx_control with each TX packet. No, the phase1 key is not dependent on the packet so it should not be there. It should be set via the regular key setting mechanism, instead of the TKIP key the driver would be given the phase1 key if required. johannes - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Mon, 2006-10-23 at 20:56, Jiri Benc wrote: > On Mon, 23 Oct 2006 14:48:00 +0200, Johannes Berg wrote: > > On Mon, 2006-10-23 at 14:40 +0200, Jiri Benc wrote: > > > I don't like extending ieee80211_tx_control by 16 more bytes. The > > > driver is required to store a copy of each ieee80211_tx_control > > > (because it's copied to ieee80211_tx_status). I don't have a better > > > idea, though. Anybody? > > > > A pointer that goes somewhere else? I suppose it could even be in the > > skb's cb field. > > Yes, that could work. Thanks! > > > Also, something I just came to think of, bcm43xx does phase2 mixing in > > hw and requires phase1 in software. Do we handle that with or without > > this patch? If not, it'd be nice to fix it up together. > > It's not supported now. And it's really a good idea to extend the patch > to support the bcm43xx case as well. > > Jiri It's really strange bcm43xx only does phase2 mixing in hw. With all the code that does phase2 mixing, doing phase1 mixing is very cheap. With Jiri's ieee80211_tx_ctrl_common patch, I think we can put the rc4key or phase1 key in ieee80211_tx_control with each TX packet. Thanks, Hong - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Mon, 23 Oct 2006 08:29:31 -0700, David Kimdon wrote:
> We could be more selective about what the driver is required to return
> in ieee80211_tx_status, the rc4key isn't particularily interesting to
> ieee80211_tx_status(). I expect there are other uninteresting fields
> (tx_rate, rts_cts_rate, come to mind, for example). We could put the
> fields we are interested in directly in ieee80211_tx_status, or have a
> new structure rather than re-using ieee80211_tx_control inside
> ieee80211_tx_status.
Something like this?
Jiri
---
drivers/net/wireless/d80211/adm8211/adm8211.c |7 +
include/net/d80211.h | 28 ---
net/d80211/ieee80211.c| 98 +-
net/d80211/ieee80211_scan.c |4 -
net/d80211/ieee80211_sta.c|4 -
5 files changed, 73 insertions(+), 68 deletions(-)
--- dscape.orig/drivers/net/wireless/d80211/adm8211/adm8211.c
+++ dscape/drivers/net/wireless/d80211/adm8211/adm8211.c
@@ -439,7 +439,7 @@ static void adm8211_interrupt_tci(struct
pci_unmap_single(priv->pdev, priv->tx_buffers[entry].mapping,
priv->tx_buffers[entry].skb->len,
PCI_DMA_TODEVICE);
- if ((priv->tx_buffers[entry].tx_status.control.flags &
+ if ((priv->tx_buffers[entry].tx_status.common.flags &
IEEE80211_TXCTL_REQ_TX_STATUS) ||
!is_multicast_ether_addr(ieee80211_get_DA(&priv->tx_buffers[entry].hdr))) {
struct ieee80211_hdr *hdr;
@@ -1780,7 +1780,8 @@ static void adm8211_tx_raw(struct net_de
priv->tx_buffers[entry].skb = skb;
priv->tx_buffers[entry].mapping = mapping;
- memcpy(&priv->tx_buffers[entry].tx_status.control, control,
sizeof(*control));
+ memcpy(&priv->tx_buffers[entry].tx_status.common, &control->common,
+ sizeof(control->common));
memcpy(&priv->tx_buffers[entry].hdr, hdr, sizeof(*hdr));
priv->tx_ring[entry].buffer1 = cpu_to_le32(mapping);
@@ -1862,7 +1863,7 @@ static int adm8211_tx(struct net_device
if (short_preamble)
txhdr->header_control |=
cpu_to_le16(ADM8211_TXHDRCTL_SHORT_PREAMBLE);
- if (control->flags & IEEE80211_TXCTL_USE_RTS_CTS)
+ if (control->common.flags & IEEE80211_TXCTL_USE_RTS_CTS)
txhdr->header_control |=
cpu_to_le16(ADM8211_TXHDRCTL_ENABLE_RTS);
if (fc & IEEE80211_FCTL_PROTECTED)
--- dscape.orig/include/net/d80211.h
+++ dscape/include/net/d80211.h
@@ -134,13 +134,7 @@ struct ieee80211_low_level_stats {
* the hardware to use given values (depending on what is supported). */
#define HW_KEY_IDX_INVALID -1
-struct ieee80211_tx_control {
- enum { PKT_NORMAL = 0, PKT_PROBE_RESP } pkt_type;
- int tx_rate; /* Transmit rate, given as the hw specific value for the
- * rate (from struct ieee80211_rate) */
- int rts_cts_rate; /* Transmit rate for RTS/CTS frame, given as the hw
- * specific value for the rate (from
- * struct ieee80211_rate) */
+struct ieee80211_tx_ctrl_common {
#define IEEE80211_TXCTL_REQ_TX_STATUS (1<<0)/* request TX status callback for
* this frame */
@@ -161,6 +155,20 @@ struct ieee80211_tx_control {
* the frame */
u16 flags; /* tx control flags defined
* above */
+ enum { PKT_NORMAL = 0, PKT_PROBE_RESP } pkt_type;
+ u8 queue; /* hardware queue to use for this frame;
+* 0 = highest, hw->queues-1 = lowest */
+ int type; /* internal */
+ int ifindex;/* internal */
+};
+
+struct ieee80211_tx_control {
+ struct ieee80211_tx_ctrl_common common;
+ int tx_rate; /* Transmit rate, given as the hw specific value for the
+ * rate (from struct ieee80211_rate) */
+ int rts_cts_rate; /* Transmit rate for RTS/CTS frame, given as the hw
+ * specific value for the rate (from
+ * struct ieee80211_rate) */
u16 rts_cts_duration; /* duration field for RTS/CTS frame */
u8 retry_limit; /* 1 = only first attempt, 2 = one retry, .. */
u8 power_level; /* per-packet transmit power level, in dBm */
@@ -169,8 +177,6 @@ struct ieee80211_tx_control {
* hw->set_key() */
u8 icv_len; /* length of the ICV/MIC field in octets */
u8 iv_len; /* length of the IV field in octets */
- u8 queue; /* hardware queue to use for this frame;
-* 0 = highest, hw->queues-1 = lowest */
u8 sw_retry_attempt;/* number of times hw has tried to
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Mon, Oct 23, 2006 at 02:40:28PM +0200, Jiri Benc wrote:
> On Fri, 20 Oct 2006 17:19:36 +0800, Hong Liu wrote:
> > --- a/include/net/d80211.h
> > +++ b/include/net/d80211.h
> > @@ -176,6 +176,7 @@ struct ieee80211_tx_control {
> > */
> > int icv_len:8; /* Length of the ICV/MIC field in octets */
> > int iv_len:8; /* Length of the IV field in octets */
> > + u8 rc4key[16]; /* generated RC4 key for hw TKIP */
>
> I don't like extending ieee80211_tx_control by 16 more bytes. The
> driver is required to store a copy of each ieee80211_tx_control
> (because it's copied to ieee80211_tx_status). I don't have a better
> idea, though. Anybody?
We could be more selective about what the driver is required to return
in ieee80211_tx_status, the rc4key isn't particularily interesting to
ieee80211_tx_status(). I expect there are other uninteresting fields
(tx_rate, rts_cts_rate, come to mind, for example). We could put the
fields we are interested in directly in ieee80211_tx_status, or have a
new structure rather than re-using ieee80211_tx_control inside
ieee80211_tx_status.
-David
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Mon, Oct 23, 2006 at 02:40:28PM +0200, Jiri Benc wrote: > > int icv_len:8; /* Length of the ICV/MIC field in octets */ > > int iv_len:8; /* Length of the IV field in octets */ > > + u8 rc4key[16]; /* generated RC4 key for hw TKIP */ > > I don't like extending ieee80211_tx_control by 16 more bytes. The > driver is required to store a copy of each ieee80211_tx_control > (because it's copied to ieee80211_tx_status). I don't have a better > idea, though. Anybody? If the key isn't passed directly in via the tx_control structure, there needs to be a way for the driver to be able to get the key for that packet. Many chipsets won't care, but many expect the encryption key to be passed in with each packet. (For WEP, TKIP, or AES-CCMP) Instead of expanding tx_control, adding an API call might suffice. with TKIP all you need to do the RC4 mixing is already in the packet (macaddr, EIV (keynum+PN), and payload. The stack just needs to be able to look up the PTK given the macaddr+keyidx, and the chipset driver can then derive whatever key it needs and stuff the hardware with it. Crypto is the main way where wireless chipsets make life difficult for a common stack.. - Solomon -- Solomon Peachy pizza at shaftnet dot org Melbourne, FL ^^ (mail/jabber/gtalk) ^^ Quidquid latine dictum sit, altum viditur. ICQ: 1318344 pgp3YPhdswmRf.pgp Description: PGP signature
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Mon, 23 Oct 2006 14:48:00 +0200, Johannes Berg wrote: > On Mon, 2006-10-23 at 14:40 +0200, Jiri Benc wrote: > > I don't like extending ieee80211_tx_control by 16 more bytes. The > > driver is required to store a copy of each ieee80211_tx_control > > (because it's copied to ieee80211_tx_status). I don't have a better > > idea, though. Anybody? > > A pointer that goes somewhere else? I suppose it could even be in the > skb's cb field. Yes, that could work. Thanks! > Also, something I just came to think of, bcm43xx does phase2 mixing in > hw and requires phase1 in software. Do we handle that with or without > this patch? If not, it'd be nice to fix it up together. It's not supported now. And it's really a good idea to extend the patch to support the bcm43xx case as well. Jiri -- Jiri Benc SUSE Labs - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Mon, 2006-10-23 at 14:40 +0200, Jiri Benc wrote: > I don't like extending ieee80211_tx_control by 16 more bytes. The > driver is required to store a copy of each ieee80211_tx_control > (because it's copied to ieee80211_tx_status). I don't have a better > idea, though. Anybody? A pointer that goes somewhere else? I suppose it could even be in the skb's cb field. > Please write more descriptive comments (e.g. there should be stated > that tkip_include_mmic is relevant only when using hw crypto). Also, something I just came to think of, bcm43xx does phase2 mixing in hw and requires phase1 in software. Do we handle that with or without this patch? If not, it'd be nice to fix it up together. johannes - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Fri, 20 Oct 2006 17:19:36 +0800, Hong Liu wrote:
> --- a/include/net/d80211.h
> +++ b/include/net/d80211.h
> @@ -176,6 +176,7 @@ struct ieee80211_tx_control {
> */
> int icv_len:8; /* Length of the ICV/MIC field in octets */
> int iv_len:8; /* Length of the IV field in octets */
> + u8 rc4key[16]; /* generated RC4 key for hw TKIP */
I don't like extending ieee80211_tx_control by 16 more bytes. The
driver is required to store a copy of each ieee80211_tx_control
(because it's copied to ieee80211_tx_status). I don't have a better
idea, though. Anybody?
> @@ -476,6 +477,12 @@ struct ieee80211_hw {
> /* Force software encryption for TKIP packets if WMM is enabled. */
> unsigned int no_tkip_wmm_hwaccel:1;
>
> + /* Do TKIP key mixing in stack, send the generated RC4 key with
> + * with each TX frame */
> + unsigned int tkip_include_rc4key:1;
> + /* calculate michael MIC in stack */
> + unsigned int tkip_include_mmic:1;
Please write more descriptive comments (e.g. there should be stated
that tkip_include_mmic is relevant only when using hw crypto).
Also, it would help if you don't use bitfileds and rebase your patch
on top of http://www.spinics.net/lists/netdev/msg17316.html and
http://www.spinics.net/lists/netdev/msg17314.html; but I can do it for
you when applying the patch.
> +void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key,
> +u8 *rc4key, u8* ta)
Put the destination buffer (rc4key) as the last parameter.
Thanks,
Jiri
--
Jiri Benc
SUSE Labs
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2]d80211: hardware TKIP support for ipw3945
On Fri, 20 Oct 2006 17:19:36 +0800, Hong Liu wrote: > ipw3945 TKIP hwcrypto only support RC4 encryption, > so the stack needs to pre compute the michael MIC and the RC4key > for it. I suppose the hw doesn't do any hardware mic for the RX path. It believe it should be also added. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/2]d80211: hardware TKIP support for ipw3945
ipw3945 TKIP hwcrypto only support RC4 encryption,
so the stack needs to pre compute the michael MIC and the RC4key
for it.
Signed-off-by: Hong Liu <[EMAIL PROTECTED]>
---
include/net/d80211.h |7 +++
net/d80211/tkip.c| 25 +++--
net/d80211/tkip.h|2 ++
net/d80211/wpa.c | 10 +-
4 files changed, 33 insertions(+), 11 deletions(-)
a759f6251a0a5c7a8529c2ebd579acd6f0183cb4
diff --git a/include/net/d80211.h b/include/net/d80211.h
index a80f48b..48e1e01 100644
--- a/include/net/d80211.h
+++ b/include/net/d80211.h
@@ -176,6 +176,7 @@ struct ieee80211_tx_control {
*/
int icv_len:8; /* Length of the ICV/MIC field in octets */
int iv_len:8; /* Length of the IV field in octets */
+ u8 rc4key[16]; /* generated RC4 key for hw TKIP */
unsigned int queue:4; /* hardware queue to use for this frame;
* 0 = highest, hw->queues-1 = lowest */
unsigned int sw_retry_attempt:4; /* no. of times hw has tried to
@@ -476,6 +477,12 @@ struct ieee80211_hw {
/* Force software encryption for TKIP packets if WMM is enabled. */
unsigned int no_tkip_wmm_hwaccel:1;
+ /* Do TKIP key mixing in stack, send the generated RC4 key with
+* with each TX frame */
+ unsigned int tkip_include_rc4key:1;
+ /* calculate michael MIC in stack */
+ unsigned int tkip_include_mmic:1;
+
/* set if the payload needs to be padded at even boundaries after the
* header */
unsigned int extra_hdr_room:1;
diff --git a/net/d80211/tkip.c b/net/d80211/tkip.c
index 7e3665a..c031814 100644
--- a/net/d80211/tkip.c
+++ b/net/d80211/tkip.c
@@ -190,17 +190,9 @@ u8 * ieee80211_tkip_add_iv(u8 *pos, stru
return pos;
}
-
-/* Encrypt packet payload with TKIP using @key. @pos is a pointer to the
- * beginning of the buffer containing payload. This payload must include
- * headroom of eight octets for IV and Ext. IV and taildroom of four octets
- * for ICV. @payload_len is the length of payload (_not_ including extra
- * headroom and tailroom). @ta is the transmitter addresses. */
-void ieee80211_tkip_encrypt_data(struct crypto_tfm *tfm, struct ieee80211_key
*key,
-u8 *pos, size_t payload_len, u8 *ta)
+void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key,
+ u8 *rc4key, u8* ta)
{
- u8 rc4key[16];
-
/* Calculate per-packet key */
if (key->u.tkip.iv16 == 0 || !key->u.tkip.tx_initialized) {
/* IV16 wrapped around - perform TKIP phase 1 */
@@ -212,6 +204,19 @@ void ieee80211_tkip_encrypt_data(struct
tkip_mixing_phase2(key->u.tkip.p1k, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
key->u.tkip.iv16, rc4key);
+}
+
+/* Encrypt packet payload with TKIP using @key. @pos is a pointer to the
+ * beginning of the buffer containing payload. This payload must include
+ * headroom of eight octets for IV and Ext. IV and taildroom of four octets
+ * for ICV. @payload_len is the length of payload (_not_ including extra
+ * headroom and tailroom). @ta is the transmitter addresses. */
+void ieee80211_tkip_encrypt_data(struct crypto_tfm *tfm, struct ieee80211_key
*key,
+u8 *pos, size_t payload_len, u8 *ta)
+{
+ u8 rc4key[16];
+
+ ieee80211_tkip_gen_rc4key(key, rc4key, ta);
pos = ieee80211_tkip_add_iv(pos, key, rc4key[0], rc4key[1], rc4key[2]);
ieee80211_wep_encrypt_data(tfm, rc4key, 16, pos, payload_len);
}
diff --git a/net/d80211/tkip.h b/net/d80211/tkip.h
index e36b85c..407556b 100644
--- a/net/d80211/tkip.h
+++ b/net/d80211/tkip.h
@@ -15,6 +15,8 @@ #include "ieee80211_key.h"
u8 * ieee80211_tkip_add_iv(u8 *pos, struct ieee80211_key *key,
u8 iv0, u8 iv1, u8 iv2);
+void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key,
+ u8 *rc4key, u8* ta);
void ieee80211_tkip_encrypt_data(struct crypto_tfm *tfm, struct ieee80211_key
*key,
u8 *pos, size_t payload_len, u8 *ta);
enum {
diff --git a/net/d80211/wpa.c b/net/d80211/wpa.c
index 31abf9b..5e62464 100644
--- a/net/d80211/wpa.c
+++ b/net/d80211/wpa.c
@@ -104,7 +104,8 @@ #ifdef CONFIG_HOSTAPD_WPA_TESTING
#endif /* CONFIG_HOSTAPD_WPA_TESTING */
if (!tx->key->force_sw_encrypt && !tx->local->conf.sw_decrypt &&
- !tx->fragmented && !wpa_test) {
+ !tx->fragmented && !tx->local->hw->tkip_include_mmic &&
+ !wpa_test) {
/* hwaccel - with no need for preallocated room for Michael MIC
*/
return TXRX_CONTINUE;
@@ -336,6 +337,13 @@ #endif /* CONFIG_HOSTAPD_WPA_TESTING */
0x7f),
(u8) key->u.tkip.iv16);
+ if (tx->local->hw->tkip_include_rc4key) {
+ hdr = (struct
