> Hello,
>
> It happened that since Emmanuel's commit
> 3afc2167f60a327a2c1e1e2600ef209a3c2b75b7 (cfg80211/mac80211: ignore
> signal if the frame was heard on wrong channel) my atmel-based wifi
> device couldn't scan anymore.
>
> I looked at this issue and I found the cause, but currently I have
> some doubts, and I would like asking for clarification and comments,
> before trying to patch the driver or mac80211 itself....
>
> The problem is triggered because the at76x50x-usb driver does not
> provide frequency information in rx_status (and AFAIK it difficultly
> could do that during scan right now).
I'd say this is a bad behavior, but I am not the authoritative opinion here :)
>
> Emmanuel's commit make certain mac80211 functions stop getting the
> channel information from the beacon/probe response, and start getting
> it from rx_status, but the channel results now NULL, and the frame is
> discarded.
> The relevant check is done in mlme.c, ieee80211_rx_bss_info
> if (!channel)
> return;
> just after Emmanuel's change.
>
> So my first question is:
> Are mac80211 drivers obligated to report this information?
> Does this commit just rely on something that should be already in that
> way (and the at7950x-usb driver is simply buggy not doing this)?
> Reading Emmanuel's commit message it seems he didn't have any
> intention to introduce this constraint now.
Right - but the at7950x-usb doesn't provide this information, then the channel is taken from the DS IE - only if it exists, so what if this IE doesn't exist?
>
> Said that, I dig in mac80211 code, and I saw Emmanuel commit does
> affect frames processed in sta (and ibss, but i didn't look at this)
> rx path.
>
> Summarizing it a bit, the call path should be:
> __ieee80211_rx_handle_packet()
> ieee80211_invoke_rx_handlers()
> ieee80211_rx_h_mgmt()
> -- work queue --
> ieee80211_iface_work()
> ieee80211_sta_rx_queued_mgmt()
> ieee80211_rx_bss_info() - patched to get channel from driver's rx_status.
> And finally
> ieee80211_bss_info_update()
>
> is this right?
>
> What is surprising for me is that
> __ieee80211_rx_handle_packet()
> contains a shorter path for updating BSS information by
> ieee80211_scan_rx() that directly calls
> ieee80211_bss_info_update()
Yes - the frame comes to the bss update from 2 paths when we are scanning.
>
> The interesting thing is that ieee80211_scan_rx was already written in
> the way Emmanuel's patched ieee80211_rx_bss_info(): It get channel
> info from rx_status.
>
> This suggests that the at79c50x-usb driver, prior to Emmanuel's
> commit, was able to scan by getting mgmt frames processed by the
> former, longer, RX path, and that the ieee80211_scan_rx() path was
> never really used.
Yep.
>
> If this is correct, my question is:
> Isn't this a redundant path duplication (provided that drivers
> supplies channel information)?
This has been discussed in the mailing list already, but yes.
>
> This seems to be confirmed by the fact that if I modify mac80211 to
> avoid discarding frame without channel information (by putting a valid
> channel information directly in mac80211 just before the check for
> NULL), the scan works by either doing this in ieee80211_scan_rx() or
> in ieee80211_rx_bss_info() (reverting Emmanuel change in mlme.c).
Can you try this:
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 3fb07e9..9d2f0d0 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2791,8 +2791,13 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
sdata_assert_lock(sdata);
channel = ieee80211_get_channel(local->hw.wiphy, rx_status->freq);
- if (!channel)
- return;
+ if (!channel && elems->ds_params) {
+ int freq = ieee80211_channel_to_frequency(elems->ds_params[0],
+ rx_status->band);
+ channel = ieee80211_get_channel(local->hw.wiphy, freq);
+ if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
+ return;
+ }
bss = ieee80211_bss_info_update(local, rx_status, mgmt, len, elems,
channel);
>
> Finally my latest question is:
> Emmanuel's patch makes cfg80211_inform_bss_width_frame() compare the
> RX channel and the actual BSS channel (known from beacon/probe
> response), and it does this by comparing the two pointers.
>
> Are we guaranteed that only one instance of every channel object does
> exists ? isn't it safer to compare the frequency field value?
No - this is fine. We do that all over the stack.
��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
