Re: bwfm: fix ifconfig media display on rpi2 usb wifi dongle
On Sun, Jun 26, 2022 at 12:23:14PM +0200, Stefan Sperling wrote: > On Sun, Jun 26, 2022 at 07:48:46PM +1000, Jonathan Gray wrote: > > sta.rssi is later used which is 'Fields valid for ver >= 4' > > but it seems with the earlier zeroing the use here should be fine? > > Thanks! I missed that. > > Testing suggests it makes more sense to keep the RSSI value which > was discovered during the scan phase. > > With the new patch below ifconfig shows -51dBm. > With the previous patch ifconfig showed -70dBm for the same AP (with > client/AP location unchanged). ok jsg@ > > diff /usr/src > commit - 9badb9ad8932c12f4ece484255eb2703a2518c17 > path + /usr/src > blob - 0c9c948115a0c115a43bd365ad4e389ba694c4a8 > file + sys/dev/ic/bwfm.c > --- sys/dev/ic/bwfm.c > +++ sys/dev/ic/bwfm.c > @@ -703,22 +703,24 @@ bwfm_update_node(void *arg, struct ieee80211_node *ni) > if (!IEEE80211_ADDR_EQ(ni->ni_macaddr, sta.ea)) > return; > > - if (le16toh(sta.ver) < 4) > + if (le16toh(sta.ver) < 3) > return; > > flags = le32toh(sta.flags); > if ((flags & BWFM_STA_SCBSTATS) == 0) > return; > > - rssi = 0; > - for (i = 0; i < BWFM_ANT_MAX; i++) { > - if (sta.rssi[i] >= 0) > - continue; > - if (rssi == 0 || sta.rssi[i] > rssi) > - rssi = sta.rssi[i]; > + if (le16toh(sta.ver) >= 4) { > + rssi = 0; > + for (i = 0; i < BWFM_ANT_MAX; i++) { > + if (sta.rssi[i] >= 0) > + continue; > + if (rssi == 0 || sta.rssi[i] > rssi) > + rssi = sta.rssi[i]; > + } > + if (rssi) > + ni->ni_rssi = rssi; > } > - if (rssi) > - ni->ni_rssi = rssi; > > txrate = le32toh(sta.tx_rate); /* in kbit/s */ > if (txrate == 0x) /* Seen this happening during association. */ > >
Re: bwfm: fix ifconfig media display on rpi2 usb wifi dongle
On Sun, Jun 26, 2022 at 07:48:46PM +1000, Jonathan Gray wrote: > sta.rssi is later used which is 'Fields valid for ver >= 4' > but it seems with the earlier zeroing the use here should be fine? Thanks! I missed that. Testing suggests it makes more sense to keep the RSSI value which was discovered during the scan phase. With the new patch below ifconfig shows -51dBm. With the previous patch ifconfig showed -70dBm for the same AP (with client/AP location unchanged). diff /usr/src commit - 9badb9ad8932c12f4ece484255eb2703a2518c17 path + /usr/src blob - 0c9c948115a0c115a43bd365ad4e389ba694c4a8 file + sys/dev/ic/bwfm.c --- sys/dev/ic/bwfm.c +++ sys/dev/ic/bwfm.c @@ -703,22 +703,24 @@ bwfm_update_node(void *arg, struct ieee80211_node *ni) if (!IEEE80211_ADDR_EQ(ni->ni_macaddr, sta.ea)) return; - if (le16toh(sta.ver) < 4) + if (le16toh(sta.ver) < 3) return; flags = le32toh(sta.flags); if ((flags & BWFM_STA_SCBSTATS) == 0) return; - rssi = 0; - for (i = 0; i < BWFM_ANT_MAX; i++) { - if (sta.rssi[i] >= 0) - continue; - if (rssi == 0 || sta.rssi[i] > rssi) - rssi = sta.rssi[i]; + if (le16toh(sta.ver) >= 4) { + rssi = 0; + for (i = 0; i < BWFM_ANT_MAX; i++) { + if (sta.rssi[i] >= 0) + continue; + if (rssi == 0 || sta.rssi[i] > rssi) + rssi = sta.rssi[i]; + } + if (rssi) + ni->ni_rssi = rssi; } - if (rssi) - ni->ni_rssi = rssi; txrate = le32toh(sta.tx_rate); /* in kbit/s */ if (txrate == 0x) /* Seen this happening during association. */
Re: bwfm: fix ifconfig media display on rpi2 usb wifi dongle
On Sat, Jun 25, 2022 at 10:07:21PM +0200, Stefan Sperling wrote: > There is an off-by-one in bwfm_update_node(). This function reads > the tx_rate field from station information returned by firmware. > > According to a comment in the Linux driver, this field is valid > for sta_info command versions >= 3. > > linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h > struct brcmf_sta_info_le { > __le16 ver; /* version of this struct */ > [...] > /* Fields valid for ver >= 3 */ > [...] > __le32 tx_rate; /* Rate of last successful tx frame */ > > However, our driver only reads Tx rate info if the version is >= 4. > > On the rpi2 usb wifi adapter (WLU6331) this breaks media mode display. > Firmware on this device uses command version 3, and the media mode > displayed is always "DS1 11g". Diff below fixes this. ifconfig will > now display 11n mode while associated to an 11n AP. > > ok? sta.rssi is later used which is 'Fields valid for ver >= 4' but it seems with the earlier zeroing the use here should be fine? ok jsg@ > > diff /usr/src > commit - 9badb9ad8932c12f4ece484255eb2703a2518c17 > path + /usr/src > blob - 0c9c948115a0c115a43bd365ad4e389ba694c4a8 > file + sys/dev/ic/bwfm.c > --- sys/dev/ic/bwfm.c > +++ sys/dev/ic/bwfm.c > @@ -703,7 +703,7 @@ bwfm_update_node(void *arg, struct ieee80211_node *ni) > if (!IEEE80211_ADDR_EQ(ni->ni_macaddr, sta.ea)) > return; > > - if (le16toh(sta.ver) < 4) > + if (le16toh(sta.ver) < 3) > return; > > flags = le32toh(sta.flags); > >
bwfm: fix ifconfig media display on rpi2 usb wifi dongle
There is an off-by-one in bwfm_update_node(). This function reads the tx_rate field from station information returned by firmware. According to a comment in the Linux driver, this field is valid for sta_info command versions >= 3. linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h struct brcmf_sta_info_le { __le16 ver; /* version of this struct */ [...] /* Fields valid for ver >= 3 */ [...] __le32 tx_rate; /* Rate of last successful tx frame */ However, our driver only reads Tx rate info if the version is >= 4. On the rpi2 usb wifi adapter (WLU6331) this breaks media mode display. Firmware on this device uses command version 3, and the media mode displayed is always "DS1 11g". Diff below fixes this. ifconfig will now display 11n mode while associated to an 11n AP. ok? diff /usr/src commit - 9badb9ad8932c12f4ece484255eb2703a2518c17 path + /usr/src blob - 0c9c948115a0c115a43bd365ad4e389ba694c4a8 file + sys/dev/ic/bwfm.c --- sys/dev/ic/bwfm.c +++ sys/dev/ic/bwfm.c @@ -703,7 +703,7 @@ bwfm_update_node(void *arg, struct ieee80211_node *ni) if (!IEEE80211_ADDR_EQ(ni->ni_macaddr, sta.ea)) return; - if (le16toh(sta.ver) < 4) + if (le16toh(sta.ver) < 3) return; flags = le32toh(sta.flags);