On 19/11/2018 20:18, Jouni Malinen wrote:
> On Mon, Nov 19, 2018 at 01:36:39PM +0100, Arnout Vandecappelle wrote:
>>> +#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?
>
> If this ends up setting just the 4-address mode, yes, it would seem to
> make sense to call this with name matching the use.
>
>> 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...
>
> That's interesting.. If such combination is not supported, it sounds
> like it would be useful to make sure it can be used at least with
> mac80211-based drivers and IEEE 802.11 frame generation in mac80211.
> That said, the current mac80211 design should already allow different
> combinations of stations connected to an AP mode interface when using
> separate AP VLANs.
VLANs could indeed be a way to make this work.
> However, this patch seems to be using set_multi_ap()
> only in station mode (backhaul STA)
Yes, my remark was about the AP side, i.e. hostapd. Patch 1/2 apparently
doesn't set 4-address mode, so I'm not sure how/if it works then.
> and that is a clearer case since
> nl80211 requires the NL80211_ATTR_4ADDR attribute to be used for that
> and there is only a single AP in that connection so no need to address
> different modes within a netdev.
When transmitting a local frame (i.e. with source address == the STA MAC
address), the backhaul STA may actually use 3-address mode. Fortunately, the
Multi-AP specification allows to use 4-address mode in that case as well, so
indeed a global 4ADDR is sufficient for the STA side.
>
>>> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
>
>>> +#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?
>
> Agreed on the not switching this type of things during an association
> part.. I'm not familiar enough with the specific use case on how the
> netdev got added to comment on that nl80211_create_iface() part. In
> other words, can this be used with the main netdev (say, wlan0) as the
> backhaul STA or is there an expectation that a new netdev/vif is created
> when backhaul STA functionality is enabled?
The main netdev may already be in use by an AP, so indeed it may be needed to
create a new vif. But OpenWRT at least handles that automatically already (it
always creates a new netdev, except if the main netdev is unconfigured). There's
nothing really special about that for the multi-AP case.
>>> diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
>>> +#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.
>
> I'd probably agree on this, but would need to see an example sequence on
> how this is used to fully understand what needs to happen and how this
> ssid->multi_ap_enabled gets set.
I explained this in my other mail, but here's a summary:
Optional: start with WPS (covered by patch 999135 (wpa_supplicant) and 978101
(hostapd).
1. Fronthaul BSS beacons advertise WPS support (nothing multi-ap specific).
2. Enrollee sends authentication req (nothing multi-ap specific).
3. AP sends authentication resp (nothing multi-ap specific).
4. Enrollee sends association req with Multi-AP IE.
5. AP send association resp with Multi-AP IE.
6. Enrollee sends M1 with additional Multi-AP subelement
7. AP sends M8 with additional Multi-AP subelement and backhaul (instead of
fronthaul) credentials.
8. Enrollee sends Deauth.
Now the enrollee can use the backhaul credentials to operate a backhaul STA.
>From this point on, the sequence is the same whether the credentials were
received through WPS or through some other means.
1. Backhaul BSS beacons do not advertise WPS support (other than that, nothing
multi-ap specific).
2. STA sends authentication req (nothing multi-ap specific).
3. AP sends authentication resp (nothing multi-ap specific).
4. STA sends association req with Multi-AP IE.
5. AP send association resp with Multi-AP IE.
6. STA and AP both use 4-address mode for data.
>
>>> diff --git a/wpa_supplicant/driver_i.h b/wpa_supplicant/driver_i.h
>
>>> +#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.
>
> If all the callers use wpa_s->bridge_ifname here, sure, no point in
> passing that as an argument to the wrapper function. As discussed above,
> this should likely be renamed, though, to make it clearer what the
> driver operation is doing (just enabled 4-address mode in backhaul STA
> case? or something beyond that?). hostapd does have code for dynamically
> adding/removing netdevs to/from a bridge,
Why does hostapd have that feature?
> but I'm not sure that's the
> best approach to copy here with wpa_supplicant. High layer description
> of the exact expected sequence would be helpful in understanding this
> and determining whether the bridge interface should be covered here or
> not.
There is not really any difference w.r.t. bridging in the multi-AP case.
Perhaps a router implementation would want to put a multi-AP backhaul link
automatically in the "LAN" bridge, while for a non-multi-AP link it would treat
it as a WAN with masquerading. But that's really policy which has nothing to do
with hostapd/wpa_supplicant IMO.
Regards,
Arnout
[snip]
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
