[PATCH 4/6] nl80211: propagate S1G widths and freq offsets

[Lachlan Hodges]

Hi,

On Tue, Sep 30, 2025 at 03:13:19PM -0400, James Ewing wrote:
> Teach the nl80211 backend to recognise the new S1G channel widths and to
> forward the kHz-level frequency metadata reported by the kernel. This
> includes advertising the right width in STA opmode notifications,
> packaging centre-frequency offsets for hostapd, and populating scan
> results with freq_khz/freq_offset so higher layers can keep sub-MHz
> precision.
> 
> Tested: tests/hwsim/run_s1g_smoke.sh
> Signed-off-by: James Ewing <james at teledatics.com>
> ---
>   hostapd/ctrl_iface.c               | 17 ++++++
>   src/common/hw_features_common.c    | 16 +++++-
>   src/drivers/driver.h               | 21 +++++++
>   src/drivers/driver_nl80211.c       | 90 +++++++++++++++++++++++++++++-
>   src/drivers/driver_nl80211_event.c | 28 ++++++++++
>   src/drivers/driver_nl80211_scan.c  | 32 +++++++++++
>   6 files changed, 200 insertions(+), 4 deletions(-)
> 
>       const u8 *ie, *beacon_ie;
> @@ -844,15 +846,45 @@ nl80211_parse_bss_info(struct 
[...]
>       r->flags |= WPA_SCAN_NOISE_INVALID;
>       if (bss[NL80211_BSS_SIGNAL_MBM]) {
>           r->level = nla_get_u32(bss[NL80211_BSS_SIGNAL_MBM]);

Seems like something went wrong when formatting or submitting the patch?

>    * @flags: information flags about the BSS/IBSS (WPA_SCAN_*)
>    * @bssid: BSSID
>    * @freq: frequency of the channel in MHz (e.g., 2412 = channel 1)
> + * @freq_khz: frequency in kHz when provided by the driver (0 if unknown)
> + * @freq_offset: frequency offset in kHz relative to @freq (legacy drivers
> + *    that do not report this leave it zero)
>    * @max_cw: the max channel width of the connection (calculated during 
> scan
>    * result processing)
>    * @beacon_int: beacon interval in TUs (host byte order)
> @@ -410,6 +413,8 @@ struct wpa_scan_res {
>       unsigned int flags;
>       u8 bssid[ETH_ALEN];
>       int freq;
> +    unsigned int freq_khz;
> +    unsigned int freq_offset;

Storing a separate frequency value is probably not a good idea. What would
probably be preferred is using the existing values in MHz and the freq_offset
value in KHz. That saves us passing duplicate information everywhere. This is
how it's done in the kernel and IMO this is how it should be done in usermode.

As a result of this you also introduce duplicate fields in KHz for center_freq1.
This could be made much simpler with some basic helpers and adding freq_offset
in KHz similar to how the kernel does it.

> +    /**
> +     * center_freq2_khz - Segment 1 center frequency in kHz (0 if unknown)
> +     */
> +    unsigned int center_freq2_khz;

center_freq2 is unused by S1G.

> -    if (freq->vht_enabled ||
> -        ((freq->he_enabled || freq->eht_enabled) && !is_24ghz)) {
> -        enum nl80211_chan_width cw;
> +    if (freq->bandwidth == 16 || freq->bandwidth == 10 ||
> +        freq->bandwidth == 8 || freq->bandwidth == 5 ||
> +        freq->bandwidth == 4 || freq->bandwidth == 2 ||
> +        freq->bandwidth == 1) {
> +        wpa_printf(MSG_DEBUG, "  * bandwidth=%d", freq->bandwidth);
> +        switch (freq->bandwidth) {
> +        case 16:
> +            cw = NL80211_CHAN_WIDTH_16;
> +            break;
> +        case 10:
> +            cw = NL80211_CHAN_WIDTH_10;
> +            break;
> +        case 8:
> +            cw = NL80211_CHAN_WIDTH_8;
> +            break;
> +        case 5:
> +            cw = NL80211_CHAN_WIDTH_5;
> +            break;
> +        case 4:
> +            cw = NL80211_CHAN_WIDTH_4;
> +            break;
> +        case 2:
> +            cw = NL80211_CHAN_WIDTH_2;
> +            break;
> +        case 1:
> +            cw = NL80211_CHAN_WIDTH_1;
> +            break;
> +        default:
> +            return -EINVAL;
> +        }

Is there a reason 5MHz and 10MHz widths are lumped together here? Also could
probably use a function like xyz_is_s1g_width or something to clean up that
if statement.

> +    if (freq->freq_khz) {
> +        unsigned int freq_offset = freq->freq_offset;
> +
> +        if (!freq_offset && freq->freq > 0 &&
> +            freq->freq_khz >= (unsigned int) freq->freq * 1000)
> +            freq_offset = freq->freq_khz - freq->freq * 1000;
> +        if (freq_offset) {
> +            wpa_printf(MSG_DEBUG, "  * freq_offset=%u", freq_offset);
> +            if (nla_put_u32(msg, NL80211_ATTR_WIPHY_FREQ_OFFSET,
> +                    freq_offset))
> +                return -ENOBUFS;
> +        }
>       }
> +
> +    if (freq->center_freq1 && freq->center_freq1_khz &&
> +        freq->center_freq1_khz >= (unsigned int) freq->center_freq1 * 
> 1000) {
> +        unsigned int cf1_offset =
> +            freq->center_freq1_khz -
> +            freq->center_freq1 * 1000;
> +
> +        if (cf1_offset) {
> +            wpa_printf(MSG_DEBUG, "  * center_freq1_offset=%u",
> +                   cf1_offset);
> +            if (nla_put_u32(msg, NL80211_ATTR_CENTER_FREQ1_OFFSET,
> +                    cf1_offset))
> +                return -ENOBUFS;
> +        }
> +    }
> +

As mentioned, these new parameters already introduce unneeded complexity.
Even still, should make use of MHZ_TO_KHZ/KHZ_TO_MHZ macros (or add them
if they don't exist).

lachlan