Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH 3/8 V2] rtlwifi: rtl8192cu: Fix smatch warning in rtl8192cu/trx.c
From: Sergei Shtylyov @ 2013-09-16 19:05 UTC (permalink / raw)
  To: Larry Finger; +Cc: linville, linux-wireless, netdev
In-Reply-To: <1379357722-17687-4-git-send-email-Larry.Finger@lwfinger.net>

On 09/16/2013 10:55 PM, Larry Finger wrote:

> Smatch lists the following:
>    CHECK   drivers/net/wireless/rtlwifi/rtl8192cu/trx.c
> drivers/net/wireless/rtlwifi/rtl8192cu/trx.c:367 _rtl_rx_process() warn: assigning (-98) to unsigned variable 'stats.noise'

> This variable is unused, thus it is removed.

    It's a structure field initializer you're removing, not a variable. And 
also a comment.

> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>   drivers/net/wireless/rtlwifi/rtl8192cu/trx.c | 2 --
>   1 file changed, 2 deletions(-)

> diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/trx.c b/drivers/net/wireless/rtlwifi/rtl8192cu/trx.c
> index 763cf1d..04c7e57 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192cu/trx.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192cu/trx.c
> @@ -349,7 +349,6 @@ bool rtl92cu_rx_query_desc(struct ieee80211_hw *hw,
>   	}
>   	/*rx_status->qual = stats->signal; */
>   	rx_status->signal = stats->rssi + 10;
> -	/*rx_status->noise = -stats->noise; */
>   	return true;
>   }
>
> @@ -364,7 +363,6 @@ static void _rtl_rx_process(struct ieee80211_hw *hw, struct sk_buff *skb)
>   	u8 *rxdesc;
>   	struct rtl_stats stats = {
>   		.signal = 0,
> -		.noise = -98,
>   		.rate = 0,
>   	};

WBR, Sergei



^ permalink raw reply

* Re: [PATCH 5/8 V2] [PATCH 5/7: rtlwifi: Fix smatch warning in pci.c
From: Sergei Shtylyov @ 2013-09-16 19:04 UTC (permalink / raw)
  To: Larry Finger; +Cc: linville, linux-wireless, netdev
In-Reply-To: <1379357722-17687-6-git-send-email-Larry.Finger@lwfinger.net>

On 09/16/2013 10:55 PM, Larry Finger wrote:

> Smatch reports the following:
>    CHECK   drivers/net/wireless/rtlwifi/pci.c
> drivers/net/wireless/rtlwifi/pci.c:739 _rtl_pci_rx_interrupt() warn: assigning (-98) to unsigned variable 'stats.noise'

> This variable is not used and is removed.

    It is not a variable but a structure field initializer you're removing.

> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>   drivers/net/wireless/rtlwifi/pci.c | 1 -
>   1 file changed, 1 deletion(-)

> diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
> index 703f839..6295ed2 100644
> --- a/drivers/net/wireless/rtlwifi/pci.c
> +++ b/drivers/net/wireless/rtlwifi/pci.c
> @@ -736,7 +736,6 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>
>   	struct rtl_stats stats = {
>   		.signal = 0,
> -		.noise = -98,
>   		.rate = 0,
>   	};

WBR, Sergei


^ permalink raw reply

* Re: [PATCH 4/8 V2] rtlwifi: rtl8192_common: Fix smatch errors and warnings in rtl8192c/dm_common.c
From: Sergei Shtylyov @ 2013-09-16 19:02 UTC (permalink / raw)
  To: Larry Finger; +Cc: linville, linux-wireless, netdev
In-Reply-To: <1379357722-17687-5-git-send-email-Larry.Finger@lwfinger.net>

Hello.

On 09/16/2013 10:55 PM, Larry Finger wrote:

> Smatch lists the following:
>    CHECK   drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor() info: ignoring unreachable code.
> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor() info: ignoring unreachable code.
> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:882 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:883 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:891 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:892 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2

> The unreachable code message is fixed by commenting out the code that follows a return.

    You've commented out the whole function body, where is the *return* you're 
talking about?

> The code is not deleted in case it is needed later.

> The errors are fixed by increasing the size of txpwr_level.

> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>   drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)

> diff --git a/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c b/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
> index d2d57a2..0721756 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
> @@ -541,6 +541,7 @@ EXPORT_SYMBOL(rtl92c_dm_write_dig);
>
>   static void rtl92c_dm_pwdb_monitor(struct ieee80211_hw *hw)
>   {
> +/*
>   	struct rtl_priv *rtlpriv = rtl_priv(hw);
>   	long tmpentry_max_pwdb = 0, tmpentry_min_pwdb = 0xff;
>
> @@ -564,6 +565,7 @@ static void rtl92c_dm_pwdb_monitor(struct ieee80211_hw *hw)
>   	h2c_parameter[0] = 0;
>
>   	rtl92c_fill_h2c_cmd(hw, H2C_RSSI_REPORT, 3, h2c_parameter);
> + */
>   }

WBR, Sergei


^ permalink raw reply

* [PATCH 8/8 V2] rtlwifi: Remove variable 'noise' from rtl_stats struct
From: Larry Finger @ 2013-09-16 18:55 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379357722-17687-1-git-send-email-Larry.Finger@lwfinger.net>

All uses of this variable have been removed. Now delete it from the struct
and from a few places that use it in commented-out lines.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/rtlwifi/rtl8188ee/trx.c | 1 -
 drivers/net/wireless/rtlwifi/rtl8192ce/trx.c | 1 -
 drivers/net/wireless/rtlwifi/rtl8192se/trx.c | 1 -
 drivers/net/wireless/rtlwifi/rtl8723ae/trx.c | 1 -
 drivers/net/wireless/rtlwifi/wifi.h          | 1 -
 5 files changed, 5 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c b/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c
index a8871d6..5e628ab 100644
--- a/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c
@@ -477,7 +477,6 @@ bool rtl88ee_rx_query_desc(struct ieee80211_hw *hw,
 
 	/*rx_status->qual = status->signal; */
 	rx_status->signal = status->recvsignalpower + 10;
-	/*rx_status->noise = -status->noise; */
 	if (status->packet_report_type == TX_REPORT2) {
 		status->macid_valid_entry[0] =
 			 GET_RX_RPT2_DESC_MACID_VALID_1(pdesc);
diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c b/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
index 6ad23b4..52abf0a 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
@@ -420,7 +420,6 @@ bool rtl92ce_rx_query_desc(struct ieee80211_hw *hw,
 
 	/*rx_status->qual = stats->signal; */
 	rx_status->signal = stats->recvsignalpower + 10;
-	/*rx_status->noise = -stats->noise; */
 
 	return true;
 }
diff --git a/drivers/net/wireless/rtlwifi/rtl8192se/trx.c b/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
index c709511..222d2e7 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
@@ -330,7 +330,6 @@ bool rtl92se_rx_query_desc(struct ieee80211_hw *hw, struct rtl_stats *stats,
 
 	/*rx_status->qual = stats->signal; */
 	rx_status->signal = stats->rssi + 10;
-	/*rx_status->noise = -stats->noise; */
 
 	return true;
 }
diff --git a/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c b/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c
index c72758d..e85ec21 100644
--- a/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c
@@ -359,7 +359,6 @@ bool rtl8723ae_rx_query_desc(struct ieee80211_hw *hw,
 
 	/*rx_status->qual = status->signal; */
 	rx_status->signal = status->recvsignalpower + 10;
-	/*rx_status->noise = -status->noise; */
 
 	return true;
 }
diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h
index cc03e7c..284ee8d 100644
--- a/drivers/net/wireless/rtlwifi/wifi.h
+++ b/drivers/net/wireless/rtlwifi/wifi.h
@@ -1535,7 +1535,6 @@ struct rtl_stats {
 	u32 mac_time[2];
 	s8 rssi;
 	u8 signal;
-	u8 noise;
 	u8 rate;		/* hw desc rate */
 	u8 received_channel;
 	u8 control;
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 7/8 V2] rtlwifi: rtl8188ee: Fix smatch warning in rtl8188ee/hw.c
From: Larry Finger @ 2013-09-16 18:55 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev, Stable
In-Reply-To: <1379357722-17687-1-git-send-email-Larry.Finger@lwfinger.net>

Smatch lists the following:
  CHECK   drivers/net/wireless/rtlwifi/rtl8188ee/hw.c
drivers/net/wireless/rtlwifi/rtl8188ee/hw.c:149 _rtl88ee_set_fw_clock_on() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/rtl8188ee/hw.c:149 _rtl88ee_set_fw_clock_on() info: ignoring unreachable code.

This info message is the result of a real error due to a missing break statement
in a "while (1)" loop.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org> [3.10+]
---
 drivers/net/wireless/rtlwifi/rtl8188ee/hw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/rtlwifi/rtl8188ee/hw.c b/drivers/net/wireless/rtlwifi/rtl8188ee/hw.c
index b68cae3..e06971b 100644
--- a/drivers/net/wireless/rtlwifi/rtl8188ee/hw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8188ee/hw.c
@@ -143,6 +143,7 @@ static void _rtl88ee_set_fw_clock_on(struct ieee80211_hw *hw,
 		} else {
 			rtlhal->fw_clk_change_in_progress = false;
 			spin_unlock_bh(&rtlpriv->locks.fw_ps_lock);
+			break;
 		}
 	}
 
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 6/8 V2] rtlwifi: Fix smatch warnings in usb.c
From: Larry Finger @ 2013-09-16 18:55 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379357722-17687-1-git-send-email-Larry.Finger@lwfinger.net>

