mac80211 hardware encryption

mac80211 hardware encryption

[Ivo van Doorn]

Hi,

I'm investigating hardware encryption support for the rt2x00 drivers,
Support for rt61pci and rt73usb should be the easiest to do so currently I'm
only focussing on that, and decide later if rt2500pci and rt2500usb should
also be implemented.

What I am currently facing is the following problem:
For each TX frame the device needs 2 fields in the descriptor namely the
IV and the EIV. From what I can gather from mac80211 those 2 fields are
created by mac80211 with the following code in wpa.c:

ieee80211_tkip_add_iv(pos, key,
		      (u8) (key->u.tkip.iv16 >> 8),
		      (u8) (((key->u.tkip.iv16 >> 8) | 0x20) &
			    0x7f),
		      (u8) key->u.tkip.iv16);

And the following function in tkip.c:

u8 * ieee80211_tkip_add_iv(u8 *pos, struct ieee80211_key *key,
			   u8 iv0, u8 iv1, u8 iv2)
{
	*pos++ = iv0;
	*pos++ = iv1;
	*pos++ = iv2;
	*pos++ = (key->conf.keyidx << 6) | (1 << 5) /* Ext IV */;
	*pos++ = key->u.tkip.iv32 & 0xff;
	*pos++ = (key->u.tkip.iv32 >> 8) & 0xff;
	*pos++ = (key->u.tkip.iv32 >> 16) & 0xff;
	*pos++ = (key->u.tkip.iv32 >> 24) & 0xff;
	return pos;
}

If I understand correctly mac80211 is inserting the values just after
the ieee80211 header in the frame. Now there are 2 ways rt2x00 can
handle this.
	1) copy the frame to the DMA in 2 steps, skipping the inserted IV by mac80211
	    and copy the IV into the descriptor.
	2) Add flag to mac80211 to provide the IV and EIV seperately to the driver
	    so there won't be overhead by mac80211 to insert the IV after the ieee80211
	    header when the driver doesn't want it to.

Thanks,

Ivo

Re: mac80211 hardware encryption

[Ivo van Doorn]

Hi,

> I'm investigating hardware encryption support for the rt2x00 drivers,
> Support for rt61pci and rt73usb should be the easiest to do so currently I'm
> only focussing on that, and decide later if rt2500pci and rt2500usb should
> also be implemented.

After some more research I have a similar question for the RX path.
The device will strip the IV out of the ieee80211 frame but provides
the IV/EIV seperately in the descriptor.

Would it be any added value to instead of setting the RX_FLAG_IV_STRIPPED
flag the IV is reinserted into the frame, provide the IV/EIV seperately in
struct ieee80211_rx_status, or simply set RX_FLAG_IV_STRIPPED and ignore
the entire field?

Thanks.

Ivo

Re: mac80211 hardware encryption

[Jouni Malinen]

On Sun, Apr 06, 2008 at 06:44:14PM +0200, Ivo van Doorn wrote:

> After some more research I have a similar question for the RX path.
> The device will strip the IV out of the ieee80211 frame but provides
> the IV/EIV seperately in the descriptor.
> 
> Would it be any added value to instead of setting the RX_FLAG_IV_STRIPPED
> flag the IV is reinserted into the frame, provide the IV/EIV seperately in
> struct ieee80211_rx_status, or simply set RX_FLAG_IV_STRIPPED and ignore
> the entire field?

Yes, I would highly recommend delivering IV/EIV to mac80211 so that
replay protection can be implemented properly. This is required if the
hardware/firmware does not have full implementation of replay protection
(including support for multiple access categories in case QoS is used).

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: mac80211 hardware encryption

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

Hi Ivo,

In addition to what Jouni already mentioned about replay detection, I
have a question.

> If I understand correctly mac80211 is inserting the values just after
> the ieee80211 header in the frame. 

Well, yes, that's where they belong on air.

> Now there are 2 ways rt2x00 can
> handle this.
> 	1) copy the frame to the DMA in 2 steps, skipping the inserted IV by mac80211
> 	    and copy the IV into the descriptor.

How much overhead do you reckon this would be?

