| Message ID | 20240328134725.1484257-2-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] band: make HE/no-HT rate estimators return more descriptive | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| prestwoj/iwd-ci-gitlint | success | GitLint |
Hi James, On 3/28/24 08:47, James Prestwood wrote: > In most cases any failure here is likely just due to the AP not > supporting the feature, whether its HE/VHT/HE. This should result > in the estimation returning -ENOTSUP in which case we move down > the list. Any other non-zero return we will now warn to make it > clear the IEs did exist, but were not properly formatted. > > In addition HE specifically has an extra validation function which, > if failed, was bailing out of the estimation function entirely. > Instead this is now treated as if there was no HE capabilities and > the logic can move down to VHT, HT, or basic rates. > --- > src/wiphy.c | 34 +++++++++++++++++++++++++--------- > 1 file changed, 25 insertions(+), 9 deletions(-) > <snip> > @@ -1037,8 +1038,10 @@ int wiphy_estimate_data_rate(struct wiphy *wiphy, > vht_operation = iter.data - 2; > break; > case IE_TYPE_HE_CAPABILITIES: > - if (!ie_validate_he_capabilities(iter.data, iter.len)) > - return -EBADMSG; > + if (!ie_validate_he_capabilities(iter.data, iter.len)) { > + l_warn("invalid HE capabilities"); > + continue; > + } I guess you would want to treat all IEs this way? Suppose VHT Capabilities is somehow the wrong size? > > he_capabilities = iter.data; > break; Regards, -Denis
On 3/28/24 7:15 AM, Denis Kenzior wrote: > Hi James, > > On 3/28/24 08:47, James Prestwood wrote: >> In most cases any failure here is likely just due to the AP not >> supporting the feature, whether its HE/VHT/HE. This should result >> in the estimation returning -ENOTSUP in which case we move down >> the list. Any other non-zero return we will now warn to make it >> clear the IEs did exist, but were not properly formatted. >> >> In addition HE specifically has an extra validation function which, >> if failed, was bailing out of the estimation function entirely. >> Instead this is now treated as if there was no HE capabilities and >> the logic can move down to VHT, HT, or basic rates. >> --- >> src/wiphy.c | 34 +++++++++++++++++++++++++--------- >> 1 file changed, 25 insertions(+), 9 deletions(-) >> > > <snip> > >> @@ -1037,8 +1038,10 @@ int wiphy_estimate_data_rate(struct wiphy *wiphy, >> vht_operation = iter.data - 2; >> break; >> case IE_TYPE_HE_CAPABILITIES: >> - if (!ie_validate_he_capabilities(iter.data, iter.len)) >> - return -EBADMSG; >> + if (!ie_validate_he_capabilities(iter.data, iter.len)) { >> + l_warn("invalid HE capabilities"); >> + continue; >> + } > > I guess you would want to treat all IEs this way? Suppose VHT > Capabilities is somehow the wrong size? Sure, that sounds good to me. > >> he_capabilities = iter.data; >> break; > > Regards, > -Denis >
diff --git a/src/wiphy.c b/src/wiphy.c index 0d64b1b3..705fb9cc 100644 --- a/src/wiphy.c +++ b/src/wiphy.c @@ -989,6 +989,7 @@ int wiphy_estimate_data_rate(struct wiphy *wiphy, const void *he_capabilities = NULL; const struct band *bandp; enum band_freq band; + int ret; if (band_freq_to_channel(bss->frequency, &band) == 0) return -ENOTSUP; @@ -1037,8 +1038,10 @@ int wiphy_estimate_data_rate(struct wiphy *wiphy, vht_operation = iter.data - 2; break; case IE_TYPE_HE_CAPABILITIES: - if (!ie_validate_he_capabilities(iter.data, iter.len)) - return -EBADMSG; + if (!ie_validate_he_capabilities(iter.data, iter.len)) { + l_warn("invalid HE capabilities"); + continue; + } he_capabilities = iter.data; break; @@ -1047,26 +1050,39 @@ int wiphy_estimate_data_rate(struct wiphy *wiphy, } } - if (!band_estimate_he_rx_rate(bandp, he_capabilities, + ret = band_estimate_he_rx_rate(bandp, he_capabilities, bss->signal_strength / 100, - out_data_rate)) + out_data_rate); + if (!ret) return 0; + else if (ret != -ENOTSUP) + l_warn("error parsing HE capabilities"); - if (!band_estimate_vht_rx_rate(bandp, vht_capabilities, vht_operation, + ret = band_estimate_vht_rx_rate(bandp, vht_capabilities, vht_operation, ht_capabilities, ht_operation, bss->signal_strength / 100, - out_data_rate)) + out_data_rate); + if (!ret) return 0; + else if (ret != -ENOTSUP) + l_warn("error parsing VHT capabilities"); - if (!band_estimate_ht_rx_rate(bandp, ht_capabilities, ht_operation, + ret = band_estimate_ht_rx_rate(bandp, ht_capabilities, ht_operation, bss->signal_strength / 100, - out_data_rate)) + out_data_rate); + if (!ret) return 0; + else if (ret != -ENOTSUP) + l_warn("error parsing HT capabilities"); - return band_estimate_nonht_rate(bandp, supported_rates, + ret = band_estimate_nonht_rate(bandp, supported_rates, ext_supported_rates, bss->signal_strength / 100, out_data_rate); + if (ret != 0 && ret != -ENOTSUP) + l_warn("error parsing non-HT rates"); + + return ret; } bool wiphy_regdom_is_updating(struct wiphy *wiphy)