diff mbox series

[v3,14/46] PR: Replace format_and_bw attribute with preamble/BW bitmaps

Message ID 20260513100010.1947710-15-kavita.kavita@oss.qualcomm.com
State Changes Requested
Headers show
Series PR: Add nl80211 support and ranging for Proximity Detection | expand

Commit Message

Kavita Kavita May 13, 2026, 9:59 a.m. UTC 
From: Peddolla Harshavardhan Reddy <peddolla.reddy@oss.qualcomm.com>

The PD ranging capability used a single format-and-bandwidth
attribute in wpa_driver_capa to represent the best supported
combination. This only considered the highest supported bandwidth
and assumed all lower bandwidths were also supported, which is
not always the case. The channel population logic inherited the
same assumption.

Replace the attribute with separate preamble and bandwidth
bitmaps so each supported combination is explicitly represented.
Parse the corresponding nl80211 attributes to populate these
bitmaps and remove the now redundant attribute from driver.h.
Update the channel population logic to check each operating
class bandwidth directly against the bitmap rather than
inferring support from the highest supported value.

Fixes: f45cc220e4bb ("PR: Update PR device configs and capabilities from driver")
Fixes: cfe7a215db0b ("PR: Determine channels that are supported to perform ranging")
Signed-off-by: Peddolla Harshavardhan Reddy <peddolla.reddy@oss.qualcomm.com>
---
 src/common/proximity_ranging.h |   8 ++
 src/drivers/driver.h           |  24 ++++-
 wpa_supplicant/pr_supplicant.c | 181 ++++++++++++++++++---------------
 3 files changed, 127 insertions(+), 86 deletions(-)

Comments

Jouni Malinen May 21, 2026, 7:53 a.m. UTC | #1 
On Wed, May 13, 2026 at 03:29:38PM +0530, Kavita Kavita wrote:
> The PD ranging capability used a single format-and-bandwidth
> attribute in wpa_driver_capa to represent the best supported
> combination. This only considered the highest supported bandwidth
> and assumed all lower bandwidths were also supported, which is
> not always the case. The channel population logic inherited the
> same assumption.
> 
> Replace the attribute with separate preamble and bandwidth
> bitmaps so each supported combination is explicitly represented.
> Parse the corresponding nl80211 attributes to populate these
> bitmaps and remove the now redundant attribute from driver.h.
> Update the channel population logic to check each operating
> class bandwidth directly against the bitmap rather than
> inferring support from the highest supported value.

What is that discussion about parsing nl80211 attributes about? This
patch does not include any changes to src/drivers/driver_nl80211*.c.