Smatch displays the following:
  CHECK   drivers/net/wireless/rtlwifi/usb.c
drivers/net/wireless/rtlwifi/usb.c:458 _rtl_usb_rx_process_agg() warn: assigning (-98) to unsigned variable 'stats.noise'
drivers/net/wireless/rtlwifi/usb.c:503 _rtl_usb_rx_process_noagg() warn: assigning (-98) to unsigned variable 'stats.noise'
drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring unreachable code.

The negative number to an unsigned quantity is fixed by removing the variable
as it is no longer used. The unreachable code info is fixed by including the
appropriate section inside #ifdef .. #endif constructions.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/rtlwifi/usb.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/usb.c b/drivers/net/wireless/rtlwifi/usb.c
index e56778c..60cb0b4 100644
--- a/drivers/net/wireless/rtlwifi/usb.c
+++ b/drivers/net/wireless/rtlwifi/usb.c
@@ -455,7 +455,6 @@ static void _rtl_usb_rx_process_agg(struct ieee80211_hw *hw,
 	struct ieee80211_rx_status rx_status = {0};
 	struct rtl_stats stats = {
 		.signal = 0,
-		.noise = -98,
 		.rate = 0,
 	};
 
@@ -498,7 +497,6 @@ static void _rtl_usb_rx_process_noagg(struct ieee80211_hw *hw,
 	struct ieee80211_rx_status rx_status = {0};
 	struct rtl_stats stats = {
 		.signal = 0,
-		.noise = -98,
 		.rate = 0,
 	};
 
@@ -582,12 +580,15 @@ static void _rtl_rx_work(unsigned long param)
 static unsigned int _rtl_rx_get_padding(struct ieee80211_hdr *hdr,
 					unsigned int len)
 {
+#if NET_IP_ALIGN != 0
 	unsigned int padding = 0;
+#endif
 
 	/* make function no-op when possible */
-	if (NET_IP_ALIGN == 0 || len < sizeof(*hdr))
+	if (NET_IP_ALIGN == 0 || len < sizeof(struct ieee80211_hdr))
 		return 0;
 
+#if NET_IP_ALIGN != 0
 	/* alignment calculation as in lbtf_rx() / carl9170_rx_copy_data() */
 	/* TODO: deduplicate common code, define helper function instead? */
 
@@ -608,6 +609,7 @@ static unsigned int _rtl_rx_get_padding(struct ieee80211_hdr *hdr,
 		padding ^= NET_IP_ALIGN;
 
 	return padding;
+#endif
 }
 
 #define __RADIO_TAP_SIZE_RSV	32
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 5/8 V2] [PATCH 5/7: rtlwifi: Fix smatch warning in pci.c
From: Larry Finger @ 2013-09-16 18:55 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379357722-17687-1-git-send-email-Larry.Finger@lwfinger.net>

Smatch reports the following:
  CHECK   drivers/net/wireless/rtlwifi/pci.c
drivers/net/wireless/rtlwifi/pci.c:739 _rtl_pci_rx_interrupt() warn: assigning (-98) to unsigned variable 'stats.noise'

This variable is not used and is removed.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/rtlwifi/pci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
index 703f839..6295ed2 100644
--- a/drivers/net/wireless/rtlwifi/pci.c
+++ b/drivers/net/wireless/rtlwifi/pci.c
@@ -736,7 +736,6 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
 
 	struct rtl_stats stats = {
 		.signal = 0,
-		.noise = -98,
 		.rate = 0,
 	};
 	int index = rtlpci->rx_ring[rx_queue_idx].idx;
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 4/8 V2] rtlwifi: rtl8192_common: Fix smatch errors and warnings in rtl8192c/dm_common.c
From: Larry Finger @ 2013-09-16 18:55 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379357722-17687-1-git-send-email-Larry.Finger@lwfinger.net>

Smatch lists the following:
  CHECK   drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:882 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:883 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:891 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:892 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2

The unreachable code message is fixed by commenting out the code that follows a return.
The code is not deleted in case it is needed later.

The errors are fixed by increasing the size of txpwr_level.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c b/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
index d2d57a2..0721756 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
@@ -541,6 +541,7 @@ EXPORT_SYMBOL(rtl92c_dm_write_dig);
 
 static void rtl92c_dm_pwdb_monitor(struct ieee80211_hw *hw)
 {
+/*
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
 	long tmpentry_max_pwdb = 0, tmpentry_min_pwdb = 0xff;
 
@@ -564,6 +565,7 @@ static void rtl92c_dm_pwdb_monitor(struct ieee80211_hw *hw)
 	h2c_parameter[0] = 0;
 
 	rtl92c_fill_h2c_cmd(hw, H2C_RSSI_REPORT, 3, h2c_parameter);
+ */
 }
 
 void rtl92c_dm_init_edca_turbo(struct ieee80211_hw *hw)
@@ -673,7 +675,7 @@ static void rtl92c_dm_txpower_tracking_callback_thermalmeter(struct ieee80211_hw
 	s8 cck_index = 0;
 	int i;
 	bool is2t = IS_92C_SERIAL(rtlhal->version);
-	s8 txpwr_level[2] = {0, 0};
+	s8 txpwr_level[3] = {0, 0, 0};
 	u8 ofdm_min_index = 6, rf;
 
 	rtlpriv->dm.txpower_trackinginit = true;
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 3/8 V2] rtlwifi: rtl8192cu: Fix smatch warning in rtl8192cu/trx.c
From: Larry Finger @ 2013-09-16 18:55 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379357722-17687-1-git-send-email-Larry.Finger@lwfinger.net>

Smatch lists the following:
  CHECK   drivers/net/wireless/rtlwifi/rtl8192cu/trx.c
drivers/net/wireless/rtlwifi/rtl8192cu/trx.c:367 _rtl_rx_process() warn: assigning (-98) to unsigned variable 'stats.noise'

This variable is unused, thus it is removed.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/rtlwifi/rtl8192cu/trx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/trx.c b/drivers/net/wireless/rtlwifi/rtl8192cu/trx.c
index 763cf1d..04c7e57 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192cu/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192cu/trx.c
@@ -349,7 +349,6 @@ bool rtl92cu_rx_query_desc(struct ieee80211_hw *hw,
 	}
 	/*rx_status->qual = stats->signal; */
 	rx_status->signal = stats->rssi + 10;
-	/*rx_status->noise = -stats->noise; */
 	return true;
 }
 
@@ -364,7 +363,6 @@ static void _rtl_rx_process(struct ieee80211_hw *hw, struct sk_buff *skb)
 	u8 *rxdesc;
 	struct rtl_stats stats = {
 		.signal = 0,
-		.noise = -98,
 		.rate = 0,
 	};
 	struct rx_fwinfo_92c *p_drvinfo;
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 2/8 V2] rtlwifi: rtl8192de: Fix smatch warnings in rtl8192de/hw.c
From: Larry Finger @ 2013-09-16 18:55 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379357722-17687-1-git-send-email-Larry.Finger@lwfinger.net>