> 	2) Add flag to mac80211 to provide the IV and EIV seperately to the driver
> 	    so there won't be overhead by mac80211 to insert the IV after the ieee80211
> 	    header when the driver doesn't want it to.

I'd prefer 1) over 2) just because it seems a rather special case for
this particular hardware.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: mac80211 hardware encryption

[Ivo van Doorn]

Hi,

> > Now there are 2 ways rt2x00 can
> > handle this.
> > 	1) copy the frame to the DMA in 2 steps, skipping the inserted IV by mac80211
> > 	    and copy the IV into the descriptor.
> 
> How much overhead do you reckon this would be?

For rt61pci not much, it simply means 2 memcpy() calls instead of 1.
For rt73usb it is trickier since it passes the skb->data directly to the device,
which would mean a memmove() would be required for the header...

The same goes for the RX path to reinsert the IV back behind the header.

> > 	2) Add flag to mac80211 to provide the IV and EIV seperately to the driver
> > 	    so there won't be overhead by mac80211 to insert the IV after the ieee80211
> > 	    header when the driver doesn't want it to.
> 
> I'd prefer 1) over 2) just because it seems a rather special case for
> this particular hardware.

I agree, but overall I think this would mean for rt2x00 it will be easier to stop
sending the skb->data directly to the USB host and use the preallocated DMA
instead.
Especially the RX path could benefit since otherwise it will be continuously using
memmove on the header and payload to insert the IV and have the payload 4-byte aligned.

P.S. rt2500pci and rt2500usb have the same IV/EIV behavior as rt61pci and rt73usb.
The upcoming rt2800pci/rt2800usb drivers are different and generate the IV/EIV in the hardware.

Ivo

Re: mac80211 hardware encryption

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

Hi,

For the TX path, I don't think a memmove() of the header further down is
really expensive, it should be within the CPU cache since we operate on
the skb header a lot. However, you'd have to undo that too before TX
status reporting to get radiotap working properly :/

> I agree, but overall I think this would mean for rt2x00 it will be easier to stop
> sending the skb->data directly to the USB host and use the preallocated DMA
> instead.
> Especially the RX path could benefit since otherwise it will be continuously using
> memmove on the header and payload to insert the IV and have the payload 4-byte aligned.

How would you go about handling out-of-band IV in the RX path? In the TX
path, I can see how you could do that, but in RX?

In any case, if you can make usable patches for out-of-band IV I'm not
totally against it, but I'd like to keep the overhead as low as
possible. In particular, I was thinking of embedding tx_conf into
skb->cb[] so you wouldn't have space in there for the IV.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: mac80211 hardware encryption

[Ivo van Doorn]

Hi,

> For the TX path, I don't think a memmove() of the header further down=
 is
> really expensive, it should be within the CPU cache since we operate =
on
> the skb header a lot. However, you'd have to undo that too before TX
> status reporting to get radiotap working properly :/
>=20
> > I agree, but overall I think this would mean for rt2x00 it will be =
easier to stop
> > sending the skb->data directly to the USB host and use the prealloc=
ated DMA
> > instead.
> > Especially the RX path could benefit since otherwise it will be con=
tinuously using
> > memmove on the header and payload to insert the IV and have the pay=
load 4-byte aligned.
>=20
> How would you go about handling out-of-band IV in the RX path? In the=
 TX
> path, I can see how you could do that, but in RX?

I'm not sure if I understand completely what you mean but rt61pci/rt73u=
sb hardware does the following:

1) Receive frame
2) Determine key from register
3) Put IV/EIV into descriptor
4) Decrypt
5) Notifies driver about the frame +  decryption status

So the IV/EIV is only provided for information about the description. I=
t will require some testing
to see what happened to the IV/EIV when decryption failed...

Side-information:
rt2500pci and rt2500usb handle this differently because they require th=
e driver to set
the key before a frame can be decrypted.

> In any case, if you can make usable patches for out-of-band IV I'm no=
t
> totally against it, but I'd like to keep the overhead as low as
> possible. In particular, I was thinking of embedding tx_conf into
> skb->cb[] so you wouldn't have space in there for the IV.

