2015-06-01 19:38:58

by Liu CF/TW

Subject: [PATCH] mac80211: support IEEE80211_KEY_FLAG_RESERVE_TAILROOM for CCMP and TKIP.

The change reserves neccessary tailroom in CCMP and TKIP if required by
drivers that sets IEEE80211_KEY_FLAG_RESERVE_TAILROOM. For example,
ath10k HW engine in raw Tx/Rx encap mode requires SW reserve MIC/ICV space.

Signed-off-by: David Liu <[email protected]>
---
net/mac80211/wpa.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 9d63d93..b56f31a 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -192,7 +192,9 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)

if (info->control.hw_key &&
!(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
- !(info->control.hw_key->flags & IEEE80211_KEY_FLAG_PUT_IV_SPACE)) {
+ !(info->control.hw_key->flags & IEEE80211_KEY_FLAG_PUT_IV_SPACE) &&
+ !(info->control.hw_key->flags &
+ IEEE80211_KEY_FLAG_RESERVE_TAILROOM)) {
/* hwaccel - with no need for software-generated IV */
return 0;
}
@@ -200,7 +202,8 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
hdrlen = ieee80211_hdrlen(hdr->frame_control);
len = skb->len - hdrlen;

- if (info->control.hw_key)
+ if (info->control.hw_key && !(info->control.hw_key->flags &
+ IEEE80211_KEY_FLAG_RESERVE_TAILROOM))
tail = 0;
else
tail = IEEE80211_TKIP_ICV_LEN;
@@ -227,8 +230,12 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
spin_unlock(&key->u.tkip.txlock);

/* hwaccel - with software IV */
- if (info->control.hw_key)
+ if (info->control.hw_key) {
+ if (info->control.hw_key->flags &
+ IEEE80211_KEY_FLAG_RESERVE_TAILROOM)
+ skb_put(skb, tail);
return 0;
+ }

/* Add room for ICV */
skb_put(skb, IEEE80211_TKIP_ICV_LEN);
@@ -411,6 +418,8 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
if (info->control.hw_key &&
!(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
!(info->control.hw_key->flags & IEEE80211_KEY_FLAG_PUT_IV_SPACE) &&
+ !(info->control.hw_key->flags &
+ IEEE80211_KEY_FLAG_RESERVE_TAILROOM) &&
!((info->control.hw_key->flags &
IEEE80211_KEY_FLAG_GENERATE_IV_MGMT) &&
ieee80211_is_mgmt(hdr->frame_control))) {
@@ -424,7 +433,8 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
hdrlen = ieee80211_hdrlen(hdr->frame_control);
len = skb->len - hdrlen;

- if (info->control.hw_key)
+ if (info->control.hw_key && !(info->control.hw_key->flags &
+ IEEE80211_KEY_FLAG_RESERVE_TAILROOM))
tail = 0;
else
tail = mic_len;
@@ -456,8 +466,12 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
ccmp_pn2hdr(pos, pn, key->conf.keyidx);

/* hwaccel - with software CCMP header */
- if (info->control.hw_key)
+ if (info->control.hw_key) {
+ if (info->control.hw_key->flags &
+ IEEE80211_KEY_FLAG_RESERVE_TAILROOM)
+ skb_put(skb, tail);
return 0;
+ }

pos += IEEE80211_CCMP_HDR_LEN;
ccmp_special_blocks(skb, pn, b_0, aad);
--
2.1.4

2015-06-01 20:34:23

by Johannes Berg

Subject: Re: [PATCH] mac80211: support IEEE80211_KEY_FLAG_RESERVE_TAILROOM for CCMP and TKIP.

On Mon, 2015-06-01 at 12:38 -0700, David Liu wrote:
> The change reserves neccessary tailroom in CCMP and TKIP if required by
> drivers that sets IEEE80211_KEY_FLAG_RESERVE_TAILROOM. For example,
> ath10k HW engine in raw Tx/Rx encap mode requires SW reserve MIC/ICV space.

I don't think this makes all that much sense. The flag is already
honoured today to actually leave enough tailroom in the SKB, and the
remainder of the code can very easily be done in the driver (in fact, if
you have a half-decent DMA engine, it's far more efficient to *not* set
this flag!)

You're also breaking other drivers with your change. Just do it in the
driver.

johannes

2015-06-01 23:48:54

by Liu CF/TW

Subject: Re: [PATCH] mac80211: support IEEE80211_KEY_FLAG_RESERVE_TAILROOM for CCMP and TKIP.

Hi Johannes, yes indeed you are right. This change is indeed not required/right.
It's the special sw crypto path I'm working on in ath10k that resulted
in the ieee80211_key_enable_hw_accel() code being skipped.
I will fix this in ath10k driver internally.

Thanks
David

On Mon, Jun 1, 2015 at 1:34 PM, Johannes Berg <[email protected]> wrote:
> On Mon, 2015-06-01 at 12:38 -0700, David Liu wrote:
>> The change reserves neccessary tailroom in CCMP and TKIP if required by
>> drivers that sets IEEE80211_KEY_FLAG_RESERVE_TAILROOM. For example,
>> ath10k HW engine in raw Tx/Rx encap mode requires SW reserve MIC/ICV space.
>
> I don't think this makes all that much sense. The flag is already
> honoured today to actually leave enough tailroom in the SKB, and the
> remainder of the code can very easily be done in the driver (in fact, if
> you have a half-decent DMA engine, it's far more efficient to *not* set
> this flag!)
>
> You're also breaking other drivers with your change. Just do it in the
> driver.
>
> johannes
>