* Re: iwlwifi connection problems
From: Johannes Berg @ 2010-07-27 6:37 UTC (permalink / raw)
To: Alex Romosan; +Cc: Guy, Wey-Yi, linux-wireless@vger.kernel.org
In-Reply-To: <87mxtd5xlq.fsf@sycorax.lbl.gov>
On Mon, 2010-07-26 at 21:59 -0700, Alex Romosan wrote:
> any ideas on how i can debug this? i tried to do a git-bisect but i
> didn't get anywhere. i have the driver compiled in the kernel if that
> makes any difference. as i played with the later kernels (after 2.6.34)
> i discovered that if kept bringing the interface down and then up again
> eventually i would get "cfg80211: Calling CRDA to update world
> regulatory domain" the antenna light on the laptop (i have a thinkpad
> t61) would go off and then at the next ifup the driver connected. looks
> like some kind of reset/initialization problem to me.
So is the issue maybe the channel the AP is no, not the fact that it's
hidden? or something like that?
johannes
^ permalink raw reply
* [WIP] mac80211: AMPDU rx reorder timeout timer
From: Christian Lamparter @ 2010-07-27 6:20 UTC (permalink / raw)
To: linux-wireless
This (rather _unfinished_ :-D ) patch adds a timer which
if trigger schedules the automatic release of all expired
A-MPDU frames. This implementation has the advantage that
all queued-up MPDU will now arrive in the network core
within a timely manner.
In my "experiments", the patch helped to ironed out the
sudden throughput drops that have been _killing_ my
average tcp performance ever since I can remember.
But there is a catch: The logs will quickly fill up with:
"release an RX reorder frame due to timeout on earlier frames".
and the package loss will goes through the roof...
Now, I can think of several reasons why this could happen:
1. we don't wait long enough for the HT peer to
finish their _retry_ procedure?
2. previously - when the stream simply _stopped_ - we had
to wait until the tcp retry kick in again. So we had
some "silent" periods and therefore less noise from
the reordering code?
3. ->bugs<- but where are they?
Regards,
Christian
---
yeah, it's past 8 AM here...
so please excuse me, if this is nothing but utter rubbish.
---
diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 965b272..947352e 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -86,6 +86,11 @@ void ___ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
tid, 0, reason);
del_timer_sync(&tid_rx->session_timer);
+ del_timer_sync(&tid_rx->reorder_timer);
+
+ spin_lock_bh(&local->tid_rx_reorder_lock);
+ list_del_init(&tid_rx->reorder_head);
+ spin_unlock_bh(&local->tid_rx_reorder_lock);
call_rcu(&tid_rx->rcu_head, ieee80211_free_tid_rx);
}
@@ -120,6 +125,28 @@ static void sta_rx_agg_session_timer_expired(unsigned long data)
ieee80211_queue_work(&sta->local->hw, &sta->ampdu_mlme.work);
}
+static void sta_rx_agg_reorder_timer_expired(unsigned long data)
+{
+ /* not an elegant detour, but there is no choice as the timer passes
+ * only one argument, and various sta_info are needed here, so init
+ * flow in sta_info_create gives the TID as data, while the timer_to_id
+ * array gives the sta through container_of */
+ u8 *ptid = (u8 *)data;
+ u8 *timer_to_id = ptid - *ptid;
+ struct sta_info *sta = container_of(timer_to_id, struct sta_info,
+ timer_to_tid[0]);
+
+ struct ieee80211_local *local = sta->local;
+ struct tid_ampdu_rx *tid_rx = sta->ampdu_mlme.tid_rx[*ptid];
+
+ spin_lock_bh(&local->tid_rx_reorder_lock);
+ if (list_empty(&tid_rx->reorder_head))
+ list_add_tail(&tid_rx->reorder_head, &local->tid_rx_reorder);
+ spin_unlock_bh(&local->tid_rx_reorder_lock);
+
+ tasklet_schedule(&local->tasklet);
+}
+
static void ieee80211_send_addba_resp(struct ieee80211_sub_if_data *sdata, u8 *da, u16 tid,
u8 dialog_token, u16 status, u16 policy,
u16 buf_size, u16 timeout)
@@ -251,11 +278,17 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,
goto end;
}
- /* rx timer */
+ /* rx session timer */
tid_agg_rx->session_timer.function = sta_rx_agg_session_timer_expired;
tid_agg_rx->session_timer.data = (unsigned long)&sta->timer_to_tid[tid];
init_timer(&tid_agg_rx->session_timer);
+ /* rx reorder timer */
+ INIT_LIST_HEAD(&tid_agg_rx->reorder_head);
+ tid_agg_rx->reorder_timer.function = sta_rx_agg_reorder_timer_expired;
+ tid_agg_rx->reorder_timer.data = (unsigned long)&sta->timer_to_tid[tid];
+ init_timer(&tid_agg_rx->reorder_timer);
+
/* prepare reordering buffer */
tid_agg_rx->reorder_buf =
kcalloc(buf_size, sizeof(struct sk_buff *), GFP_ATOMIC);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c6b5c2d..b8bd0f9 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -704,6 +704,8 @@ struct ieee80211_local {
struct tasklet_struct tasklet;
struct sk_buff_head skb_queue;
struct sk_buff_head skb_queue_unreliable;
+ spinlock_t tid_rx_reorder_lock;
+ struct list_head tid_rx_reorder;
/* Station data */
/*
@@ -1241,6 +1243,8 @@ int ieee80211_wk_remain_on_channel(struct ieee80211_sub_if_data *sdata,
int ieee80211_wk_cancel_remain_on_channel(
struct ieee80211_sub_if_data *sdata, u64 cookie);
+void ieee80211_release_reorder_timeout(struct sta_info *sta, unsigned int tid);
+
/* channel management */
enum ieee80211_chan_mode {
CHAN_MODE_UNDEFINED,
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 0e95c75..7bff2db 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -281,6 +281,26 @@ static void ieee80211_tasklet_handler(unsigned long data)
break;
}
}
+
+ spin_lock_bh(&local->tid_rx_reorder_lock);
+ while (!list_empty(&local->tid_rx_reorder)) {
+ struct tid_ampdu_rx *iter;
+ struct sta_info *sta;
+ u8 *ptid, *timer_to_id;
+
+ iter = list_first_entry(&local->tid_rx_reorder,
+ struct tid_ampdu_rx,
+ reorder_head);
+
+ ptid = (u8 *)iter->session_timer.data;
+ timer_to_id = ptid - *ptid;
+ sta = container_of(timer_to_id, struct sta_info,
+ timer_to_tid[0]);
+
+ list_del_init(&iter->reorder_head);
+ ieee80211_release_reorder_timeout(sta, *ptid);
+ }
+ spin_unlock_bh(&local->tid_rx_reorder_lock);
}
static void ieee80211_restart_work(struct work_struct *work)
@@ -448,6 +468,8 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
local->uapsd_max_sp_len = IEEE80211_DEFAULT_MAX_SP_LEN;
INIT_LIST_HEAD(&local->interfaces);
+ INIT_LIST_HEAD(&local->tid_rx_reorder);
+ spin_lock_init(&local->tid_rx_reorder_lock);
__hw_addr_init(&local->mc_list);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index d70e1a9..fae1dbe 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -538,20 +538,12 @@ static void ieee80211_release_reorder_frame(struct ieee80211_hw *hw,
int index,
struct sk_buff_head *frames)
{
- struct ieee80211_supported_band *sband;
- struct ieee80211_rate *rate = NULL;
struct sk_buff *skb = tid_agg_rx->reorder_buf[index];
- struct ieee80211_rx_status *status;
if (!skb)
goto no_frame;
- status = IEEE80211_SKB_RXCB(skb);
-
/* release the reordered frames to stack */
- sband = hw->wiphy->bands[status->band];
- if (!(status->flag & RX_FLAG_HT))
- rate = &sband->bitrates[status->rate_idx];
tid_agg_rx->stored_mpdu_num--;
tid_agg_rx->reorder_buf[index] = NULL;
__skb_queue_tail(frames, skb);
@@ -583,6 +575,73 @@ static void ieee80211_release_reorder_frames(struct ieee80211_hw *hw,
*/
#define HT_RX_REORDER_BUF_TIMEOUT (HZ / 10)
+static void ieee80211_sta_reorder_timeout(struct ieee80211_hw *hw,
+ struct tid_ampdu_rx *tid_agg_rx,
+ struct sk_buff_head *frames)
+{
+ int index;
+
+ /* release the buffer until next missing frame */
+ index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
+ tid_agg_rx->buf_size;
+ if (!tid_agg_rx->reorder_buf[index] &&
+ tid_agg_rx->stored_mpdu_num > 1) {
+ /*
+ * No buffers ready to be released, but check whether any
+ * frames in the reorder buffer have timed out.
+ */
+ int j;
+ int skipped = 1;
+ for (j = (index + 1) % tid_agg_rx->buf_size; j != index;
+ j = (j + 1) % tid_agg_rx->buf_size) {
+ if (!tid_agg_rx->reorder_buf[j]) {
+ skipped++;
+ continue;
+ }
+ if (!time_after(jiffies, tid_agg_rx->reorder_time[j] +
+ HT_RX_REORDER_BUF_TIMEOUT)) {
+
+ mod_timer(&tid_agg_rx->reorder_timer, jiffies -
+ tid_agg_rx->reorder_time[j] + 1 +
+ HT_RX_REORDER_BUF_TIMEOUT);
+ break;
+ }
+
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ if (net_ratelimit())
+ printk(KERN_DEBUG "%s: release an RX reorder "
+ "frame due to timeout on earlier "
+ "frames\n",
+ wiphy_name(hw->wiphy));
+#endif
+ ieee80211_release_reorder_frame(hw, tid_agg_rx,
+ j, frames);
+
+ /*
+ * Increment the head seq# also for the skipped slots.
+ */
+ tid_agg_rx->head_seq_num =
+ (tid_agg_rx->head_seq_num + skipped) & SEQ_MASK;
+ skipped = 0;
+ }
+ } else {
+ while (tid_agg_rx->reorder_buf[index]) {
+ ieee80211_release_reorder_frame(hw, tid_agg_rx,
+ index, frames);
+ index = seq_sub(tid_agg_rx->head_seq_num,
+ tid_agg_rx->ssn) % tid_agg_rx->buf_size;
+ }
+
+ if (tid_agg_rx->stored_mpdu_num) {
+ mod_timer(&tid_agg_rx->reorder_timer, jiffies -
+ tid_agg_rx->reorder_time[index] + 1 +
+ HT_RX_REORDER_BUF_TIMEOUT);
+ } else {
+ del_timer(&tid_agg_rx->reorder_timer);
+ }
+ }
+}
+
/*
* As this function belongs to the RX path it must be under
* rcu_read_lock protection. It returns false if the frame
@@ -643,49 +702,8 @@ static bool ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
tid_agg_rx->reorder_buf[index] = skb;
tid_agg_rx->reorder_time[index] = jiffies;
tid_agg_rx->stored_mpdu_num++;
- /* release the buffer until next missing frame */
- index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
- tid_agg_rx->buf_size;
- if (!tid_agg_rx->reorder_buf[index] &&
- tid_agg_rx->stored_mpdu_num > 1) {
- /*
- * No buffers ready to be released, but check whether any
- * frames in the reorder buffer have timed out.
- */
- int j;
- int skipped = 1;
- for (j = (index + 1) % tid_agg_rx->buf_size; j != index;
- j = (j + 1) % tid_agg_rx->buf_size) {
- if (!tid_agg_rx->reorder_buf[j]) {
- skipped++;
- continue;
- }
- if (!time_after(jiffies, tid_agg_rx->reorder_time[j] +
- HT_RX_REORDER_BUF_TIMEOUT))
- break;
-#ifdef CONFIG_MAC80211_HT_DEBUG
- if (net_ratelimit())
- printk(KERN_DEBUG "%s: release an RX reorder "
- "frame due to timeout on earlier "
- "frames\n",
- wiphy_name(hw->wiphy));
-#endif
- ieee80211_release_reorder_frame(hw, tid_agg_rx,
- j, frames);
-
- /*
- * Increment the head seq# also for the skipped slots.
- */
- tid_agg_rx->head_seq_num =
- (tid_agg_rx->head_seq_num + skipped) & SEQ_MASK;
- skipped = 0;
- }
- } else while (tid_agg_rx->reorder_buf[index]) {
- ieee80211_release_reorder_frame(hw, tid_agg_rx, index, frames);
- index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
- tid_agg_rx->buf_size;
- }
+ ieee80211_sta_reorder_timeout(hw, tid_agg_rx, frames);
return true;
}
@@ -2267,19 +2285,47 @@ static void ieee80211_rx_cooked_monitor(struct ieee80211_rx_data *rx,
dev_kfree_skb(skb);
}
-
-static void ieee80211_invoke_rx_handlers(struct ieee80211_sub_if_data *sdata,
- struct ieee80211_rx_data *rx,
- struct sk_buff *skb,
- struct ieee80211_rate *rate)
+static void ieee80211_rx_handlers_result(struct ieee80211_rx_data *rx,
+ ieee80211_rx_result res)
{
- struct sk_buff_head reorder_release;
- ieee80211_rx_result res = RX_DROP_MONITOR;
+ struct ieee80211_rx_status *status;
- __skb_queue_head_init(&reorder_release);
+ switch (res) {
+ case RX_DROP_MONITOR:
+ I802_DEBUG_INC(rx->sdata->local->rx_handlers_drop);
+ if (rx->sta)
+ rx->sta->rx_dropped++;
+ /* fall through */
+ case RX_CONTINUE: {
+ struct ieee80211_rate *rate = NULL;
+ struct ieee80211_supported_band *sband;
- rx->skb = skb;
- rx->sdata = sdata;
+ status = IEEE80211_SKB_RXCB((rx->skb));
+
+ sband = rx->local->hw.wiphy->bands[status->band];
+ if (!(status->flag & RX_FLAG_HT))
+ rate = &sband->bitrates[status->rate_idx];
+
+ ieee80211_rx_cooked_monitor(rx, rate);
+ break;
+ }
+ case RX_DROP_UNUSABLE:
+ I802_DEBUG_INC(rx->sdata->local->rx_handlers_drop);
+ if (rx->sta)
+ rx->sta->rx_dropped++;
+ dev_kfree_skb(rx->skb);
+ break;
+ case RX_QUEUED:
+ I802_DEBUG_INC(rx->sdata->local->rx_handlers_queued);
+ break;
+ }
+}
+
+static void ieee80211_rx_handlers(struct ieee80211_rx_data *rx,
+ struct sk_buff_head *frames)
+{
+ ieee80211_rx_result res = RX_DROP_MONITOR;
+ struct sk_buff *skb;
#define CALL_RXH(rxh) \
do { \
@@ -2288,17 +2334,7 @@ static void ieee80211_invoke_rx_handlers(struct ieee80211_sub_if_data *sdata,
goto rxh_next; \
} while (0);
- /*
- * NB: the rxh_next label works even if we jump
- * to it from here because then the list will
- * be empty, which is a trivial check
- */
- CALL_RXH(ieee80211_rx_h_passive_scan)
- CALL_RXH(ieee80211_rx_h_check)
-
- ieee80211_rx_reorder_ampdu(rx, &reorder_release);
-
- while ((skb = __skb_dequeue(&reorder_release))) {
+ while ((skb = __skb_dequeue(frames))) {
/*
* all the other fields are valid across frames
* that belong to an aMPDU since they are on the
@@ -2316,42 +2352,88 @@ static void ieee80211_invoke_rx_handlers(struct ieee80211_sub_if_data *sdata,
CALL_RXH(ieee80211_rx_h_remove_qos_control)
CALL_RXH(ieee80211_rx_h_amsdu)
#ifdef CONFIG_MAC80211_MESH
- if (ieee80211_vif_is_mesh(&sdata->vif))
+ if (ieee80211_vif_is_mesh(&rx->sdata->vif))
CALL_RXH(ieee80211_rx_h_mesh_fwding);
#endif
CALL_RXH(ieee80211_rx_h_data)
/* special treatment -- needs the queue */
- res = ieee80211_rx_h_ctrl(rx, &reorder_release);
+ res = ieee80211_rx_h_ctrl(rx, frames);
if (res != RX_CONTINUE)
goto rxh_next;
CALL_RXH(ieee80211_rx_h_action)
CALL_RXH(ieee80211_rx_h_mgmt)
+ rxh_next:
+ ieee80211_rx_handlers_result(rx, res);
+
#undef CALL_RXH
+ }
+}
+
+static void ieee80211_invoke_rx_handlers(struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_rx_data *rx,
+ struct sk_buff *skb,
+ struct ieee80211_rate *rate)
+{
+ struct sk_buff_head reorder_release;
+ ieee80211_rx_result res = RX_DROP_MONITOR;
+
+ __skb_queue_head_init(&reorder_release);
+
+ rx->skb = skb;
+ rx->sdata = sdata;
+
+#define CALL_RXH(rxh) \
+ do { \
+ res = rxh(rx); \
+ if (res != RX_CONTINUE) \
+ goto rxh_next; \
+ } while (0);
+
+ /*
+ * NB: the rxh_next label works even if we jump
+ * to it from here because then the list will
+ * be empty, which is a trivial check
+ */
+ CALL_RXH(ieee80211_rx_h_passive_scan)
+ CALL_RXH(ieee80211_rx_h_check)
+
+ ieee80211_rx_reorder_ampdu(rx, &reorder_release);
+ ieee80211_rx_handlers(rx, &reorder_release);
+ return;
rxh_next:
- switch (res) {
- case RX_DROP_MONITOR:
- I802_DEBUG_INC(sdata->local->rx_handlers_drop);
- if (rx->sta)
- rx->sta->rx_dropped++;
- /* fall through */
- case RX_CONTINUE:
- ieee80211_rx_cooked_monitor(rx, rate);
- break;
- case RX_DROP_UNUSABLE:
- I802_DEBUG_INC(sdata->local->rx_handlers_drop);
- if (rx->sta)
- rx->sta->rx_dropped++;
- dev_kfree_skb(rx->skb);
- break;
- case RX_QUEUED:
- I802_DEBUG_INC(sdata->local->rx_handlers_queued);
- break;
- }
- }
+ ieee80211_rx_handlers_result(rx, res);
+
+#undef CALL_RXH
+}
+
+void ieee80211_release_reorder_timeout(struct sta_info *sta, unsigned int tid)
+{
+ struct sk_buff_head frames;
+ struct ieee80211_rx_data rx = { };
+
+ __skb_queue_head_init(&frames);
+
+ rx.sdata = sta->sdata;
+ rx.local = sta->local;
+ rx.sta = sta;
+ rx.queue = tid;
+ rx.flags |= IEEE80211_RX_RA_MATCH;
+
+ if (unlikely(test_bit(SCAN_HW_SCANNING, &sta->local->scanning) ||
+ test_bit(SCAN_OFF_CHANNEL, &sta->local->scanning)))
+ rx.flags |= IEEE80211_RX_IN_SCAN;
+
+ rcu_read_lock();
+ ieee80211_sta_reorder_timeout(&sta->local->hw,
+ sta->ampdu_mlme.tid_rx[tid], &frames);
+
+ ieee80211_rx_handlers(&rx, &frames);
+ rcu_read_unlock();
+
}
/* main receive path */
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 54262e7..4dccce8 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -110,6 +110,7 @@ struct tid_ampdu_tx {
* @timeout: reset timer value (in TUs).
* @dialog_token: dialog token for aggregation session
* @rcu_head: RCU head used for freeing this struct
+ * @reorder_head: used to release expired frames
*
* This structure is protected by RCU and the per-station
* spinlock. Assignments to the array holding it must hold
@@ -121,9 +122,11 @@ struct tid_ampdu_tx {
*/
struct tid_ampdu_rx {
struct rcu_head rcu_head;
+ struct list_head reorder_head;
struct sk_buff **reorder_buf;
unsigned long *reorder_time;
struct timer_list session_timer;
+ struct timer_list reorder_timer;
u16 head_seq_num;
u16 stored_mpdu_num;
u16 ssn;
^ permalink raw reply related
* Re: Hardware needs to know when EAP nego is complete
From: Juuso Oikarinen @ 2010-07-27 5:17 UTC (permalink / raw)
To: ext Jouni Malinen
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
Coelho Luciano (Nokia-MS/Helsinki)
In-Reply-To: <20100726143047.GB7324@jm.kir.nu>
On Mon, 2010-07-26 at 16:30 +0200, ext Jouni Malinen wrote:
> On Mon, Jul 26, 2010 at 02:06:40PM +0300, Juuso Oikarinen wrote:
> > There are lots of timing issues involved, and there is a priority
> > between WLAN an BT. To ensure BT A2DP and SCO work properly, BT needs to
> > have enough priority at the expense of WLAN performance. While this
> > works well enough when connected, during WLAN association and especially
> > during EAP negotiation the priority needs to be more on the WLAN side to
> > ensure reliability.
>
> Could you please explain why EAP negotiation is a special case? It is
> using data frames just like any other operation after it..
I'm not aware of all the facts, but I believe it's more about the EAP
negotiation being more sensitive to lost frames for instance.
> > So, what this all boils down to is that the wl1271 driver needs to know
> > when the association, including the possible EAP negotations are fully
> > complete, to be able to adjust the priority.
>
> This sounds like a horrible hack and really, the unreliability during
> EAP should likely fixed in some other way.
To be truthful, I think it's a hack all the way - also on the wl1271.
This way of WLAN-BT coexistence has proven to be a pandora's box of
problems - all these weird issues keep popping up. And this particular
issue is probably one of those, and this way around it, is probably a
last minute hack to solve the issue on the chipset vendor part.
> > Currently, there is no such information available to the driver. In fact
> > this information is not available anywhere in the kernel level either
> > (as the details of the EAP negotiation, needed keys etc are controlled
> > in user-space), so the trigger would need to come from user-space.
>
> That's not actually true.. At least when run with wpa_supplicant is
> setting operation state to IF_OPER_UP when the full authentication
> sequence (i.e., not only EAP, but also the following 4-way handshake
> with EAPOL-Key frames) is done (and IF_OPER_DORMANT when not ready).
> This is needed for other purposes to indicate when real data traffic can
> be sent, e.g., for DHCP clients.
> I don't think I would support the idea of using that information to
> change the WLAN vs. BT priority without some additional data on what
> exactly goes wrong during EAP negotiation, but at least the information
> should already be in the kernel, so we do not need to change nl80211 for
> this.
>
Yeah, this sounds like something that could be used for a trigger. I
will look into it.
I did not really think this proposal would be accepted. I stated out the
problem mainly to get a second opinion, and possibly ideas.
In the meanwhile, we will have to somehow make this work. I thank you
for your thoughts. :)
-Juuso
^ permalink raw reply
* Re: iwlwifi connection problems
From: Alex Romosan @ 2010-07-27 4:59 UTC (permalink / raw)
To: Johannes Berg; +Cc: Guy, Wey-Yi, linux-wireless@vger.kernel.org
In-Reply-To: <1280135867.3693.5.camel@jlt3.sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> On Fri, 2010-07-23 at 13:38 -0700, Guy, Wey-Yi wrote:
>> Hi Johannes,
>>
>> On Fri, 2010-07-23 at 13:14 -0700, Alex Romosan wrote:
>> > since kernel 2.6.35-rc3 (i didn't try 1 or 2) i haven't been able to
>> > connect to a hidden wireless access point using the iwlwifi driver. i
>> > can connect to open ones though (the ones that broadcast their name).
>> > 2.6.34 worked without any problems. any ideas?
>> >
>> > this is with the driver in the standard linux kernel (2.6.35-rc6 is the
>> > latest one i tried).
>>
>> Do you aware any changes?
>
> Nope, and it works fine here on wireless-testing.
any ideas on how i can debug this? i tried to do a git-bisect but i
didn't get anywhere. i have the driver compiled in the kernel if that
makes any difference. as i played with the later kernels (after 2.6.34)
i discovered that if kept bringing the interface down and then up again
eventually i would get "cfg80211: Calling CRDA to update world
regulatory domain" the antenna light on the laptop (i have a thinkpad
t61) would go off and then at the next ifup the driver connected. looks
like some kind of reset/initialization problem to me.
--alex--
--
| I believe the moment is at hand when, by a paranoiac and active |
| advance of the mind, it will be possible (simultaneously with |
| automatism and other passive states) to systematize confusion |
| and thus to help to discredit completely the world of reality. |
^ permalink raw reply
* Re: [ath9k-devel] [RFC] ath9k: improve aggregation throughput by using only first rate
From: Ranga Rao Ravuri @ 2010-07-27 4:48 UTC (permalink / raw)
To: Felix Fietkau
Cc: Björn Smedman, ath9k-devel@lists.ath9k.org, linux-wireless
In-Reply-To: <4C4DE4F5.3010907@openwrt.org>
On 07/27/2010 01:11 AM, Felix Fietkau wrote:
> On 2010-07-26 9:23 PM, Björn Smedman wrote:
>> 2010/7/26 Felix Fietkau<nbd@openwrt.org>:
>>> On 2010-07-26 7:10 PM, Björn Smedman wrote:
>>>> I think there are some (in theory) simple improvements that can be
>>>> done to the tx aggregation / rate control logic. A proof of concept of
>>>> one such improvement is provided below. Basically, it's a hack that
>>> I think it makes sense to rely less on on-chip MRR for fallback, but I
>>> think to make this workable, we really should use the MRR table for
>>> something, otherwise the rate control algorithm will take much longer to
>>> adapt.
>>> It's probably better to fix this properly after I'm done with my A-MPDU
>>> rewrite, because then I can more easily push parts of the software
>>> retransmission behaviour into minstrel_ht directly.
>> Sounds very reasonable. I'm sure you've thought of it but now that
>> it's fresh in my head it would be great if the new aggregation design
>> allowed us to experiment with stuff like this:
>>
>> * The rate control logic treats the average aggregate length as a
>> measured independent variable, when in fact it depends heavily on the
>> rates selected (via the 4 ms txop limit).
> Yes, with the new design maybe we could use the initial rate lookup only
> for setting the sampling flag, and then doing a separate per-AMPDU
> lookup, which properly takes the AMPDU length into account.
>
>> * When tx is aggregated most rate control probe frames end up inside
>> aggregates and are never used for probing (effective probe frequency
>> is divided by average aggregate length).
> Nope, a probing frame never ends up inside an aggregate. It's always
> sent out as a single frame, which is why I had to make the decision
> about sending a probing frame more complex in minstrel_ht, compared to
> minstrel - the previous 10% stuff was limiting aggregation size.
>
>> * When setting up a hardware MRR for an aggregate the focus should be
>> on throughput (as explained earlier in this thread). But there are
>> situations when reliability is important: e.g. when a subframe in the
>> aggregate is about to expire (because of time or block ack window). It
>> may even be advantageous to tx the subframes that are about to expire
>> in their own aggregate with lower / more reliable bitrate?
> Yes, that's what I was thinking as well. We should probably make this
> decision based on the number of sw-retransmitted frames, and maybe
> consider the offset of seqno vs baw_tail as well.
>
>> * In many busy radio environments the packet success rate depends very
>> much on the protection method being used (none, cts-to-self or
>> rts-cts), often more so than on the bitrate itself. It would be
>> interesting to experiment with including the protection method in the
>> rate selection, i.e. to probe for the optimal protection method and
>> bitrate combination.
> Sounds good.
>
>> * In order to have the best possible rate control in very dynamic rf
>> environments it's important to keep the hardware queue short and
>> select rates as late as possible (to not introduce unnecessary delay
>> when selecting new rates). I have no idea how to do this but it would
>> be great if the tx queue could be kept long enough to never stall tx,
>> but no longer.
> This would work with what I suggested above - per-AMPDU rate lookup.
> With software scheduling that's easy to do, since we already restrict
> the queue to max. 2 AMPDUs
>
>> * If I understand correctly the Atheros hardware does not adjust the
>> rts / cts-to-self duration field when going through the MRR
>> (correct?). In that case it may be even more advantageous to use
>> software retry as much as possible when some form of protection is
>> enabled.
> Not sure, but I think it does adjust the duration field according to the
> rate, while transmitting.
[ranga] Yes it does. If you enable RTS on all rates, you would see
different RTSs coming with different duration.
>> Looking forward to the new aggregation code!
> That will still take some time, I recently came up with some better
> design ideas, which require some larger changes to the code that I
> already wrote.
>
> - Felix
> _______________________________________________
> ath9k-devel mailing list
> ath9k-devel@lists.ath9k.org
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
^ permalink raw reply
* Re: Confused - bisecting wireless-testing
From: Luis R. Rodriguez @ 2010-07-27 3:30 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless
In-Reply-To: <AANLkTim-0qoS7o+fcQ6uA18AC_gQSiiDe4Tv_L1FT1Ae@mail.gmail.com>
On Mon, Jul 26, 2010 at 8:27 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> 04:23 * mcgrof is confused, I just did a git bisect -i on
> wireless-testing on some patch on Fri Jul 2 00:09:49 2010 and ended
> up with a 2.6.34 top level
> Makefile
> 04:23 < mcgrof> it was git rebase -i ba17bc5e55ba541d2a8765fca53b6883b667ab21
> 04:23 < mcgrof> eh
> 04:24 < mcgrof> how am I supposed to bisect wl now
> 04:24 < mcgrof> the odd thing though is that the top commit is ancient
> 04:24 < mcgrof> http://paste.pocoo.org/show/242077/
> 04:24 < mcgrof> but the other ones are OK
>
> I realize we should use wireless-2.6.git to bisect stable but I need
> to bisect against recent patches that spans different master- tags
> from you, I figured git bisecting wireless-testing would work now that
> you are using a different method to move your tree forward but am I
> wrong? Should I only bisect between master tags still?
Heh I just realize what I said didn't make sense, what I meant was I
was *going* to bisect but in order to first find my first good
starting point I am going through the tree trying to find a good
starting point to bisect and to do that I use interactive rebase (git
rebase -i sha1sum) to specific sha1sums until I find a good compile
that works. Only after I've done this would I then bisect.
The interactive bisect on ba17bc5e55ba541d2a8765fca53b6883b667ab21
throws me back to 2.6.34 though, for some odd reason.
Luis
^ permalink raw reply
* Confused - bisecting wireless-testing
From: Luis R. Rodriguez @ 2010-07-27 3:27 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless
04:23 * mcgrof is confused, I just did a git bisect -i on
wireless-testing on some patch on Fri Jul 2 00:09:49 2010 and ended
up with a 2.6.34 top level
Makefile
04:23 < mcgrof> it was git rebase -i ba17bc5e55ba541d2a8765fca53b6883b667ab21
04:23 < mcgrof> eh
04:24 < mcgrof> how am I supposed to bisect wl now
04:24 < mcgrof> the odd thing though is that the top commit is ancient
04:24 < mcgrof> http://paste.pocoo.org/show/242077/
04:24 < mcgrof> but the other ones are OK
I realize we should use wireless-2.6.git to bisect stable but I need
to bisect against recent patches that spans different master- tags
from you, I figured git bisecting wireless-testing would work now that
you are using a different method to move your tree forward but am I
wrong? Should I only bisect between master tags still?
Luis
^ permalink raw reply
* Re: ath9k: /proc/net/wireless always shows status of 0
From: Luis R. Rodriguez @ 2010-07-26 23:51 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless
In-Reply-To: <4C4E1C0B.9040608@candelatech.com>
On Mon, Jul 26, 2010 at 4:36 PM, Ben Greear <greearb@candelatech.com> wrote:
> On 07/26/2010 04:30 PM, Luis R. Rodriguez wrote:
>>
>> On Mon, Jul 26, 2010 at 4:17 PM, Ben Greear<greearb@candelatech.com>
>> wrote:
>>>
>>> On 07/26/2010 03:54 PM, Luis R. Rodriguez wrote:
>>>>
>>>> On Mon, Jul 26, 2010 at 3:00 PM, Ben Greear<greearb@candelatech.com>
>>>> wrote:
>>>>>
>>>>> On 07/26/2010 12:49 PM, Ben Greear wrote:
>>>>>>
>>>>>> This is with today's wireless-testing (with no extra hacks)
>>>>>
>>>>> Maybe a better question: In 2.6.31 you could mount debugfs at /debug
>>>>> and get the state with:
>>>>> cat /debug/ieee80211/sta0/state
>>>>
>>>> debugfs is not static API. In fact that's a big reason why we use it.
>>>> I considered defining a filesystem API for 802.11 using configfs but
>>>> our priority is to first get nl80211 feature-complete with respect to
>>>> wext, which we have completed now. But anyway, debugfs is *not* API
>>>> and you should not rely on it, if you want to define API consider
>>>> configfs.
>>>>
>>>>> It seems in .34 there is only phy0 info in debugfs. Or, am I
>>>>> just not looking in the right place?
>>>>>
>>>>> I don't mind hacking code, but I'd appreciate a pointer to the
>>>>> right part of the code if anyone has suggestions.
>>>>
>>>> The other thing you pointed out with wext, you should stop using wext
>>>> and instead rely on nl80211. It is true that if you see something on
>>>> /proc/net/wireless/ that was there before but not there now it is a
>>>> regression and it should be addressed. Not sure where the status crap
>>>> comes from on wext.
>>>
>>> I really don't care what API I end up using..I am just looking for some
>>> way to report to users whether the STA is in the various states that
>>> used to be defined as something similar to:
>>>
>>> enum ieee80211_state {
>>> IEEE80211_UNINITIALIZED = 0,
>>> IEEE80211_INITIALIZED,
>>> IEEE80211_ASSOCIATING,
>>> IEEE80211_ASSOCIATED,
>>> IEEE80211_AUTHENTICATING,
>>> IEEE80211_AUTHENTICATED,
>>> IEEE80211_SHUTDOWN
>>> };
>>>
>>> I have been reading the net/wireless code, but so far I don't see
>>> any state machine type logic where this state might be kept. Maybe
>>> it would have to be inferred from something else?
>>
>> You can use nl28011 and register for netlink multicast messages which
>> broadcast device state changes like the ones you mentioned. These come
>> in on iw via event.c, see print_event() and see the case statements
>> for NL80211_CMD_ASSOCIATE, NL80211_CMD_DEAUTHENTICATE,
>> NL80211_CMD_DISASSOCIATE, etc, you even get reason codes parsed for
>> you too.
>
> Ahhh, that is the kind of thing I'm looking for. I'll check out that
> code in detail tomorrow.
:) wext is dead
^ permalink raw reply
* Re: udevd / ext4 issue mounting 2.6.35-rc5
From: Luis R. Rodriguez @ 2010-07-26 23:43 UTC (permalink / raw)
To: Daniel J Blueman, Luis R. Rodriguez
Cc: linux-ext4, Rafael J. Wysocki, Ubuntu Kernel Team, linux-kernel,
linux-wireless
In-Reply-To: <AANLkTimTdy2DmcdXtgFvtREMX8bMADk73Oz5bwpeHw6H@mail.gmail.com>
On Fri, Jul 23, 2010 at 10:56 AM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Fri, Jul 23, 2010 at 9:49 AM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>> On Thu, Jul 22, 2010 at 2:10 AM, Daniel J Blueman
>> <daniel.blueman@gmail.com> wrote:
>>> On 22 July 2010 02:06, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>>>> On Wed, Jul 21, 2010 at 1:43 AM, Daniel J Blueman
>>>> <daniel.blueman@gmail.com> wrote:
>>>>> Hi Luis,
>>>>>
>>>>> On 21 July 2010 01:36, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>>>>>> I have been reluctant to boot to 2.6.35-rc due to the large set of
>>>>>> regression list and the amount of work I needed to actually get done
>>>>>> on 2.6.35. Last I checked the regression list it was getting small so
>>>>>> I gave it a spin today. No luck. I get some bootup error from udevd
>>>>>> and ext2/ext3/ext4, something like this:
>>>>>>
>>>>>> EXT3-fs (sda1): error: couldn't mount because of unsupported optional
>>>>>> features (240)
>>>>>> EXT2-fs (sda1): error: couldn't mount because of unsupported optional
>>>>>> features (240)
>>>>>> EXT4-fs (sda1): mounted filesystem with ordered data mode. Opts: (null)
>>>>>
>>>>> This succeeded.
>>>>
>>>> Heh, OK :)
>>>>
>>>>>> VFS: Mounted root (ext4 filesystem) readonly on device 8:1
>>>>>> Freeing unused kernel memory: 708k freed
>>>>>> Write protecting the kernel read-only data: 102040k
>>>>>> Freeing unused kernel memory: 764k freed
>>>>>> Freeing unused kernel memory: 1796k freed
>>>>>> udevd: failed to create queue file: No such file or directory
>>>>>> udevd: error creating queue file
>>>>>
>>>>> It looks like you need to enable:
>>>>>
>>>>> CONFIG_DEVTMPFS
>>>>> CONFIG_DEVTMPFS_MOUNT
>>>>
>>>> Thanks, it also turned out that when I upgraded from Ubuntu 9.10 to
>>>> Ubuntu 10.04 it replaced my own /sbin/installkernel so this was likely
>>>> another issue. My /sbin/installkernel changes allow for easy initramfs
>>>> installation on Debian/Ubuntu but my patches have been ignored my the
>>>> maintainer.
>>>>
>>>> --- installkernel-ubuntu-10.04 2010-07-21 18:03:34.607678010 -0700
>>>> +++ installkernel 2010-01-29 13:17:10.000000000 -0800
>>>> @@ -36,7 +36,8 @@
>>>> # Create backups of older versions before installing
>>>> updatever () {
>>>> if [ -f "$dir/$1-$ver" ] ; then
>>>> - mv "$dir/$1-$ver" "$dir/$1-$ver.old"
>>>> + #mv "$dir/$1-$ver" "$dir/$1-$ver.old"
>>>> + rm -f "$dir/$1-$ver" "$dir/$1-$ver.old"
>>>> fi
>>>>
>>>> cat "$2" > "$dir/$1-$ver"
>>>> @@ -75,5 +76,16 @@
>>>> if [ -f "$config" ] ; then
>>>> updatever config "$config"
>>>> fi
>>>> +
>>>> +LSB_RED_ID=$(/usr/bin/lsb_release -i -s)
>>>> +
>>>> +case $LSB_RED_ID in
>>>> +"Ubuntu")
>>>> + update-initramfs -c -k $ver
>>>> + update-grub
>>>> + ;;
>>>> +*)
>>>> + ;;
>>>> +esac
>>>>
>>>> exit 0
>>>>
>>>> But anyway I also now get another boot failure with:
>>>>
>>>> mount: mounting /dev on /root/dev failed: No such file or directory
>>>> mount: mounting /sys on /root/sys failed: No such file or directory
>>>
>>> Hmm...the scripts in the initrd are not doing what is expected -
>>> perhaps if you didn't use:
>>> linux$ fakeroot make-kpkg --append-to-version -luis1 --initrd kernel-image
>>
>> I am not using that to build my kernels I just build my kernels with
>>
>> make
>> sudo make modules_install install
>>
>>> ...or if there are eg initrd script modifications on the filesystem
>>> when it cooked the initd.
>>
>> I haven't modified any initrd scripts.
>>
>>> You could just try eg:
>>> http://archive.ubuntu.com/ubuntu/pool/main/l/linux/linux-image-2.6.35-9-generic_2.6.35-9.14_amd64.deb
>
> Fun, so that kernel actually works but the one I am building from
> wireless-testing.git does not. The curious thing is it doesn't boot
> even if I remove my 802.11 module... so something is fishy. This is
> likely a config issue. After booting with the above kernel though I
> generated a new one with
>
> make localmodconfig
>
> and then enabled my 802.11 modules. Still, no luck.. Going to reset my
> tree, I had manually merged Linus' latest stuff in but I don't think
> this should matter.
That didn't work, but it seems this was just my config, the same
config worked on older kernels but I am not motivated enough to figure
out what I actually did enable which fixed this. But just for the
record
config which did not work:
http://bombadil.infradead.org/~mcgrof/configs/2010/config-issue/config-old.txt
config which worked:
http://bombadil.infradead.org/~mcgrof/configs/2010/config-issue/config.txt
The diff:
http://bombadil.infradead.org/~mcgrof/configs/2010/config-issue/diff-34-35.patch
Luis
^ permalink raw reply
* Re: [ath9k-devel] ath9k: performance regressions / tx semi-stuck somehow
From: Björn Smedman @ 2010-07-26 23:36 UTC (permalink / raw)
To: Felix Fietkau; +Cc: ath9k-devel, linux-wireless
In-Reply-To: <AANLkTimGvPtolVncEu_csK22xvf1SIFFPusCSqxYiU-A@mail.gmail.com>
2010/7/23 Björn Smedman <bjorn.smedman@venatech.se>:
> 2010/7/22 Felix Fietkau <nbd@openwrt.org>:
>> On 2010-07-22 12:17 AM, Björn Smedman wrote:
>>> I just tried out compat-wireless-2010-07-16 on AR913x (with
>>> openwrt/trunk@r22321) and saw some weird performance problems.
>>>
>> Could you please try if the earlier version that was in OpenWrt
>> (2010-07-06) has the same issues?
I had some trouble reproducing this but now I feel convinced that this
performance issue was caused by interference, although I think ath9k
could do a better job in difficult radio environments.
Note how downstream throughput goes from ~55 Mbps down to around 3
Mbps. Nothing is moved in this experiment.
bjorn-smedmans-macbook-2:~ bjornsmedman$ iperf -w 256K -s
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 256 KByte
------------------------------------------------------------
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 58060
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-30.0 sec 194 MBytes 54.1 Mbits/sec
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 58926
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-30.0 sec 203 MBytes 56.6 Mbits/sec
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 58990
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-30.0 sec 190 MBytes 53.2 Mbits/sec
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 59236
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-30.0 sec 189 MBytes 52.8 Mbits/sec
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 64327
[Some interference source probably comes in here]
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-34.5 sec 5.64 MBytes 1.37 Mbits/sec
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 64424
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-30.3 sec 8.06 MBytes 2.23 Mbits/sec
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 64625
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-31.3 sec 13.6 MBytes 3.64 Mbits/sec
If I then switch the AP channel (from 1 to 11) performance is looking
good again:
bjorn-smedmans-macbook-2:~ bjornsmedman$ iperf -w 256K -s
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 256 KByte
------------------------------------------------------------
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 55550
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-120.0 sec 621 MBytes 43.4 Mbits/sec
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 55562
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-120.0 sec 764 MBytes 53.4 Mbits/sec
[ 4] local 192.168.78.119 port 5001 connected with 192.168.78.211 port 55568
[ ID] Interval Transfer Bandwidth
[ 4] 0.0-10.0 sec 68.1 MBytes 56.9 Mbits/sec
/Björn
^ permalink raw reply
* Re: ath9k: /proc/net/wireless always shows status of 0
From: Ben Greear @ 2010-07-26 23:36 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linux-wireless
In-Reply-To: <AANLkTimnZ9pSfVLsEPgiDPyc=_eUnyLj89RHCC6rsaX8@mail.gmail.com>
On 07/26/2010 04:30 PM, Luis R. Rodriguez wrote:
> On Mon, Jul 26, 2010 at 4:17 PM, Ben Greear<greearb@candelatech.com> wrote:
>> On 07/26/2010 03:54 PM, Luis R. Rodriguez wrote:
>>>
>>> On Mon, Jul 26, 2010 at 3:00 PM, Ben Greear<greearb@candelatech.com>
>>> wrote:
>>>>
>>>> On 07/26/2010 12:49 PM, Ben Greear wrote:
>>>>>
>>>>> This is with today's wireless-testing (with no extra hacks)
>>>>
>>>> Maybe a better question: In 2.6.31 you could mount debugfs at /debug
>>>> and get the state with:
>>>> cat /debug/ieee80211/sta0/state
>>>
>>> debugfs is not static API. In fact that's a big reason why we use it.
>>> I considered defining a filesystem API for 802.11 using configfs but
>>> our priority is to first get nl80211 feature-complete with respect to
>>> wext, which we have completed now. But anyway, debugfs is *not* API
>>> and you should not rely on it, if you want to define API consider
>>> configfs.
>>>
>>>> It seems in .34 there is only phy0 info in debugfs. Or, am I
>>>> just not looking in the right place?
>>>>
>>>> I don't mind hacking code, but I'd appreciate a pointer to the
>>>> right part of the code if anyone has suggestions.
>>>
>>> The other thing you pointed out with wext, you should stop using wext
>>> and instead rely on nl80211. It is true that if you see something on
>>> /proc/net/wireless/ that was there before but not there now it is a
>>> regression and it should be addressed. Not sure where the status crap
>>> comes from on wext.
>>
>> I really don't care what API I end up using..I am just looking for some
>> way to report to users whether the STA is in the various states that
>> used to be defined as something similar to:
>>
>> enum ieee80211_state {
>> IEEE80211_UNINITIALIZED = 0,
>> IEEE80211_INITIALIZED,
>> IEEE80211_ASSOCIATING,
>> IEEE80211_ASSOCIATED,
>> IEEE80211_AUTHENTICATING,
>> IEEE80211_AUTHENTICATED,
>> IEEE80211_SHUTDOWN
>> };
>>
>> I have been reading the net/wireless code, but so far I don't see
>> any state machine type logic where this state might be kept. Maybe
>> it would have to be inferred from something else?
>
> You can use nl28011 and register for netlink multicast messages which
> broadcast device state changes like the ones you mentioned. These come
> in on iw via event.c, see print_event() and see the case statements
> for NL80211_CMD_ASSOCIATE, NL80211_CMD_DEAUTHENTICATE,
> NL80211_CMD_DISASSOCIATE, etc, you even get reason codes parsed for
> you too.
Ahhh, that is the kind of thing I'm looking for. I'll check out that
code in detail tomorrow.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: ath9k: /proc/net/wireless always shows status of 0
From: Luis R. Rodriguez @ 2010-07-26 23:30 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless
In-Reply-To: <4C4E1784.3030702@candelatech.com>
On Mon, Jul 26, 2010 at 4:17 PM, Ben Greear <greearb@candelatech.com> wrote:
> On 07/26/2010 03:54 PM, Luis R. Rodriguez wrote:
>>
>> On Mon, Jul 26, 2010 at 3:00 PM, Ben Greear<greearb@candelatech.com>
>> wrote:
>>>
>>> On 07/26/2010 12:49 PM, Ben Greear wrote:
>>>>
>>>> This is with today's wireless-testing (with no extra hacks)
>>>
>>> Maybe a better question: In 2.6.31 you could mount debugfs at /debug
>>> and get the state with:
>>> cat /debug/ieee80211/sta0/state
>>
>> debugfs is not static API. In fact that's a big reason why we use it.
>> I considered defining a filesystem API for 802.11 using configfs but
>> our priority is to first get nl80211 feature-complete with respect to
>> wext, which we have completed now. But anyway, debugfs is *not* API
>> and you should not rely on it, if you want to define API consider
>> configfs.
>>
>>> It seems in .34 there is only phy0 info in debugfs. Or, am I
>>> just not looking in the right place?
>>>
>>> I don't mind hacking code, but I'd appreciate a pointer to the
>>> right part of the code if anyone has suggestions.
>>
>> The other thing you pointed out with wext, you should stop using wext
>> and instead rely on nl80211. It is true that if you see something on
>> /proc/net/wireless/ that was there before but not there now it is a
>> regression and it should be addressed. Not sure where the status crap
>> comes from on wext.
>
> I really don't care what API I end up using..I am just looking for some
> way to report to users whether the STA is in the various states that
> used to be defined as something similar to:
>
> enum ieee80211_state {
> IEEE80211_UNINITIALIZED = 0,
> IEEE80211_INITIALIZED,
> IEEE80211_ASSOCIATING,
> IEEE80211_ASSOCIATED,
> IEEE80211_AUTHENTICATING,
> IEEE80211_AUTHENTICATED,
> IEEE80211_SHUTDOWN
> };
>
> I have been reading the net/wireless code, but so far I don't see
> any state machine type logic where this state might be kept. Maybe
> it would have to be inferred from something else?
You can use nl28011 and register for netlink multicast messages which
broadcast device state changes like the ones you mentioned. These come
in on iw via event.c, see print_event() and see the case statements
for NL80211_CMD_ASSOCIATE, NL80211_CMD_DEAUTHENTICATE,
NL80211_CMD_DISASSOCIATE, etc, you even get reason codes parsed for
you too.
Luis
^ permalink raw reply
* Re: ath9k: /proc/net/wireless always shows status of 0
From: Ben Greear @ 2010-07-26 23:17 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linux-wireless
In-Reply-To: <AANLkTi=qJXCxtMZcEm3VQT1s7ioJsdhj2kJzkgGSf3ND@mail.gmail.com>
On 07/26/2010 03:54 PM, Luis R. Rodriguez wrote:
> On Mon, Jul 26, 2010 at 3:00 PM, Ben Greear<greearb@candelatech.com> wrote:
>> On 07/26/2010 12:49 PM, Ben Greear wrote:
>>>
>>> This is with today's wireless-testing (with no extra hacks)
>>
>> Maybe a better question: In 2.6.31 you could mount debugfs at /debug
>> and get the state with:
>> cat /debug/ieee80211/sta0/state
>
> debugfs is not static API. In fact that's a big reason why we use it.
> I considered defining a filesystem API for 802.11 using configfs but
> our priority is to first get nl80211 feature-complete with respect to
> wext, which we have completed now. But anyway, debugfs is *not* API
> and you should not rely on it, if you want to define API consider
> configfs.
>
>> It seems in .34 there is only phy0 info in debugfs. Or, am I
>> just not looking in the right place?
>>
>> I don't mind hacking code, but I'd appreciate a pointer to the
>> right part of the code if anyone has suggestions.
>
> The other thing you pointed out with wext, you should stop using wext
> and instead rely on nl80211. It is true that if you see something on
> /proc/net/wireless/ that was there before but not there now it is a
> regression and it should be addressed. Not sure where the status crap
> comes from on wext.
I really don't care what API I end up using..I am just looking for some
way to report to users whether the STA is in the various states that
used to be defined as something similar to:
enum ieee80211_state {
IEEE80211_UNINITIALIZED = 0,
IEEE80211_INITIALIZED,
IEEE80211_ASSOCIATING,
IEEE80211_ASSOCIATED,
IEEE80211_AUTHENTICATING,
IEEE80211_AUTHENTICATED,
IEEE80211_SHUTDOWN
};
I have been reading the net/wireless code, but so far I don't see
any state machine type logic where this state might be kept. Maybe
it would have to be inferred from something else?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Luis R. Rodriguez @ 2010-07-26 22:56 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Matthew Garrett, Luis R. Rodriguez, David Quan,
ath5k-devel@lists.ath5k.org, linux-wireless@vger.kernel.org,
linux-kernel, David@venema.h4ckr.net,
kernel-team@lists.ubuntu.com, Jussi Kivilinna,
tim.gardner@canonical.com
In-Reply-To: <20100726201322.GI14855@tux>
On Mon, Jul 26, 2010 at 01:13:22PM -0700, Luis R. Rodriguez wrote:
> On Sat, Jun 19, 2010 at 08:32:44AM -0700, Maxim Levitsky wrote:
> > On Sat, 2010-06-19 at 16:02 +0300, Maxim Levitsky wrote:
> > > On Sat, 2010-06-19 at 08:38 -0400, Bob Copeland wrote:
> > > > On Sat, Jun 19, 2010 at 10:49:34AM +0300, Maxim Levitsky wrote:
> > > > > How this patch?
> > > >
> > > > Looks fine to me. Some nitpicking below but feel free to add my
> > > >
> > > > Acked-by: Bob Copeland <me@bobcopeland.com>
> > > >
> > Done.
> >
> > Best regards,
> > Maxim Levitsky
> >
> > ---
> >
> > commit 616afa397b3e843f2aba06be12a30e72dfff7740
> > Author: Maxim Levitsky <maximlevitsky@gmail.com>
> > Date: Thu Jun 17 23:21:42 2010 +0300
> >
> > ath5k: disable ASPM
> >
> > Atheros card on Acer Aspire One (AOA150, Atheros Communications Inc. AR5001
> > Wireless Network Adapter [168c:001c] (rev 01)) doesn't work well with ASPM
> > enabled. With ASPM ath5k will eventually stall on heavy traffic with often
> > 'unsupported jumbo' warnings appearing. Disabling ASPM L0s in ath5k fixes
> > these problems.
> > Also card sends a storm of RXORN interrupts even though medium is idle.
> >
> > Reproduced with pcie_aspm=force and by using 'nc < /dev/zero > /dev/null' at
> > both ends (usually stalls within seconds).
> >
> > Unfortunately BIOS enables ASPM on this card by default on these machines
> > This means that, problem shows up (less often) without pcie_aspm=force too.
> > Therefore to benefit from this fix you need to _enable_ CONFIG_PCIEASPM
> >
> >
> > All credit for this patch goes to Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
> > for finding and fixing this bug.
> >
> > Based on patch that is
> > From: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
> >
> >
> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> > Acked-by: Bob Copeland <me@bobcopeland.com>
>
> Acked-by: Luis R. Rodriguez <lrodriguez@atheros.com>
So NACK now, to really fix this correctly its best to instead disable L0s but
to force enable also L1 if the device is a PCI Express device. All Atheros legacy
devices (ath5k) should work correctly with L1. Kernels with CONFIG_PCIEASPM
would end up disabling even L1 for pre PCI 1.1 devices.
Luis
^ permalink raw reply
* Re: ath9k: /proc/net/wireless always shows status of 0
From: Luis R. Rodriguez @ 2010-07-26 22:54 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless
In-Reply-To: <4C4E0580.3060404@candelatech.com>
On Mon, Jul 26, 2010 at 3:00 PM, Ben Greear <greearb@candelatech.com> wrote:
> On 07/26/2010 12:49 PM, Ben Greear wrote:
>>
>> This is with today's wireless-testing (with no extra hacks)
>
> Maybe a better question: In 2.6.31 you could mount debugfs at /debug
> and get the state with:
> cat /debug/ieee80211/sta0/state
debugfs is not static API. In fact that's a big reason why we use it.
I considered defining a filesystem API for 802.11 using configfs but
our priority is to first get nl80211 feature-complete with respect to
wext, which we have completed now. But anyway, debugfs is *not* API
and you should not rely on it, if you want to define API consider
configfs.
> It seems in .34 there is only phy0 info in debugfs. Or, am I
> just not looking in the right place?
>
> I don't mind hacking code, but I'd appreciate a pointer to the
> right part of the code if anyone has suggestions.
The other thing you pointed out with wext, you should stop using wext
and instead rely on nl80211. It is true that if you see something on
/proc/net/wireless/ that was there before but not there now it is a
regression and it should be addressed. Not sure where the status crap
comes from on wext.
Luis
^ permalink raw reply
* [PATCH] mac80211: Fix key freeing to handle unlinked keys
From: Jouni Malinen @ 2010-07-26 22:52 UTC (permalink / raw)
To: Johannes Berg; +Cc: John Linville, linux-wireless
In-Reply-To: <1280154447.3693.9.camel@jlt3.sipsolutions.net>
Key locking simplification removed key->sdata != NULL verification from
ieee80211_key_free(). While that is fine for most use cases, there is one
path where this function can be called with an unlinked key (i.e.,
key->sdata == NULL && key->local == NULL). This results in a NULL pointer
dereference with the current implementation. This is known to happen at
least with FT protocol when wpa_supplicant tries to configure the key
before association.
Avoid the issue by passing in the local pointer to
ieee80211_key_free(). In addition, do not clear the key from hw_accel
or debugfs if it has not yet been added. At least the hw_accel one could
trigger another NULL pointer dereference.
Signed-off-by: Jouni Malinen <j@w1.fi>
---
net/mac80211/cfg.c | 6 +++---
net/mac80211/key.c | 13 ++++++-------
net/mac80211/key.h | 3 ++-
net/mac80211/sta_info.c | 2 +-
4 files changed, 12 insertions(+), 12 deletions(-)
(Note: The reference to key locking simplification is to the commit
ad0e2b5a00dbec303e4682b403bb6703d11dcdb2 which is not yet in linux-2.6.)
--- wireless-testing.orig/net/mac80211/cfg.c 2010-07-26 14:44:01.000000000 -0700
+++ wireless-testing/net/mac80211/cfg.c 2010-07-26 14:44:43.000000000 -0700
@@ -158,7 +158,7 @@ static int ieee80211_add_key(struct wiph
if (mac_addr) {
sta = sta_info_get_bss(sdata, mac_addr);
if (!sta) {
- ieee80211_key_free(key);
+ ieee80211_key_free(sdata->local, key);
err = -ENOENT;
goto out_unlock;
}
@@ -192,7 +192,7 @@ static int ieee80211_del_key(struct wiph
goto out_unlock;
if (sta->key) {
- ieee80211_key_free(sta->key);
+ ieee80211_key_free(sdata->local, sta->key);
WARN_ON(sta->key);
ret = 0;
}
@@ -205,7 +205,7 @@ static int ieee80211_del_key(struct wiph
goto out_unlock;
}
- ieee80211_key_free(sdata->keys[key_idx]);
+ ieee80211_key_free(sdata->local, sdata->keys[key_idx]);
WARN_ON(sdata->keys[key_idx]);
ret = 0;
--- wireless-testing.orig/net/mac80211/key.c 2010-07-26 14:43:07.000000000 -0700
+++ wireless-testing/net/mac80211/key.c 2010-07-26 14:44:43.000000000 -0700
@@ -323,13 +323,15 @@ static void __ieee80211_key_destroy(stru
if (!key)
return;
- ieee80211_key_disable_hw_accel(key);
+ if (key->local)
+ ieee80211_key_disable_hw_accel(key);
if (key->conf.alg == ALG_CCMP)
ieee80211_aes_key_free(key->u.ccmp.tfm);
if (key->conf.alg == ALG_AES_CMAC)
ieee80211_aes_cmac_key_free(key->u.aes_cmac.tfm);
- ieee80211_debugfs_key_remove(key);
+ if (key->local)
+ ieee80211_debugfs_key_remove(key);
kfree(key);
}
@@ -410,15 +412,12 @@ static void __ieee80211_key_free(struct
__ieee80211_key_destroy(key);
}
-void ieee80211_key_free(struct ieee80211_key *key)
+void ieee80211_key_free(struct ieee80211_local *local,
+ struct ieee80211_key *key)
{
- struct ieee80211_local *local;
-
if (!key)
return;
- local = key->sdata->local;
-
mutex_lock(&local->key_mtx);
__ieee80211_key_free(key);
mutex_unlock(&local->key_mtx);
--- wireless-testing.orig/net/mac80211/key.h 2010-07-26 14:43:07.000000000 -0700
+++ wireless-testing/net/mac80211/key.h 2010-07-26 14:44:43.000000000 -0700
@@ -135,7 +135,8 @@ struct ieee80211_key *ieee80211_key_allo
void ieee80211_key_link(struct ieee80211_key *key,
struct ieee80211_sub_if_data *sdata,
struct sta_info *sta);
-void ieee80211_key_free(struct ieee80211_key *key);
+void ieee80211_key_free(struct ieee80211_local *local,
+ struct ieee80211_key *key);
void ieee80211_set_default_key(struct ieee80211_sub_if_data *sdata, int idx);
void ieee80211_set_default_mgmt_key(struct ieee80211_sub_if_data *sdata,
int idx);
--- wireless-testing.orig/net/mac80211/sta_info.c 2010-07-26 14:43:07.000000000 -0700
+++ wireless-testing/net/mac80211/sta_info.c 2010-07-26 14:44:43.000000000 -0700
@@ -647,7 +647,7 @@ static int __must_check __sta_info_destr
return ret;
if (sta->key) {
- ieee80211_key_free(sta->key);
+ ieee80211_key_free(local, sta->key);
WARN_ON(sta->key);
}
--
Jouni Malinen PGP id EFC895FA
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Matthew Garrett @ 2010-07-26 22:50 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Maxim Levitsky, ath5k-devel@lists.ath5k.org,
linux-wireless@vger.kernel.org, David Quan, Luis R. Rodriguez,
linux-kernel, kernel-team@lists.ubuntu.com, Luis Rodriguez,
Jussi Kivilinna, tim.gardner@canonical.com
In-Reply-To: <AANLkTinChEx_B9wa-n+a6xDr1vLOBOD3Mhcs=iqcYwen@mail.gmail.com>
On Mon, Jul 26, 2010 at 03:43:04PM -0700, Luis R. Rodriguez wrote:
> I see.. thanks Mathew... in that case since L1 works on all devices we
> could just force enable L1 for all PCIE devices. What do you think?
Works for me.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Luis R. Rodriguez @ 2010-07-26 22:43 UTC (permalink / raw)
To: Matthew Garrett
Cc: Maxim Levitsky, ath5k-devel@lists.ath5k.org,
linux-wireless@vger.kernel.org, David Quan, Luis R. Rodriguez,
linux-kernel, kernel-team@lists.ubuntu.com, Luis Rodriguez,
Jussi Kivilinna, tim.gardner@canonical.com
In-Reply-To: <20100726223326.GA6904@srcf.ucam.org>
On Mon, Jul 26, 2010 at 3:33 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Mon, Jul 26, 2010 at 03:31:28PM -0700, Luis R. Rodriguez wrote:
>> On Mon, Jul 26, 2010 at 3:29 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>> > On Mon, Jul 26, 2010 at 03:26:37PM -0700, Luis R. Rodriguez wrote:
>> >
>> >> What I meant was that the PCI config space would already have L1
>> >> enabled if L1 worked, so I don't see why we would need to nitpick out
>> >> specifics here. All Atheros PCIE chips should work with L1. The advise
>> >> given is to disable L0s though. I believe AR2425 would be one which
>> >> likely had L0s enabled but requires it to be disabled. Not sure of
>> >> others. But this is why I am saying this can be done globally for all
>> >> ath5k chipsets.
>> >
>> > If L1 is set but the chip is pre-PCIe 1.1 then we'll disable L1 unless
>> > the driver tells us that it's functional. The .inf from the Windows
>> > driver seemed to suggest that only a subset of the chips re-enabled L1
>> > there, but if it's ok in general then that's a straightforward one-line
>> > patch.
>>
>> But why can't we just rely on what the device already has on its PCI
>> config space and only ensure to disable L0s?
>
> Because we globally disable ASPM on pre-1.1 devices, because that's what
> Windows does. It makes it easier for us to figure out what level of
> support we can expect from different hardware revisions.
I see.. thanks Mathew... in that case since L1 works on all devices we
could just force enable L1 for all PCIE devices. What do you think?
Luis
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Matthew Garrett @ 2010-07-26 22:33 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Maxim Levitsky, ath5k-devel@lists.ath5k.org,
linux-wireless@vger.kernel.org, David Quan, Luis R. Rodriguez,
linux-kernel, kernel-team@lists.ubuntu.com, Luis Rodriguez,
Jussi Kivilinna, tim.gardner@canonical.com
In-Reply-To: <AANLkTinysGjYS7RKN4X_7T2q+rw_-OWdn-UJYM0poWXu@mail.gmail.com>
On Mon, Jul 26, 2010 at 03:31:28PM -0700, Luis R. Rodriguez wrote:
> On Mon, Jul 26, 2010 at 3:29 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > On Mon, Jul 26, 2010 at 03:26:37PM -0700, Luis R. Rodriguez wrote:
> >
> >> What I meant was that the PCI config space would already have L1
> >> enabled if L1 worked, so I don't see why we would need to nitpick out
> >> specifics here. All Atheros PCIE chips should work with L1. The advise
> >> given is to disable L0s though. I believe AR2425 would be one which
> >> likely had L0s enabled but requires it to be disabled. Not sure of
> >> others. But this is why I am saying this can be done globally for all
> >> ath5k chipsets.
> >
> > If L1 is set but the chip is pre-PCIe 1.1 then we'll disable L1 unless
> > the driver tells us that it's functional. The .inf from the Windows
> > driver seemed to suggest that only a subset of the chips re-enabled L1
> > there, but if it's ok in general then that's a straightforward one-line
> > patch.
>
> But why can't we just rely on what the device already has on its PCI
> config space and only ensure to disable L0s?
Because we globally disable ASPM on pre-1.1 devices, because that's what
Windows does. It makes it easier for us to figure out what level of
support we can expect from different hardware revisions.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Luis R. Rodriguez @ 2010-07-26 22:31 UTC (permalink / raw)
To: Matthew Garrett
Cc: Maxim Levitsky, ath5k-devel@lists.ath5k.org,
linux-wireless@vger.kernel.org, David Quan, Luis R. Rodriguez,
linux-kernel, kernel-team@lists.ubuntu.com, Luis Rodriguez,
Jussi Kivilinna, tim.gardner@canonical.com
In-Reply-To: <20100726222909.GA6773@srcf.ucam.org>
On Mon, Jul 26, 2010 at 3:29 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Mon, Jul 26, 2010 at 03:26:37PM -0700, Luis R. Rodriguez wrote:
>
>> What I meant was that the PCI config space would already have L1
>> enabled if L1 worked, so I don't see why we would need to nitpick out
>> specifics here. All Atheros PCIE chips should work with L1. The advise
>> given is to disable L0s though. I believe AR2425 would be one which
>> likely had L0s enabled but requires it to be disabled. Not sure of
>> others. But this is why I am saying this can be done globally for all
>> ath5k chipsets.
>
> If L1 is set but the chip is pre-PCIe 1.1 then we'll disable L1 unless
> the driver tells us that it's functional. The .inf from the Windows
> driver seemed to suggest that only a subset of the chips re-enabled L1
> there, but if it's ok in general then that's a straightforward one-line
> patch.
But why can't we just rely on what the device already has on its PCI
config space and only ensure to disable L0s?
Luis
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Luis R. Rodriguez @ 2010-07-26 22:29 UTC (permalink / raw)
To: Matthew Garrett
Cc: Luis R. Rodriguez, linux-wireless@vger.kernel.org, David Quan,
ath5k-devel@lists.ath5k.org, linux-kernel,
kernel-team@lists.ubuntu.com, Luis Rodriguez, Jussi Kivilinna,
tim.gardner@canonical.com
In-Reply-To: <20100726222451.GB6487@srcf.ucam.org>
On Mon, Jul 26, 2010 at 3:24 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Mon, Jul 26, 2010 at 03:20:23PM -0700, Luis R. Rodriguez wrote:
>> On Mon, Jul 26, 2010 at 2:14 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>> > On Mon, Jul 26, 2010 at 02:06:51PM -0700, Luis R. Rodriguez wrote:
>> >
>> >> No, ASPM must be enabled by the Systems Integrator through the BIOS, there are
>> >> other settings that have to be taken care of like modifying some PCI entrance and
>> >> exit latency timers, the number of FTS packets we send to exit L0s, amongst
>> >> other things. If a user selectively enables L1 but the BIOS had it disabled on
>> >> the device it may not work correctly.
>> >
>> > That's really the job of the driver.
>>
>> The problem is that sometimes tweaks need to be done on the PCI
>> controller/root complex, not the PCIE device/endpoint. Today these
>> sort of changes *are* handled by the respective systems
>> integrator/BIOS team and varies depending on the root complex used.
>> Atheros does not handle these at all in the driver.
>
> Really? Your Windows drivers declare that they support ASPM for some
> parts, which will trigger Vista and later to program L0s and L1
> (depending on system config) without further reference to the driver. If
> we need hooks in the kernel for drivers to provide further feedback to
> make sure this works correctly then we can certainly provide them...
Sure, agreed, I'm just pointing out today there are other changes made
and even Microsoft does not get involved with them, so it is the same
for us. However I do agree if the kernel can expose API to tweak
things then great.
>> > If the ASPM policy is set to
>> > powersave, the fadt doesn't indicate that ASPM should be disabled and
>> > the bus's _OSC method grants full control then the kernel will enable
>> > whatever combination of L states meet the latency constraints. If the
>> > hardware has additional constraints then the hardware-specific driver
>> > needs to handle them.
>>
>> This makes sense but Is there an API for this?
>
> Right now, no. What do you need?
Not sure, let me check internally and see if I can get the exact details.
>> > We don't rely on the BIOS to set up ASPM states. Nor does Windows.
>>
>> Understood, but today some tweaks seem to be done on the BIOS
>> depending on the endpoint / root complex.
>
> The current situation is that we won't modify anything that the BIOS has
> done, other than the link and clock pm bits. So if the BIOS has done
> something for us, we won't (currently) step on it.
Sure, understood.
Luis
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Matthew Garrett @ 2010-07-26 22:29 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Maxim Levitsky, ath5k-devel@lists.ath5k.org,
linux-wireless@vger.kernel.org, David Quan, Luis R. Rodriguez,
linux-kernel, kernel-team@lists.ubuntu.com, Luis Rodriguez,
Jussi Kivilinna, tim.gardner@canonical.com
In-Reply-To: <AANLkTinq-SxMHTrJM23WaeRMkU-2cySiVrkwCjDRS_o+@mail.gmail.com>
On Mon, Jul 26, 2010 at 03:26:37PM -0700, Luis R. Rodriguez wrote:
> What I meant was that the PCI config space would already have L1
> enabled if L1 worked, so I don't see why we would need to nitpick out
> specifics here. All Atheros PCIE chips should work with L1. The advise
> given is to disable L0s though. I believe AR2425 would be one which
> likely had L0s enabled but requires it to be disabled. Not sure of
> others. But this is why I am saying this can be done globally for all
> ath5k chipsets.
If L1 is set but the chip is pre-PCIe 1.1 then we'll disable L1 unless
the driver tells us that it's functional. The .inf from the Windows
driver seemed to suggest that only a subset of the chips re-enabled L1
there, but if it's ok in general then that's a straightforward one-line
patch.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Luis R. Rodriguez @ 2010-07-26 22:26 UTC (permalink / raw)
To: Matthew Garrett
Cc: Maxim Levitsky, ath5k-devel@lists.ath5k.org,
linux-wireless@vger.kernel.org, David Quan, Luis R. Rodriguez,
linux-kernel, kernel-team@lists.ubuntu.com, Luis Rodriguez,
Jussi Kivilinna, tim.gardner@canonical.com
In-Reply-To: <20100726222113.GA6487@srcf.ucam.org>
On Mon, Jul 26, 2010 at 3:21 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Mon, Jul 26, 2010 at 03:15:32PM -0700, Luis R. Rodriguez wrote:
>> On Mon, Jul 26, 2010 at 2:25 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>> > This may need to be done on a chip by chip basis. Take a look at
>> > http://www.atheros.cz/inffile.php?inf=68&bit=32&atheros=AR5002G&system=4
>> > and some of the other inf files on that site to see which devices
>> > provide the PciASPMOptIn flag - those should support ASPM states even if
>> > they're pre-1.1 devices.
>>
>> I rather we not bother with these, lets simply follow the kernel's
>> lead here for its rule matching.
>
> Sorry? The idea is to indicate which chips support ASPM even though
> they're pre-PCIe 1.1. If all Atheros parts work fine with L1 then that
> makes things much easier, but it would be good to know the correct set
> of chips that are broken with L0s.
What I meant was that the PCI config space would already have L1
enabled if L1 worked, so I don't see why we would need to nitpick out
specifics here. All Atheros PCIE chips should work with L1. The advise
given is to disable L0s though. I believe AR2425 would be one which
likely had L0s enabled but requires it to be disabled. Not sure of
others. But this is why I am saying this can be done globally for all
ath5k chipsets.
Luis
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Matthew Garrett @ 2010-07-26 22:24 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Maxim Levitsky, Luis Rodriguez, Bob Copeland, Luis R. Rodriguez,
Jussi Kivilinna, ath5k-devel@lists.ath5k.org,
linux-wireless@vger.kernel.org, linux-kernel,
kernel-team@lists.ubuntu.com, tim.gardner@canonical.com,
David Quan
In-Reply-To: <AANLkTimn1XvpfUrgdgkO-E3kX55fz7N8w+_24Lz1ySLn@mail.gmail.com>
On Mon, Jul 26, 2010 at 03:20:23PM -0700, Luis R. Rodriguez wrote:
> On Mon, Jul 26, 2010 at 2:14 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > On Mon, Jul 26, 2010 at 02:06:51PM -0700, Luis R. Rodriguez wrote:
> >
> >> No, ASPM must be enabled by the Systems Integrator through the BIOS, there are
> >> other settings that have to be taken care of like modifying some PCI entrance and
> >> exit latency timers, the number of FTS packets we send to exit L0s, amongst
> >> other things. If a user selectively enables L1 but the BIOS had it disabled on
> >> the device it may not work correctly.
> >
> > That's really the job of the driver.
>
> The problem is that sometimes tweaks need to be done on the PCI
> controller/root complex, not the PCIE device/endpoint. Today these
> sort of changes *are* handled by the respective systems
> integrator/BIOS team and varies depending on the root complex used.
> Atheros does not handle these at all in the driver.
Really? Your Windows drivers declare that they support ASPM for some
parts, which will trigger Vista and later to program L0s and L1
(depending on system config) without further reference to the driver. If
we need hooks in the kernel for drivers to provide further feedback to
make sure this works correctly then we can certainly provide them...
> > If the ASPM policy is set to
> > powersave, the fadt doesn't indicate that ASPM should be disabled and
> > the bus's _OSC method grants full control then the kernel will enable
> > whatever combination of L states meet the latency constraints. If the
> > hardware has additional constraints then the hardware-specific driver
> > needs to handle them.
>
> This makes sense but Is there an API for this?
Right now, no. What do you need?
> > We don't rely on the BIOS to set up ASPM states. Nor does Windows.
>
> Understood, but today some tweaks seem to be done on the BIOS
> depending on the endpoint / root complex.
The current situation is that we won't modify anything that the BIOS has
done, other than the link and clock pm bits. So if the BIOS has done
something for us, we won't (currently) step on it.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* Re: [ath5k-devel] [PATCH v3] ath5k: disable ASPM
From: Matthew Garrett @ 2010-07-26 22:21 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Maxim Levitsky, ath5k-devel@lists.ath5k.org,
linux-wireless@vger.kernel.org, David Quan, Luis R. Rodriguez,
linux-kernel, kernel-team@lists.ubuntu.com, Luis Rodriguez,
Jussi Kivilinna, tim.gardner@canonical.com
In-Reply-To: <AANLkTi=5_Dc8SumPi7cr8u7=DmnOHgSo6C76w9b0E4Kj@mail.gmail.com>
On Mon, Jul 26, 2010 at 03:15:32PM -0700, Luis R. Rodriguez wrote:
> On Mon, Jul 26, 2010 at 2:25 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > This may need to be done on a chip by chip basis. Take a look at
> > http://www.atheros.cz/inffile.php?inf=68&bit=32&atheros=AR5002G&system=4
> > and some of the other inf files on that site to see which devices
> > provide the PciASPMOptIn flag - those should support ASPM states even if
> > they're pre-1.1 devices.
>
> I rather we not bother with these, lets simply follow the kernel's
> lead here for its rule matching.
Sorry? The idea is to indicate which chips support ASPM even though
they're pre-PCIe 1.1. If all Atheros parts work fine with L1 then that
makes things much easier, but it would be good to know the correct set
of chips that are broken with L0s.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ 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