Smatch lists the following:
  CHECK   drivers/net/wireless/rtlwifi/rtl8192de/hw.c
drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info: ignoring unreachable code.

Dead code is commented out. It has not been deleted in case I find a need for
it later.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/rtlwifi/rtl8192de/hw.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/hw.c b/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
index 7dd8f6d..c9b0894 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
@@ -1194,6 +1194,7 @@ void rtl92d_linked_set_reg(struct ieee80211_hw *hw)
  * mac80211 will send pkt when scan */
 void rtl92de_set_qos(struct ieee80211_hw *hw, int aci)
 {
+/*
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
 	rtl92d_dm_init_edca_turbo(hw);
 	return;
@@ -1213,6 +1214,7 @@ void rtl92de_set_qos(struct ieee80211_hw *hw, int aci)
 		RT_ASSERT(false, "invalid aci: %d !\n", aci);
 		break;
 	}
+ */
 }
 
 void rtl92de_enable_interrupt(struct ieee80211_hw *hw)
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 1/8 V2] rtlwifi: rtl8192du: Fix smatch errors in /rtl8192de/dm.c
From: Larry Finger @ 2013-09-16 18:55 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379357722-17687-1-git-send-email-Larry.Finger@lwfinger.net>

Smatch lists the following:
  CHECK   drivers/net/wireless/rtlwifi/rtl8192de/dm.c
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1054 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'ofdm_index' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1056 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'ofdm_index' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1126 rtl92d_dm_txpower_tracking_callback_thermalmeter() debug: remove_pools: nr_children over 4000 (4596). (rtlpriv->dbg.global_debuglevel merged)
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1126 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1129 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1132 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1135 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1138 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1141 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1144 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1147 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1151 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1154 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1157 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1160 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1163 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1166 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1169 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1172 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255

