diff mbox

mac80211: support IEEE80211_KEY_FLAG_RESERVE_TAILROOM for CCMP and TKIP.

Message ID 1433187498-15124-1-git-send-email-cfliu.tw@gmail.com (mailing list archive)
State Rejected
Delegated to: Johannes Berg
Headers show

Commit Message

Liu CF/TW June 1, 2015, 7:38 p.m. UTC 
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 <cfliu.tw@gmail.com>
---
 net/mac80211/wpa.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

Comments

Johannes Berg June 1, 2015, 8:34 p.m. UTC | #1 
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

--
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
Liu CF/TW June 1, 2015, 11:48 p.m. UTC | #2 
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 <johannes@sipsolutions.net> 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
>
--
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
diff mbox

Patch

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);