Hi:
Thanks for your reply!
On 11/23/2022 12:43 AM, Johannes Berg wrote:
On Mon, 2022-11-21 at 15:29 +0800, Kang Yang wrote:
Hi:
1.Do you have any new version about this RFC patch?
Not really, no.
2.I have some questions about this patch:
(a couple of blank lines and some trimming of the context really would
help ... please try next time)
+static u16
+ieee80211_extract_dis_subch_bmap(const struct ieee80211_eht_operation *eht_oper,
+ struct cfg80211_chan_def *chandef, u16 bitmap)
+{
+ int sta_center_freq = ieee80211_channel_to_frequency(eht_oper->ccfs,
+ chandef->chan->band);
+ u32 center_freq = chandef->chan->center_freq;
The shift is calculated by the difference of old and new channel center
frequency.The new channel center frequency should be
"chandef->center_freq1" after BW negotitaion.
"chandef->chan->center_freq" is the primary channel frequency.
Yeah I think we did fix a couple of bugs in this area later.
+ u8 sta_bw = 20 * BIT(u8_get_bits(eht_oper->chan_width,
+ IEEE80211_EHT_OPER_CHAN_WIDTH));
+ u8 bw = 20 * BIT(ieee80211_chan_width_to_rx_bw(chandef->width));
+ int sta_start_freq = sta_center_freq - sta_bw / 2;
+ int start_freq = center_freq - bw / 2;
+ u16 shift = (start_freq - sta_start_freq) / 20;
+ u16 mask = BIT(sta_bw / 20) - 1;
The mask is used to extra the valid bit according to the new BW,
but current algorithm is using the old bandwidth.
:)
+ while (chandef->width > NL80211_CHAN_WIDTH_40) {
+ extracted =
+ ieee80211_extract_dis_subch_bmap(eht_oper, chandef,
+ bitmap);
+
+ if (ieee80211_valid_disable_subchannel_bitmap(&bitmap,
+ chandef->width))
Here extract the bitmap according new negotiated BW. After extracting,
check whether it is valid.
I think you should use "&extracted" instead of "&bitmap"
Yeah I guess that makes sense.
I don't know what fixes we have and what version of this patch this is.
+static bool ieee80211_config_puncturing(struct ieee80211_sub_if_data *sdata,
+ const struct ieee80211_eht_operation *eht_oper,
+ u64 *changed)
+{
+ u16 bitmap, extracted;
+ u8 bw;
+
+ if (!u8_get_bits(eht_oper->present_bm,
+ IEEE80211_EHT_OPER_DISABLED_SUBCHANNEL_BITMAP_PRESENT))
+ bitmap = 0;
+ else
+ bitmap = get_unaligned_le16(eht_oper->disable_subchannel_bitmap);
+
Should check initial bitmap here.
What do you mean by "initial" here?
"Initial bitmap" is the original bitmap that AP send in beacon . STA
will extract it according to the negotiated BW. So i call it "Initial
bitmap".
This is my personal statement.
+ extracted = ieee80211_extract_dis_subch_bmap(eht_oper,
+ &sdata->vif.bss_conf.chandef,
+ bitmap);
+
+ /* accept if there are no changes */
+ if (!(*changed & BSS_CHANGED_BANDWIDTH) &&
+ extracted == sdata->vif.bss_conf.eht_puncturing)
+ return true;
+
+ bw = u8_get_bits(eht_oper->chan_width, IEEE80211_EHT_OPER_CHAN_WIDTH);
+
+ if (!ieee80211_valid_disable_subchannel_bitmap(&bitmap, bw)) {
+ sdata_info(sdata,
+ "Got an invalid disable subchannel bitmap from AP %pM: bitmap = 0x%x, bw = 0x%x. disconnect\n",
+ sdata->u.mgd.associated->bssid, bitmap, bw);
+ return false;
+ }
The initial bitmap received from the AP is checked here.
I think it should be carried out before the extraction above.
Ah, yes I guess that makes sense.
Anyway the more fundamental thing we have to figure out here (and thanks
for bringing this back) is how we treat the puncturing - QCOM's AP-side
puncturing patch treated it as part of the chandef, but that's not
working well for client side ...
Yes, to my understanding, I think it's more appropriate to define it
like you in "ieee80211_bss_conf".
But this should be discussed by people like you. I'm just responsible
for developing according to these definitions.
johannes
My question is over.
Thanks again for your patient reply.
Best regard
Kang