This patch fixes several off-by-one errors. It also removes a commented-out line
referincing variable 'noise'. That variable will be removed later in this patch
set.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/rtlwifi/rtl8192de/dm.c  | 8 ++++++--
 drivers/net/wireless/rtlwifi/rtl8192de/trx.c | 1 -
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/dm.c b/drivers/net/wireless/rtlwifi/rtl8192de/dm.c
index 47875ba..eaeee77 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192de/dm.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/dm.c
@@ -840,9 +840,9 @@ static void rtl92d_dm_txpower_tracking_callback_thermalmeter(
 	bool internal_pa = false;
 	long ele_a = 0, ele_d, temp_cck, val_x, value32;
 	long val_y, ele_c = 0;
-	u8 ofdm_index[2];
+	u8 ofdm_index[3];
 	s8 cck_index = 0;
-	u8 ofdm_index_old[2] = {0, 0};
+	u8 ofdm_index_old[3] = {0, 0, 0};
 	s8 cck_index_old = 0;
 	u8 index;
 	int i;
@@ -1118,6 +1118,10 @@ static void rtl92d_dm_txpower_tracking_callback_thermalmeter(
 				 val_x, val_y, ele_a, ele_c, ele_d,
 				 val_x, val_y);
 
+			if (cck_index >= CCK_TABLE_SIZE)
+				cck_index = CCK_TABLE_SIZE - 1;
+			if (cck_index < 0)
+				cck_index = 0;
 			if (rtlhal->current_bandtype == BAND_ON_2_4G) {
 				/* Adjust CCK according to IQK result */
 				if (!rtlpriv->dm.cck_inch14) {
diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/trx.c b/drivers/net/wireless/rtlwifi/rtl8192de/trx.c
index b8ec718..945ddec 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192de/trx.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/trx.c
@@ -526,7 +526,6 @@ bool rtl92de_rx_query_desc(struct ieee80211_hw *hw,	struct rtl_stats *stats,
 	}
 	/*rx_status->qual = stats->signal; */
 	rx_status->signal = stats->rssi + 10;
-	/*rx_status->noise = -stats->noise; */
 	return true;
 }
 
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 0/8i V2] rtlwifi: Patches to fix problems shown by smatch
From: Larry Finger @ 2013-09-16 18:55 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev

Fix smatch warnings and errors in the rtlwifi family of drivers.

V2 addresses comments by David Laight and Sergei Shtylyov.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

Larry Finger (8):
  rtlwifi: rtl8192du: Fix smatch errors in /rtl8192de/dm.c
  rtlwifi: rtl8192de: Fix smatch warnings in rtl8192de/hw.c
  rtlwifi: rtl8192cu: Fix smatch warning in rtl8192cu/trx.c
  rtlwifi: rtl8192_common: Fix smatch errors and warnings in
    rtl8192c/dm_common.c
  [PATCH 5/7: rtlwifi: Fix smatch warning in pci.c
  rtlwifi: Fix smatch warnings in usb.c
  rtlwifi: rtl8188ee: Fix smatch warning in rtl8188ee/hw.c
  rtlwifi: Remove variable 'noise' from rtl_stats struct

 drivers/net/wireless/rtlwifi/pci.c                | 1 -
 drivers/net/wireless/rtlwifi/rtl8188ee/hw.c       | 1 +
 drivers/net/wireless/rtlwifi/rtl8188ee/trx.c      | 1 -
 drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c | 4 +++-
 drivers/net/wireless/rtlwifi/rtl8192ce/trx.c      | 1 -
 drivers/net/wireless/rtlwifi/rtl8192cu/trx.c      | 2 --
 drivers/net/wireless/rtlwifi/rtl8192de/dm.c       | 8 ++++++--
 drivers/net/wireless/rtlwifi/rtl8192de/hw.c       | 2 ++
 drivers/net/wireless/rtlwifi/rtl8192de/trx.c      | 1 -
 drivers/net/wireless/rtlwifi/rtl8192se/trx.c      | 1 -
 drivers/net/wireless/rtlwifi/rtl8723ae/trx.c      | 1 -
 drivers/net/wireless/rtlwifi/usb.c                | 8 +++++---
 drivers/net/wireless/rtlwifi/wifi.h               | 1 -
 13 files changed, 17 insertions(+), 15 deletions(-)

-- 
1.8.1.4


^ permalink raw reply

* Re: [PATCH 5/7: rtlwifi: Fix smatch warning in pci.c
From: Larry Finger @ 2013-09-16 17:39 UTC (permalink / raw)
  To: David Laight; +Cc: linville, linux-wireless, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B733B@saturn3.aculab.com>

On 09/16/2013 04:26 AM, David Laight wrote:
>> Smatch reports the following:
>>    CHECK   drivers/net/wireless/rtlwifi/pci.c
>> drivers/net/wireless/rtlwifi/pci.c:739 _rtl_pci_rx_interrupt() warn: assigning (-98) to unsigned
>> variable 'stats.noise'
>>
>> This problem is fixed by changing the value to 256 - 98.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> ---
>>   drivers/net/wireless/rtlwifi/pci.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
>> index 703f839..bf498f5 100644
>> --- a/drivers/net/wireless/rtlwifi/pci.c
>> +++ b/drivers/net/wireless/rtlwifi/pci.c
>> @@ -736,7 +736,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>>
>>   	struct rtl_stats stats = {
>>   		.signal = 0,
>> -		.noise = -98,
>> +		.noise = 158,	/* -98 dBm */
>>   		.rate = 0,
>>   	};
>>   	int index = rtlpci->rx_ring[rx_queue_idx].idx;
>
> That doesn't look nice at all.
> Something like (unsigned int)-98 would be slightly better,
> but it looks as though something is actually wrong with
> the type of 'noise' itself.

The type of 'noise' is probably a legacy of wireless extensions. In fact, that 
variable is not used and will be deleted in V2 of the patches.

Thanks,

Larry



^ permalink raw reply

* Re: [PATCHv2 0/7] ath10k: improve TX path
From: Kalle Valo @ 2013-09-16 17:05 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless
In-Reply-To: <1379074618-30534-1-git-send-email-michal.kazior@tieto.com>

Michal Kazior <michal.kazior@tieto.com> writes:

> Hi,
>
> This patchset addresses two issues:
>
>  * system/userspace starvation on heavy briding
>    UDP TX
>  * unstable/inconsistent UDP TX throughput
>
> In short the patchset simplifies TX path by
> removing HTC TX workers, makes WMI commands block
> and makes ath10k more responsive to queues
> becoming full. This contributes to both improved
> throughput and makes the system more responsive
> under heavy UDP TX load.
>
> Max stable briding TX (ath10k as TX):
>  UDP w/o patchset: 520mbps
>  UDP w/  patchset: 570mbps
>
>  TCP w/o patchset: 350mbps
>  TCP w/  patchset: 400mbps
>
> Measured with two 2x2 cards, one acting as an AP
> on AP135 board, the other as a STA on a core i5
> laptop. The AP135 was passing traffic from a
> station on LAN/eth1 to the STA/wlan0.
>
> v2:
>  * use DIV_ROUND_UP
>  * comment usage of ath10k_wmi_tx_beacons_nowait
>  * additional check for tx_credits_flow_enabled
>
>
> Michal Kazior (7):
>   ath10k: simplify HTC credits calculation
>   ath10k: add HTC TX credits replenishing notification
>   ath10k: make WMI commands block by design
>   ath10k: simplify HTC command submitting
>   ath10k: improve beacon submission latency
>   ath10k: remove wmi pending count limit
>   ath10k: remove wmi event worker thread

Thanks, applied.

-- 
Kalle Valo

^ permalink raw reply

* RE: [PATCH v4] Bluetooth: btmrvl: add calibration data download support
From: Amitkumar Karwar @ 2013-09-16 15:51 UTC (permalink / raw)
  To: 'Mike Frysinger', Bing Zhao
  Cc: linux-bluetooth@vger.kernel.org, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, linux-wireless@vger.kernel.org, Hyuckjoo Lee
In-Reply-To: <CAAbOScnG8RHWbPdjq6=e=EBJPr_f6+R4o36f3h0T=NxGgQt7AQ@mail.gmail.com>

Hi Mike,

Thanks for your comments. We will take care of them in updated version.

> 
> On Fri, Sep 13, 2013 at 7:32 PM, Bing Zhao wrote:
> > --- a/drivers/bluetooth/btmrvl_main.c
> > +++ b/drivers/bluetooth/btmrvl_main.c
> >
> > +static int btmrvl_parse_cal_cfg(const u8 *src, u32 len, u8 *dst, u32
> dst_size)
> 
> would be nice if you put a comment above this func explaining the
> expected format.  otherwise, we see arbitrary parsing with no idea if
> it's correct.

Sure. We will add below information.

Calibrated input data should contain hex bytes separated by space or
new line character. Here is an example
00 1C 01 37 FF FF FF FF 02 04 7F 01
CE BA 00 00 00 2D C6 C0 00 00 00 00
00 F0 00 00

> 
> > +       while ((s - src) < len) {
> > +               if (isspace(*s)) {
> > +                       s++;
> > +                       continue;
> > +               }
> > +
> > +               if (isxdigit(*s)) {
> > +                       if ((d - dst) >= dst_size) {
> > +                               BT_ERR("calibration data file too
> big!!!");
> > +                               return -EINVAL;
> > +                       }
> > +
> > +                       memcpy(tmp, s, 2);
> > +
> > +                       ret = kstrtou8(tmp, 16, d++);
> > +                       if (ret < 0)
> > +                               return ret;
> > +
> > +                       s += 2;
> > +               } else {
> > +                       s++;
> > +               }
> > +       }
> 
> so if it's a space, you skip it.  if it's a hexdigit, you parse two
> bytes.  if it's anything else, you skip it.  i'd imagine the "non
> space and non hexdigit" case should throw a warning if not reject the
> file out right.  otherwise, if you want to keep this logic, punt the
> explicit "isspace" check.

You are right. Rejecting the file containing non space, non new line and non hexdigit character makes sense.

> 
> you might also copy one more byte than you should ?  your limit is "(s
> - src) < len", yet the isxdigit code always copies two bytes.

Thanks for pointing this out. We will replace "(s - src) < len" with "(s -src) <= (len - 2)". 

> 
> > +static int btmrvl_load_cal_data(struct btmrvl_private *priv,
> > +                               u8 *config_data)
> > +{
> > +       struct sk_buff *skb;
> > +       struct btmrvl_cmd *cmd;
> > +       int i;
> > +
> > +       skb = bt_skb_alloc(sizeof(*cmd), GFP_ATOMIC);
> 
> maybe i'm unfamiliar with bluetooth and this is common, but why is
> your code so special as to require GFP_ATOMIC allocations ?

GFP_ATOMIC was used to match other usages in bluetooth code.
I just found that as per commit "Remove GFP_ATOMIC usage from l2cap_core.c"(commit id: 8bcde1f2ab..), since bluetooth core is now running in process context, we don't need to use GFP_ATOMIC.

We will replace it with GFP_KERNEL in updated version.

> 
> > +       for (i = 4; i < BT_CMD_DATA_SIZE; i++)
> > +               cmd->data[i] = config_data[(i/4)*8 - 1 - i];
> 
> style nit, but there should be spacing around those math operators.
> ignoring the fact that this is some funky funky buffer offsets.
> 
> i = 4
>  config_data[(4 / 4) * 8 - 1 - 4] ->
>  config_data[8 - 1 - 4] ->
>  config_data[3]
> 
> i = 5
>  config_data[(5 / 4) * 8 - 1 - 5] ->
>  config_data[8 - 1 - 5] ->
>  config_data[2]
> 
> i = 6
>  config_data[(6 / 4) * 8 - 1 - 6] ->
>  config_data[8 - 1 - 6] ->
>  config_data[1]
> 
> i = 7
>  config_data[(7 / 4) * 8 - 1 - 7] ->
>  config_data[8 - 1 - 7] ->
>  config_data[0]
> 
> i = 8
>  config_data[(8 / 4) * 8 - 1 - 8] ->
>  config_data[16 - 1 - 8] ->
>  config_data[7]
> 
> i = {4,5,6,7} -> config_data[{3,2,1,0}]
> i = {8,9,10,11} -> config_data[{7,6,5,4}]
> 
> that really could do with a comment explaining the mapping of input
> bytes to output bytes.

Sure. We add a comment here.
Actually each 4 bytes are being swapped. Considering 4 byte SDIO header offset, it becomes
input{3, 2, 1, 0} -> output{0+4, 1+4, 2+4, 3+4}

Regards,
Amitkumar Karwar

> -mike

^ permalink raw reply

* Re: guidance on struct alignment for rtl8192cu driver
From: Larry Finger @ 2013-09-16 15:33 UTC (permalink / raw)
  To: Seth Forshee; +Cc: Jason Andrews, linux-wireless@vger.kernel.org
In-Reply-To: <20130916143506.GC18593@thinkpad-t410>

On 09/16/2013 09:35 AM, Seth Forshee wrote:
> On Sat, Sep 14, 2013 at 09:08:34AM -0500, Larry Finger wrote:
>> On 09/14/2013 12:36 AM, Jason Andrews wrote:
>>> I'm using an ASUS USB N13 on an ARM platform with the rtl8192cu driver.
>>> Linux kernel is 3.10 so I probably don't have the latest and greatest driver.
>>>
>>> When I booted I got an ARM alignment trap caused by the driver.
>>>
>>> I determined the cause was the 1st argument to spin_lock_irqsave() has an unaligned address.
>>>
>>> By trial-and-error I found that if I edit wifi.h and insert 2 dummy bytes into the rtl_priv struct just above priv (last variable) the locks work and the driver works fine.
>>>
>>> What is the recommended way to make sure the last variable in the rtl_priv struct (u8 priv[0]) is aligned on a 4 byte boundary so the driver works on ARM machines?
>>
>> There are a lot of improvements for this driver in 3.11. The
>> backports release has that code. In addition, I am currently working
>> at improving the power management for 3.13.
>>
>> The presence of unaligned variables that cause alignment traps on
>> ARM does not surprise me as I test only on x86 and ppc
>> architectures. I now own a Raspberry Pi and I will soon be testing
>> with it as well.
>>
>> What does surprise me is that the first argument in all the calls to
>> spin_lock_irqsave() are contained within the rtl_locks struct and
>> everything there should be aligned. Perhaps some ARM expert will
>> know why aligning the last item in the rtl_priv struct fixes the
>> problem.
>
> Depending on architecture version and configuration ARM may or may not
> allow unaligned accesses. Even when allowed there is a cost though, so
> it's better to properly align the data. In the past this would have
> always meant 4-byte alignment, but my ARM experience is a bit dated now
> and I don't know about 64-bit ARM. That variable-size array probably
> only has byte alignment.
>
>> As far as I know, the proper way to do a 4-byte alignment is as in
>> the following patch:
>>
>> Index: wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
>> ===================================================================
>> --- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/wifi.h
>> +++ wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
>> @@ -2057,7 +2057,7 @@ struct rtl_priv {
>>   	  that it points to the data allocated
>>   	  beyond  this structure like:
>>   	  rtl_pci_priv or rtl_usb_priv */
>> -	u8 priv[0];
>> +	u8 __aligned(4) priv[0];
>>   };
>
> __attribute__((aligned)) might be a safer bet, as this will align it to
> the largest alignment that could possibly be needed.

Seth,

Thanks for the help. So far, I have not heard if my original patch helps or not. 
When, and if, I hear, I will use your suggestion for the final patch.

Larry



^ permalink raw reply

* Re: guidance on struct alignment for rtl8192cu driver
From: Seth Forshee @ 2013-09-16 14:35 UTC (permalink / raw)
  To: Larry Finger; +Cc: Jason Andrews, linux-wireless@vger.kernel.org
In-Reply-To: <52346DE2.9090904@lwfinger.net>

On Sat, Sep 14, 2013 at 09:08:34AM -0500, Larry Finger wrote:
> On 09/14/2013 12:36 AM, Jason Andrews wrote:
> >I'm using an ASUS USB N13 on an ARM platform with the rtl8192cu driver.
> >Linux kernel is 3.10 so I probably don't have the latest and greatest driver.
> >
> >When I booted I got an ARM alignment trap caused by the driver.
> >
> >I determined the cause was the 1st argument to spin_lock_irqsave() has an unaligned address.
> >
> >By trial-and-error I found that if I edit wifi.h and insert 2 dummy bytes into the rtl_priv struct just above priv (last variable) the locks work and the driver works fine.
> >
> >What is the recommended way to make sure the last variable in the rtl_priv struct (u8 priv[0]) is aligned on a 4 byte boundary so the driver works on ARM machines?
> 
> There are a lot of improvements for this driver in 3.11. The
> backports release has that code. In addition, I am currently working
> at improving the power management for 3.13.
> 
> The presence of unaligned variables that cause alignment traps on
> ARM does not surprise me as I test only on x86 and ppc
> architectures. I now own a Raspberry Pi and I will soon be testing
> with it as well.
> 
> What does surprise me is that the first argument in all the calls to
> spin_lock_irqsave() are contained within the rtl_locks struct and
> everything there should be aligned. Perhaps some ARM expert will
> know why aligning the last item in the rtl_priv struct fixes the
> problem.

Depending on architecture version and configuration ARM may or may not
allow unaligned accesses. Even when allowed there is a cost though, so
it's better to properly align the data. In the past this would have
always meant 4-byte alignment, but my ARM experience is a bit dated now
and I don't know about 64-bit ARM. That variable-size array probably
only has byte alignment.

> As far as I know, the proper way to do a 4-byte alignment is as in
> the following patch:
> 
> Index: wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
> ===================================================================
> --- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/wifi.h
> +++ wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
> @@ -2057,7 +2057,7 @@ struct rtl_priv {
>  	  that it points to the data allocated
>  	  beyond  this structure like:
>  	  rtl_pci_priv or rtl_usb_priv */
> -	u8 priv[0];
> +	u8 __aligned(4) priv[0];
>  };

__attribute__((aligned)) might be a safer bet, as this will align it to
the largest alignment that could possibly be needed.

Seth

^ permalink raw reply

* Re: [Ilw] [RFC 0/5] mac80211/iwlwifi: quiesce before restart hw
From: Stanislaw Gruszka @ 2013-09-16 14:10 UTC (permalink / raw)
  To: Grumbach, Emmanuel; +Cc: ilw@linux.intel.com, linux-wireless@vger.kernel.org
In-Reply-To: <0BA3FCBA62E2DC44AF3030971E174FB301DAF383@HASMSX103.ger.corp.intel.com>

On Sun, Sep 15, 2013 at 08:31:34AM +0000, Grumbach, Emmanuel wrote:
> BTW - I guess a dump_stack doesn't trigger the abrt, does it?

No, dump_stack does not trigger abrt, WARN* or BUG_ON (from process
context) is required for that.

Thanks
Stanislaw

^ permalink raw reply

* [RFC 4/4] ath10k: align RX frames properly
From: Michal Kazior @ 2013-09-16 12:49 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior
In-Reply-To: <1379335757-15180-1-git-send-email-michal.kazior@tieto.com>

Ethernet-like decapping mode leaves IP protocol
frame not aligned to 4-byte boundaries. This leads
to re-aligning in mac80211 which in turn leads to
CPU cache thrashing on lower end host machines and
very poor performance.

Since HW doesn't allow to change payload offset
properly the solution is to force HW to decap in
Native Wifi mode which always has 24-bytes long
802.11 header (even for QoS frames). This means IP
frame is properly aligned in this decap mode.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/hw.h |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 643f0c9..8c1be768 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -74,7 +74,11 @@ enum ath10k_mcast2ucast_mode {
 #define TARGET_RX_CHAIN_MASK			(BIT(0) | BIT(1) | BIT(2))
 #define TARGET_RX_TIMEOUT_LO_PRI		100
 #define TARGET_RX_TIMEOUT_HI_PRI		40
-#define TARGET_RX_DECAP_MODE			ATH10K_HW_TXRX_ETHERNET
+
+/* Native Wifi decap mode is used to align IP frames to 4-byte boundaries and
+ * avoid a very expensive re-alignment in mac80211. */
+#define TARGET_RX_DECAP_MODE			ATH10K_HW_TXRX_NATIVE_WIFI
+
 #define TARGET_SCAN_MAX_PENDING_REQS		4
 #define TARGET_BMISS_OFFLOAD_MAX_VDEV		3
 #define TARGET_ROAM_OFFLOAD_MAX_VDEV		3
-- 
1.7.9.5


^ permalink raw reply related

* [RFC 3/4] ath10k: cleanup RX decap handling
From: Michal Kazior @ 2013-09-16 12:49 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior
In-Reply-To: <1379335757-15180-1-git-send-email-michal.kazior@tieto.com>

Native Wifi frames are always decapped as non-QoS
data frames meaning mac80211 couldn't set sk_buff
priority properly. The patch fixes this by using
the original 802.11 header.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c |   70 +++++++++++++++++-------------
 1 file changed, 41 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index dad584f..083dd9a 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -618,7 +618,7 @@ static void ath10k_htt_rx_amsdu(struct ath10k_htt *htt,
 	enum rx_msdu_decap_format fmt;
 	enum htt_rx_mpdu_encrypt_type enctype;
 	struct ieee80211_hdr *hdr;
-	u8 hdr_buf[64];
+	u8 hdr_buf[64], addr[ETH_ALEN], *qos;
 	unsigned int hdr_len;
 
 	rxd = (void *)skb->data - sizeof(*rxd);
@@ -652,6 +652,19 @@ static void ath10k_htt_rx_amsdu(struct ath10k_htt *htt,
 			skb_trim(skb, skb->len - FCS_LEN);
 			break;
 		case RX_MSDU_DECAP_NATIVE_WIFI:
+			hdr = (struct ieee80211_hdr *)skb->data;
+			hdr_len = ieee80211_hdrlen(hdr->frame_control);
+			memcpy(addr, ieee80211_get_DA(hdr), ETH_ALEN);
+			skb_pull(skb, hdr_len);
+
+			hdr = (struct ieee80211_hdr *)hdr_buf;
+			hdr_len = ieee80211_hdrlen(hdr->frame_control);
+			memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
+
+			hdr = (struct ieee80211_hdr *)skb->data;
+			qos = ieee80211_get_qos_ctl(hdr);
+			qos[0] &= ~IEEE80211_QOS_CTL_A_MSDU_PRESENT;
+			memcpy(ieee80211_get_DA(hdr), addr, ETH_ALEN);
 			break;
 		case RX_MSDU_DECAP_ETHERNET2_DIX:
 			decap_len = 0;
@@ -687,6 +700,9 @@ static void ath10k_htt_rx_msdu(struct ath10k_htt *htt, struct htt_rx_info *info)
 	struct ieee80211_hdr *hdr;
 	enum rx_msdu_decap_format fmt;
 	enum htt_rx_mpdu_encrypt_type enctype;
+	u8 addr[ETH_ALEN];
+	int hdr_len;
+	void *rfc1042;
 
 	/* This shouldn't happen. If it does than it may be a FW bug. */
 	if (skb->next) {
@@ -700,48 +716,44 @@ static void ath10k_htt_rx_msdu(struct ath10k_htt *htt, struct htt_rx_info *info)
 			RX_MSDU_START_INFO1_DECAP_FORMAT);
 	enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
 			RX_MPDU_START_INFO0_ENCRYPT_TYPE);
-	hdr = (void *)skb->data - RX_HTT_HDR_STATUS_LEN;
+	hdr = (struct ieee80211_hdr *)rxd->rx_hdr_status;
+	hdr_len = ieee80211_hdrlen(hdr->frame_control);
 
 	skb->ip_summed = ath10k_htt_rx_get_csum_state(skb);
 
 	switch (fmt) {
 	case RX_MSDU_DECAP_RAW:
-		/* remove trailing FCS */
-		skb_trim(skb, skb->len - 4);
+		skb_trim(skb, skb->len - FCS_LEN);
 		break;
 	case RX_MSDU_DECAP_NATIVE_WIFI:
-		/* nothing to do here */
+		hdr = (struct ieee80211_hdr *)skb->data;
+		hdr_len = ieee80211_hdrlen(hdr->frame_control);
+		memcpy(addr, ieee80211_get_DA(hdr), ETH_ALEN);
+		skb_pull(skb, hdr_len);
+
+		hdr = (struct ieee80211_hdr *)rxd->rx_hdr_status;
+		hdr_len = ieee80211_hdrlen(hdr->frame_control);
+		memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
+
+		hdr = (struct ieee80211_hdr *)skb->data;
+		memcpy(ieee80211_get_DA(hdr), addr, ETH_ALEN);
 		break;
 	case RX_MSDU_DECAP_ETHERNET2_DIX:
-		/* macaddr[6] + macaddr[6] + ethertype[2] */
-		skb_pull(skb, 6 + 6 + 2);
+		rfc1042 = hdr;
+		rfc1042 += roundup(hdr_len, 4);
+		rfc1042 += roundup(ath10k_htt_rx_crypto_param_len(enctype), 4);
+
+		skb_pull(skb, sizeof(struct ethhdr));
+		memcpy(skb_push(skb, sizeof(struct rfc1042_hdr)),
+		       rfc1042, sizeof(struct rfc1042_hdr));
+		memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
 		break;
 	case RX_MSDU_DECAP_8023_SNAP_LLC:
-		/* macaddr[6] + macaddr[6] + len[2] */
-		/* we don't need this for non-A-MSDU */
-		skb_pull(skb, 6 + 6 + 2);
+		skb_pull(skb, sizeof(struct amsdu_subframe_hdr));
+		memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
 		break;
 	}
 
-	if (fmt == RX_MSDU_DECAP_ETHERNET2_DIX) {
-		void *llc;
-		int llclen;
-
-		llclen = 8;
-		llc  = hdr;
-		llc += roundup(ieee80211_hdrlen(hdr->frame_control), 4);
-		llc += roundup(ath10k_htt_rx_crypto_param_len(enctype), 4);
-
-		skb_push(skb, llclen);
-		memcpy(skb->data, llc, llclen);
-	}
-
-	if (fmt >= RX_MSDU_DECAP_ETHERNET2_DIX) {
-		int len = ieee80211_hdrlen(hdr->frame_control);
-		skb_push(skb, len);
-		memcpy(skb->data, hdr, len);
-	}
-
 	info->skb = skb;
 	info->encrypt_type = enctype;
 
-- 
1.7.9.5


^ permalink raw reply related

* [RFC 1/4] ath10k: report A-MSDU subframes individually
From: Michal Kazior @ 2013-09-16 12:49 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior
In-Reply-To: <1379335757-15180-1-git-send-email-michal.kazior@tieto.com>

HW reports each A-MSDU subframe as a separate
sk_buff. It is impossible to configure it to
behave differently.

Until now ath10k was reconstructing A-MSDUs from
subframes which involved a lot of memory
operations. This proved to be a significant
contributor to degraded RX performance.

This at least doubles RX performance on AP135.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt.h    |    1 +
 drivers/net/wireless/ath/ath10k/htt_rx.c |  189 ++++++++++--------------------
 drivers/net/wireless/ath/ath10k/txrx.c   |    5 +
 3 files changed, 71 insertions(+), 124 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index e090902..9bcff3e 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1181,6 +1181,7 @@ struct htt_rx_info {
 		u32 info2;
 	} rate;
 	bool fcs_err;
+	bool amsdu_more;
 };
 
 struct ath10k_htt {
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index a39fbf4..dad584f 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -41,6 +41,10 @@
 /* when under memory pressure rx ring refill may fail and needs a retry */
 #define HTT_RX_RING_REFILL_RETRY_MS 50
 
+
+static int ath10k_htt_rx_get_csum_state(struct sk_buff *skb);
+
+
 static int ath10k_htt_rx_ring_size(struct ath10k_htt *htt)
 {
 	int size;
@@ -591,136 +595,92 @@ static bool ath10k_htt_rx_hdr_is_amsdu(struct ieee80211_hdr *hdr)
 	return false;
 }
 
-static int ath10k_htt_rx_amsdu(struct ath10k_htt *htt,
-			struct htt_rx_info *info)
+struct rfc1042_hdr {
+	u8 llc_dsap;
+	u8 llc_ssap;
+	u8 llc_ctrl;
+	u8 snap_oui[3];
+	__be16 snap_type;
+} __packed;
+
+struct amsdu_subframe_hdr {
+	u8 dst[ETH_ALEN];
+	u8 src[ETH_ALEN];
+	__be16 len;
+} __packed;
+
+static void ath10k_htt_rx_amsdu(struct ath10k_htt *htt,
+				struct htt_rx_info *info)
 {
 	struct htt_rx_desc *rxd;
-	struct sk_buff *amsdu;
 	struct sk_buff *first;
-	struct ieee80211_hdr *hdr;
 	struct sk_buff *skb = info->skb;
 	enum rx_msdu_decap_format fmt;
 	enum htt_rx_mpdu_encrypt_type enctype;
+	struct ieee80211_hdr *hdr;
+	u8 hdr_buf[64];
 	unsigned int hdr_len;
-	int crypto_len;
 
 	rxd = (void *)skb->data - sizeof(*rxd);
-	fmt = MS(__le32_to_cpu(rxd->msdu_start.info1),
-			RX_MSDU_START_INFO1_DECAP_FORMAT);
 	enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
 			RX_MPDU_START_INFO0_ENCRYPT_TYPE);
 
-	/* FIXME: No idea what assumptions are safe here. Need logs */
-	if ((fmt == RX_MSDU_DECAP_RAW && skb->next)) {
-		ath10k_htt_rx_free_msdu_chain(skb->next);
-		skb->next = NULL;
-		return -ENOTSUPP;
-	}
-
-	/* A-MSDU max is a little less than 8K */
-	amsdu = dev_alloc_skb(8*1024);
-	if (!amsdu) {
-		ath10k_warn("A-MSDU allocation failed\n");
-		ath10k_htt_rx_free_msdu_chain(skb->next);
-		skb->next = NULL;
-		return -ENOMEM;
-	}
-
-	if (fmt >= RX_MSDU_DECAP_NATIVE_WIFI) {
-		int hdrlen;
-
-		hdr = (void *)rxd->rx_hdr_status;
-		hdrlen = ieee80211_hdrlen(hdr->frame_control);
-		memcpy(skb_put(amsdu, hdrlen), hdr, hdrlen);
-	}
+	hdr = (void *)rxd->rx_hdr_status;
+	hdr_len = ieee80211_hdrlen(hdr->frame_control);
+	memcpy(hdr_buf, hdr, hdr_len);
 
 	first = skb;
 	while (skb) {
 		void *decap_hdr;
-		int decap_len = 0;
+		int decap_len;
 
 		rxd = (void *)skb->data - sizeof(*rxd);
 		fmt = MS(__le32_to_cpu(rxd->msdu_start.info1),
-				RX_MSDU_START_INFO1_DECAP_FORMAT);
+			 RX_MSDU_START_INFO1_DECAP_FORMAT);
 		decap_hdr = (void *)rxd->rx_hdr_status;
 
-		if (skb == first) {
-			/* We receive linked A-MSDU subframe skbuffs. The
-			 * first one contains the original 802.11 header (and
-			 * possible crypto param) in the RX descriptor. The
-			 * A-MSDU subframe header follows that. Each part is
-			 * aligned to 4 byte boundary. */
-
-			hdr = (void *)amsdu->data;
-			hdr_len = ieee80211_hdrlen(hdr->frame_control);
-			crypto_len = ath10k_htt_rx_crypto_param_len(enctype);
-
-			decap_hdr += roundup(hdr_len, 4);
-			decap_hdr += roundup(crypto_len, 4);
-		}
-
-		/* When fmt == RX_MSDU_DECAP_8023_SNAP_LLC:
-		 *
-		 * SNAP 802.3 consists of:
-		 * [dst:6][src:6][len:2][dsap:1][ssap:1][ctl:1][snap:5]
-		 * [data][fcs:4].
-		 *
-		 * Since this overlaps with A-MSDU header (da, sa, len)
-		 * there's nothing extra to do. */
-
-		if (fmt == RX_MSDU_DECAP_ETHERNET2_DIX) {
-			/* Ethernet2 decap inserts ethernet header in place of
-			 * A-MSDU subframe header. */
-			skb_pull(skb, 6 + 6 + 2);
-
-			/* A-MSDU subframe header length */
-			decap_len += 6 + 6 + 2;
-
-			/* Ethernet2 decap also strips the LLC/SNAP so we need
-			 * to re-insert it. The LLC/SNAP follows A-MSDU
-			 * subframe header. */
-			/* FIXME: Not all LLCs are 8 bytes long */
-			decap_len += 8;
-
-			memcpy(skb_put(amsdu, decap_len), decap_hdr, decap_len);
-		}
-
-		if (fmt == RX_MSDU_DECAP_NATIVE_WIFI) {
-			/* Native Wifi decap inserts regular 802.11 header
-			 * in place of A-MSDU subframe header. */
-			hdr = (struct ieee80211_hdr *)skb->data;
-			skb_pull(skb, ieee80211_hdrlen(hdr->frame_control));
+		skb->ip_summed = ath10k_htt_rx_get_csum_state(skb);
 
-			/* A-MSDU subframe header length */
-			decap_len += 6 + 6 + 2;
-
-			memcpy(skb_put(amsdu, decap_len), decap_hdr, decap_len);
+		/* First frame in an A-MSDU chain has more decapped data. */
+		if (skb == first) {
+			decap_hdr += round_up(ieee80211_hdrlen(hdr->frame_control), 4);
+			decap_hdr += round_up(ath10k_htt_rx_crypto_param_len(enctype), 4);
 		}
 
-		if (fmt == RX_MSDU_DECAP_RAW)
-			skb_trim(skb, skb->len - 4); /* remove FCS */
-
-		memcpy(skb_put(amsdu, skb->len), skb->data, skb->len);
-
-		/* A-MSDU subframes are padded to 4bytes
-		 * but relative to first subframe, not the whole MPDU */
-		if (skb->next && ((decap_len + skb->len) & 3)) {
-			int padlen = 4 - ((decap_len + skb->len) & 3);
-			memset(skb_put(amsdu, padlen), 0, padlen);
+		switch (fmt) {
+		case RX_MSDU_DECAP_RAW:
+			skb_trim(skb, skb->len - FCS_LEN);
+			break;
+		case RX_MSDU_DECAP_NATIVE_WIFI:
+			break;
+		case RX_MSDU_DECAP_ETHERNET2_DIX:
+			decap_len = 0;
+			decap_len += sizeof(struct rfc1042_hdr);
+			decap_len += sizeof(struct amsdu_subframe_hdr);
+
+			skb_pull(skb, sizeof(struct ethhdr));
+			memcpy(skb_push(skb, decap_len), decap_hdr, decap_len);
+			memcpy(skb_push(skb, hdr_len), hdr_buf, hdr_len);
+			break;
+		case RX_MSDU_DECAP_8023_SNAP_LLC:
+			memcpy(skb_push(skb, hdr_len), hdr_buf, hdr_len);
+			break;
 		}
 
+		info->skb = skb;
+		info->amsdu_more = (skb->next != NULL);
+		info->encrypt_type = enctype;
 		skb = skb->next;
-	}
+		info->skb->next = NULL;
 
-	info->skb = amsdu;
-	info->encrypt_type = enctype;
-
-	ath10k_htt_rx_free_msdu_chain(first);
+		ath10k_process_rx(htt->ar, info);
+	}
 
-	return 0;
+	/* FIXME: It might be nice to re-assemble the A-MSDU when there's a
+	 * monitor interface active for sniffing purposes. */
 }
 
-static int ath10k_htt_rx_msdu(struct ath10k_htt *htt, struct htt_rx_info *info)
+static void ath10k_htt_rx_msdu(struct ath10k_htt *htt, struct htt_rx_info *info)
 {
 	struct sk_buff *skb = info->skb;
 	struct htt_rx_desc *rxd;
@@ -742,6 +702,8 @@ static int ath10k_htt_rx_msdu(struct ath10k_htt *htt, struct htt_rx_info *info)
 			RX_MPDU_START_INFO0_ENCRYPT_TYPE);
 	hdr = (void *)skb->data - RX_HTT_HDR_STATUS_LEN;
 
+	skb->ip_summed = ath10k_htt_rx_get_csum_state(skb);
+
 	switch (fmt) {
 	case RX_MSDU_DECAP_RAW:
 		/* remove trailing FCS */
@@ -782,7 +744,8 @@ static int ath10k_htt_rx_msdu(struct ath10k_htt *htt, struct htt_rx_info *info)
 
 	info->skb = skb;
 	info->encrypt_type = enctype;
-	return 0;
+
+	ath10k_process_rx(htt->ar, info);
 }
 
 static bool ath10k_htt_rx_has_decrypt_err(struct sk_buff *skb)
@@ -854,8 +817,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	int fw_desc_len;
 	u8 *fw_desc;
 	int i, j;
-	int ret;
-	int ip_summed;
 
 	memset(&info, 0, sizeof(info));
 
@@ -930,11 +891,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 				continue;
 			}
 
-			/* The skb is not yet processed and it may be
-			 * reallocated. Since the offload is in the original
-			 * skb extract the checksum now and assign it later */
-			ip_summed = ath10k_htt_rx_get_csum_state(msdu_head);
-
 			info.skb     = msdu_head;
 			info.fcs_err = ath10k_htt_rx_has_fcs_err(msdu_head);
 			info.signal  = ATH10K_DEFAULT_NOISE_FLOOR;
@@ -947,24 +903,9 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 			hdr = ath10k_htt_rx_skb_get_hdr(msdu_head);
 
 			if (ath10k_htt_rx_hdr_is_amsdu(hdr))
-				ret = ath10k_htt_rx_amsdu(htt, &info);
+				ath10k_htt_rx_amsdu(htt, &info);
 			else
-				ret = ath10k_htt_rx_msdu(htt, &info);
-
-			if (ret && !info.fcs_err) {
-				ath10k_warn("error processing msdus %d\n", ret);
-				dev_kfree_skb_any(info.skb);
-				continue;
-			}
-
-			if (ath10k_htt_rx_hdr_is_amsdu((void *)info.skb->data))
-				ath10k_dbg(ATH10K_DBG_HTT, "htt mpdu is amsdu\n");
-
-			info.skb->ip_summed = ip_summed;
-
-			ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "htt mpdu: ",
-					info.skb->data, info.skb->len);
-			ath10k_process_rx(htt->ar, &info);
+				ath10k_htt_rx_msdu(htt, &info);
 		}
 	}
 
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index 68b6fae..ed54bce 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -261,6 +261,9 @@ void ath10k_process_rx(struct ath10k *ar, struct htt_rx_info *info)
 	if (info->fcs_err)
 		status->flag |= RX_FLAG_FAILED_FCS_CRC;
 
+	if (info->amsdu_more)
+		status->flag |= RX_FLAG_AMSDU_MORE;
+
 	status->signal = info->signal;
 
 	spin_lock_bh(&ar->data_lock);
@@ -293,6 +296,8 @@ void ath10k_process_rx(struct ath10k *ar, struct htt_rx_info *info)
 		   status->vht_nss,
 		   status->freq,
 		   status->band);