Well as an alternative to adding it to tx_control, perhaps a callback f=
unction for drivers
could be provided? At the moment mac80211 calls:

ieee80211_tkip_add_iv(pos, key,
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0 =A0(u8) (key->=
u.tkip.iv16 >> 8),
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0 =A0(u8) (((key=
->u.tkip.iv16 >> 8) | 0x20) &
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
 =A0 =A00x7f),
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 =A0 =A0(u8) key->u=
=2Etkip.iv16);

to insert the IV into the skb, if the driver could get a calback functi=
on that calls the above
function and writes the result into a char* buffer you get the same eff=
ect except that the
driver can put the iv wherever it wants.
That way rt2x00 doesn't have to set the IEEE80211_KEY_FLAG_GENERATE_IV =
flag, and
can request the IV manually from mac80211.

Ivo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mac80211 hardware encryption

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1313 bytes --]


> I'm not sure if I understand completely what you mean but rt61pci/rt73usb hardware does the following:
> 
> 1) Receive frame
> 2) Determine key from register
> 3) Put IV/EIV into descriptor
> 4) Decrypt
> 5) Notifies driver about the frame +  decryption status

Ok, so you get those out-of-band.

> Well as an alternative to adding it to tx_control, perhaps a callback function for drivers
> could be provided? At the moment mac80211 calls:
> 
> ieee80211_tkip_add_iv(pos, key,
>                       (u8) (key->u.tkip.iv16 >> 8),
>                       (u8) (((key->u.tkip.iv16 >> 8) | 0x20) &
>                             0x7f),
>                       (u8) key->u.tkip.iv16);
> 
> to insert the IV into the skb, if the driver could get a calback function that calls the above
> function and writes the result into a char* buffer you get the same effect except that the
> driver can put the iv wherever it wants.
> That way rt2x00 doesn't have to set the IEEE80211_KEY_FLAG_GENERATE_IV flag, and
> can request the IV manually from mac80211.

No, you can't do that because the IV is already incremented at that
point, so you wouldn't be able to perfectly match things up because
ops->tx() need not be serialised with this due to deferring to the
master interface.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: mac80211 hardware encryption

[Ivo van Doorn]

On Monday 07 April 2008, Johannes Berg wrote:
> 
> > I'm not sure if I understand completely what you mean but rt61pci/rt73usb hardware does the following:
> > 
> > 1) Receive frame
> > 2) Determine key from register
> > 3) Put IV/EIV into descriptor
> > 4) Decrypt
> > 5) Notifies driver about the frame +  decryption status
> 
> Ok, so you get those out-of-band.
> 
> > Well as an alternative to adding it to tx_control, perhaps a callback function for drivers
> > could be provided? At the moment mac80211 calls:
> > 
> > ieee80211_tkip_add_iv(pos, key,
> >                       (u8) (key->u.tkip.iv16 >> 8),
> >                       (u8) (((key->u.tkip.iv16 >> 8) | 0x20) &
> >                             0x7f),
> >                       (u8) key->u.tkip.iv16);
> > 
> > to insert the IV into the skb, if the driver could get a calback function that calls the above
> > function and writes the result into a char* buffer you get the same effect except that the
> > driver can put the iv wherever it wants.
> > That way rt2x00 doesn't have to set the IEEE80211_KEY_FLAG_GENERATE_IV flag, and
> > can request the IV manually from mac80211.
> 
> No, you can't do that because the IV is already incremented at that
> point, so you wouldn't be able to perfectly match things up because
> ops->tx() need not be serialised with this due to deferring to the
> master interface.

Ok, so just to summarize (so I have everything right :) )

TX path)
rt2x00 sets the IEEE80211_KEY_FLAG_GENERATE_IV flag
grabs the IV/EIV from behind the ieee80211 header
memmove ieee80211 header to remove the IV/EIV

TX done path)
memmove ieee80211 header to make room for IV/EIV
reinsert IV/EIV behind ieee80211 header
call ieee80211_tx_status() for mac80211

RX path)
memmove ieee80211 header to make room for IV/EIV
insert IV/EIV behind ieee80211 header
call ieee80211_rx() for mac80211

