On Thu, 2011-05-05 at 16:00 +0300, Luciano Coelho wrote:
> +TRACE_EVENT(drv_sched_scan_start,
> + TP_PROTO(struct ieee80211_local *local,
> + struct ieee80211_sub_if_data *sdata,
> + struct cfg80211_sched_scan_request *req),
Since you're not tracing "req" (right now), could you please use
DECLARE_EVENT_CLASS() and define a common class for this and some others
that only have local and sdata?
> +TRACE_EVENT(api_sched_scan_results,
> + TP_PROTO(struct ieee80211_local *local),
and use the local_only_evt class here, cf. drv_start() for example.
> +TRACE_EVENT(api_sched_scan_stopped,
> + TP_PROTO(struct ieee80211_local *local),
ditto.
Also -- no API tracers for results/stopped?
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index 6187766..9a20671 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -47,7 +47,7 @@ void ieee80211_configure_filter(struct ieee80211_local *local)
> if (atomic_read(&local->iff_allmultis))
> new_flags |= FIF_ALLMULTI;
>
> - if (local->monitors || local->scanning)
> + if (local->monitors || local->scanning || local->sched_scanning)
> new_flags |= FIF_BCN_PRBRESP_PROMISC;
Hmm, that seems like an implementation detail of the driver in this
case. I'd rather not do this here, other drivers might not have a
special filter they need for scheduled scans.
> @@ -358,7 +358,8 @@ static void ieee80211_restart_work(struct work_struct *work)
> flush_workqueue(local->workqueue);
>
> mutex_lock(&local->mtx);
> - WARN(test_bit(SCAN_HW_SCANNING, &local->scanning),
> + WARN(test_bit(SCAN_HW_SCANNING, &local->scanning) ||
> + local->sched_scanning,
> "%s called with hardware scan in progress\n", __func__);
> mutex_unlock(&local->mtx);
Good catch, I should've thought of that in the remain-on-channel stuff,
would've saved me some debugging :)
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 599fa90..1158164 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -408,7 +408,8 @@ ieee80211_rx_h_passive_scan(struct ieee80211_rx_data *rx)
> return RX_CONTINUE;
>
> if (test_bit(SCAN_HW_SCANNING, &local->scanning) ||
> - test_bit(SCAN_SW_SCANNING, &local->scanning))
> + test_bit(SCAN_SW_SCANNING, &local->scanning) ||
> + local->sched_scanning)
> return ieee80211_scan_rx(rx->sdata, skb);
I'm still not convinced that this is right.
> @@ -2801,7 +2802,8 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
> local->dot11ReceivedFragmentCount++;
>
> if (unlikely(test_bit(SCAN_HW_SCANNING, &local->scanning) ||
> - test_bit(SCAN_SW_SCANNING, &local->scanning)))
> + test_bit(SCAN_SW_SCANNING, &local->scanning) ||
> + local->sched_scanning))
> status->rx_flags |= IEEE80211_RX_IN_SCAN;
Nor this I guess, though we don't use this much of course.
> + ret = drv_sched_scan_start(local, sdata, req,
> + &local->sched_scan_ies);
> + if (!ret)
> + local->sched_scanning = true;
> +out:
> + mutex_unlock(&sdata->local->mtx);
> +
> + return ret;
> +
> +out_free:
> + while (i > 0)
> + kfree(local->sched_scan_ies.ie[--i]);
> + goto out;
Not freeing it when the driver callback fails?
Seems like you should reorder:
ret = drv...();
if (ret == 0) {
... = true;
goto out;
}
out_free:
...
out:
...
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
