On 18/11/2018 23:18, Jouni Malinen wrote:
> From: Venkateswara Naralasetty <vnaralas@xxxxxxxxxxxxxx>
>
> Advertise vendor specific Multi-AP IE in (Re)Association request
> and process Multi-AP IE from (Re)Association response frames if
> user enable Multi-AP fuctionality.
>
> This patch introducing new configuration parameter 'multi_ap_enabled'
> to enable/disable Multi-AP funtionality from user.
>
> Signed-off-by: Venkateswara Naralasetty <vnaralas@xxxxxxxxxxxxxx>
> Signed-off-by: Jouni Malinen <jouni@xxxxxxxxxxxxxx>
[snip]
> +#ifdef CONFIG_MULTI_AP
> + /**
> + * set_multi_ap - Enable/disable Multi-AP functionality
> + * @priv: Private driver interface data
> + * @bridge_ifname: Name of the bridge interface
> + * @val: 0 for Multi-AP disable, 1 for Multi-AP enable
> + */
> + int (*set_multi_ap)(void *priv, const char *bridge_ifname, int val);
Wouldn't it be more logical to have a driver callback for setting 4addr rather
than setting Multi-AP?
Also, it's not really setting the BSS to multi-AP support, it's setting it to
backhaul BSS.
By the way, as far as I understand, the multi-AP specification allows a BSS to
be fronthaul and backhaul BSS at the same time - which means it should use 3addr
for the fronthaul STAs and 4addr for the backhaul STAs. I don't know if it's
even possible to program a radio like that...
> +#endif /* CONFIG_MULTI_AP */
> };
>
> /**
> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> index 771e766..6ec0d0a 100644
> --- a/src/drivers/driver_nl80211.c
> +++ b/src/drivers/driver_nl80211.c
> @@ -10647,6 +10647,47 @@ fail:
> return ret;
> }
>
> +#ifdef CONFIG_MULTI_AP
> +static int nl80211_set_multi_ap(void *priv, const char *bridge_ifname, int val)
> +{
> + struct i802_bss *bss = priv;
> + struct wpa_driver_nl80211_data *drv = bss->drv;
> + struct nl_msg *msg;
> + int ret = -ENOBUFS;
> +
> + msg = nl80211_cmd_msg(drv->first_bss, 0, NL80211_CMD_SET_INTERFACE);
> + if (!msg || nla_put_u8(msg, NL80211_ATTR_4ADDR, val))
> + goto fail;
> +
> + if (bridge_ifname[0] && bss->added_if_into_bridge && !val) {
> + if (linux_br_del_if(drv->global->ioctl_sock,
> + bridge_ifname, bss->ifname)) {
> + wpa_printf(MSG_ERROR, "nl80211: Failed to remove the "
> + "interface %s to a bridge %s",
> + bss->ifname, bridge_ifname);
I don't understand why this is needed. Actually, I don't see why you'd want to
support switching the backhaul option on or off while you're associated. In my
opinion, this option is as much part of the BSS configuration as the SSID. So
actually, it would make more sense to me to just set the wds argument to
nl80211_create_iface() as appropriate. Or is that too early because we haven't
checked the Multi-AP IE yet?
> + return -1;
> + }
> + bss->added_if_into_bridge = 0;
> + }
> +
> + ret = send_and_recv_msgs(drv, msg, NULL, NULL);
> + msg = NULL;
> + if (!ret) {
> + if (bridge_ifname[0] && val) {
> + if (i802_check_bridge(drv, bss,
> + bridge_ifname, bss->ifname) < 0)
> + return -1;
> + }
> + return 0;
> + }
> +
> +fail:
> + nlmsg_free(msg);
> + wpa_printf(MSG_ERROR, "nl80211: Failed to set Multi-AP");
> +
> + return ret;
> +}
> +#endif /* CONFIG_MULTI_AP */
[snip]
> diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
> index 77a3133..b158e9a 100644
> --- a/wpa_supplicant/ctrl_iface.c
> +++ b/wpa_supplicant/ctrl_iface.c
> @@ -3328,6 +3328,15 @@ static int wpa_supplicant_ctrl_iface_update_network(
> else if (os_strcmp(name, "priority") == 0)
> wpa_config_update_prio_list(wpa_s->conf);
>
> +#ifdef CONFIG_MULTI_AP
> + if (os_strcmp(name, "multi_ap_enabled") == 0) {
> + wpa_supplicant_deauthenticate(wpa_s, WLAN_REASON_DEAUTH_LEAVING);
> + if (wpa_drv_set_multi_ap(wpa_s, wpa_s->bridge_ifname,
> + ssid->multi_ap_enabled))
As mentioned before: it would make more sense to me to do nothing here, and let
it be handled on the next association.
> + wpa_printf(MSG_ERROR, "failed to set Multi-AP\n");
> + }
> +#endif /* CONFIG_MULTI_AP */
> +
> return 0;
> }
>
> diff --git a/wpa_supplicant/defconfig b/wpa_supplicant/defconfig
> index 08f5857..3c3081c 100644
> --- a/wpa_supplicant/defconfig
> +++ b/wpa_supplicant/defconfig
> @@ -593,3 +593,6 @@ CONFIG_BACKEND=file
> # Opportunistic Wireless Encryption (OWE)
> # Experimental implementation of draft-harkins-owe-07.txt
> #CONFIG_OWE=y
> +
> +#Multi-AP protocol support
> +#CONFIG_MULTI_AP=y
> diff --git a/wpa_supplicant/driver_i.h b/wpa_supplicant/driver_i.h
> index 078de23..d5809c7 100644
> --- a/wpa_supplicant/driver_i.h
> +++ b/wpa_supplicant/driver_i.h
> @@ -30,6 +30,17 @@ static inline void wpa_drv_deinit(struct wpa_supplicant *wpa_s)
> wpa_s->driver->deinit(wpa_s->drv_priv);
> }
>
> +#ifdef CONFIG_MULTI_AP
> +static inline int wpa_drv_set_multi_ap(struct wpa_supplicant *wpa_s,
> + const char *bridge_ifname, int val)
> +{
> + if (wpa_s->driver->set_multi_ap)
> + return wpa_s->driver->set_multi_ap(wpa_s->drv_priv,
> + bridge_ifname, val);
bridge_ifname *must* be wpa_s->bridge_ifname, so I'd remove it as a parameter
of wpa_drv_set_multi_ap and only pass it to the driver callback. That is,
assuming it is needed at all of course.
> + return -1;
> +}
> +#endif /* CONFIG_MULTI_AP */
> +
> static inline int wpa_drv_set_param(struct wpa_supplicant *wpa_s,
> const char *param)
> {
> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
> index dd6dd52..837f036 100644
> --- a/wpa_supplicant/events.c
> +++ b/wpa_supplicant/events.c
> @@ -2266,6 +2266,26 @@ static void interworking_process_assoc_resp(struct wpa_supplicant *wpa_s,
>
> #endif /* CONFIG_INTERWORKING */
>
> +#ifdef CONFIG_MULTI_AP
> +static int wpa_validate_multi_ap_ie(struct wpa_supplicant *wpa_s,
> + const u8 *ies, size_t ies_len)
> +{
> + struct ieee802_11_elems elems;
> +
> + if (ies == NULL)
> + return -1;
> +
> + if (ieee802_11_parse_elems(ies, ies_len, &elems, 1) == ParseFailed)
> + return -1;
> +
> + if (wpa_s->current_ssid->multi_ap_enabled && !elems.multi_ap) {
> + wpa_printf(MSG_INFO, " AP doesn't support Multi-AP");
> + return -1;
Ideally, the check should be something like:
(wpa_s->current_ssid->multi_ap_enabled &&
!(multi_ap->sub_elem_val & MULTI_AP_BACKHAUL_BSS)) ||
(!wpa_s->current_ssid->multi_ap_enabled &&
!(multi_ap->sub_elem_val & MULTI_AP_FRONTHAUL_BSS))
i.e., if we are configured as backhaul STA, then the AP should announce
backhaul BSS; if we are configured as ordinary STA, the AP should announce
fronthaul BSS.
> + }
> +
> + return 0;
> +}
> +#endif /* CONFIG_MULTI_AP */
>
> #ifdef CONFIG_FST
> static int wpas_fst_update_mbie(struct wpa_supplicant *wpa_s,
> @@ -2343,6 +2363,20 @@ static int wpa_supplicant_event_associnfo(struct wpa_supplicant *wpa_s,
> get_ie(data->assoc_info.resp_ies,
> data->assoc_info.resp_ies_len, WLAN_EID_VHT_CAP))
> wpa_s->ieee80211ac = 1;
> +#ifdef CONFIG_MULTI_AP
> + if (wpa_validate_multi_ap_ie(wpa_s, data->assoc_info.resp_ies,
> + data->assoc_info.resp_ies_len)) {
Doesn't this break the non-multi-ap case? If the IE is missing (which it
currently is on all access points...), wpa_validate_multi_ap_ie will return -1.
So probably it should just return 0 if resp_ies == NULL.
> + wpa_supplicant_deauthenticate(wpa_s,
> + WLAN_REASON_INVALID_IE);
> + return -1;
> + } else if (wpa_drv_set_multi_ap(wpa_s, wpa_s->bridge_ifname,
> + wpa_s->current_ssid->multi_ap_enabled)) {
> + wpa_printf(MSG_ERROR, "failed to set Multi-AP\n");
> + wpa_supplicant_deauthenticate(wpa_s,
> + WLAN_REASON_DEAUTH_LEAVING);
> + return -1;
> + }
> +#endif /* CONFIG_MULTI_AP */
> }
> if (data->assoc_info.beacon_ies)
> wpa_hexdump(MSG_DEBUG, "beacon_ies",
> diff --git a/wpa_supplicant/sme.c b/wpa_supplicant/sme.c
> index d569aca..5d1ec35 100644
> --- a/wpa_supplicant/sme.c
> +++ b/wpa_supplicant/sme.c
> @@ -39,6 +39,26 @@ static void sme_obss_scan_timeout(void *eloop_ctx, void *timeout_ctx);
> static void sme_stop_sa_query(struct wpa_supplicant *wpa_s);
> #endif /* CONFIG_IEEE80211W */
>
> +#ifdef CONFIG_MULTI_AP
> +static struct wpabuf *multi_ap_build_assoc_req_ie(void)
Now we have three implemntations of the construction of this vendor-specific
element. Isn't there a way to factor them into a single function?
Regards,
Arnout
[snip]
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
