diff mbox

[GIT] Networking

Message ID CAAA5faFWPCrk0C3sjT==VMVCq1jcR8_HANV=RRE-Ummv4_psHA@mail.gmail.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Reinoud Koornstra May 19, 2016, 9:14 a.m. UTC 
On Thu, May 19, 2016 at 2:20 AM, Reinoud Koornstra
<reinoudkoornstra@gmail.com> wrote:
> On Wed, May 18, 2016 at 12:51 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Wed, May 18, 2016 at 11:45 AM, Linus Torvalds
>> <torvalds@linux-foundation.org> 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

Comments

Sedat Dilek May 19, 2016, 9:40 a.m. UTC | #1 
On 5/19/16, Reinoud Koornstra <reinoudkoornstra@gmail.com> wrote:
> On Thu, May 19, 2016 at 2:20 AM, Reinoud Koornstra
> <reinoudkoornstra@gmail.com> wrote:
>> On Wed, May 18, 2016 at 12:51 PM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>> On Wed, May 18, 2016 at 11:45 AM, Linus Torvalds
>>> <torvalds@linux-foundation.org> 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,
>

I am not affected by any of these iwlwifi issues.

Can you next time add the commit-subject together with the commit-id,
like this...

commit 5c08b0f5026fcc13efb947c4d1f2ca3558145f68
"iwlwifi: mvm: don't override the rate with the AMSDU len"

...it's easier for followers of this thread.

- Sedat -

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5c08b0f5026fcc13efb947c4d1f2ca3558145f68
diff mbox

Patch

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;