> diff --git a/src/common/proximity_ranging.h b/src/common/proximity_ranging.h
> @@ -98,6 +98,7 @@ enum edca_format_and_bw_value {
>  	EDCA_FORMAT_AND_BW_VHT160_SINGLE_LO = 16,
> +	EDCA_FORMAT_AND_BW_MAX,

>  /**
> @@ -114,6 +115,7 @@ enum ntb_format_and_bw_value {
>  	NTB_FORMAT_AND_BW_HE160_SINGLE_LO = 5,
> +	NTB_FORMAT_AND_BW_MAX,

How exactly are these values expected to be used? Aren't those only for
implementation specific purposes to indicate no matching value was
found? However, the implementation seems to be encoding these in
pr_get_edca_capabilities() into an actual capability field.. Is that
expected? It was a bit difficult to understand the changes since these
*_FORMAT_AND_BW_MAX values were actually not checked against anywhere,
i.e., they were only returned when there was no match.

> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
>  
> +/**
> + * Channel width index values used in PD ranging capability bitmaps
> + * (pd_bandwidths). The bit position in the bitmap corresponds to the
> + * channel width; these values match the nl80211_chan_width enum so
> + * that drivers can use them directly.
> + */
> +#define WPA_PR_CHAN_WIDTH_20		1
> +#define WPA_PR_CHAN_WIDTH_40		2
> +#define WPA_PR_CHAN_WIDTH_80		3
> +#define WPA_PR_CHAN_WIDTH_80P80	4
> +#define WPA_PR_CHAN_WIDTH_160		5
> +
> +/**
> + * Preamble index values used in PD ranging capability bitmaps
> + * (pd_preambles). The bit position in the bitmap corresponds to the
> + * preamble type; these values match the nl80211_preamble enum so
> + * that drivers can use them directly.
> + */
> +#define WPA_PR_PREAMBLE_HT	1
> +#define WPA_PR_PREAMBLE_VHT	2
> +#define WPA_PR_PREAMBLE_HE	4

It is not a good idea to try to match values of nl80211.h enums in this
manner in a context that is supposed to be independent of the driver
interface. In other words, the references to nl80211 needs to be removed
here and driver_nl80211*.c should use mapping functions between the
wpa_supplicant specific values and the values in nl80211.h.
diff mbox series

Patch

diff --git a/src/common/proximity_ranging.h b/src/common/proximity_ranging.h
index 85fc40770..2643f8534 100644
--- a/src/common/proximity_ranging.h
+++ b/src/common/proximity_ranging.h
@@ -98,6 +98,7 @@  enum edca_format_and_bw_value {
 	EDCA_FORMAT_AND_BW_VHT80P80 = 14,
 	EDCA_FORMAT_AND_BW_VHT160_DUAL_LO = 15,
 	EDCA_FORMAT_AND_BW_VHT160_SINGLE_LO = 16,
+	EDCA_FORMAT_AND_BW_MAX,
 };
 
 /**
@@ -114,6 +115,7 @@  enum ntb_format_and_bw_value {
 	NTB_FORMAT_AND_BW_HE80P80 = 3,
 	NTB_FORMAT_AND_BW_HE160_DUAL_LO = 4,
 	NTB_FORMAT_AND_BW_HE160_SINGLE_LO = 5,
+	NTB_FORMAT_AND_BW_MAX,
 };
 
 struct pr_capabilities {
@@ -356,6 +358,7 @@  struct pr_config {
 
 	bool edca_rsta_support;
 
+	/* Best single format_bw value derived from pd_format_bw_bitmap */
 	u8 edca_format_and_bw;
 
 	u32 edca_min_ranging_interval;
@@ -398,6 +401,11 @@  struct pr_config {
 
 	u8 max_tx_sts_gt_80;
 
+	/* PD ranging preamble and bandwidth bitmaps (shared by EDCA and NTB) */
+	u32 pd_preamble_bitmap;
+	u32 pd_format_bw_bitmap;
+
+	/* Best single format_bw value derived from pd_format_bw_bitmap */
 	u8 ntb_format_and_bw;
 
 	u32 ntb_min_ranging_interval;
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index 32cdbe800..a1f922f9a 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -2246,6 +2246,28 @@  struct nan_capa {
 	bool he_valid;
 };
 
+/**
+ * Channel width index values used in PD ranging capability bitmaps
+ * (pd_bandwidths). The bit position in the bitmap corresponds to the
+ * channel width; these values match the nl80211_chan_width enum so
+ * that drivers can use them directly.
+ */
+#define WPA_PR_CHAN_WIDTH_20		1
+#define WPA_PR_CHAN_WIDTH_40		2
+#define WPA_PR_CHAN_WIDTH_80		3
+#define WPA_PR_CHAN_WIDTH_80P80	4
+#define WPA_PR_CHAN_WIDTH_160		5
+
+/**
+ * Preamble index values used in PD ranging capability bitmaps
+ * (pd_preambles). The bit position in the bitmap corresponds to the
+ * preamble type; these values match the nl80211_preamble enum so
+ * that drivers can use them directly.
+ */
+#define WPA_PR_PREAMBLE_HT	1
+#define WPA_PR_PREAMBLE_VHT	2
+#define WPA_PR_PREAMBLE_HE	4
+
 /**
  * struct wpa_driver_capa - Driver capability information
  */
@@ -2657,13 +2679,11 @@  struct wpa_driver_capa {
 	size_t max_probe_req_ie_len;
 
 	/* EDCA based ranging capabilities */
-	u8 edca_format_and_bw;
 	u8 max_tx_antenna;
 	u8 max_rx_antenna;
 	u32 edca_min_ranging_interval;
 
 	/* NTB based ranging capabilities */
-	u8 ntb_format_and_bw;
 	u8 max_tx_ltf_repetations;
 	u8 max_rx_ltf_repetations;
 	u8 max_tx_ltf_total;
diff --git a/wpa_supplicant/pr_supplicant.c b/wpa_supplicant/pr_supplicant.c
index be5477b44..f50712a96 100644
--- a/wpa_supplicant/pr_supplicant.c
+++ b/wpa_supplicant/pr_supplicant.c
@@ -24,115 +24,118 @@  static void wpas_pr_pasn_timeout(void *eloop_ctx, void *timeout_ctx);
 #endif /* CONFIG_PASN */
 
 
-static int wpas_pr_edca_get_bw(enum edca_format_and_bw_value format_and_bw)
+static enum edca_format_and_bw_value
+wpas_pr_best_edca_format_bw(u32 bw_bitmap, u32 preamble_bitmap)
 {
-	switch (format_and_bw) {
-	case EDCA_FORMAT_AND_BW_VHT20:
-		return 20;
-	case EDCA_FORMAT_AND_BW_HT40:
-	case EDCA_FORMAT_AND_BW_VHT40:
-		return 40;
-	case EDCA_FORMAT_AND_BW_VHT80:
-		return 80;
-	case EDCA_FORMAT_AND_BW_VHT80P80:
-	case EDCA_FORMAT_AND_BW_VHT160_DUAL_LO:
-	case EDCA_FORMAT_AND_BW_VHT160_SINGLE_LO:
-		return 160;
-	default:
-		return 0;
-	}
+	/* Prefer highest bandwidth first */
+	if ((bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_160)) &&
+	    (preamble_bitmap & BIT(WPA_PR_PREAMBLE_VHT)))
+		return EDCA_FORMAT_AND_BW_VHT160_DUAL_LO;
+	if ((bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_80P80)) &&
+	    (preamble_bitmap & BIT(WPA_PR_PREAMBLE_VHT)))
+		return EDCA_FORMAT_AND_BW_VHT80P80;
+	if ((bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_80)) &&
+	    (preamble_bitmap & BIT(WPA_PR_PREAMBLE_VHT)))
+		return EDCA_FORMAT_AND_BW_VHT80;
+	if ((bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_40)) &&
+	    (preamble_bitmap & BIT(WPA_PR_PREAMBLE_VHT)))
+		return EDCA_FORMAT_AND_BW_VHT40;
+	if ((bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_40)) &&
+	    (preamble_bitmap & BIT(WPA_PR_PREAMBLE_HT)))
+		return EDCA_FORMAT_AND_BW_HT40;
+	if ((bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_20)) &&
+	    (preamble_bitmap & BIT(WPA_PR_PREAMBLE_VHT)))
+		return EDCA_FORMAT_AND_BW_VHT20;
+	return EDCA_FORMAT_AND_BW_MAX;
 }
 
 
