| Message ID | 20230821105903.7482379cde47.Ib72645d02fadc24b520db118abd82e861c87316e@changeid |
|---|---|
| State | New |
| Headers | show |
| Series | Revert "mac80211: add parse regulatory info in 6 GHz operation information" | expand |
On Mon, 2023-08-21 at 10:59 +0200, Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > > This reverts commit cb751b7a57e5 ("mac80211: add parse regulatory info > in 6 GHz operation information") which added a station type bss_conf > assignment in a parsing helper function, which will corrupt mesh data. > Ah crap this won't work, rtw89 already uses this. Wen please send a fix for this ASAP. johannes
On 8/21/2023 7:34 PM, Johannes Berg wrote: > On Mon, 2023-08-21 at 19:11 +0800, Wen Gong wrote: >> On 8/21/2023 6:57 PM, Johannes Berg wrote: >>> On Mon, 2023-08-21 at 18:36 +0800, Wen Gong wrote: >>>> On 8/21/2023 5:06 PM, Johannes Berg wrote: >>>>> On Mon, 2023-08-21 at 10:59 +0200, Johannes Berg wrote: >>>>>> From: Johannes Berg <johannes.berg@intel.com> >>>>>> >>>>>> This reverts commit cb751b7a57e5 ("mac80211: add parse regulatory info >>>>>> in 6 GHz operation information") which added a station type bss_conf >>>>>> assignment in a parsing helper function, which will corrupt mesh data. >>>>>> >>>>> Ah crap this won't work, rtw89 already uses this. >>>>> >>>>> Wen please send a fix for this ASAP. >>>>> >>>>> johannes >>>> Hi Johannes, >>>> >>>> I looked the patch some times, but I do not know how it corrupt mesh data, >>>> >>>> Is there any clue for me? >>> Hah, no, I'm wrong ... I looked at it and for some reason thought of >>> u.mgd instead of vif.bss_conf. Sorry! >>> >>> Still it's not correct though to write to vif.bss_conf in this function >>> because it's called from mesh_matches_local() to see if it's even >>> compatible, for example, so the mere calling a "give me the 6 GHz >>> chandef" function doesn't indicate we actually are going to use it now. >> Do you mean mesh_matches_local() is only a try to call >> ieee80211_chandef_he_6ghz_oper(), >> >> NOT real use the 6 GHz chandef? > Yes, I believe so. > > Anyway it would seem better for a utility function to have clearer > defined output, I think? > > johannes Do you mean move the logic of set bss_conf->power_type to another new function from ieee80211_chandef_he_6ghz_oper()?
On Tue, 2023-08-22 at 14:17 +0800, Wen Gong wrote: > > > > Anyway it would seem better for a utility function to have clearer > > defined output, I think? > > > > johannes > > Do you mean move the logic of set bss_conf->power_type to another new > function from > > ieee80211_chandef_he_6ghz_oper()? > No, not really. But we could have a pointer to the bss_conf or even the particular value, as an obvious output from the function. johannes
Hi Johannes,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.5-rc7 next-20230823]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/Revert-mac80211-add-parse-regulatory-info-in-6-GHz-operation-information/20230821-170019
base: linus/master
patch link: https://lore.kernel.org/r/20230821105903.7482379cde47.Ib72645d02fadc24b520db118abd82e861c87316e%40changeid
patch subject: [PATCH] Revert "mac80211: add parse regulatory info in 6 GHz operation information"
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230824/202308240547.O8ZxrrXI-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230824/202308240547.O8ZxrrXI-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308240547.O8ZxrrXI-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/net/wireless/realtek/rtw89/regd.c: In function 'rtw89_reg_6ghz_power_recalc':
>> drivers/net/wireless/realtek/rtw89/regd.c:514:39: error: 'struct ieee80211_bss_conf' has no member named 'power_type'; did you mean 'txpower_type'?
514 | switch (vif->bss_conf.power_type) {
| ^~~~~~~~~~
| txpower_type
vim +514 drivers/net/wireless/realtek/rtw89/regd.c
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 505
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 506 void rtw89_reg_6ghz_power_recalc(struct rtw89_dev *rtwdev,
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 507 struct rtw89_vif *rtwvif, bool active)
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 508 {
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 509 struct ieee80211_vif *vif = rtwvif_to_vif(rtwvif);
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 510
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 511 lockdep_assert_held(&rtwdev->mutex);
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 512
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 513 if (active) {
f6baa1d3d5703f Zong-Zhe Yang 2023-06-02 @514 switch (vif->bss_conf.power_type) {
diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 3a8a2d2c58c3..813d4a654470 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -637,7 +637,6 @@ struct ieee80211_fils_discovery { * interval. * @beacon_tx_rate: The configured beacon transmit rate that needs to be passed * to driver when rate control is offloaded to firmware. - * @power_type: power type of BSS for 6 GHz * @tx_pwr_env: transmit power envelope array of BSS. * @tx_pwr_env_num: number of @tx_pwr_env. * @pwr_reduction: power constraint of BSS. @@ -746,7 +745,6 @@ struct ieee80211_bss_conf { struct ieee80211_fils_discovery fils_discovery; u32 unsol_bcast_probe_resp_interval; struct cfg80211_bitrate_mask beacon_tx_rate; - enum ieee80211_ap_reg_power power_type; struct ieee80211_tx_pwr_env tx_pwr_env[IEEE80211_TPE_MAX_IE_COUNT]; u8 tx_pwr_env_num; u8 pwr_reduction; diff --git a/net/mac80211/util.c b/net/mac80211/util.c index 8a6917cf63cf..7bd3b64ddac6 100644 --- a/net/mac80211/util.c +++ b/net/mac80211/util.c @@ -3841,7 +3841,6 @@ bool ieee80211_chandef_he_6ghz_oper(struct ieee80211_sub_if_data *sdata, const struct ieee80211_sta_eht_cap *eht_cap; struct cfg80211_chan_def he_chandef = *chandef; const struct ieee80211_he_6ghz_oper *he_6ghz_oper; - struct ieee80211_bss_conf *bss_conf = &sdata->vif.bss_conf; bool support_80_80, support_160, support_320; u8 he_phy_cap, eht_phy_cap; u32 freq; @@ -3894,19 +3893,6 @@ bool ieee80211_chandef_he_6ghz_oper(struct ieee80211_sub_if_data *sdata, NL80211_BAND_6GHZ); he_chandef.chan = ieee80211_get_channel(sdata->local->hw.wiphy, freq); - switch (u8_get_bits(he_6ghz_oper->control, - IEEE80211_HE_6GHZ_OPER_CTRL_REG_INFO)) { - case IEEE80211_6GHZ_CTRL_REG_LPI_AP: - bss_conf->power_type = IEEE80211_REG_LPI_AP; - break; - case IEEE80211_6GHZ_CTRL_REG_SP_AP: - bss_conf->power_type = IEEE80211_REG_SP_AP; - break; - default: - bss_conf->power_type = IEEE80211_REG_UNSET_AP; - break; - } - if (!eht_oper || !(eht_oper->params & IEEE80211_EHT_OPER_INFO_PRESENT)) { switch (u8_get_bits(he_6ghz_oper->control,