[PATCH 1/2] band: make HE/no-HT rate estimators return more descriptive

[PATCH 1/2] band: make HE/no-HT rate estimators return more descriptive

[James Prestwood]

If the IEs were not provided return -ENOTSUP rather than -EINVAL
or -EBADMSG since this just means the AP doesn't support those
capabilities.
---
 src/band.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/band.c b/src/band.c
index bf40f789..11cd965e 100644
--- a/src/band.c
+++ b/src/band.c
@@ -90,7 +90,7 @@ int band_estimate_nonht_rate(const struct band *band,
 	int i;
 
 	if (!supported_rates && !ext_supported_rates)
-		return -EINVAL;
+		return -ENOTSUP;
 
 	/*
 	 * Start at the back of the array.  Rates are generally given in
@@ -607,7 +607,7 @@ int band_estimate_he_rx_rate(const struct band *band, const uint8_t *hec,
 	uint8_t width_set;
 
 	if (!hec || !band->he_capabilities)
-		return -EBADMSG;
+		return -ENOTSUP;
 
 	for (entry = l_queue_get_entries(band->he_capabilities);
 						entry; entry = entry->next) {
-- 
2.34.1

[PATCH 2/2] wiphy: add better handling for rate estimation errors

[James Prestwood]

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(-)

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)
-- 
2.34.1

Re: [PATCH 2/2] wiphy: add better handling for rate estimation errors

[Denis Kenzior]

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

Re: [PATCH 2/2] wiphy: add better handling for rate estimation errors

[James Prestwood]

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
>