* [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface
From: Masami Hiramatsu @ 2016-08-15 9:40 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
Pieter-Paul Giesberts, Rafał Miłecki
Cc: linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel
In-Reply-To: <147125403645.9434.8008546579326856373.stgit@devbox>
Check rtnl_lock is locked in brcmf_p2p_ifp_removed() by passing
rtnl_locked flag. Actually the caller brcmf_del_if() checks whether
the rtnl_lock is locked, but doesn't pass it to brcmf_p2p_ifp_removed().
Without this fix, wpa_supplicant goes softlockup with rtnl_lock
holding (this means all other process using netlink are locked up too)
e.g.
[ 4495.876627] INFO: task wpa_supplicant:7307 blocked for more than 10 seconds.
[ 4495.876632] Tainted: G W 4.8.0-rc1+ #8
[ 4495.876635] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 4495.876638] wpa_supplicant D ffff974c647b39a0 0 7307 1 0x00000000
[ 4495.876644] ffff974c647b39a0 0000000000000000 ffff974c00000000 ffff974c7dc59c58
[ 4495.876651] ffff974c6b7417c0 ffff974c645017c0 ffff974c647b4000 ffffffff86f16c08
[ 4495.876657] ffff974c645017c0 0000000000000246 00000000ffffffff ffff974c647b39b8
[ 4495.876664] Call Trace:
[ 4495.876671] [<ffffffff868aeccc>] schedule+0x3c/0x90
[ 4495.876676] [<ffffffff868af065>] schedule_preempt_disabled+0x15/0x20
[ 4495.876682] [<ffffffff868b0996>] mutex_lock_nested+0x176/0x3b0
[ 4495.876686] [<ffffffff867a2067>] ? rtnl_lock+0x17/0x20
[ 4495.876690] [<ffffffff867a2067>] rtnl_lock+0x17/0x20
[ 4495.876720] [<ffffffffc0ae9a5d>] brcmf_p2p_ifp_removed+0x4d/0x70 [brcmfmac]
[ 4495.876741] [<ffffffffc0aebde6>] brcmf_remove_interface+0x196/0x1b0 [brcmfmac]
[ 4495.876760] [<ffffffffc0ae9901>] brcmf_p2p_del_vif+0x111/0x220 [brcmfmac]
[ 4495.876777] [<ffffffffc0adefab>] brcmf_cfg80211_del_iface+0x21b/0x270 [brcmfmac]
[ 4495.876820] [<ffffffffc097b39e>] nl80211_del_interface+0xfe/0x3a0 [cfg80211]
[ 4495.876825] [<ffffffff867ca335>] genl_family_rcv_msg+0x1b5/0x370
[ 4495.876832] [<ffffffff860e5d8d>] ? trace_hardirqs_on+0xd/0x10
[ 4495.876836] [<ffffffff867ca56d>] genl_rcv_msg+0x7d/0xb0
[ 4495.876839] [<ffffffff867ca4f0>] ? genl_family_rcv_msg+0x370/0x370
[ 4495.876846] [<ffffffff867c9a47>] netlink_rcv_skb+0x97/0xb0
[ 4495.876849] [<ffffffff867ca168>] genl_rcv+0x28/0x40
[ 4495.876854] [<ffffffff867c93c3>] netlink_unicast+0x1d3/0x2f0
[ 4495.876860] [<ffffffff867c933b>] ? netlink_unicast+0x14b/0x2f0
[ 4495.876866] [<ffffffff867c97cb>] netlink_sendmsg+0x2eb/0x3a0
[ 4495.876870] [<ffffffff8676dad8>] sock_sendmsg+0x38/0x50
[ 4495.876874] [<ffffffff8676e4df>] ___sys_sendmsg+0x27f/0x290
[ 4495.876882] [<ffffffff8628b935>] ? mntput_no_expire+0x5/0x3f0
[ 4495.876888] [<ffffffff8628b9be>] ? mntput_no_expire+0x8e/0x3f0
[ 4495.876894] [<ffffffff8628b935>] ? mntput_no_expire+0x5/0x3f0
[ 4495.876899] [<ffffffff8628bd44>] ? mntput+0x24/0x40
[ 4495.876904] [<ffffffff86267830>] ? __fput+0x190/0x200
[ 4495.876909] [<ffffffff8676f125>] __sys_sendmsg+0x45/0x80
[ 4495.876914] [<ffffffff8676f172>] SyS_sendmsg+0x12/0x20
[ 4495.876918] [<ffffffff868b5680>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 4495.876924] [<ffffffff860e2b8f>] ? trace_hardirqs_off_caller+0x1f/0xc0
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +-
.../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 8 +++++---
.../net/wireless/broadcom/brcm80211/brcmfmac/p2p.h | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 8d16f02..65e8c87 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -743,7 +743,7 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx,
* serious troublesome side effects. The p2p module will clean
* up the ifp if needed.
*/
- brcmf_p2p_ifp_removed(ifp);
+ brcmf_p2p_ifp_removed(ifp, rtnl_locked);
kfree(ifp);
}
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 66f942f..de19c7c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2297,7 +2297,7 @@ int brcmf_p2p_del_vif(struct wiphy *wiphy, struct wireless_dev *wdev)
return err;
}
-void brcmf_p2p_ifp_removed(struct brcmf_if *ifp)
+void brcmf_p2p_ifp_removed(struct brcmf_if *ifp, bool rtnl_locked)
{
struct brcmf_cfg80211_info *cfg;
struct brcmf_cfg80211_vif *vif;
@@ -2306,9 +2306,11 @@ void brcmf_p2p_ifp_removed(struct brcmf_if *ifp)
vif = ifp->vif;
cfg = wdev_to_cfg(&vif->wdev);
cfg->p2p.bss_idx[P2PAPI_BSSCFG_DEVICE].vif = NULL;
- rtnl_lock();
+ if (!rtnl_locked)
+ rtnl_lock();
cfg80211_unregister_wdev(&vif->wdev);
- rtnl_unlock();
+ if (!rtnl_locked)
+ rtnl_unlock();
brcmf_free_vif(vif);
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
index a3bd18c..8ce9447 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
@@ -155,7 +155,7 @@ struct wireless_dev *brcmf_p2p_add_vif(struct wiphy *wiphy, const char *name,
int brcmf_p2p_del_vif(struct wiphy *wiphy, struct wireless_dev *wdev);
int brcmf_p2p_ifchange(struct brcmf_cfg80211_info *cfg,
enum brcmf_fil_p2p_if_types if_type);
-void brcmf_p2p_ifp_removed(struct brcmf_if *ifp);
+void brcmf_p2p_ifp_removed(struct brcmf_if *ifp, bool rtnl_locked);
int brcmf_p2p_start_device(struct wiphy *wiphy, struct wireless_dev *wdev);
void brcmf_p2p_stop_device(struct wiphy *wiphy, struct wireless_dev *wdev);
int brcmf_p2p_scan_prep(struct wiphy *wiphy,
^ permalink raw reply related
* [BUGFIX PATCH 0/2] Bugfixes for brcmfmac
From: Masami Hiramatsu @ 2016-08-15 9:40 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
Pieter-Paul Giesberts, Rafał Miłecki
Cc: linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel
Hi,
Here are 2 patches for fixing bugs which I recently faced on my PC.
There are 2 bugs I've hit on brcmfmac, one issue was critical,
the other was just found when I investigated the first issue.
1) when I shutdown or reboot my pc with wifi, it always stopped
when disabling networking. I tried to just disable wifi and saw
task hung up messages on dmesg.
All those taskes were blocked on rtnl_lock according to the
stacktrace, and found a suspicious task in the list. Actually
the wpa_supplicant is blocked while stopping the interface.
2) I also tried to get more information about that and enabled
DEBUG_ATOMIC_SLEEP and got another warning in brcmfmac. That
warned a mutex (which can yeild/sleep) is held in !TASK_RUNNING
state. I've found a mutex is held when in wait_event_timeout()
condition parameter.
I traced the source code and found that #1 was caused by double
locking of rtnl_lock in brcmfmac driver, because it doesn't
check the rtnl_lock is already held in a path (actually, other
paths checked that). So I fixed it by checking rtnl_locked and
skip locking rtnl_lock. It works, but not seems the best way
to fix, since original code (rtnl_lock locking around
cfg80211_unregister_wdev) itself looks add-hoc. Anyway, since
I don't have any knowladge of this subsystem, I'd like to ask
maintainer's help.
To fix #2 issue, I've checked the mutex (vif_event_lock) in
struct brcmf_cfg80211_vif_event just protect updating other
members and can be replaced by a spinlock because in the
protected regions are not involving any scheduler related
code.
Thank you,
---
Masami Hiramatsu (2):
brcmfmac: Check rtnl_lock is locked when removing interface
brcmfmac: Change vif_event_lock to spinlock
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 26 ++++++++++----------
.../broadcom/brcm80211/brcmfmac/cfg80211.h | 2 +-
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +-
.../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 8 ++++--
.../net/wireless/broadcom/brcm80211/brcmfmac/p2p.h | 2 +-
5 files changed, 21 insertions(+), 19 deletions(-)
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* RE: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface
From: Wright Feng @ 2016-08-15 8:11 UTC (permalink / raw)
To: Kalle Valo, Arend Van Spriel
Cc: brcm80211-dev-list.pdl@broadcom.com, franky.lin@broadcom.com,
hante.meuleman@broadcom.com, pieterpg@broadcom.com, Chi-Hsien Lin,
linux-wireless@vger.kernel.org
In-Reply-To: <871t1qe9u3.fsf@kamboji.qca.qualcomm.com>
Hi Kalle and Arend,
> -----Original Message-----
> From: Kalle Valo [mailto:kvalo@codeaurora.org]
> Sent: Monday, August 15, 2016 4:05 PM
> To: Arend Van Spriel <arend.vanspriel@broadcom.com>
> Cc: Wright Feng <wefe@cypress.com>; brcm80211-dev-
> list.pdl@broadcom.com; franky.lin@broadcom.com;
> hante.meuleman@broadcom.com; pieterpg@broadcom.com; Chi-Hsien Lin
> <chln@cypress.com>; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on
> primary interface
>
> Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>
> >> This message and any attachments may contain Cypress (or its
> >> subsidiaries) confidential information. If it has been received in
> >> error, please advise the sender and immediately delete this message.
> >
> > Is there any way for you to get rid of this foot note. It may keep
> > Kalle from taking this patch.
>
> Correct. I'll automatically drop patches with these kind of notices.
I've removed the foot note and sent the [PATCH v2] to you on 8/11.
Is that okay to you?
>
> --
> Kalle Valo
This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.
^ permalink raw reply
* Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface
From: Kalle Valo @ 2016-08-15 8:24 UTC (permalink / raw)
To: Wright Feng
Cc: Arend Van Spriel, brcm80211-dev-list.pdl@broadcom.com,
franky.lin@broadcom.com, hante.meuleman@broadcom.com,
pieterpg@broadcom.com, Chi-Hsien Lin,
linux-wireless@vger.kernel.org
In-Reply-To: <DM5PR06MB28259759C91C4ACBDB719EF9D9120@DM5PR06MB2825.namprd06.prod.outlook.com>
Wright Feng <wefe@cypress.com> writes:
>> -----Original Message-----
>> From: Kalle Valo [mailto:kvalo@codeaurora.org]
>> Sent: Monday, August 15, 2016 4:05 PM
>> To: Arend Van Spriel <arend.vanspriel@broadcom.com>
>> Cc: Wright Feng <wefe@cypress.com>; brcm80211-dev-
>> list.pdl@broadcom.com; franky.lin@broadcom.com;
>> hante.meuleman@broadcom.com; pieterpg@broadcom.com; Chi-Hsien Lin
>> <chln@cypress.com>; linux-wireless@vger.kernel.org
>> Subject: Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on
>> primary interface
>>
>> Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>> >> This message and any attachments may contain Cypress (or its
>> >> subsidiaries) confidential information. If it has been received in
>> >> error, please advise the sender and immediately delete this message.
>> >
>> > Is there any way for you to get rid of this foot note. It may keep
>> > Kalle from taking this patch.
>>
>> Correct. I'll automatically drop patches with these kind of notices.
>
> I've removed the foot note and sent the [PATCH v2] to you on 8/11.
> Is that okay to you?
>From a quick look seems to be ok. But I'll do proper review later, need
to go through pile of mails after coming back from vacation.
--
Kalle Valo
^ permalink raw reply
* [PATCH 2/2 v2] wlcore: Remove wl pointer from wl_sta structure
From: Maxim Altshul @ 2016-08-15 8:23 UTC (permalink / raw)
To: linux-kernel; +Cc: Maxim Altshul, Kalle Valo, linux-wireless
This field was added to wl_sta struct to get hw in situations
where it was not given to driver by mac80211.
In our case, get_expected_throughput op did not send hw to driver.
This patch reverts the change, as it is no longer needed due to
get_expected_throughput op change (hw is now sent as a parameter)
Signed-off-by: Maxim Altshul <maxim.altshul@ti.com>
---
Changed the commit message to better explain the changey
drivers/net/wireless/ti/wlcore/main.c | 1 -
drivers/net/wireless/ti/wlcore/wlcore_i.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 1ec3545..8589e5a 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -5043,7 +5043,6 @@ static int wl12xx_sta_add(struct wl1271 *wl,
return ret;
wl_sta = (struct wl1271_station *)sta->drv_priv;
- wl_sta->wl = wl;
hlid = wl_sta->hlid;
ret = wl12xx_cmd_add_peer(wl, wlvif, sta, hlid);
diff --git a/drivers/net/wireless/ti/wlcore/wlcore_i.h b/drivers/net/wireless/ti/wlcore/wlcore_i.h
index 3875190..8ee5206 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore_i.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore_i.h
@@ -352,7 +352,6 @@ struct wl1271_station {
* Used in both AP and STA mode.
*/
u64 total_freed_pkts;
- struct wl1271 *wl;
};
struct wl12xx_vif {
--
2.9.0
^ permalink raw reply related
* Re: pull request: iwlwifi 2016-04-12
From: Kalle Valo @ 2016-08-15 8:14 UTC (permalink / raw)
To: Emmanuel Grumbach; +Cc: Grumbach, Emmanuel, linux-wireless@vger.kernel.org
In-Reply-To: <CANUX_P27s1jzXfLzBca19db10c9yo1MKQ6T9b=pw-ZQLZ0KKVQ@mail.gmail.com>
Emmanuel Grumbach <egrumbach@gmail.com> writes:
> On Sun, Aug 7, 2016 at 7:35 AM, Grumbach, Emmanuel
> <emmanuel.grumbach@intel.com> wrote:
>>
>> Hi Kalle,
>>
>> Here is a pull request for 4.6. I have here a patch that was merged to
>> -next already, but clearly, it should have been routed through the
>> current cycle's trees. Sorry about that.
>> The patch is:
>
>
> Wow.. I knew evolution was sometimes stupid, but this is new to me..
> Obviously you should ignore that.
> Sorry...
Ok, I dropped this :)
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] brcmfmac: shut down AP and set IBSS mode only on primary interface
From: Kalle Valo @ 2016-08-15 8:05 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Wright Feng, brcm80211-dev-list.pdl@broadcom.com,
franky.lin@broadcom.com, hante.meuleman@broadcom.com,
pieterpg@broadcom.com, Chi-Hsien Lin,
linux-wireless@vger.kernel.org
In-Reply-To: <cab5a204-3487-ebe6-7e82-ca6dbb04ff40@broadcom.com>
Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>> This message and any attachments may contain Cypress (or its
>> subsidiaries) confidential information. If it has been received in
>> error, please advise the sender and immediately delete this message.
>
> Is there any way for you to get rid of this foot note. It may keep Kalle
> from taking this patch.
Correct. I'll automatically drop patches with these kind of notices.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 1/2] mac80211/wlcore: Add ieee80211_hw variable to get_expected_throughput
From: Kalle Valo @ 2016-08-15 7:59 UTC (permalink / raw)
To: Maxim Altshul
Cc: linux-kernel, john.stultz, Johannes Berg, Eliad Peller,
Yaniv Machani, linux-wireless
In-Reply-To: <20160804124314.7636-2-maxim.altshul@ti.com>
Maxim Altshul <maxim.altshul@ti.com> writes:
> - The variable is added to allow the driver an easy access
> to it's own hw->priv when the op is invoked.
>
> - Change wlcore op accordingly.
>
> Signed-off-by: Maxim Altshul <maxim.altshul@ti.com>
You didn't CC linux-wireless, adding it now. Others can find the full
discussion here:
http://lkml.kernel.org/g/20160804124314.7636-2-maxim.altshul@ti.com
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 2/2] wlcore: Remove wl pointer from wl_sta structure
From: Kalle Valo @ 2016-08-15 7:56 UTC (permalink / raw)
To: Maxim Altshul
Cc: linux-kernel, john.stultz, Eliad Peller, Yaniv Machani,
linux-wireless
In-Reply-To: <20160804124314.7636-3-maxim.altshul@ti.com>
Maxim Altshul <maxim.altshul@ti.com> writes:
> No longer needed due to get_expected_throughput op change
>
> Signed-off-by: Maxim Altshul <maxim.altshul@ti.com>
The commit log is very vague, please improve it. But most importantly
you did not CC linux-wireless (adding it now) so lots of wireless people
missed this patch. Please resend.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 2/2] wlcore: Remove wl pointer from wl_sta structure
From: Kalle Valo @ 2016-08-15 7:56 UTC (permalink / raw)
To: Maxim Altshul
Cc: linux-kernel, john.stultz, Eliad Peller, Yaniv Machani,
linux-wireless
In-Reply-To: <20160804124314.7636-3-maxim.altshul@ti.com>
Maxim Altshul <maxim.altshul@ti.com> writes:
> No longer needed due to get_expected_throughput op change
>
> Signed-off-by: Maxim Altshul <maxim.altshul@ti.com>
The commit log is very vague, please improve it. But most importantly
you did not CC linux-wireless (adding it now) so lots of wireless people
missed this patch. Please resend.
--
Kalle Valo
^ permalink raw reply
* [PATCH] NFC: Delete owner assignment
From: SF Markus Elfring @ 2016-08-15 7:12 UTC (permalink / raw)
To: linux-nfc, linux-wireless, Aloisio Almeida Jr, Krzysztof Opasiak,
Lauro Ramos Venancio, Robert Baldyga, Samuel Ortiz
Cc: LKML, kernel-janitors, Julia Lawall
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 15 Aug 2016 09:00:26 +0200
The field "owner" is set by core. Thus delete an extra initialisation.
Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/nfc/nfcmrvl/i2c.c | 1 -
drivers/nfc/pn533/i2c.c | 1 -
drivers/nfc/s3fwrn5/i2c.c | 1 -
3 files changed, 3 deletions(-)
diff --git a/drivers/nfc/nfcmrvl/i2c.c b/drivers/nfc/nfcmrvl/i2c.c
index 78b7aa8..45c64a2 100644
--- a/drivers/nfc/nfcmrvl/i2c.c
+++ b/drivers/nfc/nfcmrvl/i2c.c
@@ -278,7 +278,6 @@ static struct i2c_driver nfcmrvl_i2c_driver = {
.remove = nfcmrvl_i2c_remove,
.driver = {
.name = "nfcmrvl_i2c",
- .owner = THIS_MODULE,
.of_match_table = of_match_ptr(of_nfcmrvl_i2c_match),
},
};
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index 1dc8924..90798f0 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -265,7 +265,6 @@ MODULE_DEVICE_TABLE(i2c, pn533_i2c_id_table);
static struct i2c_driver pn533_i2c_driver = {
.driver = {
.name = PN533_I2C_DRIVER_NAME,
- .owner = THIS_MODULE,
.of_match_table = of_match_ptr(of_pn533_i2c_match),
},
.probe = pn533_i2c_probe,
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index 3ed0adf..2e762c6 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -290,7 +290,6 @@ MODULE_DEVICE_TABLE(of, of_s3fwrn5_i2c_match);
static struct i2c_driver s3fwrn5_i2c_driver = {
.driver = {
- .owner = THIS_MODULE,
.name = S3FWRN5_I2C_DRIVER_NAME,
.of_match_table = of_match_ptr(of_s3fwrn5_i2c_match),
},
--
2.9.2
^ permalink raw reply related
* Re: [PATCH] Staging: rtl8723au: os_intfs: fixed case statement is variable issue
From: Johannes Berg @ 2016-08-15 6:07 UTC (permalink / raw)
To: Joe Perches, sunbing, Jes Sorensen
Cc: Larry.Finger, gregkh, linux-wireless, devel, linux-kernel,
sunbing.linux
In-Reply-To: <1471177398.5201.9.camel@perches.com>
On Sun, 2016-08-14 at 05:23 -0700, Joe Perches wrote:
>
> Maybe this test should be sparse version checked after
> sparse is updated.
*If* sparse ever gets updated :) I don't think it's been updated much
lately.
That said, I'm not even sure how, and what version, etc. so obviously
that'd have to be done after the fact. But since nobody will ever
compile the kernel with sparse's code generator, it also doesn't matter
anyway.
johannes
^ permalink raw reply
* Re: [PATCH v2] RANDOM: ATH9K RNG delivers zero bits of entropy
From: Jason Cooper @ 2016-08-14 18:11 UTC (permalink / raw)
To: Theodore Ts'o, Pan, Miaoqing, Stephan Mueller,
Sepehrdad, Pouyan, herbert@gondor.apana.org.au,
linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
ath9k-devel, linux-wireless@vger.kernel.org,
ath9k-devel@lists.ath9k.org, Kalle Valo
In-Reply-To: <20160810234425.GG10523@thunk.org>
Hey Ted,
On Wed, Aug 10, 2016 at 07:44:25PM -0400, Theodore Ts'o wrote:
> On Tue, Aug 09, 2016 at 02:04:44PM +0000, Jason Cooper wrote:
> > iiuc, Ted, you're saying using the hw_random framework would be
> > disasterous because despite most drivers having a default quality of 0,
> > rngd assumes 1 bit of entropy for every bit read?
>
> Sorry, what I was trying to say (but failed) was that bypassing the
> hwrng framework and injecting entropy directly the entropy pool was
> disatrous.
Ok, whew. :)
> > Thankfully, most hw_random drivers don't set the quality. So unless the
> > user sets the default_quality param, it's zero.
>
> The fact that this is "most" and not "all" does scare me a little.
My recent grep showed that only virtio-rng set it to a non-zero value.
> As far as I'm concerned *all* hw_random drivers should set quality to
> zero, since it should be up to the system administrator.
Agreed.
Gathering conversation about this from a few related threads, I have one
concern. Apparently there is some confusion in userspace consumers of
/dev/hwrng data as to the quality of it. Specifically, rngd (spotted by
Stephan Mueller) appears to assume 1bit of entropy per 1 bit read. :-/
So, while moving ath9k-rng to the hwrng framework makes complete sense
internally, it's not so good for existing userspace assumptions. I'd
think that timeriomem-rng falls in this same category.
In light of this, do you think it's worth the effort (I'm volunteering)
to create a subcategory of hwrng drivers that are 'environemntal' rngs?
They can contribute to the kernel entropy pools, but not to /dev/hwrng.
thx,
Jason.
^ permalink raw reply
* Re: [PATCH] Staging: rtl8723au: os_intfs: fixed case statement is variable issue
From: Joe Perches @ 2016-08-14 12:23 UTC (permalink / raw)
To: Johannes Berg, sunbing, Jes Sorensen
Cc: Larry.Finger, gregkh, linux-wireless, devel, linux-kernel,
sunbing.linux
In-Reply-To: <1471176951.5903.6.camel@sipsolutions.net>
On Sun, 2016-08-14 at 14:15 +0200, Johannes Berg wrote:
> >
> > It's a sparse defect.
> >
> > Try again after patching sparse with Jes' patch:
> > http://marc.info/?l=linux-sparse&m=147091200720267&w=3
> Mine, not Jes's ;-)
Right, sorry 'bout that.
> But there's another patch going into the kernel to fix it:
>
> https://www.ozlabs.org/~akpm/mmotm/broken-out/byteswap-dont-use-__builtin_bswap-with-sparse.patch
Thanks.
Maybe this test should be sparse version checked after
sparse is updated.
^ permalink raw reply
* Re: [PATCH] Staging: rtl8723au: os_intfs: fixed case statement is variable issue
From: Johannes Berg @ 2016-08-14 12:15 UTC (permalink / raw)
To: Joe Perches, sunbing, Jes Sorensen
Cc: Larry.Finger, gregkh, linux-wireless, devel, linux-kernel,
sunbing.linux
In-Reply-To: <1471176423.5201.7.camel@perches.com>
> It's a sparse defect.
>
> Try again after patching sparse with Jes' patch:
> http://marc.info/?l=linux-sparse&m=147091200720267&w=3
Mine, not Jes's ;-)
But there's another patch going into the kernel to fix it:
https://www.ozlabs.org/~akpm/mmotm/broken-out/byteswap-dont-use-__builtin_bswap-with-sparse.patch
johannes
^ permalink raw reply
* Re: [PATCH] Staging: rtl8723au: os_intfs: fixed case statement is variable issue
From: Joe Perches @ 2016-08-14 12:07 UTC (permalink / raw)
To: sunbing, Jes Sorensen
Cc: Larry.Finger, gregkh, linux-wireless, devel, linux-kernel,
sunbing.linux, Johannes Berg
In-Reply-To: <E0CA1BD5-A2ED-4FDC-8E58-8024E2F3C24C@redflag-linux.com>
On Sat, 2016-08-13 at 17:26 +0800, sunbing wrote:
> On Aug 12, 2016, at 22:30, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> > sunbing <sunbing@redflag-linux.com> writes:
> > > On Aug 11, 2016, at 23:25, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> > > > Bing Sun <sunbing@redflag-linux.com> writes:
> > > > >
> > > > > Fixed sparse parse error:
> > > > > Expected constant expression in case statement.
[]
> > > > Pardon me here, but I find it really hard to see how this change is an
> > > > improvement over the old code in any shape or form.
> > > There is no functional improvement.
> > > But before this patch, when we do: make C=1 M=drivers/staging/rtl8723au/
> > > An error output:
> > > drivers/staging/rtl8723au//os_dep/os_intfs.c:287:14: error: Expected
> > > constant expression in case statement
> > > To avoid sparse parse error, a case statement converts to an if statement.
> > > So we got this patch.
> > Hello
> >
> > I understand this part, but it seems to me we are changing the code due
> > to a broken test case in sparse. Does the warning go away if you use
> > __constant_htons() instead of htons()?
> >
> > Jes
> Thanks for your guidance.
>
> 1. If I use __constant_htons, checkpatch.pl will warning:
> WARNING: __constant_htons should be htons
>
> 2. In os_intfs.c: rtw_classify8021d, there are only one case statement and a
> default statement. So, convert "switch case" to "if else" is more readable in my opinion.
>
> So, I pushed this patch.
>
> There are some patches convert use of __constant_htons to htons in kernel logs.
> Will there be a new patch convert to htons in the future if I use __constant_htons now ?
>
> After search through kernel code, there are 158 "case htons(...)" statements and
> 2 "case __constant_htons(...)" statements. Does this mean we can ignore sparse
> error and use "case htons(...)" ?
>
> It makes me confused. More help, please.
It's a sparse defect.
Try again after patching sparse with Jes' patch:
http://marc.info/?l=linux-sparse&m=147091200720267&w=3
^ permalink raw reply
* Re: pull-request: mac80211-next 2016-08-12
From: David Miller @ 2016-08-13 22:11 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <1470993646-14778-1-git-send-email-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 12 Aug 2016 11:20:45 +0200
> This first pull request is pretty small, but there's no point
> in hanging on to it for long, so here it goes anyway. Nothing
> all that interesting, I think.
>
> We might have a bunch of new APIs that were promised to me
> coming up soon, for new features, but I don't know how quickly
> that's actually going to happen.
>
> Let me know if there's any problem.
Pulled, thanks a lot.
^ permalink raw reply
* Re: [PATCH 00/16] net: don't print error when allocating urb fails
From: David Miller @ 2016-08-13 21:55 UTC (permalink / raw)
To: wsa-dev
Cc: linux-usb, brcm80211-dev-list.pdl, linux-can, linux-wireless,
netdev
In-Reply-To: <1470949539-25392-1-git-send-email-wsa-dev@sang-engineering.com>
From: Wolfram Sang <wsa-dev@sang-engineering.com>
Date: Thu, 11 Aug 2016 23:05:19 +0200
> This per-subsystem series is part of a tree wide cleanup. usb_alloc_urb() uses
> kmalloc which already prints enough information on failure. So, let's simply
> remove those "allocation failed" messages from drivers like we did already for
> other -ENOMEM cases. gkh acked this approach when we talked about it at LCJ in
> Tokyo a few weeks ago.
Series applied to net-next, thank you.
^ permalink raw reply
* Re: [PATCH] Staging: rtl8723au: os_intfs: fixed case statement is variable issue
From: sunbing @ 2016-08-13 9:26 UTC (permalink / raw)
To: Jes Sorensen
Cc: Larry.Finger, gregkh, linux-wireless, devel, linux-kernel,
sunbing.linux
In-Reply-To: <wrfj7fbmawl1.fsf@redhat.com>
On Aug 12, 2016, at 22:30, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> sunbing <sunbing@redflag-linux.com> writes:
>> On Aug 11, 2016, at 23:25, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>>
>>> Bing Sun <sunbing@redflag-linux.com> writes:
>>>> Fixed sparse parse error:
>>>> Expected constant expression in case statement.
>>>>
>>>> Signed-off-by: Bing Sun <sunbing@redflag-linux.com>
>>>> ---
>>>> drivers/staging/rtl8723au/os_dep/os_intfs.c | 11 +++++------
>>>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/rtl8723au/os_dep/os_intfs.c b/drivers/staging/rtl8723au/os_dep/os_intfs.c
>>>> index b8848c2..f30d5d2 100644
>>>> --- a/drivers/staging/rtl8723au/os_dep/os_intfs.c
>>>> +++ b/drivers/staging/rtl8723au/os_dep/os_intfs.c
>>>> @@ -283,14 +283,13 @@ static u32 rtw_classify8021d(struct sk_buff *skb)
>>>> */
>>>> if (skb->priority >= 256 && skb->priority <= 263)
>>>> return skb->priority - 256;
>>>> - switch (skb->protocol) {
>>>> - case htons(ETH_P_IP):
>>>> +
>>>> + if (skb->protocol == htons(ETH_P_IP)) {
>>>> dscp = ip_hdr(skb)->tos & 0xfc;
>>>> - break;
>>>> - default:
>>>> - return 0;
>>>> + return dscp >> 5;
>>>> }
>>>> - return dscp >> 5;
>>>> +
>>>> + return 0;
>>>> }
>>>
>>> Pardon me here, but I find it really hard to see how this change is an
>>> improvement over the old code in any shape or form.
>>>
>>> Jes
>>
>> There is no functional improvement.
>> But before this patch, when we do: make C=1 M=drivers/staging/rtl8723au/
>> An error output:
>> drivers/staging/rtl8723au//os_dep/os_intfs.c:287:14: error: Expected
>> constant expression in case statement
>> To avoid sparse parse error, a case statement converts to an if statement.
>> So we got this patch.
>
> Hello
>
> I understand this part, but it seems to me we are changing the code due
> to a broken test case in sparse. Does the warning go away if you use
> __constant_htons() instead of htons()?
>
> Jes
Thanks for your guidance.
1. If I use __constant_htons, checkpatch.pl will warning:
WARNING: __constant_htons should be htons
2. In os_intfs.c: rtw_classify8021d, there are only one case statement and a
default statement. So, convert "switch case" to "if else" is more readable in my opinion.
So, I pushed this patch.
There are some patches convert use of __constant_htons to htons in kernel logs.
Will there be a new patch convert to htons in the future if I use __constant_htons now ?
After search through kernel code, there are 158 "case htons(...)" statements and
2 "case __constant_htons(...)" statements. Does this mean we can ignore sparse
error and use "case htons(...)" ?
It makes me confused. More help, please.
Regards.
^ permalink raw reply
* [PATCH v7] cfg80211: Provision to allow the support for different beacon intervals
From: Purushottam Kushwaha @ 2016-08-13 5:02 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, jouni, usdutt, amarnath, djindal, pkushwah
This commit provides a mechanism for the host drivers to advertise the
support for different beacon intervals among the respective interface
combinations in a group, through diff_beacon_int_gcd_min (u32).
Following sets the expectation for diff_beacon_int_gcd_min.
= 0 - all beacon intervals for different interfaces must be same.
> 0 - different beacon intervals must have a GCD that's at
least as big as this value.
Signed-off-by: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>
---
include/net/cfg80211.h | 9 ++++++++-
include/uapi/linux/nl80211.h | 8 ++++++--
net/wireless/core.h | 2 +-
net/wireless/nl80211.c | 13 ++++++++++---
net/wireless/util.c | 46 ++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 69 insertions(+), 9 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9c23f4d3..c3dd46c 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2939,6 +2939,11 @@ struct ieee80211_iface_limit {
* only in special cases.
* @radar_detect_widths: bitmap of channel widths supported for radar detection
* @radar_detect_regions: bitmap of regions supported for radar detection
+ * @diff_beacon_int_gcd_min: This interface combination supports different
+ * beacon intervals.
+ * = 0 - all beacon intervals for different interface must be same.
+ * > 0 - different beacon intervals must have a GCD that's at
+ * least as big as this value.
*
* With this structure the driver can describe which interface
* combinations it supports concurrently.
@@ -2959,7 +2964,7 @@ struct ieee80211_iface_limit {
* };
*
*
- * 2. Allow #{AP, P2P-GO} <= 8, channels = 1, 8 total:
+ * 2. Allow #{AP, P2P-GO} <= 8, diff BI min gcd = 10, channels = 1, 8 total:
*
* struct ieee80211_iface_limit limits2[] = {
* { .max = 8, .types = BIT(NL80211_IFTYPE_AP) |
@@ -2970,6 +2975,7 @@ struct ieee80211_iface_limit {
* .n_limits = ARRAY_SIZE(limits2),
* .max_interfaces = 8,
* .num_different_channels = 1,
+ * .diff_beacon_int_gcd_min = 10,
* };
*
*
@@ -2997,6 +3003,7 @@ struct ieee80211_iface_combination {
bool beacon_int_infra_match;
u8 radar_detect_widths;
u8 radar_detect_regions;
+ u32 diff_beacon_int_gcd_min;
};
struct ieee80211_txrx_stypes {
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 2206941..894d131 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4203,6 +4203,9 @@ enum nl80211_iface_limit_attrs {
* of supported channel widths for radar detection.
* @NL80211_IFACE_COMB_RADAR_DETECT_REGIONS: u32 attribute containing the bitmap
* of supported regulatory regions for radar detection.
+ * @NL80211_IFACE_COMB_DIFF_BI_GCD_MIN: u32 attribute specifying the minimum GCD
+ * of different beacon intervals supported by all the interface combinations
+ * in this group (if not present, all beacon interval must match).
* @NUM_NL80211_IFACE_COMB: number of attributes
* @MAX_NL80211_IFACE_COMB: highest attribute number
*
@@ -4210,8 +4213,8 @@ enum nl80211_iface_limit_attrs {
* limits = [ #{STA} <= 1, #{AP} <= 1 ], matching BI, channels = 1, max = 2
* => allows an AP and a STA that must match BIs
*
- * numbers = [ #{AP, P2P-GO} <= 8 ], channels = 1, max = 8
- * => allows 8 of AP/GO
+ * numbers = [ #{AP, P2P-GO} <= 8 ], diff BI min gcd, channels = 1, max = 8,
+ * => allows 8 of AP/GO that can have BI gcd >= min gcd
*
* numbers = [ #{STA} <= 2 ], channels = 2, max = 2
* => allows two STAs on different channels
@@ -4237,6 +4240,7 @@ enum nl80211_if_combination_attrs {
NL80211_IFACE_COMB_NUM_CHANNELS,
NL80211_IFACE_COMB_RADAR_DETECT_WIDTHS,
NL80211_IFACE_COMB_RADAR_DETECT_REGIONS,
+ NL80211_IFACE_COMB_DIFF_BI_GCD_MIN,
/* keep last */
NUM_NL80211_IFACE_COMB,
diff --git a/net/wireless/core.h b/net/wireless/core.h
index eee9144..5fffe58 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -475,7 +475,7 @@ int ieee80211_get_ratemask(struct ieee80211_supported_band *sband,
u32 *mask);
int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
- u32 beacon_int);
+ enum nl80211_iftype iftype, u32 beacon_int);
void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
enum nl80211_iftype iftype, int num);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4997857..c1741d9 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1020,6 +1020,10 @@ static int nl80211_put_iface_combinations(struct wiphy *wiphy,
nla_put_u32(msg, NL80211_IFACE_COMB_RADAR_DETECT_REGIONS,
c->radar_detect_regions)))
goto nla_put_failure;
+ if (c->diff_beacon_int_gcd_min &&
+ nla_put_u32(msg, NL80211_IFACE_COMB_DIFF_BI_GCD_MIN,
+ c->diff_beacon_int_gcd_min))
+ goto nla_put_failure;
nla_nest_end(msg, nl_combi);
}
@@ -3433,7 +3437,8 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
params.dtim_period =
nla_get_u32(info->attrs[NL80211_ATTR_DTIM_PERIOD]);
- err = cfg80211_validate_beacon_int(rdev, params.beacon_interval);
+ err = cfg80211_validate_beacon_int(rdev, dev->ieee80211_ptr->iftype,
+ params.beacon_interval);
if (err)
return err;
@@ -7768,7 +7773,8 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
ibss.beacon_interval =
nla_get_u32(info->attrs[NL80211_ATTR_BEACON_INTERVAL]);
- err = cfg80211_validate_beacon_int(rdev, ibss.beacon_interval);
+ err = cfg80211_validate_beacon_int(rdev, NL80211_IFTYPE_ADHOC,
+ ibss.beacon_interval);
if (err)
return err;
@@ -9245,7 +9251,8 @@ static int nl80211_join_mesh(struct sk_buff *skb, struct genl_info *info)
setup.beacon_interval =
nla_get_u32(info->attrs[NL80211_ATTR_BEACON_INTERVAL]);
- err = cfg80211_validate_beacon_int(rdev, setup.beacon_interval);
+ err = cfg80211_validate_beacon_int(rdev, NL80211_IFTYPE_MESH_POINT,
+ setup.beacon_interval);
if (err)
return err;
}
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 0675f51..844f02a 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1553,20 +1553,62 @@ bool ieee80211_chandef_to_operating_class(struct cfg80211_chan_def *chandef,
}
EXPORT_SYMBOL(ieee80211_chandef_to_operating_class);
+struct diff_beacon_int {
+ u32 gcd;
+ bool valid;
+};
+
+static void
+cfg80211_validate_diff_beacon_int(const struct ieee80211_iface_combination *c,
+ void *data)
+{
+ struct diff_beacon_int *diff_bi = data;
+
+ if (c->diff_beacon_int_gcd_min &&
+ (diff_bi->gcd >= c->diff_beacon_int_gcd_min))
+ diff_bi->valid = true;
+}
+
int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
- u32 beacon_int)
+ enum nl80211_iftype iftype, u32 beacon_int)
{
struct wireless_dev *wdev;
int res = 0;
+ int iftype_num[NUM_NL80211_IFTYPES];
if (beacon_int < 10 || beacon_int > 10000)
return -EINVAL;
+ memset(iftype_num, 0, sizeof(iftype_num));
+ list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
+ if (!wdev->beacon_interval)
+ continue;
+ iftype_num[wdev->iftype]++;
+ }
+ iftype_num[iftype]++;
+
list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
if (!wdev->beacon_interval)
continue;
if (wdev->beacon_interval != beacon_int) {
- res = -EINVAL;
+ struct diff_beacon_int diff_bi = {
+ wdev->beacon_interval,
+ false,
+ };
+
+ /* Get the GCD */
+ while (beacon_int != 0) {
+ u32 tmp_bi = beacon_int;
+ beacon_int = diff_bi.gcd % beacon_int;
+ diff_bi.gcd = tmp_bi;
+ }
+
+ res = cfg80211_iter_combinations(&rdev->wiphy, 0, 0, iftype_num,
+ cfg80211_validate_diff_beacon_int,
+ &diff_bi);
+ if (res)
+ return res;
+ res = (diff_bi.valid) ? 0 : -EINVAL;
break;
}
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/2] etherdevice: add is_zero_ether_addr_unaligned()
From: Joe Perches @ 2016-08-13 4:59 UTC (permalink / raw)
To: Petri Gynther; +Cc: linux-wireless, kvalo, David Miller, Amitkumar Karwar
In-Reply-To: <CAGXr9JHXdQxObUJC_=4ZaJKmvFS+QygK8ytMtXCJJws62a8S7A@mail.gmail.com>
On Fri, 2016-08-12 at 21:40 -0700, Petri Gynther wrote:
> But, we need to return true (1) when the bitwise OR result is zero.
> Same logic as in is_zero_ether_addr().
right.
^ permalink raw reply
* Re: [PATCH 1/2] etherdevice: add is_zero_ether_addr_unaligned()
From: Petri Gynther @ 2016-08-13 4:40 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-wireless, kvalo, David Miller, Amitkumar Karwar
In-Reply-To: <1471061519.3467.7.camel@perches.com>
On Fri, Aug 12, 2016 at 9:11 PM, Joe Perches <joe@perches.com> wrote:
> On Fri, 2016-08-12 at 19:59 -0700, Petri Gynther wrote:
>> Add a generic routine to test if possibly unaligned to u16
>> Ethernet address is a zero address.
> []
>> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> []
>> +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]) == 0;
>> +#endif
>
> Because the return is bool, the == 0 is unnecessary.
>
But, we need to return true (1) when the bitwise OR result is zero.
Same logic as in is_zero_ether_addr().
^ permalink raw reply
* Re: [PATCH 1/2] etherdevice: add is_zero_ether_addr_unaligned()
From: Joe Perches @ 2016-08-13 4:11 UTC (permalink / raw)
To: Petri Gynther, linux-wireless; +Cc: kvalo, davem, akarwar
In-Reply-To: <1471057200-58166-1-git-send-email-pgynther@google.com>
On Fri, 2016-08-12 at 19:59 -0700, Petri Gynther wrote:
> Add a generic routine to test if possibly unaligned to u16
> Ethernet address is a zero address.
[]
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
[]
> +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]) == 0;
> +#endif
Because the return is bool, the == 0 is unnecessary.
^ permalink raw reply
* Re: [PATCH] Modify is_zero_ether_addr() to handle byte-aligned addresses
From: David Miller @ 2016-08-13 4:05 UTC (permalink / raw)
To: joe; +Cc: pgynther, linux-wireless, kvalo, akarwar
In-Reply-To: <1471060827.3467.5.camel@perches.com>
From: Joe Perches <joe@perches.com>
Date: Fri, 12 Aug 2016 21:00:27 -0700
> One solution might be to copy a hardware interface structure
> to another struct with appropriate alignment.
Right, especially if this is a slow path that would be an appropriate
way to handle this.
^ permalink raw reply
* Re: [PATCH] Modify is_zero_ether_addr() to handle byte-aligned addresses
From: Joe Perches @ 2016-08-13 4:00 UTC (permalink / raw)
To: David Miller, pgynther; +Cc: linux-wireless, kvalo, akarwar
In-Reply-To: <20160812.202613.197744153887748230.davem@davemloft.net>
On Fri, 2016-08-12 at 20:26 -0700, David Miller wrote:
> From: Petri Gynther <pgynther@google.com>
> Date: Fri, 12 Aug 2016 17:15:30 -0700
>
> > Root cause:
> > mwifiex driver calls is_zero_ether_addr() against byte-aligned address:
>
> MAC addresses really must be 16-bit aligned to be used with any of the
> ethernet address manipulation and test interfaces.
>
> Therefore this driver should do whatever it takes to avoid passing
> a byte-aligned MAC address anywhere.
>
> We _SHOULD NOT_ add support for byte aligned addresses to these
> interfaces, nor add routines which by-name can handle them.
My recollection is that batman uses unaligned addresses
and some form of _unaligned tests are required there.
It seems that other wireless drivers also use _unaligned
in various forms.
> Fix this driver instead of polluting out common interfaces
> unnecessarily.
One solution might be to copy a hardware interface structure
to another struct with appropriate alignment.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox