| Message ID | 20190416123401.12379-1-john@phrozen.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/5] nl80211: add HW capabilities parsing support | expand |
Thanks, all five patches applied with some cleanup and fixes.
On 25/04/2019 13:59, Jouni Malinen wrote: > Thanks, all five patches applied with some cleanup and fixes. > Hi Jouni, I am about to send a V2 of the series :-) can you still drop them or shall I send an incremental series ? John
On Thu, Apr 25, 2019 at 02:53:35PM +0200, John Crispin wrote: > On 25/04/2019 13:59, Jouni Malinen wrote: > >Thanks, all five patches applied with some cleanup and fixes. > I am about to send a V2 of the series :-) can you still drop them or shall I > send an incremental series ? Too late to drop patches once they have been applied to hostap.git. The only option would be to revert the applied commits and then apply something new, but I'm unlikely to accept such process without very strong justification.. In other words, please provide incremental patches on top of the current master branch snapshot. And for future, please do let me know immediately if you start working on updated version on patches so that I can drop the earlier version from my queue.
On Tuesday, 16 April 2019 14:33:57 CEST John Crispin wrote: > Add code to parse the iftype element when reading the band info. This is > required to find out about the HE capabilities of an AP. > > Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com> > Signed-off-by: John Crispin <john@phrozen.org> > --- [...] > +static int phy_info_iftype(struct hostapd_hw_modes *mode, struct nlattr *nl_iftype) > +{ > + struct nlattr *tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_MAX + 1]; > + struct he_capabilities *he_capab = &mode->he_capab; > + struct nlattr *tb_iftypes[NL80211_IFTYPE_MAX + 1]; > + int len; > + > + nla_parse(tb_band_iftypes, NL80211_BAND_IFTYPE_ATTR_MAX, nla_data(nl_iftype), > + nla_len(nl_iftype), NULL); > + > + if (!tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_IFTYPES]) > + return NL_STOP; > + > + if (nla_parse_nested(tb_iftypes, NL80211_IFTYPE_MAX, tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_IFTYPES], NULL)) > + return NL_STOP; > + > + if (!nla_get_flag(tb_iftypes[NL80211_IFTYPE_AP])) > + return NL_OK; This looks wrong - or at least too limited. nl80211 exports this information not only for AP but for all other iftypes. So for all other iftypes than AP (which hostapd+wpa_supplicant supports) the extracted information most likely is incorrect. Seems like we need to extend this. I was first trying to extend get_hw_feature_data to accept a opmode but it turns out that wpa_supplicant doesn't have a concept of opmode for an interface - only for ssid. So we need to change this parsing function to actually get the he_capab for all iftypes and not only for APs. Since the code for the he_capab (just checked 1b90aae9299f0a8618001b38882317787a690485..HEAD) came from you: Do you have patches in the pipeline which modify the he_capab? Because these might also need further adjustments. Kind regards, Sven
On 14/06/2019 14:10, Sven Eckelmann wrote: > On Tuesday, 16 April 2019 14:33:57 CEST John Crispin wrote: >> Add code to parse the iftype element when reading the band info. This is >> required to find out about the HE capabilities of an AP. >> >> Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com> >> Signed-off-by: John Crispin <john@phrozen.org> >> --- > [...] >> +static int phy_info_iftype(struct hostapd_hw_modes *mode, struct nlattr *nl_iftype) >> +{ >> + struct nlattr *tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_MAX + 1]; >> + struct he_capabilities *he_capab = &mode->he_capab; >> + struct nlattr *tb_iftypes[NL80211_IFTYPE_MAX + 1]; >> + int len; >> + >> + nla_parse(tb_band_iftypes, NL80211_BAND_IFTYPE_ATTR_MAX, nla_data(nl_iftype), >> + nla_len(nl_iftype), NULL); >> + >> + if (!tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_IFTYPES]) >> + return NL_STOP; >> + >> + if (nla_parse_nested(tb_iftypes, NL80211_IFTYPE_MAX, tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_IFTYPES], NULL)) >> + return NL_STOP; >> + >> + if (!nla_get_flag(tb_iftypes[NL80211_IFTYPE_AP])) >> + return NL_OK; > This looks wrong - or at least too limited. nl80211 exports this information > not only for AP but for all other iftypes. So for all other iftypes than AP > (which hostapd+wpa_supplicant supports) the extracted information most likely > is incorrect. > > Seems like we need to extend this. I was first trying to extend > get_hw_feature_data to accept a opmode but it turns out that wpa_supplicant > doesn't have a concept of opmode for an interface - only for ssid. So we need > to change this parsing function to actually get the he_capab for all iftypes > and not only for APs. > > Since the code for the he_capab (just checked > 1b90aae9299f0a8618001b38882317787a690485..HEAD) came from you: Do you have > patches in the pipeline which modify the he_capab? Because these might > also need further adjustments. Hi Sven, no further patches in the pipeline for this feature from me. Feel free to fix/extend the feature John > _______________________________________________ > Hostap mailing list > Hostap@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/hostap
diff --git a/src/drivers/driver.h b/src/drivers/driver.h index e7c8f318f..7658e4b56 100644 --- a/src/drivers/driver.h +++ b/src/drivers/driver.h @@ -159,7 +159,9 @@ struct hostapd_channel_data { }; #define HE_MAX_NUM_SS 8 -#define HE_MAX_PHY_CAPAB_SIZE 3 +#define HE_MAX_MAC_CAPAB_SIZE 6 +#define HE_MAX_PHY_CAPAB_SIZE 11 +#define HE_MAX_MCS_CAPAB_SIZE 12 /** * struct he_ppe_threshold - IEEE 802.11ax HE PPE Threshold @@ -175,9 +177,9 @@ struct he_ppe_threshold { */ struct he_capabilities { u8 he_supported; - u32 phy_cap[HE_MAX_PHY_CAPAB_SIZE]; - u32 mac_cap; - u32 mcs; + u8 phy_cap[HE_MAX_PHY_CAPAB_SIZE]; + u8 mac_cap[HE_MAX_MAC_CAPAB_SIZE]; + u8 mcs[HE_MAX_MCS_CAPAB_SIZE]; struct he_ppe_threshold ppet; }; diff --git a/src/drivers/driver_nl80211_capa.c b/src/drivers/driver_nl80211_capa.c index 37eeb5e66..85e6546eb 100644 --- a/src/drivers/driver_nl80211_capa.c +++ b/src/drivers/driver_nl80211_capa.c @@ -18,7 +18,6 @@ #include "common/qca-vendor-attr.h" #include "driver_nl80211.h" - static int protocol_feature_handler(struct nl_msg *msg, void *arg) { u32 *feat = arg; @@ -1598,6 +1597,70 @@ static int phy_info_rates(struct hostapd_hw_modes *mode, struct nlattr *tb) } +static int phy_info_iftype(struct hostapd_hw_modes *mode, struct nlattr *nl_iftype) +{ + struct nlattr *tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_MAX + 1]; + struct he_capabilities *he_capab = &mode->he_capab; + struct nlattr *tb_iftypes[NL80211_IFTYPE_MAX + 1]; + int len; + + nla_parse(tb_band_iftypes, NL80211_BAND_IFTYPE_ATTR_MAX, nla_data(nl_iftype), + nla_len(nl_iftype), NULL); + + if (!tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_IFTYPES]) + return NL_STOP; + + if (nla_parse_nested(tb_iftypes, NL80211_IFTYPE_MAX, tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_IFTYPES], NULL)) + return NL_STOP; + + if (!nla_get_flag(tb_iftypes[NL80211_IFTYPE_AP])) + return NL_OK; + + he_capab->he_supported = 1; + + if (tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_PHY]) { + len = nla_len(tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_PHY]); + + if (len > sizeof(he_capab->phy_cap)) + len = sizeof(he_capab->phy_cap); + os_memcpy(he_capab->phy_cap, + nla_data(tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_PHY]), + len); + } + + if (tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_MAC]) { + len = nla_len(tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_MAC]); + + if (len > sizeof(he_capab->mac_cap)) + len = sizeof(he_capab->mac_cap); + os_memcpy(he_capab->mac_cap, + nla_data(tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_MAC]), + len); + } + + if (tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_MCS_SET]) { + len = nla_len(tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_MCS_SET]); + + if (len > sizeof(he_capab->mcs)) + len = sizeof(he_capab->mcs); + os_memcpy(he_capab->mcs, + nla_data(tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_MCS_SET]), + len); + } + + if (tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_PPE]) { + len = nla_len(tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_PPE]); + + if (len > sizeof(he_capab->ppet)) + len = sizeof(he_capab->ppet); + os_memcpy(&he_capab->ppet, + nla_data(tb_band_iftypes[NL80211_BAND_IFTYPE_ATTR_HE_CAP_PPE]), + len); + } + + return NL_OK; +} + static int phy_info_band(struct phy_info_arg *phy_info, struct nlattr *nl_band) { struct nlattr *tb_band[NL80211_BAND_ATTR_MAX + 1]; @@ -1654,6 +1717,18 @@ static int phy_info_band(struct phy_info_arg *phy_info, struct nlattr *nl_band) return ret; } + if (tb_band[NL80211_BAND_ATTR_IFTYPE_DATA]) { + struct nlattr *nl_iftype; + int rem_band; + + nla_for_each_nested(nl_iftype, tb_band[NL80211_BAND_ATTR_IFTYPE_DATA], rem_band) + { + ret = phy_info_iftype(mode, nl_iftype); + if (ret != NL_OK) + return ret; + } + } + return NL_OK; }