* [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G
@ 2026-10-02 7:29 Lachlan Hodges
2026-10-02 7:58 ` Johannes Berg
2026-10-06 7:44 ` Johannes Berg
0 siblings, 2 replies; 7+ messages in thread
From: Lachlan Hodges @ 2026-10-02 7:29 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, benjamin.berg, arien.judge, Lachlan Hodges
6d531b9af16e ("wifi: mac80211: rework RX packet handling") reworked
Rx frame handling with a specific focus on MLD, but it also introduced
some changes for non-MLD interfaces. Previously when a station was
looked up (either because the driver did not pass one or handling
a management frame) it was done via hdr->addr2 and that was it. Now
the received frames frequency is validated against the links
control channel alongside the station lookup.
S1G has a unique feature where the control or primary channel can
either be 1MHz or 2MHz. However there is _always_ a 1MHz primary.
S1G drivers advertise these 1MHz primaries but it means if a 2MHz
primary is used the control channel inside the chandef may not be
the channel used to pass management frames. As a result, the rework
causes the following two issues on an S1G link:
1. When data frames are passed to mac80211 without a station (for
example using ieee80211_rx_ni()) the frames rx status frequency
is compared against the 1MHz control frequency used by the link
via ieee80211_rx_valid_freq. Since the rx status reported is of
the center frequency of transmission (i.e either the operating
channel or the 2MHz primary in the case of multicast/EAPOL etc.)
these frames are _all_ dropped.
2. In the case of protected management frames such as ADDBA, deauths
etc. the station is always looked up so it does not matter if a
station is passed from the driver, the frequency conversion against
the 1MHz control channel will always fail and these frames will be
dropped.
For regular, unprotected management frames such as assoc requests,
auth etc. these frames don't depend on a station and are matched via
BSSID so are unaffected by this change.
To fix, we can reuse the logic implemented in 31e7681da78d ("wifi:
mac80211: correctly initialise S1G chandef for STA") where we find
the sibling 1MHz and calculate the 2MHz center frequency. For the
ocassional protected management frames this is fine, obviously it
is still ideal for drivers to pass the station directly to mac80211
such that the lookup avoids needing the extra work of finding the
2MHz center frequency.
Fixes: 6d531b9af16e ("wifi: mac80211: rework RX packet handling")
Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
A followup question:
1. is rx_status->freq always meant to be the control channel? For
management frames obviously makes sense but for data frames
sent on a wider channel wouldn't this be set to the operating
channel? Especially for i.e sniffer? The mm81x driver reports
it like this but maybe it should just report the control channel?
I guess my confusion stems from that if we have a data frame
wouldn't we compare against the chandefs frequency? Or am I
misunderstanding something?
Thanks,
lachlan
---
include/net/mac80211.h | 2 ++
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/mlme.c | 36 ++-----------------------------
net/mac80211/rx.c | 43 ++++++++++++++++++++++++++++++++------
4 files changed, 44 insertions(+), 40 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index cb9f8e14b3b6..b2a7857f9503 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1741,6 +1741,8 @@ enum mac80211_rx_encoding {
* @freq: frequency the radio was tuned to when receiving this frame, in MHz
* This field must be set for management frames, but isn't strictly needed
* for data (other) frames - for those it only affects radiotap reporting.
+ * For S1G, when operating on a 2MHz primary channel, this may be the
+ * center frequency of the 2MHz primary rather than the 1MHz primary.
* @freq_offset: @freq has a positive offset of 500Khz.
* @signal: signal strength when receiving this frame, either in dBm, in dB or
* unspecified depending on the hardware capabilities flags
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 1430527d216c..2ad93cb462ad 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2012,6 +2012,9 @@ void ieee80211_clear_fast_rx(struct sta_info *sta);
bool ieee80211_is_our_addr(struct ieee80211_sub_if_data *sdata,
const u8 *addr, int *out_link_id);
+bool __ieee80211_rx_valid_freq(struct wiphy *wiphy,
+ struct ieee80211_rx_status *status,
+ const struct cfg80211_chan_def *chandef);
/* AP code */
void ieee80211_ap_rx_queued_frame(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 3a17d1ccd32a..8a07d14b4c43 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -8091,38 +8091,6 @@ static bool ieee80211_mgd_ssid_mismatch(struct ieee80211_sub_if_data *sdata,
return memcmp(elems->ssid, cfg->ssid, cfg->ssid_len);
}
-static bool
-ieee80211_rx_beacon_freq_valid(struct ieee80211_local *local,
- struct ieee80211_mgmt *mgmt,
- struct ieee80211_rx_status *rx_status,
- struct ieee80211_chanctx_conf *chanctx)
-{
- u32 pri_2mhz_khz;
- struct ieee80211_channel *s1g_sibling_1mhz;
- u32 pri_khz = ieee80211_channel_to_khz(chanctx->def.chan);
- u32 rx_khz = ieee80211_rx_status_to_khz(rx_status);
-
- if (rx_khz == pri_khz)
- return true;
-
- if (!chanctx->def.s1g_primary_2mhz)
- return false;
-
- /*
- * If we have an S1G interface with a 2MHz primary, beacons are
- * sent on the center frequency of the 2MHz primary. Find the sibling
- * 1MHz channel and calculate the 2MHz primary center frequency.
- */
- s1g_sibling_1mhz = cfg80211_s1g_get_primary_sibling(local->hw.wiphy,
- &chanctx->def);
- if (!s1g_sibling_1mhz)
- return false;
-
- pri_2mhz_khz =
- (pri_khz + ieee80211_channel_to_khz(s1g_sibling_1mhz)) / 2;
- return rx_khz == pri_2mhz_khz;
-}
-
static void ieee80211_rx_mgmt_beacon(struct ieee80211_link_data *link,
struct ieee80211_hdr *hdr, size_t len,
struct ieee80211_rx_status *rx_status)
@@ -8176,8 +8144,8 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_link_data *link,
return;
}
- if (!ieee80211_rx_beacon_freq_valid(local, mgmt, rx_status,
- chanctx_conf)) {
+ if (!__ieee80211_rx_valid_freq(local->hw.wiphy, rx_status,
+ &chanctx_conf->def)) {
rcu_read_unlock();
return;
}
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index b3990b7a7299..9a29928a3a52 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -5318,11 +5318,41 @@ static void __ieee80211_rx_handle_8023(struct ieee80211_hw *hw,
dev_kfree_skb(skb);
}
-static bool ieee80211_rx_valid_freq(int freq, struct ieee80211_link_data *link)
+bool __ieee80211_rx_valid_freq(struct wiphy *wiphy,
+ struct ieee80211_rx_status *status,
+ const struct cfg80211_chan_def *chandef)
+{
+ u32 pri_khz = ieee80211_channel_to_khz(chandef->chan);
+ u32 rx_khz = ieee80211_rx_status_to_khz(status);
+ struct ieee80211_channel *sibling;
+
+ if (rx_khz == pri_khz)
+ return true;
+
+ /* Any non-S1G case from here is not a valid freq */
+ if (!cfg80211_chandef_is_s1g(chandef))
+ return false;
+
+ if (!chandef->s1g_primary_2mhz)
+ return false;
+
+ /*
+ * Find the sibling 1MHz channel of the 2MHz primary to calculate
+ * the 2MHz primary center frequency.
+ */
+ sibling = cfg80211_s1g_get_primary_sibling(wiphy, chandef);
+ if (!sibling)
+ return false;
+
+ return rx_khz == (pri_khz + ieee80211_channel_to_khz(sibling)) / 2;
+}
+
+static bool ieee80211_rx_valid_freq(struct ieee80211_rx_status *status,
+ struct ieee80211_link_data *link)
{
struct ieee80211_chanctx_conf *conf;
- if (!freq || link->sdata->vif.type == NL80211_IFTYPE_NAN ||
+ if (!status->freq || link->sdata->vif.type == NL80211_IFTYPE_NAN ||
link->sdata->vif.type == NL80211_IFTYPE_NAN_DATA)
return true;
@@ -5330,7 +5360,8 @@ static bool ieee80211_rx_valid_freq(int freq, struct ieee80211_link_data *link)
if (!conf || !conf->def.chan)
return false;
- return freq == conf->def.chan->center_freq;
+ return __ieee80211_rx_valid_freq(link->sdata->local->hw.wiphy, status,
+ &conf->def);
}
/*
@@ -5430,7 +5461,7 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
continue;
link = &sta->sdata->deflink;
- if (!ieee80211_rx_valid_freq(status->freq, link))
+ if (!ieee80211_rx_valid_freq(status, link))
continue;
if (rx_data_pending) {
@@ -5454,7 +5485,7 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
sdata = sta->sdata;
link = rcu_dereference(sdata->link[link_sta->link_id]);
- if (!ieee80211_rx_valid_freq(status->freq, link))
+ if (!ieee80211_rx_valid_freq(status, link))
continue;
if (rx_data_pending) {
@@ -5524,7 +5555,7 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
* RX using the station.
*/
if (link_sta && link &&
- ieee80211_rx_valid_freq(status->freq, link)) {
+ ieee80211_rx_valid_freq(status, link)) {
if (rx_data_pending) {
ieee80211_prepare_and_rx_handle(&rx, skb, false);
rx_data_pending = false;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G
2026-10-02 7:29 [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G Lachlan Hodges
@ 2026-10-02 7:58 ` Johannes Berg
2026-10-02 8:11 ` Johannes Berg
2026-10-02 9:54 ` Lachlan Hodges
2026-10-06 7:44 ` Johannes Berg
1 sibling, 2 replies; 7+ messages in thread
From: Johannes Berg @ 2026-10-02 7:58 UTC (permalink / raw)
To: Lachlan Hodges; +Cc: linux-wireless, benjamin.berg, arien.judge
On Fri, 2026-10-02 at 17:29 +1000, Lachlan Hodges wrote:
> 6d531b9af16e ("wifi: mac80211: rework RX packet handling") reworked
> Rx frame handling with a specific focus on MLD, but it also introduced
> some changes for non-MLD interfaces. Previously when a station was
> looked up (either because the driver did not pass one or handling
> a management frame) it was done via hdr->addr2 and that was it. Now
> the received frames frequency is validated against the links
> control channel alongside the station lookup.
No surprise, I guess, but sorry.
> S1G has a unique feature where the control or primary channel can
> either be 1MHz or 2MHz. However there is _always_ a 1MHz primary.
> S1G drivers advertise these 1MHz primaries but it means if a 2MHz
> primary is used the control channel inside the chandef may not be
> the channel used to pass management frames.
Yeah, so I think part of this is also related to how we through the 1
MHz vs. 2 MHz in S1G. I feel like the spec thinks this differently,
while in the chandef we basically think it almost as if 2 MHz was
equivalent to 40 MHz HT, combined out of two 1 MHz channels, except we
added the flag:
* @s1g_primary_2mhz: Indicates if the control channel pointed to
* by 'chan' exists as a 1MHz primary subchannel within an
* S1G 2MHz primary channel.
To also answer your question first:
> is rx_status->freq always meant to be the control channel?
For HT/..., yes, that's how it works. Not sure that's really *by design*
as much as by historical accident, but I don't think we will (even can)
change it now.
In some way it makes sense though because otherwise you'd see this
flicker around all the time as frames with different bandwidths are
transmitted, which would be strange too in a single BSS.
Note it also affects how radiotap is reported, so S1G might be in the
opposite camp now, wanting to report the 2 MHz channel center frequency?
> For
> management frames obviously makes sense but for data frames
> sent on a wider channel wouldn't this be set to the operating
> channel? Especially for i.e sniffer? The mm81x driver reports
> it like this but maybe it should just report the control channel?
So .. are you saying you always report the _overall_ center frequency,
even for 4/8/16 MHz widths, or just for 1 MHz vs 2 MHz primary?
I note that even S1G radiotap didn't really specify where the channel
is: https://radiotap.org/fields/S1G, so I'm not sure it can even report
everything correctly? The "Channel" field can't even cover the 1/2 MHz
centers.
So I think in some way this is almost more of a question of what you
want/need for radiotap than anything else.
Internally, reporting just the (1 MHz) "chandef primary" would be
matching the HT/... behaviour.
> As a result, the rework
> causes the following two issues on an S1G link:
>
> 1. When data frames are passed to mac80211 without a station (for
> example using ieee80211_rx_ni()) the frames rx status frequency
> is compared against the 1MHz control frequency used by the link
> via ieee80211_rx_valid_freq. Since the rx status reported is of
> the center frequency of transmission (i.e either the operating
> channel or the 2MHz primary in the case of multicast/EAPOL etc.)
> these frames are _all_ dropped.
Of course if you're using rx_ni() now then you could possibly arrange to
call ieee80211_rx_napi(hw, link_sta, skb, NULL) with the correct station
looked up beforehand, and entirely bypass the question, doing whatever
you want with the frequency?
johannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G
2026-10-02 7:58 ` Johannes Berg
@ 2026-10-02 8:11 ` Johannes Berg
2026-10-02 9:54 ` Lachlan Hodges
1 sibling, 0 replies; 7+ messages in thread
From: Johannes Berg @ 2026-10-02 8:11 UTC (permalink / raw)
To: Lachlan Hodges; +Cc: linux-wireless, benjamin.berg, arien.judge
On Fri, 2026-10-02 at 09:58 +0200, Johannes Berg wrote:
>
> Of course if you're using rx_ni() now then you could possibly arrange to
> call ieee80211_rx_napi(hw, link_sta, skb, NULL) with the correct station
> looked up beforehand, and entirely bypass the question, doing whatever
> you want with the frequency?
Oh, Benjamin says that doesn't work since we don't use it for non-data
frames - guess I should've read the code first, somehow I thought that
if the driver gives a STA we just use it anyway.
johannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G
2026-10-02 7:58 ` Johannes Berg
2026-10-02 8:11 ` Johannes Berg
@ 2026-10-02 9:54 ` Lachlan Hodges
2026-10-05 13:53 ` Johannes Berg
1 sibling, 1 reply; 7+ messages in thread
From: Lachlan Hodges @ 2026-10-02 9:54 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, benjamin.berg, arien.judge
> > S1G has a unique feature where the control or primary channel can
> > either be 1MHz or 2MHz. However there is _always_ a 1MHz primary.
> > S1G drivers advertise these 1MHz primaries but it means if a 2MHz
> > primary is used the control channel inside the chandef may not be
> > the channel used to pass management frames.
>
> Yeah, so I think part of this is also related to how we through the 1
> MHz vs. 2 MHz in S1G. I feel like the spec thinks this differently,
> while in the chandef we basically think it almost as if 2 MHz was
> equivalent to 40 MHz HT, combined out of two 1 MHz channels, except we
> added the flag:
>
> * @s1g_primary_2mhz: Indicates if the control channel pointed to
> * by 'chan' exists as a 1MHz primary subchannel within an
> * S1G 2MHz primary channel.
>
> To also answer your question first:
>
> > is rx_status->freq always meant to be the control channel?
>
> For HT/..., yes, that's how it works. Not sure that's really *by design*
> as much as by historical accident, but I don't think we will (even can)
> change it now.
>
> In some way it makes sense though because otherwise you'd see this
> flicker around all the time as frames with different bandwidths are
> transmitted, which would be strange too in a single BSS.
>
> Note it also affects how radiotap is reported, so S1G might be in the
> opposite camp now, wanting to report the 2 MHz channel center frequency?
Yea that is true, Ill respond to radiotap stuff below.
> > For
> > management frames obviously makes sense but for data frames
> > sent on a wider channel wouldn't this be set to the operating
> > channel? Especially for i.e sniffer? The mm81x driver reports
> > it like this but maybe it should just report the control channel?
>
> So .. are you saying you always report the _overall_ center frequency,
> even for 4/8/16 MHz widths, or just for 1 MHz vs 2 MHz primary?
Yes, our firmware reports the frame at the overall center frequency,
even for 4/8/16 MHz widths. Looking at our out of tree shim driver
(5G mapping) it does the same thing i.e report the center frequency
of 40/80/160 channel which is probably why mm81x does this.
Anyways this is not really a big deal knowing that we can just
override it with the 1MHz control channel.
> I note that even S1G radiotap didn't really specify where the channel
> is: https://radiotap.org/fields/S1G, so I'm not sure it can even report
> everything correctly? The "Channel" field can't even cover the 1/2 MHz
> centers.
Yea this is on the TODO list.. Once initial monitor mode support lands,
which I was going to send today but this bug has appeared, then we'll
be fixing up the radiotap standard to better convey everything.
> So I think in some way this is almost more of a question of what you
> want/need for radiotap than anything else.
So another question then, Im looking at a poorly sourced 802.11ac
wireshark capture and in the radiotap header it has:
802.11 radio information
PHY type: 802.11ac (VHT) (8)
Short GI: False
Bandwidth: 80 MHz (4)
TXOP_PS_NOT_ALLOWED: False
User 0: MCS 7
Data rate: 292.5 Mb/s
Channel: 36
Frequency: 5180MHz
Signal strength (dBm): -40 dBm
Noise level (dBm): -96 dBm
Signal/noise ratio (dB): 56 dB
TSF timestamp: 1911262072856970
[Duration: 53µs]
Where this is just a QoS data frame.. channel 36 is a 20MHz control
channel @ 5180MHz and a width of 80MHz. I am assuming then, that if
we were to follow this (as mentioned above) in the radiotap portion
it would show as the 1MHz control (for channel + freq) and the width
being the operating 4/8/.. ?
Just a bit different to how I've learnt but that should be fine :)
> > As a result, the rework
> > causes the following two issues on an S1G link:
> >
> > 1. When data frames are passed to mac80211 without a station (for
> > example using ieee80211_rx_ni()) the frames rx status frequency
> > is compared against the 1MHz control frequency used by the link
> > via ieee80211_rx_valid_freq. Since the rx status reported is of
> > the center frequency of transmission (i.e either the operating
> > channel or the 2MHz primary in the case of multicast/EAPOL etc.)
> > these frames are _all_ dropped.
>
> Of course if you're using rx_ni() now then you could possibly arrange to
> call ieee80211_rx_napi(hw, link_sta, skb, NULL) with the correct station
> looked up beforehand, and entirely bypass the question, doing whatever
> you want with the frequency?
Yea, as you mentioned in the other email that works for data but
management frames and is something I tried first before running into
the ADDBA problems.
Anyways, it seems like a small patch to handle freq_offset and then
just changing the driver to report the 1MHz primary should hopefully
fix all this up so Ill send that once I confirm.
Thanks,
lachlan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G
2026-10-02 9:54 ` Lachlan Hodges
@ 2026-10-05 13:53 ` Johannes Berg
2026-10-06 6:06 ` Lachlan Hodges
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2026-10-05 13:53 UTC (permalink / raw)
To: Lachlan Hodges; +Cc: linux-wireless, benjamin.berg, arien.judge
On Fri, 2026-10-02 at 19:54 +1000, Lachlan Hodges wrote:
> >
> > > For
> > > management frames obviously makes sense but for data frames
> > > sent on a wider channel wouldn't this be set to the operating
> > > channel? Especially for i.e sniffer? The mm81x driver reports
> > > it like this but maybe it should just report the control channel?
> >
> > So .. are you saying you always report the _overall_ center frequency,
> > even for 4/8/16 MHz widths, or just for 1 MHz vs 2 MHz primary?
>
> Yes, our firmware reports the frame at the overall center frequency,
> even for 4/8/16 MHz widths. Looking at our out of tree shim driver
> (5G mapping) it does the same thing i.e report the center frequency
> of 40/80/160 channel which is probably why mm81x does this.
I guess that makes some sense.
> Anyways this is not really a big deal knowing that we can just
> override it with the 1MHz control channel.
Sure, but should you?
> > I note that even S1G radiotap didn't really specify where the channel
> > is: https://radiotap.org/fields/S1G, so I'm not sure it can even report
> > everything correctly? The "Channel" field can't even cover the 1/2 MHz
> > centers.
>
> Yea this is on the TODO list.. Once initial monitor mode support lands,
> which I was going to send today but this bug has appeared, then we'll
> be fixing up the radiotap standard to better convey everything.
:)
> > So I think in some way this is almost more of a question of what you
> > want/need for radiotap than anything else.
>
> So another question then, Im looking at a poorly sourced 802.11ac
> wireshark capture and in the radiotap header it has:
>
> 802.11 radio information
> PHY type: 802.11ac (VHT) (8)
> Short GI: False
> Bandwidth: 80 MHz (4)
> TXOP_PS_NOT_ALLOWED: False
> User 0: MCS 7
> Data rate: 292.5 Mb/s
> Channel: 36
> Frequency: 5180MHz
> Signal strength (dBm): -40 dBm
> Noise level (dBm): -96 dBm
> Signal/noise ratio (dB): 56 dB
> TSF timestamp: 1911262072856970
> [Duration: 53µs]
>
> Where this is just a QoS data frame.. channel 36 is a 20MHz control
> channel @ 5180MHz and a width of 80MHz. I am assuming then, that if
> we were to follow this (as mentioned above) in the radiotap portion
> it would show as the 1MHz control (for channel + freq) and the width
> being the operating 4/8/.. ?
I don't know - I believe that "802.11 radio information" is synthesised
by wireshark from the other fields. It'd have to actually implement that
first, presumably by parsing some S1G data.
But once that's there I guess yes, that's what it'd show. You could also
just have it synthesise it differently I guess, or take only things from
some (extended) S1G field, or ... any number of things?
> Anyways, it seems like a small patch to handle freq_offset and then
> just changing the driver to report the 1MHz primary should hopefully
> fix all this up so Ill send that once I confirm.
If that's OK then I guess might be simplest overall.
johannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G
2026-10-05 13:53 ` Johannes Berg
@ 2026-10-06 6:06 ` Lachlan Hodges
0 siblings, 0 replies; 7+ messages in thread
From: Lachlan Hodges @ 2026-10-06 6:06 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, benjamin.berg, arien.judge
On Mon, Oct 05, 2026 at 03:53:01PM +0200, Johannes Berg wrote:
> On Fri, 2026-10-02 at 19:54 +1000, Lachlan Hodges wrote:
> > >
> > > > For
> > > > management frames obviously makes sense but for data frames
> > > > sent on a wider channel wouldn't this be set to the operating
> > > > channel? Especially for i.e sniffer? The mm81x driver reports
> > > > it like this but maybe it should just report the control channel?
> > >
> > > So .. are you saying you always report the _overall_ center frequency,
> > > even for 4/8/16 MHz widths, or just for 1 MHz vs 2 MHz primary?
> >
> > Yes, our firmware reports the frame at the overall center frequency,
> > even for 4/8/16 MHz widths. Looking at our out of tree shim driver
> > (5G mapping) it does the same thing i.e report the center frequency
> > of 40/80/160 channel which is probably why mm81x does this.
>
> I guess that makes some sense.
>
> > Anyways this is not really a big deal knowing that we can just
> > override it with the 1MHz control channel.
>
> Sure, but should you?
Okay so I did some more testing (sorry for all the back and forth :))
and I think hard coding the 1MHz control actually has some tricky
edge cases such as scanning while passing traffic, since while we are
scanning we'd need to pass the correct channel we received the probe
response on (since we do hwscan).
Additionally just thinking about the problem as a whole, it would be
more correct to just let mac80211 actually handle this whole scenario
of 1 vs 2MHz primary since after all they are both "control" channels.
We already do this for beacons so moving the logic such that all
management frames are handled makes sense to me. Then for data frames
the station can just be passed so this is bypassed anyway.
Let me know what you think, maybe re-review the original patch and I
can resend if needed.
Thanks,
lachlan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G
2026-10-02 7:29 [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G Lachlan Hodges
2026-10-02 7:58 ` Johannes Berg
@ 2026-10-06 7:44 ` Johannes Berg
1 sibling, 0 replies; 7+ messages in thread
From: Johannes Berg @ 2026-10-06 7:44 UTC (permalink / raw)
To: Lachlan Hodges; +Cc: linux-wireless, benjamin.berg, arien.judge
On Fri, 2026-10-02 at 17:29 +1000, Lachlan Hodges wrote:
>
> +++ b/include/net/mac80211.h
> @@ -1741,6 +1741,8 @@ enum mac80211_rx_encoding {
> * @freq: frequency the radio was tuned to when receiving this frame, in MHz
> * This field must be set for management frames, but isn't strictly needed
> * for data (other) frames - for those it only affects radiotap reporting.
> + * For S1G, when operating on a 2MHz primary channel, this may be the
> + * center frequency of the 2MHz primary rather than the 1MHz primary.
Should it really say "may be"? Vs. something more specific about being
1/2 MHz center depending on how it was transmitted/received?
> -static bool ieee80211_rx_valid_freq(int freq, struct ieee80211_link_data *link)
> +bool __ieee80211_rx_valid_freq(struct wiphy *wiphy,
> + struct ieee80211_rx_status *status,
> + const struct cfg80211_chan_def *chandef)
> +{
> + u32 pri_khz = ieee80211_channel_to_khz(chandef->chan);
> + u32 rx_khz = ieee80211_rx_status_to_khz(status);
> + struct ieee80211_channel *sibling;
> +
> + if (rx_khz == pri_khz)
> + return true;
This is also in the original, but now it's called more I think, might
make sense to check them before conversion to kHz? But not sure what the
compiler would do here for the *1000.
> + /* Any non-S1G case from here is not a valid freq */
> + if (!cfg80211_chandef_is_s1g(chandef))
> + return false;
> +
> + if (!chandef->s1g_primary_2mhz)
> + return false;
> +
> + /*
> + * Find the sibling 1MHz channel of the 2MHz primary to calculate
> + * the 2MHz primary center frequency.
> + */
> + sibling = cfg80211_s1g_get_primary_sibling(wiphy, chandef);
> + if (!sibling)
> + return false;
> +
> + return rx_khz == (pri_khz + ieee80211_channel_to_khz(sibling)) / 2;
All of this gets really complex, IMHO.
Maybe here's another thought: we have
u16 freq: 13, freq_offset: 1;
What if we make that
u16 freq: 13, freq_offset: 1,
s1g_width_2mhz: 1;
(or something, handwaving about the name) and ask that the driver puts
the 1 MHz center frequency into freq/_offset (so mac80211's comparison
on RX is just ==), but we can still get back the right frequency to
report further out (scan, radiotap) again? Or use two bits (above/below)
then we don't even need to have the sibling channel lookup (we'd just
believe the driver).
I don't know, I'm just thinking out loud, but I feel like maybe that'd
make it more likely we don't break it (again) when we have HT/VHT/etc.
in mind?
johannes
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-06 7:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-02 7:29 [PATCH RFC wireless-next] wifi: mac80211: correct rx freq handling for S1G Lachlan Hodges
2026-10-02 7:58 ` Johannes Berg
2026-10-02 8:11 ` Johannes Berg
2026-10-02 9:54 ` Lachlan Hodges
2026-10-05 13:53 ` Johannes Berg
2026-10-06 6:06 ` Lachlan Hodges
2026-10-06 7:44 ` Johannes Berg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.