On 28/08/19(Wed) 12:54, Stefan Sperling wrote:
> I find myself regularly asking people to run 'ifconfig iwm0 debug' to
> figure out reasons behind association failures from dmesg output.
>
> I would like to make this information more accessible. Making it part
> of regular ifconfig output seems like a reasonable thing to do. This
> becomes possible now that node information is cached across scans.
>
> The current net80211 code is already computing failure reason codes which
> are visible in 'ifconfig debug' output. So far such codes were magic numbers.
> We can simply create macros for them and expose them in ieee80211_ioctl.h,
> and add another code to represent the "wrong-WPA-key" situation.
>
> To avoid changing the existing ifconfig output too much, the following
> association failure reasons are not displayed:
> [...]
> Is this going in the right direction?
I love it. Comments below
> diff refs/heads/master refs/heads/assocfail
> blob - 543e09f0d6e6b4e75456a21d4218468729b61a5e
> blob + 03e4818ef5fa11d7e5dbf8307affb9f0ea5522a0
> --- sbin/ifconfig/ifconfig.c
> +++ sbin/ifconfig/ifconfig.c
> @@ -2348,10 +2348,27 @@ print_cipherset(u_int32_t cipherset)
> }
>
> void
> +print_assoc_failures(uint32_t assoc_fail, const char *intro, const char
> *outro)
> +{
> + /* Filter out the most obvious failure cases. */
> + assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_ESSID;
> + if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY)
> + assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_WPA_PROTO;
> + assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY;
It's not clear to me why you suggest we should filter out these failure
cases.
> +
> + if (assoc_fail == 0)
> + return;
> +
> + printf("%s", intro);
> + printb_status(assoc_fail, IEEE80211_NODEREQ_ASSOCFAIL_BITS);
> + printf("%s", outro);
> +}
> +
> +void
> ieee80211_status(void)
> {
> int len, inwid, ijoin, inwkey, ipsk, ichan, ipwr;
> - int ibssid, iwpa;
> + int ibssid, iwpa, assocfail = 0;
> struct ieee80211_nwid nwid;
> struct ieee80211_join join;
> struct ieee80211_nwkey nwkey;
> @@ -2431,11 +2448,15 @@ ieee80211_status(void)
> bzero(&nr, sizeof(nr));
> bcopy(bssid.i_bssid, &nr.nr_macaddr, sizeof(nr.nr_macaddr));
> strlcpy(nr.nr_ifname, name, sizeof(nr.nr_ifname));
> - if (ioctl(s, SIOCG80211NODE, &nr) == 0 && nr.nr_rssi) {
> - if (nr.nr_max_rssi)
> - printf(" %u%%", IEEE80211_NODEREQ_RSSI(&nr));
> - else
> - printf(" %ddBm", nr.nr_rssi);
> + if (ioctl(s, SIOCG80211NODE, &nr) == 0) {
> + if (nr.nr_rssi) {
> + if (nr.nr_max_rssi)
> + printf(" %u%%",
> + IEEE80211_NODEREQ_RSSI(&nr));
> + else
> + printf(" %ddBm", nr.nr_rssi);
> + }
> + assocfail = nr.nr_assoc_fail;
> }
> }
>
> @@ -2478,6 +2499,10 @@ ieee80211_status(void)
> putchar(' ');
> printb_status(ifr.ifr_flags, IEEE80211_F_USERBITS);
> }
> +
> + if (assocfail)
> + print_assoc_failures(assocfail, " !(", ")");
> +
> putchar('\n');
> if (show_join)
> join_status();
> @@ -2751,6 +2776,8 @@ ieee80211_printnode(struct ieee80211_nodereq *nr)
> if ((nr->nr_flags & IEEE80211_NODEREQ_AP) == 0)
> printb_status(IEEE80211_NODEREQ_STATE(nr->nr_state),
> IEEE80211_NODEREQ_STATE_BITS);
> + else if (nr->nr_assoc_fail)
> + print_assoc_failures(nr->nr_assoc_fail, "!(", ") ");
> }
>
> void
> blob - 425ee5f5a726e3fe0445c82debb0469d4878407f
> blob + 6b3b8a5834db39d5b2f2dc7693f42d0b5adbcfbb
> --- sys/net80211/ieee80211_ioctl.c
> +++ sys/net80211/ieee80211_ioctl.c
> @@ -104,6 +104,7 @@ ieee80211_node2req(struct ieee80211com *ic, const stru
> nr->nr_txseq = ni->ni_txseq;
> nr->nr_rxseq = ni->ni_rxseq;
> nr->nr_fails = ni->ni_fails;
> + nr->nr_assoc_fail = ni->ni_assoc_fail; /* flag values are the same */
> nr->nr_inact = ni->ni_inact;
> nr->nr_txrate = ni->ni_txrate;
> nr->nr_state = ni->ni_state;
> @@ -821,7 +822,11 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t
> break;
> case SIOCG80211NODE:
> nr = (struct ieee80211_nodereq *)data;
> - ni = ieee80211_find_node(ic, nr->nr_macaddr);
> + if (ic->ic_bss &&
> + IEEE80211_ADDR_EQ(nr->nr_macaddr, ic->ic_bss->ni_macaddr))
> + ni = ic->ic_bss;
> + else
> + ni = ieee80211_find_node(ic, nr->nr_macaddr);
> if (ni == NULL) {
> error = ENOENT;
> break;
> blob - 99b17a709fec1326d108be8e39ae8596f5b7f0f6
> blob + ac031edda4662154bf384a3bd6d722c72547f136
> --- sys/net80211/ieee80211_node.c
> +++ sys/net80211/ieee80211_node.c
> @@ -780,7 +792,7 @@ ieee80211_reset_scan(struct ifnet *ifp)
> }
>
> /*
> - * Increase a node's inactitivy counter.
> + * Increase a node's inactivity counter.
> * This counter get reset to zero if a frame is received.
> * This function is intended for station mode only.
> * See ieee80211_node_cache_timeout() for hostap mode.
> @@ -1043,66 +1055,71 @@ ieee80211_match_bss(struct ieee80211com *ic, struct ie
> * decline to associate with that AP.
> */
> if ((ni->ni_rsnprotos & ic->ic_rsnprotos) == 0)
> - fail |= 0x40;
> + fail |= IEEE80211_NODE_ASSOCFAIL_WPA_PROTO;
> if ((ni->ni_rsnakms & ic->ic_rsnakms) == 0)
> - fail |= 0x40;
> + fail |= IEEE80211_NODE_ASSOCFAIL_WPA_PROTO;
> if ((ni->ni_rsnakms & ic->ic_rsnakms &
> ~(IEEE80211_AKM_PSK | IEEE80211_AKM_SHA256_PSK)) == 0) {
> /* AP only supports PSK AKMPs */
> if (!(ic->ic_flags & IEEE80211_F_PSK))
> - fail |= 0x40;
> + fail |= IEEE80211_NODE_ASSOCFAIL_WPA_PROTO;
> }
> if (ni->ni_rsngroupcipher != IEEE80211_CIPHER_WEP40 &&
> ni->ni_rsngroupcipher != IEEE80211_CIPHER_TKIP &&
> ni->ni_rsngroupcipher != IEEE80211_CIPHER_CCMP &&
> ni->ni_rsngroupcipher != IEEE80211_CIPHER_WEP104)
> - fail |= 0x40;
> + fail |= IEEE80211_NODE_ASSOCFAIL_WPA_PROTO;
> if ((ni->ni_rsnciphers & ic->ic_rsnciphers) == 0)
> - fail |= 0x40;
> + fail |= IEEE80211_NODE_ASSOCFAIL_WPA_PROTO;
>
> /* we only support BIP as the IGTK cipher */
> if ((ni->ni_rsncaps & IEEE80211_RSNCAP_MFPC) &&
> ni->ni_rsngroupmgmtcipher != IEEE80211_CIPHER_BIP)
> - fail |= 0x40;
> + fail |= IEEE80211_NODE_ASSOCFAIL_WPA_PROTO;
>
> /* we do not support MFP but AP requires it */
> if (!(ic->ic_caps & IEEE80211_C_MFP) &&
> (ni->ni_rsncaps & IEEE80211_RSNCAP_MFPR))
> - fail |= 0x40;
> + fail |= IEEE80211_NODE_ASSOCFAIL_WPA_PROTO;
>
> /* we require MFP but AP does not support it */
> if ((ic->ic_caps & IEEE80211_C_MFP) &&
> (ic->ic_flags & IEEE80211_F_MFPR) &&
> !(ni->ni_rsncaps & IEEE80211_RSNCAP_MFPC))
> - fail |= 0x40;
> + fail |= IEEE80211_NODE_ASSOCFAIL_WPA_PROTO;
> }
>
> if (ic->ic_if.if_flags & IFF_DEBUG) {
> printf("%s: %c %s%c", ic->ic_if.if_xname, fail ? '-' : '+',
> ether_sprintf(ni->ni_bssid),
> - fail & 0x20 ? '!' : ' ');
> + fail & IEEE80211_NODE_ASSOCFAIL_BSSID ? '!' : ' ');
> printf(" %3d%c", ieee80211_chan2ieee(ic, ni->ni_chan),
> - fail & 0x01 ? '!' : ' ');
> + fail & IEEE80211_NODE_ASSOCFAIL_CHAN ? '!' : ' ');
> printf(" %+4d", ni->ni_rssi);
> printf(" %2dM%c", (rate & IEEE80211_RATE_VAL) / 2,
> - fail & 0x08 ? '!' : ' ');
> + fail & IEEE80211_NODE_ASSOCFAIL_BASIC_RATE ? '!' : ' ');
> printf(" %4s%c",
> (ni->ni_capinfo & IEEE80211_CAPINFO_ESS) ? "ess" :
> (ni->ni_capinfo & IEEE80211_CAPINFO_IBSS) ? "ibss" :
> "????",
> - fail & 0x02 ? '!' : ' ');
> + fail & IEEE80211_NODE_ASSOCFAIL_IBSS ? '!' : ' ');
> printf(" %7s%c ",
> (ni->ni_capinfo & IEEE80211_CAPINFO_PRIVACY) ?
> "privacy" : "no",
> - fail & 0x04 ? '!' : ' ');
> + fail & IEEE80211_NODE_ASSOCFAIL_PRIVACY ? '!' : ' ');
> printf(" %3s%c ",
> (ic->ic_flags & IEEE80211_F_RSNON) ?
> "rsn" : "no",
> - fail & 0x40 ? '!' : ' ');
> + fail & IEEE80211_NODE_ASSOCFAIL_WPA_PROTO ? '!' : ' ');
> ieee80211_print_essid(ni->ni_essid, ni->ni_esslen);
> - printf("%s\n", fail & 0x10 ? "!" : "");
> + printf("%s\n",
> + fail & IEEE80211_NODE_ASSOCFAIL_ESSID ? "!" : "");
> }
Is there any value to keep the IFF_DEBUG output if ifconfig(8) already
report the failures?
> + ni->ni_assoc_fail |= fail;
> + if ((fail & IEEE80211_NODE_ASSOCFAIL_ESSID) == 0)
> + ic->ic_bss->ni_assoc_fail |= ni->ni_assoc_fail;
> +
> return fail;
> }
>
> @@ -1160,14 +1177,24 @@ ieee80211_node_join_bss(struct ieee80211com *ic, struc
> {
> enum ieee80211_phymode mode;
> struct ieee80211_node *ni;
> + uint32_t assoc_fail = 0;
>
> /* Reinitialize media mode and channels if needed. */
> mode = ieee80211_chan2mode(ic, selbs->ni_chan);
> if (mode != ic->ic_curmode)
> ieee80211_setmode(ic, mode);
>
> + /* Keep recorded association failures for this BSS/ESS intact. */
> + if (IEEE80211_ADDR_EQ(ic->ic_bss->ni_macaddr, selbs->ni_macaddr) ||
> + (ic->ic_des_esslen > 0 && ic->ic_des_esslen == selbs->ni_esslen &&
> + memcmp(ic->ic_des_essid, selbs->ni_essid, selbs->ni_esslen) == 0))
> + assoc_fail = ic->ic_bss->ni_assoc_fail;
> +
> (*ic->ic_node_copy)(ic, ic->ic_bss, selbs);
> ni = ic->ic_bss;
> + ni->ni_assoc_fail |= assoc_fail;
I couldn't convince myself why two divers are overwriting `ic_node_copy',
but shouldn't this logic be part of ieee80211_node_copy() instead? It is
just to work around the copy, right?
> +
> + ic->ic_curmode = ieee80211_chan2mode(ic, ni->ni_chan);
>
> /* Make sure we send valid rates in an association request. */
> if (ic->ic_opmode == IEEE80211_M_STA)
> blob - b2b80656a4b4aa907b4d729ecaecb81827739c52
> blob + ddd746729a48b15fa3c21d85390682263d2934d9
> --- sys/net80211/ieee80211_node.h
> +++ sys/net80211/ieee80211_node.h
> @@ -353,6 +353,16 @@ struct ieee80211_node {
> u_int16_t ni_qos_txseqs[IEEE80211_NUM_TID];
> u_int16_t ni_qos_rxseqs[IEEE80211_NUM_TID];
> int ni_fails; /* failure count to associate */
> + uint32_t ni_assoc_fail; /* assoc failure reasons */
> +#define IEEE80211_NODE_ASSOCFAIL_CHAN 0x01
> +#define IEEE80211_NODE_ASSOCFAIL_IBSS 0x02
> +#define IEEE80211_NODE_ASSOCFAIL_PRIVACY 0x04
> +#define IEEE80211_NODE_ASSOCFAIL_BASIC_RATE 0x08
> +#define IEEE80211_NODE_ASSOCFAIL_ESSID 0x10
> +#define IEEE80211_NODE_ASSOCFAIL_BSSID 0x20
> +#define IEEE80211_NODE_ASSOCFAIL_WPA_PROTO 0x40
> +#define IEEE80211_NODE_ASSOCFAIL_WPA_KEY 0x80
> +
> int ni_inact; /* inactivity mark count */
> int ni_txrate; /* index to ni_rates[] */
> int ni_state;
> blob - ba8c1872a2df596a0667ecb7c306f7e7f9dd0ffc
> blob + daf43cd6457134728ce5dd485229036ea4822445
> --- sys/net80211/ieee80211_pae_input.c
> +++ sys/net80211/ieee80211_pae_input.c
> @@ -650,6 +650,7 @@ ieee80211_recv_4way_msg3(struct ieee80211com *ic,
> ether_sprintf(ni->ni_macaddr)));
> ni->ni_port_valid = 1;
> ieee80211_set_link_state(ic, LINK_STATE_UP);
> + ni->ni_assoc_fail = 0;
There are other places where `ni_port_valid' is set to 1 and another
one where the link is set to UP where the flags aren't cleared.
Another question, could these flags be used in AP mode? To report
client errors?