+	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "rx skb: ",
+			info->skb->data, info->skb->len);
 
 	ieee80211_rx(ar->hw, info->skb);
 }
-- 
1.7.9.5


^ permalink raw reply related

* [RFC 2/4] ath10k: document decap modes
From: Michal Kazior @ 2013-09-16 12:49 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior
In-Reply-To: <1379335757-15180-1-git-send-email-michal.kazior@tieto.com>

Clarify how each decap mode works in one place.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/rx_desc.h |   24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h b/drivers/net/wireless/ath/ath10k/rx_desc.h
index bfec6c8..1c584c4 100644
--- a/drivers/net/wireless/ath/ath10k/rx_desc.h
+++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
@@ -422,10 +422,30 @@ struct rx_mpdu_end {
 #define RX_MSDU_START_INFO1_IP_FRAG             (1 << 14)
 #define RX_MSDU_START_INFO1_TCP_ONLY_ACK        (1 << 15)
 
+/* The decapped header (rx_hdr_status) contains the following:
+ *  a) 802.11 header
+ *  [padding to 4 bytes]
+ *  b) HW crypto parameter
+ *     - 0 bytes for no security
+ *     - 4 bytes for WEP
+ *     - 8 bytes for TKIP, AES
+ *  [padding to 4 bytes]
+ *  c) A-MSDU subframe header (14 bytes) if appliable
+ *  d) LLC/SNAP (RFC1042, 8 bytes)
+ *
+ * In case of A-MSDU only first frame in sequence contains (a) and (b). */
 enum rx_msdu_decap_format {
-	RX_MSDU_DECAP_RAW           = 0,
-	RX_MSDU_DECAP_NATIVE_WIFI   = 1,
+	RX_MSDU_DECAP_RAW = 0,
+
+	/* Note: QoS frames are reported as non-QoS. The rx_hdr_status in
+	 * htt_rx_desc contains the original decapped 802.11 header. */
+	RX_MSDU_DECAP_NATIVE_WIFI = 1,
+
+	/* Payload contains an ethernet header (struct ethhdr). */
 	RX_MSDU_DECAP_ETHERNET2_DIX = 2,
+
+	/* Payload contains two 48-bit addresses and 2-byte length (14 bytes
+	 * total), followed by an RFC1042 header (8 bytes). */
 	RX_MSDU_DECAP_8023_SNAP_LLC = 3
 };
 
-- 
1.7.9.5


^ permalink raw reply related

* [RFC 0/4] ath10k: improve RX performance
From: Michal Kazior @ 2013-09-16 12:49 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Hi,

This patchset gives clear RX performance
improvement on AP135. Throughput is at least
doubled (300mbps -> 600mbps, UDP RX, 2x2).

Patches depend on my RFC patch "mac80211: support
reporting A-MSDU subframes individually".


Michal.


Michal Kazior (4):
  ath10k: report A-MSDU subframes individually
  ath10k: document decap modes
  ath10k: cleanup RX decap handling
  ath10k: align RX frames properly

 drivers/net/wireless/ath/ath10k/htt.h     |    1 +
 drivers/net/wireless/ath/ath10k/htt_rx.c  |  251 ++++++++++++-----------------
 drivers/net/wireless/ath/ath10k/hw.h      |    6 +-
 drivers/net/wireless/ath/ath10k/rx_desc.h |   24 ++-
 drivers/net/wireless/ath/ath10k/txrx.c    |    5 +
 5 files changed, 135 insertions(+), 152 deletions(-)

-- 
1.7.9.5


^ permalink raw reply

* [RFC] mac80211: support reporting A-MSDU subframes individually
From: Michal Kazior @ 2013-09-16 12:38 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Michal Kazior

Some devices may not be able to report A-MSDUs in
single buffers. Drivers for such devices were
forced to re-assemble A-MSDUs which would then
be eventually disassembled by mac80211. This could
lead to CPU cache thrashing and poor performance.

Since A-MSDU has a single sequence number all
subframes share it. This was in conflict with
retransmission/duplication recovery
(IEEE802.11-2012: 9.3.2.10).

Patch introduces a new flag that is meant to be
set for all individually reported A-MSDU subframes
except the last one. This ensures the
last_seq_ctrl is updated after the last subframe
is processed. If an A-MSDU is actually a duplicate
transmission all reported subframes will be
properly discarded.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 include/net/mac80211.h |   10 ++++++++++
 net/mac80211/rx.c      |    2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 06ffae8..f8f049d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -829,6 +829,15 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info)
  * @RX_FLAG_STBC_MASK: STBC 2 bit bitmask. 1 - Nss=1, 2 - Nss=2, 3 - Nss=3
  * @RX_FLAG_10MHZ: 10 MHz (half channel) was used
  * @RX_FLAG_5MHZ: 5 MHz (quarter channel) was used
