On Wed, May 02, 2018 at 11:52:18AM +0200, Stefan Sperling wrote:
> On Wed, May 02, 2018 at 12:30:30PM +0300, Paul Irofti wrote:
> > On Mon, Apr 30, 2018 at 10:55:22AM +0200, Stefan Sperling wrote:
> > > + /*
> > > + * During a scan on 5Ghz, prefer RSSI measured for probe
> > > + * response frames. i.e. don't allow beacons to lower the
> > > + * measured RSSI. Some 5GHz APs send beacons with much
> > > + * less Tx power than they use for probe responses.
> > > + */
> > > + if (isprobe)
> >
> > Properly indent the if clause here
> >
> > > + ni->ni_rssi = rxi->rxi_rssi;
> > > + else if (ni->ni_rssi < rxi->rxi_rssi)
> >
> > Can't this be an OR in the former if clause?
>
> Yes it could.
>
> > > + ni->ni_rssi = rxi->rxi_rssi;
> > > + } else
> > > + ni->ni_rssi = rxi->rxi_rssi;
> >
> > And actually, can't all of this be turned into a single if clause? :)
> > Maybe I am reading this wrong, but aren't you setting everywhere
> > ni->ni_rssi to rxi->rxi_rssi?
>
> No. I don't update ni_rssi if the frame is a beacon (isprobe == false)
> which reduces an already recorded rssi value.
>
> > I am a bit confused why this did not work before (when you were setting
> > the value to rxi_rssi no matter what) and why this extra checking fixed
> > it.
>
> It didn't always work before because a beacon is not a probe response.
> Both types of frames contain the same data but semantics are different:
> The AP sends beacons at a fixed interval, and a probe response only
> when it has received a probe request from us.
> Sending probe requests during a scan is also known as an "active scan"
> (client performs an active tranmission), as opposed to a "passive scan"
> which only listens for beacons (client makes no tranmission).
>
> The issue observed is that this AP is sending beacons with a much
> lower amount of transmit power than probe responses.
Got it, thanks!
I tried to contract the if clauses,
if (!(ic->ic_state == IEEE80211_S_SCAN &&
IEEE80211_IS_CHAN_5GHZ(ni->ni_chan) &&
(isprobe || ni->ni_rssi < rxi->rxi_rssi)))
ni->ni_rssi = rxi->rxi_rssi;
but I think what you have now is most readable.
OK pirofti@