Thanks,

Ivo

Re: mac80211 hardware encryption

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]


> Ok, so just to summarize (so I have everything right :) )
> 
> TX path)
> rt2x00 sets the IEEE80211_KEY_FLAG_GENERATE_IV flag
> grabs the IV/EIV from behind the ieee80211 header
> memmove ieee80211 header to remove the IV/EIV
> 
> TX done path)
> memmove ieee80211 header to make room for IV/EIV
> reinsert IV/EIV behind ieee80211 header
> call ieee80211_tx_status() for mac80211
> 
> RX path)
> memmove ieee80211 header to make room for IV/EIV
> insert IV/EIV behind ieee80211 header
> call ieee80211_rx() for mac80211

Yes, that looks right.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: mac80211 hardware encryption

[Ivo van Doorn]

On Monday 07 April 2008, Johannes Berg wrote:
> 
> > Ok, so just to summarize (so I have everything right :) )
> > 
> > TX path)
> > rt2x00 sets the IEEE80211_KEY_FLAG_GENERATE_IV flag
> > grabs the IV/EIV from behind the ieee80211 header
> > memmove ieee80211 header to remove the IV/EIV
> > 
> > TX done path)
> > memmove ieee80211 header to make room for IV/EIV
> > reinsert IV/EIV behind ieee80211 header
> > call ieee80211_tx_status() for mac80211
> > 
> > RX path)
> > memmove ieee80211 header to make room for IV/EIV
> > insert IV/EIV behind ieee80211 header
> > call ieee80211_rx() for mac80211
> 
> Yes, that looks right.

Excellent, thanks. :)

Ivo

Re: mac80211 hardware encryption

[Ivo van Doorn]

Hi,

I've implemented the steps for hardware encryption
including the part where the IV/EIV is being moved around.

I now have a different problem, the hw_key_idx is u8,
so is very limited to what can be stored in it. I am trying
to find a solution for the following problem without increasing
the size of hw_key_idx since it is also used in the ieee80211_tx_control
structure which should remain as small as possible to fix in skb->cb.

The problem is as follows:
rt61pci, rt73usb, rt2800pci and rt2800usb support both shared keys
as well as pairwise keys. When the "address" argument of set_key()
is a valid MAC address, then the key is considered to be pairwise,
otherwise it is a shared key.

rt61pci and rt73usb supports:
shared keys: 16 (4 per allowed virtual interface)
pairwise keys: 64 (16 per allowed virtual interface)

rt2800pci and rt2800usb supports:
shared keys: 32 (4 per allowed virtual interface)
pairwise keys: 256 (32 per allowed virtual interface)

This means that the hw_key_idx with rt2800 hardware is already
full when all keys are being supported. I am thinking of adding a flag
to the key structure and ieee80211_tx_control structure to indicate
if the key is shared or pairwise. This will safe 1 bit that otherwise has
to be reserved from the hw_key_idx.
Do you agree with such a flag addition, or should the driver not care
about pairwise vs. shared.

But now the fun part begins, the hardware needs the ieee80211_key_alg value
for encryption (don't ask why, the algorithm is also stored in the register
so it should be easy for the hardware to look it up directly).
What should be the best way to obtain this algorithm, should a callback
function be added, or should the driver keep a list in memory for all
added keys (which would be memory duplication since mac80211 also
has it in memory).

Thanks,

Ivo

Re: mac80211 hardware encryption

[Tomas Winkler]

