From patchwork Sat Dec 27 11:57:10 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Lamparter X-Patchwork-Id: 5544611 Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 23504BEEA8 for ; Sat, 27 Dec 2014 11:57:27 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 298F720173 for ; Sat, 27 Dec 2014 11:57:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D76962016C for ; Sat, 27 Dec 2014 11:57:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751219AbaL0L5P (ORCPT ); Sat, 27 Dec 2014 06:57:15 -0500 Received: from mail-wg0-f51.google.com ([74.125.82.51]:61181 "EHLO mail-wg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750826AbaL0L5O (ORCPT ); Sat, 27 Dec 2014 06:57:14 -0500 Received: by mail-wg0-f51.google.com with SMTP id x12so15786649wgg.10 for ; Sat, 27 Dec 2014 03:57:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding:content-type; bh=IHC76kyPnzT9UZmUWlvj9H3b5LLiqb9JX9GMlFk6ac8=; b=ATZEdyojo1aNnj+icyES+7TjH/oXnTozLfzHubqcP+Tit5xZlYdwAOWEy8xwiMn8Tc pGlTKn2zcq2wJS7UblSGqAmwAG1gnIvGhF95vw9mjZSldaWQghZCcVR0CurnnyAch1B8 zV6sIqkJhTQ9oyjWJmHBcqhAJsubgLzjMFRpuOsr6xXLb3gHOv0P8kHUZIdg2d9augcR 910zwv7/7nHm1X/yXjHaKv/e5v0YOU6nNN/UOUqdcA1G8F5QkYltwXUXu2b8picWBi6D vjP6j6OMADk1FxY2AEqVmBf0TTKMK+86h7vjxrLvk/gzs6rYCqvDWvX0PIfNhPZI3xvm eGqQ== X-Received: by 10.180.108.143 with SMTP id hk15mr78567026wib.6.1419681432955; Sat, 27 Dec 2014 03:57:12 -0800 (PST) Received: from debian64.daheim (p5B2E61F7.dip0.t-ipconnect.de. [91.46.97.247]) by mx.google.com with ESMTPSA id i3sm22955522wjw.2.2014.12.27.03.57.11 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 27 Dec 2014 03:57:11 -0800 (PST) Received: from localhost.daheim ([127.0.0.1] helo=debian64.localnet) by debian64.daheim with esmtps (TLS1.0:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84) (envelope-from ) id 1Y4pza-00067B-EX; Sat, 27 Dec 2014 12:57:10 +0100 From: Christian Lamparter To: Christopher Chavez Cc: linux-wireless@vger.kernel.org, Larry Finger , Johannes Berg Subject: Re: p54usb kernel panic on recent mainline kernels Date: Sat, 27 Dec 2014 12:57:10 +0100 Message-ID: <5946673.YueIgFzjsn@debian64> User-Agent: KMail/4.14.2 (Linux/3.19.0-rc1-wl+; KDE/4.14.2; x86_64; ; ) In-Reply-To: <2296382.VCr2c4tJc9@debian64> References: <2296382.VCr2c4tJc9@debian64> MIME-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Alright, here's lunch [for the people in CET]. On Saturday, December 27, 2014 11:10:16 AM Christian Lamparter wrote: > On Saturday, December 27, 2014 12:15:58 AM Christopher Chavez wrote: > > > My bisection led to a branch commit d17ec4d as the "bad" commit. > > > Rather than finding out where the bisection went bad, I added > > > code to check skb->tail, skb->end, and the length to be added. > > > At the time of the call that panics, there are 6 bytes between > > > tail and end with 8 bytes needed. > > > > > > I will be looking for the place where the driver calculates how > > > large the skb should be. > > From looking at a other patch from that time and context. I think: " > > commit ca34e3b5c808385b175650605faa29e71e91991b > Author: Ido Yariv > Date: Tue Jul 29 15:38:53 2014 +0300 > > mac80211: Fix accounting of the tailroom-needed counter [1] > > [...] > I can think of several ways of dealing with this issue: > > 2. add extra IEEE80211_KEY_FLAG_ or HW_FLAG to restore the old behavior. > This should be possible and relatively simple. But we/I have to be > especially careful to differentiate properly between the old and new. > [i.e.: I need to know what the deal is behind: > IEEE80211_KEY_FLAG_GENERATE_IV_MGMT in this case? Looks like it can > be ignored?] > Tested-by: Larry Finger Tested-by: Christopher Chavez --- [Note: for convenience this patch is rolled into one for now - if it and the concept works, I'll make two parts. one for p54 one for mac80211 [both -stable]. It would be great if someone could proofread the commit message though - and provide "tested-by" tags if possible] mac80211: re-enable tailroom resizing when needed for hardware encryption The patch "mac80211: Fix accounting of the tailroom-needed counter" reduced the overhead associated with unnecessary resizing of outgoing frames. Unfortunately this change broke the assumption that there is always enough tailroom and this in turn caused p54* to panic. Reported-by: Christopher Chavez Signed-off-by: Christian Lamparter --- -- 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 --git a/drivers/net/wireless/p54/main.c b/drivers/net/wireless/p54/main.c index 97aeff0..b877c7f 100644 --- a/drivers/net/wireless/p54/main.c +++ b/drivers/net/wireless/p54/main.c @@ -751,6 +751,7 @@ struct ieee80211_hw *p54_init_common(size_t priv_data_len) IEEE80211_HW_SUPPORTS_PS | IEEE80211_HW_PS_NULLFUNC_STACK | IEEE80211_HW_MFP_CAPABLE | + IEEE80211_HW_TAILROOM_CRYPTO | IEEE80211_HW_REPORTS_TX_ACK_STATUS; dev->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) | diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 58d719d..c04ac04 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -1270,8 +1270,11 @@ struct ieee80211_vif *wdev_to_ieee80211_vif(struct wireless_dev *wdev); * * @IEEE80211_KEY_FLAG_GENERATE_IV: This flag should be set by the * driver to indicate that it requires IV generation for this - * particular key. Setting this flag does not necessarily mean that SKBs - * will have sufficient tailroom for ICV or MIC. + * particular key. Setting this flag does not mean that SKBs will + * have sufficient tailroom for ICV or MIC. If additional tailroom + * tailroom needs to be reserved for the ICV or MIC, the driver + * should also set the hardware feature flag: + * %IEEE80211_HW_TAILROOM_CRYPTO. * @IEEE80211_KEY_FLAG_GENERATE_MMIC: This flag should be set by * the driver for a TKIP key if it requires Michael MIC * generation in software. @@ -1583,6 +1586,10 @@ struct ieee80211_tx_control { * @IEEE80211_HW_MFP_CAPABLE: * Hardware supports management frame protection (MFP, IEEE 802.11w). * + * @IEEE80211_HW_TAILROOM_CRYPTO: The driver would like to have sufficient + * tailroom for ICV or MIC for outgoing frames in order to perform + * hardware encryption without any additional resizing overhead. + * * @IEEE80211_HW_SUPPORTS_UAPSD: * Hardware supports Unscheduled Automatic Power Save Delivery * (U-APSD) in managed mode. The mode is configured with @@ -1673,7 +1680,7 @@ enum ieee80211_hw_flags { IEEE80211_HW_MFP_CAPABLE = 1<<13, IEEE80211_HW_WANT_MONITOR_VIF = 1<<14, IEEE80211_HW_NO_AUTO_VIF = 1<<15, - /* free slot */ + IEEE80211_HW_TAILROOM_CRYPTO = 1<<16, IEEE80211_HW_SUPPORTS_UAPSD = 1<<17, IEEE80211_HW_REPORTS_TX_ACK_STATUS = 1<<18, IEEE80211_HW_CONNECTION_MONITOR = 1<<19, diff --git a/net/mac80211/key.c b/net/mac80211/key.c index 0bb7038..c3e9a9a 100644 --- a/net/mac80211/key.c +++ b/net/mac80211/key.c @@ -140,7 +140,8 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key) if (!ret) { key->flags |= KEY_FLAG_UPLOADED_TO_HARDWARE; - if (!(key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIC)) + if (!((key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIC) || + (key->local->hw.flags & IEEE80211_HW_TAILROOM_CRYPTO))) sdata->crypto_tx_tailroom_needed_cnt--; WARN_ON((key->conf.flags & IEEE80211_KEY_FLAG_PUT_IV_SPACE) && @@ -188,7 +189,8 @@ static void ieee80211_key_disable_hw_accel(struct ieee80211_key *key) sta = key->sta; sdata = key->sdata; - if (!(key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIC)) + if (!((key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIC) || + (key->local->hw.flags & IEEE80211_HW_TAILROOM_CRYPTO))) increment_tailroom_need_count(sdata); ret = drv_set_key(key->local, DISABLE_KEY, sdata, @@ -884,7 +886,8 @@ void ieee80211_remove_key(struct ieee80211_key_conf *keyconf) if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) { key->flags &= ~KEY_FLAG_UPLOADED_TO_HARDWARE; - if (!(key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIC)) + if (!((key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIC) || + (key->local->hw.flags & IEEE80211_HW_TAILROOM_CRYPTO))) increment_tailroom_need_count(key->sdata); }