-static int wpas_pr_ntb_get_bw(enum ntb_format_and_bw_value format_and_bw)
+static enum ntb_format_and_bw_value
+wpas_pr_best_ntb_format_bw(u32 bw_bitmap, u32 preamble_bitmap)
 {
-	switch (format_and_bw) {
-	case NTB_FORMAT_AND_BW_HE20:
-		return 20;
-	case NTB_FORMAT_AND_BW_HE40:
-		return 40;
-	case NTB_FORMAT_AND_BW_HE80:
-		return 80;
-	case NTB_FORMAT_AND_BW_HE80P80:
-	case NTB_FORMAT_AND_BW_HE160_DUAL_LO:
-	case NTB_FORMAT_AND_BW_HE160_SINGLE_LO:
-		return 160;
-	default:
-		return 0;
-	}
+	if (!(preamble_bitmap & BIT(WPA_PR_PREAMBLE_HE)))
+		return NTB_FORMAT_AND_BW_MAX;
+
+	if (bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_160))
+		return NTB_FORMAT_AND_BW_HE160_SINGLE_LO;
+	if (bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_80P80))
+		return NTB_FORMAT_AND_BW_HE80P80;
+	if (bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_80))
+		return NTB_FORMAT_AND_BW_HE80;
+	if (bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_40))
+		return NTB_FORMAT_AND_BW_HE40;
+	if (bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_20))
+		return NTB_FORMAT_AND_BW_HE20;
+	return NTB_FORMAT_AND_BW_MAX;
 }
 
 
 static bool