On Mon, Apr 14, 2008 at 7:27 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> Hi,
>
> I've implemented the steps for hardware encryption
> including the part where the IV/EIV is being moved around.
>
> I now have a different problem, the hw_key_idx is u8,
> so is very limited to what can be stored in it. I am trying
> to find a solution for the following problem without increasing
> the size of hw_key_idx since it is also used in the ieee80211_tx_control
> structure which should remain as small as possible to fix in skb->cb.
>
> The problem is as follows:
> rt61pci, rt73usb, rt2800pci and rt2800usb support both shared keys
> as well as pairwise keys. When the "address" argument of set_key()
> is a valid MAC address, then the key is considered to be pairwise,
> otherwise it is a shared key.
>
> rt61pci and rt73usb supports:
> shared keys: 16 (4 per allowed virtual interface)
> pairwise keys: 64 (16 per allowed virtual interface)
>
> rt2800pci and rt2800usb supports:
> shared keys: 32 (4 per allowed virtual interface)
> pairwise keys: 256 (32 per allowed virtual interface)
>
> This means that the hw_key_idx with rt2800 hardware is already
> full when all keys are being supported. I am thinking of adding a flag
> to the key structure and ieee80211_tx_control structure to indicate
> if the key is shared or pairwise. This will safe 1 bit that otherwise has
> to be reserved from the hw_key_idx.
> Do you agree with such a flag addition, or should the driver not care
> about pairwise vs. shared.
>
> But now the fun part begins, the hardware needs the ieee80211_key_alg value
> for encryption (don't ask why, the algorithm is also stored in the register
> so it should be easy for the hardware to look it up directly).
> What should be the best way to obtain this algorithm, should a callback
> function be added, or should the driver keep a list in memory for all
> added keys (which would be memory duplication since mac80211 also
> has it in memory).
>
> Thanks,
>
>
Not sure what was intention for hw_key_idx but currently there are assignment
from key->conf.keyidx to hw_key_idx and visa versa.  keyidx is used
for 802.11 key index [1..4]
So it's quit risky to use it for something else
>
>
> Ivo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: mac80211 hardware encryption

[Ivo van Doorn]

On Monday 14 April 2008, Tomas Winkler wrote:
> On Mon, Apr 14, 2008 at 7:27 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> > Hi,
> >
> > I've implemented the steps for hardware encryption
> > including the part where the IV/EIV is being moved around.
> >
> > I now have a different problem, the hw_key_idx is u8,
> > so is very limited to what can be stored in it. I am trying
> > to find a solution for the following problem without increasing
> > the size of hw_key_idx since it is also used in the ieee80211_tx_control
> > structure which should remain as small as possible to fix in skb->cb.
> >
> > The problem is as follows:
> > rt61pci, rt73usb, rt2800pci and rt2800usb support both shared keys
> > as well as pairwise keys. When the "address" argument of set_key()
> > is a valid MAC address, then the key is considered to be pairwise,
> > otherwise it is a shared key.
> >
> > rt61pci and rt73usb supports:
> > shared keys: 16 (4 per allowed virtual interface)
> > pairwise keys: 64 (16 per allowed virtual interface)
> >
> > rt2800pci and rt2800usb supports:
> > shared keys: 32 (4 per allowed virtual interface)
> > pairwise keys: 256 (32 per allowed virtual interface)
> >
> > This means that the hw_key_idx with rt2800 hardware is already
> > full when all keys are being supported. I am thinking of adding a flag
> > to the key structure and ieee80211_tx_control structure to indicate
> > if the key is shared or pairwise. This will safe 1 bit that otherwise has
> > to be reserved from the hw_key_idx.
> > Do you agree with such a flag addition, or should the driver not care
> > about pairwise vs. shared.
> >
> > But now the fun part begins, the hardware needs the ieee80211_key_alg value
> > for encryption (don't ask why, the algorithm is also stored in the register
> > so it should be easy for the hardware to look it up directly).
> > What should be the best way to obtain this algorithm, should a callback
> > function be added, or should the driver keep a list in memory for all
> > added keys (which would be memory duplication since mac80211 also
> > has it in memory).
> >
> > Thanks,
> >
> >
> Not sure what was intention for hw_key_idx but currently there are assignment
> from key->conf.keyidx to hw_key_idx and visa versa.  keyidx is used
> for 802.11 key index [1..4]
> So it's quit risky to use it for something else

That souds like a bug in mac80211 then. hw_key_idx is for internal use for the driver,
and mac80211 should never expect the driver to set it to a particular value.
Otherwise drivers won't be able to differentiate between keys for particular interfaces,
or pairwise keys, which should be supported since mac80211 passes the address argument
for a reason ;)

Ivo

Re: mac80211 hardware encryption

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 831 bytes --]


