Hi,
> -----Original Message-----
> From: Hostap <hostap-bounces@xxxxxxxxxxxxxxxxxxx> On Behalf Of
> Veerendranath Jakkam
> Sent: Saturday, October 01, 2022 11:21
> To: hostap@xxxxxxxxxxxxxxxxxxx
> Cc: quic_vjakkam@xxxxxxxxxxx
> Subject: [PATCH v2 05/17] MLD STA: set MLO connection info to wpa_sm
>
> Update below MLO connection info to wpa_sm:
> - AP MLD address and link ID of the (re)association link.
> - For each requested link
> - own link address
> - AP's link bssid, RSNE, RSNXE
>
> Signed-off-by: Veerendranath Jakkam <quic_vjakkam@xxxxxxxxxxx>
> +int wpa_sm_set_mlo_params(struct wpa_sm *sm, const struct
> wpa_sm_mlo
> +*mlo) {
> + int i;
> + char title[50];
> + int ret;
> +
> + if (!sm)
> + return -1;
> +
> + os_memcpy(sm->mlo.ap_mld_addr, mlo->ap_mld_addr,
> ETH_ALEN);
> + sm->mlo.assoc_link_id = mlo->assoc_link_id;
> + sm->mlo.setup_links = mlo->setup_links;
> + sm->mlo.req_links = mlo->req_links;
Maybe verify that setup_links is a subset of requested links? And that assoc_link_id is part of the setup links
> + for (i = 0; i < MAX_NUM_MLD_LINKS; i++) {
> + const u8 *ie;
> + size_t len;
> +
> + os_memcpy(sm->mlo.links[i].addr, mlo->links[i].addr,
> ETH_ALEN);
> + os_memcpy(sm->mlo.links[i].bssid, mlo->links[i].bssid,
> + ETH_ALEN);
This should be done only for the requested links, as otherwise it would contain
garbage.
> +
> + ie = mlo->links[i].ap_rsne;
> + len = mlo->links[i].ap_rsne_len;
> + os_free(sm->mlo.links[i].ap_rsne);
> + if (ie == NULL || len == 0) {
I think I asked this before: I do not think that it is valid to have an MLD connection without RSN.
> + wpa_dbg(sm->ctx->msg_ctx, MSG_DEBUG,
> + "WPA: clearing MLO link[%u] AP RSNE", i);
> + sm->mlo.links[i].ap_rsne = NULL;
> + sm->mlo.links[i].ap_rsne_len = 0;
This can be done unconditionally (also for the rsnxe).
> + } else {
> + ret = os_snprintf(title, sizeof(title),
> + "RSN: set MLO link[%u] AP RSNE", i);
> + if (!os_snprintf_error(sizeof(title), ret))
> + wpa_hexdump(MSG_DEBUG, title, ie, len);
> + sm->mlo.links[i].ap_rsne = os_memdup(ie, len);
> + if (!sm->mlo.links[i].ap_rsne)
> + return -1;
> + sm->mlo.links[i].ap_rsne_len = len;
> + }
> +
> + ie = mlo->links[i].ap_rsnxe;
> + len = mlo->links[i].ap_rsnxe_len;
> + os_free(sm->mlo.links[i].ap_rsnxe);
> + if (ie == NULL || len == 0) {
> + wpa_dbg(sm->ctx->msg_ctx, MSG_DEBUG,
> + "WPA: clearing MLO link[%u] AP RSNXE", i);
> + sm->mlo.links[i].ap_rsnxe = NULL;
> + sm->mlo.links[i].ap_rsnxe_len = 0;
Same here.
> + } else {
> + ret = os_snprintf(title, sizeof(title),
> + "RSN: set MLO link[%u] AP RSNXE",
> i);
> + if (!os_snprintf_error(sizeof(title), ret))
> + wpa_hexdump(MSG_DEBUG, title, ie, len);
> + sm->mlo.links[i].ap_rsnxe = os_memdup(ie, len);
> + if (!sm->mlo.links[i].ap_rsnxe)
> + return -1;
> + sm->mlo.links[i].ap_rsnxe_len = len;
> + }
> + }
> +
> + return 0;
> +}
Snip ..
> +static void wpas_get_basic_mle_links_info(const u8 *mle, size_t mle_len,
> + struct links_info *info)
> +{
> + size_t rem_len;
> + const u8 *pos;
> +#define ML_CTRL_FIELD_LEN 2
Move the definition to ieee802_11_defs.h?
> + if (mle_len < (ML_CTRL_FIELD_LEN + 1) ||
> + (mle_len - ML_CTRL_FIELD_LEN < mle[ML_CTRL_FIELD_LEN]))
> + return;
> +
> + // Skip Common Info
> + pos = mle + ML_CTRL_FIELD_LEN + mle[ML_CTRL_FIELD_LEN];
> + rem_len = mle_len - ML_CTRL_FIELD_LEN -
> mle[ML_CTRL_FIELD_LEN];
> +
> + // Parse Subelements
> + while (rem_len > 2) {
> + int ie_len = 2 + pos[1];
> +
> + if (rem_len < ie_len)
> + return;
> +
> + if (pos[0] == MULTI_LINK_SUB_ELEM_ID_PER_STA_PROFILE)
> {
> + u8 link_id;
> + const u8 *sta_profile;
> +
> +#define BASIC_ML_STA_INFO_STA_MAC_IDX \
> + (2 + /* STA Control field */ \
> + 1) /* STA Info Length field (Basic) */
Also move to ieee802_11_defs.h
> + if (pos[1] < (BASIC_ML_STA_INFO_STA_MAC_IDX +
> ETH_ALEN))
> + goto next_subelem;
> +
> + sta_profile = &pos[2];
> + link_id = sta_profile[0] &
> + BASIC_ML_STA_CTRL0_LINK_ID_MASK;
> + if (link_id >= MAX_NUM_MLD_LINKS)
> + goto next_subelem;
> +
> + if (!(sta_profile[0] &
> BASIC_ML_STA_CTRL0_PRES_STA_MAC))
> + goto next_subelem;
> +
> + info->non_assoc_links |= BIT(link_id);
> + os_memcpy(info->addr[link_id],
> +
> &sta_profile[BASIC_ML_STA_INFO_STA_MAC_IDX],
> + ETH_ALEN);
> + }
> +next_subelem:
> + pos += ie_len;
> + rem_len -= ie_len;
> + }
> +}
> +
> +
> +static int wpa_sm_set_ml_info(struct wpa_supplicant *wpa_s,
> + union wpa_event_data *data)
> +{
> + int i;
> + struct wpabuf *mle;
> + struct ieee802_11_elems req_elems, resp_elems;
> + struct links_info req_links, resp_links;
> + struct wpa_sm_mlo mlo;
> + const u8 *bss_rsn = NULL, *bss_rsnx = NULL;
> +
> + os_memset(&mlo, 0, sizeof(mlo));
> + if (!wpa_s->valid_links)
> + return wpa_sm_set_mlo_params(wpa_s->wpa, &mlo);
> +
> + if (!data || !data->assoc_info.req_ies || !data->assoc_info.resp_ies)
> + return -1;
> +
> + if (ieee802_11_parse_elems(data->assoc_info.resp_ies,
> + data->assoc_info.resp_ies_len,
> &resp_elems,
> + 0) == ParseFailed ||
> + ieee802_11_parse_elems(data->assoc_info.req_ies,
> + data->assoc_info.req_ies_len, &req_elems,
> + 0) == ParseFailed) {
> + wpa_dbg(wpa_s, MSG_ERROR,
> + "MLO: Failed to parse Association request/response
> IEs");
> + return -1;
> + }
> +
> + mle = ieee802_11_defrag_mle(&req_elems,
> MULTI_LINK_CONTROL_TYPE_BASIC);
> + if (!mle) {
> + wpa_dbg(wpa_s, MSG_ERROR,
> + "MLO: Basic Multi-Link element not found in
> Association request");
> + return -1;
> + }
> + os_memset(&req_links, 0, sizeof(req_links));
> + wpas_get_basic_mle_links_info((const u8 *) wpabuf_head(mle),
> + wpabuf_len(mle), &req_links);
> + wpabuf_free(mle);
> +
> + mle = ieee802_11_defrag_mle(&resp_elems,
> MULTI_LINK_CONTROL_TYPE_BASIC);
> + if (!mle) {
> + wpa_dbg(wpa_s, MSG_ERROR,
> + "MLO: Basic Multi-Link element not found in
> Association response");
> + return -1;
> + }
> + os_memset(&resp_links, 0, sizeof(resp_links));
> + wpas_get_basic_mle_links_info((const u8 *) wpabuf_head(mle),
> + wpabuf_len(mle), &resp_links);
> + wpabuf_free(mle);
> +
> + if (req_links.non_assoc_links != resp_links.non_assoc_links) {
> + wpa_dbg(wpa_s, MSG_ERROR,
> + "MLO: Association request and response links bitmap
> not equal");
> + return -1;
> + }
> +
> + mlo.assoc_link_id = wpa_s->mlo_assoc_link_id;
> + mlo.setup_links = wpa_s->valid_links;
> + mlo.req_links = req_links.non_assoc_links | BIT(mlo.assoc_link_id);
> + os_memcpy(mlo.ap_mld_addr, wpa_s->ap_mld_addr, ETH_ALEN);
> + for (i = 0; i < MAX_NUM_MLD_LINKS; i++) {
> + struct wpa_bss *bss;
> +
> + if (!(mlo.req_links & BIT(i)))
> + continue;
> +
> + if (mlo.setup_links & BIT(i)) {
> + bss = wpa_s->links[i].bss;
> + } else {
> + bss = wpa_supplicant_get_new_bss(wpa_s,
> + resp_links.addr[i]);
> + if (!bss) {
> +
Since setup links should be subset of requested links, why not use 'bss = wpa_s->links[i].bss' as well?
Regards,
Ilan.
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