-wpas_pr_edca_is_valid_op_class(enum edca_format_and_bw_value format_and_bw,
+wpas_pr_edca_is_valid_op_class(u32 bw_bitmap, u32 preamble_bitmap,
 			       const struct oper_class_map *op_class_map)
 {
-	int bw = 0, op_class_bw = 0;
-
 	if (!op_class_map)
 		return false;
 
-	op_class_bw = oper_class_bw_to_int(op_class_map);
-	bw = wpas_pr_edca_get_bw(format_and_bw);
-
-	if (!op_class_bw || !bw)
+	switch (op_class_map->bw) {
+	case BW20:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_20)) &&
+		       !!(preamble_bitmap & BIT(WPA_PR_PREAMBLE_VHT));
+	case BW40PLUS:
+	case BW40MINUS:
+	case BW40:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_40)) &&
+		       !!(preamble_bitmap & (BIT(WPA_PR_PREAMBLE_VHT) |
+					     BIT(WPA_PR_PREAMBLE_HT)));
+	case BW80:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_80)) &&
+		       !!(preamble_bitmap & BIT(WPA_PR_PREAMBLE_VHT));
+	case BW80P80:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_80P80)) &&
+		       !!(preamble_bitmap & BIT(WPA_PR_PREAMBLE_VHT));
+	case BW160:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_160)) &&
+		       !!(preamble_bitmap & BIT(WPA_PR_PREAMBLE_VHT));
+	default:
 		return false;
-
-	if (format_and_bw <= EDCA_FORMAT_AND_BW_VHT80 &&
-	    format_and_bw >= EDCA_FORMAT_AND_BW_VHT20 &&
-	    op_class_bw <= bw)
-		return true;
-
-	if (format_and_bw == EDCA_FORMAT_AND_BW_VHT80P80 &&
-	    (op_class_bw < bw || op_class_map->bw == BW80P80))
-		return true;
-
-	if ((format_and_bw == EDCA_FORMAT_AND_BW_VHT160_DUAL_LO ||
-	     format_and_bw == EDCA_FORMAT_AND_BW_VHT160_SINGLE_LO) &&
-	    (op_class_bw < bw || op_class_map->bw == BW160))
-		return true;
-
-	return false;
+	}
 }
 
 
 static bool