> > Not sure what was intention for hw_key_idx but currently there are assignment
> > from key->conf.keyidx to hw_key_idx and visa versa.  keyidx is used
> > for 802.11 key index [1..4]
> > So it's quit risky to use it for something else
> 
> That souds like a bug in mac80211 then. hw_key_idx is for internal use for the driver,
> and mac80211 should never expect the driver to set it to a particular value.

Yeah, that'd be a bug, and I can't find any such assignment. In fact,
mac80211 doesn't contain any assignment to hw_key_idx or to a 1-4
key_idx from hw_key_idx. Never mind the fact that the tx_control member
could be called hw_key_idx as well...

In any case, for the problem at hand, I wouldn't mind increasing
hw_key_idx to a u16 all the way through, or pass the key_conf pointer
instead.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: mac80211 hardware encryption

[Ivo van Doorn]

On Tuesday 15 April 2008, Johannes Berg wrote:
> 
> > > Not sure what was intention for hw_key_idx but currently there are assignment
> > > from key->conf.keyidx to hw_key_idx and visa versa.  keyidx is used
> > > for 802.11 key index [1..4]
> > > So it's quit risky to use it for something else
> > 
> > That souds like a bug in mac80211 then. hw_key_idx is for internal use for the driver,
> > and mac80211 should never expect the driver to set it to a particular value.
> 
> Yeah, that'd be a bug, and I can't find any such assignment. In fact,
> mac80211 doesn't contain any assignment to hw_key_idx or to a 1-4
> key_idx from hw_key_idx. Never mind the fact that the tx_control member
> could be called hw_key_idx as well...
> 
> In any case, for the problem at hand, I wouldn't mind increasing
> hw_key_idx to a u16 all the way through, or pass the key_conf pointer
> instead.

I'll create a patch that changes it to the key_conf pointer then,
that sounds like the safest option to allow changes to key_conf later.

Note that this change (either adding the key_conf pointer or the hw_key_idx to u16)
will cause ieee80211_tx_control to exceed the 48 bytes. So that will make it
harder to move it into the skb->cb array later.

Ivo

Re: mac80211 hardware encryption

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 480 bytes --]


> I'll create a patch that changes it to the key_conf pointer then,
> that sounds like the safest option to allow changes to key_conf later.

Ok.

> Note that this change (either adding the key_conf pointer or the hw_key_idx to u16)
> will cause ieee80211_tx_control to exceed the 48 bytes. So that will make it
> harder to move it into the skb->cb array later.

Yeah, well, tough luck, unless we can convince netdev to increase cb[]
on 64-bit platforms.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: mac80211 hardware encryption

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

On Tue, 2008-04-15 at 17:17 +0200, Ivo van Doorn wrote:

> > In any case, for the problem at hand, I wouldn't mind increasing
> > hw_key_idx to a u16 all the way through, or pass the key_conf pointer
> > instead.
> 
> I'll create a patch that changes it to the key_conf pointer then,
> that sounds like the safest option to allow changes to key_conf later.

That'll also solve your problem of having to know the algorithm, and b43
might benefit of that too, I think it currently keeps a table for the
algorithms.

One thing to keep in mind that you'll want to document clearly: the
key_conf pointer there will only be valid until ops->tx() returns, so
you must not access it (e.g. via the tx control copy you need to make
for tx status) outside of the tx() routine unless you want to somehow
make sure it isn't removed while in use (which is possible [1] since
set_key() may sleep.)

Hence, if you defer ops->tx() to some workqueue or something, you need
to copy out all the data you need from the key_conf beforehand.

Tomas: was it you who mentioned we should remove tx_control from
tx_status and just require copying the fields we need? If so, did you
ever look into that?

> Note that this change (either adding the key_conf pointer or the hw_key_idx to u16)
> will cause ieee80211_tx_control to exceed the 48 bytes. So that will make it
> harder to move it into the skb->cb array later.

I just had a look, we could make the rate pointers index into the rate
array instead which would make them go from 24 bytes (12 on 32-bit) to
3 bytes (we only need a u8 index.)

johannes

[1] using creative bit-locking in hw_key_idx.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: mac80211 hardware encryption