+ * @RX_FLAG_AMSDU_MORE: Some drivers may prefer to report separate A-MSDU
+ *	subframes instead of a one huge frame for performance reasons.
+ *	All, but the last MSDU from an A-MSDU should have this flag set. E.g.
+ *	if an A-MSDU has 3 frames, the first 2 must have the flag set, while
+ *	the 3rd (last) one must not have this flag set. The flag is used to
+ *	deal with retransmission/duplication recovery properly since A-MSDU
+ *	subframes share the same sequence number. Reported subframes can be
+ *	either regular MSDU or singly A-MSDUs. Subframes must not be
+ *	interleaved with other frames.
  */
 enum mac80211_rx_flags {
 	RX_FLAG_MMIC_ERROR		= BIT(0),
@@ -859,6 +868,7 @@ enum mac80211_rx_flags {
 	RX_FLAG_STBC_MASK		= BIT(26) | BIT(27),
 	RX_FLAG_10MHZ			= BIT(28),
 	RX_FLAG_5MHZ			= BIT(29),
+	RX_FLAG_AMSDU_MORE		= BIT(30),
 };
 
 #define RX_FLAG_STBC_SHIFT		26
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 8e908e1..43bee02 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -995,7 +995,7 @@ ieee80211_rx_h_check(struct ieee80211_rx_data *rx)
 				rx->sta->num_duplicates++;
 			}
 			return RX_DROP_UNUSABLE;