-wpas_pr_ntb_is_valid_op_class(enum ntb_format_and_bw_value format_and_bw,
+wpas_pr_ntb_is_valid_op_class(u32 bw_bitmap, u32 preamble_bitmap,
 			      const struct oper_class_map *op_class_map)
 {
-	int bw = 0, op_class_bw = 0;
-
 	if (!op_class_map)
 		return false;
 
-	op_class_bw = oper_class_bw_to_int(op_class_map);
-	bw = wpas_pr_ntb_get_bw(format_and_bw);
-
-	if (!op_class_bw || !bw)
+	/* NTB ranging requires HE preamble */
+	if (!(preamble_bitmap & BIT(WPA_PR_PREAMBLE_HE)))
 		return false;
 
-	if (format_and_bw <= NTB_FORMAT_AND_BW_HE80 &&
-	    format_and_bw >= NTB_FORMAT_AND_BW_HE20 &&
-	    op_class_bw <= bw)
-		return true;
-
-	if (format_and_bw == NTB_FORMAT_AND_BW_HE80P80 &&
-		   (op_class_bw < bw || op_class_map->bw == BW80P80))
-		return true;
-
-	if ((format_and_bw == NTB_FORMAT_AND_BW_HE160_DUAL_LO ||
-	     format_and_bw == NTB_FORMAT_AND_BW_HE160_SINGLE_LO) &&
-	    (op_class_bw < bw || op_class_map->bw == BW160))
-		return true;
-
-	return false;
+	switch (op_class_map->bw) {
+	case BW20:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_20));
+	case BW40PLUS:
+	case BW40MINUS:
+	case BW40:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_40));
+	case BW80:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_80));
+	case BW80P80:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_80P80));
+	case BW160:
+		return !!(bw_bitmap & BIT(WPA_PR_CHAN_WIDTH_160));
+	default:
+		return false;
+	}
 }
 
 
 static void
 wpas_pr_setup_edca_channels(struct wpa_supplicant *wpa_s,
 			    struct pr_channels *chan,
-			    enum edca_format_and_bw_value format_and_bw)
+			    u32 bw_bitmap, u32 preamble_bitmap)
 {
 	struct hostapd_hw_modes *mode;
 	int cla = 0, i;
@@ -145,7 +148,8 @@  wpas_pr_setup_edca_channels(struct wpa_supplicant *wpa_s,
 		mode = get_mode(wpa_s->hw.modes, wpa_s->hw.num_modes, o->mode,
 				is_6ghz_op_class(o->op_class));
 		if (!mode || is_6ghz_op_class(o->op_class) ||
-		    !wpas_pr_edca_is_valid_op_class(format_and_bw, o))
+		    !wpas_pr_edca_is_valid_op_class(bw_bitmap, preamble_bitmap,
+						    o))
 			continue;
 
 		for (ch = o->min_chan; ch <= o->max_chan; ch += o->inc) {
@@ -191,7 +195,7 @@  wpas_pr_setup_edca_channels(struct wpa_supplicant *wpa_s,
 static void
 wpas_pr_setup_ntb_channels(struct wpa_supplicant *wpa_s,
 			   struct pr_channels *chan,
-			   enum ntb_format_and_bw_value format_and_bw,
+			   u32 bw_bitmap, u32 preamble_bitmap,
 			   bool allow_6ghz)
 {
 	int cla = 0, i;
@@ -205,7 +209,8 @@  wpas_pr_setup_ntb_channels(struct wpa_supplicant *wpa_s,
 		mode = get_mode(wpa_s->hw.modes, wpa_s->hw.num_modes, o->mode,
 				is_6ghz_op_class(o->op_class));
 		if (!mode || (!allow_6ghz && is_6ghz_op_class(o->op_class)) ||
-		    !wpas_pr_ntb_is_valid_op_class(format_and_bw, o))
+		    !wpas_pr_ntb_is_valid_op_class(bw_bitmap, preamble_bitmap,
+						   o))
 			continue;
 
 		for (ch = o->min_chan; ch <= o->max_chan; ch += o->inc) {
@@ -352,16 +357,23 @@  int wpas_pr_init(struct wpa_global *global, struct wpa_supplicant *wpa_s,
 
 	pr.edca_ista_support = capa->ista.support_edca;
 	pr.edca_rsta_support = capa->rsta.support_edca;
-	pr.edca_format_and_bw = capa->edca_format_and_bw;
+	pr.pd_format_bw_bitmap = capa->pd_bandwidths;
+	pr.pd_preamble_bitmap = capa->pd_preambles;
+	pr.edca_format_and_bw =
+		wpas_pr_best_edca_format_bw(capa->pd_bandwidths,
+					    capa->pd_preambles);
 	pr.max_rx_antenna = capa->max_rx_antenna;
 	pr.max_tx_antenna = capa->max_tx_antenna;
 
 	wpas_pr_setup_edca_channels(wpa_s, &pr.edca_channels,
-				    pr.edca_format_and_bw);
+				    capa->pd_bandwidths,
+				    capa->pd_preambles);
 
 	pr.ntb_ista_support = capa->ista.support_ntb;
 	pr.ntb_rsta_support = capa->rsta.support_ntb;
-	pr.ntb_format_and_bw = capa->ntb_format_and_bw;
+	pr.ntb_format_and_bw =
+		wpas_pr_best_ntb_format_bw(capa->pd_bandwidths,
+					   capa->pd_preambles);
 	pr.max_tx_ltf_repetations = capa->max_tx_ltf_repetations;
 	pr.max_rx_ltf_repetations = capa->max_rx_ltf_repetations;
 	pr.max_tx_ltf_total = capa->max_tx_ltf_total;
@@ -390,7 +402,8 @@  int wpas_pr_init(struct wpa_global *global, struct wpa_supplicant *wpa_s,
 	pr.secure_he_ltf = wpa_s->drv_flags2 & WPA_DRIVER_FLAGS2_SEC_LTF_STA;
 
 	wpas_pr_setup_ntb_channels(wpa_s, &pr.ntb_channels,
-				   pr.ntb_format_and_bw,
+				   capa->pd_bandwidths,
+				   capa->pd_preambles,
 				   pr.support_6ghz);
 
 	if (wpa_s->conf->country[0] && wpa_s->conf->country[1]) {