[Ivo van Doorn]

On Thursday 17 April 2008, Johannes Berg wrote:
> On Tue, 2008-04-15 at 17:17 +0200, Ivo van Doorn wrote:
>=20
> > > In any case, for the problem at hand, I wouldn't mind increasing
> > > hw_key_idx to a u16 all the way through, or pass the key_conf poi=
nter
> > > instead.
> >=20
> > I'll create a patch that changes it to the key_conf pointer then,
> > that sounds like the safest option to allow changes to key_conf lat=
er.
>=20
> That'll also solve your problem of having to know the algorithm, and =
b43
> might benefit of that too, I think it currently keeps a table for the
> algorithms.

Exactly :)
A follow up patch that I will create later will add a flag to the key_c=
onf
structure that marks the key as shared or pairwise. After that adding t=
he
final bits for HW encryption to rt61pci and rt73usb should be very easy=
=2E :)

> One thing to keep in mind that you'll want to document clearly: the
> key_conf pointer there will only be valid until ops->tx() returns, so
> you must not access it (e.g. via the tx control copy you need to make
> for tx status) outside of the tx() routine unless you want to somehow
> make sure it isn't removed while in use (which is possible [1] since
> set_key() may sleep.)
>=20
> Hence, if you defer ops->tx() to some workqueue or something, you nee=
d
> to copy out all the data you need from the key_conf beforehand.

Ok, I'll update my patch with the above warning.

> Tomas: was it you who mentioned we should remove tx_control from
> tx_status and just require copying the fields we need? If so, did you
> ever look into that?
>=20
> > Note that this change (either adding the key_conf pointer or the hw=
_key_idx to u16)
> > will cause ieee80211_tx_control to exceed the 48 bytes. So that wil=
l make it
> > harder to move it into the skb->cb array later.
>=20
> I just had a look, we could make the rate pointers index into the rat=
e
> array instead which would make them go from 24=EF=BB=BF bytes (12 on =
32-bit) to
> 3 bytes (we only need a u8 index.)
>=20
> johannes
>=20
> [1] using creative bit-locking in hw_key_idx.

Ivo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mac80211 hardware encryption

[Tomas Winkler]

On Tue, Apr 15, 2008 at 1:35 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>  > > Not sure what was intention for hw_key_idx but currently there are assignment
>  > > from key->conf.keyidx to hw_key_idx and visa versa.  keyidx is used
>  > > for 802.11 key index [1..4]
>  > > So it's quit risky to use it for something else
>  >
>  > That souds like a bug in mac80211 then. hw_key_idx is for internal use for the driver,
>  > and mac80211 should never expect the driver to set it to a particular value.
>
>  Yeah, that'd be a bug, and I can't find any such assignment. In fact,
>  mac80211 doesn't contain any assignment to hw_key_idx or to a 1-4
>  key_idx from hw_key_idx. Never mind the fact that the tx_control member
>  could be called hw_key_idx as well...
>
Sorry my mistake I get confused by a bug from past. It was bug in
iwlwifi not in mac80211. mac80211 looks OK.

Tomas

Re: mac80211 hardware encryption

[Jouni Malinen]

On Mon, Apr 14, 2008 at 09:39:00PM +0300, Tomas Winkler wrote:

> Not sure what was intention for hw_key_idx but currently there are assignment
> from key->conf.keyidx to hw_key_idx and visa versa.  keyidx is used
> for 802.11 key index [1..4]
> So it's quit risky to use it for something else

hw_key_idx was supposed to be a value that the hardware driver assigns
for each key. This allows the driver to use suitable values that map
easily to whatever key table is used in the hardware/firmware. This is
_not_ same as 802.11 key index (0..3) and no code should made such an
assumption. Based on a quick search, I did not find any net/mac80211
code assigning key->conf.keyidx into hw_key_idx (or struct
ieee80211_tx_control::key_idx).

At least as far as the original design is concerned, hw_key_idx is
controlled by the hardware driver and it is only an internal identifier
for a specific hwaccel key.
 
-- 
Jouni Malinen                                            PGP id EFC895FA