Hi Joe,
On Fri, Aug 12, 2016 at 5:28 PM, Joe Perches <joe@xxxxxxxxxxx> wrote:
> On Fri, 2016-08-12 at 17:15 -0700, Petri Gynther wrote:
>> $ iwconfig mlan0 essid MySSID
>> [ 36.930000] Path: /sbin/iwconfig
>> [ 36.930000] CPU: 0 PID: 203 Comm: iwconfig Not tainted 4.7.0 #2
>> [ 36.940000] task: 866f83a0 ti: 866a6000 task.ti: 866a6000
>> [ 36.940000]
>> [ECR ]: 0x00230400 => Misaligned r/w from 0x8677f403
>> [ 36.960000] [EFA ]: 0x8677f403
>> [ 36.960000] [BLINK ]: mwifiex_scan_networks+0x17a/0x198c [mwifiex]
>> [ 36.960000] [ERET ]: mwifiex_scan_networks+0x18a/0x198c [mwifiex]
>> [ 36.980000] [STAT32]: 0x00000206 : K E2 E1
>> [ 36.980000] BTA: 0x700736e2 SP: 0x866a7d0c FP: 0x5faddc84
>> [ 37.000000] LPS: 0x806a37ec LPE: 0x806a37fa LPC: 0x00000000
>> [ 37.000000] r00: 0x8677f401 r01: 0x8668aa08 r02: 0x00000001
>> r03: 0x00000000 r04: 0x8668b600 r05: 0x8677f406
>> r06: 0x8702b600 r07: 0x00000000 r08: 0x8702b600
>> r09: 0x00000000 r10: 0x870b3b00 r11: 0x00000000
>> r12: 0x00000000
>> [ 37.040000]
>> [ 37.040000] Stack Trace:
>> [ 37.040000] mwifiex_scan_networks+0x18a/0x198c [mwifiex]
>>
>> Root cause:
>> mwifiex driver calls is_zero_ether_addr() against byte-aligned address:
>>
>> drivers/net/wireless/marvell/mwifiex/fw.h:
>> struct mwifiex_scan_cmd_config {
>> /*
>> * BSS mode to be sent in the firmware command
>> */
>> u8 bss_mode;
>>
>> /* Specific BSSID used to filter scan results in the firmware */
>> u8 specific_bssid[ETH_ALEN];
>>
>> ...
>> } __packed;
>>
>> drivers/net/wireless/marvell/mwifiex/scan.c:
>> mwifiex_config_scan(..., struct mwifiex_scan_cmd_config *scan_cfg_out, ...)
>> ...
>> if (adapter->ext_scan &&
>> !is_zero_ether_addr(scan_cfg_out->specific_bssid)) {
>> ...
>> }
>>
>> Since firmware-related struct mwifiex_scan_cmd_config cannot be changed,
>> modify is_zero_ether_addr() to handle byte-aligned addresses.
>
> Or add is_zero_ether_addr_unaligned and use it here.
>
>> Cc: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
>> Cc: David S. Miller <davem@xxxxxxxxxxxxx>
>> Cc: Joe Perches <joe@xxxxxxxxxxx>
>> Cc: Amitkumar Karwar <akarwar@xxxxxxxxxxx>
>> Signed-off-by: Petri Gynther <pgynther@xxxxxxxxxx>
>> ---
>> include/linux/etherdevice.h | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
>> index 37ff4a6..0cfd243 100644
>> --- a/include/linux/etherdevice.h
>> +++ b/include/linux/etherdevice.h
>> @@ -90,11 +90,13 @@ static inline bool is_link_local_ether_addr(const u8 *addr)
>> * @addr: Pointer to a six-byte array containing the Ethernet address
>> *
>> * Return true if the address is all zeroes.
>> - *
>> - * Please note: addr must be aligned to u16.
>> */
>> static inline bool is_zero_ether_addr(const u8 *addr)
>> {
>> + if ((u32)addr & 0x1)
>> + return (addr[0] | addr[1] | addr[2] | addr[3] | addr[4] |
>> + addr[5]) == 0;
>> +
>
> So why skip the CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> optimization below?
>
>> #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
>> return ((*(const u32 *)addr) | (*(const u16 *)(addr + 4))) == 0;
>> #else
>
> How about adding:
>
> static inline bool is_zero_ether_addr_unaligned(const u8 *addr)
> {
> #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> return is_zero_ether_addr(addr);
> #else
> return addr[0] | addr[1] | addr[2] | addr[3] | addr[4] | addr[5];
> #endif
> }
>
I like your idea. I'll add is_zero_ether_addr_unaligned() and use it
in mwifiex driver.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
