* [PATCH] ieee80211: add definition for interworking support
From: Bing Zhao @ 2013-07-27 0:02 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg, Avinash Patil, Bing Zhao
From: Avinash Patil <patila@marvell.com>
IEEE802.11u interworking support is advertised via extended
capabilities IE bit 31. This is 7th bit of 4th byte of extended
capabilities.
Signed-off-by: Avinash Patil <patila@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
include/linux/ieee80211.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index b0dc87a..22c9094 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1860,6 +1860,11 @@ enum ieee80211_tdls_actioncode {
WLAN_TDLS_DISCOVERY_REQUEST = 10,
};
+/* Interworking capabilities are set in 7th bit of 4th byte of the
+ * @WLAN_EID_EXT_CAPABILITY information element
+ */
+#define WLAN_EXT_CAPA4_INTERWORKING_ENABLED BIT(7)
+
/*
* TDLS capabililites to be enabled in the 5th byte of the
* @WLAN_EID_EXT_CAPABILITY information element
--
1.8.0
^ permalink raw reply related
* [PATCH] mac80211: ibss - remove not authorized station earlier
From: Antonio Quartulli @ 2013-07-26 19:15 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Antonio Quartulli
From: Antonio Quartulli <antonio@open-mesh.com>
A station which is not authorized has to be purged earlier
to give it a chance to re-try to establish an IBSS/RSN
session soon. Set the timeout to 10 seconds.
Some refactoring has also been done to allow the IBSS
submodule to have its own expiring function.
Reported-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
net/mac80211/ibss.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 5e6836c..e08387c 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -30,6 +30,7 @@
#define IEEE80211_IBSS_MERGE_INTERVAL (30 * HZ)
#define IEEE80211_IBSS_INACTIVITY_LIMIT (60 * HZ)
+#define IEEE80211_IBSS_RSN_INACTIVITY_LIMIT (10 * HZ)
#define IEEE80211_IBSS_MAX_STA_ENTRIES 128
@@ -740,6 +741,33 @@ static int ieee80211_sta_active_ibss(struct ieee80211_sub_if_data *sdata)
return active;
}
+static void ieee80211_ibss_sta_expire(struct ieee80211_sub_if_data *sdata)
+{
+ struct ieee80211_local *local = sdata->local;
+ struct sta_info *sta, *tmp;
+ unsigned long exp_time = IEEE80211_IBSS_INACTIVITY_LIMIT;
+ unsigned long exp_rsn_time = IEEE80211_IBSS_RSN_INACTIVITY_LIMIT;
+
+ mutex_lock(&local->sta_mtx);
+
+ list_for_each_entry_safe(sta, tmp, &local->sta_list, list) {
+ if (sdata != sta->sdata)
+ continue;
+
+ if (time_after(jiffies, sta->last_rx + exp_time) ||
+ (time_after(jiffies, sta->last_rx + exp_rsn_time) &&
+ sta->sta_state != IEEE80211_STA_AUTHORIZED)) {
+ sta_dbg(sta->sdata, "expiring inactive %sSTA %pM\n",
+ sta->sta_state != IEEE80211_STA_AUTHORIZED ?
+ "not authorized " : "", sta->sta.addr);
+
+ WARN_ON(__sta_info_destroy(sta));
+ }
+ }
+
+ mutex_unlock(&local->sta_mtx);
+}
+
/*
* This function is called with state == IEEE80211_IBSS_MLME_JOINED
*/
@@ -754,7 +782,7 @@ static void ieee80211_sta_merge_ibss(struct ieee80211_sub_if_data *sdata)
mod_timer(&ifibss->timer,
round_jiffies(jiffies + IEEE80211_IBSS_MERGE_INTERVAL));
- ieee80211_sta_expire(sdata, IEEE80211_IBSS_INACTIVITY_LIMIT);
+ ieee80211_ibss_sta_expire(sdata);
if (time_before(jiffies, ifibss->last_scan_completed +
IEEE80211_IBSS_MERGE_INTERVAL))
--
1.8.1.5
^ permalink raw reply related
* [PATCH] [PATCH] mac80211: handle VHT radiotap info for tx status
From: Karl Beldan @ 2013-07-26 17:58 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Karl Beldan, Karl Beldan
From: Karl Beldan <karl.beldan@rivierawaves.com>
The radiotap VHT info is 12 bytes (required to be aligned on 2) :
u16 known - IEEE80211_RADIOTAP_VHT_KNOWN_*
u8 flags - IEEE80211_RADIOTAP_VHT_FLAG_*
u8 bandwidth
u8 mcs_nss[4]
u8 coding
u8 group_id
u16 partial_aid
ATM mac80211 can properly handle IEEE80211_RADIOTAP_VHT_KNOWN_GI,
IEEE80211_RADIOTAP_VHT_KNOWN_BANDWIDTH and mcs_nss[0] (i.e single user).
Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
---
net/mac80211/status.c | 76 ++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 63 insertions(+), 13 deletions(-)
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 6ad4c14..1a1c7fd 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -235,7 +235,8 @@ static int ieee80211_tx_radiotap_len(struct ieee80211_tx_info *info)
/* IEEE80211_RADIOTAP_RATE rate */
if (info->status.rates[0].idx >= 0 &&
- !(info->status.rates[0].flags & IEEE80211_TX_RC_MCS))
+ !(info->status.rates[0].flags & (IEEE80211_TX_RC_MCS |
+ IEEE80211_TX_RC_VHT_MCS)))
len += 2;
/* IEEE80211_RADIOTAP_TX_FLAGS */
@@ -244,16 +245,21 @@ static int ieee80211_tx_radiotap_len(struct ieee80211_tx_info *info)
/* IEEE80211_RADIOTAP_DATA_RETRIES */
len += 1;
- /* IEEE80211_TX_RC_MCS */
- if (info->status.rates[0].idx >= 0 &&
- info->status.rates[0].flags & IEEE80211_TX_RC_MCS)
- len += 3;
+ /* IEEE80211_RADIOTAP_MCS
+ * IEEE80211_RADIOTAP_VHT */
+ if (info->status.rates[0].idx >= 0) {
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_MCS)
+ len += 3;
+ else if (info->status.rates[0].flags & IEEE80211_TX_RC_VHT_MCS)
+ len += ALIGN(len, 2) + 12;
+ }
return len;
}
static void
-ieee80211_add_tx_radiotap_header(struct ieee80211_supported_band *sband,
+ieee80211_add_tx_radiotap_header(struct ieee80211_local *local,
+ struct ieee80211_supported_band *sband,
struct sk_buff *skb, int retry_count,
int rtap_len, int shift)
{
@@ -280,7 +286,8 @@ ieee80211_add_tx_radiotap_header(struct ieee80211_supported_band *sband,
/* IEEE80211_RADIOTAP_RATE */
if (info->status.rates[0].idx >= 0 &&
- !(info->status.rates[0].flags & IEEE80211_TX_RC_MCS)) {
+ !(info->status.rates[0].flags & (IEEE80211_TX_RC_MCS |
+ IEEE80211_TX_RC_VHT_MCS))) {
u16 rate;
rthdr->it_present |= cpu_to_le32(1 << IEEE80211_RADIOTAP_RATE);
@@ -310,9 +317,12 @@ ieee80211_add_tx_radiotap_header(struct ieee80211_supported_band *sband,
*pos = retry_count;
pos++;
- /* IEEE80211_TX_RC_MCS */
- if (info->status.rates[0].idx >= 0 &&
- info->status.rates[0].flags & IEEE80211_TX_RC_MCS) {
+ if (info->status.rates[0].idx < 0)
+ return;
+
+ /* IEEE80211_RADIOTAP_MCS
+ * IEEE80211_RADIOTAP_VHT */
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_MCS) {
rthdr->it_present |= cpu_to_le32(1 << IEEE80211_RADIOTAP_MCS);
pos[0] = IEEE80211_RADIOTAP_MCS_HAVE_MCS |
IEEE80211_RADIOTAP_MCS_HAVE_GI |
@@ -325,8 +335,48 @@ ieee80211_add_tx_radiotap_header(struct ieee80211_supported_band *sband,
pos[1] |= IEEE80211_RADIOTAP_MCS_FMT_GF;
pos[2] = info->status.rates[0].idx;
pos += 3;
- }
+ } else if (info->status.rates[0].flags & IEEE80211_TX_RC_VHT_MCS) {
+ u16 known = local->hw.radiotap_vht_details &
+ (IEEE80211_RADIOTAP_VHT_KNOWN_GI |
+ IEEE80211_RADIOTAP_VHT_KNOWN_BANDWIDTH);
+
+ rthdr->it_present |= cpu_to_le32(1 << IEEE80211_RADIOTAP_VHT);
+
+ /* required alignment from rthdr */
+ pos = (u8 *)rthdr + ALIGN(pos - (u8 *)rthdr, 2);
+ /* u16 known - IEEE80211_RADIOTAP_VHT_KNOWN_* */
+ put_unaligned_le16(known, pos);
+ pos += 2;
+
+ /* u8 flags - IEEE80211_RADIOTAP_VHT_FLAG_* */
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_SHORT_GI)
+ *pos |= IEEE80211_RADIOTAP_VHT_FLAG_SGI;
+ pos++;
+
+ /* u8 bandwidth */
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_40_MHZ_WIDTH)
+ *pos = 1;
+ else if (info->status.rates[0].flags & IEEE80211_TX_RC_80_MHZ_WIDTH)
+ *pos = 4;
+ else if (info->status.rates[0].flags & IEEE80211_TX_RC_160_MHZ_WIDTH)
+ *pos = 11;
+ else /* IEEE80211_TX_RC_20_MHZ_WIDTH */
+ *pos = 0;
+ pos++;
+
+ /* u8 mcs_nss[4] */
+ *pos = (ieee80211_rate_get_vht_mcs(&info->status.rates[0]) << 4) |
+ ieee80211_rate_get_vht_nss(&info->status.rates[0]);
+ pos += 4;
+
+ /* u8 coding */
+ pos++;
+ /* u8 group_id */
+ pos++;
+ /* u16 partial_aid */
+ pos += 2;
+ }
}
static void ieee80211_report_used_skb(struct ieee80211_local *local,
@@ -631,8 +681,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
dev_kfree_skb(skb);
return;
}
- ieee80211_add_tx_radiotap_header(sband, skb, retry_count, rtap_len,
- shift);
+ ieee80211_add_tx_radiotap_header(local, sband, skb, retry_count,
+ rtap_len, shift);
/* XXX: is this sufficient for BPF? */
skb_set_mac_header(skb, 0);
--
1.8.2
^ permalink raw reply related
* Re: [WT PATCH 4/6] mac80211: Add per-sdata station hash, and sdata hash.
From: Felix Fietkau @ 2013-07-26 17:59 UTC (permalink / raw)
To: Ben Greear; +Cc: Johannes Berg, linux-wireless
In-Reply-To: <51F29F1C.9000607@candelatech.com>
On 2013-07-26 6:09 PM, Ben Greear wrote:
> On 07/26/2013 08:38 AM, Felix Fietkau wrote:
>> The global hash (with added vif-addr mixing) not only completely fixes
>> the many-STA-vif case, also has some other advantages compared to the
>> per-sdata hash:
>> - Lookup is easier in setups with multiple AP VLANs
>> - Better cache footprint (especially important for small embedded devices).
>> - You don't need a separate sdata lookup before the sta lookup.
>>
>> I'm not convinced that keeping separate hashes is cleaner. Especially in
>> the AP_VLAN case, ownership is not clear in any way, since there's some
>> overlap between multiple sdata entities (belonging to the same BSS).
> If someone wants to post such a patch, we can run it through our test
> rigs, but I have little time or interest for re-doing the
> hashing code again at this time. If your approach does fix the performance
> issues we saw, then I'll be more than happy to drop my patch and use
> your method.
I don't have time to create such a patch myself at this point. I just
want to make sure that changes you post don't negatively affect small
embedded devices - and this is where the per-sdata hashing could be
problematic in my opinion.
- Felix
^ permalink raw reply
* Re: [PATCH 3.11] genetlink: release cb_lock before requesting additional module
From: Pravin Shelar @ 2013-07-26 17:08 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: David S. Miller, linux-wireless, linville, netdev, Thomas Graf,
Stephen Hemminger, rjones, Marcel Holtmann, Jeff Layton
In-Reply-To: <20130726090010.GA1756@redhat.com>
On Fri, Jul 26, 2013 at 2:00 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Requesting external module with cb_lock taken can result in
> the deadlock like showed below:
>
> [ 2458.111347] Showing all locks held in the system:
> [ 2458.111347] 1 lock held by NetworkManager/582:
> [ 2458.111347] #0: (cb_lock){++++++}, at: [<ffffffff8162bc79>] genl_rcv+0x19/0x40
> [ 2458.111347] 1 lock held by modprobe/603:
> [ 2458.111347] #0: (cb_lock){++++++}, at: [<ffffffff8162baa5>] genl_lock_all+0x15/0x30
>
> [ 2461.579457] SysRq : Show Blocked State
> [ 2461.580103] task PC stack pid father
> [ 2461.580103] NetworkManager D ffff880034b84500 4040 582 1 0x00000080
> [ 2461.580103] ffff8800197ff720 0000000000000046 00000000001d5340 ffff8800197fffd8
> [ 2461.580103] ffff8800197fffd8 00000000001d5340 ffff880019631700 7fffffffffffffff
> [ 2461.580103] ffff8800197ff880 ffff8800197ff878 ffff880019631700 ffff880019631700
> [ 2461.580103] Call Trace:
> [ 2461.580103] [<ffffffff817355f9>] schedule+0x29/0x70
> [ 2461.580103] [<ffffffff81731ad1>] schedule_timeout+0x1c1/0x360
> [ 2461.580103] [<ffffffff810e69eb>] ? mark_held_locks+0xbb/0x140
> [ 2461.580103] [<ffffffff817377ac>] ? _raw_spin_unlock_irq+0x2c/0x50
> [ 2461.580103] [<ffffffff810e6b6d>] ? trace_hardirqs_on_caller+0xfd/0x1c0
> [ 2461.580103] [<ffffffff81736398>] wait_for_completion_killable+0xe8/0x170
> [ 2461.580103] [<ffffffff810b7fa0>] ? wake_up_state+0x20/0x20
> [ 2461.580103] [<ffffffff81095825>] call_usermodehelper_exec+0x1a5/0x210
> [ 2461.580103] [<ffffffff817362ed>] ? wait_for_completion_killable+0x3d/0x170
> [ 2461.580103] [<ffffffff81095cc3>] __request_module+0x1b3/0x370
> [ 2461.580103] [<ffffffff810e6b6d>] ? trace_hardirqs_on_caller+0xfd/0x1c0
> [ 2461.580103] [<ffffffff8162c5c9>] ctrl_getfamily+0x159/0x190
> [ 2461.580103] [<ffffffff8162d8a4>] genl_family_rcv_msg+0x1f4/0x2e0
> [ 2461.580103] [<ffffffff8162d990>] ? genl_family_rcv_msg+0x2e0/0x2e0
> [ 2461.580103] [<ffffffff8162da1e>] genl_rcv_msg+0x8e/0xd0
> [ 2461.580103] [<ffffffff8162b729>] netlink_rcv_skb+0xa9/0xc0
> [ 2461.580103] [<ffffffff8162bc88>] genl_rcv+0x28/0x40
> [ 2461.580103] [<ffffffff8162ad6d>] netlink_unicast+0xdd/0x190
> [ 2461.580103] [<ffffffff8162b149>] netlink_sendmsg+0x329/0x750
> [ 2461.580103] [<ffffffff815db849>] sock_sendmsg+0x99/0xd0
> [ 2461.580103] [<ffffffff810bb58f>] ? local_clock+0x5f/0x70
> [ 2461.580103] [<ffffffff810e96e8>] ? lock_release_non_nested+0x308/0x350
> [ 2461.580103] [<ffffffff815dbc6e>] ___sys_sendmsg+0x39e/0x3b0
> [ 2461.580103] [<ffffffff810565af>] ? kvm_clock_read+0x2f/0x50
> [ 2461.580103] [<ffffffff810218b9>] ? sched_clock+0x9/0x10
> [ 2461.580103] [<ffffffff810bb2bd>] ? sched_clock_local+0x1d/0x80
> [ 2461.580103] [<ffffffff810bb448>] ? sched_clock_cpu+0xa8/0x100
> [ 2461.580103] [<ffffffff810e33ad>] ? trace_hardirqs_off+0xd/0x10
> [ 2461.580103] [<ffffffff810bb58f>] ? local_clock+0x5f/0x70
> [ 2461.580103] [<ffffffff810e3f7f>] ? lock_release_holdtime.part.28+0xf/0x1a0
> [ 2461.580103] [<ffffffff8120fec9>] ? fget_light+0xf9/0x510
> [ 2461.580103] [<ffffffff8120fe0c>] ? fget_light+0x3c/0x510
> [ 2461.580103] [<ffffffff815dd1d2>] __sys_sendmsg+0x42/0x80
> [ 2461.580103] [<ffffffff815dd222>] SyS_sendmsg+0x12/0x20
> [ 2461.580103] [<ffffffff81741ad9>] system_call_fastpath+0x16/0x1b
> [ 2461.580103] modprobe D ffff88000f2c8000 4632 603 602 0x00000080
> [ 2461.580103] ffff88000f04fba8 0000000000000046 00000000001d5340 ffff88000f04ffd8
> [ 2461.580103] ffff88000f04ffd8 00000000001d5340 ffff8800377d4500 ffff8800377d4500
> [ 2461.580103] ffffffff81d0b260 ffffffff81d0b268 ffffffff00000000 ffffffff81d0b2b0
> [ 2461.580103] Call Trace:
> [ 2461.580103] [<ffffffff817355f9>] schedule+0x29/0x70
> [ 2461.580103] [<ffffffff81736d4d>] rwsem_down_write_failed+0xed/0x1a0
> [ 2461.580103] [<ffffffff810bb200>] ? update_cpu_load_active+0x10/0xb0
> [ 2461.580103] [<ffffffff8137b473>] call_rwsem_down_write_failed+0x13/0x20
> [ 2461.580103] [<ffffffff8173492d>] ? down_write+0x9d/0xb2
> [ 2461.580103] [<ffffffff8162baa5>] ? genl_lock_all+0x15/0x30
> [ 2461.580103] [<ffffffff8162baa5>] genl_lock_all+0x15/0x30
> [ 2461.580103] [<ffffffff8162cbb3>] genl_register_family+0x53/0x1f0
> [ 2461.580103] [<ffffffffa01dc000>] ? 0xffffffffa01dbfff
> [ 2461.580103] [<ffffffff8162d650>] genl_register_family_with_ops+0x20/0x80
> [ 2461.580103] [<ffffffffa01dc000>] ? 0xffffffffa01dbfff
> [ 2461.580103] [<ffffffffa017fe84>] nl80211_init+0x24/0xf0 [cfg80211]
> [ 2461.580103] [<ffffffffa01dc000>] ? 0xffffffffa01dbfff
> [ 2461.580103] [<ffffffffa01dc043>] cfg80211_init+0x43/0xdb [cfg80211]
> [ 2461.580103] [<ffffffff810020fa>] do_one_initcall+0xfa/0x1b0
> [ 2461.580103] [<ffffffff8105cb93>] ? set_memory_nx+0x43/0x50
> [ 2461.580103] [<ffffffff810f75af>] load_module+0x1c6f/0x27f0
> [ 2461.580103] [<ffffffff810f2c90>] ? store_uevent+0x40/0x40
> [ 2461.580103] [<ffffffff810f82c6>] SyS_finit_module+0x86/0xb0
> [ 2461.580103] [<ffffffff81741ad9>] system_call_fastpath+0x16/0x1b
> [ 2461.580103] Sched Debug Version: v0.10, 3.11.0-0.rc1.git4.1.fc20.x86_64 #1
>
> Problem start to happen after adding net-pf-16-proto-16-family-nl80211
> alias name to cfg80211 module by below commit (though that commit
> itself is perfectly fine):
>
> commit fb4e156886ce6e8309e912d8b370d192330d19d3
> Author: Marcel Holtmann <marcel@holtmann.org>
> Date: Sun Apr 28 16:22:06 2013 -0700
>
> nl80211: Add generic netlink module alias for cfg80211/nl80211
>
> Reported-and-tested-by: Jeff Layton <jlayton@redhat.com>
> Reported-by: Richard W.M. Jones <rjones@redhat.com>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
> net/netlink/genetlink.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
> index 2fd6dbe..1076fe1 100644
> --- a/net/netlink/genetlink.c
> +++ b/net/netlink/genetlink.c
> @@ -877,8 +877,10 @@ static int ctrl_getfamily(struct sk_buff *skb, struct genl_info *info)
> #ifdef CONFIG_MODULES
> if (res == NULL) {
> genl_unlock();
> + up_read(&cb_lock);
> request_module("net-pf-%d-proto-%d-family-%s",
> PF_NETLINK, NETLINK_GENERIC, name);
> + down_read(&cb_lock);
> genl_lock();
> res = genl_family_find_byname(name);
> }
This is genl issue and it was introduced by commit def3117493eafd
(genl: Allow concurrent genl callbacks.).
Reviewed-by: Pravin B Shelar <pshelar@nicira.com>
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [WT PATCH 4/6] mac80211: Add per-sdata station hash, and sdata hash.
From: Ben Greear @ 2013-07-26 16:09 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Johannes Berg, linux-wireless
In-Reply-To: <51F297F8.5090608@openwrt.org>
On 07/26/2013 08:38 AM, Felix Fietkau wrote:
> On 2013-07-26 5:22 PM, Ben Greear wrote:
>> On 07/26/2013 02:56 AM, Felix Fietkau wrote:
>>> On 2013-07-26 10:53 AM, Johannes Berg wrote:
>>>> On Thu, 2013-07-11 at 08:29 -0700, Ben Greear wrote:
>>>>
>>>>>> I also don't like maintaining two separate hash tables and all that.
>>>>>>
>>>>>> I'd reconsider if you actually remove the hash entirely, but that'll be
>>>>>> tricky to walk the station list and will quite possibly make the RX path
>>>>>> there more expensive?
>>>>>
>>>>> Remove local->sta_hash ?
>>>>
>>>> To be honest, I'm undecided. Yes, I was thinking that, but I also think
>>>> having a huge hashtable like that for each virtual interface is way
>>>> overkill, in particular for station interfaces that usually have one
>>>> peer (the AP) and maybe a few TDLS peers. Or P2P-Device interfaces that
>>>> have no peers at all ...
>>>>
>>>> I don't see a good way to improve the hash either, since we don't always
>>>> (e.g. in RX path) have the interface address.
>>> How about mixing in the interface address into the hash. Theoretically
>>> you should always have that available, even in the rx path. Multicast
>>> data packets contain the BSSID, so you can get the address from there.
>>> You just need to be careful about checking the DS bits to figure out
>>> which address to use ;)
>>> I think this is a much better solution than duplicating the hash, or
>>> moving it into sdata entirely.
>>
>> I think I could probably get rid of the big global per wiphy hash and
>> use the per-wiphy sdata-hash and per-sdata station hashes.
>>
>> To me, that is cleanest because it gives a nice ownership relationship
>> between wiphy, sdata, and stations.
>>
>> For what it's worth, my hashing scheme has been working well on highly
>> loaded APs and Station machines.
> The global hash (with added vif-addr mixing) not only completely fixes
> the many-STA-vif case, also has some other advantages compared to the
> per-sdata hash:
> - Lookup is easier in setups with multiple AP VLANs
> - Better cache footprint (especially important for small embedded devices).
> - You don't need a separate sdata lookup before the sta lookup.
>
> I'm not convinced that keeping separate hashes is cleaner. Especially in
> the AP_VLAN case, ownership is not clear in any way, since there's some
> overlap between multiple sdata entities (belonging to the same BSS).
If someone wants to post such a patch, we can run it through our test
rigs, but I have little time or interest for re-doing the
hashing code again at this time. If your approach does fix the performance
issues we saw, then I'll be more than happy to drop my patch and use
your method.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [WT PATCH 4/6] mac80211: Add per-sdata station hash, and sdata hash.
From: Felix Fietkau @ 2013-07-26 15:38 UTC (permalink / raw)
To: Ben Greear; +Cc: Johannes Berg, linux-wireless
In-Reply-To: <51F29439.3050607@candelatech.com>
On 2013-07-26 5:22 PM, Ben Greear wrote:
> On 07/26/2013 02:56 AM, Felix Fietkau wrote:
>> On 2013-07-26 10:53 AM, Johannes Berg wrote:
>>> On Thu, 2013-07-11 at 08:29 -0700, Ben Greear wrote:
>>>
>>>>> I also don't like maintaining two separate hash tables and all that.
>>>>>
>>>>> I'd reconsider if you actually remove the hash entirely, but that'll be
>>>>> tricky to walk the station list and will quite possibly make the RX path
>>>>> there more expensive?
>>>>
>>>> Remove local->sta_hash ?
>>>
>>> To be honest, I'm undecided. Yes, I was thinking that, but I also think
>>> having a huge hashtable like that for each virtual interface is way
>>> overkill, in particular for station interfaces that usually have one
>>> peer (the AP) and maybe a few TDLS peers. Or P2P-Device interfaces that
>>> have no peers at all ...
>>>
>>> I don't see a good way to improve the hash either, since we don't always
>>> (e.g. in RX path) have the interface address.
>> How about mixing in the interface address into the hash. Theoretically
>> you should always have that available, even in the rx path. Multicast
>> data packets contain the BSSID, so you can get the address from there.
>> You just need to be careful about checking the DS bits to figure out
>> which address to use ;)
>> I think this is a much better solution than duplicating the hash, or
>> moving it into sdata entirely.
>
> I think I could probably get rid of the big global per wiphy hash and
> use the per-wiphy sdata-hash and per-sdata station hashes.
>
> To me, that is cleanest because it gives a nice ownership relationship
> between wiphy, sdata, and stations.
>
> For what it's worth, my hashing scheme has been working well on highly
> loaded APs and Station machines.
The global hash (with added vif-addr mixing) not only completely fixes
the many-STA-vif case, also has some other advantages compared to the
per-sdata hash:
- Lookup is easier in setups with multiple AP VLANs
- Better cache footprint (especially important for small embedded devices).
- You don't need a separate sdata lookup before the sta lookup.
I'm not convinced that keeping separate hashes is cleaner. Especially in
the AP_VLAN case, ownership is not clear in any way, since there's some
overlap between multiple sdata entities (belonging to the same BSS).
- Felix
^ permalink raw reply
* Re: [WT PATCH 4/6] mac80211: Add per-sdata station hash, and sdata hash.
From: Ben Greear @ 2013-07-26 15:27 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1374828780.8248.24.camel@jlt4.sipsolutions.net>
On 07/26/2013 01:53 AM, Johannes Berg wrote:
> On Thu, 2013-07-11 at 08:29 -0700, Ben Greear wrote:
>
>>> I also don't like maintaining two separate hash tables and all that.
>>>
>>> I'd reconsider if you actually remove the hash entirely, but that'll be
>>> tricky to walk the station list and will quite possibly make the RX path
>>> there more expensive?
>>
>> Remove local->sta_hash ?
>
> To be honest, I'm undecided. Yes, I was thinking that, but I also think
> having a huge hashtable like that for each virtual interface is way
> overkill, in particular for station interfaces that usually have one
> peer (the AP) and maybe a few TDLS peers. Or P2P-Device interfaces that
> have no peers at all ...
>
> I don't see a good way to improve the hash either, since we don't always
> (e.g. in RX path) have the interface address.
>
> The basic problem really is that the hash now is designed to work well
> for more regular use cases than yours, where you talk to any number of
> different stations but degrades really badly when you talk only to a
> single one many times. That use case is really special, and I don't want
> to 'fix' that in a way that would make the other use case significantly
> worse in memory consumption or CPU utilisation.
I could make the hash size configurable I suppose, or just make it always
be small for stations and larger for AP interfaces. That should
mitigate the memory usage issues. The sdata hash in the wiphy can
remain big, but there are rarely more than a few wiphy in a system, so
I think the cost is low for that.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [WT PATCH 4/6] mac80211: Add per-sdata station hash, and sdata hash.
From: Ben Greear @ 2013-07-26 15:22 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Johannes Berg, linux-wireless
In-Reply-To: <51F247C1.3090702@openwrt.org>
On 07/26/2013 02:56 AM, Felix Fietkau wrote:
> On 2013-07-26 10:53 AM, Johannes Berg wrote:
>> On Thu, 2013-07-11 at 08:29 -0700, Ben Greear wrote:
>>
>>>> I also don't like maintaining two separate hash tables and all that.
>>>>
>>>> I'd reconsider if you actually remove the hash entirely, but that'll be
>>>> tricky to walk the station list and will quite possibly make the RX path
>>>> there more expensive?
>>>
>>> Remove local->sta_hash ?
>>
>> To be honest, I'm undecided. Yes, I was thinking that, but I also think
>> having a huge hashtable like that for each virtual interface is way
>> overkill, in particular for station interfaces that usually have one
>> peer (the AP) and maybe a few TDLS peers. Or P2P-Device interfaces that
>> have no peers at all ...
>>
>> I don't see a good way to improve the hash either, since we don't always
>> (e.g. in RX path) have the interface address.
> How about mixing in the interface address into the hash. Theoretically
> you should always have that available, even in the rx path. Multicast
> data packets contain the BSSID, so you can get the address from there.
> You just need to be careful about checking the DS bits to figure out
> which address to use ;)
> I think this is a much better solution than duplicating the hash, or
> moving it into sdata entirely.
I think I could probably get rid of the big global per wiphy hash and
use the per-wiphy sdata-hash and per-sdata station hashes.
To me, that is cleanest because it gives a nice ownership relationship
between wiphy, sdata, and stations.
For what it's worth, my hashing scheme has been working well on highly
loaded APs and Station machines.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: TP-LINK wireless USB adapter don't work.
From: Sedat Dilek @ 2013-07-26 15:05 UTC (permalink / raw)
To: atar; +Cc: Oleksij Rempel, linux-wireless
In-Reply-To: <CA+icZUVbHkKz7VLs-NoVf47S_fUF855aho0CSGG3p5skpRDNmg@mail.gmail.com>
On Fri, Jul 26, 2013 at 5:00 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Fri, Jul 26, 2013 at 4:22 PM, atar <atar.yosef@gmail.com> wrote:
>> Thank you about your answer, but how can I install the firmware? which
>> processes are need to be taken in order to accomplish the firmware
>> installation and configuration?
>>
>
> Please, do not top-post.
>
> You should get a bit familiar with your operating system.
> This mailing-list is no $distro support-channel.
>
> We now have Debian/wheezy (v7.x)!
> You should upgrade (check release-notes and upgrade-process).
> Can't say off-hand which of the firmware-package you need, just check
> by yourself [1] (wild speculation: install firmware-linux-nonfree) or
> consult one of Debian's WiFi wiki-pages.
> Or ask on Debian's IRC channel for users - it's sometimes faster.
>
If you insist on Debian/squeeze you can activate squeeze-backports
software-archive (or download it directly and install manually).
As these are only firmware-blobs you might take a higher version of the package.
- Sedat -
[1] http://packages.debian.org/search?keywords=firmware-atheros
[2] http://packages.debian.org/squeeze-backports/firmware-atheros
> - Sedat -
>
> [1] http://packages.debian.org/search?keywords=firmware
>
>> Thanks in advance!!
>>
>> atar.
>>
>>> Am 25.07.2013 18:47, schrieb atar:
>>>>
>>>> Hi there!!
>>>>
>>>>
>>>> I've a TP-LINK USB wireless adapter with model number TL-WN722N. When I
>>>> plug it in to one of the USB slots in my PC, its LED diode isn't
>>>> flashing (although under WIN XP it is flashing) and I cannot use it.
>>>> from the messages of the 'dmesg' command it seems that the kernel detect
>>>> it, but I unable to use it because it doesn't appear on the wicd window
>>>> of available interfaces.
>>>>
>>>> My Linux distro is Debian squeeze with kernel 2.6.32-5-686.
>>>>
>>>> What should I do in order to solve this problem?
>>>
>>>
>>> It is ath9k-htc based adapter. Please update your kernel and use latest
>>> firmware. I think Debian squeeze do not provided this firmware, so you
>>> should install it manually.
>>>
>>>
>> --
>>
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: TP-LINK wireless USB adapter don't work.
From: Sedat Dilek @ 2013-07-26 15:00 UTC (permalink / raw)
To: atar; +Cc: Oleksij Rempel, linux-wireless
In-Reply-To: <op.w0t16tbme4gg2u@xp>
On Fri, Jul 26, 2013 at 4:22 PM, atar <atar.yosef@gmail.com> wrote:
> Thank you about your answer, but how can I install the firmware? which
> processes are need to be taken in order to accomplish the firmware
> installation and configuration?
>
Please, do not top-post.
You should get a bit familiar with your operating system.
This mailing-list is no $distro support-channel.
We now have Debian/wheezy (v7.x)!
You should upgrade (check release-notes and upgrade-process).
Can't say off-hand which of the firmware-package you need, just check
by yourself [1] (wild speculation: install firmware-linux-nonfree) or
consult one of Debian's WiFi wiki-pages.
Or ask on Debian's IRC channel for users - it's sometimes faster.
- Sedat -
[1] http://packages.debian.org/search?keywords=firmware
> Thanks in advance!!
>
> atar.
>
>> Am 25.07.2013 18:47, schrieb atar:
>>>
>>> Hi there!!
>>>
>>>
>>> I've a TP-LINK USB wireless adapter with model number TL-WN722N. When I
>>> plug it in to one of the USB slots in my PC, its LED diode isn't
>>> flashing (although under WIN XP it is flashing) and I cannot use it.
>>> from the messages of the 'dmesg' command it seems that the kernel detect
>>> it, but I unable to use it because it doesn't appear on the wicd window
>>> of available interfaces.
>>>
>>> My Linux distro is Debian squeeze with kernel 2.6.32-5-686.
>>>
>>> What should I do in order to solve this problem?
>>
>>
>> It is ath9k-htc based adapter. Please update your kernel and use latest
>> firmware. I think Debian squeeze do not provided this firmware, so you
>> should install it manually.
>>
>>
> --
>
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* changing dev->needed_headroom/needed_tailroom?
From: Johannes Berg @ 2013-07-26 14:50 UTC (permalink / raw)
To: netdev; +Cc: linux-wireless
Does it seem reasonable to change dev->needed_headroom and
dev->needed_tailroom on the fly?
We currently set needed_headroom to the max of what we need, but we
could do better like making it depend on the interface type (e.g. only
asking for mesh space on mesh interfaces). This would be done only when
the interface isn't connected, I can't promise it would be down but the
carrier would be off.
Another thing we might want to do is change it according to the
currently configured crypto (this would also affect
dev->needed_tailroom) since we actually only need tailroom when TKIP is
used. This could might be done on the fly, but could also be done when
the carrier is still down during connection establishment (which would
not be a complete optimisation but still be better than what we have
now)
Thoughts?
johannes
^ permalink raw reply
* Re: [PATCH 35/50] media: usb: cx231xx: spin_lock in complete() cleanup
From: Hans Verkuil @ 2013-07-26 14:28 UTC (permalink / raw)
To: Ming Lei
Cc: Greg Kroah-Hartman, linux-usb, Oliver Neukum, Alan Stern,
linux-input, linux-bluetooth, netdev, linux-wireless, linux-media,
alsa-devel, Mauro Carvalho Chehab, Hans Verkuil
In-Reply-To: <1373533573-12272-36-git-send-email-ming.lei@canonical.com>
On 07/11/2013 11:05 AM, Ming Lei wrote:
> Complete() will be run with interrupt enabled, so change to
> spin_lock_irqsave().
>
> Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> drivers/media/usb/cx231xx/cx231xx-audio.c | 6 ++++++
> drivers/media/usb/cx231xx/cx231xx-core.c | 10 ++++++----
> drivers/media/usb/cx231xx/cx231xx-vbi.c | 5 +++--
> 3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/usb/cx231xx/cx231xx-audio.c b/drivers/media/usb/cx231xx/cx231xx-audio.c
> index 81a1d97..58c1b5c 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-audio.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-audio.c
> @@ -136,6 +136,7 @@ static void cx231xx_audio_isocirq(struct urb *urb)
> stride = runtime->frame_bits >> 3;
>
> for (i = 0; i < urb->number_of_packets; i++) {
> + unsigned long flags;
> int length = urb->iso_frame_desc[i].actual_length /
> stride;
> cp = (unsigned char *)urb->transfer_buffer +
> @@ -158,6 +159,7 @@ static void cx231xx_audio_isocirq(struct urb *urb)
> length * stride);
> }
>
> + local_irq_save(flags);
> snd_pcm_stream_lock(substream);
Can't you use snd_pcm_stream_lock_irqsave here?
Ditto for the other media drivers where this happens: em28xx and tlg2300.
I've reviewed the media driver changes and they look OK to me, so if
my comment above is fixed, then I can merge them for 3.12. Or are these
changes required for 3.11?
Regards,
Hans
>
> dev->adev.hwptr_done_capture += length;
> @@ -174,6 +176,7 @@ static void cx231xx_audio_isocirq(struct urb *urb)
> period_elapsed = 1;
> }
> snd_pcm_stream_unlock(substream);
> + local_irq_restore(flags);
> }
> if (period_elapsed)
> snd_pcm_period_elapsed(substream);
> @@ -224,6 +227,7 @@ static void cx231xx_audio_bulkirq(struct urb *urb)
> stride = runtime->frame_bits >> 3;
>
> if (1) {
> + unsigned long flags;
> int length = urb->actual_length /
> stride;
> cp = (unsigned char *)urb->transfer_buffer;
> @@ -242,6 +246,7 @@ static void cx231xx_audio_bulkirq(struct urb *urb)
> length * stride);
> }
>
> + local_irq_save(flags);
> snd_pcm_stream_lock(substream);
>
> dev->adev.hwptr_done_capture += length;
> @@ -258,6 +263,7 @@ static void cx231xx_audio_bulkirq(struct urb *urb)
> period_elapsed = 1;
> }
> snd_pcm_stream_unlock(substream);
> + local_irq_restore(flags);
> }
> if (period_elapsed)
> snd_pcm_period_elapsed(substream);
> diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c
> index 4ba3ce0..593b397 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-core.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-core.c
> @@ -798,6 +798,7 @@ static void cx231xx_isoc_irq_callback(struct urb *urb)
> container_of(dma_q, struct cx231xx_video_mode, vidq);
> struct cx231xx *dev = container_of(vmode, struct cx231xx, video_mode);
> int i;
> + unsigned long flags;
>
> switch (urb->status) {
> case 0: /* success */
> @@ -813,9 +814,9 @@ static void cx231xx_isoc_irq_callback(struct urb *urb)
> }
>
> /* Copy data from URB */
> - spin_lock(&dev->video_mode.slock);
> + spin_lock_irqsave(&dev->video_mode.slock, flags);
> dev->video_mode.isoc_ctl.isoc_copy(dev, urb);
> - spin_unlock(&dev->video_mode.slock);
> + spin_unlock_irqrestore(&dev->video_mode.slock, flags);
>
> /* Reset urb buffers */
> for (i = 0; i < urb->number_of_packets; i++) {
> @@ -842,6 +843,7 @@ static void cx231xx_bulk_irq_callback(struct urb *urb)
> struct cx231xx_video_mode *vmode =
> container_of(dma_q, struct cx231xx_video_mode, vidq);
> struct cx231xx *dev = container_of(vmode, struct cx231xx, video_mode);
> + unsigned long flags;
>
> switch (urb->status) {
> case 0: /* success */
> @@ -857,9 +859,9 @@ static void cx231xx_bulk_irq_callback(struct urb *urb)
> }
>
> /* Copy data from URB */
> - spin_lock(&dev->video_mode.slock);
> + spin_lock_irqsave(&dev->video_mode.slock, flags);
> dev->video_mode.bulk_ctl.bulk_copy(dev, urb);
> - spin_unlock(&dev->video_mode.slock);
> + spin_unlock_irqrestore(&dev->video_mode.slock, flags);
>
> /* Reset urb buffers */
> urb->status = usb_submit_urb(urb, GFP_ATOMIC);
> diff --git a/drivers/media/usb/cx231xx/cx231xx-vbi.c b/drivers/media/usb/cx231xx/cx231xx-vbi.c
> index c027942..38e78f8 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-vbi.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-vbi.c
> @@ -306,6 +306,7 @@ static void cx231xx_irq_vbi_callback(struct urb *urb)
> struct cx231xx_video_mode *vmode =
> container_of(dma_q, struct cx231xx_video_mode, vidq);
> struct cx231xx *dev = container_of(vmode, struct cx231xx, vbi_mode);
> + unsigned long flags;
>
> switch (urb->status) {
> case 0: /* success */
> @@ -322,9 +323,9 @@ static void cx231xx_irq_vbi_callback(struct urb *urb)
> }
>
> /* Copy data from URB */
> - spin_lock(&dev->vbi_mode.slock);
> + spin_lock_irqsave(&dev->vbi_mode.slock, flags);
> dev->vbi_mode.bulk_ctl.bulk_copy(dev, urb);
> - spin_unlock(&dev->vbi_mode.slock);
> + spin_unlock_irqrestore(&dev->vbi_mode.slock, flags);
>
> /* Reset status */
> urb->status = 0;
>
^ permalink raw reply
* Re: TP-LINK wireless USB adapter don't work.
From: atar @ 2013-07-26 14:22 UTC (permalink / raw)
To: Oleksij Rempel; +Cc: linux-wireless
In-Reply-To: <51F16993.20207@rempel-privat.de>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=iso-8859-8-i; format=flowed; delsp=yes, Size: 959 bytes --]
Thank you about your answer, but how can I install the firmware? which
processes are need to be taken in order to accomplish the firmware
installation and configuration?
Thanks in advance!!
atar.
> Am 25.07.2013 18:47, schrieb atar:
>> Hi there!!
>>
>> I've a TP-LINK USB wireless adapter with model number TL-WN722N. When I
>> plug it in to one of the USB slots in my PC, its LED diode isn't
>> flashing (although under WIN XP it is flashing) and I cannot use it.
>> from the messages of the 'dmesg' command it seems that the kernel detect
>> it, but I unable to use it because it doesn't appear on the wicd window
>> of available interfaces.
>>
>> My Linux distro is Debian squeeze with kernel 2.6.32-5-686.
>>
>> What should I do in order to solve this problem?
>
> It is ath9k-htc based adapter. Please update your kernel and use latest
> firmware. I think Debian squeeze do not provided this firmware, so you
> should install it manually.
>
>
^ permalink raw reply
* Re: Help adding trace events to xHCI
From: Steven Rostedt @ 2013-07-26 14:05 UTC (permalink / raw)
To: Johannes Berg
Cc: Sarah Sharp, Xenia Ragiadakou, OPW Kernel Interns List, linux-usb,
linux-wireless, Kalle Valo
In-Reply-To: <1374846358.8248.53.camel@jlt4.sipsolutions.net>
On Fri, 2013-07-26 at 15:45 +0200, Johannes Berg wrote:
> Well, right now I can live with allocation 110 bytes for each
> tracepoint, this would just optimise it down. If I was in the middle of
> writing one such event while an interrupt came in, I'd not be able to
> reduce the ring-buffer allocation again. I doubt that an interrupt would
> come in much of the time between allocating the new event and
> deallocating it partially. The more difficult question would seem to be
> whether or not we can reliably detect an interrupt having written
> another event.
>
Hmm, you may be convincing me ;-)
As it just allocates the "max" anyway, this feature will actually help.
Don't worry about the detection of interrupt, it already does that with
the discard event. It wouldn't be that hard to extend that into a
reduction too.
-- Steve
^ permalink raw reply
* Re: Help adding trace events to xHCI
From: Johannes Berg @ 2013-07-26 13:45 UTC (permalink / raw)
To: Steven Rostedt
Cc: Sarah Sharp, Xenia Ragiadakou, OPW Kernel Interns List, linux-usb,
linux-wireless, Kalle Valo
In-Reply-To: <1374844649.6580.24.camel@gandalf.local.home>
On Fri, 2013-07-26 at 09:17 -0400, Steven Rostedt wrote:
> On Fri, 2013-07-26 at 15:06 +0200, Johannes Berg wrote:
> > On Fri, 2013-07-26 at 08:28 -0400, Steven Rostedt wrote:
>
> > Ah, yes, that'd work. I was considering putting it into the trace event
> > handling itself so I don't have to allocate those buffers and put the
> > handling into every tracepoint, but I don't know how that'd work with
> > interrupts coming in.
>
> If you create helper functions, it shouldn't be too hard.
True, and I could even export them somewhere to share the buffers
between all the different subsystems that might use this.
> > If we assume that interrupts coming in in the middle of a tracepoint
> > should be rare, we could do something like
> >
> > * allocate max buffer in on the tracing ringbuffer page
> > * write data into it
> > * if no interrupt came in, reduce reservation
> >
> > but I'm not sure how to implement step 3 :)
> >
>
> It's possible to reduce the ring buffer, it's just not implemented. I'm
> not sure I want to do that either. Interrupts coming in is not so rare
> as it can be any interrupt being traced. This means your tracepoints
> will likely waste a lot of buffer space if you are tracing interrupts as
> well.
Well, right now I can live with allocation 110 bytes for each
tracepoint, this would just optimise it down. If I was in the middle of
writing one such event while an interrupt came in, I'd not be able to
reduce the ring-buffer allocation again. I doubt that an interrupt would
come in much of the time between allocating the new event and
deallocating it partially. The more difficult question would seem to be
whether or not we can reliably detect an interrupt having written
another event.
Also, this would save the memcpy() your scheme had.
Anyway, I'm fine with the current status quo, but if more people want to
trace variable length things like formatted strings I think it might
make sense to add some way of making that more efficient.
johannes
^ permalink raw reply
* Re: [PATCH 3.11] iwlwifi: dvm: fix calling ieee80211_chswitch_done() with NULL
From: Johannes Berg @ 2013-07-26 13:28 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: linux-wireless, ilw
In-Reply-To: <20130726132909.GA21925@redhat.com>
On Fri, 2013-07-26 at 15:29 +0200, Stanislaw Gruszka wrote:
> If channel switch is pending and we remove interface we can
> crash like showed below due to passing NULL vif to mac80211:
>
> BUG: unable to handle kernel paging request at fffffffffffff8cc
> IP: [<ffffffff8130924d>] strnlen+0xd/0x40
> Call Trace:
> [<ffffffff8130ad2e>] string.isra.3+0x3e/0xd0
> [<ffffffff8130bf99>] vsnprintf+0x219/0x640
> [<ffffffff8130c481>] vscnprintf+0x11/0x30
> [<ffffffff81061585>] vprintk_emit+0x115/0x4f0
> [<ffffffff81657bd5>] printk+0x61/0x63
> [<ffffffffa048987f>] ieee80211_chswitch_done+0xaf/0xd0 [mac80211]
> [<ffffffffa04e7b34>] iwl_chswitch_done+0x34/0x40 [iwldvm]
> [<ffffffffa04f83c3>] iwlagn_commit_rxon+0x2a3/0xdc0 [iwldvm]
> [<ffffffffa04ebc50>] ? iwlagn_set_rxon_chain+0x180/0x2c0 [iwldvm]
> [<ffffffffa04e5e76>] iwl_set_mode+0x36/0x40 [iwldvm]
> [<ffffffffa04e5f0d>] iwlagn_mac_remove_interface+0x8d/0x1b0 [iwldvm]
> [<ffffffffa0459b3d>] ieee80211_do_stop+0x29d/0x7f0 [mac80211]
>
> This is because we nulify ctx->vif in iwlagn_mac_remove_interface()
> before calling some other functions that teardown interface. To fix
> just check ctx->vif on iwl_chswitch_done(). We should not call
> ieee80211_chswitch_done() as channel switch works were already canceled
> by mac80211 in ieee80211_do_stop() -> ieee80211_mgd_stop().
Thanks, I've picked this up.
johannes
^ permalink raw reply
* [PATCH 3.11] iwlwifi: dvm: fix calling ieee80211_chswitch_done() with NULL
From: Stanislaw Gruszka @ 2013-07-26 13:29 UTC (permalink / raw)
To: linux-wireless; +Cc: ilw
If channel switch is pending and we remove interface we can
crash like showed below due to passing NULL vif to mac80211:
BUG: unable to handle kernel paging request at fffffffffffff8cc
IP: [<ffffffff8130924d>] strnlen+0xd/0x40
Call Trace:
[<ffffffff8130ad2e>] string.isra.3+0x3e/0xd0
[<ffffffff8130bf99>] vsnprintf+0x219/0x640
[<ffffffff8130c481>] vscnprintf+0x11/0x30
[<ffffffff81061585>] vprintk_emit+0x115/0x4f0
[<ffffffff81657bd5>] printk+0x61/0x63
[<ffffffffa048987f>] ieee80211_chswitch_done+0xaf/0xd0 [mac80211]
[<ffffffffa04e7b34>] iwl_chswitch_done+0x34/0x40 [iwldvm]
[<ffffffffa04f83c3>] iwlagn_commit_rxon+0x2a3/0xdc0 [iwldvm]
[<ffffffffa04ebc50>] ? iwlagn_set_rxon_chain+0x180/0x2c0 [iwldvm]
[<ffffffffa04e5e76>] iwl_set_mode+0x36/0x40 [iwldvm]
[<ffffffffa04e5f0d>] iwlagn_mac_remove_interface+0x8d/0x1b0 [iwldvm]
[<ffffffffa0459b3d>] ieee80211_do_stop+0x29d/0x7f0 [mac80211]
This is because we nulify ctx->vif in iwlagn_mac_remove_interface()
before calling some other functions that teardown interface. To fix
just check ctx->vif on iwl_chswitch_done(). We should not call
ieee80211_chswitch_done() as channel switch works were already canceled
by mac80211 in ieee80211_do_stop() -> ieee80211_mgd_stop().
Resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=979581
Cc: stable@vger.kernel.org
Reported-by: Lukasz Jagiello <jagiello.lukasz@gmail.com>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/iwlwifi/dvm/mac80211.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/iwlwifi/dvm/mac80211.c b/drivers/net/wireless/iwlwifi/dvm/mac80211.c
index 323e4a3..9a817df 100644
--- a/drivers/net/wireless/iwlwifi/dvm/mac80211.c
+++ b/drivers/net/wireless/iwlwifi/dvm/mac80211.c
@@ -1046,7 +1046,10 @@ void iwl_chswitch_done(struct iwl_priv *priv, bool is_success)
if (test_bit(STATUS_EXIT_PENDING, &priv->status))
return;
- if (test_and_clear_bit(STATUS_CHANNEL_SWITCH_PENDING, &priv->status))
+ if (!test_and_clear_bit(STATUS_CHANNEL_SWITCH_PENDING, &priv->status))
+ return;
+
+ if (ctx->vif)
ieee80211_chswitch_done(ctx->vif, is_success);
}
--
1.7.11.7
^ permalink raw reply related
* Re: Help adding trace events to xHCI
From: Steven Rostedt @ 2013-07-26 13:17 UTC (permalink / raw)
To: Johannes Berg
Cc: Sarah Sharp, Xenia Ragiadakou, OPW Kernel Interns List, linux-usb,
linux-wireless, Kalle Valo
In-Reply-To: <1374844013.8248.47.camel@jlt4.sipsolutions.net>
On Fri, 2013-07-26 at 15:06 +0200, Johannes Berg wrote:
> On Fri, 2013-07-26 at 08:28 -0400, Steven Rostedt wrote:
> Ah, yes, that'd work. I was considering putting it into the trace event
> handling itself so I don't have to allocate those buffers and put the
> handling into every tracepoint, but I don't know how that'd work with
> interrupts coming in.
If you create helper functions, it shouldn't be too hard.
>
> If we assume that interrupts coming in in the middle of a tracepoint
> should be rare, we could do something like
>
> * allocate max buffer in on the tracing ringbuffer page
> * write data into it
> * if no interrupt came in, reduce reservation
>
> but I'm not sure how to implement step 3 :)
>
It's possible to reduce the ring buffer, it's just not implemented. I'm
not sure I want to do that either. Interrupts coming in is not so rare
as it can be any interrupt being traced. This means your tracepoints
will likely waste a lot of buffer space if you are tracing interrupts as
well.
That said, I can probably implement a reduce feature of the ring buffer
if needed.
-- Steve
^ permalink raw reply
* Re: Help adding trace events to xHCI
From: Johannes Berg @ 2013-07-26 13:06 UTC (permalink / raw)
To: Steven Rostedt
Cc: Sarah Sharp, Xenia Ragiadakou, OPW Kernel Interns List, linux-usb,
linux-wireless, Kalle Valo
In-Reply-To: <1374841699.6580.21.camel@gandalf.local.home>
On Fri, 2013-07-26 at 08:28 -0400, Steven Rostedt wrote:
> > My original though here was that we should be able to reserve (maximum)
> > space on the per-CPU buffer, and then relinquish some of it after the
> > tracepoint was written to the data, but I never had the time to check if
> > that was possible to implement. It would make it a little inefficient at
> > page boundaries, but that seems OK given that our maximum size is only
> > ~100 bytes or so now.
> Yes that would be trivial to implement. If the max buffer is ~100 bytes,
> allocate 256 byte buffers per cpu. Also have a per cpu index, and then
> have something like this:
>
> [snip]
Ah, yes, that'd work. I was considering putting it into the trace event
handling itself so I don't have to allocate those buffers and put the
handling into every tracepoint, but I don't know how that'd work with
interrupts coming in.
If we assume that interrupts coming in in the middle of a tracepoint
should be rare, we could do something like
* allocate max buffer in on the tracing ringbuffer page
* write data into it
* if no interrupt came in, reduce reservation
but I'm not sure how to implement step 3 :)
johannes
^ permalink raw reply
* Re: Help adding trace events to xHCI
From: Steven Rostedt @ 2013-07-26 12:28 UTC (permalink / raw)
To: Johannes Berg
Cc: Sarah Sharp, Xenia Ragiadakou, OPW Kernel Interns List, linux-usb,
linux-wireless, Kalle Valo
In-Reply-To: <1374830340.8248.43.camel@jlt4.sipsolutions.net>
On Fri, 2013-07-26 at 11:19 +0200, Johannes Berg wrote:
> My original though here was that we should be able to reserve (maximum)
> space on the per-CPU buffer, and then relinquish some of it after the
> tracepoint was written to the data, but I never had the time to check if
> that was possible to implement. It would make it a little inefficient at
> page boundaries, but that seems OK given that our maximum size is only
> ~100 bytes or so now.
>
Yes that would be trivial to implement. If the max buffer is ~100 bytes,
allocate 256 byte buffers per cpu. Also have a per cpu index, and then
have something like this:
index = local_add_return(this_cpu_ptr(trace_index), len);
if (index >= MAX_BUF) {
/* write tracepoint with failed buffer */
local_sub(this_cpu_ptr(trace_index));
return;
}
index -= len;
memcpy(this_cpu_ptr(trace_buf) + index, string, len);
/* write tracepoint with trace_buf */
local_sub(this_cpu_ptr(trace_index), len);
This way the trace_buf will work like a stack allocator. The
local_add_return() will reserve the space in the buffer such that if an
interrupt were to come in, it would allocate after that space. The check
for MAX_BUF tests for the case that the buffer was too big and had to
bail. Still trace that event to let the user (yourself) know you need a
bigger buffer.
-- Steve
^ permalink raw reply
* Re: [WT PATCH 4/6] mac80211: Add per-sdata station hash, and sdata hash.
From: Felix Fietkau @ 2013-07-26 9:56 UTC (permalink / raw)
To: Johannes Berg; +Cc: Ben Greear, linux-wireless
In-Reply-To: <1374828780.8248.24.camel@jlt4.sipsolutions.net>
On 2013-07-26 10:53 AM, Johannes Berg wrote:
> On Thu, 2013-07-11 at 08:29 -0700, Ben Greear wrote:
>
>> > I also don't like maintaining two separate hash tables and all that.
>> >
>> > I'd reconsider if you actually remove the hash entirely, but that'll be
>> > tricky to walk the station list and will quite possibly make the RX path
>> > there more expensive?
>>
>> Remove local->sta_hash ?
>
> To be honest, I'm undecided. Yes, I was thinking that, but I also think
> having a huge hashtable like that for each virtual interface is way
> overkill, in particular for station interfaces that usually have one
> peer (the AP) and maybe a few TDLS peers. Or P2P-Device interfaces that
> have no peers at all ...
>
> I don't see a good way to improve the hash either, since we don't always
> (e.g. in RX path) have the interface address.
How about mixing in the interface address into the hash. Theoretically
you should always have that available, even in the rx path. Multicast
data packets contain the BSSID, so you can get the address from there.
You just need to be careful about checking the DS bits to figure out
which address to use ;)
I think this is a much better solution than duplicating the hash, or
moving it into sdata entirely.
- Felix
^ permalink raw reply
* Re: Help adding trace events to xHCI
From: Johannes Berg @ 2013-07-26 9:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: Sarah Sharp, Xenia Ragiadakou, OPW Kernel Interns List, linux-usb,
linux-wireless, Kalle Valo
In-Reply-To: <1373570955.17876.58.camel@gandalf.local.home>
On Thu, 2013-07-11 at 15:29 -0400, Steven Rostedt wrote:
> > Note that there's no easy way to dynamically allocate the right amount
> > of space in the ringbuffer, or at least I haven't found one. We
> > therefore have a static size, which is somewhat inefficient.
>
> Can you add a helper function? If these trace events can't nest (one in
> interrupt context and another in normal context with interrupts
> enabled), then you can just allocate a per-cpu buffer and store the
> string in that, and then move the string into the buffer.
In my situation personally, I can't, because I can have them in
interrupt and regular context (with interrupts enabled).
My original though here was that we should be able to reserve (maximum)
space on the per-CPU buffer, and then relinquish some of it after the
tracepoint was written to the data, but I never had the time to check if
that was possible to implement. It would make it a little inefficient at
page boundaries, but that seems OK given that our maximum size is only
~100 bytes or so now.
johannes
^ permalink raw reply
* Re: Help adding trace events to xHCI
From: Johannes Berg @ 2013-07-26 9:16 UTC (permalink / raw)
To: Sarah Sharp
Cc: Xenia Ragiadakou, OPW Kernel Interns List, linux-usb,
linux-wireless, Kalle Valo, Steven Rostedt
In-Reply-To: <20130711190035.GD5240@xanatos>
[sorry for the late response!]
> Yes, that does look inefficient. Would it make sense to add a couple
> different trace event classes for different sized buffers, and call
> those trace classes conditionally, based on the size of the formatted
> string? Or would that be too much of a mess.
I don't see a way you could even do that, since you'd have to print the
string to decide which event to use, but then at the time you'd have
done most of the work already, so that'd just penalise the
tracing-disabled case, I think.
> Either way, it's only inefficient when trace events are turned on. We
> don't expect xHCI debugging to be enabled by users very often. I do
> expect that there will be a lot of debugging when it gets turned on.
> Can userspace increase the size of the ring buffer? How much space do
> we have by default?
I don't know, I think it's dynamic up to some maximum.
> > > int xhci_debug_address(const char *fmt, ...)
> >
> > This confuses me somewhat -- why is it called "xhci_debug_address()"
> > when it takes arbitrary parameters? Where's the "address" part?
>
> It's debugging the xHCI Address command output. Depending on the status
> of the command, we print different statement in the xHCI code. E.g. we
> print when the command times out, or finishes with an error status
> because there was a USB transfer error on the bus.
>
> We can work on better names later. :)
I was thinking less about the names and more about the semantics of the
tracing. It might be worth, for example, to not print different
statements in the tracepoint but instead add the (partial) command and
its status, and have a tracing plugin that post-processes it into a
better string. But I'm not familiar enough with xHCI to say whether
that's possible or not.
For example, in iwlwifi, I can trace the communication with the device,
so you'd have the actual command I send over the PCIe bus to the device,
and the response I'm getting from the firmware running in the device.
Those are just binary blobs in the trace, but I can post-process to
print their contents.
> > I don't really know what xhci does, but I suppose it has register
> > read/write, maybe packet (urb?) submissions etc. so something like the
> > iwlwifi_io system in drivers/net/wireless/iwlwifi/iwl-devtrace.h might
> > (also) be (more) useful. In iwlwifi I have tracing for
> > * IO accesses & interrupt notifications/reasons
> > * commands and TX packets submitted to the device
> > * notifications/RX packets received from the device
> > * previously existing debug messages
>
> My initial list of specific trace points was something like:
>
> 1. xHCI host initialization and shutdown
>
> 2. xHCI memory allocation (dynamic ring resizing, structure alloc, etc)
>
> 3. A few individual xHCI host controller command tracepoints:
> * status only for all completed commands
> * Address Device command status and output
> * Configure Endpoint and Evaluate Context output
> * individual trace points for other xHCI commands
>
> 4. Tracepoints for all USB transfer types:
> * Control TX output (only for non-successful transfers)
> * Bulk TX
> * Interrupt TX
> * Isoc TX
>
> 5. URB cancellation
>
> And probably more. Basically, I want to be able to control what gets
> printed, based on where I think the xHCI bug might be. Does that sound
> reasonable?
You'd have to explain xHCI to me in more detail :-)
Some of these are driver things only, right? Like memory allocation,
etc. Those would make sense as separate tracepoints I think. For the
communication with the device, I've found for our device that having
just partial information usually isn't very useful unless there's a very
specific bug like having the wrong bit set in some command, so I'm
recording all of that without much differentiation.
I think I'd not split it as detailed as you suggested, but rather have a
tracepoint for the xHCI host controller command submission and another
one for status, and similarly two (or need just one maybe?) for the
different transfer types.
Btw, a good thing for something like this is to use multiple trace
systems, like in iwlwifi or mac80211 for example, we #define
TRACE_SYSTEM multiple times so you can record sets of tracepoints more
easily and don't have to select a bunch of single ones.
> > However, tracing all the debug messages is actually fairly expensive, I
> > think in part because of the string formatting and in part because of
> > the always-max allocations in the ringbuffer.
>
> So are there parts of the driver that don't have the debug messages go
> through the trace events code, in order to not fill up the ring buffer?
Very very few, but the debug messages have two ways of showing up:
* tracing (all of them all the time)
* printk (selected per debug 'level' or 'functional subsystem')
> > > I'm actually wondering if the call to ath6kl_printk is somehow necessary
> > > in order to be able to pass arguments on the stack.
> >
> > I don't think it is, but formatting the messages *only* for tracing
> > seems a bit odd?
>
> Why is it odd?
Well, it's just a feeling really, but if I think about where it comes
from it seems that you might be able to build more detailed tracepoints
(though you don't want to overdo it!) that get the input data, instead
of formatting it.
Let me make an example. In iwlwifi (sorry, but that's what I'm really
familiar with ...) I have device commands, and get a status response
(possibly with more data) for each of them. The status can be an empty
response (just 'ok'), or with a 32-bit status field, or with more
information.
I could now do something like this:
if (cmd->len == 4)
trace_msg("empty response for cmd %d", cmd->id);
else /* must be >= 8 due to alignment etc. */
trace_msg("response with status %d for cmd %d", cmd->status,
cmd->id);
Instead though, what I did is this:
trace_status(cmd->len, (void *)cmd);
which will just put the binary data coming from the device there, so I
can analyse it later.
johannes
^ permalink raw reply
* Re: [PATCH] iw: print out mesh configuration element on scan
From: Johannes Berg @ 2013-07-26 8:58 UTC (permalink / raw)
To: Chun-Yeow Yeoh; +Cc: linux-wireless
In-Reply-To: <1373627826-20095-1-git-send-email-yeohchunyeow@gmail.com>
On Fri, 2013-07-12 at 19:17 +0800, Chun-Yeow Yeoh wrote:
> Print out mesh configuration element which will be useful for
> decision making whether to join or not join the mesh network.
Applied, thanks.
johannes
^ 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