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

[PATCH v2 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 v2 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.

All length check failures were changed to continue instead of
fail. This will now treat invalid lengths as if the IE did not
exist.

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 | 44 ++++++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

v2:
 - Make all length checks continue instead of return

diff --git a/src/wiphy.c b/src/wiphy.c
index 0d64b1b3..712d20cf 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;
@@ -1005,7 +1006,7 @@ int wiphy_estimate_data_rate(struct wiphy *wiphy,
 		switch (tag) {
 		case IE_TYPE_SUPPORTED_RATES:
 			if (iter.len > 8)
-				return -EBADMSG;
+				continue;
 
 			supported_rates = iter.data - 2;
 			break;
@@ -1014,31 +1015,33 @@ int wiphy_estimate_data_rate(struct wiphy *wiphy,
 			break;
 		case IE_TYPE_HT_CAPABILITIES:
 			if (iter.len != 26)
-				return -EBADMSG;
+				continue;
 
 			ht_capabilities = iter.data - 2;
 			break;
 		case IE_TYPE_HT_OPERATION:
 			if (iter.len != 22)
-				return -EBADMSG;
+				continue;
 
 			ht_operation = iter.data - 2;
 			break;
 		case IE_TYPE_VHT_CAPABILITIES:
 			if (iter.len != 12)
-				return -EBADMSG;
+				continue;
 
 			vht_capabilities = iter.data - 2;
 			break;
 		case IE_TYPE_VHT_OPERATION:
 			if (iter.len != 5)
-				return -EBADMSG;
+				continue;
 
 			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 v2 1/2] band: make HE/no-HT rate estimators return more descriptive

[Denis Kenzior]

Hi James,

On 3/28/24 09:26, James Prestwood wrote:
> 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(-)
> 

Both applied, thanks.

Regards,
-Denis