On 2018 May 01 (Tue) at 11:20:54 +0200 (+0200), Stefan Sperling wrote:
:On Mon, Apr 30, 2018 at 08:57:23AM +0200, Peter Hessler wrote:
:> On 2018 Apr 29 (Sun) at 11:51:26 +0200 (+0200), Stefan Sperling wrote:
:> :This diff tries to avoid situations where background scans play
:> :ping-pong between different APs with nearly equal RSSI, as
:> :observed by phessler.
:> :
:> :Not all drivers represent RSSI values in dBm or percentage, so the
:> :diff includes the possibility for drivers to override the new RSSI
:> :comparison function. However, since the threshold is rather low
:> :applying this to all drivers for now should not do any harm, unless
:> :there is a driver where the RSSI value range is ridiculously small.
:> :I'm not aware of any such driver at present.
:> :
:>
:> I'm concerned about two things.
:>
:> The usage is confusing, because you pass two incompatible things to be
:> compared. I would prefer ieee80211_node_cmprssi(ic, rssi_one, rssi_two).
:
:Agreed. Updated diff below. I've chosen to make it compare two nodes
:since the function lives in the nodes namespace. It makes the function
:a bit less flexible as it cannot be used to compare naked rssi values.
:But that's not really needed for the problem at hand; after all, we
:want to compare APs.
:
:> Also, in the case where ni->ni_rssi has a very weak signal, the second
:> comparison can underflow.
:
:Indeed. Thanks for catching this!
:
OK
:Index: ieee80211_node.c
:===================================================================
:RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
:retrieving revision 1.129
:diff -u -p -r1.129 ieee80211_node.c
:--- ieee80211_node.c 28 Apr 2018 14:49:07 -0000 1.129
:+++ ieee80211_node.c 30 Apr 2018 07:30:43 -0000
:@@ -67,6 +67,8 @@ u_int8_t ieee80211_node_getrssi(struct i
: const struct ieee80211_node *);
: int ieee80211_node_checkrssi(struct ieee80211com *,
: const struct ieee80211_node *);
:+int ieee80211_node_cmprssi(struct ieee80211com *,
:+ const struct ieee80211_node *, const struct ieee80211_node *);
: void ieee80211_setup_node(struct ieee80211com *, struct ieee80211_node *,
: const u_int8_t *);
: void ieee80211_free_node(struct ieee80211com *, struct ieee80211_node *);
:@@ -133,6 +135,7 @@ ieee80211_node_attach(struct ifnet *ifp)
: ic->ic_node_copy = ieee80211_node_copy;
: ic->ic_node_getrssi = ieee80211_node_getrssi;
: ic->ic_node_checkrssi = ieee80211_node_checkrssi;
:+ ic->ic_node_cmprssi = ieee80211_node_cmprssi;
: ic->ic_scangen = 1;
: ic->ic_max_nnodes = ieee80211_cache_size;
:
:@@ -761,12 +764,15 @@ ieee80211_end_scan(struct ifnet *ifp)
:
: if (ic->ic_caps & IEEE80211_C_SCANALLBAND) {
: if (IEEE80211_IS_CHAN_2GHZ(ni->ni_chan) &&
:- (selbs2 == NULL || ni->ni_rssi > selbs2->ni_rssi))
:+ (selbs2 == NULL ||
:+ ic->ic_node_cmprssi(ic, ni, selbs2) > 0))
: selbs2 = ni;
: else if (IEEE80211_IS_CHAN_5GHZ(ni->ni_chan) &&
:- (selbs5 == NULL || ni->ni_rssi > selbs5->ni_rssi))
:+ (selbs5 == NULL ||
:+ ic->ic_node_cmprssi(ic, ni, selbs5) > 0))
: selbs5 = ni;
:- } else if (selbs == NULL || ni->ni_rssi > selbs->ni_rssi)
:+ } else if (selbs == NULL ||
:+ ic->ic_node_cmprssi(ic, ni, selbs) > 0)
: selbs = ni;
: }
:
:@@ -782,9 +788,12 @@ ieee80211_end_scan(struct ifnet *ifp)
: */
: if (selbs5 && selbs5->ni_rssi > min_5ghz_rssi)
: selbs = selbs5;
:- else if (selbs5 && selbs2)
:- selbs = (selbs5->ni_rssi >= selbs2->ni_rssi ? selbs5 : selbs2);
:- else if (selbs2)
:+ else if (selbs5 && selbs2) {
:+ if (ic->ic_node_cmprssi(ic, selbs5, selbs2) >= 0)
:+ selbs = selbs5;
:+ else
:+ selbs = selbs2;
:+ } else if (selbs2)
: selbs = selbs2;
: else if (selbs5)
: selbs = selbs5;
:@@ -989,6 +998,38 @@ ieee80211_node_checkrssi(struct ieee8021
: IEEE80211_RSSI_THRES_2GHZ :
: IEEE80211_RSSI_THRES_5GHZ;
: return (ni->ni_rssi >= (u_int8_t)thres);
:+}
:+
:+/*
:+ * Determine if RSSI values of two nodes are significantly different.
:+ * This function assumes RSSI values are represented either in dBm or
:+ * as a percentage of ic_max_rssi. Drivers should override this function
:+ * in case their RSSI values use a different representation.
:+ */
:+int
:+ieee80211_node_cmprssi(struct ieee80211com *ic,
:+ const struct ieee80211_node *ni1, const struct ieee80211_node *ni2)
:+{
:+ uint8_t thres;
:+
:+ if (ic->ic_max_rssi)
:+ thres = IEEE80211_RSSI_CMP_THRES_RATIO;
:+ else
:+ thres = IEEE80211_RSSI_CMP_THRES_DBM;
:+
:+ /* cap threshold to prevent overflow in calculations below */
:+ while (thres > 0) {
:+ if (ni1->ni_rssi + thres >= ni1->ni_rssi &&
:+ ni1->ni_rssi - thres <= ni1->ni_rssi)
:+ break;
:+ thres--;
:+ }
:+
:+ if (ni1->ni_rssi + thres < ni2->ni_rssi)
:+ return -1;
:+ if (ni1->ni_rssi - thres > ni2->ni_rssi)
:+ return 1;
:+ return 0;
: }
:
: void
:Index: ieee80211_var.h
:===================================================================
:RCS file: /cvs/src/sys/net80211/ieee80211_var.h,v
:retrieving revision 1.85
:diff -u -p -r1.85 ieee80211_var.h
:--- ieee80211_var.h 26 Apr 2018 12:50:07 -0000 1.85
:+++ ieee80211_var.h 30 Apr 2018 07:30:56 -0000
:@@ -62,6 +62,9 @@
: #define IEEE80211_RSSI_THRES_RATIO_2GHZ 60 /* in percent */
: #define IEEE80211_RSSI_THRES_RATIO_5GHZ 50 /* in percent */
:
:+#define IEEE80211_RSSI_CMP_THRES_DBM 8 /* in dBm */
:+#define IEEE80211_RSSI_CMP_THRES_RATIO 5 /* in percent */
:+
: #define IEEE80211_BGSCAN_FAIL_MAX 360 /* units of 500 msec */
:
: enum ieee80211_phytype {
:@@ -269,6 +272,9 @@ struct ieee80211com {
: u_int8_t (*ic_node_getrssi)(struct ieee80211com *,
: const struct ieee80211_node *);
: int (*ic_node_checkrssi)(struct ieee80211com *,
:+ const struct ieee80211_node *);
:+ int (*ic_node_cmprssi)(struct ieee80211com *,
:+ const struct ieee80211_node *,
: const struct ieee80211_node *);
: u_int8_t ic_max_rssi;
: struct ieee80211_tree ic_tree;
:
--
Never let your sense of morals prevent you from doing what is right.
-- Salvor Hardin, "Foundation"
