On Mon, Nov 19, 2018 at 11:05:49AM +0100, Arnout Vandecappelle wrote: > On 18/11/2018 23:30, Jouni Malinen wrote: > > I had couple of other patches waiting for review and sending out to the > > list, but those got delayed as well. It would be good to see what kind > > of combination of functionality from this set of patches would be best > > for moving ahead with the Multi-AP functionality. The relevant patches > > are here: > > > > http://patchwork.ozlabs.org/patch/978101/ > > http://patchwork.ozlabs.org/patch/999135/ > > > > http://patchwork.ozlabs.org/patch/999588/ > > http://patchwork.ozlabs.org/patch/999587/ > > > > I'll try to go these in more detail over the coming week, so any > > comments would be welcome. Based on a very quick review, there is some > > common functionality and some additional functionality in each set, so > > some kind of merge of the patches could be needed. > > It sure looks like that. In particular, the hostapd.conf options are too much > overlapping I think. Actually, it looks like the duplicated functionality is pretty limited in these patches and this is mainly focused in how the hostapd.conf and wpa_supplicant.conf parameters are handled. > Jouni, will you be working on combining the patches in a single series? Based on my review, quite a few changes beyond merging these together is going to be needed, so it would probably be better if someone else (or multiple people with some coordination) would volunteer to propose updated patches with changes to address the comments. Other than the configuration details, most of the changes look straightforward and also the four patches are mostly independent in core functionality in areas beyond those configuration parameters. I would like to finally get version 2.7 released, so my main focus in the near term is to go through the pending contributions and to merge in clear bug fixes while leaving new functionality (like this Multi-AP) to be post-2.7 items. If there are any issues in getting these Multi-AP patch updates coordinated, I may try to find time to work on them myself after that release, but that should really be considered a backup plan rather than something that would result in getting these in any time soon.. I'm including my notes from going through the four patches below. Please let me know if you have any other comments or different views on the comments/proposed changes listed below. http://patchwork.ozlabs.org/patch/978101/ easymesh: add backhaul BSS support for WPS M8 functionality: - adds hostapd configuration to use an externally generated file as a WPS Credential for backhaul BSS; this is added to WPS M8 conditionally - parsing and processing of of Multi-AP extension from WPS M1 comments: - rename "easymesh" to "Multi-AP" - should not need wps_cred_processing_easymesh config parameter, i.e., presence of easymesh_backhaul_ap_settings is sufficient to determine whether the feature is enabled - cannot use hardcoded external file to build the Credential for backhaul STA since MAC Address attribute depends on the Enrollee's MAC address and changes for each device * configure needed SSID/passphrase/etc. parameters instead and allow hostapd to build this Credential dynamically when needed http://patchwork.ozlabs.org/patch/999135/ easymesh: add a config option to enable EasyMesh backhaul STA functionality: - add Multi-AP extension into WPS M1 - adds global wpa_supplicant config parameter easymesh_backhaul_sta=0/1 comments: - rename "easymesh" to "Multi-AP" - wps_build_vendor_ext_m1_wfa() adds a new WFA vendor extension into M1 even though wps_build_wfa_ext() has already done that for the other WFA vendor extensions; this does not look correct, i.e., there should be only a single WFA vendor extension attribute that would include the existing extensions (e.g., Version2) and the Multi-AP extension; wps_build_wfa_ext() would need to be extended instead of adding a new function - the commit message talks about adding a Multi-AP IE into (Re)Association Request frames, but this patch does not do that * simply remove that comment from the commit message(?) (i.e., handle (Re)Association Request frame in a separate patch) http://patchwork.ozlabs.org/patch/999588/ hostapd: Add Multi-AP protocol support functionality: - adds hostapd configuration parameter multi_ap_enabled (disabled, backhaul-BSS, fronthaul-BSS, both backhaul and fronthaul-BSS) - adds Multi-AP IE into (Re)Association Response frames - checks Multi-AP IE in (Re)Association Request frames - configures STA for 4-address WDS mode based on Multi-AP capability comments: - struct multi_ap_ie is hardcoded to include a single Multi-AP Extension subelement * this is not ideal for subelement design * not acceptable for parsing received Multi-AP IE since there could be other subelements - hostapd_validate_multi_ap_ie() does not check anything about the Multi-AP IE payload (i.e., would not handle unexpected subelement before the Multi-AP extension and does not check that the extension has an acceptable value); WLAN_STA_MULTI_AP flag should be added only if the contents is appropriate for backhaul STA http://patchwork.ozlabs.org/patch/999587/ wpa_supplicant: Add Multi-AP protocol support to supplicant functionality: - set_multi_ap() driver wrapper to configure backhaul STA interface into 4-address mode (including nl80211 interface implementation) - multi_ap_enabled network profile parameter in wpa_supplicant - add Multi-AP IE into (Re)Association Request frames - validate Multi-AP IE in (Re)Association Response frames comments: - should consider renaming set_multi_ap() - would be good to review high level sequence of backhaul STA addition and the needed driver configuration steps; including how the dynamic multi_ap_enabled config parameter changes are used (and if they are really needed) - Multi-AP IE parsing needs to be aware of multiple possible subelements - should not break non-Multi-AP case in association event processing - should use a single helper function to build all different cases of Multi-AP IE contents -- Jouni Malinen PGP id EFC895FA _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