-		} else
+		} else if (!(status->flag & RX_FLAG_AMSDU_MORE))
 			rx->sta->last_seq_ctrl[rx->seqno_idx] = hdr->seq_ctrl;
 	}
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] mwifiex: fix command timeout with SDIO interrupts enabled
From: Daniel Mack @ 2013-09-16 11:39 UTC (permalink / raw)
  To: linux-wireless
  Cc: s.neumann, afenkart, bzhao, linville, johannes.berg, Daniel Mack

We've been hunting a command timeout issue in the mwifiex driver which
occurs on an AM33xx platform when Andreas Fenkart's omap_hsmmc SDIO IRQ
patches are applied (they are not yet in the mainline kernel). We first
suspected the mmc host driver to be racy, but that wasn't the case.

I dug a little through the changes between 3.7 and 3.10 on the mwifiex
driver, and it turned out that the culprit is 601216e12 ("mwifiex:
process RX packets in SDIO IRQ thread directly"), especially the
situation where mwifiex_main_process() bails when mwifiex_processing is
set.

In other words: if an SDIO irq arrives while the driver is processing
mwifiex_main_process() from the work queue, we effectively loose the
interrupt, which then results in a command timeout.

I've written a little test case scenario that calls
mwifiex_dump_station_info() through the SIOCGIWSTATS ioctl excessively,
and that reproduces the problem after a couple of seconds already:

  https://gist.github.com/zonque/6579314


The fix for this issue is quite simple and works very well.


Thanks,
Daniel


Daniel Mack (1):
  mwifiex: queue main work from main process when bailing on races

 drivers/net/wireless/mwifiex/main.c | 1 +
 1 file changed, 1 insertion(+)

-- 
1.8.3.1


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox