From patchwork Thu May 19 09:14:55 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Reinoud Koornstra X-Patchwork-Id: 9125411 X-Patchwork-Delegate: kvalo@adurom.com 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.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 8C35BBF29F for ; Thu, 19 May 2016 09:15:33 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7E653201BC for ; Thu, 19 May 2016 09:15:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 245AC20212 for ; Thu, 19 May 2016 09:15:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932508AbcESJPH (ORCPT ); Thu, 19 May 2016 05:15:07 -0400 Received: from mail-qk0-f196.google.com ([209.85.220.196]:34226 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932130AbcESJPB (ORCPT ); Thu, 19 May 2016 05:15:01 -0400 Received: by mail-qk0-f196.google.com with SMTP id i7so7000049qkd.1; Thu, 19 May 2016 02:15:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=fVxXXaPX7HbeJA/DQDUu8nkznvVy4yl2GQohIas2pdA=; b=I825MRHt125xcQPBEMYxHauCsY9t1sM597aYVY2Ge/A3P1SzoatSemg3XC9Cf2fn7T LfsnYTfs/AAzLuATZWgrPtPOK1jaBHxj8FiNbKyQoASw3JqTtizOyZ6b+wsqY+ww6mS4 ET2CCS/6Mp3If3KrGJQwmCG1nwUlaZaxq2BC291xeg+dpCUtvqL8c/Sc1LS5JKJBJgUh HV/T5MkiISYXl/2Ltf0AdvVGggUffK3Alupl1Xs/VPSNBy52G/i/0Dwvxn2FdAHCqms1 ZVRlUcJNNVGidPvBKMvhSbu0YZ5IvasXnTvg0GU0lQPnt6UgEA2JNfKMdUCmdio9oQbI pQRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=fVxXXaPX7HbeJA/DQDUu8nkznvVy4yl2GQohIas2pdA=; b=PRrfg/9t44HOcGR4YUqP68lig2JB1zA3nC1Unj11kC9P6s/Emh+Gy5Y74bg9DsEpjW //pGuH2gr/yTk5MjS9F7I0RHse1to8wW1hM2ZGn+2kDbsuXD5NmPjQGOkcnSJtlWt0/w 0gPOIcZB3N5Xs2ToS3G90CSDZxTxRaGfanUL0vBKjAEQqmFxqlagOLFbnW6ZRJMBxvFc Hgs19kGZ34g1Z2xPSZjuv51IiIocbLUyhdScgyXDPjGE+C7esmlc1AapGQdITCEVM2Um eKzhGpC3HhRT7ON2ZvpUoYkU7M45oKOtnFatc2qE81GmKrkKN1rcJZMid0Qi9kWb/rFv 1YRQ== X-Gm-Message-State: AOPr4FX8SfFL74WZfWESW0aaTI2cwoiwtTF439SzYcBhYV4ALYIl0A5WxidJjX/9tG/2yFz+sLy5UQhDMitjvg== MIME-Version: 1.0 X-Received: by 10.55.143.129 with SMTP id r123mr12747682qkd.170.1463649295243; Thu, 19 May 2016 02:14:55 -0700 (PDT) Received: by 10.55.101.5 with HTTP; Thu, 19 May 2016 02:14:55 -0700 (PDT) In-Reply-To: References: <20160517.151113.367799295750703003.davem@davemloft.net> <1463568714.13625.18.camel@intel.com> <1463575273.13625.23.camel@intel.com> <1463581411.29999.1.camel@intel.com> Date: Thu, 19 May 2016 03:14:55 -0600 Message-ID: Subject: Re: [GIT] Networking From: Reinoud Koornstra To: Linus Torvalds Cc: "Coelho, Luciano" , "linux-kernel@vger.kernel.org" , linuxwifi , "Berg, Johannes" , "akpm@linux-foundation.org" , "kvalo@codeaurora.org" , "egrumbach@gmail.com" , "netdev@vger.kernel.org" , "davem@davemloft.net" , "linux-wireless@vger.kernel.org" , "Grumbach, Emmanuel" Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=unavailable 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 On Thu, May 19, 2016 at 2:20 AM, Reinoud Koornstra wrote: > On Wed, May 18, 2016 at 12:51 PM, Linus Torvalds > wrote: >> On Wed, May 18, 2016 at 11:45 AM, Linus Torvalds >> wrote: >>> >>> From what I can tell, there's a merge bug in commit 909b27f70643, >>> where David seems to have lost some of the changes to >>> iwl_mvm_set_tx_cmd(). >>> >>> I do not know if that's the reason for the problem I see. But I will test. >> >> Yes. The attached patch that fixes the incorrect merge seems to fix >> things for me. >> >> That should mean that the assumption that this problem existed in v4.6 >> too was wrong, because the incorrect merge came in later. I think >> Luciano mis-understood "v4.6+" to mean plain v4.6. >> >> Reinoud Koornstra, does this patch fix things for you too? > > Indeed, I meant 4.6+, not 4.6. > The patch you attached doesn't change existing code for me in 4.6+ as > these two lines are already in there. > Thanks, > > Reinoud. > In the 4.6+ code from today I reverted commit 5c08b0f5026f. Now iwlwifi works fine for me again. So it's as the Intel guys suspected. I'll attached my revert compared to the current 4.6+ development code. Thanks, Reinoud. >> >> Linus diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c index c53aa0f..00c9298 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c @@ -211,7 +211,6 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm, struct sk_buff *skb, struct iwl_tx_cmd *tx_cmd, struct ieee80211_tx_info *info, u8 sta_id) { - struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb); struct ieee80211_hdr *hdr = (void *)skb->data; __le16 fc = hdr->frame_control; u32 tx_flags = le32_to_cpu(tx_cmd->tx_flags); @@ -295,7 +294,7 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm, struct sk_buff *skb, tx_cmd->tx_flags = cpu_to_le32(tx_flags); /* Total # bytes to be transmitted */ tx_cmd->len = cpu_to_le16((u16)skb->len + - (uintptr_t)skb_info->driver_data[0]); + (uintptr_t)info->driver_data[0]); tx_cmd->life_time = cpu_to_le32(TX_CMD_LIFE_TIME_INFINITE); tx_cmd->sta_id = sta_id; @@ -443,11 +442,10 @@ static void iwl_mvm_set_tx_cmd_crypto(struct iwl_mvm *mvm, */ static struct iwl_device_cmd * iwl_mvm_set_tx_params(struct iwl_mvm *mvm, struct sk_buff *skb, - struct ieee80211_tx_info *info, int hdrlen, - struct ieee80211_sta *sta, u8 sta_id) + int hdrlen, struct ieee80211_sta *sta, u8 sta_id) { struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; - struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb); + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); struct iwl_device_cmd *dev_cmd; struct iwl_tx_cmd *tx_cmd; @@ -467,10 +465,10 @@ iwl_mvm_set_tx_params(struct iwl_mvm *mvm, struct sk_buff *skb, iwl_mvm_set_tx_cmd_rate(mvm, tx_cmd, info, sta, hdr->frame_control); - memset(&skb_info->status, 0, sizeof(skb_info->status)); - memset(skb_info->driver_data, 0, sizeof(skb_info->driver_data)); + memset(&info->status, 0, sizeof(info->status)); + memset(info->driver_data, 0, sizeof(info->driver_data)); - skb_info->driver_data[1] = dev_cmd; + info->driver_data[1] = dev_cmd; return dev_cmd; } @@ -478,25 +476,22 @@ iwl_mvm_set_tx_params(struct iwl_mvm *mvm, struct sk_buff *skb, int iwl_mvm_tx_skb_non_sta(struct iwl_mvm *mvm, struct sk_buff *skb) { struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; - struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb); - struct ieee80211_tx_info info; + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); struct iwl_device_cmd *dev_cmd; struct iwl_tx_cmd *tx_cmd; u8 sta_id; int hdrlen = ieee80211_hdrlen(hdr->frame_control); - memcpy(&info, skb->cb, sizeof(info)); - - if (WARN_ON_ONCE(info.flags & IEEE80211_TX_CTL_AMPDU)) + if (WARN_ON_ONCE(info->flags & IEEE80211_TX_CTL_AMPDU)) return -1; - if (WARN_ON_ONCE(info.flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM && - (!info.control.vif || - info.hw_queue != info.control.vif->cab_queue))) + if (WARN_ON_ONCE(info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM && + (!info->control.vif || + info->hw_queue != info->control.vif->cab_queue))) return -1; /* This holds the amsdu headers length */ - skb_info->driver_data[0] = (void *)(uintptr_t)0; + info->driver_data[0] = (void *)(uintptr_t)0; /* * IWL_MVM_OFFCHANNEL_QUEUE is used for ROC packets that can be used @@ -505,7 +500,7 @@ int iwl_mvm_tx_skb_non_sta(struct iwl_mvm *mvm, struct sk_buff *skb) * and hence needs to be sent on the aux queue */ if (IEEE80211_SKB_CB(skb)->hw_queue == IWL_MVM_OFFCHANNEL_QUEUE && - info.control.vif->type == NL80211_IFTYPE_STATION) + info->control.vif->type == NL80211_IFTYPE_STATION) IEEE80211_SKB_CB(skb)->hw_queue = mvm->aux_queue; /* @@ -518,14 +513,14 @@ int iwl_mvm_tx_skb_non_sta(struct iwl_mvm *mvm, struct sk_buff *skb) * AUX station. */ sta_id = mvm->aux_sta.sta_id; - if (info.control.vif) { + if (info->control.vif) { struct iwl_mvm_vif *mvmvif = - iwl_mvm_vif_from_mac80211(info.control.vif); + iwl_mvm_vif_from_mac80211(info->control.vif); - if (info.control.vif->type == NL80211_IFTYPE_P2P_DEVICE || - info.control.vif->type == NL80211_IFTYPE_AP) + if (info->control.vif->type == NL80211_IFTYPE_P2P_DEVICE || + info->control.vif->type == NL80211_IFTYPE_AP) sta_id = mvmvif->bcast_sta.sta_id; - else if (info.control.vif->type == NL80211_IFTYPE_STATION && + else if (info->control.vif->type == NL80211_IFTYPE_STATION && is_multicast_ether_addr(hdr->addr1)) { u8 ap_sta_id = ACCESS_ONCE(mvmvif->ap_sta_id); @@ -534,18 +529,19 @@ int iwl_mvm_tx_skb_non_sta(struct iwl_mvm *mvm, struct sk_buff *skb) } } - IWL_DEBUG_TX(mvm, "station Id %d, queue=%d\n", sta_id, info.hw_queue); + IWL_DEBUG_TX(mvm, "station Id %d, queue=%d\n", sta_id, info->hw_queue); - dev_cmd = iwl_mvm_set_tx_params(mvm, skb, &info, hdrlen, NULL, sta_id); + dev_cmd = iwl_mvm_set_tx_params(mvm, skb, hdrlen, NULL, sta_id); if (!dev_cmd) return -1; + /* From now on, we cannot access info->control */ tx_cmd = (struct iwl_tx_cmd *)dev_cmd->payload; /* Copy MAC header from skb into command buffer */ memcpy(tx_cmd->hdr, hdr, hdrlen); - if (iwl_trans_tx(mvm->trans, skb, dev_cmd, info.hw_queue)) { + if (iwl_trans_tx(mvm->trans, skb, dev_cmd, info->hw_queue)) { iwl_trans_free_tx_cmd(mvm->trans, dev_cmd); return -1; } @@ -564,11 +560,11 @@ int iwl_mvm_tx_skb_non_sta(struct iwl_mvm *mvm, struct sk_buff *skb) #ifdef CONFIG_INET static int iwl_mvm_tx_tso(struct iwl_mvm *mvm, struct sk_buff *skb, - struct ieee80211_tx_info *info, struct ieee80211_sta *sta, struct sk_buff_head *mpdus_skb) { struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta); + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); struct ieee80211_hdr *hdr = (void *)skb->data; unsigned int mss = skb_shinfo(skb)->gso_size; struct sk_buff *tmp, *next; @@ -677,8 +673,6 @@ static int iwl_mvm_tx_tso(struct iwl_mvm *mvm, struct sk_buff *skb, /* This skb fits in one single A-MSDU */ if (num_subframes * mss >= tcp_payload_len) { - struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb); - /* * Compute the length of all the data added for the A-MSDU. * This will be used to compute the length to write in the TX @@ -687,10 +681,11 @@ static int iwl_mvm_tx_tso(struct iwl_mvm *mvm, struct sk_buff *skb, * already had one set of SNAP / IP / TCP headers. */ num_subframes = DIV_ROUND_UP(tcp_payload_len, mss); + info = IEEE80211_SKB_CB(skb); amsdu_add = num_subframes * sizeof(struct ethhdr) + (num_subframes - 1) * (snap_ip_tcp + pad); /* This holds the amsdu headers length */ - skb_info->driver_data[0] = (void *)(uintptr_t)amsdu_add; + info->driver_data[0] = (void *)(uintptr_t)amsdu_add; __skb_queue_tail(mpdus_skb, skb); return 0; @@ -730,14 +725,12 @@ segment: ip_hdr(tmp)->id = htons(ip_base_id + i * num_subframes); if (tcp_payload_len > mss) { - struct ieee80211_tx_info *skb_info = - IEEE80211_SKB_CB(tmp); num_subframes = DIV_ROUND_UP(tcp_payload_len, mss); + info = IEEE80211_SKB_CB(tmp); amsdu_add = num_subframes * sizeof(struct ethhdr) + (num_subframes - 1) * (snap_ip_tcp + pad); - skb_info->driver_data[0] = - (void *)(uintptr_t)amsdu_add; + info->driver_data[0] = (void *)(uintptr_t)amsdu_add; skb_shinfo(tmp)->gso_size = mss; } else { qc = ieee80211_get_qos_ctl((void *)tmp->data); @@ -759,7 +752,6 @@ segment: } #else /* CONFIG_INET */ static int iwl_mvm_tx_tso(struct iwl_mvm *mvm, struct sk_buff *skb, - struct ieee80211_tx_info *info, struct ieee80211_sta *sta, struct sk_buff_head *mpdus_skb) { @@ -803,10 +795,10 @@ static void iwl_mvm_tx_add_stream(struct iwl_mvm *mvm, * Sets the fields in the Tx cmd that are crypto related */ static int iwl_mvm_tx_mpdu(struct iwl_mvm *mvm, struct sk_buff *skb, - struct ieee80211_tx_info *info, struct ieee80211_sta *sta) { struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); struct iwl_mvm_sta *mvmsta; struct iwl_device_cmd *dev_cmd; struct iwl_tx_cmd *tx_cmd; @@ -827,8 +819,7 @@ static int iwl_mvm_tx_mpdu(struct iwl_mvm *mvm, struct sk_buff *skb, if (WARN_ON_ONCE(mvmsta->sta_id == IWL_MVM_STATION_COUNT)) return -1; - dev_cmd = iwl_mvm_set_tx_params(mvm, skb, info, hdrlen, - sta, mvmsta->sta_id); + dev_cmd = iwl_mvm_set_tx_params(mvm, skb, hdrlen, sta, mvmsta->sta_id); if (!dev_cmd) goto drop; @@ -928,8 +919,7 @@ int iwl_mvm_tx_skb(struct iwl_mvm *mvm, struct sk_buff *skb, struct ieee80211_sta *sta) { struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta); - struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb); - struct ieee80211_tx_info info; + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); struct sk_buff_head mpdus_skbs; unsigned int payload_len; int ret; @@ -940,23 +930,21 @@ int iwl_mvm_tx_skb(struct iwl_mvm *mvm, struct sk_buff *skb, if (WARN_ON_ONCE(mvmsta->sta_id == IWL_MVM_STATION_COUNT)) return -1; - memcpy(&info, skb->cb, sizeof(info)); - /* This holds the amsdu headers length */ - skb_info->driver_data[0] = (void *)(uintptr_t)0; + info->driver_data[0] = (void *)(uintptr_t)0; if (!skb_is_gso(skb)) - return iwl_mvm_tx_mpdu(mvm, skb, &info, sta); + return iwl_mvm_tx_mpdu(mvm, skb, sta); payload_len = skb_tail_pointer(skb) - skb_transport_header(skb) - tcp_hdrlen(skb) + skb->data_len; if (payload_len <= skb_shinfo(skb)->gso_size) - return iwl_mvm_tx_mpdu(mvm, skb, &info, sta); + return iwl_mvm_tx_mpdu(mvm, skb, sta); __skb_queue_head_init(&mpdus_skbs); - ret = iwl_mvm_tx_tso(mvm, skb, &info, sta, &mpdus_skbs); + ret = iwl_mvm_tx_tso(mvm, skb, sta, &mpdus_skbs); if (ret) return ret; @@ -966,7 +954,7 @@ int iwl_mvm_tx_skb(struct iwl_mvm *mvm, struct sk_buff *skb, while (!skb_queue_empty(&mpdus_skbs)) { skb = __skb_dequeue(&mpdus_skbs); - ret = iwl_mvm_tx_mpdu(mvm, skb, &info, sta); + ret = iwl_mvm_tx_mpdu(mvm, skb, sta); if (ret) { __skb_queue_purge(&mpdus_skbs); return ret;