| Message ID | 20240409065023.30664-1-michael-cy.lee@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | wpa_supplicant: MLD STA: find partner links by BSSID and SSID | expand |
Hi, On Tue, 2024-04-09 at 14:50 +0800, Michael-CY Lee wrote: > MLD STA finds MLD AP's partner links by BSSID from the scan results. > However, if the scan results contain BSSs with same BSSID but different > BSS information, the MLD STA might assign a wrong BSS to one of the MLD > AP's partner links. As a clarification, why/how do we get into the situation that the same BSSIDs exists with the wrong SSID in the supplicant? Did the AP change its SSID and we still have the old one in the cache? If that is the case, then maybe what we should be doing is to verify that the BSS parameter change count matches. If not, it makes sense to reject the BSS for now, which should trigger an ML probe request to update the information and the next attempt will work fine. Benjamin > This patch avoids the problem by using both BSSID and SSID to find the > MLD AP's partner links. > > Signed-off-by: Michael-CY Lee <michael-cy.lee@mediatek.com> > --- > wpa_supplicant/bss.c | 9 +++++++-- > wpa_supplicant/sme.c | 14 ++++++++------ > 2 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/wpa_supplicant/bss.c b/wpa_supplicant/bss.c > index 289035310..4b8a98adf 100644 > --- a/wpa_supplicant/bss.c > +++ b/wpa_supplicant/bss.c > @@ -1529,8 +1529,13 @@ wpa_bss_parse_ml_rnr_ap_info(struct > wpa_supplicant *wpa_s, > wpa_printf(MSG_DEBUG, > "MLD: Reported link not part of > MLD"); > } else if (!(BIT(link_id) & *seen)) { > - struct wpa_bss *neigh_bss = > - wpa_bss_get_bssid(wpa_s, pos + 1); > + struct wpa_bss *neigh_bss; > + > + if (ssid) > + neigh_bss = wpa_bss_get(wpa_s, pos + > 1, ssid->ssid, > + ssid- > >ssid_len); > + else > + neigh_bss = wpa_bss_get_bssid(wpa_s, > pos + 1); > > *seen |= BIT(link_id); > wpa_printf(MSG_DEBUG, "MLD: mld ID=%u, link > ID=%u", > diff --git a/wpa_supplicant/sme.c b/wpa_supplicant/sme.c > index f08184f98..0643774a5 100644 > --- a/wpa_supplicant/sme.c > +++ b/wpa_supplicant/sme.c > @@ -390,7 +390,8 @@ static void wpas_ml_handle_removed_links(struct > wpa_supplicant *wpa_s, > > #ifdef CONFIG_TESTING_OPTIONS > static struct wpa_bss * wpas_ml_connect_pref(struct wpa_supplicant > *wpa_s, > - struct wpa_bss *bss) > + struct wpa_bss *bss, > + struct wpa_ssid *ssid) > { > unsigned int low, high, i; > > @@ -456,7 +457,7 @@ found: > MAC2STR(wpa_s->links[i].bssid)); > > /* Get the BSS entry and do the switch */ > - bss = wpa_bss_get_bssid(wpa_s, wpa_s->links[i].bssid); > + bss = wpa_bss_get(wpa_s, wpa_s->links[i].bssid, ssid->ssid, > ssid->ssid_len); > wpa_s->mlo_assoc_link_id = i; > > return bss; > @@ -515,7 +516,7 @@ out: > > > static void wpas_sme_set_mlo_links(struct wpa_supplicant *wpa_s, > - struct wpa_bss *bss) > + struct wpa_bss *bss, struct > wpa_ssid *ssid) > { > u8 i; > > @@ -533,7 +534,8 @@ static void wpas_sme_set_mlo_links(struct > wpa_supplicant *wpa_s, > if (bss->mld_link_id == i) > wpa_s->links[i].bss = bss; > else > - wpa_s->links[i].bss = > wpa_bss_get_bssid(wpa_s, bssid); > + wpa_s->links[i].bss = wpa_bss_get(wpa_s, > bssid, ssid->ssid, > + ssid- > >ssid_len); > } > } > > @@ -576,10 +578,10 @@ static void sme_send_authentication(struct > wpa_supplicant *wpa_s, > NULL, ssid, NULL) && > bss->valid_links) { > wpa_printf(MSG_DEBUG, "MLD: In authentication"); > - wpas_sme_set_mlo_links(wpa_s, bss); > + wpas_sme_set_mlo_links(wpa_s, bss, ssid); > > #ifdef CONFIG_TESTING_OPTIONS > - bss = wpas_ml_connect_pref(wpa_s, bss); > + bss = wpas_ml_connect_pref(wpa_s, bss, ssid); > > if (wpa_s->conf->mld_force_single_link) { > wpa_printf(MSG_DEBUG, "MLD: Force single > link");
Hi Michael, On Mon, 2024-04-22 at 02:35 +0000, Michael-cy Lee (李峻宇) wrote: > On Wed, 2024-04-17 at 09:06 +0200, Benjamin Berg wrote: > > On Tue, 2024-04-09 at 14:50 +0800, Michael-CY Lee wrote: > > > MLD STA finds MLD AP's partner links by BSSID from the scan results. > > > However, if the scan results contain BSSs with same BSSID but different > > > BSS information, the MLD STA might assign a wrong BSS to one of the MLD > > > AP's partner links. > > > > As a clarification, why/how do we get into the situation that the > > same BSSIDs exists with the wrong SSID in the supplicant? > > > > Did the AP change its SSID and we still have the old one in the > > cache? > > Yes, that's exactly the situation we've faced. > To be more precise, after we changed the SSID and restarted the AP MLD, > we triggered the STA scan and saw the STA's scan results containing two > BSSs with the same BSSID but different SSIDs, then the STA incorrectly > matched one of the AP MLD's links to the wrong BSS (old SSID). > > > > > If that is the case, then maybe what we should be doing is to verify > > that the BSS parameter change count matches. If not, it makes sense to > > reject the BSS for now, which should trigger an ML probe request to > > update the information and the next attempt will work fine. > > We think the BSS parameter change count (BPCC) might not be helpful > here since a change in SSID will not trigger a critical update. > Additionally, An SSID change is usually followed by an AP restart, > which will reset the BPCC value to 0. Right, I guess that is fair then. I guess it feels a bit weird to me overall, because I would expect that we only ever have one BSS entry for a BSSID (except as a workaround for some broken legacy APs). And it seems to me like one could increase the BPCC across an AP restart. So, I don't have a good alternative to your solution, I think. That said, maybe we should still add an additional check for the BPCC? Benjamin
diff --git a/wpa_supplicant/bss.c b/wpa_supplicant/bss.c index 289035310..4b8a98adf 100644 --- a/wpa_supplicant/bss.c +++ b/wpa_supplicant/bss.c @@ -1529,8 +1529,13 @@ wpa_bss_parse_ml_rnr_ap_info(struct wpa_supplicant *wpa_s, wpa_printf(MSG_DEBUG, "MLD: Reported link not part of MLD"); } else if (!(BIT(link_id) & *seen)) { - struct wpa_bss *neigh_bss = - wpa_bss_get_bssid(wpa_s, pos + 1); + struct wpa_bss *neigh_bss; + + if (ssid) + neigh_bss = wpa_bss_get(wpa_s, pos + 1, ssid->ssid, + ssid->ssid_len); + else + neigh_bss = wpa_bss_get_bssid(wpa_s, pos + 1); *seen |= BIT(link_id); wpa_printf(MSG_DEBUG, "MLD: mld ID=%u, link ID=%u", diff --git a/wpa_supplicant/sme.c b/wpa_supplicant/sme.c index f08184f98..0643774a5 100644 --- a/wpa_supplicant/sme.c +++ b/wpa_supplicant/sme.c @@ -390,7 +390,8 @@ static void wpas_ml_handle_removed_links(struct wpa_supplicant *wpa_s, #ifdef CONFIG_TESTING_OPTIONS static struct wpa_bss * wpas_ml_connect_pref(struct wpa_supplicant *wpa_s, - struct wpa_bss *bss) + struct wpa_bss *bss, + struct wpa_ssid *ssid) { unsigned int low, high, i; @@ -456,7 +457,7 @@ found: MAC2STR(wpa_s->links[i].bssid)); /* Get the BSS entry and do the switch */ - bss = wpa_bss_get_bssid(wpa_s, wpa_s->links[i].bssid); + bss = wpa_bss_get(wpa_s, wpa_s->links[i].bssid, ssid->ssid, ssid->ssid_len); wpa_s->mlo_assoc_link_id = i; return bss; @@ -515,7 +516,7 @@ out: static void wpas_sme_set_mlo_links(struct wpa_supplicant *wpa_s, - struct wpa_bss *bss) + struct wpa_bss *bss, struct wpa_ssid *ssid) { u8 i; @@ -533,7 +534,8 @@ static void wpas_sme_set_mlo_links(struct wpa_supplicant *wpa_s, if (bss->mld_link_id == i) wpa_s->links[i].bss = bss; else - wpa_s->links[i].bss = wpa_bss_get_bssid(wpa_s, bssid); + wpa_s->links[i].bss = wpa_bss_get(wpa_s, bssid, ssid->ssid, + ssid->ssid_len); } } @@ -576,10 +578,10 @@ static void sme_send_authentication(struct wpa_supplicant *wpa_s, NULL, ssid, NULL) && bss->valid_links) { wpa_printf(MSG_DEBUG, "MLD: In authentication"); - wpas_sme_set_mlo_links(wpa_s, bss); + wpas_sme_set_mlo_links(wpa_s, bss, ssid); #ifdef CONFIG_TESTING_OPTIONS - bss = wpas_ml_connect_pref(wpa_s, bss); + bss = wpas_ml_connect_pref(wpa_s, bss, ssid); if (wpa_s->conf->mld_force_single_link) { wpa_printf(MSG_DEBUG, "MLD: Force single link");
MLD STA finds MLD AP's partner links by BSSID from the scan results. However, if the scan results contain BSSs with same BSSID but different BSS information, the MLD STA might assign a wrong BSS to one of the MLD AP's partner links. This patch avoids the problem by using both BSSID and SSID to find the MLD AP's partner links. Signed-off-by: Michael-CY Lee <michael-cy.lee@mediatek.com> --- wpa_supplicant/bss.c | 9 +++++++-- wpa_supplicant/sme.c | 14 ++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-)